[Bf-codereview] Hue Saturation Value Node for Cycles (issue 5447073)

dfelinto at gmail.com dfelinto at gmail.com
Fri Dec 2 11:19:53 CET 2011


Reviewers: bf-codereview_blender.org, brechtvl,

Description:
conversion of HSV Node to Cycles

OSL:
----
I moved the rgb_to_hsv and hsv_to_rgb out of the node_mix.osl to the
node_color.h (where we have srgb/linear space functios).

Also, I had to change the variables name (in relation to the nodes.cpp).
Because there are two variables named Color. Since I can't build OSL, I
couldn't really test if the arguments will be parsed by order or by the
name.

SVM:
----
I moved the rgb... functions to the new svm_hsv.h.
I think that it makes more sense to be there than to be in the svm_mix.h
It can be moved to a new svm_color.h if you think it's better.

Other:
-----
I'm using fmod instead of the manual operation we have in the blender
render node:
hsv[0] += hue - 0.5; if hsv[0] < 0: hsv[0] -= 1.0;  if hsv[0] > 1:
hsv[0] += 1.0

I don't know if fmod is faster or not though.

Also, Hue, Saturation and Value can be calculated separated. So in
theory this node could be broke in 3 nodes that could run in parallel.
I don't know if the code would take advantage of this though (or if the
overhead would make it worse). And I'm not sure if we actually get
multithreading when having multiple nodes (I think not, but that may be
another way).

Please review this at http://codereview.appspot.com/5447073/

Affected files:
   intern/cycles/app/cycles_xml.cpp
   intern/cycles/blender/blender_shader.cpp
   intern/cycles/kernel/CMakeLists.txt
   intern/cycles/kernel/osl/nodes/CMakeLists.txt
   intern/cycles/kernel/osl/nodes/node_color.h
   intern/cycles/kernel/osl/nodes/node_hsv.osl
   intern/cycles/kernel/osl/nodes/node_mix.osl
   intern/cycles/kernel/svm/svm.h
   intern/cycles/kernel/svm/svm_hsv.h
   intern/cycles/kernel/svm/svm_mix.h
   intern/cycles/kernel/svm/svm_types.h
   intern/cycles/render/nodes.cpp
   intern/cycles/render/nodes.h
   source/blender/nodes/shader/nodes/node_shader_hueSatVal.c




More information about the Bf-codereview mailing list