Page MenuHome

Fix T846224: Extra user for geometry nodes data-blocks
AbandonedPublic

Authored by Fabian Schempp (fabian_schempp) on Jan 14 2021, 10:18 AM.

Details

Summary

When adding a Geometry Nodes Tree or unlinking a Geometry Nodes Tree to a Modifier, the User count always stays above 0. (id-us >= 1)
(see: https://developer.blender.org/T84624
for more detail)

This seems to be caused by two things

  1. On all Node Trees/Node Groups the function is id_us_ensure_real() is called.

this sets LIB_TAG_EXTRAUSER and LIB_TAG_EXTRAUSER_SET flags which than cause that id_min() can not set the user count to 0 even after removing the last user. My solution here is to not call id_us_ensure_real() for Geometry Node Type, since it seems not working right here.

But. I don't fully understand why it is needed and working for other node types. A more mighty wizard is needed to clarify how this works.

  1. RNA update function seems to be called only after assignment and so only on the newly assigned value not the old one.. So, when we add a new node_group to the Modifier,

we need to store the old value, assign the new value and than decrement the user count on the old value manualy. Since the interface button calls a Python function to add a new Node Group to the Modifier I added a new rna function called node_group_new() for modifiers, to handle this.

Diff Detail

Repository
rB Blender
Branch
FixT84624 (branched from master)
Build Status
Buildable 12180
Build 12180: arc lint + arc unit

Event Timeline

Fabian Schempp (fabian_schempp) requested review of this revision.Jan 14 2021, 10:18 AM
Fabian Schempp (fabian_schempp) created this revision.
Hans Goudey (HooglyBoogly) retitled this revision from Fixes T846224 to Fix T846224: Extra user for geometry nodes data-blocks.
Hans Goudey (HooglyBoogly) requested changes to this revision.Jan 18 2021, 7:48 AM

While there's plenty of information in the bug report, the description of this patch isn't complete.

While making this patch you must have thought of alternative solutions, decided not to do them? How does this compare to what's done for other node tree data blocks? etc.
I'm not saying it needs to be exhaustive, but a little more would be great. See more here: https://wiki.blender.org/wiki/Process/Contributing_Code#Ingredients_of_a_Patch

This revision now requires changes to proceed.Jan 18 2021, 7:48 AM
Fabian Schempp (fabian_schempp) edited the summary of this revision. (Show Details)
Jacques Lucke (JacquesLucke) requested changes to this revision.Feb 15 2021, 6:37 PM

In my simple test, most of this patch does not seem to be necessary to get to the intended behavior. Just the change in space_node.c seems to be enough. Can you confirm that?

source/blender/editors/space_node/space_node.c
84

It seems like just doing this is enough. However, I do wonder why this should be done.

Related: rBd75e0f320b4af13d5a35c5787a8ff992b959149e, T36024.

Given that we have can store id pointers in custom properties, and we can set fake users, it seems like we could remove id_us_ensure_real here for all node tree types.
It seems that this has been a "hack" in the first place. Either the editor is an owner of the data block, or it is not. Currently the behavior seems confusing because the "real user" is not removed when the node group is not used anymore.

source/blender/modifiers/intern/MOD_nodes.cc
837

It seems like this does not do anything. ntree->id.tag is 0 for me.

Closing this, because I committed another simpler fix for the release.