Details

Reviewers

Brecht Van Lommel (brecht)
Jacques Lucke (JacquesLucke)

Commits

rCfc5c162b12a0: Shading: Extend Noise node to other dimenstions
rB23564583a498: Shading: Extend Noise node to other dimenstions.

Summary

This patch extends perlin noise to operate in 1D, 2D, 3D, and 4D
space. An AVX implementation of the noise will follow.

Previously, a constant offset of 13.5 was added to the noise to act as a seed for generating distortion textures. Additionally, swizzling was performed to act as a seed for the Color outputs. This cased some issues, in particular, swizzling meant zero components may result in identical noise outputs and a constant value of 13.5 meant that distortion may happen only at close to zero areas.
To solve this, we use random offset functions that generates offsets at compile time, you can see this in details in the code. This change meant that the pattern of distortion will change and won't be identical to the previous version of the node.

Diff Detail

Repository

rB Blender

Branch

noise-node (branched from master)

Build Status

Buildable 4560
Build 4560: arc lint + arc unit

Event Timeline

Omar Emara (OmarSquircleArt) created this revision.Aug 22 2019, 8:42 PM

Harbormaster completed remote builds in B4554: Diff 17420.Aug 22 2019, 8:42 PM

Bump subversion and add versioning code.

Harbormaster completed remote builds in B4555: Diff 17421.Aug 22 2019, 9:16 PM

Omar Emara (OmarSquircleArt) retitled this revision from Shading: Extend Noise node to other dimenstions. to Shading: Extend Noise node to other dimensions..Aug 22 2019, 9:24 PM

Omar Emara (OmarSquircleArt) edited the summary of this revision. (Show Details)

Revert socket renamming.

Harbormaster completed remote builds in B4556: Diff 17422.Aug 22 2019, 9:35 PM

Omar Emara (OmarSquircleArt) added a child revision: D5566: Shading: Extend Musgrave node to other dimensions..Aug 23 2019, 10:15 AM

Omar Emara (OmarSquircleArt) mentioned this in T68823: GSoC 2019 - Cycles/EEVEE Improvements.Aug 23 2019, 10:19 AM

Use more consistent names.

Harbormaster completed remote builds in B4560: Diff 17432.Aug 23 2019, 11:58 AM

Are you able to test how this affects Cycles CUDA or OpenCL rendering performance?

Increasing the kernel size like this often affects performance in all scenes, even without those using the specific node. And then we need to do some tweaking to avoid it. I can run some benchmarks, but it would help if you could do it, even if only the simple bmw27 benchmark for example.
https://svn.blender.org/svnroot/bf-blender/trunk/lib/benchmarks/cycles/

@Brecht Van Lommel (brecht) I can't test CUDA as I don't have an Nvidea GPU.

I do have an RX 590, however, OpenCl compilation hits the assert assert(kernel->second == NULL); in OpenCLDevice::OpenCLProgram::create_kernels after 25 seconds from starting compilation. So I haven't been able to test OpenCL so far. This also happens in master, so it is not caused by something I did. Is this a driver issue? Should I try to remove the assert and render?

The render times for the BMW benchmark are as follows:

Branch	Timing 1	Timing 2	Timing 3	Average
Master	13:29.49	13:29.92	13:28.99	13:29.467
Patch	13:28.01	13:27.78	13:28.74	13:28.177

So this patch is, on average, 1.29s faster.

The benchmarks used cached kernels, so those results don't include the compilation times. The benchmarks were done by executing Blender in background mode in a tty with no xserver.

Omar Emara (OmarSquircleArt) mentioned this in D5566: Shading: Extend Musgrave node to other dimensions..Aug 25 2019, 11:38 PM

Jacques Lucke (JacquesLucke) added inline comments.Aug 26 2019, 5:13 PM

intern/cycles/kernel/shaders/node_noise.h
86	Are the three `noise_turbulence` implementations here exactly the same? Either deduplicate or write a comment for why the code is duplicated three times. Also why is `turbulence` in the name of these functions? Isn't this normal perlin noise?
intern/cycles/kernel/shaders/node_voronoi_texture.osl
19	Is this supposed to be in this patch?
intern/cycles/kernel/svm/svm_noise.h
65	`standard` (typo)
intern/cycles/kernel/svm/svm_texture.h
21	same comment as above
intern/cycles/util/util_ssef.h
640	Can this have a better name? Or at least a description for what it is doing?

I also tested the patch. Did not find any issues, but obviously there are limits to how well I can test this manually.

Omar Emara (OmarSquircleArt) added inline comments.Aug 26 2019, 5:34 PM

