Based on changes and comments in D13069.
Move smoothness clamping inside function.
Prevent division by zero.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- master
- Build Status
Buildable 18479 Build 18479: arc lint + arc unit
Event Timeline
| intern/cycles/kernel/osl/shaders/node_voronoi_texture.osl | ||
|---|---|---|
| 119–120 | The reason I suggested to move them together is so that you could do it in one go: float smoothness_clamped = clamp(smoothness / 2.0, FLT_MIN, 0.5); I don't think a separate smoothness_min is needed. | |
| 925 | This should not have been moved, just float smoothness = clamp(Smoothness / 2.0, FLT_MIN, 0.5); here should be all that is needed for OSL. | |
| intern/cycles/kernel/svm/voronoi.h | ||
| 966 | This should not have been moved, just smoothness = clamp(smoothness / 2.0f, FLT_MIN, 0.5f) here should be all that is needed for SVM. | |
| source/blender/nodes/shader/nodes/node_shader_tex_voronoi.cc | ||
| 335 | rand clamp should also be moved. The API is less error prone if all inputs are automatically clamped rather than having implicit assumption about valid range, which is part of why I suggested to move the smoothness clamping. | |
Comment Actions
@Brecht Van Lommel (brecht) I repeated the same response for some of the comments in regard to consistency between implementations. I'll wait for guidance before submitting a new patch.
| intern/cycles/kernel/osl/shaders/node_voronoi_texture.osl | ||
|---|---|---|
| 119–120 | smoothness_min is needed for the division used in the smoothstep function. Keeping this separate ensures correctionFactor is zero when smoothness is zero. This was picked up in review of D13069. This is why smoothness can't just be clamped to FLT_MIN, 0.5. | |
| 925 | I tried to keep consistency between all implementations. This suggests clamping inside function for noise.cc but not for Cycles/OSL? Is that due to the way the functions are called or because the functions in noise.cc are likely to be reused? I'm assuming that since noise.cc maybe reused it can be implemented differently here? | |
| intern/cycles/kernel/svm/voronoi.h | ||
| 966 | I tried to keep consistency between all implementations. This suggests clamping inside function for noise.cc but not for Cycles/OSL? Is that due to the way the functions are called or because the functions in noise.cc are likely to be reused? I'm assuming that since noise.cc maybe reused it can be implemented differently here? | |
| source/blender/nodes/shader/nodes/node_shader_tex_voronoi.cc | ||
| 335 | I tried to keep consistency between all implementations. This suggests clamping inside function for noise.cc but not for Cycles/OSL? Is that due to the way the functions are called or because the functions in noise.cc are likely to be reused? I'm assuming that since noise.cc maybe reused it can be implemented differently here? In this case, should I move rand clamping inside all voronoi functions in noise.cc? | |