Add Fractal Voronoi Noise V.2 #106827

Merged
Jacques Lucke merged 77 commits from Hoshinova/blender:add-fractal-voronoi  into main 2023-06-13 09:18:18 +02:00
Member

A continuation of https://archive.blender.org/developer/D16262

Following the discussions there I decided to work more closely with artists which resulted in this thread on blenderartists.org.

Using their suggestions I updated my original implementation of Fractal Voronoi Noise to Fractal Voronoi Noise V.2.

Since there are some differences between the original implementation and V.2 I will write a new documentation here:

For the F1, Smooth F1, F2 Feature:

Distance and Color outputs:
Multiple Layers of Voronoi Noise are computed and outputs of each layer added together to create the final output.

The amount of layers is being controlled by the "Detail" input.
If the "Detail" input has a non-integer value two instances of Fractal Voronoi will be computed with each having the greatest whole integer smaller than the "Detail" input value and the smallest whole integer greater than the "Detail" input value as the amount of layers respectively.
The fractional part of "Detail" will then be used as a factor to do a linear interpolation between the two instances.

The Distance output of each layer is being multiplied by the "Roughness" input to the power of the current layer-1.
This means that for example if the "Roughness" input is 2.0 the the Distance output of the first layer is being multiplied by 2.0 ^ 0 = 1.0 for the second layer it being multiplied by 2.0 ^ 1 = 2.0 for the third layer it is being multiplied by 2.0 ^ 2 = 4.0 and so on.

The Scale input of each layer is being multiplied by the "Lacunarity" input to the power of the current layer-1.

Position output:
Essentially the same as with the Distance and Color outputs but instead of adding the layers together a linear interpolation is performed using the Roughness input as the lerp factor.

For the Distance to Edge Feature:

Multiple Layers of Voronoi Distance to Edge Textures are computed and the resulting Distance output is the minimum of the Distance output of all layers.

The amount of layers is being controlled by the "Detail" input.
If the "Detail" input has a non-integer value two instances of Fractal Voronoi will be computed with each having the greatest whole integer smaller than the "Detail" input value and the smallest whole integer greater than the "Detail" input value as the amount of layers respectively.
The fractional part of "Detail" will then be used as a factor to do a linear interpolation between the two instances.

The Roughness input is used to lerp between the current layer and the previous layer.

The Scale input of each layer is being multiplied by the "Lacunarity" input to the power of the current layer-1.

Additionally a Normalize boolean input is added which if checked will remap the Distance and Color outputs to a [0.0, 1.0] range.

Unlike for the other features F2 normalization doesn't actually calculate the maximal amplitude.
This is mainly because the actual maximal amplitude of F2 is very expensive to compute.
Thus it instead it uses the maximal amplitude of F1 scaled by a factor of 2.0, with the logic behind that being that the highest F2 Distance output is always \leq the highest F1 Distance output times 2.0.

As in the original diff thread there were requests for more practical use cases of Fractal Voronoi noise I will include some selected ones here:

Landscapes by Lumpengnom:

Fabric Patterns by KickAir_8P:

Hand Painted Wall by Hoshinova:

Unfortunately projects.blender.org won't let me upload more pictures but I can share some more variations in a comment if you want.

Apart from the already amazing still pictures shown above I want to specifically mention Tenkai Raiko's contribution to this.

Essentially he discovered that Fractal Voronoi noise can be used to procedurally simulate all kinds of smoke and fluid simulations using volumes.
Apart from being able to create procedural simulations of both arbitrary size and resolution with virtually no RAM overhead he has also shown that it gives the user a level of control not easily achievable using traditional simulaton methods.
All of this can be seen in the following video he posted to demonstrate said use cases:
Please respect the creators wish and don't copy, repost or reupload any parts of the video.

Thank you for reading and reviewing my Pull Request!

A continuation of https://archive.blender.org/developer/D16262 Following the discussions there I decided to work more closely with artists which resulted in [this thread on blenderartists.org](https://blenderartists.org/t/your-help-is-needed-for-adding-fractal-voronoi-noise-into-blender/1448508). Using their suggestions I updated my original implementation of Fractal Voronoi Noise to Fractal Voronoi Noise V.2. Since there are some differences between the original implementation and V.2 I will write a new documentation here: **For the F1, Smooth F1, F2 Feature:** Distance and Color outputs: Multiple Layers of Voronoi Noise are computed and outputs of each layer added together to create the final output. The amount of layers is being controlled by the "Detail" input. If the "Detail" input has a non-integer value two instances of Fractal Voronoi will be computed with each having the greatest whole integer smaller than the "Detail" input value and the smallest whole integer greater than the "Detail" input value as the amount of layers respectively. The fractional part of "Detail" will then be used as a factor to do a linear interpolation between the two instances. The Distance output of each layer is being multiplied by the "Roughness" input to the power of the current layer-1. This means that for example if the "Roughness" input is 2.0 the the Distance output of the first layer is being multiplied by 2.0 ^ 0 = 1.0 for the second layer it being multiplied by 2.0 ^ 1 = 2.0 for the third layer it is being multiplied by 2.0 ^ 2 = 4.0 and so on. The Scale input of each layer is being multiplied by the "Lacunarity" input to the power of the current layer-1. Position output: Essentially the same as with the Distance and Color outputs but instead of adding the layers together a linear interpolation is performed using the Roughness input as the lerp factor. **For the Distance to Edge Feature:** Multiple Layers of Voronoi Distance to Edge Textures are computed and the resulting Distance output is the minimum of the Distance output of all layers. The amount of layers is being controlled by the "Detail" input. If the "Detail" input has a non-integer value two instances of Fractal Voronoi will be computed with each having the greatest whole integer smaller than the "Detail" input value and the smallest whole integer greater than the "Detail" input value as the amount of layers respectively. The fractional part of "Detail" will then be used as a factor to do a linear interpolation between the two instances. The Roughness input is used to lerp between the current layer and the previous layer. The Scale input of each layer is being multiplied by the "Lacunarity" input to the power of the current layer-1. Additionally a Normalize boolean input is added which if checked will remap the Distance and Color outputs to a [0.0, 1.0] range. Unlike for the other features F2 normalization doesn't actually calculate the maximal amplitude. This is mainly because the actual maximal amplitude of F2 is very expensive to compute. Thus it instead it uses the maximal amplitude of F1 scaled by a factor of 2.0, with the logic behind that being that the highest F2 Distance output is always $\leq$ the highest F1 Distance output times 2.0. As in the [original diff thread](https://archive.blender.org/developer/D16262) there were requests for more practical use cases of Fractal Voronoi noise I will include some selected ones here: Landscapes by Lumpengnom: ![Mountains 1.png](/attachments/41cdd65a-e595-491c-a9d9-0da9e1c876b7) ![Icy Landscape.png](/attachments/811a9d8e-2c99-426d-a937-3fd52da01def) ![Landcape 1.png](/attachments/eb11b70d-3cd7-41d2-becc-657ae1fb330a) Fabric Patterns by KickAir_8P: ![Stitched Elephant.png](/attachments/2b0b72ca-606a-4f69-9d16-d7b5276b9ca4) Hand Painted Wall by Hoshinova: ![Hand Painted Wall.png](/attachments/e2ac3c25-28f2-48ac-a692-9206ceec7704) Unfortunately projects.blender.org won't let me upload more pictures but I can share some more variations in a comment if you want. Apart from the already amazing still pictures shown above I want to specifically mention Tenkai Raiko's contribution to this. Essentially he discovered that Fractal Voronoi noise can be used to procedurally simulate all kinds of smoke and fluid simulations using volumes. Apart from being able to create procedural simulations of both arbitrary size and resolution with virtually no RAM overhead he has also shown that it gives the user a level of control not easily achievable using traditional simulaton methods. All of this can be seen in the following video he posted to demonstrate said use cases: https://www.youtube.com/watch?v=WCJm-g5ngSA **Please respect the creators wish and don't copy, repost or reupload any parts of the video.** Thank you for reading and reviewing my Pull Request!
3.9 MiB
2.0 MiB
210 KiB
2.3 MiB
1.6 MiB
added 36 commits 2023-04-11 20:15:39 +02:00
Add boolean input "Normalize"
Add float input "Lacunarity"
Fix Normalize not normalizing correctly with metrics other than Euclidean.
Implement Fractal Voronoi Noise to Voronoi Texture Node.

---

For F1, Smooth F1, F2 Feature output:
The concept behind Fractal Voronoi Noise is the same as the one behind Fractal Perlin Noise.
Multiple Layers of Voronoi Noise are computed and the Distance outputs of each layer added together to create the final Distance output.

The amount of layers is being controlled by the "Detail" input.
If the "Detail" input has a non-integer value two instances of Fractal Voronoi will be computed
with each having the greatest whole integer smaller than the "Detail" input value
and the smallest whole integer greater than the "Detail" input value as the amount of layers respectively.
The fractional part of "Detail" will the be used as a factor to lerp between the two instances.

The Distance output of each layer is being multiplied by the "Roughness" input to the power of the zero based number of the current layer.
This means that for example if the "Roughness" input is 2.0 the the Distance output of the first layer is being multiplied by 2.0 ^ 0 = 1.0
for the second layer it being multiplied by 2.0 ^ 1 = 2.0 for the third layer it is being multiplied by 2.0 ^ 2 = 4.0 and so on.

The Scale input of each layer is being multiplied by the "Lacunarity" input to the power of the zero based number of the current layer.
This means that for example if the "Lacunarity" input is 2.0 the Scale input of the first layer is being multiplied by 2.0 ^ 0 = 1.0
for the second layer it is being multiplied by 2.0 ^ 1 = 2.0 for the third layer it is being multiplied by 2.0 ^ 2 = 4.0 and so on.

Additionally a "Normalize" boolean input is added which if checked will remap the entire Distance output to a 0.0 to 1.0 range.

The Color and Position outputs simply give out the Color and Position outputs of the highest Voronoi Layer being computed.

---

For Distance to Edge Feature output:
Multiple Layers of Voronoi Distance to Edge Textures are computed.
The resulting Distance output is the minimum of the Distance output of all layers.

The amount of layers is being controlled by the "Detail" input.
If the "Detail" input has a non-integer value the smallest whole integer greater than the "Detail" input value will be the amount of layers

The Scale input of each layer is being multiplied by the "Lacunarity" input to the power of the zero based number of the current layer.
This means that for example if the "Lacunarity" input is 2.0 the Scale input of the first layer is being multiplied by 2.0 ^ 0 = 1.0
for the second layer it is being multiplied by 2.0 ^ 1 = 2.0 for the third layer it is being multiplied by 2.0 ^ 2 = 4.0 and so on.

Additionally a "Normalize" boolean input is added which if checked will remap the entire Distance output to a 0.0 to 1.0 range.

---

