From fad44198a3994696811fafff4be96e801ef325a4 Mon Sep 17 00:00:00 2001 From: Casey Bianco-Davis Date: Wed, 27 Aug 2025 14:21:11 +0200 Subject: [PATCH] Fix #130293: Grease Pencil: Edit mode `Join` operator splits points When the `Join` operator was added to Grease Pencil v3 the behavior when joining points was changed. The selected point would now be split from the existing strokes and put into a new one. This behavior is often undesirable, leading to multiple user reporting it as a bug #130293, #141368, #131036, #132201, #136144 and #144300. This PR adds a new mode, `Join Strokes` that behaves the same as legacy grease pencil, and sets it as default. This PR also renames the existing modes to `SplitAndCopy`, `SplitPoints` to better indicate the expected behavior. Pull Request: https://projects.blender.org/blender/blender/pulls/144666 --- .../keyconfig/keymap_data/blender_default.py | 4 +- .../intern/grease_pencil_join_selection.cc | 69 +++++++++++++------ 2 files changed, 50 insertions(+), 23 deletions(-) diff --git a/scripts/presets/keyconfig/keymap_data/blender_default.py b/scripts/presets/keyconfig/keymap_data/blender_default.py index 55b2986a79d..90318e95f08 100644 --- a/scripts/presets/keyconfig/keymap_data/blender_default.py +++ b/scripts/presets/keyconfig/keymap_data/blender_default.py @@ -4037,9 +4037,9 @@ def km_grease_pencil_edit_mode(params): # Join selection ("grease_pencil.join_selection", {"type": 'J', "value": 'PRESS', "ctrl": True}, - {"properties": [("type", 'JOIN')]}), + {"properties": [("type", 'JOINSTROKES')]}), ("grease_pencil.join_selection", {"type": 'J', "value": 'PRESS', "shift": True, "ctrl": True}, - {"properties": [("type", 'JOINCOPY')]}), + {"properties": [("type", 'SPLITCOPY')]}), ("grease_pencil.duplicate_move", {"type": 'D', "value": 'PRESS', "shift": True}, None), diff --git a/source/blender/editors/grease_pencil/intern/grease_pencil_join_selection.cc b/source/blender/editors/grease_pencil/intern/grease_pencil_join_selection.cc index 67e74788657..8c94666ef1f 100644 --- a/source/blender/editors/grease_pencil/intern/grease_pencil_join_selection.cc +++ b/source/blender/editors/grease_pencil/intern/grease_pencil_join_selection.cc @@ -40,7 +40,7 @@ struct PointsRange { enum class ActionOnNextRange { Nothing, ReverseExisting, ReverseAddition, ReverseBoth }; -enum class ActiveLayerBehavior { JoinAndCopySelection, JoinSelection }; +enum class ActiveLayerBehavior { JoinStrokes, SplitAndCopy, SplitPoints }; /** * Iterates over \a drawings and returns a vector with all the selected ranges of points. @@ -50,6 +50,7 @@ enum class ActiveLayerBehavior { JoinAndCopySelection, JoinSelection }; */ Vector retrieve_selection_ranges(Object &object, const Span drawings, + const ActiveLayerBehavior active_layer_behavior, int64_t &r_total_points_selected, IndexMaskMemory &memory) { @@ -57,6 +58,23 @@ Vector retrieve_selection_ranges(Object &object, r_total_points_selected = 0; for (const MutableDrawingInfo &info : drawings) { + if (active_layer_behavior == ActiveLayerBehavior::JoinStrokes) { + IndexMask curves_selection = retrieve_editable_and_selected_strokes( + object, info.drawing, info.layer_index, memory); + if (curves_selection.is_empty()) { + continue; + } + + const OffsetIndices points_by_curve = info.drawing.strokes().points_by_curve(); + curves_selection.foreach_index([&](const int curve_i) { + const IndexRange points = points_by_curve[curve_i]; + selected_ranges.append({&info.drawing, points}); + r_total_points_selected += points.size(); + }); + + continue; + } + IndexMask points_selection = retrieve_editable_and_selected_points( object, info.drawing, info.layer_index, memory); if (points_selection.is_empty()) { @@ -71,7 +89,6 @@ Vector retrieve_selection_ranges(Object &object, * i.e, if both the end of an stroke and the beginning of the next are selected, all the * indices end up in the same range. Let's refine the splitting */ - Vector ranges{}; const Array points_map = info.drawing.strokes().point_to_curve_map(); for (const IndexRange initial_range : initial_ranges) { if (points_map[initial_range.first()] == points_map[initial_range.last()]) { @@ -499,7 +516,7 @@ wmOperatorStatus grease_pencil_join_selection_exec(bContext *C, wmOperator *op) const Vector editable_drawings = retrieve_editable_drawings(*scene, grease_pencil); Vector ranges_selected = retrieve_selection_ranges( - *object, editable_drawings, selected_points_count, memory); + *object, editable_drawings, active_layer_behavior, selected_points_count, memory); if (ranges_selected.size() <= 1) { /* Nothing to join */ return OPERATOR_FINISHED; @@ -521,22 +538,27 @@ wmOperatorStatus grease_pencil_join_selection_exec(bContext *C, wmOperator *op) clear_selection_attribute(working_range_collection); bke::CurvesGeometry &dst_curves = dst_drawing->strokes_for_write(); - if (active_layer_behavior == ActiveLayerBehavior::JoinSelection) { + if (ELEM(active_layer_behavior, + ActiveLayerBehavior::SplitPoints, + ActiveLayerBehavior::JoinStrokes)) + { remove_selected_points(ranges_selected); } append_strokes_from(std::move(tmp_curves), dst_curves); - bke::GSpanAttributeWriter selection = ed::curves::ensure_selection_attribute( - dst_curves, selection_domain, bke::AttrType::Bool); + if (active_layer_behavior != ActiveLayerBehavior::JoinStrokes) { + bke::GSpanAttributeWriter selection = ed::curves::ensure_selection_attribute( + dst_curves, selection_domain, bke::AttrType::Bool); - if (selection_domain == bke::AttrDomain::Curve) { - ed::curves::fill_selection_true(selection.span.take_back(tmp_curves.curves_num())); + if (selection_domain == bke::AttrDomain::Curve) { + ed::curves::fill_selection_true(selection.span.take_back(tmp_curves.curves_num())); + } + else { + ed::curves::fill_selection_true(selection.span.take_back(tmp_curves.points_num())); + } + selection.finish(); } - else { - ed::curves::fill_selection_true(selection.span.take_back(tmp_curves.points_num())); - } - selection.finish(); dst_curves.update_curve_types(); dst_curves.tag_topology_changed(); @@ -551,16 +573,21 @@ wmOperatorStatus grease_pencil_join_selection_exec(bContext *C, wmOperator *op) void GREASE_PENCIL_OT_join_selection(wmOperatorType *ot) { static const EnumPropertyItem active_layer_behavior[] = { - {int(ActiveLayerBehavior::JoinAndCopySelection), - "JOINCOPY", + {int(ActiveLayerBehavior::JoinStrokes), + "JOINSTROKES", 0, - "Join and Copy", - "Copy the selection in the new stroke"}, - {int(ActiveLayerBehavior::JoinSelection), - "JOIN", + "Join Strokes", + "Join the selected strokes into one stroke"}, + {int(ActiveLayerBehavior::SplitAndCopy), + "SPLITCOPY", 0, - "Join", - "Move the selection to the new stroke"}, + "Split and Copy", + "Copy the selected points to a new stroke"}, + {int(ActiveLayerBehavior::SplitPoints), + "SPLIT", + 0, + "Split", + "Split the selected point to a new stroke"}, {0, nullptr, 0, nullptr, nullptr}, }; @@ -580,7 +607,7 @@ void GREASE_PENCIL_OT_join_selection(wmOperatorType *ot) ot->srna, "type", active_layer_behavior, - int(ActiveLayerBehavior::JoinSelection), + int(ActiveLayerBehavior::JoinStrokes), "Type", "Defines how the operator will behave on the selection in the active layer"); }