From ad2ef731600bb96be1575fa081f07cc0b4408e27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Mon, 8 May 2023 13:09:24 +0200 Subject: [PATCH 1/3] Fix: Viewer node crashes when the editor is pinned and has no Object ID. This case was not properly handled by the previous fix in #107621. The viewer node path function still assumes that the node editor space has a valid ID pointer (`snode.id`) and that it is an `Object`. The ID pointer can be null when setting a pinned node tree through the python API, like so: ``` space.pin = True space.node_tree = some_other_node_tree ``` In this case the `ED_node_tree_start` function is called without and ID pointer, which is allowed! The viewer node path function needs to handle this case. --- source/blender/editors/util/ed_viewer_path.cc | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/source/blender/editors/util/ed_viewer_path.cc b/source/blender/editors/util/ed_viewer_path.cc index 6f67d92681b..5ba673a061e 100644 --- a/source/blender/editors/util/ed_viewer_path.cc +++ b/source/blender/editors/util/ed_viewer_path.cc @@ -26,34 +26,36 @@ static void viewer_path_for_geometry_node(const SpaceNode &snode, { BKE_viewer_path_init(&r_dst); - Object *ob = reinterpret_cast(snode.id); - IDViewerPathElem *id_elem = BKE_viewer_path_elem_new_id(); - id_elem->id = &ob->id; - BLI_addtail(&r_dst.path, id_elem); + if (snode.id != nullptr && GS(snode.id->name) == ID_OB) { + Object *ob = reinterpret_cast(snode.id); + IDViewerPathElem *id_elem = BKE_viewer_path_elem_new_id(); + id_elem->id = &ob->id; + BLI_addtail(&r_dst.path, id_elem); - NodesModifierData *modifier = nullptr; - LISTBASE_FOREACH (ModifierData *, md, &ob->modifiers) { - if (md->type != eModifierType_Nodes) { - continue; + NodesModifierData *modifier = nullptr; + LISTBASE_FOREACH (ModifierData *, md, &ob->modifiers) { + if (md->type != eModifierType_Nodes) { + continue; + } + NodesModifierData *nmd = reinterpret_cast(md); + if (nmd->node_group != snode.nodetree) { + continue; + } + if (snode.flag & SNODE_PIN) { + /* If the node group is pinned, use the first matching modifier. This can be improved by + * storing the modifier name in the node editor when the context is pinned. */ + modifier = nmd; + break; + } + if (md->flag & eModifierFlag_Active) { + modifier = nmd; + } } - NodesModifierData *nmd = reinterpret_cast(md); - if (nmd->node_group != snode.nodetree) { - continue; + if (modifier != nullptr) { + ModifierViewerPathElem *modifier_elem = BKE_viewer_path_elem_new_modifier(); + modifier_elem->modifier_name = BLI_strdup(modifier->modifier.name); + BLI_addtail(&r_dst.path, modifier_elem); } - if (snode.flag & SNODE_PIN) { - /* If the node group is pinned, use the first matching modifier. This can be improved by - * storing the modifier name in the node editor when the context is pinned. */ - modifier = nmd; - break; - } - if (md->flag & eModifierFlag_Active) { - modifier = nmd; - } - } - if (modifier != nullptr) { - ModifierViewerPathElem *modifier_elem = BKE_viewer_path_elem_new_modifier(); - modifier_elem->modifier_name = BLI_strdup(modifier->modifier.name); - BLI_addtail(&r_dst.path, modifier_elem); } Vector tree_path = snode.treepath; -- 2.30.2 From 6e11fe3da11e12998040c750ec43f06371dd3996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Mon, 8 May 2023 13:50:44 +0200 Subject: [PATCH 2/3] Check node space context object before trying to construct viewer path. --- source/blender/editors/util/ed_viewer_path.cc | 63 ++++++++++--------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/source/blender/editors/util/ed_viewer_path.cc b/source/blender/editors/util/ed_viewer_path.cc index 5ba673a061e..d569ba7011d 100644 --- a/source/blender/editors/util/ed_viewer_path.cc +++ b/source/blender/editors/util/ed_viewer_path.cc @@ -24,38 +24,39 @@ static void viewer_path_for_geometry_node(const SpaceNode &snode, const bNode &node, ViewerPath &r_dst) { + /* Only valid if the node space has a context object. */ + BLI_assert(snode.id != nullptr && GS(snode.id->name) == ID_OB); + BKE_viewer_path_init(&r_dst); - if (snode.id != nullptr && GS(snode.id->name) == ID_OB) { - Object *ob = reinterpret_cast(snode.id); - IDViewerPathElem *id_elem = BKE_viewer_path_elem_new_id(); - id_elem->id = &ob->id; - BLI_addtail(&r_dst.path, id_elem); + Object *ob = reinterpret_cast(snode.id); + IDViewerPathElem *id_elem = BKE_viewer_path_elem_new_id(); + id_elem->id = &ob->id; + BLI_addtail(&r_dst.path, id_elem); - NodesModifierData *modifier = nullptr; - LISTBASE_FOREACH (ModifierData *, md, &ob->modifiers) { - if (md->type != eModifierType_Nodes) { - continue; - } - NodesModifierData *nmd = reinterpret_cast(md); - if (nmd->node_group != snode.nodetree) { - continue; - } - if (snode.flag & SNODE_PIN) { - /* If the node group is pinned, use the first matching modifier. This can be improved by - * storing the modifier name in the node editor when the context is pinned. */ - modifier = nmd; - break; - } - if (md->flag & eModifierFlag_Active) { - modifier = nmd; - } + NodesModifierData *modifier = nullptr; + LISTBASE_FOREACH (ModifierData *, md, &ob->modifiers) { + if (md->type != eModifierType_Nodes) { + continue; } - if (modifier != nullptr) { - ModifierViewerPathElem *modifier_elem = BKE_viewer_path_elem_new_modifier(); - modifier_elem->modifier_name = BLI_strdup(modifier->modifier.name); - BLI_addtail(&r_dst.path, modifier_elem); + NodesModifierData *nmd = reinterpret_cast(md); + if (nmd->node_group != snode.nodetree) { + continue; } + if (snode.flag & SNODE_PIN) { + /* If the node group is pinned, use the first matching modifier. This can be improved by + * storing the modifier name in the node editor when the context is pinned. */ + modifier = nmd; + break; + } + if (md->flag & eModifierFlag_Active) { + modifier = nmd; + } + } + if (modifier != nullptr) { + ModifierViewerPathElem *modifier_elem = BKE_viewer_path_elem_new_modifier(); + modifier_elem->modifier_name = BLI_strdup(modifier->modifier.name); + BLI_addtail(&r_dst.path, modifier_elem); } Vector tree_path = snode.treepath; @@ -89,7 +90,9 @@ void activate_geometry_node(Main &bmain, SpaceNode &snode, bNode &node) } ViewerPath new_viewer_path{}; BLI_SCOPED_DEFER([&]() { BKE_viewer_path_clear(&new_viewer_path); }); - viewer_path_for_geometry_node(snode, node, new_viewer_path); + if (snode.id != nullptr && GS(snode.id->name) == ID_OB) { + viewer_path_for_geometry_node(snode, node, new_viewer_path); + } bool found_view3d_with_enabled_viewer = false; View3D *any_view3d_without_viewer = nullptr; @@ -356,7 +359,9 @@ bNode *find_geometry_nodes_viewer(const ViewerPath &viewer_path, SpaceNode &snod } ViewerPath tmp_viewer_path; BLI_SCOPED_DEFER([&]() { BKE_viewer_path_clear(&tmp_viewer_path); }); - viewer_path_for_geometry_node(snode, *possible_viewer, tmp_viewer_path); + if (snode.id != nullptr && GS(snode.id->name) == ID_OB) { + viewer_path_for_geometry_node(snode, *possible_viewer, tmp_viewer_path); + } if (BKE_viewer_path_equal(&viewer_path, &tmp_viewer_path)) { return possible_viewer; -- 2.30.2 From 5df7ba1ba24051f4d6ccd31e370db4b70eefc169 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Mon, 8 May 2023 14:25:47 +0200 Subject: [PATCH 3/3] Early exit from viewer search if the node space context has no object. --- source/blender/editors/util/ed_viewer_path.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/source/blender/editors/util/ed_viewer_path.cc b/source/blender/editors/util/ed_viewer_path.cc index d569ba7011d..8586af8c29c 100644 --- a/source/blender/editors/util/ed_viewer_path.cc +++ b/source/blender/editors/util/ed_viewer_path.cc @@ -346,6 +346,11 @@ bool is_active_geometry_nodes_viewer(const bContext &C, bNode *find_geometry_nodes_viewer(const ViewerPath &viewer_path, SpaceNode &snode) { + /* Viewer path is only valid if the context object is set. */ + if (snode.id == nullptr || GS(snode.id->name) != ID_OB) { + return nullptr; + } + const std::optional parsed_viewer_path = parse_geometry_nodes_viewer(viewer_path); if (!parsed_viewer_path.has_value()) { @@ -359,9 +364,7 @@ bNode *find_geometry_nodes_viewer(const ViewerPath &viewer_path, SpaceNode &snod } ViewerPath tmp_viewer_path; BLI_SCOPED_DEFER([&]() { BKE_viewer_path_clear(&tmp_viewer_path); }); - if (snode.id != nullptr && GS(snode.id->name) == ID_OB) { - viewer_path_for_geometry_node(snode, *possible_viewer, tmp_viewer_path); - } + viewer_path_for_geometry_node(snode, *possible_viewer, tmp_viewer_path); if (BKE_viewer_path_equal(&viewer_path, &tmp_viewer_path)) { return possible_viewer; -- 2.30.2