The N-Sphere Radius Feature output is left unmodified.
.# Please enter the commit message for your changes. Lines starting
Add "ccl_device_inline" before lerp() function in \intern\cycles\util\math.h
added 1 commit 2023-04-11 20:16:31 +02:00
Some checks failed
buildbot/vexp-code-patch-coordinator Build done.
d7a7bd0024
requested review from Brecht Van Lommel 2023-04-11 20:17:17 +02:00
requested review from Omar Emara 2023-04-11 20:17:26 +02:00
requested review from Jacques Lucke 2023-04-11 20:17:41 +02:00
closed this pull request 2023-04-11 20:22:00 +02:00
reopened this pull request 2023-04-11 20:22:05 +02:00
First-time contributor

Hi I'm Tenkai Raiko the creator of the Fractal Voronoi Noise Demonstration video.

As it seems, due to a combination of rendering at a low sampling count and YouTube's video compression most of the details in the render are gone.
To show that the volumes really have "infinite" resolution I'll upload a few screenshots here.

Hi I'm Tenkai Raiko the creator of the Fractal Voronoi Noise Demonstration video. As it seems, due to a combination of rendering at a low sampling count and YouTube's video compression most of the details in the render are gone. To show that the volumes really have "infinite" resolution I'll upload a few screenshots here. ![250.png](/attachments/1a848435-1acd-4918-abbe-0cb886c9eb97) ![600.png](/attachments/febae004-f7ef-4379-b9ff-a900f6d357b2) ![1100.png](/attachments/30ef03a6-a842-40ff-812d-456435554d2c)
6.3 MiB
6.8 MiB
8.8 MiB
Member

@blender-bot package

@blender-bot package
Member

added 1 commit 2023-04-12 17:14:05 +02:00
Owner

My main concern with this is the amount of code it adds, almost 4000 lines for this feature is a lot, when it's not a critical feature in my opinion.

Isn't it possible to reduce that more? Some ideas:

• Add VoronoiParams struct to pass arguments.
• Add VoronoiOutput struct to return output.
• Add utility function to accumulate octaves into VoronoiOutput.
• Use reference instead of pointers for return values, to make C++ and GLSL/OSL code more similar.
• Maybe some of the differences between voronoi types can be switches deeper in the inner loop to avoid duplicate functions.

I wouldn't want you to spend the time to do this for all implementations, but maybe just one and then @JacquesLucke and I can check if the complexity is acceptable?

My main concern with this is the amount of code it adds, almost 4000 lines for this feature is a lot, when it's not a critical feature in my opinion. Isn't it possible to reduce that more? Some ideas: * Add VoronoiParams struct to pass arguments. * Add VoronoiOutput struct to return output. * Add utility function to accumulate octaves into VoronoiOutput. * Use reference instead of pointers for return values, to make C++ and GLSL/OSL code more similar. * Maybe some of the differences between voronoi types can be switches deeper in the inner loop to avoid duplicate functions. I wouldn't want you to spend the time to do this for all implementations, but maybe just one and then @JacquesLucke and I can check if the complexity is acceptable?
requested changes 2023-04-13 19:21:25 +02:00
left a comment
Owner

Mark as request changes based on above comment.

Mark as request changes based on above comment.
Author
Member

I'll make the requested changes to the implementation in voronoi.h.

Just 2 questions before I do so:
What exactly do you mean by
"Add utility function to accumulate octaves into VoronoiOutput."?

As for "Maybe some of the differences between voronoi types can be switches deeper in the inner loop to avoid duplicate functions."
I think that in the C++ implementations almost all duplicate functions can be avoided by using a combination of structs and templates, but that won't work in OSL and GLSL.
Is it OK if I do the changes just in voronoi.h and see what can be done in OSL and GLSL?

I'll make the requested changes to the implementation in voronoi.h. Just 2 questions before I do so: What exactly do you mean by "Add utility function to accumulate octaves into VoronoiOutput."? As for "Maybe some of the differences between voronoi types can be switches deeper in the inner loop to avoid duplicate functions." I think that in the C++ implementations almost all duplicate functions can be avoided by using a combination of structs and templates, but that won't work in OSL and GLSL. Is it OK if I do the changes just in voronoi.h and see what can be done in OSL and GLSL?
Owner

Just 2 questions before I do so:
What exactly do you mean by
"Add utility function to accumulate octaves into VoronoiOutput."?

Looking at the OSL code for example, this part seems to duplicated in multiple functions. So that could be turned into a function.

    if (detail == 0.0 || roughness == 0.0 || lacunarity == 0.0) {
max_amplitude = 1.0;
outDistance = octave_distance;
outColor = octave_color;
outPosition = octave_postion;
return;
}
else if (i <= detail) {
max_amplitude += octave_amplitude;
outDistance += octave_distance * octave_amplitude;
outColor += octave_color * octave_amplitude;
outPosition = mix(outPosition, octave_postion / octave_scale, octave_amplitude);
octave_scale *= lacunarity;
octave_amplitude *= roughness;
}
else {
float remainder = detail - floor(detail);
if (remainder != 0.0) {
max_amplitude = mix(max_amplitude, max_amplitude + octave_amplitude, remainder);
outDistance = mix(
outDistance, outDistance + octave_distance * octave_amplitude, remainder);
outColor = mix(outColor, outColor + octave_color * octave_amplitude, remainder);
outPosition = mix(outPosition,
mix(outPosition, octave_postion / octave_scale, octave_amplitude),
remainder);
}
}


As for "Maybe some of the differences between voronoi types can be switches deeper in the inner loop to avoid duplicate functions."
I think that in the C++ implementations almost all duplicate functions can be avoided by using a combination of structs and templates, but that won't work in OSL and GLSL.
Is it OK if I do the changes just in voronoi.h and see what can be done in OSL and GLSL?

Yes, let's first see what is possible in C++. For OSL/GLSL there will likely be a bit more duplication. Maybe a macro could be used if it really reduces code a lot.

But for example that code I pasted above is still duplicated 3x with the same type in OSL.

> Just 2 questions before I do so: > What exactly do you mean by > "Add utility function to accumulate octaves into VoronoiOutput."? Looking at the OSL code for example, this part seems to duplicated in multiple functions. So that could be turned into a function.  if (detail == 0.0 || roughness == 0.0 || lacunarity == 0.0) { max_amplitude = 1.0; outDistance = octave_distance; outColor = octave_color; outPosition = octave_postion; return; } else if (i <= detail) { max_amplitude += octave_amplitude; outDistance += octave_distance * octave_amplitude; outColor += octave_color * octave_amplitude; outPosition = mix(outPosition, octave_postion / octave_scale, octave_amplitude); octave_scale *= lacunarity; octave_amplitude *= roughness; } else { float remainder = detail - floor(detail); if (remainder != 0.0) { max_amplitude = mix(max_amplitude, max_amplitude + octave_amplitude, remainder); outDistance = mix( outDistance, outDistance + octave_distance * octave_amplitude, remainder); outColor = mix(outColor, outColor + octave_color * octave_amplitude, remainder); outPosition = mix(outPosition, mix(outPosition, octave_postion / octave_scale, octave_amplitude), remainder); } }  > As for "Maybe some of the differences between voronoi types can be switches deeper in the inner loop to avoid duplicate functions." > I think that in the C++ implementations almost all duplicate functions can be avoided by using a combination of structs and templates, but that won't work in OSL and GLSL. > Is it OK if I do the changes just in voronoi.h and see what can be done in OSL and GLSL? Yes, let's first see what is possible in C++. For OSL/GLSL there will likely be a bit more duplication. Maybe a macro could be used if it really reduces code a lot. But for example that code I pasted above is still duplicated 3x with the same type in OSL.
added 8 commits 2023-04-22 21:13:53 +02:00
Todo: Normalize functions.
Author
Member

@brecht I was able to eliminate all duplicate functions in voronoi.h by moving both the switch statements and the normalization functions into the fractalization functions.

voronoi.hin my new optimized version of Fractal Voronoi V.2 has a total line count of exactly 1222 lines of code compared to the pre-optimized version which had 1574 lines.

The version currently in the official main branch of Blender has a total line count of exactly 1152 lines, meaning that I was able to implement all features of Fractal Voronoi V.2 using just 70 additional lines of code.

The best thing about this is that methods I used to optimize voronoi.h can also be used to optimize the OSL and GLSL implementations.
This means that 20 out of the 40 new fractalization functions can be eliminated leaving 20 in the optimized version and 56 out of the 64 new normalization functions can be eliminated leaving only 8 in the optimized version.

So if all optimizations are implemented there should be a code reduction of 50-60% meaning that the optimized version will likely add about 1500-2000 new lines of code in total.

Apart from that I also noticed a slight performance improvement in the optimized version of Fractal Voronoi V.2 of about 5%, nothing spectacular but pretty nice nonetheless.

What do you think?

@brecht I was able to eliminate all duplicate functions in voronoi.h by moving both the switch statements and the normalization functions into the fractalization functions. voronoi.hin my new optimized version of Fractal Voronoi V.2 has a total line count of exactly 1222 lines of code compared to the pre-optimized version which had 1574 lines. The version currently in the official main branch of Blender has a total line count of exactly 1152 lines, meaning that I was able to implement all features of Fractal Voronoi V.2 using just 70 additional lines of code. The best thing about this is that methods I used to optimize voronoi.h can also be used to optimize the OSL and GLSL implementations. This means that 20 out of the 40 new fractalization functions can be eliminated leaving 20 in the optimized version and 56 out of the 64 new normalization functions can be eliminated leaving only 8 in the optimized version. So if all optimizations are implemented there should be a code reduction of 50-60% meaning that the optimized version will likely add about 1500-2000 new lines of code in total. Apart from that I also noticed a slight performance improvement in the optimized version of Fractal Voronoi V.2 of about 5%, nothing spectacular but pretty nice nonetheless. What do you think?
Member

@Hoshinova As I once told you, it would be much better to do a more general code cleanup first.
Now that you have been able to reduce copying, is it possible to say that you would make functions out of simple and understandable tasks? Or was this code unified only because of its large size and duplication?
Can we say that the ease of understanding of this code has not suffered, or even improved?
With additional features, it can be even harder to understand...

@Hoshinova As I once told you, it would be much better to do a more general code cleanup first. Now that you have been able to reduce copying, is it possible to say that you would make functions out of simple and understandable tasks? Or was this code unified only because of its large size and duplication? Can we say that the ease of understanding of this code has not suffered, or even improved? With additional features, it can be even harder to understand...
Author
Member

@mod_moder "As I once told you, it would be much better to do a more general code cleanup first."
I have already altered and cleaned up the code wherever it was necessary.
Apart from that the development of Fractal Voronoi noise already takes up all of the time I can devote to Blender Development.
It simply isn't feasible for me to do a general refactor of all the pre-existing code I touch.

"Now that you have been able to reduce copying, is it possible to say that you would make functions out of simple and understandable tasks? Or was this code unified only because of its large size and duplication? Can we say that the ease of understanding of this code has not suffered, or even improved?"
The code is broken up so that each function logically does as few things as possible while also avoiding duplication. For example while there may not be a logical incentive to integrate the normalization step into the fractalization functions, doing so eliminates 56 duplicate normalization functions.

"With additional features, it can be even harder to understand..."
Well yes, more features tend to lead to more complexity.

