The geometry node port of voronoi_smooth_f1 function has a
division by zero when smoothness is set to zero.
Using a safe_divide within the function causes issues
and was noted in the original patch D12725.
Solution in this case is to clamp zero smoothness to FLT_EPSILON.
Details
Details
- Reviewers
Jacques Lucke (JacquesLucke) - Maniphest Tasks
- T92736: Geometry Nodes: Hole in mesh after Set Position
- Commits
- rB6981bee2c77f: Fix T92736: Hole in mesh after Set Position
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
I think either this should be done in noise.cc or there has to be an assert in noise.cc to make sure that smoothness > 0.
Also I wonder if it should be FLT_MIN instead of FLT_EPSILON.
| source/blender/blenlib/intern/noise.cc | ||
|---|---|---|
| 1563 | Dividing by a "factor" is always a bit weird.. Might be interesting if there is a measurable performance improvement by replacing division with multiplication here, but that should be done separately. | |
| 1564 | Does this really have to be changed as well? | |
Comment Actions
I assume you've checked that this does in fact solve the issue in the original bug report.
Comment Actions
@Brecht Van Lommel (brecht) Now that textures are used across Cycles/Eevee and GN, I'm wondering if this fix should be made to the shader nodes? Using a safe_divide here does not work btw.
@Jacques Lucke (JacquesLucke)/@Brecht Van Lommel (brecht) The other possibility is that when smoothness is zero, then standard voronoi_smooth_f1 is called. That is more performant when smoothness is zero but would introduce an if condition when smoothness is non-zero.
Comment Actions
Yes, we should keep the clamping consistent between all implementations.
There's already clamping happening earlier, so it would be more efficient to do it there:
const float smth = std::min(std::max(smoothness[i] / 2.0f, 0.0f), 0.5f);
I suggest to move all such clamping from node_shader_tex_voronoi.cc to noise.cc.
I would not bother optimizing specifically for the smoothness zero case, it's hard to verify that the results will match and that there will be no discontinuity.
Comment Actions
@Brecht Van Lommel (brecht) Ok, I'll prepare a patch to keep things aligned between the code bases.