Custom Nodes

Custom Nodes

[brechtvl, 2013-01-21 12:03:57]

Managed to get the patch uploaded here, just running "python
source/tools/codereview_upload.py" worked.

I will continue to read through the code and post comments.

https://codereview.appspot.com/7099061/diff/1/release/scripts/startup/bl_oper...
File release/scripts/startup/bl_operators/node.py (right):

https://codereview.appspot.com/7099061/diff/1/release/scripts/startup/bl_oper...
release/scripts/startup/bl_operators/node.py:193: node =
self.create_node(context, item[len(node_type_prefix):])
I think NODE_OT_add_search needs to be derived from NodeAddOperator for this to
work?

https://codereview.appspot.com/7099061/diff/1/source/blender/nodes/shader/nod...
File source/blender/nodes/shader/node_shader_tree.c (right):

https://codereview.appspot.com/7099061/diff/1/source/blender/nodes/shader/nod...
source/blender/nodes/shader/node_shader_tree.c:70: * This will probably change
in the future, then each engine will get its own poll function.
We discussed this before, but I still think we should not do this at the node
tree level. It's useful to be able to have setups for multiple engines in one
node tree, and some node groups can contain nodes that work in multiple engines.
Maya/XSI/... also allow such setups.

I think such a poll function should be implemented at the node level, and then
nodes that don't work in certain engines can be greyed out in the node editor.

https://codereview.appspot.com/7099061/diff/1/source/blender/nodes/shader/nod...
source/blender/nodes/shader/node_shader_tree.c:84: if (snode->shaderfrom ==
SNODE_SHADER_OBJECT) {
Is it a safe assumption that snode is in the context?

[brechtvl, 2013-01-21 12:40:22]

Testing the patch, I found these issues:

* For Cycles in the material properties, it shows "No output node" in the
Surface and Displacement panels.

* For color sockets, it now shows the color box aligned to the right of the node
rather than the left. Is this intentional?  With e.g. a Diffuse BSDF node this
looks quite confusing to me.

* When doing "Make Group", there is a submenu with one item. That seems
pointless, and I'm not even sure why there has do be a popup in the first place.

* When entering a group, there is a green background color shown. I'm not sure a
different background color is needed, a good indication in the header is enough
I think.

* Group Input/Ouptut nodes have a black header here, missing version patch for
theming?

* It seems that you can put Group Input/Output nodes in groups themselves. I
guess that's not intenional.

* When there are no nodes selected or use nodes is disabled, it doesn't show the
grid lines. I think they should show still, I think it's a good visual cue for
quickly seeing which editor type you're looking at. Other editors like graph or
image do the same.

* Cycles Render > open node editor > add new material, it prints
"RNA_string_set: NodeSocketShader.value_property not found." in the console.

* Cycles render > open node editor > add new material > press pin button > split
node editor in two > crash.

* Cycles render > open node editor > add new material > Diffuse BSDF color has 0
alpha value instead of 1.

* Cycles render > open node editor > add new material > select Diffuse BSDF >
make group > exit group. Group color socket has black rather than white color.

* Cycles render > open node editor > add new material > pin > change to display
compositing nodes:
    layout.prop(snode_id.render, "use_free_unused_nodes", text="Free Unused")
AttributeError: 'Material' object has no attribute 'render'

* Cycles render > open node editor > change to texture nodes > crash.

* I think the pin icon should be to the right of the datablock selector as it is
in the image editor.

[brechtvl, 2013-01-22 14:21:38]

Opening this file causes a crash with the patch applied.
http://projects.blender.org/tracker/download.php/9/498/33896/23575/pm_bug.blend

0   blender                             0x000000010e58784c blender_crash_handler
+ 412
1   libsystem_c.dylib                   0x00007fff8b0228ea _sigtramp + 26
2   ???                                 0xfffffffffffffff0 0x0 +
18446744073709551600
3   blender                             0x000000010eb381fc node_group_input_init
+ 44
4   blender                             0x000000010ea9d42b nodeAddNode + 459
5   blender                             0x000000010eb64742 do_versions + 12274
6   blender                             0x000000010eb76096
blo_read_file_internal + 2518

[brechtvl, 2013-01-22 15:31:44]

https://codereview.appspot.com/7099061/diff/30005/doc/python_api/examples/bpy...
File doc/python_api/examples/bpy.types.NodeTree.1.py (right):

https://codereview.appspot.com/7099061/diff/30005/doc/python_api/examples/bpy...
doc/python_api/examples/bpy.types.NodeTree.1.py:23: return
context.scene.render.engine == 'CYCLES'
The code snippet here is not an example of get_from_context

https://codereview.appspot.com/7099061/diff/30005/intern/cycles/blender/blend...
File intern/cycles/blender/blender_shader.cpp (right):

https://codereview.appspot.com/7099061/diff/30005/intern/cycles/blender/blend...
intern/cycles/blender/blender_shader.cpp:172: * are subclasses of another.
I don't see how that's a problem, I wouldn't use XXX in comments unless
something is broken and needs to be fixed.

https://codereview.appspot.com/7099061/diff/30005/intern/cycles/render/nodes.h
File intern/cycles/render/nodes.h (right):

https://codereview.appspot.com/7099061/diff/30005/intern/cycles/render/nodes....
intern/cycles/render/nodes.h:197: class GroupInputNode : public ShaderNode {
These two class are not used, so can be removed?

https://codereview.appspot.com/7099061/diff/30005/release/scripts/startup/bl_...
File release/scripts/startup/bl_ui/node.py (right):

https://codereview.appspot.com/7099061/diff/30005/release/scripts/startup/bl_...
release/scripts/startup/bl_ui/node.py:23: 
Does this file have a purpose?