@mod_moder "As I once told you, it would be much better to do a more general code cleanup first." I have already altered and cleaned up the code wherever it was necessary. Apart from that the development of Fractal Voronoi noise already takes up all of the time I can devote to Blender Development. It simply isn't feasible for me to do a general refactor of all the pre-existing code I touch. "Now that you have been able to reduce copying, is it possible to say that you would make functions out of simple and understandable tasks? Or was this code unified only because of its large size and duplication? Can we say that the ease of understanding of this code has not suffered, or even improved?" The code is broken up so that each function logically does as few things as possible while also avoiding duplication. For example while there may not be a logical incentive to integrate the normalization step into the fractalization functions, doing so eliminates 56 duplicate normalization functions. "With additional features, it can be even harder to understand..." Well yes, more features tend to lead to more complexity.
added 3 commits 2023-04-23 18:23:53 +02:00
Owner

@Hoshinova I did a bit more refactoring, with those changes the amount of added complexity seems fine to me. It's probably also easier to port to OSL and GLSL with fewer templates.
https://projects.blender.org/brecht/paste/src/branch/main/svm_voronoi.patch

What do you think?

@Hoshinova I did a bit more refactoring, with those changes the amount of added complexity seems fine to me. It's probably also easier to port to OSL and GLSL with fewer templates. https://projects.blender.org/brecht/paste/src/branch/main/svm_voronoi.patch What do you think?
Author
Member

@brecht It generally looks pretty great to me, though I noticed that after your refactor all 4D outputs of F1, Smooth F1 and F2 have been somehow distorted when using the Minkowski metric:

Reference:

After Refactor:

Difference:

Test .blend file:
Fractal_Voronoi_Test_File.blend

The strange thing is that it only appears when using the Minkowski metric in 4D meaning, that all other combinations of dimension and metric seem to be fine.
But I also couldn't find anything that could be causing this bug, the only thing that is used specifically by the Minkowski metric is this line:

 + return powf(reduce_add(pow(fabs(a - b), params.exponent)), 1.0f / params.exponent);

Or maybe it's just a problem on my machine so could you please have a look at this?

@brecht It generally looks pretty great to me, though I noticed that after your refactor all 4D outputs of F1, Smooth F1 and F2 have been somehow distorted when using the Minkowski metric: Reference: ![Reference.png](/attachments/d1ea5a2b-5d97-4aa0-9b7a-8b6a42b4c643) ![Reference Cube.png](/attachments/c4233510-fcea-420b-a9ed-9d2b8768b390) After Refactor: ![After Refactor.png](/attachments/ca7101a3-3a65-4fb5-b762-ab480f5d8d35) ![After Refactor Cube.png](/attachments/5aacf869-725c-4706-a3be-22650a893ddb) Difference: ![Difference.png](/attachments/1f973998-1fca-47e3-a172-faf2f9b066b6) Test .blend file: [Fractal_Voronoi_Test_File.blend](/attachments/25bbde75-19fd-4a3d-a984-da7af3b4424f) The strange thing is that it only appears when using the Minkowski metric in 4D meaning, that all other combinations of dimension and metric seem to be fine. But I also couldn't find anything that could be causing this bug, the only thing that is used specifically by the Minkowski metric is this line: https://projects.blender.org/brecht/paste/src/commit/de85a994f64c01221ff79023531a32ad2f42a585/svm_voronoi.patch#L94 Or maybe it's just a problem on my machine so could you please have a look at this?
1.1 MiB
5.3 MiB
1.0 MiB
5.8 MiB
2.3 MiB
Author
Member

Apart from that I have a question regarding the const VArray<> &s in

 const VArray &vector = get_vector(param++);

Is it better to keep using VArray references in a VoronoiParams  struct, which would mean that they'd all need to be initialized using the constructor, or is it OK to just copy the return value of e.g. get_vector(param++) into a new VArray<> variable in the VoronoiParams  struct?

Which would look something like this:
params.vector = get_vector(param++);

Apart from that I have a question regarding the const VArray<> &s in https://projects.blender.org/blender/blender/src/commit/5a6db19da490848c5a7eadbbbe8a348f097eadbe/source/blender/nodes/shader/nodes/node_shader_tex_voronoi.cc#L905 Is it better to keep using VArray references in a VoronoiParams  struct, which would mean that they'd all need to be initialized using the constructor, or is it OK to just copy the return value of e.g. get_vector(param++) into a new VArray<> variable in the VoronoiParams  struct? Which would look something like this: params.vector = get_vector(param++);
Author
Member

Also regarding the GLSL implementation, as far as I know the GLSL voronoi implementation starts in this file:

 randomness = clamp(randomness, 0.0, 1.0);

I don't see a general starting point for the GLSL version in that file where a VoronoiParams struct could be initialized, so it would probably be best to do that in whatever function calls the GLSL Voronoi functions.
But I have trouble finding that function as for example breaking points don't work in the GLSL files for some reason.

So could you please also tell me which function calls the GLSL Voronoi functions?

Also regarding the GLSL implementation, as far as I know the GLSL voronoi implementation starts in this file: https://projects.blender.org/blender/blender/src/commit/557a245dd5ce30f9e802e5b11a986f8378e3ed33/source/blender/gpu/shaders/material/gpu_shader_material_tex_voronoi.glsl#L528 I don't see a general starting point for the GLSL version in that file where a VoronoiParams struct could be initialized, so it would probably be best to do that in whatever function calls the GLSL Voronoi functions. But I have trouble finding that function as for example breaking points don't work in the GLSL files for some reason. So could you please also tell me which function calls the GLSL Voronoi functions?
Owner

For the Minkowski distance, there was a bug in float4 pow that I fixed in main now: 63dfbdc.

For VArray, I guess it's best to keep the implementation in noise.cc scalar for now, including VoronoiParams? To avoid making that much more complicated and keep it similar to the other implementations.

If you structure the implementation similar to the SVM one, I think you can deduplicate may of the get_vector and similar calls as well.

For the Minkowski distance, there was a bug in float4 pow that I fixed in main now: 63dfbdc. For VArray, I guess it's best to keep the implementation in noise.cc scalar for now, including VoronoiParams? To avoid making that much more complicated and keep it similar to the other implementations. If you structure the implementation similar to the SVM one, I think you can deduplicate may of the get_vector and similar calls as well.
Owner

Those GLSL functions are chosen from node_shader_tex_voronoi.cc. I guess there are two approaches there. Either there could be one function, relying on the optimizer to do good constant folding. Or all the functions could remain, with some macros to deduplicate code like packing the parameters into a struct.

Those GLSL functions are chosen from node_shader_tex_voronoi.cc. I guess there are two approaches there. Either there could be one function, relying on the optimizer to do good constant folding. Or all the functions could remain, with some macros to deduplicate code like packing the parameters into a struct.
Author
Member

For VArray, I guess it's best to keep the implementation in noise.cc scalar for now, including VoronoiParams? To avoid making that much more complicated and keep it similar to the other implementations.

If you structure the implementation similar to the SVM one, I think you can deduplicate may of the get_vector and similar calls as well.

Umm what do you mean by keeping it scalar? Honestly I'm not sure what exactly get_vector and friends are doing in the first place, until now I've just been kinda copying how it's used everywhere else.

> For VArray, I guess it's best to keep the implementation in noise.cc scalar for now, including VoronoiParams? To avoid making that much more complicated and keep it similar to the other implementations. > > If you structure the implementation similar to the SVM one, I think you can deduplicate may of the get_vector and similar calls as well. Umm what do you mean by keeping it scalar? Honestly I'm not sure what exactly get_vector and friends are doing in the first place, until now I've just been kinda copying how it's used everywhere else.
Owner

Function nodes run on an array of values instead of individual ones, which helps improve performance. For example the feature/dimension switch only has to be done once for all elements.

The voronoi evaluation still runs on one element at a time, and I think it's fine to keep it like that. And so that would mean VoronoiParams does not need to contain a VArray. Rather you can construct it for every element, getting the values from the VArray for each input.

Function nodes run on an array of values instead of individual ones, which helps improve performance. For example the feature/dimension switch only has to be done once for all elements. The voronoi evaluation still runs on one element at a time, and I think it's fine to keep it like that. And so that would mean VoronoiParams does not need to contain a VArray. Rather you can construct it for every element, getting the values from the VArray for each input.
added 3 commits 2023-04-26 18:40:22 +02:00
* More functional style definition to clarify what gets mutated where. Make
VoronoiParams const, pass coordinate as own argument and return output by
value.
* Don't use template class, subclassing and methods. Easier to use same logic
in OSL and GLSL then. Slightly more overhead for always having fractal
position accumulated in 4D, but should be ok.
* Don't use VoronoiOutput for distance and radius.
* Various other naming and organization tweaks.
* Put normalization into it's own function to match GLSL implementation.
* Remove "octave_scale" variable.
* Remove "voronoi_" prefixes.
Author
Member

@brecht I've made some final changes to voronoi.h. I would then copy the voronoi.h implementation to the other versions as closely as reasonable, so is there still anything you would like to have changed?

@brecht I've made some final changes to voronoi.h. I would then copy the voronoi.h implementation to the other versions as closely as reasonable, so is there still anything you would like to have changed?
Author
Member

The voronoi evaluation still runs on one element at a time, and I think it's fine to keep it like that. And so that would mean VoronoiParams does not need to contain a VArray. Rather you can construct it for every element, getting the values from the VArray for each input

So do you mean something like this?:

for (int64_t i : mask) {
param = 0;
params.vector = get_vector(param++)[i];
params.w = get_w(param++)[i];
params.scale = get_scale(param++)[i];

further code...
}


But wouldn't getting the parameters every time be much slower than just copying the entire VArray into VoronoiParams once?

The voronoi evaluation still runs on one element at a time, and I think it's fine to keep it like that. And so that would mean VoronoiParams does not need to contain a VArray. Rather you can construct it for every element, getting the values from the VArray for each input So do you mean something like this?: C++ for (int64_t i : mask) { param = 0; params.vector = get_vector(param++)[i]; params.w = get_w(param++)[i]; params.scale = get_scale(param++)[i]; further code... }  But wouldn't getting the parameters every time be much slower than just copying the entire VArray into VoronoiParams once?
Author
Member

As for GLSL I'll just #define a macro (something like INITIALIZE_VORONOIPARAMS) which would take care of the initialization of VoronoiParams.

As for GLSL I'll just #define a macro (something like INITIALIZE_VORONOIPARAMS) which would take care of the initialization of VoronoiParams.
Owner

I meant something like this:

