[Bf-committers] [Bf-blender-cvs] [2aa4b60] master: Cycles: Fix wrong closure counter in feature adaptive kernel

[Sergey Sharybin]

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 <noreply at git.blender.org>
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
> Bf-blender-cvs at blender.org
> https://lists.blender.org/mailman/listinfo/bf-blender-cvs
>



-- 
With best regards, Sergey Sharybin