Node: Gabor Noise Texture #110802

Open
Charlie Jolly wants to merge 68 commits from CharlieJolly/blender:gabor into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Member

Code comments in GLSL

Mini tutorial and feedback on Devtalk thread: https://devtalk.blender.org/t/gabor-procedural-texture-node/25934/34

PR Builds: https://builder.blender.org/download/patch/PR110802/ (These are PR patch builds so be careful with your files etc.)

Gabor Noise

This patch adds procedural Gabor noise. This has implementations for Shading (Cycles, Eevee, OSL) and GN.
The noise works by splatting kernels over grid cells similar to Voronoi. The node has controls for fractal layering and kernel shape variance. The noise is similar to Voronoi for expense.

Based on: Blender patch D287 & D3495
With additional controls for kernel variance.

This implementation is adapted from the original built-in OSL implementation based on the 2009
paper "Procedural noise using sparse Gabor convolution". Some parts are also adapted from the
2011 paper "Filtering Solid Gabor Noise" but this does not include the filtering and slicing.
References to the papers are for copyright and reference.

Notes and changes from the original OSL implementation and reference papers:

  • For 2D noise, as with Voronoi the calculation uses a 2x2 grid rather than slicing 3D noise.
    This is more performant when only 2D texture is required.
  • For artistic control, calculations for Bandwidth have been simplified and replaced with
    separate controls for Frequency, Bandwidth (Power) and Radius. This provides finer control where
    before frequency and bandwidth were bound to the same parameter. Radius values over 1 may result
    in artefacts and discontinuities. This is not clamped as pushing the radius can create some
    potentially useful noise.
  • Phasor noise has been added. Since this is based on Gabor and sums the changes in phase it is
    trivial to add.
  • Additional sincos based kernels have been added which provide different texture control in a
    similar way to distance metrics in Voronoi.
  • Added Roughness, Scale Lacunarity, Frequency Lacunarity and Rotation Lacunarity to control
    additive fractal noise.
  • Uses built-in Blender hashes instead of adding a separate gabor rng.

Adapted from Open Shading Language implementation.
Copyright (c) 2009-2010 Sony Pictures Imageworks Inc., et al.
All Rights Reserved.

OSL Gabor noise references:
Lagae, Lefebvre, Drettakis and Dutré, 2009. Procedural noise using sparse Gabor convolution
Lagae, A. and Drettakis, G. 2011. Filtering Solid Gabor Noise
Tavernier et al. 2018, Gabor Noise Revisited

Phasor noise reference:
Tricard et al. 2019. Procedural Phasor Noise


Brief notes on parameters, excluding obvious ones like vector and scale etc

  • Kernel: Gabor/Ring/Cross/Square & Phasor/Ring/Cross/Square> Type of kernel used to calculate the noise. Similar to Distance metric in Voronoi noise.
  • Periodic: Repeats the grid for periodic patterns across the [0-1] texture space. This can be tileable but it is not guaranteed depending on other parameters.
  • Detail: Additive noise, each additional octave is calculated with a higher frequency and added to the base noise.
  • Bandwidth: Controls the amount of kernel detail that is passed through.
  • Impulses: Controls the amount of kernel splats, high values are slower to compute.
  • Frequency: Kernel shape frequency, higher values provides more detail.
  • Phase: Controls the phase of the kernel signal.
  • Phase Randomness: This controls the Phase randomisation.
  • Cell Randomness: Like with Voronoi, this controls the position used within the cell. When set to zero, the noise is more regular in appearance.
  • Rotation: Controls the base rotation used for each splat.
  • Rotation Variance: Controls the amount of random rotation used for each splat.
  • Tilt Randomness: Controls the tilt of the splat.
  • Anisotropy Factor: Mix between Isotropic controls and Fixed Direction.
  • Direction: Controls the fixed direction of the splats.

Shadertoy test version -- outdated:
https://www.shadertoy.com/view/NtdfRj

Devtalk feedback thread:
https://devtalk.blender.org/t/gabor-procedural-texture-node/25934

Based on these patches and OSL:
https://archive.blender.org/developer/D3495
https://archive.blender.org/developer/D287


Parameters, 2D Gabor, Normalized

image

image

image

Comparison with OSL native implementation, why Bandwidth is split into Frequency.

image

Artefacts and use of cosine envelope instead of gaussian

image

Kernel shapes shown in 3D

image

Latest UI

image

Code comments in GLSL Mini tutorial and feedback on Devtalk thread: https://devtalk.blender.org/t/gabor-procedural-texture-node/25934/34 PR Builds: https://builder.blender.org/download/patch/PR110802/ (These are PR patch builds so be careful with your files etc.) ### Gabor Noise This patch adds procedural Gabor noise. This has implementations for Shading (Cycles, Eevee, OSL) and GN. The noise works by splatting kernels over grid cells similar to Voronoi. The node has controls for fractal layering and kernel shape variance. The noise is similar to Voronoi for expense. Based on: Blender patch D287 & D3495 With additional controls for kernel variance. This implementation is adapted from the original built-in OSL implementation based on the 2009 paper "Procedural noise using sparse Gabor convolution". Some parts are also adapted from the 2011 paper "Filtering Solid Gabor Noise" but this does not include the filtering and slicing. References to the papers are for copyright and reference. Notes and changes from the original OSL implementation and reference papers: - For 2D noise, as with Voronoi the calculation uses a 2x2 grid rather than slicing 3D noise. This is more performant when only 2D texture is required. - For artistic control, calculations for Bandwidth have been simplified and replaced with separate controls for Frequency, Bandwidth (Power) and Radius. This provides finer control where before frequency and bandwidth were bound to the same parameter. Radius values over 1 may result in artefacts and discontinuities. This is not clamped as pushing the radius can create some potentially useful noise. - Phasor noise has been added. Since this is based on Gabor and sums the changes in phase it is trivial to add. - Additional sincos based kernels have been added which provide different texture control in a similar way to distance metrics in Voronoi. - Added Roughness, Scale Lacunarity, Frequency Lacunarity and Rotation Lacunarity to control additive fractal noise. - Uses built-in Blender hashes instead of adding a separate gabor rng. Adapted from Open Shading Language implementation. Copyright (c) 2009-2010 Sony Pictures Imageworks Inc., et al. All Rights Reserved. OSL Gabor noise references: Lagae, Lefebvre, Drettakis and Dutré, 2009. Procedural noise using sparse Gabor convolution Lagae, A. and Drettakis, G. 2011. Filtering Solid Gabor Noise Tavernier et al. 2018, Gabor Noise Revisited Phasor noise reference: Tricard et al. 2019. Procedural Phasor Noise ------------------------ Brief notes on parameters, excluding obvious ones like vector and scale etc - Kernel: Gabor/Ring/Cross/Square & Phasor/Ring/Cross/Square> Type of kernel used to calculate the noise. Similar to Distance metric in Voronoi noise. - Periodic: Repeats the grid for periodic patterns across the [0-1] texture space. This can be tileable but it is not guaranteed depending on other parameters. - Detail: Additive noise, each additional octave is calculated with a higher frequency and added to the base noise. - Bandwidth: Controls the amount of kernel detail that is passed through. - Impulses: Controls the amount of kernel splats, high values are slower to compute. - Frequency: Kernel shape frequency, higher values provides more detail. - Phase: Controls the phase of the kernel signal. - Phase Randomness: This controls the Phase randomisation. - Cell Randomness: Like with Voronoi, this controls the position used within the cell. When set to zero, the noise is more regular in appearance. - Rotation: Controls the base rotation used for each splat. - Rotation Variance: Controls the amount of random rotation used for each splat. - Tilt Randomness: Controls the tilt of the splat. - Anisotropy Factor: Mix between Isotropic controls and Fixed Direction. - Direction: Controls the fixed direction of the splats. Shadertoy test version -- outdated: https://www.shadertoy.com/view/NtdfRj Devtalk feedback thread: https://devtalk.blender.org/t/gabor-procedural-texture-node/25934 Based on these patches and OSL: https://archive.blender.org/developer/D3495 https://archive.blender.org/developer/D287 ----------- Parameters, 2D Gabor, Normalized ![image](/attachments/11988e58-fc63-4cf4-b7be-a675b2cece67) ![image](/attachments/286c97a1-16fc-4c79-b9ce-9b6b932e6fbb) ![image](/attachments/7edd8c4b-902a-48b1-8eea-32873e8dcf03) Comparison with OSL native implementation, why Bandwidth is split into Frequency. ![image](/attachments/928b34cf-28ee-4137-bb5a-33368c832822) Artefacts and use of cosine envelope instead of gaussian ![image](/attachments/f074297b-eb2a-491e-b9d0-6e85a542b5ed) Kernel shapes shown in 3D ![image](/attachments/dd08ca98-f325-491d-9387-575e433b0aae) Latest UI ![image](/attachments/081d57b2-6654-41bd-89a4-b2c35fbf62a7)
Charlie Jolly added 1 commit 2023-08-04 17:33:56 +02:00
8496518229 Node: Gabor Noise Texture
This patch adds procedural Gabor noise.

