Page MenuHome

Cloning materials with too smart ShaderNodeCustomGroup breaks user counters of both source and target nodes
Confirmed, NormalPublicBUG

Description

System Information
Operating system: Linux-5.0.0-37-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.82 (sub 4), branch: master, commit date: 2019-12-08 18:49, hash: rB761111efb8e4
Worked: (optional)

Short description of error
Some of my shader nodes need to have separate node trees for each their copies.

It seems quite legal to do like this:

def copy(self, node):
    self.node_tree = bpy.data.node_groups.new('newtree', 'ShaderNodeTree')

When the material is cloned (via ops.material.new), it makes the original tree.users == 0, and new tree.users == 2
Source node becomes zombie and dies after reloading or orphan cleaning.
Target node becomes undead and cannot die because of zero users.

Exact steps for others to reproduce the error

  1. run attached script from command line: blender --factory-startup --background --python test_op_mat_clone.py
  2. observe output: all trees are expected to have 1 users

Event Timeline

This comment was removed by Maxim Vasiliev (qmax).
Maxim Vasiliev (qmax) renamed this task from Copying materials with custom nodes messes up users counters and may produce bogus orphaned and overused trees to Copying materials (or single-usering node groups) with custom nodes messes up users counters and may produce bogus orphaned and overused trees.Dec 10 2019, 9:19 AM
This comment was removed by Maxim Vasiliev (qmax).
Maxim Vasiliev (qmax) renamed this task from Copying materials (or single-usering node groups) with custom nodes messes up users counters and may produce bogus orphaned and overused trees to Cloning materials with too smart ShaderNodeCustomGroup breaks user counters of both source and target nodes.Dec 19 2019, 2:37 AM
Maxim Vasiliev (qmax) updated the task description. (Show Details)
Jacques Lucke (JacquesLucke) changed the task status from Needs Triage to Needs Information from User.Sun, Dec 29, 4:43 PM

Please redo your tests and properly store a reference to a node tree in a node as described in https://developer.blender.org/T72579#839420.

Dalai Felinto (dfelinto) claimed this task.

No activity for more than a week. As per the tracker policy we assume the issue is gone and can be closed.
Thanks again for the report. If the problem persists please open a new report with the required information.

What is the policy, @Dalai Felinto (dfelinto) ?

Additional information is provided in referenced tasks.
The bug exists and it breakes and will break some shader add-ons.

Investigation is conducted, root cause of this and many other existing and potential bugs is discovered, fix is submitted, along with autotests.
What is a (more) proper way to help making blender better?

Do you have some kind of "fsck off, patches are not welcome" policy?

Robert Guetzkow (rjg) reopened this task as Needs Triage.EditedTue, Jan 14, 11:35 AM

@Maxim Vasiliev (qmax) this is done more or less automatically for all open tickets that have the status Needs Information from User after a week without any activity. This is documented in the Triaging Playbook - No reply after a week. I guess this was unintentionally closed without checking the patches/progress and certainly your contributions are welcome.

I'll reopen the task.

Oh. Ok.

Then I'll put retated information here to avoid any non-automated confusion.

According to my debugging, the bug occurs because python method Node::copy is called from BKE_node_copy_ex, while refcounter of nested nodetree is in inconsistent state, and reinitializing node_tree inside the method breakes it even further.
The counters are corrected later in code of BKE_id_copy_ex, in the call of BKE_library_foreach_ID_link, after all material data is completely copied.

Proposed fix D6484 moves the call to supposedly more appropriate place.
A test for this case is in D6420.

Philipp Oeser (lichtwerk) changed the task status from Needs Triage to Confirmed.Mon, Jan 20, 11:55 AM
Philipp Oeser (lichtwerk) changed the subtype of this task from "Report" to "Bug".

Can confirm.