From 297b97f2df36248408bbbbde12600b9370a65642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Wed, 16 Oct 2024 16:41:06 +0200 Subject: [PATCH] Fix #128769: GPv3: Interpolate tool takes selection into account This makes it so the stroke selection is taken into account by the interpolation tool. This does NOT use the selection _order_, just limits interpolation by index to the selection. The result will be the subset of selected strokes, excluding non-selected strokes, which is closer to the GPv2 behavior. The `sample_curve_attribute` function was using the target curve index for both the src and dst curves. This worked when all the curves are interpolated, but with selections the dst curve is generally not the same index as the src curve. The `from_curve_indices` array must be passed along as well to retrieve the correct source index. Pull Request: https://projects.blender.org/blender/blender/pulls/129096 --- .../sculpt_paint/grease_pencil_interpolate.cc | 44 ++++++++--- .../geometry/GEO_interpolate_curves.hh | 6 +- .../geometry/intern/interpolate_curves.cc | 78 ++++++++++--------- 3 files changed, 79 insertions(+), 49 deletions(-) diff --git a/source/blender/editors/sculpt_paint/grease_pencil_interpolate.cc b/source/blender/editors/sculpt_paint/grease_pencil_interpolate.cc index 976852ada86..7c670120a41 100644 --- a/source/blender/editors/sculpt_paint/grease_pencil_interpolate.cc +++ b/source/blender/editors/sculpt_paint/grease_pencil_interpolate.cc @@ -26,6 +26,7 @@ #include "DNA_grease_pencil_types.h" +#include "ED_curves.hh" #include "ED_grease_pencil.hh" #include "ED_numinput.hh" #include "ED_screen.hh" @@ -241,6 +242,7 @@ static void find_curve_mapping_from_index(const GreasePencil &grease_pencil, const bke::greasepencil::Layer &layer, const int current_frame, const bool exclude_breakdowns, + const bool only_selected, InterpolationPairs &pairs) { using bke::greasepencil::Drawing; @@ -256,17 +258,31 @@ static void find_curve_mapping_from_index(const GreasePencil &grease_pencil, const Drawing &from_drawing = *grease_pencil.get_drawing_at(layer, interval->first); const Drawing &to_drawing = *grease_pencil.get_drawing_at(layer, interval->second); - const int pairs_num = std::min(from_drawing.strokes().curves_num(), - to_drawing.strokes().curves_num()); + IndexMaskMemory memory; + IndexMask from_selection, to_selection; + if (only_selected) { + from_selection = ed::curves::retrieve_selected_curves(from_drawing.strokes(), memory); + to_selection = ed::curves::retrieve_selected_curves(to_drawing.strokes(), memory); + } + else { + from_selection = from_drawing.strokes().curves_range(); + to_selection = to_drawing.strokes().curves_range(); + } + + const int pairs_num = std::min(from_selection.size(), to_selection.size()); const int old_pairs_num = pairs.from_frames.size(); pairs.from_frames.append_n_times(interval->first, pairs_num); pairs.to_frames.append_n_times(interval->second, pairs_num); pairs.from_curves.resize(old_pairs_num + pairs_num); pairs.to_curves.resize(old_pairs_num + pairs_num); - array_utils::fill_index_range( - pairs.from_curves.as_mutable_span().slice(old_pairs_num, pairs_num)); - array_utils::fill_index_range(pairs.to_curves.as_mutable_span().slice(old_pairs_num, pairs_num)); + + /* Write source indices into the pair data. The drawing with fewer curves will discard some based + * on index. */ + from_selection.slice(0, pairs_num) + .to_indices(pairs.from_curves.as_mutable_span().slice(old_pairs_num, pairs_num)); + to_selection.slice(0, pairs_num) + .to_indices(pairs.to_curves.as_mutable_span().slice(old_pairs_num, pairs_num)); } InterpolateOpData *InterpolateOpData::from_operator(const bContext &C, const wmOperator &op) @@ -313,8 +329,13 @@ InterpolateOpData *InterpolateOpData::from_operator(const bContext &C, const wmO InterpolateOpData::LayerData &layer_data = data->layer_data[layer_index]; /* Pair from/to curves by index. */ - find_curve_mapping_from_index( - grease_pencil, layer, current_frame, data->exclude_breakdowns, layer_data.curve_pairs); + const bool only_selected = true; + find_curve_mapping_from_index(grease_pencil, + layer, + current_frame, + data->exclude_breakdowns, + only_selected, + layer_data.curve_pairs); }); const std::optional active_layer_interval = find_frames_interval( @@ -501,10 +522,11 @@ static bke::CurvesGeometry interpolate_between_curves(const GreasePencil &grease const IndexRange from_curves = from_drawing->strokes().curves_range(); const IndexRange to_curves = to_drawing->strokes().curves_range(); - /* Subset of target curves that are filled by this frame pair. */ IndexMaskMemory selection_memory; - const IndexMask selection = IndexMask::from_indices(sorted_pairs.as_span().slice(pair_range), - selection_memory); + /* Subset of target curves that are filled by this frame pair. Selection is built from pair + * indices, which correspond to dst curve indices. */ + const IndexMask dst_curve_mask = IndexMask::from_indices( + sorted_pairs.as_span().slice(pair_range), selection_memory); MutableSpan pair_from_indices = sorted_from_curve_indices.as_mutable_span().slice( pair_range); MutableSpan pair_to_indices = sorted_to_curve_indices.as_mutable_span().slice(pair_range); @@ -519,7 +541,7 @@ static bke::CurvesGeometry interpolate_between_curves(const GreasePencil &grease to_drawing->strokes(), pair_from_indices, pair_to_indices, - selection, + dst_curve_mask, dst_curve_flip, mix_factor, dst_curves); diff --git a/source/blender/geometry/GEO_interpolate_curves.hh b/source/blender/geometry/GEO_interpolate_curves.hh index b6d3c0e1060..5f5afde9d04 100644 --- a/source/blender/geometry/GEO_interpolate_curves.hh +++ b/source/blender/geometry/GEO_interpolate_curves.hh @@ -13,14 +13,14 @@ namespace blender::geometry { /** * Create new curves that are interpolated between "from" and "to" curves. - * \param selection: Selection of curves in \a dst_curves that are being filled. + * \param dst_curve_mask: Set of curves in \a dst_curves that are being filled. */ void interpolate_curves(const bke::CurvesGeometry &from_curves, const bke::CurvesGeometry &to_curves, Span from_curve_indices, Span to_curve_indices, - const IndexMask &selection, - Span curve_flip_direction, + const IndexMask &dst_curve_mask, + Span dst_curve_flip_direction, const float mix_factor, bke::CurvesGeometry &dst_curves); diff --git a/source/blender/geometry/intern/interpolate_curves.cc b/source/blender/geometry/intern/interpolate_curves.cc index 2fd49892280..bcbcef3823e 100644 --- a/source/blender/geometry/intern/interpolate_curves.cc +++ b/source/blender/geometry/intern/interpolate_curves.cc @@ -175,11 +175,12 @@ static AttributesForInterpolation gather_curve_attributes_to_interpolate( /* Resample a span of attribute values from source curves to a destination buffer. */ static void sample_curve_attribute(const bke::CurvesGeometry &src_curves, + const Span src_curve_indices, const OffsetIndices dst_points_by_curve, const GSpan src_data, - const IndexMask &curve_selection, - const Span sample_indices, - const Span sample_factors, + const IndexMask &dst_curve_mask, + const Span dst_sample_indices, + const Span dst_sample_factors, GMutableSpan dst_data) { const CPPType &type = src_data.type(); @@ -191,8 +192,8 @@ static void sample_curve_attribute(const bke::CurvesGeometry &src_curves, #ifndef NDEBUG const int dst_points_num = dst_data.size(); - BLI_assert(sample_indices.size() == dst_points_num); - BLI_assert(sample_factors.size() == dst_points_num); + BLI_assert(dst_sample_indices.size() == dst_points_num); + BLI_assert(dst_sample_factors.size() == dst_points_num); #endif bke::attribute_math::convert_to_static_type(type, [&](auto dummy) { @@ -201,24 +202,25 @@ static void sample_curve_attribute(const bke::CurvesGeometry &src_curves, MutableSpan dst = dst_data.typed(); Vector evaluated_data; - curve_selection.foreach_index([&](const int i_curve) { - const IndexRange src_points = src_points_by_curve[i_curve]; - const IndexRange dst_points = dst_points_by_curve[i_curve]; + dst_curve_mask.foreach_index([&](const int i_dst_curve, const int pos) { + const int i_src_curve = src_curve_indices[pos]; + const IndexRange src_points = src_points_by_curve[i_src_curve]; + const IndexRange dst_points = dst_points_by_curve[i_dst_curve]; - if (curve_types[i_curve] == CURVE_TYPE_POLY) { + if (curve_types[i_src_curve] == CURVE_TYPE_POLY) { length_parameterize::interpolate(src.slice(src_points), - sample_indices.slice(dst_points), - sample_factors.slice(dst_points), + dst_sample_indices.slice(dst_points), + dst_sample_factors.slice(dst_points), dst.slice(dst_points)); } else { - const IndexRange src_evaluated_points = src_evaluated_points_by_curve[i_curve]; + const IndexRange src_evaluated_points = src_evaluated_points_by_curve[i_src_curve]; evaluated_data.reinitialize(src_evaluated_points.size()); src_curves.interpolate_to_evaluated( - i_curve, src.slice(src_points), evaluated_data.as_mutable_span()); + i_src_curve, src.slice(src_points), evaluated_data.as_mutable_span()); length_parameterize::interpolate(evaluated_data.as_span(), - sample_indices.slice(dst_points), - sample_factors.slice(dst_points), + dst_sample_indices.slice(dst_points), + dst_sample_factors.slice(dst_points), dst.slice(dst_points)); } }); @@ -276,13 +278,13 @@ void interpolate_curves(const CurvesGeometry &from_curves, const CurvesGeometry &to_curves, const Span from_curve_indices, const Span to_curve_indices, - const IndexMask &selection, - const Span curve_flip_direction, + const IndexMask &dst_curve_mask, + const Span dst_curve_flip_direction, const float mix_factor, CurvesGeometry &dst_curves) { - BLI_assert(from_curve_indices.size() == selection.size()); - BLI_assert(to_curve_indices.size() == selection.size()); + BLI_assert(from_curve_indices.size() == dst_curve_mask.size()); + BLI_assert(to_curve_indices.size() == dst_curve_mask.size()); if (from_curves.curves_num() == 0 || to_curves.curves_num() == 0) { return; @@ -294,7 +296,7 @@ void interpolate_curves(const CurvesGeometry &from_curves, const Span to_evaluated_positions = to_curves.evaluated_positions(); /* All resampled curves are poly curves. */ - dst_curves.fill_curve_types(selection, CURVE_TYPE_POLY); + dst_curves.fill_curve_types(dst_curve_mask, CURVE_TYPE_POLY); MutableSpan dst_positions = dst_curves.positions_for_write(); @@ -319,7 +321,7 @@ void interpolate_curves(const CurvesGeometry &from_curves, const OffsetIndices dst_points_by_curve = dst_curves.points_by_curve(); /* Gather uniform samples based on the accumulated lengths of the original curve. */ - selection.foreach_index(GrainSize(32), [&](const int i_dst_curve, const int pos) { + dst_curve_mask.foreach_index(GrainSize(32), [&](const int i_dst_curve, const int pos) { const int i_from_curve = from_curve_indices[pos]; const int i_to_curve = to_curve_indices[pos]; const IndexRange dst_points = dst_points_by_curve[i_dst_curve]; @@ -345,7 +347,7 @@ void interpolate_curves(const CurvesGeometry &from_curves, to_sample_factors.as_mutable_span().slice(dst_points).fill(0.0f); } else { - if (curve_flip_direction[i_dst_curve]) { + if (dst_curve_flip_direction[i_dst_curve]) { length_parameterize::sample_uniform_reverse( to_lengths, !to_curves_cyclic[i_to_curve], @@ -379,35 +381,39 @@ void interpolate_curves(const CurvesGeometry &from_curves, GArray<> from_samples(dst.type(), dst.size()); GArray<> to_samples(dst.type(), dst.size()); sample_curve_attribute(from_curves, + from_curve_indices, dst_points_by_curve, src_from, - selection, + dst_curve_mask, from_sample_indices, from_sample_factors, from_samples); sample_curve_attribute(to_curves, + to_curve_indices, dst_points_by_curve, src_to, - selection, + dst_curve_mask, to_sample_indices, to_sample_factors, to_samples); - mix_arrays(from_samples, to_samples, mix_factor, selection, dst_points_by_curve, dst); + mix_arrays(from_samples, to_samples, mix_factor, dst_curve_mask, dst_points_by_curve, dst); } else if (!src_from.is_empty()) { sample_curve_attribute(from_curves, + from_curve_indices, dst_points_by_curve, src_from, - selection, + dst_curve_mask, from_sample_indices, from_sample_factors, dst); } else if (!src_to.is_empty()) { sample_curve_attribute(to_curves, + to_curve_indices, dst_points_by_curve, src_to, - selection, + dst_curve_mask, to_sample_indices, to_sample_factors, dst); @@ -420,16 +426,18 @@ void interpolate_curves(const CurvesGeometry &from_curves, /* Interpolate the evaluated positions to the resampled curves. */ sample_curve_attribute(from_curves, + from_curve_indices, dst_points_by_curve, from_evaluated_positions, - selection, + dst_curve_mask, from_sample_indices, from_sample_factors, from_samples.as_mutable_span()); sample_curve_attribute(to_curves, + to_curve_indices, dst_points_by_curve, to_evaluated_positions, - selection, + dst_curve_mask, to_sample_indices, to_sample_factors, to_samples.as_mutable_span()); @@ -437,7 +445,7 @@ void interpolate_curves(const CurvesGeometry &from_curves, mix_arrays(from_samples.as_span(), to_samples.as_span(), mix_factor, - selection, + dst_curve_mask, dst_points_by_curve, dst_positions); } @@ -461,15 +469,15 @@ void interpolate_curves(const CurvesGeometry &from_curves, if (can_mix_attribute && !src_from.is_empty() && !src_to.is_empty()) { GArray<> from_samples(dst.type(), dst.size()); GArray<> to_samples(dst.type(), dst.size()); - array_utils::copy(GVArray::ForSpan(src_from), selection, from_samples); - array_utils::copy(GVArray::ForSpan(src_to), selection, to_samples); - mix_arrays(from_samples, to_samples, mix_factor, selection, dst); + array_utils::copy(GVArray::ForSpan(src_from), dst_curve_mask, from_samples); + array_utils::copy(GVArray::ForSpan(src_to), dst_curve_mask, to_samples); + mix_arrays(from_samples, to_samples, mix_factor, dst_curve_mask, dst); } else if (!src_from.is_empty()) { - array_utils::copy(GVArray::ForSpan(src_from), selection, dst); + array_utils::copy(GVArray::ForSpan(src_from), dst_curve_mask, dst); } else if (!src_to.is_empty()) { - array_utils::copy(GVArray::ForSpan(src_to), selection, dst); + array_utils::copy(GVArray::ForSpan(src_to), dst_curve_mask, dst); } }