Cycles Vector Math node adding multiply and dividing
Needs ReviewPublic
Actions

Authored by Wasili Novratidis (WarLoc3D) on Dec 10 2017, 10:34 AM.

Details

Reviewers

Summary

This is an addition to excisting code that is already present in the Cycles Vector Math node. The same can be done with current nodes (separate/combine xyz + math nodes) but is not user friendly if you need many calculations. Hence I altered the vector math node. Power and square root could easily be added to this node but I have not found any use cases for it so I refrain from it. I tested it in Cycles by creating a blinn specular calculation behind an emitter node.

The only thing that I could not understand is this code:
\intern\cycles\test\render_graph_finalize_test.cpp
I presume this is for testing and not necessary.

\intern\cycles\render\constant_fold.cpp
In the "void ConstantFolder::fold_vector_math(NodeVectorMath type) const" I suspect that the multiply and divide should be contained but I do not quit understand the code before me because it feels like it is not coherent. Please imply if I have to change this.

Thanks in advance.

Diff Detail

Repository: rB Blender

Event Timeline

Wasili Novratidis (WarLoc3D) created this revision.Dec 10 2017, 10:34 AM

Generally fine, but this should be doing safe division.

intern/cycles/kernel/shaders/node_vector_math.osl
52	Think there is an utility function for this.
55	This should be doing safe division, similar to math node.
source/blender/nodes/shader/nodes/node_shader_vectMath.c
145	Does this mean that GLSL is not supported for this operations?

This revision now requires changes to proceed.Dec 13 2017, 11:57 AM

Dear Sergey,

I save-divided everything I could find. This resulted in a lot more code. I conformed myself to how the similar math modules where constructed. All the code has been copied from either the math nodes or util nodes. Refrained myself on the osl node to use a float and not float3 which would make things more complicated.
You where absolutely right about the GLSL code, mega thanks. I have tested the code in various ways for dividing by zero or negatives. Am pretty interested if you still notice flaws or bad coding, please state so.

Kind regards,

Wasili

Pointed out some things that stood out to me, however wait for @Sergey Sharybin (sergey) opinion before changing them, he has final say in these things.

intern/cycles/kernel/shaders/node_vector_math.osl
19	probably better to move this to an include file instead of duplicating the code.
64	@Sergey Sharybin (sergey) most other modes seem to return the normalized result of the vector here, not sure i like the arbitrary math here (this comment goes for all other implementations too) what do you think?
intern/cycles/util/util_math_float3.h
331	more code duplication, if i were lazy i'd just call safe_divide_color. If i were bold, i'd rename safe_divide_color to something more generic.

Wasili Novratidis (WarLoc3D) added inline comments.Dec 16 2017, 6:18 PM

intern/cycles/kernel/shaders/node_vector_math.osl
19	Thank. Yeah bugged me as well but is also present in the math node. Just say what you guys want and I change it.
intern/cycles/util/util_math_float3.h
331	Lol, it is the copy of that. Same as reply from above. Just say so and I change it. I do not know what is allowed yes or no. The only thing I noticed is that the safe_normalize math was also in this file, that's why I added it.