WIP: Node: Gabor Noise Texture #110802
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
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
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
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
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
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
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
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 project
No Assignees
6 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#110802
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "CharlieJolly/blender:gabor"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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:
This is more performant when only 2D texture is required.
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.
trivial to add.
similar way to distance metrics in Voronoi.
additive fractal noise.
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
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
Comparison with OSL native implementation, why Bandwidth is split into Frequency.
Artefacts and use of cosine envelope instead of gaussian
Kernel shapes shown in 3D
Latest UI
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@blender-bot build
@blender-bot package
Package build started. Download here when ready.
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)
Brackets to follow Blender's coding style.
@ -0,0 +18,4 @@
return chash * 1519588931;
}
float gabor_rng_uniform(inout int rng)
Can't we use our own random functions? Is there an advantage to using those?
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.
But is it necessary to use an RNG at all? Why not use our pseudo-number-generator with the impulse index as an input?
Updated to use built in hash functions
@ -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 */
Can we find a way to incorporate all three options into a single factor input that controls the amount of anisotropy?
This has been slightly refactored but combining to a single factor is not feasible for Gabor noise.
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.
@ -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)
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?
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.
Implemented fractal based on other noise functions.
@ -0,0 +264,4 @@
int anisotropic,
int periodic)
{
if (impulses == 0.0) {
This seems like the kind of thing that you can sanitize before passing it to the shader.
This is partly implemented in Cycles using constant folding. Do you have an example for Eevee?
@CharlieJolly That's probably just a:
But I just realized that the impulses are not constant, so you can ignore my comment here.
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.
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.
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.
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.
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.
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.
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.
@ -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);
Why change the name of the
Phase Offset
input of the Wave Texture node?Fixed locally. Although it does highlight we have different terminology here "Detail Scale" vs "Lacunarity".
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@ -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) {
Is there a need for this if clause? It makes the behavior different to the other noise nodes when scale is 0.0.
Removed this clause.
Lacunarity fix
@ -0,0 +242,4 @@
float maxamp = 0.0f;
float sum = 0.0f;
float octaves = gp.octaves;
if (gp.roughness == 0.0f) {
Sorry I misremembered.
if (gp.roughness == 0.0f || gp.scl_lacunarity == 0.0f)
was actually correct, see:Could you please change it back?
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.
@ -0,0 +280,4 @@
return mix(sum, sum2, rmd) / maxamp * gp.bandwidth;
}
else {
return sum / maxamp * gp.bandwidth;
Perhaps a little nitpicky but perhaps
return sum * gp.bandwidth / maxamp;
makes the intention a little clearer as division isn't commutative.@ -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)
Why not simply
int(impulses)
?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 returnint(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 returnint(impulses)
above 1 but this would give jumps as the impulse value increases.@ -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)
Add a note specifying that for non phasor modes, the result is just a broadcast of a single value in a
vec3
.Note added.
@ -0,0 +113,4 @@
h = cos(freq * dot(omega, position) + phi);
}
return vec3(g * h);
Why does this function return a
vec3
when the third component is always zero and ignored?Added note to gabor_kernel function, this is due to returning Phasor values.
@ -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) {
This
if
seems redundant and can be removed.Is this a redundant optimisation?
I wouldn't call it an optimization, branches like this can needlessly slow down vectorized code, so it is best avoided.
I assume this is the same for all codebases or just glsl?
This is probably for all backends, not just GLSL.
@ -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)));
If this is simply only passed to
gabor_sample
, why not call it ingabor_sample
directly?@ -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);
The same for the Gaussian componenet, just compute it in
gabor_sample
directly.Added to
gabor_kernel
function@ -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)) :
What does
sqrt(0.75 / pi)
correspond to here?Locally I have updated the comment.
@ -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);
Does clamping here mean we will get flat areas? Can't we adjust the scale factor to ensure a correct range instead?
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.
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.
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.
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.
@blender-bot package
Package build started. Download here when ready.
@ -0,0 +42,4 @@
*/
#ifndef M_EPI
# define M_EPI 0.0432139182637722
What is
M_EPI
?Added comment to define: e^(-PI)
@ -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. */
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 can add back the Hybrid mode.
@CharlieJolly So the hybrid mode was based on that section of the paper?
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, 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.
@OmarEmaraDev this is achieved using the Rotation and Rotation Variance controls.
Is isotropic the same as
Rotation Variance = 360
?Yes, in OSL implementation we have the angular frequency set to a random number
in this patch this is contolled by Rotation and Rotation Variance.
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.
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.
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)
Node panels patch may help here.
Otherwise it is difficult to reduce the params without compromising the features.
In comparison to the Principled BSDF.
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.
The UI has now been grouped using the new Panel feature.
@ -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);
Unnecessary separation between declaration and initialization, just do
vec3 rand_position = mix(...)
.@ -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;
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.
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.
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
andgabor_grid_2d
which is called bygabor_fractal_noise
. Frequency and rotation are exposed as fractal noise params of the underlying gabor noise.@blender-bot package
Package build started. Download here when ready.
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.
@CharlieJolly Can't we just increase the cutoff point of truncating the gabor kernel?
To quote the paper:
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?
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.
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.
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 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.
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.
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.
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.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
Last commit removed Anisotropic modes.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
UI: Rename Base Frequency to Frequency and move to Kernel panel
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package windows
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
Node: Gabor Noise Textureto WIP Node: Gabor Noise Texture916534f418
to890042517b
Patch updated to main only for test and comparison to new gabor implementation.
WIP Node: Gabor Noise Textureto WIP: WIP Node: Gabor Noise TextureWIP: WIP Node: Gabor Noise Textureto WIP: Node: Gabor Noise TextureCheckout
From your project repository, check out a new branch and test the changes.