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
This commit is contained in:
Casey Bianco-Davis
2025-08-27 14:21:11 +02:00
committed by Falk David
parent 193e22ee7e
commit fad44198a3
2 changed files with 50 additions and 23 deletions

View File

@@ -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),

View File

@@ -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<PointsRange> retrieve_selection_ranges(Object &object,
const Span<MutableDrawingInfo> drawings,
const ActiveLayerBehavior active_layer_behavior,
int64_t &r_total_points_selected,
IndexMaskMemory &memory)
{
@@ -57,6 +58,23 @@ Vector<PointsRange> 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<int> 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<PointsRange> 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<IndexRange> ranges{};
const Array<int> 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<MutableDrawingInfo> editable_drawings = retrieve_editable_drawings(*scene,
grease_pencil);
Vector<PointsRange> 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");
}