Fix: Nodes: missing node tree centering

When a new node tree becomes active based on the context, the node editor was
not centered on the new tree. This can easily lead to the situation where there
is no node visible, and the user first has to search for the nodes.

The reason for this is unexpectedly special:
* `snode_set_context` calls `ED_node_tree_start` which adds the `NC_SCENE |
  ND_NODES` notifier.
* Typically, this would update the `View2D` of the region in
  `node_area_listener`.
* However, `snode_set_context` is called from
  `wm_event_do_refresh_wm_and_depsgraph` which happens after(!) the listeners
  run. Therefore, the node editor is redrawn before the listener is handled.
* During redraw, the stored view center is overridden. When it is later used in
  the listener, the value is lost already.

This patch solves this by updating the view center eagerly when opening changing
what node tree is visible, instead of trying to it lazily where the required
information might be lost already.

Pull Request: https://projects.blender.org/blender/blender/pulls/138389

source/blender/editors/include/ED_node_c.hh

@@ -47,9 +47,9 @@ int ED_node_tree_path_length(SpaceNode *snode);
  */
 void ED_node_tree_path_get(SpaceNode *snode, char *value);
 
-void ED_node_tree_start(SpaceNode *snode, bNodeTree *ntree, ID *id, ID *from);
-void ED_node_tree_push(SpaceNode *snode, bNodeTree *ntree, bNode *gnode);
-void ED_node_tree_pop(SpaceNode *snode);
+void ED_node_tree_start(ARegion *region, SpaceNode *snode, bNodeTree *ntree, ID *id, ID *from);
+void ED_node_tree_push(ARegion *region, SpaceNode *snode, bNodeTree *ntree, bNode *gnode);
+void ED_node_tree_pop(ARegion *region, SpaceNode *snode);
 int ED_node_tree_depth(SpaceNode *snode);
 bNodeTree *ED_node_tree_get(SpaceNode *snode, int level);
 

source/blender/editors/space_node/node_edit.cc

@@ -31,6 +31,7 @@
 #include "BKE_report.hh"
 #include "BKE_scene.hh"
 #include "BKE_scene_runtime.hh"
+#include "BKE_screen.hh"
 
 #include "BLI_listbase.h"
 #include "BLI_math_vector.h"
@@ -737,7 +738,9 @@ void snode_set_context(const bContext &C)
   if (snode->nodetree != ntree || snode->id != id || snode->from != from ||
       (snode->treepath.last == nullptr && ntree))
   {
-    ED_node_tree_start(snode, ntree, id, from);
+    ScrArea *area = CTX_wm_area(&C);
+    ARegion *region = BKE_area_find_region_type(area, RGN_TYPE_WINDOW);
+    ED_node_tree_start(region, snode, ntree, id, from);
   }
 }
 

source/blender/editors/space_node/node_group.cc

