Ungrouping destroys NodeCustomGroups #74665

New Issue

Maxim Vasiliev · 2020-03-11T20:58:56+01:00

Maxim Vasiliev commented

2020-03-11 20:58:56 +01:00

System Information
Operating system: Linux-5.0.0-38-generic-x86_64-with-debian-buster-sid 64 Bits
Graphics card: GeForce GT 730/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 418.56

Blender Version
Broken: version: 2.83 (sub 6), branch: master, commit date: 2020-03-05 21:23, hash: eed1beff88
Worked: (optional)

Short description of error
When a group is ungrouped, user counters are not incremented for contained customgroups.
After deleting a copy of such nodes (eiter in outer tree or remaining in group) the other copy is left with 0-counted node tree and eventually get damaged

Exact steps for others to reproduce the error

load attached file and allow executing embedded script -- the script will registed a CompositorNodeCustomGroup with dumb nodetree and sockets for more dramatic effect
call operator 'add test node' and add the node to compositor
group the node
duplicate the group
ungroup one of the groups
check data view -- 'foo.tree' should have 2 users (the remaining group and compositor nodetree), but it has only 1

nodetest_ungrouping.blend

**System Information** Operating system: Linux-5.0.0-38-generic-x86_64-with-debian-buster-sid 64 Bits Graphics card: GeForce GT 730/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 418.56 **Blender Version** Broken: version: 2.83 (sub 6), branch: master, commit date: 2020-03-05 21:23, hash: `eed1beff88` Worked: (optional) **Short description of error** When a group is ungrouped, user counters are not incremented for contained customgroups. After deleting a copy of such nodes (eiter in outer tree or remaining in group) the other copy is left with 0-counted node tree and eventually get damaged **Exact steps for others to reproduce the error** 1. load attached file and allow executing embedded script -- the script will registed a CompositorNodeCustomGroup with dumb nodetree and sockets for more dramatic effect 2. call operator 'add test node' and add the node to compositor 3. group the node 4. duplicate the group 5. ungroup one of the groups 6. check data view -- 'foo.tree' should have 2 users (the remaining group and compositor nodetree), but it has only 1 [nodetest_ungrouping.blend](https://archive.blender.org/developer/F8401158/nodetest_ungrouping.blend)

Maxim Vasiliev commented

2020-03-11 20:58:56 +01:00

Added subscriber: @qwiglydee

Germano Cavalcante commented

2020-03-12 13:53:53 +01:00

Added subscriber: @mano-wii

Germano Cavalcante commented

2020-03-12 13:53:53 +01:00

Changed status from 'Needs Triage' to: 'Needs User Info'

Germano Cavalcante commented

2020-03-12 13:53:53 +01:00

I didn't understand what you meant in the point 7. "open another group and delete node 'foo' there".
Is there really a need for custom nodes and script execution?
It is good to simplify a report to the minimum steps to save time either in the reproduction of the bug or in the investigation.

A guideline for making a good bug report can be found at https://wiki.blender.org/wiki/Process/Bug_Reports

I didn't understand what you meant in the point 7. `"open another group and delete node 'foo' there".` Is there really a need for custom nodes and script execution? It is good to simplify a report to the minimum steps to save time either in the reproduction of the bug or in the investigation. A guideline for making a good bug report can be found at https://wiki.blender.org/wiki/Process/Bug_Reports

Maxim Vasiliev commented

2020-03-12 18:01:29 +01:00

In #74665#889869, @mano-wii wrote:
I didn't understand what you meant in the point 7. "open another group and delete node 'foo' there".
Is there really a need for custom nodes and script execution?

NodeCustomGroup is needed because it contains reference to NodeTree which should be usercounted.

However, I've just tested, the similar happens with ImageTexture which also reference to an ID, but via property.

It is good to simplify a report to the minimum steps to save time either in the reproduction of the bug or in the investigation.

Minimal steps would be just 1-6 that result in improper user counter, but they do not result in any destructive effects.

> In #74665#889869, @mano-wii wrote: > I didn't understand what you meant in the point 7. `"open another group and delete node 'foo' there".` > Is there really a need for custom nodes and script execution? NodeCustomGroup is needed because it contains reference to NodeTree which should be usercounted. However, I've just tested, the similar happens with ImageTexture which also reference to an ID, but via property. > It is good to simplify a report to the minimum steps to save time either in the reproduction of the bug or in the investigation. Minimal steps would be just 1-6 that result in improper user counter, but they do not result in any destructive effects.

Germano Cavalcante commented

2020-03-12 18:28:34 +01:00

Changed status from 'Needs User Info' to: 'Confirmed'

Maxim Vasiliev commented

2020-03-12 19:09:13 +01:00

This case seems resolved in D6484

Snippet:
source/blender/editors/space_node/node_group.c:193-203

This case seems resolved in [D6484](https://archive.blender.org/developer/D6484) Snippet: source/blender/editors/space_node/node_group.c:193-203

Maxim Vasiliev commented

2020-03-12 19:42:49 +01:00

This also affects ordinary nested groups.

greate a node
group it and name the group 'inner'
group the group again into 'outer' group
duplicate outer group
ungroup outer group
inner group nodetree now has 1 users counter instead of 2
(deleting either copy of inner group will leave remaining copy with 0 users)
(reloading file or cleaning orphans will destroy content of the inner group)

Should the cases with image texture and nested group be reported as separate bugs?

This also affects ordinary nested groups. 1. greate a node 2. group it and name the group 'inner' 3. group the group again into 'outer' group 4. duplicate outer group 5. ungroup outer group 6. inner group nodetree now has 1 users counter instead of 2 7. (deleting either copy of inner group will leave remaining copy with 0 users) 8. (reloading file or cleaning orphans will destroy content of the inner group) Should the cases with image texture and nested group be reported as separate bugs?

Maxim Vasiliev commented

2020-03-12 20:16:59 +01:00

Apparently, this bug used to be compensated by another bug, adressed in e993667a46

Before that fix, ungrouping operation resulted in double-increased user counters.
In the reported scenarios, stuff from froup got 3 users instead of 2. This helped to avoid destructive effects.
(I checked it for customgroup, nested groups and image texture)

Apparently, this bug used to be compensated by another bug, adressed in e993667a46 Before that fix, ungrouping operation resulted in double-increased user counters. In the reported scenarios, stuff from froup got 3 users instead of 2. This helped to avoid destructive effects. (I checked it for customgroup, nested groups and image texture)

Bastien Montagne commented

2020-04-06 16:31:50 +02:00

Added subscriber: @mont29

Bastien Montagne commented

2020-04-06 16:31:50 +02:00

Changed status from 'Confirmed' to: 'Resolved'

Bastien Montagne closed this issue

2020-04-06 16:31:50 +02:00

Bastien Montagne self-assigned this 2020-04-06 16:31:50 +02:00

Bastien Montagne commented

2020-04-06 16:31:50 +02:00

Should be solved by c544df997e.

Should be solved by c544df997e.

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

Ungrouping destroys NodeCustomGroups #74665