From e433a8eaaf7bf72ffda0d9dba2a0e2a83db182b3 Mon Sep 17 00:00:00 2001 From: Falk David Date: Fri, 20 Oct 2023 19:05:31 +0200 Subject: [PATCH] GPv3: Replace find API with `find_node_by_name` Instead of having two APIs (`find_layer_by_name` + `find_layer_group_by_name`) replace them with a single API `find_node_by_name`. This returns a `TreeNode` but the caller can easily check if it's a layer or a group. --- .../blender/blenkernel/BKE_grease_pencil.hh | 12 +--- .../blenkernel/intern/grease_pencil.cc | 62 +++++-------------- .../blenkernel/intern/grease_pencil_test.cc | 10 +-- .../intern/grease_pencil_layers.cc | 8 +-- .../makesdna/DNA_grease_pencil_types.h | 7 +-- 5 files changed, 30 insertions(+), 69 deletions(-) diff --git a/source/blender/blenkernel/BKE_grease_pencil.hh b/source/blender/blenkernel/BKE_grease_pencil.hh index 235481550d5..6a06190e723 100644 --- a/source/blender/blenkernel/BKE_grease_pencil.hh +++ b/source/blender/blenkernel/BKE_grease_pencil.hh @@ -506,16 +506,10 @@ class LayerGroup : public ::GreasePencilLayerTreeGroup { Span groups_for_write(); /** - * Returns a pointer to the layer with \a name. If no such layer was found, returns nullptr. + * Returns a pointer to the node with \a name. If no such node was found, returns nullptr. */ - const Layer *find_layer_by_name(StringRefNull name) const; - Layer *find_layer_by_name(StringRefNull name); - - /** - * Returns a pointer to the group with \a name. If no such group was found, returns nullptr. - */ - const LayerGroup *find_group_by_name(StringRefNull name) const; - LayerGroup *find_group_by_name(StringRefNull name); + const TreeNode *find_node_by_name(StringRefNull name) const; + TreeNode *find_node_by_name(StringRefNull name); /** * Print the nodes. For debugging purposes. diff --git a/source/blender/blenkernel/intern/grease_pencil.cc b/source/blender/blenkernel/intern/grease_pencil.cc index 40e9771f7aa..51efb87e9fa 100644 --- a/source/blender/blenkernel/intern/grease_pencil.cc +++ b/source/blender/blenkernel/intern/grease_pencil.cc @@ -103,8 +103,10 @@ static void grease_pencil_copy_data(Main * /*bmain*/, /* Set active layer. */ if (grease_pencil_src->has_active_layer()) { - grease_pencil_dst->set_active_layer( - grease_pencil_dst->find_layer_by_name(grease_pencil_src->active_layer->wrap().name())); + bke::greasepencil::TreeNode *active_node = grease_pencil_dst->find_node_by_name( + grease_pencil_src->active_layer->wrap().name()); + BLI_assert(active_node && active_node->is_layer()); + grease_pencil_dst->set_active_layer(&active_node->as_layer()); } CustomData_copy(&grease_pencil_src->layers_data, @@ -1006,41 +1008,21 @@ Span LayerGroup::groups_for_write() return this->runtime->layer_group_cache_.as_span(); } -const Layer *LayerGroup::find_layer_by_name(const StringRefNull name) const +const TreeNode *LayerGroup::find_node_by_name(const StringRefNull name) const { - for (const Layer *layer : this->layers()) { - if (StringRef(layer->name()) == StringRef(name)) { - return layer; + for (const TreeNode *node : this->nodes()) { + if (StringRef(node->name()) == StringRef(name)) { + return node; } } return nullptr; } -Layer *LayerGroup::find_layer_by_name(const StringRefNull name) +TreeNode *LayerGroup::find_node_by_name(const StringRefNull name) { - for (Layer *layer : this->layers_for_write()) { - if (StringRef(layer->name()) == StringRef(name)) { - return layer; - } - } - return nullptr; -} - -const LayerGroup *LayerGroup::find_group_by_name(StringRefNull name) const -{ - for (const LayerGroup *group : this->groups()) { - if (StringRef(group->name()) == StringRef(name)) { - return group; - } - } - return nullptr; -} - -LayerGroup *LayerGroup::find_group_by_name(StringRefNull name) -{ - for (LayerGroup *group : this->groups_for_write()) { - if (StringRef(group->name()) == StringRef(name)) { - return group; + for (TreeNode *node : this->nodes_for_write()) { + if (StringRef(node->name()) == StringRef(name)) { + return node; } } return nullptr; @@ -2391,28 +2373,16 @@ void GreasePencil::move_node_into(blender::bke::greasepencil::TreeNode &node, parent_group.add_node(node); } -const blender::bke::greasepencil::Layer *GreasePencil::find_layer_by_name( +const blender::bke::greasepencil::TreeNode *GreasePencil::find_node_by_name( const blender::StringRefNull name) const { - return this->root_group().find_layer_by_name(name); + return this->root_group().find_node_by_name(name); } -blender::bke::greasepencil::Layer *GreasePencil::find_layer_by_name( +blender::bke::greasepencil::TreeNode *GreasePencil::find_node_by_name( const blender::StringRefNull name) { - return this->root_group().find_layer_by_name(name); -} - -const blender::bke::greasepencil::LayerGroup *GreasePencil::find_layer_group_by_name( - blender::StringRefNull name) const -{ - return this->root_group().find_group_by_name(name); -} - -blender::bke::greasepencil::LayerGroup *GreasePencil::find_layer_group_by_name( - blender::StringRefNull name) -{ - return this->root_group().find_group_by_name(name); + return this->root_group().find_node_by_name(name); } void GreasePencil::rename_node(blender::bke::greasepencil::TreeNode &node, diff --git a/source/blender/blenkernel/intern/grease_pencil_test.cc b/source/blender/blenkernel/intern/grease_pencil_test.cc index 29ecdefca30..dcfc6918957 100644 --- a/source/blender/blenkernel/intern/grease_pencil_test.cc +++ b/source/blender/blenkernel/intern/grease_pencil_test.cc @@ -195,11 +195,11 @@ TEST(greasepencil, layer_tree_is_child_of) EXPECT_FALSE(ex.grease_pencil.root_group().is_child_of(ex.grease_pencil.root_group())); - const LayerGroup &group1 = *ex.grease_pencil.find_layer_group_by_name("Group1"); - const LayerGroup &group2 = *ex.grease_pencil.find_layer_group_by_name("Group2"); - const Layer &layer1 = *ex.grease_pencil.find_layer_by_name("Layer1"); - const Layer &layer3 = *ex.grease_pencil.find_layer_by_name("Layer3"); - const Layer &layer5 = *ex.grease_pencil.find_layer_by_name("Layer5"); + const LayerGroup &group1 = ex.grease_pencil.find_node_by_name("Group1")->as_group(); + const LayerGroup &group2 = ex.grease_pencil.find_node_by_name("Group2")->as_group(); + const Layer &layer1 = ex.grease_pencil.find_node_by_name("Layer1")->as_layer(); + const Layer &layer3 = ex.grease_pencil.find_node_by_name("Layer3")->as_layer(); + const Layer &layer5 = ex.grease_pencil.find_node_by_name("Layer5")->as_layer(); EXPECT_TRUE(layer1.is_child_of(ex.grease_pencil.root_group())); EXPECT_TRUE(layer1.is_child_of(group1)); diff --git a/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc b/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc index 91309693438..304054bf261 100644 --- a/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc +++ b/source/blender/editors/grease_pencil/intern/grease_pencil_layers.cc @@ -141,8 +141,8 @@ static int grease_pencil_layer_reorder_exec(bContext *C, wmOperator *op) op->ptr, "target_layer_name", nullptr, 0, &target_layer_name_length); const int reorder_location = RNA_enum_get(op->ptr, "location"); - Layer *target_layer = grease_pencil.find_layer_by_name(target_layer_name); - if (!target_layer) { + TreeNode *target_node = grease_pencil.find_node_by_name(target_layer_name); + if (!target_node || !target_node->is_layer()) { MEM_SAFE_FREE(target_layer_name); return OPERATOR_CANCELLED; } @@ -152,13 +152,13 @@ static int grease_pencil_layer_reorder_exec(bContext *C, wmOperator *op) case LAYER_REORDER_ABOVE: { /* NOTE: The layers are stored from bottom to top, so inserting above (visually), means * inserting the link after the target. */ - grease_pencil.move_node_after(active_layer.as_node(), target_layer->as_node()); + grease_pencil.move_node_after(active_layer.as_node(), *target_node); break; } case LAYER_REORDER_BELOW: { /* NOTE: The layers are stored from bottom to top, so inserting below (visually), means * inserting the link before the target. */ - grease_pencil.move_node_before(active_layer.as_node(), target_layer->as_node()); + grease_pencil.move_node_before(active_layer.as_node(), *target_node); break; } default: diff --git a/source/blender/makesdna/DNA_grease_pencil_types.h b/source/blender/makesdna/DNA_grease_pencil_types.h index 7151294d6ef..173f3ed99ca 100644 --- a/source/blender/makesdna/DNA_grease_pencil_types.h +++ b/source/blender/makesdna/DNA_grease_pencil_types.h @@ -492,11 +492,8 @@ typedef struct GreasePencil { blender::bke::greasepencil::LayerGroup &parent_group); /* Search functions. */ - const blender::bke::greasepencil::Layer *find_layer_by_name(blender::StringRefNull name) const; - blender::bke::greasepencil::Layer *find_layer_by_name(blender::StringRefNull name); - const blender::bke::greasepencil::LayerGroup *find_layer_group_by_name( - blender::StringRefNull name) const; - blender::bke::greasepencil::LayerGroup *find_layer_group_by_name(blender::StringRefNull name); + const blender::bke::greasepencil::TreeNode *find_node_by_name(blender::StringRefNull name) const; + blender::bke::greasepencil::TreeNode *find_node_by_name(blender::StringRefNull name); void rename_node(blender::bke::greasepencil::TreeNode &node, blender::StringRefNull new_name);