Is `ShaderNodeCustomGroup` broken for everyone or just me?

While working on my addon I come across some bugs in 2.81

  • method free is not called for nested nodes when node tree is deleted, so it’s rather useless for cleaning up
  • nothing is even called when material is cloned, nodes are shared, and any internal adjustments affect all cloned materials
  • inheriting one node class from another breaks base class, if they’re registered in wrong order

It seems like the system is broken on pretty deep level.

Or, maybe, I just misunderstand the whole concept of the ShaderNodeCustomGroup and just trying to cook them wrong way?

  • There’s no builtin check to tell if nested nodes are being used somewhere else, which would cause problems if you delete them. You can delete them explicitly in you main node free since it was on your main node init that you created them. You could also unfold all nested nodes, which is what I normally do (by making a function for build those blocks) .
  • Cloning a node will call copy(self, node) which will let you duplicate your structure. By default, cloning a node, will just make a new instance of the same nodetree.
  • Split the inheritance. Instead of having a base that inherits from ShaderNodeCustomGroup, just make a base without heritance, and only inherit the SNCG in the end node class.
    class base(): common functions and variables -> class node(ShaderNodeCustomGroup, base)
    You could also make turn your base class into a module instead, and use import module.

There’s built-in refcounter for nodes. It’s not critical, but just seems strange that I need to scan nodes, check if they have the free method defined and call it explicitely.

Cloning node calls the copy method, but cloning whole material just moves nodes to new nodetree without calling anything.

I already moved all utility methods like add_node, add_math to mixin class (I used it to modify existing material). But my base class also defines init, free, copy and needs inheritance.

The trick with putting SNCG into leaf classes should work well. But the very fact that inheritance breaks base class is pretty severe even in python’s terms.

That is something I haven’t thought about when I wrote those classes … I must look the source to figure what’s happening there. :grimacing:

I took a quick look at your addon source’s, and perhaps there’s something there failing. You don’t really need a base class for almost nothing. You can put all helper functions into modules, and import them when needed. :confused:

I’ve reported all three bugs:
https://developer.blender.org/T71698
https://developer.blender.org/T71858
https://developer.blender.org/T72150

Going to rebuild and debug blender. Wish me good luck :slight_smile:

Have you tried to just use SNCG in the final nodes, and not in your base nodes? Because I never had problems with inheritance. And that’s probably the solution for your first two bug reports.

About the third report, it does have very little to do with BKE_node_copy_ex call. When LIB_ID_CREATE_NO_MAIN or LIB_ID_COPY_LOCALIZE are set, it means the nodetree is being copied for internal purposes only. It shouldn’t call the python functions.
I suspect the solution would be to correct the dependencies (in depsgraph).

I don’t quite understand the logic of the node copying, but this condition (via no_usercount) is set when copying nodes of a material.

It took me some time to understand that part of blender too. Specially with the new depsgraph (from 2.79+) and the Copy_on_Write stuff, which broke the old NodeCustomGroup logic. But the thing is that a node_tree gets copied for a lot of reasons (editing, rendering, etc), and most of the times there’s no need to inform python nodes that it is happening.

It has been a while since I last touch the node copying logic… I’ll try to have a new look at it when I have time (probably just by the end of the month).