intern/cycles/kernel/shaders/node_noise.h
86	The only difference between them is the input vector type. For SVM, the noise function call is also different as we don't have overloading. This is what I showed in this section. I can add a comment about this, pretty sure I had one, not sure why I removed it. I don't know why it is called `turbulence`. I am guessing because the original developer wanted to differentiate "perlin noise" from "multi-octave perlin noise". I never liked the name `turbulence` anyway, I can change it to something else. Perhaps `octave_noise` or something.
intern/cycles/kernel/shaders/node_voronoi_texture.osl
19	This function was originally in `node_texture.h`, but it was only used by the voronoi function, so I moved it here and renamed `node_texture.h` to `node_noise.h` because the file only contains noise functions. This will be removed anyway in the voronoi patch.
intern/cycles/util/util_ssef.h
640	What do you suggest? The function creates an `ssef` by streaming the "low" part of an `ssef`. So for instance: low(ssef(0, 1, 2, 3)) -> ssef(0, 1, 0, 1)

Jacques Lucke (JacquesLucke) added inline comments.Aug 26 2019, 5:40 PM

intern/cycles/kernel/shaders/node_noise.h
86	Isn't this often called `fractal_{noise-type}`? Whereby fractal means that you add multiple layers of noise. At least the FastNoise library we used in AN calls it that way afaik.
intern/cycles/util/util_ssef.h
640	Is there a benefit of using this function over a shuffle? If yes, then you could also implement this as an specialization of shuffle (maybe the compiler replaces that anyway).

Omar Emara (OmarSquircleArt) added inline comments.Aug 26 2019, 5:48 PM

intern/cycles/kernel/shaders/node_noise.h
86	`fractal` looks good to me. I can rename it to that. Should it be `fractal_perlin`?
intern/cycles/util/util_ssef.h
640	Look at the `shuffle<2, 3, 2, 3>` and `shuffle<0, 1, 0, 1>` functions above. Those are already implemented as specialized shuffle functions. However, I found that the code is more readable when we have an explicit function like `low` and `high`. Look at my comments in `ssef bi_mix` function above.

Jacques Lucke (JacquesLucke) added inline comments.Aug 26 2019, 5:56 PM

intern/cycles/kernel/shaders/node_noise.h
86	Since perlin noise is apparently called just `noise` everywhere, it is probably best to just call it `fractal_noise`.
intern/cycles/util/util_ssef.h
640	Based on https://software.intel.com/sites/landingpage/IntrinsicsGuide/ it does not actually seem like these specialized instructions are actually faster then the generic shuffle. Not sure why they exist. Personally, I don't like the names `low` and `high` because they could mean many different things. The shuffle template makes what you are doing more explicit.

Omar Emara (OmarSquircleArt) added inline comments.Aug 26 2019, 5:58 PM

intern/cycles/util/util_ssef.h
640	Ok, will update the patch.

Correct typo.
Use "shuffle" function instead of "high" function.
Rename noise_turbulance to fractal_noise.

Harbormaster completed remote builds in B4604: Diff 17529.Aug 26 2019, 7:18 PM

Omar Emara (OmarSquircleArt) marked 8 inline comments as done.Aug 26 2019, 7:21 PM

B (bnzs) awarded a token.Aug 26 2019, 8:50 PM

Jacques Lucke (JacquesLucke) added inline comments.Aug 27 2019, 10:48 AM