This has implementations for Shading (Cycles, Eevee, OSL) and GN.

The noise works by splatting kernels over grid cells similar to Voronoi.

The node has controls for fractal layering and kernel shape variance.

The noise is similar to Voronoi for expense.

Shadertoy test version:
https://www.shadertoy.com/view/NtdfRj

Devtalk feedback thread:
https://devtalk.blender.org/t/gabor-procedural-texture-node/25934

Based on these patches and OSL:
https://archive.blender.org/developer/D3495
https://archive.blender.org/developer/D287

Adapted from Open Shading Language
Copyright (c) 2009-2010 Sony Pictures Imageworks Inc., et al.
All Rights Reserved.

Gabor noise originally based on:
Lagae, A. and Drettakis, G. 2011. Filtering Solid Gabor Noise.
Charlie Jolly added this to the Render & Cycles project 2023-08-04 17:34:25 +02:00
Charlie Jolly modified the project from Render & Cycles to Nodes & Physics 2023-08-04 17:34:30 +02:00
Charlie Jolly modified the project from Nodes & Physics to Render & Cycles 2023-08-04 17:34:35 +02:00
Charlie Jolly added 4 commits 2023-08-08 22:37:56 +02:00
Charlie Jolly added 1 commit 2023-08-09 17:49:02 +02:00
buildbot/vexp-code-patch-coordinator Build done. Details
231e052cd0
Use math lib in GN
Remove `gabor_cellhash` and use built-in hash functions. Noticed bug after switching to VS22 from VS19.
Iliya Katushenock added the
Interest
Nodes & Physics
label 2023-08-09 18:05:07 +02:00
Iliya Katushenock modified the project from Render & Cycles to Nodes & Physics 2023-08-09 18:05:29 +02:00
Iliya Katushenock added
Module
Render & Cycles
Interest
Geometry Nodes
and removed
Interest
Nodes & Physics
labels 2023-08-09 18:05:38 +02:00
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly added 1 commit 2023-08-09 18:47:44 +02:00
buildbot/vexp-code-patch-coordinator Build done. Details
303c9c72f1
Fix error on build - 'float' to 'double'
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly added 1 commit 2023-08-09 20:38:23 +02:00
buildbot/vexp-code-patch-coordinator Build done. Details
98b2a67673
Merge branch 'main' into gabor
Author
Member

@blender-bot build

@blender-bot build
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Brecht Van Lommel requested review from Brecht Van Lommel 2023-08-11 14:09:50 +02:00
Brecht Van Lommel requested review from Jacques Lucke 2023-08-11 14:09:50 +02:00
Brecht Van Lommel requested review from Omar Emara 2023-08-11 14:09:50 +02:00
Omar Emara requested changes 2023-08-14 19:31:02 +02:00
Omar Emara left a comment
Member

To be honest, I don't think the patch is ready for review because it lacks any kind of documentation, either in the code or in the patch description.

Even the only given reference "Lagae, A. and Drettakis, G. 2011. Filtering Solid Gabor Noise." seems to be the least relevant paper to the patch, since the patch does not implement filtering nor slicing, but only seems to borrow the phase-augmented Gabor kernel with no stated reason or hints.

The Phasor option is rather ambiguous and seems out of place, only for the reviewer to discover that it is described in a completely different paper with no mention of it.

So my recommendation would be to spend some time documenting the patch and the code as extensively as you can, mentioning any papers that you read, referencing equations that you are implementing, quoting any reasoning that you have came across, and try to make sense of any ambiguous code that you found.

Personally, I would try to rewrite the code away from the OSL code, to make documenting things easier, perhaps look at the reference implementation from the authors.

To be honest, I don't think the patch is ready for review because it lacks any kind of documentation, either in the code or in the patch description. Even the only given reference "Lagae, A. and Drettakis, G. 2011. Filtering Solid Gabor Noise." seems to be the least relevant paper to the patch, since the patch does not implement filtering nor slicing, but only seems to borrow the phase-augmented Gabor kernel with no stated reason or hints. The Phasor option is rather ambiguous and seems out of place, only for the reviewer to discover that it is described in a completely different paper with no mention of it. So my recommendation would be to spend some time documenting the patch and the code as extensively as you can, mentioning any papers that you read, referencing equations that you are implementing, quoting any reasoning that you have came across, and try to make sense of any ambiguous code that you found. Personally, I would try to rewrite the code away from the OSL code, to make documenting things easier, perhaps look at the reference implementation from the authors.
@ -0,0 +13,4 @@
floatBitsToUint(floor(cell.y)),
floatBitsToUint(floor(cell.z)),
floatBitsToUint(float(seed))));
if (chash == 0)
Member

Brackets to follow Blender's coding style.

Brackets to follow Blender's coding style.
CharlieJolly marked this conversation as resolved
@ -0,0 +18,4 @@
return chash * 1519588931;
}
float gabor_rng_uniform(inout int rng)
Member

Can't we use our own random functions? Is there an advantage to using those?

Can't we use our own random functions? Is there an advantage to using those?
Author
Member

The initial hash is using Blender hash functions to seed the RNG. We don't seem to have an RNG implementation in the shading system for so this is why this is used.

The initial hash is using Blender hash functions to seed the RNG. We don't seem to have an RNG implementation in the shading system for so this is why this is used.
Member

But is it necessary to use an RNG at all? Why not use our pseudo-number-generator with the impulse index as an input?

But is it necessary to use an RNG at all? Why not use our pseudo-number-generator with the impulse index as an input?
Author
Member

Updated to use built in hash functions

