Socket.enabled does not call update methods #89109

New Issue

Sergey · 2021-06-13T12:39:05+02:00

Sergey commented

2021-06-13 12:39:05 +02:00

System Information
Operating system: Windows-10-10.0.19041-SP0 64 Bits
Graphics card: GeForce GTX 660 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 457.09

Blender Version
Broken: version: 2.93.0, branch: master, commit date: 2021-06-02 11:21, hash: 84da05a8b8

Short description of error
Socket.enabled = False make the socket to be sort of hidden with connected link. I assume that the logic here is close to link.is_muted = True. But unlike changes of is_muted attribute changes of enabled attribute does no trigger calling update method of a tree. And it does not change the is_linked status of the socket from opposite side.
So I think there are two bugs here.

Whenever enabled attribute is changed the update method of a tree should be called. This will indicate that topology of the tree was changed.
Sockets with connected hidden links should have is_linked status as False because disabled sockets should not produce or receive any data (according to how I understand the purpose of this functionality).

Exact steps for others to reproduce the error
untitled (2).blend

Open the file
Ran the script
Read a console

**System Information** Operating system: Windows-10-10.0.19041-SP0 64 Bits Graphics card: GeForce GTX 660 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 457.09 **Blender Version** Broken: version: 2.93.0, branch: master, commit date: 2021-06-02 11:21, hash: `84da05a8b8` **Short description of error** `Socket.enabled = False` make the socket to be sort of hidden with connected link. I assume that the logic here is close to `link.is_muted = True`. But unlike changes of `is_muted` attribute changes of `enabled` attribute does no trigger calling update method of a tree. And it does not change the `is_linked` status of the socket from opposite side. So I think there are two bugs here. 1. Whenever `enabled` attribute is changed the update method of a tree should be called. This will indicate that topology of the tree was changed. 2. Sockets with connected hidden links should have `is_linked` status as False because disabled sockets should not produce or receive any data (according to how I understand the purpose of this functionality). **Exact steps for others to reproduce the error** [untitled (2).blend](https://archive.blender.org/developer/F10169284/untitled__2_.blend) 1. Open the file 2. Ran the script 3. Read a console

Sergey commented

2021-06-13 12:39:05 +02:00

Added subscriber: @randum

Philipp Oeser commented

2021-06-24 13:22:08 +02:00

Added subscribers: @HooglyBoogly, @JacquesLucke, @lichtwerk

Philipp Oeser commented

2021-06-24 13:22:08 +02:00

Changed status from 'Needs Triage' to: 'Needs Developer To Reproduce'

Philipp Oeser commented

2021-06-24 13:22:08 +02:00

I would agree here.

Going briefly over usages of SOCK_UNAVAIL this really seems to alter the tree (and just firing the NC_NODE | ND_DISPLAY notifier is not enough).
We could add something like the following to get the tree update to be called:

P2209: #89109 snippet



diff --git a/source/blender/makesrna/intern/rna_nodetree.c b/source/blender/makesrna/intern/rna_nodetree.c
index c26e9e883d6..7f54d97dd7f 100644
--- a/source/blender/makesrna/intern/rna_nodetree.c
+++ b/source/blender/makesrna/intern/rna_nodetree.c
@@ -2844,16 +2844,29 @@ static void rna_NodeSocket_type_set(PointerRNA *ptr, int value)
   nodeModifySocketType(ntree, node, sock, value, 0);
 }
 
-static void rna_NodeSocket_update(Main *bmain, Scene *UNUSED(scene), PointerRNA *ptr)
+static void rna_NodeSocket_update_ex(Main *bmain, PointerRNA *ptr, bool do_update_tree)
 {
   bNodeTree *ntree = (bNodeTree *)ptr->owner_id;
   bNodeSocket *sock = (bNodeSocket *)ptr->data;
   bNode *node;
   if (nodeFindNode(ntree, sock, &node, NULL)) {
     ED_node_tag_update_nodetree(bmain, ntree, node);
+    if (do_update_tree) {
+      ntreeUpdateTree(bmain, ntree);
+    }
   }
 }
 
