Spreadsheet editor doesn't update when changing active object #90914

New Issue

Dalai Felinto · 2021-08-25T11:13:49+02:00

Dalai Felinto commented

2021-08-25 11:13:49 +02:00

Blender Version
Broken: version: 3.0.0 Alpha, branch: master, commit date: 2021-08-24 18:38, hash: 5ef3afd87c
Worked: -

Short description of error
Spreadsheet editor is not updating when I change the active object if I use Viewer Node and have more than one geometry nodes editor open.

Exact steps for others to reproduce the error

Open:
missing-notifier.blend
Alternate selecting between the plane and the cube.
See how the spreadsheet doesn't update until you mouse hover it.

**Blender Version** Broken: version: 3.0.0 Alpha, branch: master, commit date: 2021-08-24 18:38, hash: `5ef3afd87c` Worked: - **Short description of error** Spreadsheet editor is not updating when I change the active object if I use Viewer Node and have more than one geometry nodes editor open. **Exact steps for others to reproduce the error** * Open: [missing-notifier.blend](https://archive.blender.org/developer/F10324088/missing-notifier.blend) * Alternate selecting between the plane and the cube. * See how the spreadsheet doesn't update until you mouse hover it.

Dalai Felinto commented

2021-08-25 11:13:49 +02:00

Added subscriber: @dfelinto

Dalai Felinto commented

2021-08-25 11:14:13 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Pratik Borhade commented

2021-08-25 12:41:24 +02:00

Added subscriber: @PratikPB2123

Pratik Borhade commented

2021-08-25 12:41:24 +02:00

Hi @dfelinto , I can not use this file with a little old blender versions.

Can say it does not update unless I hover over the Spreadsheet tab but this is something I am not able to reproduce with own created file
#90914.blend

Hi @dfelinto , I can not use this file with a little old blender versions. Can say it does not update unless I hover over the Spreadsheet tab but this is something I am not able to reproduce with own created file [#90914.blend](https://archive.blender.org/developer/F10323984/T90914.blend)

Dalai Felinto commented

2021-08-25 12:52:54 +02:00

Re-saved it uncompressed. As for your file you need to have a second geometry nodes editor for the bug to show.

Philipp Oeser commented

2021-08-26 10:29:08 +02:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2021-08-26 10:29:08 +02:00

Was looking at this a bit (not sure it is a Good First Issue).

Notifiers seem to be OK here. But the Spreadsheet does additional checks to determine if a redraw needs to take place.
Problem is that ED_spreadsheet_context_path_is_active can often return true (this results in the Spreadsheet not being redrawn) when it checks against stuff in Node Editors on screen [most important is snode->nodetree != root_node_tree]. And this in turn is caused by the Node Editors possibly being updated/drawn after the Spreadsheet is drawn (check when snode_set_context is called) -- so the Node Editors might not be "up-to-date" in regards to their current nodetrees. This can even be the case if only one Node Editor is on screen.

Of course we could force-update the Node Editors at the time of checking in the Spreadsheet, but I assume this could be a performance killer:
P2346: T90914_snippet



diff --git a/source/blender/editors/include/ED_node.h b/source/blender/editors/include/ED_node.h
index 1d51a3e77cf..35709432ce5 100644
--- a/source/blender/editors/include/ED_node.h
+++ b/source/blender/editors/include/ED_node.h
@@ -92,7 +92,7 @@ void ED_node_socket_draw(struct bNodeSocket *sock,
                          const struct rcti *rect,
                          const float color[4],
                          float scale);
-void ED_node_tree_update(const struct bContext *C);
+void ED_node_tree_update(const struct bContext *C, struct SpaceNode *snode);
 void ED_node_tag_update_id(struct ID *id);
 void ED_node_tag_update_nodetree(struct Main *bmain, struct bNodeTree *ntree, struct bNode *node);
 void ED_node_sort(struct bNodeTree *ntree);
diff --git a/source/blender/editors/space_node/node_add.cc b/source/blender/editors/space_node/node_add.cc
index 9264c9d3572..eb1b8860636 100644
--- a/source/blender/editors/space_node/node_add.cc
+++ b/source/blender/editors/space_node/node_add.cc
@@ -986,7 +986,7 @@ static int new_node_tree_exec(bContext *C, wmOperator *op)
   else if (snode) {
     snode->nodetree = ntree;
 
-    ED_node_tree_update(C);
+    ED_node_tree_update(C, snode);
   }
 
   return OPERATOR_FINISHED;
diff --git a/source/blender/editors/space_node/node_draw.cc b/source/blender/editors/space_node/node_draw.cc
index 3b1a55f55ab..90e7a4cc264 100644
--- a/source/blender/editors/space_node/node_draw.cc
+++ b/source/blender/editors/space_node/node_draw.cc
@@ -103,11 +103,10 @@ float ED_node_grid_size(void)
   return U.widget_unit;
 }
 
-void ED_node_tree_update(const bContext *C)
+void ED_node_tree_update(const bContext *C, SpaceNode *snode)
 {
-  SpaceNode *snode = CTX_wm_space_node(C);
   if (snode) {
-    snode_set_context(C);
+    snode_set_context(C, snode);
 
     id_us_ensure_real(&snode->nodetree->id);
   }
@@ -2154,7 +2153,7 @@ void node_draw_space(const bContext *C, ARegion *region)
   GPU_blend(GPU_BLEND_ALPHA);
 
   /* Nodes. */
-  snode_set_context(C);
+  snode_set_context(C, snode);
 
   /* Draw parent node trees. */
   if (snode->treepath.last) {
diff --git a/source/blender/editors/space_node/node_edit.cc b/source/blender/editors/space_node/node_edit.cc
index 5b1b737751c..4bf5b937a88 100644
--- a/source/blender/editors/space_node/node_edit.cc
+++ b/source/blender/editors/space_node/node_edit.cc
@@ -603,9 +603,8 @@ void ED_node_texture_default(const bContext *C, Tex *tex)
 }
 
 /* Here we set the active tree(s), even called for each redraw now, so keep it fast :) */
-void snode_set_context(const bContext *C)
+void snode_set_context(const bContext *C, SpaceNode *snode)
 {
-  SpaceNode *snode = CTX_wm_space_node(C);
   bNodeTreeType *treetype = ntreeTypeFind(snode->tree_idname);
   bNodeTree *ntree = snode->nodetree;
   ID *id = snode->id, *from = snode->from;
diff --git a/source/blender/editors/space_node/node_intern.h b/source/blender/editors/space_node/node_intern.h
index df20420e472..323946f9109 100644
--- a/source/blender/editors/space_node/node_intern.h
+++ b/source/blender/editors/space_node/node_intern.h
@@ -251,7 +251,7 @@ void NODE_OT_insert_offset(struct wmOperatorType *ot);
 /* node_edit.c */
 void snode_notify(struct bContext *C, struct SpaceNode *snode);
 void snode_dag_update(struct bContext *C, struct SpaceNode *snode);
-void snode_set_context(const struct bContext *C);
+void snode_set_context(const struct bContext *C, struct SpaceNode *snode);
 
 void snode_update(struct SpaceNode *snode, struct bNode *node);
 bool composite_node_active(struct bContext *C);
diff --git a/source/blender/editors/space_node/space_node.c b/source/blender/editors/space_node/space_node.c
index 956fb3aa867..97556a02b7a 100644
--- a/source/blender/editors/space_node/space_node.c
+++ b/source/blender/editors/space_node/space_node.c
@@ -506,7 +506,7 @@ static void node_area_refresh(const struct bContext *C, ScrArea *area)
   /* default now: refresh node is starting preview */
   SpaceNode *snode = area->spacedata.first;
 
-  snode_set_context(C);
+  snode_set_context(C, snode);
 
   if (snode->nodetree) {
     if (snode->nodetree->type == NTREE_SHADER) {
@@ -782,7 +782,8 @@ static void node_header_region_init(wmWindowManager *UNUSED(wm), ARegion *region
 static void node_header_region_draw(const bContext *C, ARegion *region)
 {
   /* find and set the context */
-  snode_set_context(C);
+  SpaceNode *snode = CTX_wm_space_node(C);
+  snode_set_context(C, snode);
 
   ED_region_header(C, region);
 }
diff --git a/source/blender/editors/space_spreadsheet/spreadsheet_context.cc b/source/blender/editors/space_spreadsheet/spreadsheet_context.cc
index c38e765caee..c9dfae5787d 100644
--- a/source/blender/editors/space_spreadsheet/spreadsheet_context.cc
+++ b/source/blender/editors/space_spreadsheet/spreadsheet_context.cc
@@ -24,6 +24,7 @@
 #include "BLI_utildefines.h"
 #include "BLI_vector.hh"
 
+#include "ED_node.h"
 #include "ED_screen.h"
 #include "ED_spreadsheet.h"
 
@@ -473,6 +474,8 @@ bool ED_spreadsheet_context_path_is_active(const bContext *C, SpaceSpreadsheet *
         continue;
       }
       SpaceNode *snode = (SpaceNode *)sl;
+      /* Make sure the Node Editor is up-to-date. */
+      ED_node_tree_update(C, snode);
       if (snode->nodetree != root_node_tree) {
         continue;
       }
diff --git a/source/blender/makesrna/intern/rna_space.c b/source/blender/makesrna/intern/rna_space.c
index 5ab13e7b44e..7df8cc356d2 100644
--- a/source/blender/makesrna/intern/rna_space.c
+++ b/source/blender/makesrna/intern/rna_space.c
@@ -2342,9 +2342,10 @@ static bool rna_SpaceNodeEditor_node_tree_poll(PointerRNA *ptr, const PointerRNA
   return (STREQ(snode->tree_idname, ntree->idname));
 }
 
-static void rna_SpaceNodeEditor_node_tree_update(const bContext *C, PointerRNA *UNUSED(ptr))
+static void rna_SpaceNodeEditor_node_tree_update(const bContext *C, PointerRNA *ptr)
 {
-  ED_node_tree_update(C);
+  SpaceNode *snode = ptr->data;
+  ED_node_tree_update(C, snode);
 }
 
 static int rna_SpaceNodeEditor_tree_type_get(PointerRNA *ptr)
@@ -2410,13 +2411,13 @@ static int rna_SpaceNodeEditor_path_length(PointerRNA *ptr)
 static void rna_SpaceNodeEditor_path_clear(SpaceNode *snode, bContext *C)
 {
   ED_node_tree_start(snode, NULL, NULL, NULL);
-  ED_node_tree_update(C);
+  ED_node_tree_update(C, snode);
 }
 
 static void rna_SpaceNodeEditor_path_start(SpaceNode *snode, bContext *C, PointerRNA *node_tree)
 {
   ED_node_tree_start(snode, (bNodeTree *)node_tree->data, NULL, NULL);
-  ED_node_tree_update(C);
+  ED_node_tree_update(C, snode);
 }
 
 static void rna_SpaceNodeEditor_path_append(SpaceNode *snode,
@@ -2425,13 +2426,13 @@ static void rna_SpaceNodeEditor_path_append(SpaceNode *snode,
                                             PointerRNA *node)
 {
   ED_node_tree_push(snode, node_tree->data, node->data);
-  ED_node_tree_update(C);
+  ED_node_tree_update(C, snode);
 }
 
 static void rna_SpaceNodeEditor_path_pop(SpaceNode *snode, bContext *C)
 {
   ED_node_tree_pop(snode);
-  ED_node_tree_update(C);
+  ED_node_tree_update(C, snode);
 }
 
 static void rna_SpaceNodeEditor_show_backdrop_update(Main *UNUSED(bmain),

Dont think we can control the order of Editors being drawn (that could be another solution)?

Was looking at this a bit (not sure it is a `Good First Issue`). Notifiers seem to be OK here. But the Spreadsheet does additional checks to determine if a redraw needs to take place. Problem is that `ED_spreadsheet_context_path_is_active` can often return true (this results in the Spreadsheet not being redrawn) when it checks against stuff in Node Editors on screen [most important is `snode->nodetree != root_node_tree`]. And this in turn is caused by the Node Editors possibly being updated/drawn **after** the Spreadsheet is drawn (check when `snode_set_context` is called) -- so the Node Editors might not be "up-to-date" in regards to their current nodetrees. This can even be the case if only one Node Editor is on screen. Of course we could force-update the Node Editors at the time of checking in the Spreadsheet, but I assume this could be a performance killer: [P2346: T90914_snippet](https://archive.blender.org/developer/P2346.txt) ``` diff --git a/source/blender/editors/include/ED_node.h b/source/blender/editors/include/ED_node.h index 1d51a3e77cf..35709432ce5 100644 --- a/source/blender/editors/include/ED_node.h +++ b/source/blender/editors/include/ED_node.h @@ -92,7 +92,7 @@ void ED_node_socket_draw(struct bNodeSocket *sock, const struct rcti *rect, const float color[4], float scale); -void ED_node_tree_update(const struct bContext *C); +void ED_node_tree_update(const struct bContext *C, struct SpaceNode *snode); void ED_node_tag_update_id(struct ID *id); void ED_node_tag_update_nodetree(struct Main *bmain, struct bNodeTree *ntree, struct bNode *node); void ED_node_sort(struct bNodeTree *ntree); diff --git a/source/blender/editors/space_node/node_add.cc b/source/blender/editors/space_node/node_add.cc index 9264c9d3572..eb1b8860636 100644 --- a/source/blender/editors/space_node/node_add.cc +++ b/source/blender/editors/space_node/node_add.cc @@ -986,7 +986,7 @@ static int new_node_tree_exec(bContext *C, wmOperator *op) else if (snode) { snode->nodetree = ntree; - ED_node_tree_update(C); + ED_node_tree_update(C, snode); } return OPERATOR_FINISHED; diff --git a/source/blender/editors/space_node/node_draw.cc b/source/blender/editors/space_node/node_draw.cc index 3b1a55f55ab..90e7a4cc264 100644 --- a/source/blender/editors/space_node/node_draw.cc +++ b/source/blender/editors/space_node/node_draw.cc @@ -103,11 +103,10 @@ float ED_node_grid_size(void) return U.widget_unit; } -void ED_node_tree_update(const bContext *C) +void ED_node_tree_update(const bContext *C, SpaceNode *snode) { - SpaceNode *snode = CTX_wm_space_node(C); if (snode) { - snode_set_context(C); + snode_set_context(C, snode); id_us_ensure_real(&snode->nodetree->id); } @@ -2154,7 +2153,7 @@ void node_draw_space(const bContext *C, ARegion *region) GPU_blend(GPU_BLEND_ALPHA); /* Nodes. */ - snode_set_context(C); + snode_set_context(C, snode); /* Draw parent node trees. */ if (snode->treepath.last) { diff --git a/source/blender/editors/space_node/node_edit.cc b/source/blender/editors/space_node/node_edit.cc index 5b1b737751c..4bf5b937a88 100644 --- a/source/blender/editors/space_node/node_edit.cc +++ b/source/blender/editors/space_node/node_edit.cc @@ -603,9 +603,8 @@ void ED_node_texture_default(const bContext *C, Tex *tex) } /* Here we set the active tree(s), even called for each redraw now, so keep it fast :) */ -void snode_set_context(const bContext *C) +void snode_set_context(const bContext *C, SpaceNode *snode) { - SpaceNode *snode = CTX_wm_space_node(C); bNodeTreeType *treetype = ntreeTypeFind(snode->tree_idname); bNodeTree *ntree = snode->nodetree; ID *id = snode->id, *from = snode->from; diff --git a/source/blender/editors/space_node/node_intern.h b/source/blender/editors/space_node/node_intern.h index df20420e472..323946f9109 100644 --- a/source/blender/editors/space_node/node_intern.h +++ b/source/blender/editors/space_node/node_intern.h @@ -251,7 +251,7 @@ void NODE_OT_insert_offset(struct wmOperatorType *ot); /* node_edit.c */ void snode_notify(struct bContext *C, struct SpaceNode *snode); void snode_dag_update(struct bContext *C, struct SpaceNode *snode); -void snode_set_context(const struct bContext *C); +void snode_set_context(const struct bContext *C, struct SpaceNode *snode); void snode_update(struct SpaceNode *snode, struct bNode *node); bool composite_node_active(struct bContext *C); diff --git a/source/blender/editors/space_node/space_node.c b/source/blender/editors/space_node/space_node.c index 956fb3aa867..97556a02b7a 100644 --- a/source/blender/editors/space_node/space_node.c +++ b/source/blender/editors/space_node/space_node.c @@ -506,7 +506,7 @@ static void node_area_refresh(const struct bContext *C, ScrArea *area) /* default now: refresh node is starting preview */ SpaceNode *snode = area->spacedata.first; - snode_set_context(C); + snode_set_context(C, snode); if (snode->nodetree) { if (snode->nodetree->type == NTREE_SHADER) { @@ -782,7 +782,8 @@ static void node_header_region_init(wmWindowManager *UNUSED(wm), ARegion *region static void node_header_region_draw(const bContext *C, ARegion *region) { /* find and set the context */ - snode_set_context(C); + SpaceNode *snode = CTX_wm_space_node(C); + snode_set_context(C, snode); ED_region_header(C, region); } diff --git a/source/blender/editors/space_spreadsheet/spreadsheet_context.cc b/source/blender/editors/space_spreadsheet/spreadsheet_context.cc index c38e765caee..c9dfae5787d 100644 --- a/source/blender/editors/space_spreadsheet/spreadsheet_context.cc +++ b/source/blender/editors/space_spreadsheet/spreadsheet_context.cc @@ -24,6 +24,7 @@ #include "BLI_utildefines.h" #include "BLI_vector.hh" +#include "ED_node.h" #include "ED_screen.h" #include "ED_spreadsheet.h" @@ -473,6 +474,8 @@ bool ED_spreadsheet_context_path_is_active(const bContext *C, SpaceSpreadsheet * continue; } SpaceNode *snode = (SpaceNode *)sl; + /* Make sure the Node Editor is up-to-date. */ + ED_node_tree_update(C, snode); if (snode->nodetree != root_node_tree) { continue; } diff --git a/source/blender/makesrna/intern/rna_space.c b/source/blender/makesrna/intern/rna_space.c index 5ab13e7b44e..7df8cc356d2 100644 --- a/source/blender/makesrna/intern/rna_space.c +++ b/source/blender/makesrna/intern/rna_space.c @@ -2342,9 +2342,10 @@ static bool rna_SpaceNodeEditor_node_tree_poll(PointerRNA *ptr, const PointerRNA return (STREQ(snode->tree_idname, ntree->idname)); } -static void rna_SpaceNodeEditor_node_tree_update(const bContext *C, PointerRNA *UNUSED(ptr)) +static void rna_SpaceNodeEditor_node_tree_update(const bContext *C, PointerRNA *ptr) { - ED_node_tree_update(C); + SpaceNode *snode = ptr->data; + ED_node_tree_update(C, snode); } static int rna_SpaceNodeEditor_tree_type_get(PointerRNA *ptr) @@ -2410,13 +2411,13 @@ static int rna_SpaceNodeEditor_path_length(PointerRNA *ptr) static void rna_SpaceNodeEditor_path_clear(SpaceNode *snode, bContext *C) { ED_node_tree_start(snode, NULL, NULL, NULL); - ED_node_tree_update(C); + ED_node_tree_update(C, snode); } static void rna_SpaceNodeEditor_path_start(SpaceNode *snode, bContext *C, PointerRNA *node_tree) { ED_node_tree_start(snode, (bNodeTree *)node_tree->data, NULL, NULL); - ED_node_tree_update(C); + ED_node_tree_update(C, snode); } static void rna_SpaceNodeEditor_path_append(SpaceNode *snode, @@ -2425,13 +2426,13 @@ static void rna_SpaceNodeEditor_path_append(SpaceNode *snode, PointerRNA *node) { ED_node_tree_push(snode, node_tree->data, node->data); - ED_node_tree_update(C); + ED_node_tree_update(C, snode); } static void rna_SpaceNodeEditor_path_pop(SpaceNode *snode, bContext *C) { ED_node_tree_pop(snode); - ED_node_tree_update(C); + ED_node_tree_update(C, snode); } static void rna_SpaceNodeEditor_show_backdrop_update(Main *UNUSED(bmain), ``` Dont think we can control the order of Editors being drawn (that could be another solution)?

Philipp Oeser commented

2021-08-26 10:47:53 +02:00

As for your file you need to have a second geometry nodes editor for the bug to show.

Stepping with a Debugger can even show this on a single Node Editor.

> As for your file you need to have a second geometry nodes editor for the bug to show. Stepping with a Debugger can even show this on a single Node Editor.

Himanshi Kalra commented

2021-08-26 11:51:59 +02:00

Added subscriber: @Calra

Jacques Lucke commented

2021-08-26 12:01:28 +02:00

Added subscriber: @JacquesLucke

Jacques Lucke commented

2021-08-26 12:01:28 +02:00

The fundamental problem here is that Blender just has no system to handle these kinds of update chains correctly. While I'm not 100% sure, I think the same fundamental problem is the cause for this thread: https://devtalk.blender.org/t/responding-to-selection-changes/15813.
I'm inclined to call this a known issue.

The fundamental problem here is that Blender just has no system to handle these kinds of update chains correctly. While I'm not 100% sure, I think the same fundamental problem is the cause for this thread: https://devtalk.blender.org/t/responding-to-selection-changes/15813. I'm inclined to call this a known issue.

Jacques Lucke commented

2021-11-03 12:17:22 +01:00

Will change this to a known issue. Here are some more details of why this is a bit tricky:

What is shown on the spreadsheet depends on what is shown in the node editor.
What is shown in the node editor depends on what object is active.
When the active object changes, we would first have to update what is shown in the node editor and then trigger yet another update in the spreadsheet.
Problem is, the dependency should be the other way around, the spreadsheet should "subscribe" to changes in the node editor, instead of the node editor triggering the spreadsheet.

We don't have a real system to handle such update chains correctly as mentioned in the previous post.

Will change this to a known issue. Here are some more details of why this is a bit tricky: * What is shown on the spreadsheet depends on what is shown in the node editor. * What is shown in the node editor depends on what object is active. * When the active object changes, we would first have to update what is shown in the node editor and then trigger yet another update in the spreadsheet. * Problem is, the dependency should be the other way around, the spreadsheet should "subscribe" to changes in the node editor, instead of the node editor triggering the spreadsheet. We don't have a real system to handle such update chains correctly as mentioned in the previous post.

Pratik Borhade commented

2022-12-07 13:53:35 +01:00

Can not reproduce this anymore in 3.4 and following version.
I'll find the resolving commit. Maybe we would be able to backport it

Can not reproduce this anymore in 3.4 and following version. I'll find the resolving commit. Maybe we would be able to backport it

Pratik Borhade commented

2022-12-07 14:27:50 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Pratik Borhade closed this issue

2022-12-07 14:27:50 +01:00

Jacques Lucke was assigned by Pratik Borhade

2022-12-07 14:27:50 +01:00

Pratik Borhade commented

2022-12-07 14:27:50 +01:00

Fixed with c55d38f00b
Definitely we can not backport the commit so I'll just close this ticket :)
If anyone still able to reproduce this issue in latest master, please reopen it :)

Fixed with c55d38f00b Definitely we can not backport the commit so I'll just close this ticket :) If anyone still able to reproduce this issue in latest master, please reopen it :)

Thomas Dinges added this to the 3.3 LTS milestone 2023-02-08 15:37:47 +01:00

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

Spreadsheet editor doesn't update when changing active object #90914