Updated to use built in hash functions
CharlieJolly marked this conversation as resolved
@ -0,0 +92,4 @@
float ovar = M_PI * (orand * 2.0 - 1.0);
float omega_t = ovar * gp.variance - gp.rotation;
if (gp.anisotropic == 1) { /* ANISO */
Member

Can we find a way to incorporate all three options into a single factor input that controls the amount of anisotropy?

Can we find a way to incorporate all three options into a single factor input that controls the amount of anisotropy?
Author
Member

This has been slightly refactored but combining to a single factor is not feasible for Gabor noise.

This has been slightly refactored but combining to a single factor is not feasible for Gabor noise.
Author
Member

Latest version now combines all three modes/options into one.
There is now an Anisotropy Factor to mix between Isotropic/Manual direction and Anisotropic/Fixed Direction.

Latest version now combines all three modes/options into one. There is now an Anisotropy Factor to mix between Isotropic/Manual direction and Anisotropic/Fixed Direction.
CharlieJolly marked this conversation as resolved
@ -0,0 +110,4 @@
return gp; // workaround gpu_codegen parser
}
vec3 gabor_fractal(GaborParams gp, float dv, inout vec3 omega, float phi, vec3 kernel_position)
Member

Why are we computing the fractcal on the freqiency as opposed to the scale to make it consistent with other noise functions? Is there a good reason for that?

Why are we computing the fractcal on the freqiency as opposed to the scale to make it consistent with other noise functions? Is there a good reason for that?
Author
Member

This was implemented per impulse/splat rather than per texture pass. This was mentioned to me by @Hoshinova so I have this marked as a todo. The results are quite different.

This was implemented per impulse/splat rather than per texture pass. This was mentioned to me by @Hoshinova so I have this marked as a todo. The results are quite different.
Author
Member

Implemented fractal based on other noise functions.