const VArray<float> &randomness = get_randomness(param++);
...
for (int64_t i : mask) {
VoronoiParams params;
params.randomness = randomness[i];
...

I meant something like this: Cpp const VArray<float> &randomness = get_randomness(param++); ... for (int64_t i : mask) { VoronoiParams params; params.randomness = randomness[i]; ... 
Author
Member

I meant something like this:

const VArray<float> &randomness = get_randomness(param++);
...
for (int64_t i : mask) {
VoronoiParams params;
params.randomness = randomness[i];
...


Are you sure about this? That looks like at least 200+ additional lines just to initialize VoronoiParams, whats wrong with just copying the VArray?

> I meant something like this: > Cpp > const VArray<float> &randomness = get_randomness(param++); > ... > for (int64_t i : mask) { > VoronoiParams params; > params.randomness = randomness[i]; > ... >  Are you sure about this? That looks like at least 200+ additional lines just to initialize VoronoiParams, whats wrong with just copying the VArray?
Owner

I'm not sure what you mean by just copying the VArray, you need to go from VArray to single float values unless you want noise.cc to work with VArray all the way through as well?

If there is a lot of extra code, you could declare a VoronoiVArrayParams or something and have a utility function to get a VoronoiParams for one element out of that. Or put the switches inside the for loop at a small performance cost.

I'm not sure what you mean by just copying the VArray, you need to go from VArray to single float values unless you want noise.cc to work with VArray all the way through as well? If there is a lot of extra code, you could declare a VoronoiVArrayParams or something and have a utility function to get a VoronoiParams for one element out of that. Or put the switches inside the for loop at a small performance cost.
Author
Member

Right, I completely forgot about noise.cc, in that case I think I'll put the switch statement in the for loop, as it would probably still be faster than a function call and the performance difference isn't worth 200 lines of extra code.

Right, I completely forgot about noise.cc, in that case I think I'll put the switch statement in the for loop, as it would probably still be faster than a function call and the performance difference isn't worth 200 lines of extra code.
added 2 commits 2023-04-30 21:20:14 +02:00
added 3 commits 2023-05-01 19:20:33 +02:00
-- Fix Geo nodes voronoi_distance() functions
Author
Member

@mod_moder Well, looks like I did end up having to change a lot of the pre-exisitng code, so you did kind of get your general code cleanup after all ;-)

@mod_moder Well, looks like I did end up having to change a lot of the pre-exisitng code, so you did kind of get your general code cleanup after all ;-)
Member

@Hoshinova And now you have 8500++ line changed PR, good luck and best health to reviewers, this main idea of splitting this in more PRs (:/

@Hoshinova And now you have 8500++ line changed PR, good luck and best health to reviewers, this main idea of splitting this in more PRs (:/
Author
Member

@mod_moder I'd figured you'll say that, but I honestly don't see how splitting this up into multiple pull requests would reduce the work for the reviewers in any way.
Apart from that as all the new code is rather interdependent, where would you wan't to split it up in the first place?

@mod_moder I'd figured you'll say that, but I honestly don't see how splitting this up into multiple pull requests would reduce the work for the reviewers in any way. Apart from that as all the new code is rather interdependent, where would you wan't to split it up in the first place?
Member

Author
Member

@brecht I have a question regarding hash_float_to_float

 hash_float_to_float(cellPosition + cellOffset) * randomness;
and friends.
Is the output range of the hash_x_to_y functions always in a [0, 1] interval or can it be different?

@brecht I have a question regarding hash_float_to_float https://projects.blender.org/blender/blender/src/commit/0652945dbda76e5ba329a7e5aee45afc0d3a83d3/source/blender/blenlib/intern/noise.cc#L1391 and friends. Is the output range of the hash_x_to_y functions always in a [0, 1] interval or can it be different?
added 1 commit 2023-05-04 21:49:55 +02:00
Rework F2, Distance to Edge normalization
added 3 commits 2023-05-07 15:25:57 +02:00
-- Use macros in both GLSL and OSL implementations of Fractal Voronoi noise to reduce code
Some checks failed
buildbot/vexp-code-patch-coordinator Build done.
25825c0a97
Member

@blender-bot package

@blender-bot package
Member

Author
Member

@brecht It is done.
Or rather it works on my machine but according to https://builder.blender.org/admin/#/builders/136/builds/1158 there seem to be errors on other machines.

For https://builder.blender.org/admin/#/builders/135/builds/1135 it apparently can't find the new functions for some reason.

Meanwhile https://builder.blender.org/admin/#/builders/132/builds/1118 and https://builder.blender.org/admin/#/builders/132/builds/1118 don't seem to fail because of the code from Fractal Voronoi noise.

I'm not sure what exactly causes these errors (it works on my machine after all), could you please have look at it?

Apart from that I would say that after fixing those errors the code is finally ready to be merged into the main branch.

@brecht It is done. Or rather it works on my machine but according to https://builder.blender.org/admin/#/builders/136/builds/1158 there seem to be errors on other machines. For https://builder.blender.org/admin/#/builders/135/builds/1135 it apparently can't find the new functions for some reason. Meanwhile https://builder.blender.org/admin/#/builders/132/builds/1118 and https://builder.blender.org/admin/#/builders/132/builds/1118 don't seem to fail because of the code from Fractal Voronoi noise. I'm not sure what exactly causes these errors (it works on my machine after all), could you please have look at it? Apart from that I would say that after fixing those errors the code is finally ready to be merged into the main branch.
requested review from Brecht Van Lommel 2023-05-07 17:46:51 +02:00
Owner

For the Metal errors you need to add the ccl_private qualifier for any pointer or reference argument. Also use mix instead of lerp.

The HIP errors seem to be some kind of compiler bug related to name mangling. Probably can be worked around somehow.

For the Metal errors you need to add the ccl_private qualifier for any pointer or reference argument. Also use mix instead of lerp. The HIP errors seem to be some kind of compiler bug related to name mangling. Probably can be worked around somehow.
added 3 commits 2023-05-13 21:50:58 +02:00
Some checks failed
buildbot/vexp-code-patch-coordinator Build done.
22550df1f2
Member

@blender-bot package

@blender-bot package
Member

Author
Member

@brecht "For the Metal errors you need to add the ccl_private qualifier for any pointer or reference argument. Also use mix instead of lerp."
Fixed that.

"The HIP errors seem to be some kind of compiler bug related to name mangling. Probably can be worked around somehow."
And uhh, how do you do that?

Apart from that is there still something needed to be changed?
I've already tested all the different noise implementations to make sure that they all give out the same output.
I also didn't encounter any bugs during that process.
The performance difference for a single layer of Voironoi noise between the Fractal Voronoi Noise node and the Voronoi noise node currently in main branch is less than 1%, as far as I can test.

@brecht "For the Metal errors you need to add the ccl_private qualifier for any pointer or reference argument. Also use mix instead of lerp." Fixed that. "The HIP errors seem to be some kind of compiler bug related to name mangling. Probably can be worked around somehow." And uhh, how do you do that? Apart from that is there still something needed to be changed? I've already tested all the different noise implementations to make sure that they all give out the same output. I also didn't encounter any bugs during that process. The performance difference for a single layer of Voironoi noise between the Fractal Voronoi Noise node and the Voronoi noise node currently in main branch is less than 1%, as far as I can test.
Author
Member

Well, looks like Metal is still complaining https://builder.blender.org/admin/#/builders/135/builds/1182 error: no matching function for call to 'reduce_add'

Should I just change the voronoi_distance() function in voronoi.h back to the old non templated version?

Well, looks like Metal is still complaining https://builder.blender.org/admin/#/builders/135/builds/1182 error: no matching function for call to 'reduce_add' Should I just change the voronoi_distance() function in voronoi.h back to the old non templated version?
added 3 commits 2023-05-15 20:33:44 +02:00
Owner

I added reduce_add for float2 in main, that should make it work on Metal.

I added reduce_add for float2 in main, that should make it work on Metal.
added 1 commit 2023-05-18 18:28:16 +02:00
--Removed "_N" translation macro
added 1 commit 2023-05-18 21:21:39 +02:00
added 1 commit 2023-05-18 21:27:10 +02:00
requested changes 2023-05-26 22:56:24 +02:00
left a comment
Member

Why not write the macros as as function-style? For instance:

#define FRACTAL_VORONOI_FUNCTION(type, voronoi_postfix) \
VoronoiOutput fractal_voronoi_##voronoi_postfix(VoronoiParams params, type coord) \
{ \
VoronoiOutput octave = voronoi_##voronoi_postfix(params, coord * scale); \
}

FRACTAL_VORONOI_FUNCTION(float, f1)
FRACTAL_VORONOI_FUNCTION(float, smooth_f1)
FRACTAL_VORONOI_FUNCTION(float, f2)

FRACTAL_VORONOI_FUNCTION(vec2, f1)
FRACTAL_VORONOI_FUNCTION(vec2, smooth_f1)
FRACTAL_VORONOI_FUNCTION(vec2, f2)

FRACTAL_VORONOI_FUNCTION(vec3, f1)
FRACTAL_VORONOI_FUNCTION(vec3, smooth_f1)
FRACTAL_VORONOI_FUNCTION(vec3, f2)

FRACTAL_VORONOI_FUNCTION(vec4, f1)
FRACTAL_VORONOI_FUNCTION(vec4, smooth_f1)
FRACTAL_VORONOI_FUNCTION(vec4, f2)


I think this is cleaner and we can just eliminate params.feature and any potential trouble that comes with it especially for GLSL.

I am not sure why you are not using the single value constructors for vectors, but those can make the code more compact and cleaner, consider:

  params.max_distance = voronoi_distance(vec4(0.0, 0.0, 0.0, 0.0),
vec4(0.5 + 0.5 * params.randomness,
0.5 + 0.5 * params.randomness,
0.5 + 0.5 * params.randomness,
0.5 + 0.5 * params.randomness),
params) *
2.0;


Vs:

params.max_distance = voronoi_distance(vec4(0.0), vec4(0.5 + 0.5 * params.randomness), params) * 2.0;


Are the existing functions in gpu_shader_material_voronoi.glsl changed in any way?

Why not write the macros as as function-style? For instance: C #define FRACTAL_VORONOI_FUNCTION(type, voronoi_postfix) \ VoronoiOutput fractal_voronoi_##voronoi_postfix(VoronoiParams params, type coord) \ { \ VoronoiOutput octave = voronoi_##voronoi_postfix(params, coord * scale); \ } FRACTAL_VORONOI_FUNCTION(float, f1) FRACTAL_VORONOI_FUNCTION(float, smooth_f1) FRACTAL_VORONOI_FUNCTION(float, f2) FRACTAL_VORONOI_FUNCTION(vec2, f1) FRACTAL_VORONOI_FUNCTION(vec2, smooth_f1) FRACTAL_VORONOI_FUNCTION(vec2, f2) FRACTAL_VORONOI_FUNCTION(vec3, f1) FRACTAL_VORONOI_FUNCTION(vec3, smooth_f1) FRACTAL_VORONOI_FUNCTION(vec3, f2) FRACTAL_VORONOI_FUNCTION(vec4, f1) FRACTAL_VORONOI_FUNCTION(vec4, smooth_f1) FRACTAL_VORONOI_FUNCTION(vec4, f2)  I think this is cleaner and we can just eliminate params.feature and any potential trouble that comes with it especially for GLSL. *** I am not sure why you are not using the single value constructors for vectors, but those can make the code more compact and cleaner, consider: Cpp params.max_distance = voronoi_distance(vec4(0.0, 0.0, 0.0, 0.0), vec4(0.5 + 0.5 * params.randomness, 0.5 + 0.5 * params.randomness, 0.5 + 0.5 * params.randomness, 0.5 + 0.5 * params.randomness), params) * 2.0;  Vs: Cpp params.max_distance = voronoi_distance(vec4(0.0), vec4(0.5 + 0.5 * params.randomness), params) * 2.0;  *** Are the existing functions in gpu_shader_material_voronoi.glsl changed in any way?
added 2 commits 2023-05-27 20:45:52 +02:00
-- GLSL and OSL Change macros to function macros
Some checks failed
buildbot/vexp-code-patch-coordinator Build done.
cb467c5ae7
-- GLSL use single value constructors for vectors
Member

@blender-bot package

@blender-bot package
Member

Author
Member

A huge problem I faced when writing the code for Fractal Voronoi noise was that there were a lot inconsistencies between the different implementation. This starts at small things like the same variables having different names, arbitrary extra variables and extra variables with the same name as a variable in a different implementation but having a different function, but it covers things like there being arbitrary extra cases, like Minkowski in voronoi.cc.
Not to mention the whole structure of the code was different in every implementation, for some implementations the noise layers were being evaluated inside the call function while in others it was done in a separate function, etc..

What all those inconsistencies meant was that I effectively had to write the same piece of code 4 times and since each implementation was slightly different it also had to be debugged separately. This actually significantly increases the amount of work to add anything, in fact I probably spent more time on porting the code across the 4 implementations than developing new features.

This is why when I did a general refactor of the complete code of Fractal Voronoi noise (including the pre-existing code) I tried to make different implementations match as closely as possible. Of course as the different programming languages have different features and constraints some changes had to be made, especially between the C++ and non-C++ versions.
If you look at the PR you will notice that the Cycles and Geo nodes versions are almost identical. Between those two versions and the OSL and GLSL versions there are some minor differences but the OSL and GLSL are in turn very close.

A huge problem I faced when writing the code for Fractal Voronoi noise was that there were a lot inconsistencies between the different implementation. This starts at small things like the same variables having different names, arbitrary extra variables and extra variables with the same name as a variable in a different implementation but having a different function, but it covers things like there being arbitrary extra cases, like Minkowski in voronoi.cc. Not to mention the whole structure of the code was different in every implementation, for some implementations the noise layers were being evaluated inside the call function while in others it was done in a separate function, etc.. What all those inconsistencies meant was that I effectively had to write the same piece of code 4 times and since each implementation was slightly different it also had to be debugged separately. This actually significantly increases the amount of work to add anything, in fact I probably spent more time on porting the code across the 4 implementations than developing new features. This is why when I did a general refactor of the complete code of Fractal Voronoi noise (including the pre-existing code) I tried to make different implementations match as closely as possible. Of course as the different programming languages have different features and constraints some changes had to be made, especially between the C++ and non-C++ versions. If you look at the PR you will notice that the Cycles and Geo nodes versions are almost identical. Between those two versions and the OSL and GLSL versions there are some minor differences but the OSL and GLSL are in turn very close.
Author
Member

Now getting to the specificly mentioned points:

Why not write the macros as as function-style? For instance:
I think this is cleaner and we can just eliminate params.feature and any potential trouble that comes with it especially for GLSL.

In my latest commit I changed the macros to function macros, but kept params.feature, for the reasons stated in my comment above. The C++ implementation uses templates instead of macros, which doesn't allow such kinds of substitutions in the function name. Apart from that I also plan to do some performance optimizations in for Voronoi after this PR for which I might need params.feature in some other places.
I didn't face any issues regarding params.feature though, what "potential trouble" do you have in mind?

I am not sure why you are not using the single value constructors for vectors, but those can make the code more compact and cleaner, consider:

Done.

Are the existing functions in gpu_shader_material_voronoi.glsl changed in any way?

Yes all functions which evaluate some kind of Voronoi layer have been substituted using the versions from voronoi.h, which however apart from the data types seem to be the same as the ones originally present.

@OmarEmaraDev Now getting to the specificly mentioned points: > Why not write the macros as as function-style? For instance: I think this is cleaner and we can just eliminate params.feature and any potential trouble that comes with it especially for GLSL. In my latest commit I changed the macros to function macros, but kept params.feature, for the reasons stated in my comment above. The C++ implementation uses templates instead of macros, which doesn't allow such kinds of substitutions in the function name. Apart from that I also plan to do some performance optimizations in for Voronoi after this PR for which I might need params.feature in some other places. I didn't face any issues regarding params.feature though, what "potential trouble" do you have in mind? > I am not sure why you are not using the single value constructors for vectors, but those can make the code more compact and cleaner, consider: Done. > Are the existing functions in gpu_shader_material_voronoi.glsl changed in any way? Yes all functions which evaluate some kind of Voronoi layer have been substituted using the versions from voronoi.h, which however apart from the data types seem to be the same as the ones originally present.
Member

The C++ implementation uses templates instead of macros, which doesn't allow such kinds of substitutions in the function name.

I am not the best person to give advice on how to do stuff in C++, but can't we just add the functions as template parameters and instantiate that function in an appropriately named function, or better yet, use an alias? I think compilers won't have any issue inlining that, so there won't be any performance concerns. For instance, see:

https://godbolt.org/z/GMM4GYcGo

I didn't face any issues regarding params.feature though, what "potential trouble" do you have in mind?

I mean things like compilers failing to optimize those branches, which does happen especially for GLSL compilers. So I think it should not be kept for now. We can bring it back when it is really needed, as in the future optimizations you want to do.

> The C++ implementation uses templates instead of macros, which doesn't allow such kinds of substitutions in the function name. I am not the best person to give advice on how to do stuff in C++, but can't we just add the functions as template parameters and instantiate that function in an appropriately named function, or better yet, use an alias? I think compilers won't have any issue inlining that, so there won't be any performance concerns. For instance, see: https://godbolt.org/z/GMM4GYcGo > I didn't face any issues regarding params.feature though, what "potential trouble" do you have in mind? I mean things like compilers failing to optimize those branches, which does happen especially for GLSL compilers. So I think it should not be kept for now. We can bring it back when it is really needed, as in the future optimizations you want to do.
Author
Member

I am not the best person to give advice on how to do stuff in C++, but can't we just add the functions as template parameters and instantiate that function in an appropriately named function, or better yet, use an alias?

Yes, we could but the thing is that it would add further complexity to those implementations for no apparent benefit.
The thing is for all versions other than GLSL, the decision of which feature is chosen needs to be at some point in the code, so if at all you'd have unoptimized branches either way.
GLSL is special in that point as that decision us done before entering the GLSL code, so this kind of branch optimization would only work in GLSL.
Of course we could also have a different implementation in GLSL, which i'd rather avoid though...

> I am not the best person to give advice on how to do stuff in C++, but can't we just add the functions as template parameters and instantiate that function in an appropriately named function, or better yet, use an alias? Yes, we could but the thing is that it would add further complexity to those implementations for no apparent benefit. The thing is for all versions other than GLSL, the decision of which feature is chosen needs to be at some point in the code, so if at all you'd have unoptimized branches either way. GLSL is special in that point as that decision us done before entering the GLSL code, so this kind of branch optimization would only work in GLSL. Of course we could also have a different implementation in GLSL, which i'd rather avoid though...
Member

I am not convinced that this would complicate things much, but in any case, we can just wait for the opinion of other reviewers on the matter.

I am not convinced that this would complicate things much, but in any case, we can just wait for the opinion of other reviewers on the matter.
Author
Member

I did some performance tests and as expected, the performance difference for a single layer of Voronoi noise between the current Fractal Voronoi Noise node implementation and the Voronoi noise node currently in the main branch is less than 1% in EEVEE on an RTX 3070:

You can convince yourself using the packaged build from the buildbot and this test file:
Voronoi_Test_File_EEVEE.blend

@OmarEmaraDev If it's just about performance I think your concerns are unfounded. I did some performance tests and as expected, the performance difference for a single layer of Voronoi noise between the current Fractal Voronoi Noise node implementation and the Voronoi noise node currently in the main branch is less than 1% in EEVEE on an RTX 3070: ![EEVEE_main_branch_test.png](/attachments/4791d620-9f84-4d50-9f3b-4515642ba801) ![EEVEE_add-fractal-voronoi_branch_test.png](/attachments/face5f6a-f06c-42b7-9ca7-89125e0df8bb) You can convince yourself using the packaged build from the buildbot and this test file: [Voronoi_Test_File_EEVEE.blend](/attachments/e80e4c76-2e8d-4629-8bd8-5d1e90a328cf)
2.2 MiB
Member

That's fine, I am not trying to convince you that a performance regression is definite. But my concern was about different compiler implementations. Just because it is fine in a single compiler and platform does not mean that it will be fine for all other compilers.

That's fine, I am not trying to convince you that a performance regression is definite. But my concern was about different compiler implementations. Just because it is fine in a single compiler and platform does not mean that it will be fine for all other compilers.
Author
Member

Fair point. Let's see what Brecht's and Jacques's opinion are on that matter.
Ah and I also didn't experience a performance regression in Cycles on my machine.

Apart from that is there anything else needed to be changed?

Fair point. Let's see what Brecht's and Jacques's opinion are on that matter. Ah and I also didn't experience a performance regression in Cycles on my machine. Apart from that is there anything else needed to be changed?
Owner

I would expect params to be effectively treated as a bunch of individual variables, and params.feature to be easily constant folded. My guess would be it's fine to leave it unchanged. But I don't have a lot of experience with quirks of the latest GLSL compilers.

I would expect params to be effectively treated as a bunch of individual variables, and params.feature to be easily constant folded. My guess would be it's fine to leave it unchanged. But I don't have a lot of experience with quirks of the latest GLSL compilers.
Member

@fclem I remember you saying that some GLSL compilers sometimes refuse to corporate and will not remove the branches even if the condition variable is a constant. Is it worth it to remove such branches, or can we just leave it as is?

For context, this is the code in question. params.feature is a constant set by the caller:

VoronoiOutput octave; \
if (params.feature == 0) {
octave = voronoi_f1(params, coord * scale);
}
else if (params.feature == 2) {
octave = voronoi_smooth_f1(params, coord * scale);
}
else {
octave = voronoi_f2(params, coord * scale);
}

@fclem I remember you saying that some GLSL compilers sometimes refuse to corporate and will not remove the branches even if the condition variable is a constant. Is it worth it to remove such branches, or can we just leave it as is? For context, this is the code in question. params.feature is a constant set by the caller: GLSL VoronoiOutput octave; \ if (params.feature == 0) { octave = voronoi_f1(params, coord * scale); } else if (params.feature == 2) { octave = voronoi_smooth_f1(params, coord * scale); } else { octave = voronoi_f2(params, coord * scale); } 
Author
Member

If it actually turns out that the branches fail to get optimized, I'd say it's bets to just a separate different implementation for GLSL then.
Instead of having the Fractalization process have it's own function I'd just write the macro so that's inlined directly in the calling function.
I'd then leave the other implementations unchanged because removing the branches will double the compile time for that part of code

If it actually turns out that the branches fail to get optimized, I'd say it's bets to just a separate different implementation for GLSL then. Instead of having the Fractalization process have it's own function I'd just write the macro so that's inlined directly in the calling function. I'd then leave the other implementations unchanged because removing the branches will double the compile time for that part of code But let's first see what @fclem has to say about this.
Member

#98190 was the original bug that made me worried that this kind of optimization scheme was not viable. Even with const keyword are not enough for these compilers. But maybe that was because of the float comparison.

If there is a problem with it, maybe just pass the technique parameter as part of the macro parameter and change the function name using macro name glue voronoi_##technique() instead of having if statements. An example of this is in /source/blender/draw/engines/eevee/shaders/closure_eval_lib.glsl.

#98190 was the original bug that made me worried that this kind of optimization scheme was not viable. Even with const keyword are not enough for these compilers. But maybe that was because of the float comparison. If there is a problem with it, maybe just pass the technique parameter as part of the macro parameter and change the function name using macro name glue voronoi_##technique() instead of having if statements. An example of this is in /source/blender/draw/engines/eevee/shaders/closure_eval_lib.glsl.
Author
Member

@fclem I didn't experience any problems with it and I also already know a solution to this.
The thing is if we optimize out the if statements we'd get a GLSL implementation which is different from the other 3 implementations, which I want to avoid.

So the question to you is actually if there may be a problem, as I didn't experience any.

@fclem I didn't experience any problems with it and I also already know a solution to this. The thing is if we optimize out the if statements we'd get a GLSL implementation which is different from the other 3 implementations, which I want to avoid. So the question to you is actually **if there may be a problem**, as I didn't experience any.
Member

@Hoshinova I would say it is ok to commit in this state. We can change it if the issue arise. Also we are going to raise the minimum requirement for 4.0 so it might just work. 🤞

@Hoshinova I would say it is ok to commit in this state. We can change it if the issue arise. Also we are going to raise the minimum requirement for 4.0 so it might just work. 🤞
requested changes 2023-05-31 11:10:23 +02:00
left a comment
Member

I have a few comments on code style, but overall it's much better than before! Besides those, the geometry nodes related code seems to be ready.
It would be nice if you could do a few simple performance comparisons in geometry nodes just to see that there are no significant performance regressions. Taking the measurements from the UI is good enough for this:

The results of the benchmark should be added to the patch description.

I have a few comments on code style, but overall it's much better than before! Besides those, the geometry nodes related code seems to be ready. It would be nice if you could do a few simple performance comparisons in geometry nodes just to see that there are no significant performance regressions. Taking the measurements from the UI is good enough for this: ![image](/attachments/e167a11a-1acf-446b-9630-5824f84e73aa) The results of the benchmark should be added to the patch description.
59 KiB
 @ -250,3 +250,3 @@   /* Linear Interpolation. */ BLI_INLINE float mix(float v0, float v1, float x) template T mix(T v0, T v1, float x)
Member

Add static.

Add static.
 @ -1375,1 +1383,4 @@ float voronoi_distance_1d(const float a, const float b, const VoronoiParams ¶ms) {  /* Supress compiler warnings, not used for 1D. */  (void)params;
Member

Use UNUSED_VARS. Arguably, params shouldn't be passed in here in this case, but it's fine for now. Maybe it will be removed as part of a more general optimization pass on the code in the future.

Use UNUSED_VARS. Arguably, params shouldn't be passed in here in this case, but it's fine for now. Maybe it will be removed as part of a more general optimization pass on the code in the future.
Member

/* params */ ?

/* params */ ?
Member

That's even better.

That's even better.
 @ -1406,2 +1482,2 @@  *r_w = targetPosition + cellPosition;  }    VoronoiOutput octave;
Member

octave -> output
Same below.

octave -> output Same below.
Author
Member

Why output instead of octave? It only computes one octave not the entire output.

Why output instead of octave? It only computes one octave not the entire output.
Member

In my current understanding, the term "octave" only makes sense in the context of fractal noise. So using the term there is reasonable. But this function just computes noise directly, it doesn't care about multiple octaves.
Don't mind too much either way, I just found it a bit confusing when reading the code at first.

In my current understanding, the term "octave" only makes sense in the context of fractal noise. So using the term there is reasonable. But this function just computes noise directly, it doesn't care about multiple octaves. Don't mind too much either way, I just found it a bit confusing when reading the code at first.
Author
Member

I see where you're coming from. Generally it's just a matter of point of view. You could see a single layer of Voronoi noise as it's own thing and Fractal Voronoi noise as multiple of them overlayed together, in which case it would make sense to use output. Or alternatively you could see a single layer of Voronoi noise as a part of Fractal Voronoi noise in which case octave would make more sense.

Since the Voronoi functions currently aren't used outside of Fractal Voronoi noise I think keeping it at octave is the better option now.

If there should be any issues regarding the naming we can always change it in the future, so I'll mark it as resolved for now.

I see where you're coming from. Generally it's just a matter of point of view. You could see a single layer of Voronoi noise as it's own thing and Fractal Voronoi noise as multiple of them overlayed together, in which case it would make sense to use output. Or alternatively you could see a single layer of Voronoi noise as a part of Fractal Voronoi noise in which case octave would make more sense. Since the Voronoi functions currently aren't used outside of Fractal Voronoi noise I think keeping it at octave is the better option now. If there should be any issues regarding the naming we can always change it in the future, so I'll mark it as resolved for now.
 @ -2340,0 +2236,4 @@  float scale = 1.0f;    VoronoiOutput output;  bool zero_input = params.detail == 0.0f || params.roughness == 0.0f || params.lacunarity == 0.0f;
Member

const

const
 @ -2340,0 +2247,4 @@    if (zero_input) {  max_amplitude = 1.0f;  output.distance = octave.distance;
Member

output = octave;

output = octave;
 @ -239,3 +304,3 @@  }    void call(const IndexMask &mask, mf::Params params, mf::Context /*context*/) const override  void call(const IndexMask &mask, mf::Params mf_params, mf::Context /*context*/) const override
Member

mf_params -> params
Same below.

mf_params -> params Same below.
Author
Member

That would create a naming collision with noise::VoronoiParams params; in line 376. And out of the option of renaming noise::VoronoiParams or mf::Params I think that renaming mf::Params is the better option as noise::VoronoiParams is not only used much more often in the Voronoi code but should also be named params to keep it consistent with the other implementations.

That would create a naming collision with noise::VoronoiParams params; in line 376. And out of the option of renaming noise::VoronoiParams or mf::Params I think that renaming mf::Params is the better option as noise::VoronoiParams is not only used much more often in the Voronoi code but should also be named params to keep it consistent with the other implementations.
Member

Ah, didn't notice that. Seems ok then.

Ah, didn't notice that. Seems ok then.
 @ -274,1 +349,4 @@  int param = 0;    const VArray &vector = (dimensions_ != 1) ? get_vector(param++) : VArray{};  const VArray &w = (dimensions_ == 1) || (dimensions_ == 4) ? get_w(param++) :
Member

Use ELEM macro, same below.

Use ELEM macro, same below.
added 1 commit 2023-05-31 21:13:53 +02:00
Author
Member

@OmarEmaraDev Given what Clement just said:

I would say it is ok to commit in this state. We can change it if the issue arise.

Is the GLSL implementation good now?

@OmarEmaraDev Given what Clement just said: > I would say it is ok to commit in this state. We can change it if the issue arise. Is the GLSL implementation good now?
Author
Member

@JacquesLucke I changed most of the things you mentioned. Could you please have another look at the remaining two comments?

Regarding

It would be nice if you could do a few simple performance comparisons in geometry nodes just to see that there are no significant performance regressions. Taking the measurements from the UI is good enough for this

I have to admit that I don't work with geometry nodes all that often. How do you imagine the performance comparisons to look like?

@JacquesLucke I changed most of the things you mentioned. Could you please have another look at the remaining two comments? Regarding > It would be nice if you could do a few simple performance comparisons in geometry nodes just to see that there are no significant performance regressions. Taking the measurements from the UI is good enough for this I have to admit that I don't work with geometry nodes all that often. How do you imagine the performance comparisons to look like?
Member

I have to admit that I don't work with geometry nodes all that often. How do you imagine the performance comparisons to look like?

Take the .blend file linked below and fill in a table like the following:

Test Old New
Voronoi 3D Distance x ms y ms
Voronoi 4D Color ... ...

Just come up with a few different tests (maybe 5 that measure different things). It does not have to be an exhaustive list, the purpose is just to get a feeling for the performance impact of this patch. A few percent slowdown are acceptable. We can probably optimize the code for a few common cases later.

> I have to admit that I don't work with geometry nodes all that often. How do you imagine the performance comparisons to look like? Take the .blend file linked below and fill in a table like the following: | Test | Old | New | |------|-----|-----| | Voronoi 3D Distance | x ms | y ms | | Voronoi 4D Color | ... | ... | Just come up with a few different tests (maybe 5 that measure different things). It does not have to be an exhaustive list, the purpose is just to get a feeling for the performance impact of this patch. A few percent slowdown are acceptable. We can probably optimize the code for a few common cases later.
825 KiB
approved these changes 2023-06-01 19:16:56 +02:00
left a comment
Owner

This looks good from my side now. I could not find a significant performance impact with CPU and NVIDIA GPU rendering.

This looks good from my side now. I could not find a significant performance impact with CPU and NVIDIA GPU rendering.
Author
Member

@JacquesLucke

3D F1 Distance 96 ms 98 ms
3D F1 Color 98 ms 95 ms
3D F1 Position 99 ms 100 ms
3D F2 Distance 100 ms 98 ms
3D F2 Color 102 ms 99 ms
3D F2 Position 100 ms 98 ms
3D Smooth F1 Distance 783 ms 480 ms
3D Smooth F1 Color 829 ms 806 ms
3D Smooth F1 Position 790 ms 535 ms
3D Distance to Edge 167 ms 168 ms
3D N-Sphere Radius 157 ms 159 ms
1D F1 Distance 0.82 ms 0.81 ms
1D F2 Distance 0.80 ms 0.82 ms
1D Smooth F1 Distance 0.79 ms 0.80 ms
1D Distance to Edge 0.77 ms 0.81 ms
1D N-Sphere Radius 0.78 ms 0.76 ms
2D F1 Distance 27 ms 25 ms
2D F2 Distance 29 ms 28 ms
2D Smooth F1 Distance 133 ms 74ms
2D Distance to Edge 39 ms 41 ms
2D N-Sphere Radius 35 ms 35 ms
4D F1 Distance 394 ms 382 ms
4D F2 Distance 399 ms 399 ms
4D Smooth F1 Distance 5255 ms 3370 ms
4D Distance to Edge 748 ms 740 ms
4D N-Sphere Radius 704 ms 718 ms
@JacquesLucke | 3D | add-fractal-voronoi | main | | ----------- | ----------- | ----------- | | 3D F1 Distance | 96 ms | 98 ms | | 3D F1 Color| 98 ms | 95 ms | | 3D F1 Position| 99 ms | 100 ms | | 3D F2 Distance | 100 ms | 98 ms | | 3D F2 Color| 102 ms | 99 ms | | 3D F2 Position| 100 ms | 98 ms | | 3D Smooth F1 Distance | 783 ms | 480 ms | | 3D Smooth F1 Color| 829 ms | 806 ms | | 3D Smooth F1 Position| 790 ms | 535 ms | | 3D Distance to Edge | 167 ms | 168 ms | | 3D N-Sphere Radius | 157 ms | 159 ms | | 1D | add-fractal-voronoi | main | | ----------- | ----------- | ----------- | | 1D F1 Distance | 0.82 ms | 0.81 ms | | 1D F2 Distance | 0.80 ms | 0.82 ms | | 1D Smooth F1 Distance | 0.79 ms | 0.80 ms | | 1D Distance to Edge | 0.77 ms | 0.81 ms | | 1D N-Sphere Radius | 0.78 ms | 0.76 ms | | 2D | add-fractal-voronoi | main | | ----------- | ----------- | ----------- | | 2D F1 Distance | 27 ms | 25 ms | | 2D F2 Distance | 29 ms | 28 ms | | 2D Smooth F1 Distance | 133 ms | 74ms | | 2D Distance to Edge | 39 ms | 41 ms | | 2D N-Sphere Radius | 35 ms | 35 ms | | 4D | add-fractal-voronoi | main | | ----------- | ----------- | ----------- | | 4D F1 Distance | 394 ms | 382 ms | | 4D F2 Distance | 399 ms | 399 ms | | 4D Smooth F1 Distance | 5255 ms | 3370 ms | | 4D Distance to Edge | 748 ms | 740 ms | | 4D N-Sphere Radius | 704 ms | 718 ms |
Member

3 strange cases with a twofold deterioration.

3 strange cases with a twofold deterioration.
Author
Member

@JacquesLucke For all combinations of feature and dimension there is neither a significant performance increase nor decrease except for Smooth F1.

The reason why Smooth F1 is slower is probably due to the fact that some optimizations in voronoi_smooth_f1 in noise.cc were removed when I refactored the code. Previously only the output that was connected was being computed, while in the refactored version it always computes all outputs. This means that the performance is once again the same when multiple outputs are used, but is worse when only one output is used.

The optimizations used in voronoi_smooth_f1 are only possible in geometry nodes though. So the question is whether it's acceptable to keep the code consistent with the other implementations for this PR and to optimize it later on.
Or if it's bad enough to need to be optimized right away, which however would also mean that parts of the implementations of F1and F2 may need to be changed as well as they partly depend on the same pieces of code.

@JacquesLucke For all combinations of feature and dimension there is neither a significant performance increase nor decrease except for Smooth F1. The reason why Smooth F1 is slower is probably due to the fact that some optimizations in voronoi_smooth_f1 in noise.cc were removed when I refactored the code. Previously only the output that was connected was being computed, while in the refactored version it always computes all outputs. This means that the performance is once again the same when multiple outputs are used, but is worse when only one output is used. The optimizations used in voronoi_smooth_f1 are only possible in geometry nodes though. So the question is whether it's acceptable to keep the code consistent with the other implementations for this PR and to optimize it later on. Or if it's bad enough to need to be optimized right away, which however would also mean that parts of the implementations of F1and F2 may need to be changed as well as they partly depend on the same pieces of code.
Author
Member

We could also try an intermediate way of both.
My theory is that for Smooth F1 it's the Color output that drives up the computing time.
I could just optimize the Color output instead of all 3 outputs as is the case in main.
This alone may be enough the reach performance parity again, while keeping the difference to the other implementations minimal.

So I think that option would be the best... If it works.

We could also try an intermediate way of both. My theory is that for Smooth F1 it's the Color output that drives up the computing time. I could just optimize the Color output instead of all 3 outputs as is the case in main. This alone may be enough the reach performance parity again, while keeping the difference to the other implementations minimal. So I think that option would be the best... If it works.
Member

Would be good if you could try the optimization you just mentioned. Mainly to see if it works and how much complexity it adds. If it's too much complexity, we can also do it separately later. I think the optimization is important enough to justify different code.

Would be good if you could try the optimization you just mentioned. Mainly to see if it works and how much complexity it adds. If it's too much complexity, we can also do it separately later. I think the optimization is important enough to justify different code.
Owner

@Hoshinova, here is a workaround for the HIP compiler bug:

diff --git a/intern/cycles/kernel/svm/voronoi.h b/intern/cycles/kernel/svm/voronoi.h
index 3dcfc4a..6bc5671 100644
--- a/intern/cycles/kernel/svm/voronoi.h
+++ b/intern/cycles/kernel/svm/voronoi.h
@@ -62,7 +62,7 @@ ccl_device float voronoi_distance(const T a, const T b, ccl_private const Vorono
return reduce_max(fabs(a - b));
}
else if (params.metric == NODE_VORONOI_MINKOWSKI) {
-    return powf(reduce_add(pow(fabs(a - b), params.exponent)), 1.0f / params.exponent);
+    return powf(reduce_add(power(fabs(a - b), params.exponent)), 1.0f / params.exponent);
}
else {
return 0.0f;
diff --git a/intern/cycles/util/math_float2.h b/intern/cycles/util/math_float2.h
index c1ba2d2..10eedd2 100644
--- a/intern/cycles/util/math_float2.h
+++ b/intern/cycles/util/math_float2.h
@@ -222,13 +222,14 @@ ccl_device_inline float2 floor(const float2 a)
return make_float2(floorf(a.x), floorf(a.y));
}

-ccl_device_inline float2 pow(float2 v, float e)
+#endif /* !__KERNEL_METAL__ */
+
+/* Consistent name for this would be pow, but HIP compiler crashes in name mangling. */
+ccl_device_inline float2 power(float2 v, float e)
{
return make_float2(powf(v.x, e), powf(v.y, e));
}

-#endif /* !__KERNEL_METAL__ */
-
ccl_device_inline float2 safe_divide_float2_float(const float2 a, const float b)
{
return (b != 0.0f) ? a / b : zero_float2();
diff --git a/intern/cycles/util/math_float3.h b/intern/cycles/util/math_float3.h
index 7376240..bcb5662 100644
--- a/intern/cycles/util/math_float3.h
+++ b/intern/cycles/util/math_float3.h
@@ -469,7 +469,7 @@ ccl_device_inline bool isequal(const float3 a, const float3 b)
#endif
}

-ccl_device_inline float3 pow(float3 v, float e)
+ccl_device_inline float3 power(float3 v, float e)
{
return make_float3(powf(v.x, e), powf(v.y, e), powf(v.z, e));
}
diff --git a/intern/cycles/util/math_float4.h b/intern/cycles/util/math_float4.h
index 1f5f391..89b46d8 100644
--- a/intern/cycles/util/math_float4.h
+++ b/intern/cycles/util/math_float4.h
@@ -593,7 +593,7 @@ ccl_device_inline float4 ensure_finite(float4 v)
return v;
}

-ccl_device_inline float4 pow(float4 v, float e)
+ccl_device_inline float4 power(float4 v, float e)
{
return make_float4(powf(v.x, e), powf(v.y, e), powf(v.z, e), powf(v.w, e));
}

@Hoshinova, here is a workaround for the HIP compiler bug: Diff diff --git a/intern/cycles/kernel/svm/voronoi.h b/intern/cycles/kernel/svm/voronoi.h index 3dcfc4a..6bc5671 100644 --- a/intern/cycles/kernel/svm/voronoi.h +++ b/intern/cycles/kernel/svm/voronoi.h @@ -62,7 +62,7 @@ ccl_device float voronoi_distance(const T a, const T b, ccl_private const Vorono return reduce_max(fabs(a - b)); } else if (params.metric == NODE_VORONOI_MINKOWSKI) { - return powf(reduce_add(pow(fabs(a - b), params.exponent)), 1.0f / params.exponent); + return powf(reduce_add(power(fabs(a - b), params.exponent)), 1.0f / params.exponent); } else { return 0.0f; diff --git a/intern/cycles/util/math_float2.h b/intern/cycles/util/math_float2.h index c1ba2d2..10eedd2 100644 --- a/intern/cycles/util/math_float2.h +++ b/intern/cycles/util/math_float2.h @@ -222,13 +222,14 @@ ccl_device_inline float2 floor(const float2 a) return make_float2(floorf(a.x), floorf(a.y)); } -ccl_device_inline float2 pow(float2 v, float e) +#endif /* !__KERNEL_METAL__ */ + +/* Consistent name for this would be pow, but HIP compiler crashes in name mangling. */ +ccl_device_inline float2 power(float2 v, float e) { return make_float2(powf(v.x, e), powf(v.y, e)); } -#endif /* !__KERNEL_METAL__ */ - ccl_device_inline float2 safe_divide_float2_float(const float2 a, const float b) { return (b != 0.0f) ? a / b : zero_float2(); diff --git a/intern/cycles/util/math_float3.h b/intern/cycles/util/math_float3.h index 7376240..bcb5662 100644 --- a/intern/cycles/util/math_float3.h +++ b/intern/cycles/util/math_float3.h @@ -469,7 +469,7 @@ ccl_device_inline bool isequal(const float3 a, const float3 b) #endif } -ccl_device_inline float3 pow(float3 v, float e) +ccl_device_inline float3 power(float3 v, float e) { return make_float3(powf(v.x, e), powf(v.y, e), powf(v.z, e)); } diff --git a/intern/cycles/util/math_float4.h b/intern/cycles/util/math_float4.h index 1f5f391..89b46d8 100644 --- a/intern/cycles/util/math_float4.h +++ b/intern/cycles/util/math_float4.h @@ -593,7 +593,7 @@ ccl_device_inline float4 ensure_finite(float4 v) return v; } -ccl_device_inline float4 pow(float4 v, float e) +ccl_device_inline float4 power(float4 v, float e) { return make_float4(powf(v.x, e), powf(v.y, e), powf(v.z, e), powf(v.w, e)); } 
added 1 commit 2023-06-02 21:36:40 +02:00
-- Implement HIP name mangling workaround
added 1 commit 2023-06-02 21:39:09 +02:00
All checks were successful
buildbot/vexp-code-patch-coordinator Build done.
cc18f8a9c0
Author
Member

@JacquesLucke Looks like my intuition was correct:

3D Distance 487 ms 480 ms
3D Color 818 ms 806 ms
3D Position 499 ms 535 ms
1D Distance 0.82 ms 0.80 ms
2D Distance 80 ms 74 ms
4D Distance 3363 ms 3370 ms

Optimizing Color alone is enough to reach performance parity.

@JacquesLucke Looks like my intuition was correct: | Smooth F1 | add-fractal-voronoi | main | | ----------- | ----------- | ----------- | | 3D Distance | 487 ms | 480 ms | | 3D Color| 818 ms | 806 ms | | 3D Position | 499 ms | 535 ms | | 1D Distance | 0.82 ms | 0.80 ms | | 2D Distance | 80 ms | 74 ms | | 4D Distance | 3363 ms | 3370 ms | Optimizing Color alone is enough to reach performance parity.
Member

@blender-bot package

@blender-bot package
Member

Author
Member

There is one thing worth to mention about Normalize.

For F1, Smooth F1 and Distance to Edge it is mathematically guaranteed that the Distance output is in a [0.0, 1.0] range.

That is not the case for F2.
The reason for that is that unlike for the other features F2 normalization doesn't calculate the maximal amplitude.
This has 2 reasons: Firstly, the actual maximal amplitude of F2 is very expensive to compute. Secondly, the actual maximal amplitude of F2 is usually significantly greater than the mean value, meaning that dividing by it would excessively reduce the Distance output in most cases.

Instead it uses the maximal amplitude of F1 scaled by a factor of 2.0, with the logic behind that being that the F2 Distance output is usually less than 2.0 times greater than F1.

In theory this could result to the output being greater than 1.0 with very particular arrangements of feature points, however as the distribution of feature points is randomized this doesn't actually happen in practice (as far as I have tested it).

So if in the future there are bug reports about the normalized F2 Distance outputting values greater than 1.0 for certain input combinations, it's a known limitation.

There is one thing worth to mention about Normalize. For F1, Smooth F1 and Distance to Edge it is mathematically guaranteed that the Distance output is in a [0.0, 1.0] range. That is not the case for F2. The reason for that is that unlike for the other features F2 normalization doesn't calculate the maximal amplitude. This has 2 reasons: Firstly, the actual maximal amplitude of F2 is very expensive to compute. Secondly, the actual maximal amplitude of F2 is usually significantly greater than the mean value, meaning that dividing by it would excessively reduce the Distance output in most cases. Instead it uses the maximal amplitude of F1 scaled by a factor of 2.0, with the logic behind that being that the F2 Distance output is usually less than 2.0 times greater than F1. In theory this could result to the output being greater than 1.0 with very particular arrangements of feature points, however as the distribution of feature points is randomized this doesn't actually happen in practice (as far as I have tested it). So if in the future there are bug reports about the normalized F2 Distance outputting values greater than 1.0 for certain input combinations, it's a known limitation.
Author
Member

Looks like everything is done.

@brecht @JacquesLucke @OmarEmaraDev Looks like everything is done. Are we ready to merge?
approved these changes 2023-06-07 18:58:47 +02:00
approved these changes 2023-06-08 14:58:46 +02:00
 @ -2167,2 +2081,2 @@  correctionFactor;  }  correctionFactor /= 1.0f + 3.0f * params.smoothness;  if (calc_color) { /* Only compute Color if necessary, as it is very expensive */
Member

Comment style: move comment to its own line below and end with a ..

Comment style: move comment to its own line below and end with a ..
added 2 commits 2023-06-10 16:41:34 +02:00
changed title from WIP: Add Fractal Voronoi Noise V.2 to Add Fractal Voronoi Noise V.2 2023-06-10 16:41:55 +02:00
merged commit 144ad4d20b into main 2023-06-13 09:18:18 +02:00
referenced this issue from a commit 2023-06-13 09:18:20 +02:00
Owner

We will need release notes and user manual docs for this. @Hoshinova, could you help with this?

The release notes can have a relatively short explanation of what the changes are, with maybe 2 images showing it in action. And then the user manual can have a more detailed explanation.

We will need release notes and user manual docs for this. @Hoshinova, could you help with this? The release notes can have a relatively short explanation of what the changes are, with maybe 2 images showing it in action. And then the user manual can have a more detailed explanation.
Author
Member

Voronoi Texture Node

• The Voronoi Texture Node has support for fractal noise now. 144ad4d20b
• For that 3 new inputs have been added. "Detail", which controls the number of layers to compute, "Roughness" which controls how much influence the higher layers have on the final output and "Lacunarity", which controls a factor by which each successive layer is scaled with.
• Additionally a "Normalize" property was added, which if checked will remap the Distance and Color outputs to a [0.0, 1.0] range. When checked the F1/Smooth F1 and Distance to Edge outputs are guaranteed to always be within that range. For F2 the Distance output may exceed 1.0 in rare cases even when "Normalize" is checked.

Below is a demonstration with left plane only showing the Voronoi output and the right plane showing the node tree using that output as a bump map in EEVEE.

Voronoi Texture Node --- - The Voronoi Texture Node has support for fractal noise now. https://projects.blender.org/blender/blender/commit/144ad4d20b051997fd9729682b3abe1f353584b1 - For that 3 new inputs have been added. "Detail", which controls the number of layers to compute, "Roughness" which controls how much influence the higher layers have on the final output and "Lacunarity", which controls a factor by which each successive layer is scaled with. - Additionally a "Normalize" property was added, which if checked will remap the Distance and Color outputs to a [0.0, 1.0] range. When checked the F1/Smooth F1 and Distance to Edge outputs are guaranteed to always be within that range. For F2 the Distance output may exceed 1.0 in rare cases even when "Normalize" is checked. Below is a demonstration with left plane only showing the Voronoi output and the right plane showing the node tree using that output as a bump map in EEVEE. ![Fractal_Voronoi_Demo_Combined.png](/attachments/a6b8bb66-8912-4c87-ad09-db7ae127d3e4)
Author
Member

@brecht That is for the release notes.

I'm quite busy right now, so I can't make the user manual entry immediately. I might be able to next week, but it could also take a little longer than that.
Since the 4.0 release is still far away I take that it's not that urgent?

@brecht That is for the release notes. I'm quite busy right now, so I can't make the user manual entry immediately. I might be able to next week, but it could also take a little longer than that. Since the 4.0 release is still far away I take that it's not that urgent?
Owner

Thanks, I've put the text in the release notes now. For the images, I don't normally put UI screenshots in there, and the green bump map is a bit too much programmer art. Also I prefer the images to be separated instead of composited into one bigger images.

Maybe just some 2D images with example patterns would be better. Like the left pattern in your image, the hand painted wall, the fabric patterns in 2D instead of applied to a 3D model. Or 3 images showing progressively more octaves to explain a bit what this does.

Thanks, I've put the text in the release notes now. For the images, I don't normally put UI screenshots in there, and the green bump map is a bit too much programmer art. Also I prefer the images to be separated instead of composited into one bigger images. Maybe just some 2D images with example patterns would be better. Like the left pattern in your image, the hand painted wall, the fabric patterns in 2D instead of applied to a 3D model. Or 3 images showing progressively more octaves to explain a bit what this does.
Author
Member

@brecht

I think it describes the concept of Fractal Voronoi noise pretty well.
I'd put it in both the release notes and the user manual.

@brecht What about this: ![Fractal_Voronoi_Explanation.png](/attachments/a4d300ca-fff0-410e-b730-5b7836f2a0ba) I think it describes the concept of Fractal Voronoi noise pretty well. I'd put it in both the release notes and the user manual.
Owner

I think that's a bit technical, mainly users want to see what the new parameters do, not so much how the implementation works. We also should avoid text in images like this, because it can't be translated, updates are difficult, doesn't work with screen readers, and doesn't follow the fonts and style of the website automatically.

I would rather have some images over the parameter range, some examples:
https://wiki.blender.org/wiki/Reference/Release_Notes/2.83/Cycles

I think that's a bit technical, mainly users want to see what the new parameters do, not so much how the implementation works. We also should avoid text in images like this, because it can't be translated, updates are difficult, doesn't work with screen readers, and doesn't follow the fonts and style of the website automatically. I would rather have some images over the parameter range, some examples: https://docs.blender.org/manual/en/latest/render/shader_nodes/textures/voronoi.html https://wiki.blender.org/wiki/Reference/Release_Notes/2.83/Cycles https://docs.blender.org/manual/en/latest/render/shader_nodes/shader/hair_principled.html
Author
Member

Do you mean something like this?:

Detail = 1.0:

Detail = 2.0:

Detail = 4.0:

Roughness = 0.25:

Roughness = 0.5:

Roughness = 1.0:

Lacunarity = 2.0:

Lacunarity = 4.0:

Lacunarity = 8.0:

Do you mean something like this?: Detail = 1.0: ![F1_Detail_1.png](/attachments/3df3a1a9-d1f3-4ac8-ae56-734aa06a2585) Detail = 2.0: ![F1_Detail_2.png](/attachments/62a2de06-3fb3-400c-b7bf-a067dff4510d) Detail = 4.0: ![F1_Detail_4.png](/attachments/35000a5e-1a66-4347-963e-083520b98b48) Roughness = 0.25: ![F1_Roughness_025.png](/attachments/a3471cb3-13a2-4729-9103-a3f8cd824ff0) Roughness = 0.5: ![F1_Roughness_05.png](/attachments/8466da44-c9f6-4f0c-93cb-be805cc47f75) Roughness = 1.0: ![F1_Roughness_1.png](/attachments/025bef58-2a4b-42f8-8925-80b739d24b25) Lacunarity = 2.0: ![F1_Lacunarity_2.png](/attachments/2ba7275a-41fd-449b-94b2-c0a22c1529f0) Lacunarity = 4.0: ![F1_Lacunarity_4.png](/attachments/ea117bb5-271b-4f24-aa43-bc0574271417) Lacunarity = 8.0: ![F1_Lacunarity_8.png](/attachments/8a98a299-64f0-41a3-9cbe-929c80865b9c)
625 KiB
614 KiB
858 KiB
611 KiB
625 KiB
724 KiB
606 KiB
774 KiB
625 KiB
Author
Member

@brecht I've got a small correction to make.
For the release notes I wrote

When checked the F1/Smooth F1 and Distance to Edge outputs are guaranteed to always be within that range. For F2 the Distance output may exceed 1.0 in rare cases even when "Normalize" is checked

This is actually incorrect. Due to certain monotony properties of the functions involved it can be shown that F2 max_amplitude \leq 2 * F1 max_amplitude.
Therefore the F2 Distance output may not exceed 1.0 when "Normalize" is checked.

Could you please remove those 2 sentences from the release notes? Thanks!

@brecht I've got a small correction to make. For the release notes I wrote > When checked the F1/Smooth F1 and Distance to Edge outputs are guaranteed to always be within that range. For F2 the Distance output may exceed 1.0 in rare cases even when "Normalize" is checked This is actually incorrect. Due to certain monotony properties of the functions involved it can be shown that F2 max_amplitude $\leq$ 2 * F1 max_amplitude. Therefore the F2 Distance output may not exceed 1.0 when "Normalize" is checked. Could you please remove those 2 sentences from the release notes? Thanks!
Owner
Thanks, I updated the release notes text and images: https://wiki.blender.org/wiki/Reference/Release_Notes/4.0/Cycles#Fractal_Voronoi_Texture
referenced this issue from a commit 2023-07-06 16:00:09 +02:00
No reviewers
No Milestone
No project
No Assignees
10 Participants