+static void rna_NodeSocket_update(Main *bmain, Scene *UNUSED(scene), PointerRNA *ptr)
+{
+  rna_NodeSocket_update_ex(bmain, ptr, false);
+}
+
+static void rna_NodeSocket_tree_update(Main *bmain, Scene *UNUSED(scene), PointerRNA *ptr)
+{
+  rna_NodeSocket_update_ex(bmain, ptr, true);
+}
+
 static bool rna_NodeSocket_is_output_get(PointerRNA *ptr)
 {
   bNodeSocket *sock = ptr->data;
@@ -10100,7 +10113,7 @@ static void rna_def_node_socket(BlenderRNA *brna)
   prop = RNA_def_property(srna, "enabled", PROP_BOOLEAN, PROP_NONE);
   RNA_def_property_boolean_negative_sdna(prop, NULL, "flag", SOCK_UNAVAIL);
   RNA_def_property_ui_text(prop, "Enabled", "Enable the socket");
-  RNA_def_property_update(prop, NC_NODE | ND_DISPLAY, NULL);
+  RNA_def_property_update(prop, NC_NODE | NA_EDITED, "rna_NodeSocket_tree_update");
 
   prop = RNA_def_property(srna, "link_limit", PROP_INT, PROP_NONE);
   RNA_def_property_int_sdna(prop, NULL, "limit");

On the is_linked issue, I would tend to agree here as well (just seems to make more sense).

Not sure these two would be classified as bugs, maybe @JacquesLucke or @HooglyBoogly can comment?

I would agree here. Going briefly over usages of `SOCK_UNAVAIL` this really seems to alter the tree (and just firing the `NC_NODE | ND_DISPLAY` notifier is not enough). We could add something like the following to get the tree update to be called: [P2209: #89109 snippet](https://archive.blender.org/developer/P2209.txt) ``` diff --git a/source/blender/makesrna/intern/rna_nodetree.c b/source/blender/makesrna/intern/rna_nodetree.c index c26e9e883d6..7f54d97dd7f 100644 --- a/source/blender/makesrna/intern/rna_nodetree.c +++ b/source/blender/makesrna/intern/rna_nodetree.c @@ -2844,16 +2844,29 @@ static void rna_NodeSocket_type_set(PointerRNA *ptr, int value) nodeModifySocketType(ntree, node, sock, value, 0); } -static void rna_NodeSocket_update(Main *bmain, Scene *UNUSED(scene), PointerRNA *ptr) +static void rna_NodeSocket_update_ex(Main *bmain, PointerRNA *ptr, bool do_update_tree) { bNodeTree *ntree = (bNodeTree *)ptr->owner_id; bNodeSocket *sock = (bNodeSocket *)ptr->data; bNode *node; if (nodeFindNode(ntree, sock, &node, NULL)) { ED_node_tag_update_nodetree(bmain, ntree, node); + if (do_update_tree) { + ntreeUpdateTree(bmain, ntree); + } } } +static void rna_NodeSocket_update(Main *bmain, Scene *UNUSED(scene), PointerRNA *ptr) +{ + rna_NodeSocket_update_ex(bmain, ptr, false); +} + +static void rna_NodeSocket_tree_update(Main *bmain, Scene *UNUSED(scene), PointerRNA *ptr) +{ + rna_NodeSocket_update_ex(bmain, ptr, true); +} + static bool rna_NodeSocket_is_output_get(PointerRNA *ptr) { bNodeSocket *sock = ptr->data; @@ -10100,7 +10113,7 @@ static void rna_def_node_socket(BlenderRNA *brna) prop = RNA_def_property(srna, "enabled", PROP_BOOLEAN, PROP_NONE); RNA_def_property_boolean_negative_sdna(prop, NULL, "flag", SOCK_UNAVAIL); RNA_def_property_ui_text(prop, "Enabled", "Enable the socket"); - RNA_def_property_update(prop, NC_NODE | ND_DISPLAY, NULL); + RNA_def_property_update(prop, NC_NODE | NA_EDITED, "rna_NodeSocket_tree_update"); prop = RNA_def_property(srna, "link_limit", PROP_INT, PROP_NONE); RNA_def_property_int_sdna(prop, NULL, "limit"); ``` On the `is_linked` issue, I would tend to agree here as well (just seems to make more sense). Not sure these two would be classified as bugs, maybe @JacquesLucke or @HooglyBoogly can comment?

Brecht Van Lommel commented

2021-06-25 09:35:56 +02:00

Added subscriber: @brecht

Brecht Van Lommel commented

2021-06-25 09:35:56 +02:00

Modifying is_linked would be a Python API breaking change. I don't think it should be changed, the link still exists and is in the ntree.links list. If that link should be used for evaluation or not is another matter.

Modifying `is_linked` would be a Python API breaking change. I don't think it should be changed, the link still exists and is in the `ntree.links` list. If that link should be used for evaluation or not is another matter.

Sergey commented

2021-06-25 09:49:58 +02:00

@brecht What about muted links? They also still exist and are in the tree.links but sockets connected to such links have is_linked status as False.

{F10201836, size=full}

@brecht What about muted links? They also still exist and are in the `tree.links` but sockets connected to such links have `is_linked` status as False. {[F10201836](https://archive.blender.org/developer/F10201836/2021-06-25_11-44-09.png), size=full}

Brecht Van Lommel commented

2021-06-25 15:33:12 +02:00

Ok, if muting does it already it's fine for it to be affected by this as well.

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Socket.enabled does not call update methods #89109