Implemented fractal based on other noise functions.
CharlieJolly marked this conversation as resolved
@ -0,0 +264,4 @@
int anisotropic,
int periodic)
{
if (impulses == 0.0) {
Member

This seems like the kind of thing that you can sanitize before passing it to the shader.

This seems like the kind of thing that you can sanitize before passing it to the shader.
Author
Member

This is partly implemented in Cycles using constant folding. Do you have an example for Eevee?

This is partly implemented in Cycles using constant folding. Do you have an example for Eevee?
Member

@CharlieJolly That's probably just a:

if (folding_condition) {
  const float base_value = 0.5f;
  return GPU_link(mat, "set_value", GPU_uniform(&base_value), &out->link);
}
@CharlieJolly That's probably just a: ``` if (folding_condition) { const float base_value = 0.5f; return GPU_link(mat, "set_value", GPU_uniform(&base_value), &out->link); } ```
Member

But I just realized that the impulses are not constant, so you can ignore my comment here.

But I just realized that the impulses are not constant, so you can ignore my comment here.
CharlieJolly marked this conversation as resolved
Author
Member

To be honest, I don't think the patch is ready for review because it lacks any kind of documentation, either in the code or in the patch description.

Even the only given reference "Lagae, A. and Drettakis, G. 2011. Filtering Solid Gabor Noise." seems to be the least relevant paper to the patch, since the patch does not implement filtering nor slicing, but only seems to borrow the phase-augmented Gabor kernel with no stated reason or hints.

This is referencing what the OSL implementation was based upon, there was a previous paper from 2009. The slicing is required to produce filtered noise. None of the existing noises in Blender provide anti-aliasing or filtered noise so this was never implemented in the Blender patches.

The Phasor option is rather ambiguous and seems out of place, only for the reviewer to discover that it is described in a completely different paper with no mention of it.

Phasor noise is an extension to Gabor noise. It was trivial to add this as an option. See: https://hal.science/hal-02118508/file/ProceduralPhasorNoise.pdf for additional information.

So my recommendation would be to spend some time documenting the patch and the code as extensively as you can, mentioning any papers that you read, referencing equations that you are implementing, quoting any reasoning that you have came across, and try to make sense of any ambiguous code that you found.

Personally, I would try to rewrite the code away from the OSL code, to make documenting things easier, perhaps look at the reference implementation from the authors.

This patch has a long history, it was based on a patch by Jason Peel https://archive.blender.org/developer/D287. This was abandoned and I picked this up as https://archive.blender.org/developer/D3495 before renewing my efforts with this patch. The D287 code was originally based/ported from the OSL Gabor noise.

This patch already deviates from the OSL code in order to expose different parameters and to provide a version of Gabor that is not tied to OSL. This approach was already accepted by Brecht in some comments on D3495 as an acceptable way forward.

Since this is mainly a port of previous code rather than a new implementation it is difficult to provide some detail you are asking for. I have included the papers for both reference and copyright reasons.

Hopefully this is sufficient information to enable the patch to progress based on its own merits as a new procedural texture.

I have added a todo to implement Fractal Noise based on recent work on Fractal Voronoi and Fractall Noise.

> To be honest, I don't think the patch is ready for review because it lacks any kind of documentation, either in the code or in the patch description. > Even the only given reference "Lagae, A. and Drettakis, G. 2011. Filtering Solid Gabor Noise." seems to be the least relevant paper to the patch, since the patch does not implement filtering nor slicing, but only seems to borrow the phase-augmented Gabor kernel with no stated reason or hints. This is referencing what the OSL implementation was based upon, there was a previous paper from 2009. The slicing is required to produce filtered noise. None of the existing noises in Blender provide anti-aliasing or filtered noise so this was never implemented in the Blender patches. > The Phasor option is rather ambiguous and seems out of place, only for the reviewer to discover that it is described in a completely different paper with no mention of it. Phasor noise is an extension to Gabor noise. It was trivial to add this as an option. See: https://hal.science/hal-02118508/file/ProceduralPhasorNoise.pdf for additional information. > So my recommendation would be to spend some time documenting the patch and the code as extensively as you can, mentioning any papers that you read, referencing equations that you are implementing, quoting any reasoning that you have came across, and try to make sense of any ambiguous code that you found. > > Personally, I would try to rewrite the code away from the OSL code, to make documenting things easier, perhaps look at the reference implementation from the authors. This patch has a long history, it was based on a patch by Jason Peel https://archive.blender.org/developer/D287. This was abandoned and I picked this up as https://archive.blender.org/developer/D3495 before renewing my efforts with this patch. The D287 code was originally based/ported from the OSL Gabor noise. This patch already deviates from the OSL code in order to expose different parameters and to provide a version of Gabor that is not tied to OSL. This approach was already accepted by Brecht in some comments on D3495 as an acceptable way forward. Since this is mainly a port of previous code rather than a new implementation it is difficult to provide some detail you are asking for. I have included the papers for both reference and copyright reasons. Hopefully this is sufficient information to enable the patch to progress based on its own merits as a new procedural texture. I have added a todo to implement Fractal Noise based on recent work on Fractal Voronoi and Fractall Noise.
Member

None of the existing noises in Blender provide anti-aliasing or filtered noise so this was never implemented in the Blender patches.

That's not my point, I am not asking why you haven't implemented filtering. I am just confused why you reference a paper that is not relevant to your code, this will just send reviewers and readers in the wrong direction and leave them confused when they try to map the code they see with the paper they read. I know that because it happened to me.

Phasor noise is an extension to Gabor noise. It was trivial to add this as an option.

I am also not questioning why you added it. I am questioning the fact that you added it with no references or explanations, either in the code or the patch description. Reference the paper in the code/patch and mention the relevant sections and equations with any needed reasoning.

Since this is mainly a port of previous code rather than a new implementation it is difficult to provide some detail you are asking for.

Code that is difficult to document at code submission will be much more difficult to maintain and review. When you submit code, we assume that you will address the review comments regardless if you wrote it or not, that includes documenting it.

Hopefully this is sufficient information to enable the patch to progress based on its own merits as a new procedural texture.

I wouldn't consider this satisfactory, it is likely that other reviewers will skip your comment, so you need to handle such documentation in the code itself and in the patch description. While the information you gave is a start, I would hope for more as I mentioned before.

> None of the existing noises in Blender provide anti-aliasing or filtered noise so this was never implemented in the Blender patches. That's not my point, I am not asking why you haven't implemented filtering. I am just confused why you reference a paper that is not relevant to your code, this will just send reviewers and readers in the wrong direction and leave them confused when they try to map the code they see with the paper they read. I know that because it happened to me. > Phasor noise is an extension to Gabor noise. It was trivial to add this as an option. I am also not questioning why you added it. I am questioning the fact that you added it with no references or explanations, either in the code or the patch description. Reference the paper in the code/patch and mention the relevant sections and equations with any needed reasoning. > Since this is mainly a port of previous code rather than a new implementation it is difficult to provide some detail you are asking for. Code that is difficult to document at code submission will be much more difficult to maintain and review. When you submit code, we assume that you will address the review comments regardless if you wrote it or not, that includes documenting it. > Hopefully this is sufficient information to enable the patch to progress based on its own merits as a new procedural texture. I wouldn't consider this satisfactory, it is likely that other reviewers will skip your comment, so you need to handle such documentation in the code itself and in the patch description. While the information you gave is a start, I would hope for more as I mentioned before.
Hoshinova reviewed 2023-08-17 18:41:01 +02:00
@ -1560,3 +1681,3 @@
SOCKET_IN_FLOAT(detail_scale, "Detail Scale", 0.0f);
SOCKET_IN_FLOAT(detail_roughness, "Detail Roughness", 0.5f);
SOCKET_IN_FLOAT(phase, "Phase Offset", 0.0f);
SOCKET_IN_FLOAT(phase, "Phase Randomness", 0.0f);
Member

Why change the name of the Phase Offset input of the Wave Texture node?

Why change the name of the `Phase Offset` input of the Wave Texture node?
Author
Member

Fixed locally. Although it does highlight we have different terminology here "Detail Scale" vs "Lacunarity".

Fixed locally. Although it does highlight we have different terminology here "Detail Scale" vs "Lacunarity".
CharlieJolly marked this conversation as resolved
Charlie Jolly added 5 commits 2023-08-19 17:30:23 +02:00
a243de001a + Added Normalisation and Fractal options, removing previous implementation of fractal noise
+ Added Roughness, Frequency Lacunarity and Rotation Lacunarity to control fractal noise
+ Added 2d dimension, this computes using a 2x2 grid rather than a 3x3 grid
+ Removed Hybrid Anistropic option (can be added back but I find it is non-intuitive)
+ Added Normal UI control to Anisotropic mode
+ Remove custom RNG and use built in hash noise
+ Add entry to GN menu
Charlie Jolly added 1 commit 2023-08-19 17:33:45 +02:00
buildbot/vexp-code-patch-coordinator Build done. Details
403d99b5d3
Merge branch 'main' into gabor
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly added 1 commit 2023-08-19 18:04:13 +02:00
buildbot/vexp-code-patch-coordinator Build done. Details
231be540a3
Fix build issues raised on build bot
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly added 3 commits 2023-08-19 20:00:45 +02:00
Hoshinova reviewed 2023-08-20 04:04:54 +02:00
@ -0,0 +242,4 @@
float maxamp = 0.0f;
float sum = 0.0f;
float octaves = gp.octaves;
if (gp.roughness == 0.0f || gp.scl_lacunarity == 0.0f) {
Member

Is there a need for this if clause? It makes the behavior different to the other noise nodes when scale is 0.0.

Is there a need for this if clause? It makes the behavior different to the other noise nodes when scale is 0.0.
Author
Member

Removed this clause.

Removed this clause.
CharlieJolly marked this conversation as resolved
Charlie Jolly added 2 commits 2023-08-20 14:20:28 +02:00
Author
Member

Lacunarity fix
image

Lacunarity fix ![image](/attachments/2b0c8e29-ef41-4da2-aaae-7a78b5c23b68)
172 KiB
Hoshinova reviewed 2023-08-20 15:27:09 +02:00
@ -0,0 +242,4 @@
float maxamp = 0.0f;
float sum = 0.0f;
float octaves = gp.octaves;
if (gp.roughness == 0.0f) {
Member

Sorry I misremembered. if (gp.roughness == 0.0f || gp.scl_lacunarity == 0.0f) was actually correct, see: 39a40d6f84/intern/cycles/kernel/svm/voronoi.h (L892)

Could you please change it back?

Sorry I misremembered. `if (gp.roughness == 0.0f || gp.scl_lacunarity == 0.0f)` was actually correct, see: https://projects.blender.org/blender/blender/src/commit/39a40d6f84a658d07860aef60487660a34aa0d3b/intern/cycles/kernel/svm/voronoi.h#L892 Could you please change it back?
Author
Member

This differs between noise and voronoi, I'm not sure which one is actually preferable. If the input is driven then it may make sense to keep it as is so the transition from 0 to 1 is smoother. The gabor function just returns midlevel value when scale lacunarity is zero.

This differs between noise and voronoi, I'm not sure which one is actually preferable. If the input is driven then it may make sense to keep it as is so the transition from 0 to 1 is smoother. The gabor function just returns midlevel value when scale lacunarity is zero.
Member

This differs between noise and voronoi, I'm not sure which one is actually preferable.
The behavior for both noise and voronoi are the same. This is because when scale lacunarity is 0.0 the coordinates of all octaves higher than the base octaves are also (0.0, 0.0, 0.0). Due to how Perlin noise works this then also results in a constant 0.0 output for all higher octaves, which means it doesn't matter if the higher octaves are being computed or not.
But actually I think your way of doing it is actually better as it makes the output continuous as a function of Lacunarity, so I think you should just keep it as is and I'll change Voronoi to also have the same behavior.

>This differs between noise and voronoi, I'm not sure which one is actually preferable. The behavior for both noise and voronoi are the same. This is because when scale lacunarity is 0.0 the coordinates of all octaves higher than the base octaves are also (0.0, 0.0, 0.0). Due to how Perlin noise works this then also results in a constant 0.0 output for all higher octaves, which means it doesn't matter if the higher octaves are being computed or not. But actually I think your way of doing it is actually better as it makes the output continuous as a function of Lacunarity, so I think you should just keep it as is and I'll change Voronoi to also have the same behavior.
CharlieJolly marked this conversation as resolved
Charlie Jolly added 3 commits 2023-08-20 19:12:21 +02:00
Hoshinova reviewed 2023-08-21 14:32:18 +02:00
@ -0,0 +280,4 @@
return mix(sum, sum2, rmd) / maxamp * gp.bandwidth;
}
else {
return sum / maxamp * gp.bandwidth;
Member

Perhaps a little nitpicky but perhaps return sum * gp.bandwidth / maxamp; makes the intention a little clearer as division isn't commutative.

Perhaps a little nitpicky but perhaps `return sum * gp.bandwidth / maxamp;` makes the intention a little clearer as division isn't commutative.
CharlieJolly marked this conversation as resolved
Omar Emara requested changes 2023-08-21 15:04:08 +02:00
@ -0,0 +75,4 @@
/* Calculate impulses per cell. Performance is optimised when impulses are set to whole numbers.
* Uniform distribution is faster than running a poisson for calculating impulses for remaining
* fractional part. */
int impulses_per_cell(int seed, vec3 cell, float impulses)
Member

Why not simply int(impulses)?

Why not simply `int(impulses)`?
Author
Member

It's useful to allow impulses less than 1 to create spotted textures which is why int(impulses) is not directly used. Above 1 this provides a smooth increase in impulses as the input value increases. It could return int(impulses) above 1 but this would give jumps as the impulse value increases.

It's useful to allow impulses less than 1 to create spotted textures which is why `int(impulses)` is not directly used. Above 1 this provides a smooth increase in impulses as the input value increases. It could return `int(impulses)` above 1 but this would give jumps as the impulse value increases.
Author
Member

It's useful to allow impulses less than 1 to create spotted textures which is why int(impulses) is not directly used. Above 1 this provides a smooth increase in impulses as the input value increases. It could return int(impulses) above 1 but this would give jumps as the impulse value increases.

It's useful to allow impulses less than 1 to create spotted textures which is why `int(impulses)` is not directly used. Above 1 this provides a smooth increase in impulses as the input value increases. It could return `int(impulses)` above 1 but this would give jumps as the impulse value increases.
CharlieJolly marked this conversation as resolved
@ -0,0 +87,4 @@
}
/* Calculates the kernel shape that is multiplied by the gaussian envelope. */
vec3 gabor_kernel(GaborParams gp, float freq, vec3 omega, float phi, vec3 position, float g)
Member

Add a note specifying that for non phasor modes, the result is just a broadcast of a single value in a vec3.

Add a note specifying that for non phasor modes, the result is just a broadcast of a single value in a `vec3`.
Author
Member

Note added.

Note added.
CharlieJolly marked this conversation as resolved
@ -0,0 +113,4 @@
h = cos(freq * dot(omega, position) + phi);
}
return vec3(g * h);
Member

Why does this function return a vec3 when the third component is always zero and ignored?

Why does this function return a `vec3` when the third component is always zero and ignored?
Author
Member

Added note to gabor_kernel function, this is due to returning Phasor values.

Added note to gabor_kernel function, this is due to returning Phasor values.
CharlieJolly marked this conversation as resolved
@ -0,0 +155,4 @@
vec3 sum = vec3(0.0);
for (int i = 0; i < num_impulses; ++i) {
vec3 rand_position = vec3(0.0);
if (gp.cell_randomness > 0.0) {
Member

This if seems redundant and can be removed.

This `if` seems redundant and can be removed.
Author
Member

Is this a redundant optimisation?

Is this a redundant optimisation?
Member

I wouldn't call it an optimization, branches like this can needlessly slow down vectorized code, so it is best avoided.

I wouldn't call it an optimization, branches like this can needlessly slow down vectorized code, so it is best avoided.
Author
Member

I assume this is the same for all codebases or just glsl?

I assume this is the same for all codebases or just glsl?
Member

This is probably for all backends, not just GLSL.

This is probably for all backends, not just GLSL.
CharlieJolly marked this conversation as resolved
@ -0,0 +168,4 @@
if (dv <= 1.0) {
vec3 omega;
float phi;
vec3 rand_values = hash_vec4_to_vec3(vec4(cell, float(seed + (num_impulses + i) * 1259)));
Member

If this is simply only passed to gabor_sample, why not call it in gabor_sample directly?

If this is simply only passed to `gabor_sample`, why not call it in `gabor_sample` directly?
CharlieJolly marked this conversation as resolved
@ -0,0 +172,4 @@
gabor_sample(
gp, cell, rand_position, rand_values.x, rand_values.y, rand_values.z, omega, phi);
const float g = (1.0 + M_EPI) * (exp(-M_PI * dv) - M_EPI);
Member

The same for the Gaussian componenet, just compute it in gabor_sample directly.

The same for the Gaussian componenet, just compute it in `gabor_sample` directly.
Author
Member

Added to gabor_kernel function

Added to `gabor_kernel` function
CharlieJolly marked this conversation as resolved
@ -0,0 +430,4 @@
/* Normalise height of noise by the number of impulses. This is empircal as there is no easy way
* to analytically determine the scaling factor for the sum of impulses. */
float g = gabor_fractal_noise(gp, co, scale, int(dimensions), int(periodic));
float impulse_scale = impulses < 1.0 ? (1.2613446229 * sqrt(gp.impulses)) :
Member

What does sqrt(0.75 / pi) correspond to here?

What does `sqrt(0.75 / pi)` correspond to here?
Author
Member

Locally I have updated the comment.

  /* Scale height of noise by the number of impulses. This is empircal as there is no easy way
   * to analytically determine the scaling factor for the sum of impulses.
   * Phasor does not require this because it is always returns [-1,1].
   * Following calculation from Lee Bruemmer OSL script implementation
   * Scale the noise so the range [-1,1] covers 6 standard deviations, or 3*sqrt(variance)
   * Since the radius is set to 1/a the scale simplifies to 3/4*sqrt(3*n/(pi*sqrt(2)))
   * float scale = 0.6162961511 * sqrt( Impulses ); but with a fixed number of impulses also divide
   * by sqrt(0.75/pi) (0.4886025119) which give 1.2613446229 * sqrt(gp.impulses).
   * In tests I've found that sqrt(2) 1.41 clips less when clamped.
   */
Locally I have updated the comment. ``` /* Scale height of noise by the number of impulses. This is empircal as there is no easy way * to analytically determine the scaling factor for the sum of impulses. * Phasor does not require this because it is always returns [-1,1]. * Following calculation from Lee Bruemmer OSL script implementation * Scale the noise so the range [-1,1] covers 6 standard deviations, or 3*sqrt(variance) * Since the radius is set to 1/a the scale simplifies to 3/4*sqrt(3*n/(pi*sqrt(2))) * float scale = 0.6162961511 * sqrt( Impulses ); but with a fixed number of impulses also divide * by sqrt(0.75/pi) (0.4886025119) which give 1.2613446229 * sqrt(gp.impulses). * In tests I've found that sqrt(2) 1.41 clips less when clamped. */ ```
CharlieJolly marked this conversation as resolved
@ -0,0 +436,4 @@
/* Normalise and clamp noise to [0.1] range. */
if (int(use_normalize) == 1) {
g = clamp(0.5 * g + 0.5, 0.0, 1.0);
Member

Does clamping here mean we will get flat areas? Can't we adjust the scale factor to ensure a correct range instead?

Does clamping here mean we will get flat areas? Can't we adjust the scale factor to ensure a correct range instead?
Author
Member

See previous comment. Due to the way the impulse sum, it is a compromise between a scale factor that is too low and clips and is too high and creates a mid level grey 0.5 texture when scale factor is too high.

See previous comment. Due to the way the impulse sum, it is a compromise between a scale factor that is too low and clips and is too high and creates a mid level grey 0.5 texture when scale factor is too high.
Member

Honestly, I'm also not entirely convinced by this solution mainly because it loses all the detail in the highs and lows.
I thought of a solution which involves using a piecewise function which would need to converge towards 0.0 when x -> -infinity and 1.0 when x -> +infinity. The function should simply be linear in the middle part and be continuously differentiable everywhere.
That way we could both remap it into a [0.0, 1.0] range and preserve all details, however I'd first need to think of a fitting function, so if necessary I'd do it in a separate PR after this one has been merged.

But ideally @CharlieJolly can find a better way.

Honestly, I'm also not entirely convinced by this solution mainly because it loses all the detail in the highs and lows. I thought of a solution which involves using a piecewise function which would need to converge towards 0.0 when x -> -infinity and 1.0 when x -> +infinity. The function should simply be linear in the middle part and be continuously differentiable everywhere. That way we could both remap it into a [0.0, 1.0] range **and** preserve all details, however I'd first need to think of a fitting function, so if necessary I'd do it in a separate PR after this one has been merged. But ideally @CharlieJolly can find a better way.
Author
Member

Some kind of gain and bias function would be useful to do this I think. Users can run this through a float curve but I'm not sure there is a perfect solution here.

Some kind of gain and bias function would be useful to do this I think. Users can run this through a float curve but I'm not sure there is a perfect solution here.
Member

If you can't think of anything immediately I think it's also fine to leave it for now.
I'll think about an appropriate function in the meantime.

If you can't think of anything immediately I think it's also fine to leave it for now. I'll think about an appropriate function in the meantime.
CharlieJolly marked this conversation as resolved
Charlie Jolly added 6 commits 2023-08-22 04:24:24 +02:00
fec2f071d3 Don't scale phasor based on impulses as this is already normalised by default
Split fractal params from GaborParams into its own struct
Address some code comments from Omar and Hoshinova
Change default frequency to 16
buildbot/vexp-code-patch-coordinator Build done. Details
fcd07d1c09
Additional comments and tidy
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly added 2 commits 2023-08-22 11:44:23 +02:00
Omar Emara requested changes 2023-08-29 11:49:17 +02:00
@ -0,0 +42,4 @@
*/
#ifndef M_EPI
# define M_EPI 0.0432139182637722
Member

What is M_EPI?

What is `M_EPI`?
Author
Member

Added comment to define: e^(-PI)

Added comment to define: e^(-PI)
CharlieJolly marked this conversation as resolved
@ -0,0 +157,4 @@
float pvar = mix(0.0, rand_values.z * 2.0 - 1.0, gp.phase_variance);
phi = M_2PI * pvar + gp.phase;
/* Anisotropic direction. */
Member

I am still not sure why the anisotropic and isotropic types can't be joined by defining an annular sector as described in section "3.2 Noise with Controllable Band-Limits" of the paper.

I am still not sure why the anisotropic and isotropic types can't be joined by defining an annular sector as described in section "3.2 Noise with Controllable Band-Limits" of the paper.
Author
Member

I can add back the Hybrid mode.

I can add back the Hybrid mode.
Member

@CharlieJolly So the hybrid mode was based on that section of the paper?

@CharlieJolly So the hybrid mode was based on that section of the paper?
Author
Member

Well TBH I'm not sure, reading it again this looks like a feature of their application which is no longer available it seems. In this case I believe this can be achieved using the Rotation Variance control or by using the fractal noise options.

Well TBH I'm not sure, reading it again this looks like a feature of their application which is no longer available it seems. In this case I believe this can be achieved using the Rotation Variance control or by using the fractal noise options.
Member

Well, the hybrid mode is actually described in section 3.3 of:

Lagae, Ares, and George Drettakis. "Filtering solid Gabor noise." ACM Transactions on Graphics (TOG) 30.4 (2011): 1-6.

But what I am after is removing the mode altogether and replacing it with intutive parameters that describe and annular sector as mentioned before.

Well, the hybrid mode is actually described in section 3.3 of: Lagae, Ares, and George Drettakis. "Filtering solid Gabor noise." ACM Transactions on Graphics (TOG) 30.4 (2011): 1-6. But what I am after is removing the mode altogether and replacing it with intutive parameters that describe and annular sector as mentioned before.
Author
Member

@OmarEmaraDev this is achieved using the Rotation and Rotation Variance controls.

image

@OmarEmaraDev this is achieved using the Rotation and Rotation Variance controls. ![image](/attachments/e4d3e0a6-737b-466c-8c23-2eba581bdebd)
Member

Is isotropic the same as Rotation Variance = 360?

Is isotropic the same as `Rotation Variance = 360`?
Author
Member

Yes, in OSL implementation we have the angular frequency set to a random number

float omega_t = float(M_TWO_PI) * rng();

in this patch this is contolled by Rotation and Rotation Variance.

float ovar = M_PI * (rand_values.x * 2.0 - 1.0);
float omega_t = ovar * gp.rot_variance - gp.rotation;
Yes, in OSL implementation we have the angular frequency set to a random number ``` float omega_t = float(M_TWO_PI) * rng(); ``` in this patch this is contolled by Rotation and Rotation Variance. ``` float ovar = M_PI * (rand_values.x * 2.0 - 1.0); float omega_t = ovar * gp.rot_variance - gp.rotation; ```
Member

Okay, so why not remove the enum, rename Rotation Variance to Anisotropy, and unify the Direction and Rotation inputs?

Okay, so why not remove the enum, rename Rotation Variance to Anisotropy, and unify the Direction and Rotation inputs?
Author
Member

Okay, so why not remove the enum, rename Rotation Variance to Anisotropy, and unify the Direction and Rotation inputs?

I did think about this but found it tricky to do this in a nice way. The Rotation controls are not shown on Anisotropic mode as it is only controlled by the direction vector. Hybrid mixed both these modes so that maybe something to consider adding back.

> Okay, so why not remove the enum, rename Rotation Variance to Anisotropy, and unify the Direction and Rotation inputs? I did think about this but found it tricky to do this in a nice way. The Rotation controls are not shown on Anisotropic mode as it is only controlled by the direction vector. Hybrid mixed both these modes so that maybe something to consider adding back.
Member

I really think we should try to reduce the number of options and inputs in the node as much as we can, because those inputs are not all orthogonal, so the node is hard to use for the average user.

I really think we should try to reduce the number of options and inputs in the node as much as we can, because those inputs are not all orthogonal, so the node is hard to use for the average user.
Author
Member

The number of inputs is definitely increased by adding the fractal controls. Removing those alone would reduce inputs by five but at the loss of that feature. (@Hoshinova)

image

Node panels patch may help here.

Otherwise it is difficult to reduce the params without compromising the features.

In comparison to the Principled BSDF.

image

The number of inputs is definitely increased by adding the fractal controls. Removing those alone would reduce inputs by five but at the loss of that feature. (@Hoshinova) ![image](/attachments/213c08e3-2dfa-4c3b-8d9c-c6b451629ebc) Node panels patch may help here. Otherwise it is difficult to reduce the params without compromising the features. In comparison to the Principled BSDF. ![image](/attachments/d12b218d-3582-496e-9f24-7e0c5bf624a7)

Please stop this project and look into adding loops to shaders and reworking all fractal noise texture types into a node group asset)

Please stop this project and look into adding loops to shaders and reworking all fractal noise texture types into a node group asset)
Author
Member

Please stop this project and look into adding loops to shaders and reworking all fractal noise texture types into a node group asset)

Voronoi and Noise have both recently had fractal noise modes added. That is why this was added to this node.

I can't comment on the feasibility of adding loops to shading nodes. Don't forget that for any shading node, there has to be three implementations for Eevee, Cycles and OSL.

> Please stop this project and look into adding loops to shaders and reworking all fractal noise texture types into a node group asset) Voronoi and Noise have both recently had fractal noise modes added. That is why this was added to this node. I can't comment on the feasibility of adding loops to shading nodes. Don't forget that for any shading node, there has to be three implementations for Eevee, Cycles and OSL.
Author
Member

The UI has now been grouped using the new Panel feature.

The UI has now been grouped using the new Panel feature.
CharlieJolly marked this conversation as resolved
@ -0,0 +181,4 @@
int num_impulses = impulses_per_cell(seed, cell, gp.impulses);
vec3 sum = vec3(0.0);
for (int i = 0; i < num_impulses; ++i) {
vec3 rand_position = vec3(0.0);
Member

Unnecessary separation between declaration and initialization, just do vec3 rand_position = mix(...).

Unnecessary separation between declaration and initialization, just do `vec3 rand_position = mix(...)`.
CharlieJolly marked this conversation as resolved
@ -0,0 +465,4 @@
if (gp.mode == SHD_GABOR_MODE_GABOR || gp.mode == SHD_GABOR_MODE_RING ||
gp.mode == SHD_GABOR_MODE_CROSS || gp.mode == SHD_GABOR_MODE_SQUARE)
{
float impulse_scale = impulses > 1.0 ? 1.2613446229 * sqrt(gp.impulses) : 1.2613446229;
Member

The reference implementation from the paper seem to compute a scaling factor that is a function of the noise variance, which is in turn a function of the bandwidth, while this doesn't seem to be reflected here. Can you look into that and perhaps provide some justification?

It also seems weird that we are scaling fractal noise instead of the noise itself, which is the opposite of what we do for other noise types. If the performance implications are not significant, we should consider scaling th noise itself.

The reference implementation from the paper seem to compute a scaling factor that is a function of the noise variance, which is in turn a function of the bandwidth, while this doesn't seem to be reflected here. Can you look into that and perhaps provide some justification? It also seems weird that we are scaling fractal noise instead of the noise itself, which is the opposite of what we do for other noise types. If the performance implications are not significant, we should consider scaling th noise itself.
Author
Member

This scaling factor here was simplification used in a script implementation by Lee Bruemmer.

The OSL reference uses the following based on the bandwidth value.

float gabor_variance = 1.0f / (4.0f * sqrtf(2.0) * (gp.a * gp.a * gp.a)); = 0.17677669529
float scale          = 1.0f / (3.0f * sqrtf(gabor_variance)); = 0.792804755

As mentioned in the notes and from my tests there is no ideal value here.

Regarding, fractal noise, the scaling has a separate lacunarity control that affects the the frequency of the Gabor noise directly.

The base noise function is gabor_grid_3d and gabor_grid_2d which is called by gabor_fractal_noise. Frequency and rotation are exposed as fractal noise params of the underlying gabor noise.

This scaling factor here was simplification used in a script implementation by Lee Bruemmer. The OSL reference uses the following based on the bandwidth value. ``` float gabor_variance = 1.0f / (4.0f * sqrtf(2.0) * (gp.a * gp.a * gp.a)); = 0.17677669529 float scale = 1.0f / (3.0f * sqrtf(gabor_variance)); = 0.792804755 ``` As mentioned in the notes and from my tests there is no ideal value here. Regarding, fractal noise, the scaling has a separate lacunarity control that affects the the frequency of the Gabor noise directly. The base noise function is `gabor_grid_3d` and `gabor_grid_2d` which is called by `gabor_fractal_noise`. Frequency and rotation are exposed as fractal noise params of the underlying gabor noise.
CharlieJolly marked this conversation as resolved
Charlie Jolly added 2 commits 2023-08-29 16:19:51 +02:00
Charlie Jolly added 2 commits 2023-08-30 01:10:27 +02:00
Charlie Jolly added 5 commits 2023-08-30 16:22:14 +02:00
Charlie Jolly added 2 commits 2023-08-30 17:40:14 +02:00
buildbot/vexp-code-patch-coordinator Build done. Details
f2c8f0f888
Merge branch 'main' into gabor
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Author
Member

Gabor noise does exhibit some discontinuities because the gaussian envelope is not continuous.

This is noted in Tavernier et. al. 2018, Gabor noise revisited which includes the improved gaussian that is used in this patch.

However, I've experimented with using a cubic pulse envelope which is smoother at the boundary.

This does alter some aspects of the noise output but I think it maybe a good compromise.

The image shows the differences. These are especially noticeable when zooming into the noise or using the noise for displacement.

Gabor noise does exhibit some discontinuities because the gaussian envelope is not continuous. This is noted in Tavernier et. al. 2018, Gabor noise revisited which includes the improved gaussian that is used in this patch. However, I've experimented with using a cubic pulse envelope which is smoother at the boundary. This does alter some aspects of the noise output but I think it maybe a good compromise. The image shows the differences. These are especially noticeable when zooming into the noise or using the noise for displacement.
Member

@CharlieJolly Can't we just increase the cutoff point of truncating the gabor kernel?

To quote the paper:

If required, this discontinuity could be alleviated by truncating at a larger kernel radius, or avoided by multiplying with a continuous window function.

It also seems weird that those discontinuities are that visible even though the paper mentions they are not significant even in the noise derivatives, do they mean when the iterations are high enough?

which in our experience does not affect the visual appearance, even when the noise derivative is used [Perlin 2002].

@CharlieJolly Can't we just increase the cutoff point of truncating the gabor kernel? To quote the paper: > If required, this discontinuity could be alleviated by truncating at a larger kernel radius, or avoided by multiplying with a continuous window function. It also seems weird that those discontinuities are that visible even though the paper mentions they are not significant even in the noise derivatives, do they mean when the iterations are high enough? > which in our experience does not affect the visual appearance, even when the noise derivative is used [Perlin 2002].
Author
Member

@CharlieJolly Can't we just increase the cutoff point of truncating the gabor kernel?

To quote the paper:

If required, this discontinuity could be alleviated by truncating at a larger kernel radius, or avoided by multiplying with a continuous window function.

It also seems weird that those discontinuities are that visible even though the paper mentions they are not significant even in the noise derivatives, do they mean when the iterations are high enough?

which in our experience does not affect the visual appearance, even when the noise derivative is used [Perlin 2002].

They might be talking about filtering in regards to derivatives. I'm not sure.

In Gabor Noise Revisited...

... and using a continuous envelope instead of truncated Gaussian (e.g. the C0 alternative 1 + exp(−π) exp(−π||x||2− exp(−π), or C1 KaiserBessel window functions) would avoid slight artifacts at splat borders for very low N or if the noise is meant to be differentiated (e.g. for bump maps).

They maybe downplaying it. The current patch is using the newer C0 alternative already.

The example I have shown to highlight the issue is at a high zoom level but the displacement is not. It is slightly exaggerated by the cavity shader.

> @CharlieJolly Can't we just increase the cutoff point of truncating the gabor kernel? > > To quote the paper: > > > If required, this discontinuity could be alleviated by truncating at a larger kernel radius, or avoided by multiplying with a continuous window function. > > It also seems weird that those discontinuities are that visible even though the paper mentions they are not significant even in the noise derivatives, do they mean when the iterations are high enough? > > > which in our experience does not affect the visual appearance, even when the noise derivative is used [Perlin 2002]. They might be talking about filtering in regards to derivatives. I'm not sure. In Gabor Noise Revisited... *... and using a continuous envelope instead of truncated Gaussian (e.g. the C0 alternative `1 + exp(−π) exp(−π||x||2− exp(−π)`, or C1 KaiserBessel window functions) would avoid slight artifacts at splat borders for very low N or if the noise is meant to be differentiated (e.g. for bump maps).* They maybe downplaying it. The current patch is using the newer C0 alternative already. The example I have shown to highlight the issue is at a high zoom level but the displacement is not. It is slightly exaggerated by the cavity shader.
Member

I think we should reconsider the big picture here. I don't think we should continue to add thousands of lines of code (which bloats the Cycles kernel, has to be maintained, refactored, optimized in the future, or even implemented again if we want to add another backend) to add these high-level noise types. Instead we should focus on working on the base features of the node system to allow such things to be created and shared as node groups.

Such work wouldn't just benefit a single noise texture node, but add possibilities (like repeat loops) to any node setup, improving flexibility everywhere.

Especially as the noise textures have become higher level combinations of more features in recent changes, I don't think the overall design makes sense anymore.

I think we should reconsider the big picture here. I don't think we should continue to add thousands of lines of code (which bloats the Cycles kernel, has to be maintained, refactored, optimized in the future, or even implemented _again_ if we want to add another backend) to add these high-level noise types. Instead we should focus on working on the base features of the node system to allow such things to be created and shared as node groups. Such work wouldn't just benefit a single noise texture node, but add possibilities (like repeat loops) to any node setup, improving flexibility everywhere. Especially as the noise textures have become higher level combinations of more features in recent changes, I don't think the overall design makes sense anymore.
Author
Member

I think we should reconsider the big picture here. I don't think we should continue to add thousands of lines of code (which bloats the Cycles kernel, has to be maintained, refactored, optimized in the future, or even implemented again if we want to add another backend) to add these high-level noise types. Instead we should focus on working on the base features of the node system to allow such things to be created and shared as node groups.

Such work wouldn't just benefit a single noise texture node, but add possibilities (like repeat loops) to any node setup, improving flexibility everywhere.

Especially as the noise textures have become higher level combinations of more features in recent changes, I don't think the overall design makes sense anymore.

This patch has some history, I picked it up again last year and added support for GN. Then I had a break and picked it up again after @pablovazquez mentioned that adding more noise options was the number one request on Right Click Select and the recent development of adding Fractal options to Voronoi and Noise.

image

I can somewhat understand the argument about implementing the fractal part of the patch using loop nodes but they aren't available on the shading side.

> I think we should reconsider the big picture here. I don't think we should continue to add thousands of lines of code (which bloats the Cycles kernel, has to be maintained, refactored, optimized in the future, or even implemented _again_ if we want to add another backend) to add these high-level noise types. Instead we should focus on working on the base features of the node system to allow such things to be created and shared as node groups. > > Such work wouldn't just benefit a single noise texture node, but add possibilities (like repeat loops) to any node setup, improving flexibility everywhere. > > Especially as the noise textures have become higher level combinations of more features in recent changes, I don't think the overall design makes sense anymore. This patch has some history, I picked it up again last year and added support for GN. Then I had a break and picked it up again after @pablovazquez mentioned that adding more noise options was the number one request on Right Click Select and the recent development of adding Fractal options to Voronoi and Noise. ![image](/attachments/da60a132-71e7-49f4-94e3-a717e255bb9a) I can somewhat understand the argument about implementing the fractal part of the patch using loop nodes but they aren't available on the shading side.
Member

This patch has some history

I see where you're coming from, but the patch having history doesn't show that it's the right choice from a design/architecture perspective.

the number one request on Right Click Select

Again, I see where you're coming from, but RCS is just feature requests, it doesn't include how they should be available and designed in Blender.

implementing the fractal part of the patch using loop nodes but they aren't available on the shading side.

That's exactly my point. There's a fundamental feature that should be implemented in shader nodes as well. Getting that working would be a much more valuable use of time in my opinion. Before we start coding a feature, we should think about how we could split it into more general, widely applicable projects.

>This patch has some history I see where you're coming from, but the patch having history doesn't show that it's the right choice from a design/architecture perspective. >the number one request on Right Click Select Again, I see where you're coming from, but RCS is just feature requests, it doesn't include how they should be available and designed in Blender. >implementing the fractal part of the patch using loop nodes but they aren't available on the shading side. That's exactly my point. There's a fundamental feature that should be implemented in shader nodes as well. Getting that working would be a much more valuable use of time in my opinion. Before we start coding a feature, we should think about how we could split it into more general, widely applicable projects.
Charlie Jolly added 2 commits 2023-09-13 15:36:54 +02:00
9653d1fe17 Merge branch 'main' into gabor
# Conflicts:
#	scripts/startup/nodeitems_builtins.py
Charlie Jolly added 4 commits 2023-09-18 22:50:43 +02:00
0583c44b03 Use cosine envelope instead of gaussian to reduce artefacts.
Change base frequency factor for better UX.
Change Bandwidth to Gain.
Add remaining sockets to Params panel, currently it is
expected that panels only appear after sockets.
Normalise omega in all cases.
Add directional randomness to all modes.
Author
Member

Screenshot to provide rationale for changing gaussian envelope to use smooth cosine envelope instead to reduce artefacts. This is especially noticeable when used for displacement. Cavity shading is turned on for these screenshots which exagerates the artefacts.

Screenshot to provide rationale for changing gaussian envelope to use smooth cosine envelope instead to reduce artefacts. This is especially noticeable when used for displacement. Cavity shading is turned on for these screenshots which exagerates the artefacts.
Charlie Jolly added 4 commits 2023-09-29 14:03:43 +02:00
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly added 2 commits 2023-09-30 17:30:46 +02:00
Author
Member

Last commit removed Anisotropic modes.

image

Last commit removed Anisotropic modes. ![image](/attachments/081d57b2-6654-41bd-89a4-b2c35fbf62a7)
115 KiB
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly added 1 commit 2023-09-30 18:36:16 +02:00
buildbot/vexp-code-patch-coordinator Build done. Details
37968e3b74
Revert unintended change in noise glsl
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly added 2 commits 2023-09-30 21:39:19 +02:00
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly requested review from Omar Emara 2023-09-30 23:24:11 +02:00
Charlie Jolly added 1 commit 2023-10-01 00:18:18 +02:00
Author
Member

UI: Rename Base Frequency to Frequency and move to Kernel panel

image

UI: Rename Base Frequency to Frequency and move to Kernel panel ![image](/attachments/d0a40b93-71c4-4e14-9a0c-e989394be20e)
Charlie Jolly added 1 commit 2023-10-01 01:21:19 +02:00
Charlie Jolly added 1 commit 2023-10-01 04:27:04 +02:00
Charlie Jolly added 3 commits 2023-10-01 17:36:41 +02:00
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly added 2 commits 2023-10-02 12:31:52 +02:00
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Author
Member

@blender-bot package windows

@blender-bot package windows
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR110802) when ready.
Charlie Jolly added 1 commit 2023-10-11 23:09:30 +02:00
Charlie Jolly added 1 commit 2023-10-12 11:14:57 +02:00
Charlie Jolly added 1 commit 2023-10-26 11:57:40 +02:00
This pull request has changes conflicting with the target branch.
  • intern/cycles/blender/shader.cpp
  • intern/cycles/kernel/svm/node_types_template.h
  • source/blender/blenkernel/BKE_node.h
  • source/blender/makesdna/DNA_node_types.h
  • source/blender/makesrna/intern/rna_nodetree.cc

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u gabor:CharlieJolly-gabor
git checkout CharlieJolly-gabor
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No Assignees
6 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#110802
No description provided.