[Bf-codereview] Channel Matte Node (issue 5602051)
dfelinto at gmail.com
dfelinto at gmail.com
Wed Feb 1 09:19:10 CET 2012
Reviewers: bf-codereview_blender.org, jbkkavt,
Description:
This is a huge node (too many sub-options) so this is my first take.
I didn't review it, but in my quick test (with rgb) it seems to match
trunk.
My questions:
1) is better to store settings and custom1 and custom2 OR the individual
values (as implemented in the patch).
2) it's better to access the class once (const int limit_min =
this->limit_min) or it's fine to do it every time it's needed (as it's
now)
3) I unified both methods (max and simple) by forcing the simple to use
the max algorithm. I think it's acceptable. The alternative would be to
have a new operation for each algorithm or have an IF statement in the
execute code.
Note: I didn't review the colorspace code. I may need to normalize the
input from the convert operation. I may not, too late to look at that.
I think there is enough on this patch to keep Jeroen busy while I take
my turn into sleep ;)
Please review this at http://codereview.appspot.com/5602051/
Affected files:
source/blender/compositor/CMakeLists.txt
source/blender/compositor/intern/COM_Converter.cpp
source/blender/compositor/nodes/COM_ChannelMatteNode.cpp
source/blender/compositor/nodes/COM_ChannelMatteNode.h
source/blender/compositor/operations/COM_ChannelMatteOperation.cpp
source/blender/compositor/operations/COM_ChannelMatteOperation.h
More information about the Bf-codereview
mailing list