From 41a3de878f64ae19e4f80c58102cc64e583d3a5f Mon Sep 17 00:00:00 2001 From: Patrick Mours Date: Fri, 11 Nov 2022 16:42:49 +0100 Subject: [PATCH 1/2] Fix part of T102450: Cycles OSL render issues for with normals in shader nodes Commit c8dd33f5a37b6a6db0b6950d24f9a7cff5ceb799 in OSL changed behavior of parameters that reference each other and are also overwritten with an instance value. This is causing the "NormalIn" parameter of a few OSL nodes in Cycles to be set to zero somehow, which should instead have received the value from a "node_geometry" node Cycles generates and connects automatically. I am not entirely sure why that is happening, but these parameters are superfluous anyway, since OSL already provides the necessary data in the global variable "N". So this patch simply removes those parameters (which mimics SVM, where these parameters do not exist either), which also fixes the rendering artifacts that occured with recent OSL. While this fixes built-in shader nodes, custom OSL scripts can still have this problem. Ref T101222 Differential Revision: https://developer.blender.org/D16470 --- intern/cycles/kernel/osl/shaders/node_geometry.osl | 5 ++--- intern/cycles/kernel/osl/shaders/node_normal_map.osl | 7 +++---- intern/cycles/kernel/osl/shaders/node_tangent.osl | 5 ++--- .../kernel/osl/shaders/node_texture_coordinate.osl | 7 +++---- intern/cycles/scene/shader_nodes.cpp | 10 ---------- intern/cycles/scene/shader_nodes.h | 5 ----- 6 files changed, 10 insertions(+), 29 deletions(-) diff --git a/intern/cycles/kernel/osl/shaders/node_geometry.osl b/intern/cycles/kernel/osl/shaders/node_geometry.osl index cc891abd6e3..5d9284deac2 100644 --- a/intern/cycles/kernel/osl/shaders/node_geometry.osl +++ b/intern/cycles/kernel/osl/shaders/node_geometry.osl @@ -3,8 +3,7 @@ #include "stdcycles.h" -shader node_geometry(normal NormalIn = N, - string bump_offset = "center", +shader node_geometry(string bump_offset = "center", output point Position = point(0.0, 0.0, 0.0), output normal Normal = normal(0.0, 0.0, 0.0), @@ -17,7 +16,7 @@ shader node_geometry(normal NormalIn = N, output float RandomPerIsland = 0.0) { Position = P; - Normal = NormalIn; + Normal = N; TrueNormal = Ng; Incoming = I; Parametric = point(1.0 - u - v, u, 0.0); diff --git a/intern/cycles/kernel/osl/shaders/node_normal_map.osl b/intern/cycles/kernel/osl/shaders/node_normal_map.osl index 3cda485c686..7e41bbf1720 100644 --- a/intern/cycles/kernel/osl/shaders/node_normal_map.osl +++ b/intern/cycles/kernel/osl/shaders/node_normal_map.osl @@ -3,13 +3,12 @@ #include "stdcycles.h" -shader node_normal_map(normal NormalIn = N, - float Strength = 1.0, +shader node_normal_map(float Strength = 1.0, color Color = color(0.5, 0.5, 1.0), string space = "tangent", string attr_name = "geom:tangent", string attr_sign_name = "geom:tangent_sign", - output normal Normal = NormalIn) + output normal Normal = N) { color mcolor = 2.0 * color(Color[0] - 0.5, Color[1] - 0.5, Color[2] - 0.5); int is_backfacing = backfacing(); @@ -71,5 +70,5 @@ shader node_normal_map(normal NormalIn = N, } if (Strength != 1.0) - Normal = normalize(NormalIn + (Normal - NormalIn) * max(Strength, 0.0)); + Normal = normalize(N + (Normal - N) * max(Strength, 0.0)); } diff --git a/intern/cycles/kernel/osl/shaders/node_tangent.osl b/intern/cycles/kernel/osl/shaders/node_tangent.osl index a302c001f08..b3808778b2f 100644 --- a/intern/cycles/kernel/osl/shaders/node_tangent.osl +++ b/intern/cycles/kernel/osl/shaders/node_tangent.osl @@ -3,8 +3,7 @@ #include "stdcycles.h" -shader node_tangent(normal NormalIn = N, - string attr_name = "geom:tangent", +shader node_tangent(string attr_name = "geom:tangent", string direction_type = "radial", string axis = "z", output normal Tangent = normalize(dPdu)) @@ -29,5 +28,5 @@ shader node_tangent(normal NormalIn = N, } T = transform("object", "world", T); - Tangent = cross(NormalIn, normalize(cross(T, NormalIn))); + Tangent = cross(N, normalize(cross(T, N))); } diff --git a/intern/cycles/kernel/osl/shaders/node_texture_coordinate.osl b/intern/cycles/kernel/osl/shaders/node_texture_coordinate.osl index 24875ce140a..cd2fdae3cb3 100644 --- a/intern/cycles/kernel/osl/shaders/node_texture_coordinate.osl +++ b/intern/cycles/kernel/osl/shaders/node_texture_coordinate.osl @@ -4,7 +4,6 @@ #include "stdcycles.h" shader node_texture_coordinate( - normal NormalIn = N, int is_background = 0, int is_volume = 0, int from_dupli = 0, @@ -27,7 +26,7 @@ shader node_texture_coordinate( point Pcam = transform("camera", "world", point(0, 0, 0)); Camera = transform("camera", P + Pcam); getattribute("NDC", Window); - Normal = NormalIn; + Normal = N; Reflection = I; } else { @@ -59,8 +58,8 @@ shader node_texture_coordinate( } Camera = transform("camera", P); Window = transform("NDC", P); - Normal = transform("world", "object", NormalIn); - Reflection = -reflect(I, NormalIn); + Normal = transform("world", "object", N); + Reflection = -reflect(I, N); } if (bump_offset == "dx") { diff --git a/intern/cycles/scene/shader_nodes.cpp b/intern/cycles/scene/shader_nodes.cpp index a9cd453947b..2c1cd3ee737 100644 --- a/intern/cycles/scene/shader_nodes.cpp +++ b/intern/cycles/scene/shader_nodes.cpp @@ -3677,9 +3677,6 @@ NODE_DEFINE(GeometryNode) { NodeType *type = NodeType::add("geometry", create, NodeType::SHADER); - SOCKET_IN_NORMAL( - normal_osl, "NormalIn", zero_float3(), SocketType::LINK_NORMAL | SocketType::OSL_INTERNAL); - SOCKET_OUT_POINT(position, "Position"); SOCKET_OUT_NORMAL(normal, "Normal"); SOCKET_OUT_NORMAL(tangent, "Tangent"); @@ -3812,9 +3809,6 @@ NODE_DEFINE(TextureCoordinateNode) SOCKET_BOOLEAN(use_transform, "Use Transform", false); SOCKET_TRANSFORM(ob_tfm, "Object Transform", transform_identity()); - SOCKET_IN_NORMAL( - normal_osl, "NormalIn", zero_float3(), SocketType::LINK_NORMAL | SocketType::OSL_INTERNAL); - SOCKET_OUT_POINT(generated, "Generated"); SOCKET_OUT_NORMAL(normal, "Normal"); SOCKET_OUT_POINT(UV, "UV"); @@ -7305,8 +7299,6 @@ NODE_DEFINE(NormalMapNode) SOCKET_STRING(attribute, "Attribute", ustring()); - SOCKET_IN_NORMAL( - normal_osl, "NormalIn", zero_float3(), SocketType::LINK_NORMAL | SocketType::OSL_INTERNAL); SOCKET_IN_FLOAT(strength, "Strength", 1.0f); SOCKET_IN_COLOR(color, "Color", make_float3(0.5f, 0.5f, 1.0f)); @@ -7400,8 +7392,6 @@ NODE_DEFINE(TangentNode) SOCKET_STRING(attribute, "Attribute", ustring()); - SOCKET_IN_NORMAL( - normal_osl, "NormalIn", zero_float3(), SocketType::LINK_NORMAL | SocketType::OSL_INTERNAL); SOCKET_OUT_NORMAL(tangent, "Tangent"); return type; diff --git a/intern/cycles/scene/shader_nodes.h b/intern/cycles/scene/shader_nodes.h index cc3a71a0697..3ab7b1041b5 100644 --- a/intern/cycles/scene/shader_nodes.h +++ b/intern/cycles/scene/shader_nodes.h @@ -907,8 +907,6 @@ class GeometryNode : public ShaderNode { return true; } int get_group(); - - NODE_SOCKET_API(float3, normal_osl) }; class TextureCoordinateNode : public ShaderNode { @@ -924,7 +922,6 @@ class TextureCoordinateNode : public ShaderNode { return true; } - NODE_SOCKET_API(float3, normal_osl) NODE_SOCKET_API(bool, from_dupli) NODE_SOCKET_API(bool, use_transform) NODE_SOCKET_API(Transform, ob_tfm) @@ -1569,7 +1566,6 @@ class NormalMapNode : public ShaderNode { NODE_SOCKET_API(ustring, attribute) NODE_SOCKET_API(float, strength) NODE_SOCKET_API(float3, color) - NODE_SOCKET_API(float3, normal_osl) }; class TangentNode : public ShaderNode { @@ -1588,7 +1584,6 @@ class TangentNode : public ShaderNode { NODE_SOCKET_API(NodeTangentDirectionType, direction_type) NODE_SOCKET_API(NodeTangentAxis, axis) NODE_SOCKET_API(ustring, attribute) - NODE_SOCKET_API(float3, normal_osl) }; class BevelNode : public ShaderNode { From 03b5be4e3cdf6a4967cb438dacd595c23075db79 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Mon, 21 Nov 2022 18:16:27 +0100 Subject: [PATCH 2/2] Cycles: use more PMJ patterns and make their size adaptive. This resolves some issues with correlation artifacts at higher sample counts. Fix T101356, correlation issues in new PMJ pattern. Differential Revision: https://developer.blender.org/D16561 --- intern/cycles/kernel/data_template.h | 6 +++ intern/cycles/kernel/sample/jitter.h | 55 ++++++++++++---------------- intern/cycles/kernel/types.h | 9 +++-- intern/cycles/scene/integrator.cpp | 14 +++++-- 4 files changed, 45 insertions(+), 39 deletions(-) diff --git a/intern/cycles/kernel/data_template.h b/intern/cycles/kernel/data_template.h index 6b89de13797..c7b50b20c70 100644 --- a/intern/cycles/kernel/data_template.h +++ b/intern/cycles/kernel/data_template.h @@ -183,6 +183,7 @@ KERNEL_STRUCT_MEMBER(integrator, int, use_lamp_mis) KERNEL_STRUCT_MEMBER(integrator, int, use_caustics) /* Sampling pattern. */ KERNEL_STRUCT_MEMBER(integrator, int, sampling_pattern) +KERNEL_STRUCT_MEMBER(integrator, int, pmj_sequence_size) KERNEL_STRUCT_MEMBER(integrator, float, scrambling_distance) /* Volume render. */ KERNEL_STRUCT_MEMBER(integrator, int, use_volumes) @@ -205,6 +206,11 @@ KERNEL_STRUCT_MEMBER(integrator, int, use_surface_guiding) KERNEL_STRUCT_MEMBER(integrator, int, use_volume_guiding) KERNEL_STRUCT_MEMBER(integrator, int, use_guiding_direct_light) KERNEL_STRUCT_MEMBER(integrator, int, use_guiding_mis_weights) + +/* Padding. */ +KERNEL_STRUCT_MEMBER(integrator, int, pad1) +KERNEL_STRUCT_MEMBER(integrator, int, pad2) +KERNEL_STRUCT_MEMBER(integrator, int, pad3) KERNEL_STRUCT_END(KernelIntegrator) /* SVM. For shader specialization. */ diff --git a/intern/cycles/kernel/sample/jitter.h b/intern/cycles/kernel/sample/jitter.h index e748f95fc7d..1cde9f9d3de 100644 --- a/intern/cycles/kernel/sample/jitter.h +++ b/intern/cycles/kernel/sample/jitter.h @@ -7,6 +7,25 @@ #pragma once CCL_NAMESPACE_BEGIN +ccl_device uint pmj_shuffled_sample_index(KernelGlobals kg, uint sample, uint dimension, uint seed) +{ + const uint sample_count = kernel_data.integrator.pmj_sequence_size; + + /* Shuffle the pattern order and sample index to better decorrelate + * dimensions and make the most of the finite patterns we have. + * The funky sample mask stuff is to ensure that we only shuffle + * *within* the current sample pattern, which is necessary to avoid + * early repeat pattern use. */ + const uint pattern_i = hash_shuffle_uint(dimension, NUM_PMJ_PATTERNS, seed); + /* sample_count should always be a power of two, so this results in a mask. */ + const uint sample_mask = sample_count - 1; + const uint sample_shuffled = nested_uniform_scramble(sample, + hash_wang_seeded_uint(dimension, seed)); + sample = (sample & ~sample_mask) | (sample_shuffled & sample_mask); + + return ((pattern_i * sample_count) + sample) % (sample_count * NUM_PMJ_PATTERNS); +} + ccl_device float pmj_sample_1D(KernelGlobals kg, uint sample, const uint rng_hash, @@ -20,22 +39,9 @@ ccl_device float pmj_sample_1D(KernelGlobals kg, seed = kernel_data.integrator.seed; } - /* Shuffle the pattern order and sample index to better decorrelate - * dimensions and make the most of the finite patterns we have. - * The funky sample mask stuff is to ensure that we only shuffle - * *within* the current sample pattern, which is necessary to avoid - * early repeat pattern use. */ - const uint pattern_i = hash_shuffle_uint(dimension, NUM_PMJ_PATTERNS, seed); - /* NUM_PMJ_SAMPLES should be a power of two, so this results in a mask. */ - const uint sample_mask = NUM_PMJ_SAMPLES - 1; - const uint sample_shuffled = nested_uniform_scramble(sample, - hash_wang_seeded_uint(dimension, seed)); - sample = (sample & ~sample_mask) | (sample_shuffled & sample_mask); - /* Fetch the sample. */ - const uint index = ((pattern_i * NUM_PMJ_SAMPLES) + sample) % - (NUM_PMJ_SAMPLES * NUM_PMJ_PATTERNS); - float x = kernel_data_fetch(sample_pattern_lut, index * 2); + const uint index = pmj_shuffled_sample_index(kg, sample, dimension, seed); + float x = kernel_data_fetch(sample_pattern_lut, index * NUM_PMJ_DIMENSIONS); /* Do limited Cranley-Patterson rotation when using scrambling distance. */ if (kernel_data.integrator.scrambling_distance < 1.0f) { @@ -61,23 +67,10 @@ ccl_device float2 pmj_sample_2D(KernelGlobals kg, seed = kernel_data.integrator.seed; } - /* Shuffle the pattern order and sample index to better decorrelate - * dimensions and make the most of the finite patterns we have. - * The funky sample mask stuff is to ensure that we only shuffle - * *within* the current sample pattern, which is necessary to avoid - * early repeat pattern use. */ - const uint pattern_i = hash_shuffle_uint(dimension, NUM_PMJ_PATTERNS, seed); - /* NUM_PMJ_SAMPLES should be a power of two, so this results in a mask. */ - const uint sample_mask = NUM_PMJ_SAMPLES - 1; - const uint sample_shuffled = nested_uniform_scramble(sample, - hash_wang_seeded_uint(dimension, seed)); - sample = (sample & ~sample_mask) | (sample_shuffled & sample_mask); - /* Fetch the sample. */ - const uint index = ((pattern_i * NUM_PMJ_SAMPLES) + sample) % - (NUM_PMJ_SAMPLES * NUM_PMJ_PATTERNS); - float x = kernel_data_fetch(sample_pattern_lut, index * 2); - float y = kernel_data_fetch(sample_pattern_lut, index * 2 + 1); + const uint index = pmj_shuffled_sample_index(kg, sample, dimension, seed); + float x = kernel_data_fetch(sample_pattern_lut, index * NUM_PMJ_DIMENSIONS); + float y = kernel_data_fetch(sample_pattern_lut, index * NUM_PMJ_DIMENSIONS + 1); /* Do limited Cranley-Patterson rotation when using scrambling distance. */ if (kernel_data.integrator.scrambling_distance < 1.0f) { diff --git a/intern/cycles/kernel/types.h b/intern/cycles/kernel/types.h index 24c5a6a4540..6d80fd3425c 100644 --- a/intern/cycles/kernel/types.h +++ b/intern/cycles/kernel/types.h @@ -1382,12 +1382,13 @@ static_assert_align(KernelShaderEvalInput, 16); /* Pre-computed sample table sizes for PMJ02 sampler. * - * NOTE: divisions *must* be a power of two, and patterns + * NOTE: min and max samples *must* be a power of two, and patterns * ideally should be as well. */ -#define NUM_PMJ_DIVISIONS 32 -#define NUM_PMJ_SAMPLES ((NUM_PMJ_DIVISIONS) * (NUM_PMJ_DIVISIONS)) -#define NUM_PMJ_PATTERNS 64 +#define MIN_PMJ_SAMPLES 256 +#define MAX_PMJ_SAMPLES 8192 +#define NUM_PMJ_DIMENSIONS 2 +#define NUM_PMJ_PATTERNS 256 /* Device kernels. * diff --git a/intern/cycles/scene/integrator.cpp b/intern/cycles/scene/integrator.cpp index ade4716242b..23f9e8b7aa8 100644 --- a/intern/cycles/scene/integrator.cpp +++ b/intern/cycles/scene/integrator.cpp @@ -257,12 +257,18 @@ void Integrator::device_update(Device *device, DeviceScene *dscene, Scene *scene kintegrator->light_inv_rr_threshold = 0.0f; } + constexpr int num_sequences = NUM_PMJ_PATTERNS; + int sequence_size = clamp(next_power_of_two(aa_samples - 1), MIN_PMJ_SAMPLES, MAX_PMJ_SAMPLES); if (kintegrator->sampling_pattern == SAMPLING_PATTERN_PMJ && - dscene->sample_pattern_lut.size() == 0) { - constexpr int sequence_size = NUM_PMJ_SAMPLES; - constexpr int num_sequences = NUM_PMJ_PATTERNS; + dscene->sample_pattern_lut.size() != + (sequence_size * NUM_PMJ_DIMENSIONS * NUM_PMJ_PATTERNS)) { + kintegrator->pmj_sequence_size = sequence_size; + + if (dscene->sample_pattern_lut.size() != 0) { + dscene->sample_pattern_lut.free(); + } float2 *directions = (float2 *)dscene->sample_pattern_lut.alloc(sequence_size * num_sequences * - 2); + NUM_PMJ_DIMENSIONS); TaskPool pool; for (int j = 0; j < num_sequences; ++j) { float2 *sequence = directions + j * sequence_size;