[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