blender
/
blender
136
406
Fork 589
Code Issues 6.3k Pull Requests 766 Projects 19 Wiki Activity

Curves: Add select more/less #104626

Merged
Falk David merged 7 commits from filedescriptor/blender:curves-select-more-less into main 2023-02-16 17:02:54 +01:00
Conversation 0 Commits 7 Files Changed 5 +38 -13
1 changed files with 38 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 8b8495f339 - Show all commits

51
source/blender/editors/curves/intern/curves_selection.cc
View File

@ -330,25 +330,50 @@ void select_adjacent(bke::CurvesGeometry &curves, const bool deselect)
invert_selection(selection.span);
}
threading::parallel_for(curves.curves_range(), 256, [&](const IndexRange range) {
for (const int curve_i : range) {
GMutableSpan selection_curve = selection.span.slice(points_by_curve[curve_i]);
if (selection.span.type().is<bool>()) {
MutableSpan<bool> selection_typed = selection.span.typed<bool>();
threading::parallel_for(curves.curves_range(), 256, [&](const IndexRange range) {
for (const int curve_i : range) {
MutableSpan<bool> selection_curve = selection_typed.slice(points_by_curve[curve_i]);
/* Handle all cases in the forward direction. */
for (int point_i = 0; point_i < points_by_curve.size(curve_i) - 1; point_i++) {
if (check_adjacent_selection(selection_curve, point_i, point_i + 1)) {
apply_selection_operation_at_index(selection_curve, point_i, SEL_OP_ADD);
/* Handle all cases in the forward direction. */
for (int point_i = 0; point_i < points_by_curve.size(curve_i) - 1; point_i++) {
if (!selection_curve[point_i] && selection_curve[point_i + 1]) {
selection_curve[point_i] = true;
}
filedescriptor marked this conversation as resolved
Hans Goudey commented 2023-02-16 14:15:10 +01:00
Review
Copy Link

Nice, that's simpler than I expected. Only needing to propagate it by one element helps!

Nice, that's simpler than I expected. Only needing to propagate it by one element helps!
👍 1
}
/* Handle all cases in the backwards direction. */
for (int point_i = points_by_curve.size(curve_i) - 1; point_i > 0; point_i--) {
if (!selection_curve[point_i] && selection_curve[point_i - 1]) {
selection_curve[point_i] = true;
filedescriptor marked this conversation as resolved
Hans Goudey commented 2023-02-13 02:02:58 +01:00
Review
Copy Link

Looks like this doesn't handle cyclic curves. For some similar logic, ControlPointNeighborFieldInput might be worth checking out.

IMO, this is pushing the boundaries of what should be done with dynamic types per index, since there's 2 reads per index. But I strongly doubt the selection operators will noticeably non-instantaneous, and it can always be improved later.

Looks like this doesn't handle cyclic curves. For some similar logic, `ControlPointNeighborFieldInput` _might_ be worth checking out. IMO, this is pushing the boundaries of what should be done with dynamic types per index, since there's 2 reads per index. But I strongly doubt the selection operators will noticeably non-instantaneous, and it can always be improved later.
}
}
}
});
}
else if (selection.span.type().is<float>()) {
MutableSpan<float> selection_typed = selection.span.typed<float>();
threading::parallel_for(curves.curves_range(), 256, [&](const IndexRange range) {
for (const int curve_i : range) {
MutableSpan<float> selection_curve = selection_typed.slice(points_by_curve[curve_i]);
/* Handle all cases in the backwards direction. */
for (int point_i = points_by_curve.size(curve_i) - 1; point_i > 0; point_i--) {
if (check_adjacent_selection(selection_curve, point_i, point_i - 1)) {
apply_selection_operation_at_index(selection_curve, point_i, SEL_OP_ADD);
/* Handle all cases in the forward direction. */
for (int point_i = 0; point_i < points_by_curve.size(curve_i) - 1; point_i++) {
if ((selection_typed[point_i] == 0.0f) && (selection_typed[point_i + 1] > 0.0f)) {
selection_curve[point_i] = 1.0f;
}
}
/* Handle all cases in the backwards direction. */
filedescriptor marked this conversation as resolved
Hans Goudey commented 2023-02-16 14:14:45 +01:00
Review
Copy Link

This mix of using selection_typed and selection_curve is confusing, it's hard to tell what's correct here.

If it's possible, I'd suggest only using actual point indices (rather than 0..points.size() for each curve). Having a local IndexRange variable with the .last(N) method should make that simpler.

This mix of using `selection_typed` and `selection_curve` is confusing, it's hard to tell what's correct here. If it's possible, I'd suggest only using actual point indices (rather than 0..points.size() for each curve). Having a local `IndexRange` variable with the `.last(N)` method should make that simpler.
Falk David commented 2023-02-16 14:23:41 +01:00
Review
Copy Link

Ah good catch, that's actually a bug I was trying to fix. It didn't work with float selection, so I think that is why.

Ah good catch, that's actually a bug I was trying to fix. It didn't work with float selection, so I think that is why.
for (int point_i = points_by_curve.size(curve_i) - 1; point_i > 0; point_i--) {
if ((selection_typed[point_i] == 0.0f) && (selection_typed[point_i - 1] > 0.0f)) {
selection_curve[point_i] = 1.0f;
}
}
}
}
});
});
}
if (deselect) {
invert_selection(selection.span);