Hi, Just so everyone knows, this commit fixed wrong render results which we were experiencing in some of the benchmark scenes [1]. Quite happy with that.
So big kudos to Thomas for identifying the root of the issue :) [1] https://docs.google.com/spreadsheets/d/1rybGWiISHtgaUI-E_DIOM0wf6DW5UG1-p1ooizHimUI/edit?ts=56d095bd#gid=0 On Mon, May 23, 2016 at 2:12 PM, Sergey Sharybin <[email protected]> wrote: > Commit: 2aa4b6045a1d249025bb8eb2f19fcc72d0739341 > Author: Sergey Sharybin > Date: Mon May 23 14:09:27 2016 +0200 > Branches: master > https://developer.blender.org/rB2aa4b6045a1d249025bb8eb2f19fcc72d0739341 > > Cycles: Fix wrong closure counter in feature adaptive kernel > > Some closures were missing from calculation, leading to an array > under-allocation, presumable causing memory corruption issues with > emission shaders on OpenCL and was causing issues with Volume 3D > textures with CUDA. > > The issue was identified by Thomas Dinges, the patch is different > from the original D2006. See the brief discussion there. Current > approach is similar (or the same) as Brecht suggested. > > =================================================================== > > M intern/cycles/kernel/svm/svm_types.h > M intern/cycles/render/graph.cpp > M intern/cycles/render/graph.h > M intern/cycles/render/nodes.h > > =================================================================== > > diff --git a/intern/cycles/kernel/svm/svm_types.h > b/intern/cycles/kernel/svm/svm_types.h > index 8c69c58..be87e35 100644 > --- a/intern/cycles/kernel/svm/svm_types.h > +++ b/intern/cycles/kernel/svm/svm_types.h > @@ -370,6 +370,9 @@ typedef enum ShaderType { > /* Closure */ > > typedef enum ClosureType { > + /* Special type, flags generic node as a non-BSDF. */ > + CLOSURE_NONE_ID, > + > CLOSURE_BSDF_ID, > > /* Diffuse */ > diff --git a/intern/cycles/render/graph.cpp > b/intern/cycles/render/graph.cpp > index 15c89cc..24e4c9f 100644 > --- a/intern/cycles/render/graph.cpp > +++ b/intern/cycles/render/graph.cpp > @@ -984,17 +984,18 @@ int ShaderGraph::get_num_closures() > { > int num_closures = 0; > foreach(ShaderNode *node, nodes) { > - if(node->special_type == SHADER_SPECIAL_TYPE_CLOSURE) { > - BsdfNode *bsdf_node = static_cast<BsdfNode*>(node); > - /* TODO(sergey): Make it more generic approach, > maybe some utility > - * macros like CLOSURE_IS_FOO()? > - */ > - if(CLOSURE_IS_BSSRDF(bsdf_node->closure)) > - num_closures = num_closures + 3; > - else if(CLOSURE_IS_GLASS(bsdf_node->closure)) > - num_closures = num_closures + 2; > - else > - num_closures = num_closures + 1; > + ClosureType closure_type = node->get_closure_type(); > + if(closure_type == CLOSURE_NONE_ID) { > + continue; > + } > + else if(CLOSURE_IS_BSSRDF(closure_type)) { > + num_closures += 3; > + } > + else if(CLOSURE_IS_GLASS(closure_type)) { > + num_closures += 2; > + } > + else { > + ++num_closures; > } > } > return num_closures; > diff --git a/intern/cycles/render/graph.h b/intern/cycles/render/graph.h > index b1ebdbf..bd3f5ca 100644 > --- a/intern/cycles/render/graph.h > +++ b/intern/cycles/render/graph.h > @@ -237,6 +237,9 @@ public: > */ > virtual int get_feature() { return bump == SHADER_BUMP_NONE ? 0 : > NODE_FEATURE_BUMP; } > > + /* Get closure ID to which the node compiles into. */ > + virtual ClosureType get_closure_type() { return CLOSURE_NONE_ID; } > + > /* Check whether settings of the node equals to another one. > * > * This is mainly used to check whether two nodes can be merged > diff --git a/intern/cycles/render/nodes.h b/intern/cycles/render/nodes.h > index 54a5220..5df34a8 100644 > --- a/intern/cycles/render/nodes.h > +++ b/intern/cycles/render/nodes.h > @@ -387,6 +387,7 @@ public: > > bool has_spatial_varying() { return true; } > void compile(SVMCompiler& compiler, ShaderInput *param1, > ShaderInput *param2, ShaderInput *param3 = NULL, ShaderInput *param4 = > NULL); > + virtual ClosureType get_closure_type() { return closure; } > > ClosureType closure; > bool scattering; > @@ -484,6 +485,7 @@ class EmissionNode : public ShaderNode { > public: > SHADER_NODE_CLASS(EmissionNode) > bool constant_fold(ShaderGraph *graph, ShaderOutput *socket, > float3 *optimized_value); > + virtual ClosureType get_closure_type() { return > CLOSURE_EMISSION_ID; } > > bool has_surface_emission() { return true; } > }; > @@ -492,12 +494,14 @@ class BackgroundNode : public ShaderNode { > public: > SHADER_NODE_CLASS(BackgroundNode) > bool constant_fold(ShaderGraph *graph, ShaderOutput *socket, > float3 *optimized_value); > + virtual ClosureType get_closure_type() { return > CLOSURE_BACKGROUND_ID; } > }; > > class HoldoutNode : public ShaderNode { > public: > SHADER_NODE_CLASS(HoldoutNode) > virtual int get_group() { return NODE_GROUP_LEVEL_1; } > + virtual ClosureType get_closure_type() { return > CLOSURE_HOLDOUT_ID; } > }; > > class AmbientOcclusionNode : public ShaderNode { > @@ -506,6 +510,7 @@ public: > > bool has_spatial_varying() { return true; } > virtual int get_group() { return NODE_GROUP_LEVEL_1; } > + virtual ClosureType get_closure_type() { return > CLOSURE_AMBIENT_OCCLUSION_ID; } > }; > > class VolumeNode : public ShaderNode { > @@ -517,6 +522,7 @@ public: > virtual int get_feature() { > return ShaderNode::get_feature() | NODE_FEATURE_VOLUME; > } > + virtual ClosureType get_closure_type() { return closure; } > > ClosureType closure; > > _______________________________________________ > Bf-blender-cvs mailing list > [email protected] > https://lists.blender.org/mailman/listinfo/bf-blender-cvs > -- With best regards, Sergey Sharybin _______________________________________________ Bf-committers mailing list [email protected] https://lists.blender.org/mailman/listinfo/bf-committers
