From b1a91c99bc85dedffdc2fa9d5499edcefcc7363d Mon Sep 17 00:00:00 2001 From: Lukas Stockner Date: Mon, 2 Oct 2023 02:19:51 +0200 Subject: [PATCH] Fix #111588: Cycles: Vector displacement with adaptive subdiv breaks normal The compact form of the differential is not enough here, we need to store and use the full vectors for bump evaluation. Pull Request: https://projects.blender.org/blender/blender/pulls/112987 --- intern/cycles/kernel/svm/bump.h | 4 ++++ intern/cycles/kernel/svm/displace.h | 18 +++++++++++++++--- intern/cycles/kernel/svm/types.h | 2 +- intern/cycles/scene/shader_nodes.cpp | 21 +++++++++++---------- intern/cycles/scene/svm.cpp | 7 ++++--- intern/cycles/scene/svm.h | 5 +++++ 6 files changed, 40 insertions(+), 17 deletions(-) diff --git a/intern/cycles/kernel/svm/bump.h b/intern/cycles/kernel/svm/bump.h index 4470cc3a94e..033bc410241 100644 --- a/intern/cycles/kernel/svm/bump.h +++ b/intern/cycles/kernel/svm/bump.h @@ -30,6 +30,10 @@ ccl_device_noinline void svm_node_enter_bump_eval(KernelGlobals kg, sd->P = P; sd->dP = differential_make_compact(dP); + + /* Save the full differential, the compact form isn't enough for svm_node_set_bump. */ + stack_store_float3(stack, offset + 4, dP.dx); + stack_store_float3(stack, offset + 7, dP.dy); } } diff --git a/intern/cycles/kernel/svm/displace.h b/intern/cycles/kernel/svm/displace.h index ba4aface005..caee25308b2 100644 --- a/intern/cycles/kernel/svm/displace.h +++ b/intern/cycles/kernel/svm/displace.h @@ -15,6 +15,9 @@ ccl_device_noinline void svm_node_set_bump(KernelGlobals kg, ccl_private float *stack, uint4 node) { + uint out_offset, bump_state_offset, dummy; + svm_unpack_node_uchar4(node.w, &out_offset, &bump_state_offset, &dummy, &dummy); + #ifdef __RAY_DIFFERENTIALS__ IF_KERNEL_NODES_FEATURE(BUMP) { @@ -25,7 +28,16 @@ ccl_device_noinline void svm_node_set_bump(KernelGlobals kg, float3 normal_in = stack_valid(normal_offset) ? stack_load_float3(stack, normal_offset) : sd->N; - differential3 dP = differential_from_compact(sd->Ng, sd->dP); + /* If we have saved bump state, read the full differential from there. + * Just using the compact form in those cases leads to incorrect normals (see #111588). */ + differential3 dP; + if (bump_state_offset == SVM_STACK_INVALID) { + dP = differential_from_compact(sd->Ng, sd->dP); + } + else { + dP.dx = stack_load_float3(stack, bump_state_offset + 4); + dP.dy = stack_load_float3(stack, bump_state_offset + 7); + } if (use_object_space) { object_inverse_normal_transform(kg, sd, &normal_in); @@ -72,10 +84,10 @@ ccl_device_noinline void svm_node_set_bump(KernelGlobals kg, object_normal_transform(kg, sd, &normal_out); } - stack_store_float3(stack, node.w, normal_out); + stack_store_float3(stack, out_offset, normal_out); } else { - stack_store_float3(stack, node.w, zero_float3()); + stack_store_float3(stack, out_offset, zero_float3()); } #endif } diff --git a/intern/cycles/kernel/svm/types.h b/intern/cycles/kernel/svm/types.h index 59f2cbc3ef1..9e26e7bba10 100644 --- a/intern/cycles/kernel/svm/types.h +++ b/intern/cycles/kernel/svm/types.h @@ -13,7 +13,7 @@ CCL_NAMESPACE_BEGIN /* SVM stack offsets with this value indicate that it's not on the stack */ #define SVM_STACK_INVALID 255 -#define SVM_BUMP_EVAL_STATE_SIZE 4 +#define SVM_BUMP_EVAL_STATE_SIZE 10 /* Nodes */ diff --git a/intern/cycles/scene/shader_nodes.cpp b/intern/cycles/scene/shader_nodes.cpp index fa6deab7420..4547014b404 100644 --- a/intern/cycles/scene/shader_nodes.cpp +++ b/intern/cycles/scene/shader_nodes.cpp @@ -6726,16 +6726,17 @@ void BumpNode::compile(SVMCompiler &compiler) ShaderOutput *normal_out = output("Normal"); /* pack all parameters in the node */ - compiler.add_node(NODE_SET_BUMP, - compiler.encode_uchar4(compiler.stack_assign_if_linked(normal_in), - compiler.stack_assign(distance_in), - invert, - use_object_space), - compiler.encode_uchar4(compiler.stack_assign(center_in), - compiler.stack_assign(dx_in), - compiler.stack_assign(dy_in), - compiler.stack_assign(strength_in)), - compiler.stack_assign(normal_out)); + compiler.add_node( + NODE_SET_BUMP, + compiler.encode_uchar4(compiler.stack_assign_if_linked(normal_in), + compiler.stack_assign(distance_in), + invert, + use_object_space), + compiler.encode_uchar4(compiler.stack_assign(center_in), + compiler.stack_assign(dx_in), + compiler.stack_assign(dy_in), + compiler.stack_assign(strength_in)), + compiler.encode_uchar4(compiler.stack_assign(normal_out), compiler.get_bump_state_offset())); } void BumpNode::compile(OSLCompiler &compiler) diff --git a/intern/cycles/scene/svm.cpp b/intern/cycles/scene/svm.cpp index 8d729f779a1..5d3527129eb 100644 --- a/intern/cycles/scene/svm.cpp +++ b/intern/cycles/scene/svm.cpp @@ -163,6 +163,7 @@ SVMCompiler::SVMCompiler(Scene *scene) : scene(scene) current_graph = NULL; background = false; mix_weight_offset = SVM_STACK_INVALID; + bump_state_offset = SVM_STACK_INVALID; compile_failed = false; /* This struct has one entry for every node, in order of ShaderNodeType definition. */ @@ -784,9 +785,8 @@ void SVMCompiler::compile_type(Shader *shader, ShaderGraph *graph, ShaderType ty } /* for the bump shader we need add a node to store the shader state */ - bool need_bump_state = (type == SHADER_TYPE_BUMP) && - (shader->get_displacement_method() == DISPLACE_BOTH); - int bump_state_offset = SVM_STACK_INVALID; + const bool need_bump_state = (type == SHADER_TYPE_BUMP) && + (shader->get_displacement_method() == DISPLACE_BOTH); if (need_bump_state) { bump_state_offset = stack_find_offset(SVM_BUMP_EVAL_STATE_SIZE); add_node(NODE_ENTER_BUMP_EVAL, bump_state_offset); @@ -846,6 +846,7 @@ void SVMCompiler::compile_type(Shader *shader, ShaderGraph *graph, ShaderType ty /* add node to restore state after bump shader has finished */ if (need_bump_state) { add_node(NODE_LEAVE_BUMP_EVAL, bump_state_offset); + bump_state_offset = SVM_STACK_INVALID; } /* if compile failed, generate empty shader */ diff --git a/intern/cycles/scene/svm.h b/intern/cycles/scene/svm.h index 53aa2804765..aa4f40b2577 100644 --- a/intern/cycles/scene/svm.h +++ b/intern/cycles/scene/svm.h @@ -106,6 +106,10 @@ class SVMCompiler { { return mix_weight_offset; } + uint get_bump_state_offset() + { + return bump_state_offset; + } ShaderType output_type() { @@ -222,6 +226,7 @@ class SVMCompiler { Stack active_stack; int max_stack_use; uint mix_weight_offset; + uint bump_state_offset; bool compile_failed; };