intern/cycles/kernel/svm/svm_fractal_noise.h
19 ↗	(On Diff #17529)	Would probably be good to have a comment indicating this before every duplicated function. Otherwise it is easy to not see this when working on one of the functions. The comment could be `/* The fractal_noise_Xd functions are all exactly the same except for the input type. */`. Otherwise "the following functions" could become a bit unspecific when more functions get added.

Merge master and update patch.

Harbormaster completed remote builds in B4727: Diff 17755.Sep 2 2019, 2:39 PM

Omar Emara (OmarSquircleArt) marked an inline comment as done.Sep 2 2019, 2:39 PM

I didn't find measurable performance regressions with this for CUDA and CPU rendering with my benchmarks.

This does break backwards compatibility for color noise and distortion, but I can't see a good way to avoid that without making a mess, so I think that's acceptable.

This revision is now accepted and ready to land.Sep 4 2019, 4:39 PM

Closed by commit rB23564583a498: Shading: Extend Noise node to other dimenstions. (authored by Omar Emara (OmarSquircleArt)). · Explain WhySep 4 2019, 6:00 PM

This revision was automatically updated to reflect the committed changes.

Omar Emara (OmarSquircleArt) added a commit: rB23564583a498: Shading: Extend Noise node to other dimenstions..

Charlie Jolly (charlie) awarded a token.Sep 4 2019, 6:12 PM

Omar Emara (OmarSquircleArt) mentioned this in D5678: Versioning: Bump subversion for the Noise node..Sep 4 2019, 6:56 PM

Omar Emara (OmarSquircleArt) mentioned this in rB6f32f0ea272b: Versioning: Bump subversion for the Noise node..Sep 4 2019, 7:01 PM

Charlie Jolly (charlie) mentioned this in T72337: Shader Node Additions.Dec 10 2019, 6:53 PM

Nathan Letwory (jesterking) added a commit: rCfc5c162b12a0: Shading: Extend Noise node to other dimenstions.Jan 13 2020, 11:23 PM

Charlie Jolly (charlie) mentioned this in T83144: Noise texture has repeating stripes in z axis.Feb 16 2021, 5:19 PM

Shading: Extend Noise node to other dimensions.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 17432

intern/cycles/blender/blender_shader.cpp

intern/cycles/kernel/shaders/CMakeLists.txt

intern/cycles/kernel/shaders/node_brick_texture.osl

intern/cycles/kernel/shaders/node_checker_texture.osl

intern/cycles/kernel/shaders/node_gradient_texture.osl

intern/cycles/kernel/shaders/node_ies_light.osl

intern/cycles/kernel/shaders/node_magic_texture.osl

intern/cycles/kernel/shaders/node_musgrave_texture.osl

intern/cycles/kernel/shaders/node_noise.h

intern/cycles/kernel/shaders/node_noise_texture.osl

intern/cycles/kernel/shaders/node_texture.h

intern/cycles/kernel/shaders/node_voronoi_texture.osl

intern/cycles/kernel/shaders/node_wave_texture.osl

intern/cycles/kernel/shaders/vector2.h

intern/cycles/kernel/shaders/vector4.h

intern/cycles/kernel/svm/svm.h

intern/cycles/kernel/svm/svm_musgrave.h

intern/cycles/kernel/svm/svm_noise.h

intern/cycles/kernel/svm/svm_noisetex.h

intern/cycles/kernel/svm/svm_texture.h

intern/cycles/kernel/svm/svm_wave.h

intern/cycles/render/nodes.h

intern/cycles/render/nodes.cpp

intern/cycles/util/util_hash.h

intern/cycles/util/util_math.h

intern/cycles/util/util_ssef.h

intern/cycles/util/util_ssei.h

source/blender/blenkernel/BKE_blender_version.h

source/blender/blenloader/intern/versioning_cycles.c

source/blender/editors/space_node/drawnode.c

source/blender/gpu/shaders/gpu_shader_material.glsl

source/blender/makesdna/DNA_node_types.h

source/blender/makesrna/intern/rna_nodetree.c

source/blender/nodes/shader/nodes/node_shader_tex_noise.c

Shading: Extend Noise node to other dimensions.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 17432

intern/cycles/blender/blender_shader.cpp

intern/cycles/kernel/shaders/CMakeLists.txt

intern/cycles/kernel/shaders/node_brick_texture.osl

intern/cycles/kernel/shaders/node_checker_texture.osl

intern/cycles/kernel/shaders/node_gradient_texture.osl

intern/cycles/kernel/shaders/node_ies_light.osl

intern/cycles/kernel/shaders/node_magic_texture.osl

intern/cycles/kernel/shaders/node_musgrave_texture.osl

intern/cycles/kernel/shaders/node_noise.h

intern/cycles/kernel/shaders/node_noise_texture.osl

intern/cycles/kernel/shaders/node_texture.h

intern/cycles/kernel/shaders/node_voronoi_texture.osl

intern/cycles/kernel/shaders/node_wave_texture.osl

intern/cycles/kernel/shaders/vector2.h

intern/cycles/kernel/shaders/vector4.h

intern/cycles/kernel/svm/svm.h

intern/cycles/kernel/svm/svm_musgrave.h

intern/cycles/kernel/svm/svm_noise.h

intern/cycles/kernel/svm/svm_noisetex.h

intern/cycles/kernel/svm/svm_texture.h

intern/cycles/kernel/svm/svm_wave.h

intern/cycles/render/nodes.h

intern/cycles/render/nodes.cpp

intern/cycles/util/util_hash.h

intern/cycles/util/util_math.h

intern/cycles/util/util_ssef.h

intern/cycles/util/util_ssei.h

source/blender/blenkernel/BKE_blender_version.h

source/blender/blenloader/intern/versioning_cycles.c

source/blender/editors/space_node/drawnode.c

source/blender/gpu/shaders/gpu_shader_material.glsl

source/blender/makesdna/DNA_node_types.h

source/blender/makesrna/intern/rna_nodetree.c

source/blender/nodes/shader/nodes/node_shader_tex_noise.c

Shading: Extend Noise node to other dimensions.
ClosedPublic
Actions

Revision Contents
Changeset List