Wrong user counting with ID properties in pynodes #61506

New Issue

Jacques Lucke · 2019-02-13T12:36:02+01:00

Jacques Lucke commented

2019-02-13 12:36:02 +01:00

Blender Version
Broken: 57f0b175d7

Short description of error
When a custom node has an idprop of type ID, then the user count is not increased when the node is copied.

Exact steps for others to reproduce the error

Execute the script.
Select an object in the node.
Duplicate that node and observe that the user count of the object does not increase (in the outliner).

lang=python
import bpy
from bpy.props import PointerProperty

class MyTreeType(bpy.types.NodeTree):
    bl_idname = "MyTreeType"
    bl_label = "My Tree Type"
    bl_icon = "NONE"
    
    
class MyNodeType(bpy.types.Node):
    bl_idname = "MyNodeType"
    bl_label = "My Node Type"

    object: PointerProperty(type=bpy.types.Object)
    
    def draw_buttons(self, context, layout):
        layout.prop(self, "object", text="")

    
bpy.utils.register_class(MyTreeType)
bpy.utils.register_class(MyNodeType)

tree = bpy.data.node_groups.new("Test Tree", "MyTreeType")
tree.nodes.new("MyNodeType")

bpy.context.area.type = "NODE_EDITOR"
bpy.context.space_data.tree_type = "MyTreeType"
bpy.context.space_data.node_tree = tree

Initial investigation

This is caused by the fact that nodeCopyNode passes LIB_ID_CREATE_NO_USER_REFCOUNT into BKE_node_copy_ex.
I suspect this is the case because the node should store a "weak" pointer to the node tree.
However, the same flag is used to copy the idprops, resulting in wrong behavior.

Are there cases when node->id should not be a weak pointer?
If not, then we should just get rid of

if ((flag & LIB_ID_CREATE_NO_USER_REFCOUNT) == 0) {
	id_us_plus(node_dst->id);
}

in BKE_node_copy_ex and don't use the flag at all.

Another related issue is that when the scene is unloaded, the object referenced by the idprop is freed, before the node is freed.
So, when the node is freed, it tries to decrement the user count of the object -> crash.
Not sure what's the best way to fix this. Usually the reference to the object should be removed from the idprop, when the object is removed, right?

**Blender Version** Broken: 57f0b175d7 **Short description of error** When a custom node has an idprop of type ID, then the user count is not increased when the node is copied. **Exact steps for others to reproduce the error** 1. Execute the script. 2. Select an object in the node. 3. Duplicate that node and observe that the user count of the object does not increase (in the outliner). ``` lang=python import bpy from bpy.props import PointerProperty class MyTreeType(bpy.types.NodeTree): bl_idname = "MyTreeType" bl_label = "My Tree Type" bl_icon = "NONE" class MyNodeType(bpy.types.Node): bl_idname = "MyNodeType" bl_label = "My Node Type" object: PointerProperty(type=bpy.types.Object) def draw_buttons(self, context, layout): layout.prop(self, "object", text="") bpy.utils.register_class(MyTreeType) bpy.utils.register_class(MyNodeType) tree = bpy.data.node_groups.new("Test Tree", "MyTreeType") tree.nodes.new("MyNodeType") bpy.context.area.type = "NODE_EDITOR" bpy.context.space_data.tree_type = "MyTreeType" bpy.context.space_data.node_tree = tree ``` **Initial investigation** This is caused by the fact that `nodeCopyNode` passes `LIB_ID_CREATE_NO_USER_REFCOUNT` into `BKE_node_copy_ex`. I suspect this is the case because the node should store a "weak" pointer to the node tree. However, the same flag is used to copy the idprops, resulting in wrong behavior. Are there cases when `node->id` should not be a weak pointer? If not, then we should just get rid of ``` if ((flag & LIB_ID_CREATE_NO_USER_REFCOUNT) == 0) { id_us_plus(node_dst->id); } ``` in `BKE_node_copy_ex` and don't use the flag at all. Another related issue is that when the scene is unloaded, the object referenced by the idprop is freed, before the node is freed. So, when the node is freed, it tries to decrement the user count of the object -> crash. Not sure what's the best way to fix this. Usually the reference to the object should be removed from the idprop, when the object is removed, right?

Jacques Lucke commented

2019-02-13 12:36:02 +01:00

Added subscriber: @JacquesLucke

Jacques Lucke commented

2019-02-13 12:37:25 +01:00

Added subscriber: @mont29

Jacques Lucke commented

2019-02-13 12:37:26 +01:00

@mont29, I can also fix this, but could you give me a hint on what behavior is expected?

Bastien Montagne self-assigned this 2019-02-13 14:38:43 +01:00

Bastien Montagne commented

2019-02-13 14:38:43 +01:00

@JacquesLucke Will do this, Node code is a real nightmare when it comes to ID management, with all its weird specific types of copying processes… :(

blender-admin commented

2019-02-13 14:58:38 +01:00

This issue was referenced by c6e3a20ab6

This issue was referenced by c6e3a20ab60ba8fb56886913a5c7a4269d45b4c3

Bastien Montagne commented

2019-02-13 15:02:40 +01:00

Changed status from 'Open' to: 'Resolved'

Bastien Montagne closed this issue

2019-02-13 15:02:40 +01:00

Jacques Lucke commented

2019-02-13 15:29:48 +01:00

Thanks for the quick fix. Unfortunately, there is still the freeing issue.
Not sure how related this is.

To reproduce:

Run the script from above.
Select the camera in the the node.
File -> New -> General.

Thanks for the quick fix. Unfortunately, there is still the freeing issue. Not sure how related this is. To reproduce: 1. Run the script from above. 2. Select the camera in the the node. 3. `File -> New -> General`.

Bastien Montagne commented

2019-02-13 15:31:12 +01:00

Eeeeeeh! What do we say about reporting several issues in same report? xD

Jacques Lucke commented

2019-02-13 15:35:31 +01:00

right, w8.

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Wrong user counting with ID properties in pynodes #61506