[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