@@ -172,6 +172,7 @@ static void remap_pairing(bNodeTree &dst_tree,
 static wmOperatorStatus node_group_edit_exec(bContext *C, wmOperator *op)
 {
   SpaceNode *snode = CTX_wm_space_node(C);
+  ARegion *region = CTX_wm_region(C);
   const StringRef node_idname = node_group_idname(C);
   const bool exit = RNA_boolean_get(op->ptr, "exit");
 
@@ -183,11 +184,11 @@ static wmOperatorStatus node_group_edit_exec(bContext *C, wmOperator *op)
     bNodeTree *ngroup = (bNodeTree *)gnode->id;
 
     if (ngroup) {
-      ED_node_tree_push(snode, ngroup, gnode);
+      ED_node_tree_push(region, snode, ngroup, gnode);
     }
   }
   else {
-    ED_node_tree_pop(snode);
+    ED_node_tree_pop(region, snode);
   }
 
   WM_event_add_notifier(C, NC_SCENE | ND_NODES, nullptr);
@@ -632,6 +633,7 @@ static const EnumPropertyItem node_group_separate_types[] = {
 static wmOperatorStatus node_group_separate_exec(bContext *C, wmOperator *op)
 {
   Main *bmain = CTX_data_main(C);
+  ARegion *region = CTX_wm_region(C);
   SpaceNode *snode = CTX_wm_space_node(C);
   int type = RNA_enum_get(op->ptr, "type");
 
@@ -663,7 +665,7 @@ static wmOperatorStatus node_group_separate_exec(bContext *C, wmOperator *op)
   }
 
   /* switch to parent tree */
-  ED_node_tree_pop(snode);
+  ED_node_tree_pop(region, snode);
 
   BKE_main_ensure_invariants(*CTX_data_main(C));
 
@@ -1234,6 +1236,7 @@ static bNode *node_group_make_from_nodes(const bContext &C,
 
 static wmOperatorStatus node_group_make_exec(bContext *C, wmOperator *op)
 {
+  ARegion &region = *CTX_wm_region(C);
   SpaceNode &snode = *CTX_wm_space_node(C);
   bNodeTree &ntree = *snode.edittree;
   const StringRef ntree_idname = group_ntree_idname(C);
@@ -1254,7 +1257,7 @@ static wmOperatorStatus node_group_make_exec(bContext *C, wmOperator *op)
 
     bke::node_set_active(ntree, *gnode);
     if (ngroup) {
-      ED_node_tree_push(&snode, ngroup, gnode);
+      ED_node_tree_push(&region, &snode, ngroup, gnode);
     }
   }
 
@@ -1290,6 +1293,7 @@ void NODE_OT_group_make(wmOperatorType *ot)
 static wmOperatorStatus node_group_insert_exec(bContext *C, wmOperator *op)
 {
   SpaceNode *snode = CTX_wm_space_node(C);
+  ARegion *region = CTX_wm_region(C);
   bNodeTree *ntree = snode->edittree;
   const StringRef node_idname = node_group_idname(C);
 
@@ -1322,7 +1326,7 @@ static wmOperatorStatus node_group_insert_exec(bContext *C, wmOperator *op)
   node_group_make_insert_selected(*C, *ntree, gnode, nodes_to_group);
 
   bke::node_set_active(*ntree, *gnode);
-  ED_node_tree_push(snode, ngroup, gnode);
+  ED_node_tree_push(region, snode, ngroup, gnode);
 
   return OPERATOR_FINISHED;
 }

source/blender/editors/space_node/space_node.cc

@@ -73,7 +73,7 @@ using blender::float2;
 
 /* ******************** tree path ********************* */
 
-void ED_node_tree_start(SpaceNode *snode, bNodeTree *ntree, ID *id, ID *from)
+void ED_node_tree_start(ARegion *region, SpaceNode *snode, bNodeTree *ntree, ID *id, ID *from)
 {
   LISTBASE_FOREACH_MUTABLE (bNodeTreePath *, path, &snode->treepath) {
     MEM_freeN(path);
@@ -85,8 +85,11 @@ void ED_node_tree_start(SpaceNode *snode, bNodeTree *ntree, ID *id, ID *from)
     path->nodetree = ntree;
     path->parent_key = blender::bke::NODE_INSTANCE_KEY_BASE;
 
-    /* copy initial offset from bNodeTree */
+    /* Set initial view center from node tree. */
     copy_v2_v2(path->view_center, ntree->view_center);
+    if (region) {
+      UI_view2d_center_set(&region->v2d, ntree->view_center[0], ntree->view_center[1]);
+    }
 
     if (id) {
       STRNCPY(path->display_name, id->name + 2);
@@ -112,7 +115,7 @@ void ED_node_tree_start(SpaceNode *snode, bNodeTree *ntree, ID *id, ID *from)
   WM_main_add_notifier(NC_SCENE | ND_NODES, nullptr);
 }
 
-void ED_node_tree_push(SpaceNode *snode, bNodeTree *ntree, bNode *gnode)
+void ED_node_tree_push(ARegion *region, SpaceNode *snode, bNodeTree *ntree, bNode *gnode)
 {
   bNodeTreePath *path = MEM_callocN<bNodeTreePath>("node tree path");
   bNodeTreePath *prev_path = (bNodeTreePath *)snode->treepath.last;
@@ -133,8 +136,11 @@ void ED_node_tree_push(SpaceNode *snode, bNodeTree *ntree, bNode *gnode)
     path->parent_key = blender::bke::NODE_INSTANCE_KEY_BASE;
   }
 
-  /* copy initial offset from bNodeTree */
+  /* Set initial view center from node tree. */
   copy_v2_v2(path->view_center, ntree->view_center);
+  if (region) {
+    UI_view2d_center_set(&region->v2d, ntree->view_center[0], ntree->view_center[1]);
+  }
 
   BLI_addtail(&snode->treepath, path);
 
@@ -148,7 +154,7 @@ void ED_node_tree_push(SpaceNode *snode, bNodeTree *ntree, bNode *gnode)
   WM_main_add_notifier(NC_SCENE | ND_NODES, nullptr);
 }
 
-void ED_node_tree_pop(SpaceNode *snode)
+void ED_node_tree_pop(ARegion *region, SpaceNode *snode)
 {
   bNodeTreePath *path = (bNodeTreePath *)snode->treepath.last;
 
@@ -164,9 +170,13 @@ void ED_node_tree_pop(SpaceNode *snode)
   path = (bNodeTreePath *)snode->treepath.last;
   snode->edittree = path->nodetree;
 
+  /* Set view center from node tree path. */
+  if (region) {
+    UI_view2d_center_set(&region->v2d, path->view_center[0], path->view_center[1]);
+  }
+
   ED_node_set_active_viewer_key(snode);
 
-  /* listener updates the View2D center from edittree */
   WM_main_add_notifier(NC_SCENE | ND_NODES, nullptr);
 }
 
@@ -777,13 +787,6 @@ static void node_area_listener(const wmSpaceTypeListenerParams *params)
     case NC_SCENE:
       switch (wmn->data) {
         case ND_NODES: {
-          ARegion *region = BKE_area_find_region_type(area, RGN_TYPE_WINDOW);
-          bNodeTreePath *path = (bNodeTreePath *)snode->treepath.last;
-          /* shift view to node tree center */
-          if (region && path) {
-            UI_view2d_center_set(&region->v2d, path->view_center[0], path->view_center[1]);
-          }
-
           node_area_tag_tree_recalc(snode, area);
           break;
         }

source/blender/makesrna/intern/rna_space.cc

@@ -2635,8 +2635,11 @@ static void rna_SpaceNodeEditor_node_tree_set(PointerRNA *ptr,
                                               const PointerRNA value,
                                               ReportList * /*reports*/)
 {
-  SpaceNode *snode = (SpaceNode *)ptr->data;
-  ED_node_tree_start(snode, (bNodeTree *)value.data, nullptr, nullptr);
+  SpaceNode *snode = ptr->data_as<SpaceNode>();
+  ScrArea *area = BKE_screen_find_area_from_space(reinterpret_cast<const bScreen *>(ptr->owner_id),
+                                                  reinterpret_cast<const SpaceLink *>(snode));
+  ARegion *region = BKE_area_find_region_type(area, RGN_TYPE_WINDOW);
+  ED_node_tree_start(region, snode, (bNodeTree *)value.data, nullptr, nullptr);
 }
 
 static bool rna_SpaceNodeEditor_geometry_nodes_tool_tree_poll(PointerRNA * /*ptr*/,
@@ -2772,13 +2775,29 @@ static int rna_SpaceNodeEditor_path_length(PointerRNA *ptr)
 
 static void rna_SpaceNodeEditor_path_clear(SpaceNode *snode, bContext *C)
 {
-  ED_node_tree_start(snode, nullptr, nullptr, nullptr);
+  ED_node_tree_start(nullptr, snode, nullptr, nullptr, nullptr);
   blender::ed::space_node::tree_update(C);
 }
 
+static ARegion *find_snode_region(SpaceNode *snode, bContext *C)
+{
+  if (wmWindowManager *wm = CTX_wm_manager(C)) {
+    LISTBASE_FOREACH (wmWindow *, win, &wm->windows) {
+      bScreen *screen = WM_window_get_active_screen(win);
+      ScrArea *area = BKE_screen_find_area_from_space(screen,
+                                                      reinterpret_cast<const SpaceLink *>(snode));
+      if (ARegion *region = BKE_area_find_region_type(area, RGN_TYPE_WINDOW)) {
+        return region;
+      }
+    }
+  }
+  return nullptr;
+}
+
 static void rna_SpaceNodeEditor_path_start(SpaceNode *snode, bContext *C, PointerRNA *node_tree)
 {
-  ED_node_tree_start(snode, (bNodeTree *)node_tree->data, nullptr, nullptr);
+  ARegion *region = find_snode_region(snode, C);
+  ED_node_tree_start(region, snode, (bNodeTree *)node_tree->data, nullptr, nullptr);
   blender::ed::space_node::tree_update(C);
 }
 
@@ -2787,14 +2806,16 @@ static void rna_SpaceNodeEditor_path_append(SpaceNode *snode,
                                             PointerRNA *node_tree,
                                             PointerRNA *node)
 {
+  ARegion *region = find_snode_region(snode, C);
   ED_node_tree_push(
-      snode, static_cast<bNodeTree *>(node_tree->data), static_cast<bNode *>(node->data));
+      region, snode, static_cast<bNodeTree *>(node_tree->data), static_cast<bNode *>(node->data));
   blender::ed::space_node::tree_update(C);
 }
 
 static void rna_SpaceNodeEditor_path_pop(SpaceNode *snode, bContext *C)
 {
-  ED_node_tree_pop(snode);
+  ARegion *region = find_snode_region(snode, C);
+  ED_node_tree_pop(region, snode);
   blender::ed::space_node::tree_update(C);
 }