From 949a7e6a60f4d2adc100a32afd89cf08092ebea3 Mon Sep 17 00:00:00 2001 From: Falk David Date: Thu, 5 Dec 2024 18:18:50 +0100 Subject: [PATCH] Fix: Grease Pencil: Separate by Selection issues Issues with the "Separate by selection" operator. Reported by the Blender studio. There were multiple problems in this code. 1. When a new object is created, the parameters are not copied. 2. When a new layer in a new object is created, the parameters are not copied. 3. The code to transfer the layer attributes was not working correctly. Fixed all the issues above. The layer attributes are now transfered once after all layers have been added. Pull Request: https://projects.blender.org/blender/blender/pulls/131449 --- .../intern/grease_pencil_edit.cc | 70 ++++++++++++++----- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/source/blender/editors/grease_pencil/intern/grease_pencil_edit.cc b/source/blender/editors/grease_pencil/intern/grease_pencil_edit.cc index a677e0d6293..20447889bed 100644 --- a/source/blender/editors/grease_pencil/intern/grease_pencil_edit.cc +++ b/source/blender/editors/grease_pencil/intern/grease_pencil_edit.cc @@ -1932,13 +1932,18 @@ static Object *duplicate_grease_pencil_object(Main *bmain, Base *base_new = object::add_duplicate(bmain, scene, view_layer, base_prev, dupflag); Object *object_dst = base_new->object; object_dst->mode = OB_MODE_OBJECT; - object_dst->data = BKE_grease_pencil_add(bmain, grease_pencil_src.id.name + 2); + GreasePencil *grease_pencil_dst = BKE_grease_pencil_add(bmain, grease_pencil_src.id.name + 2); + BKE_grease_pencil_copy_parameters(grease_pencil_src, *grease_pencil_dst); + object_dst->data = grease_pencil_dst; return object_dst; } static bke::greasepencil::Layer &find_or_create_layer_in_dst_by_name( - const int layer_index, const GreasePencil &grease_pencil_src, GreasePencil &grease_pencil_dst) + const int layer_index, + const GreasePencil &grease_pencil_src, + GreasePencil &grease_pencil_dst, + Vector &src_to_dst_layer_indices) { using namespace bke::greasepencil; @@ -1950,14 +1955,8 @@ static bke::greasepencil::Layer &find_or_create_layer_in_dst_by_name( /* If the layer can't be found in `grease_pencil_dst` by name add a new layer. */ Layer &new_layer = grease_pencil_dst.add_layer(layer_src.name()); - - /* Transfer Layer attributes. */ - bke::gather_attributes(grease_pencil_src.attributes(), - bke::AttrDomain::Layer, - bke::AttrDomain::Layer, - {}, - Span({layer_index}), - grease_pencil_dst.attributes_for_write()); + BKE_grease_pencil_copy_layer_parameters(layer_src, new_layer); + src_to_dst_layer_indices.append(layer_index); return new_layer; } @@ -1980,6 +1979,7 @@ static bool grease_pencil_separate_selected(bContext &C, /* Iterate through all the drawings at current scene frame. */ const Vector drawings_src = retrieve_editable_drawings(scene, grease_pencil_src); + Vector src_to_dst_layer_indices; for (const MutableDrawingInfo &info : drawings_src) { bke::CurvesGeometry &curves_src = info.drawing.strokes_for_write(); IndexMaskMemory memory; @@ -1990,10 +1990,9 @@ static bool grease_pencil_separate_selected(bContext &C, /* Insert Keyframe at current frame/layer. */ Layer &layer_dst = find_or_create_layer_in_dst_by_name( - info.layer_index, grease_pencil_src, grease_pencil_dst); + info.layer_index, grease_pencil_src, grease_pencil_dst, src_to_dst_layer_indices); Drawing *drawing_dst = grease_pencil_dst.insert_frame(layer_dst, info.frame_number); - /* TODO: Can we assume the insert never fails? */ BLI_assert(drawing_dst != nullptr); /* Copy strokes to new CurvesGeometry. */ @@ -2008,7 +2007,22 @@ static bool grease_pencil_separate_selected(bContext &C, } if (changed) { - grease_pencil_dst.set_active_layer(nullptr); + /* Transfer layer attributes. */ + bke::gather_attributes(grease_pencil_src.attributes(), + bke::AttrDomain::Layer, + bke::AttrDomain::Layer, + {}, + src_to_dst_layer_indices.as_span(), + grease_pencil_dst.attributes_for_write()); + + /* Set the active layer in the target object. */ + if (grease_pencil_src.has_active_layer()) { + const Layer &active_layer_src = *grease_pencil_src.get_active_layer(); + TreeNode *active_layer_dst = grease_pencil_dst.find_node_by_name(active_layer_src.name()); + if (active_layer_dst && active_layer_dst->is_layer()) { + grease_pencil_dst.set_active_layer(&active_layer_dst->as_layer()); + } + } /* Add object materials to target object. */ BKE_object_material_array_assign(&bmain, @@ -2046,8 +2060,9 @@ static bool grease_pencil_separate_layer(bContext &C, Object *object_dst = duplicate_grease_pencil_object( &bmain, &scene, &view_layer, &base_prev, grease_pencil_src); GreasePencil &grease_pencil_dst = *static_cast(object_dst->data); + Vector src_to_dst_layer_indices; Layer &layer_dst = find_or_create_layer_in_dst_by_name( - layer_i, grease_pencil_src, grease_pencil_dst); + layer_i, grease_pencil_src, grease_pencil_dst, src_to_dst_layer_indices); /* Iterate through all the drawings at current frame. */ const Vector drawings_src = retrieve_editable_drawings_from_layer( @@ -2084,6 +2099,14 @@ static bool grease_pencil_separate_layer(bContext &C, changed = true; } + /* Transfer layer attributes. */ + bke::gather_attributes(grease_pencil_src.attributes(), + bke::AttrDomain::Layer, + bke::AttrDomain::Layer, + {}, + src_to_dst_layer_indices.as_span(), + grease_pencil_dst.attributes_for_write()); + remove_unused_materials(&bmain, object_dst); DEG_id_tag_update(&grease_pencil_dst.id, ID_RECALC_GEOMETRY); @@ -2114,6 +2137,7 @@ static bool grease_pencil_separate_material(bContext &C, Object *object_dst = duplicate_grease_pencil_object( &bmain, &scene, &view_layer, &base_prev, grease_pencil_src); + GreasePencil &grease_pencil_dst = *static_cast(object_dst->data); /* Add object materials. */ BKE_object_material_array_assign(&bmain, @@ -2125,6 +2149,7 @@ static bool grease_pencil_separate_material(bContext &C, /* Iterate through all the drawings at current scene frame. */ const Vector drawings_src = retrieve_editable_drawings(scene, grease_pencil_src); + Vector src_to_dst_layer_indices; for (const MutableDrawingInfo &info : drawings_src) { bke::CurvesGeometry &curves_src = info.drawing.strokes_for_write(); IndexMaskMemory memory; @@ -2134,11 +2159,9 @@ static bool grease_pencil_separate_material(bContext &C, continue; } - GreasePencil &grease_pencil_dst = *static_cast(object_dst->data); - /* Insert Keyframe at current frame/layer. */ Layer &layer_dst = find_or_create_layer_in_dst_by_name( - info.layer_index, grease_pencil_src, grease_pencil_dst); + info.layer_index, grease_pencil_src, grease_pencil_dst, src_to_dst_layer_indices); Drawing *drawing_dst = grease_pencil_dst.insert_frame(layer_dst, info.frame_number); /* TODO: Can we assume the insert never fails? */ @@ -2150,13 +2173,22 @@ static bool grease_pencil_separate_material(bContext &C, info.drawing.tag_topology_changed(); drawing_dst->tag_topology_changed(); - DEG_id_tag_update(&grease_pencil_dst.id, ID_RECALC_GEOMETRY); - WM_event_add_notifier(&C, NC_OBJECT | ND_DRAW, &grease_pencil_dst); changed = true; } + /* Transfer layer attributes. */ + bke::gather_attributes(grease_pencil_src.attributes(), + bke::AttrDomain::Layer, + bke::AttrDomain::Layer, + {}, + src_to_dst_layer_indices.as_span(), + grease_pencil_dst.attributes_for_write()); + remove_unused_materials(&bmain, object_dst); + + DEG_id_tag_update(&grease_pencil_dst.id, ID_RECALC_GEOMETRY); + WM_event_add_notifier(&C, NC_OBJECT | ND_DRAW, &grease_pencil_dst); } if (changed) {