blender
/
blender
136
426
Fork 613
Code Issues 6.3k Pull Requests 780 Projects 19 Wiki Activity

GPv3: Dissolve Operator #111079

Merged
Pratik Borhade merged 14 commits from casey-bianco-davis/blender:GPv3-dissolve into main 2023-08-24 11:06:26 +02:00
Conversation 4 Commits 14 Files Changed 3 +5 -5
1 changed files with 5 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 8f3a092ca7 - Show all commits

10
source/blender/editors/grease_pencil/intern/grease_pencil_edit.cc
View File

@ -618,9 +618,9 @@ static int grease_pencil_dissolve_exec(bContext *C, wmOperator *op)
const Span<bool> curve_selection = points_to_dissolve.as_span().slice(points);
/* The unselected curves should not be dissolved.*/
if (!curve_selection.contains(true)) {
/* Because we are going to invert, fill with `!false` so that it does not get
/* Because we are going to invert, fill with true so that it does not get
* dissolved.*/
points_to_dissolve.as_mutable_span().slice(points).fill(!false);
points_to_dissolve.as_mutable_span().slice(points).fill(true);
casey-bianco-davis marked this conversation as resolved Outdated
Falk David commented 2023-08-14 12:23:09 +02:00
Outdated
Review
Copy Link

It might make sense to define a MutableSpan<bool> points_to_keep = points_to_dissolve.as_mutable_span(); with the comment above saying that this will be inverted later and thus it is the points to keep.

It might make sense to define a `MutableSpan<bool> points_to_keep = points_to_dissolve.as_mutable_span();` with the comment above saying that this will be inverted later and thus it is the points to keep.
}
/* `between` is just `unselect` but with the first and last segments not geting
@ -633,10 +633,10 @@ static int grease_pencil_dissolve_exec(bContext *C, wmOperator *op)
IndexRange first_range = deselection_ranges.first().shift(points.first());
filedescriptor marked this conversation as resolved Outdated
Hans Goudey commented 2023-08-13 00:38:22 +02:00
Outdated
Review
Copy Link

There's a lot of nesting here-- I'd suggest splitting the operation for a CurvesGeometry to a separate function ot make it clearer what's going on.

There's a lot of nesting here-- I'd suggest splitting the operation for a CurvesGeometry to a separate function ot make it clearer what's going on.
IndexRange last_range = deselection_ranges.last().shift(points.first());
/* Because we are going to invert, fill with `!false` so that it does not get
/* Because we are going to invert, fill with true so that it does not get
* dissolved.*/
points_to_dissolve.as_mutable_span().slice(last_range).fill(!false);
points_to_dissolve.as_mutable_span().slice(first_range).fill(!false);
points_to_dissolve.as_mutable_span().slice(last_range).fill(true);
casey-bianco-davis marked this conversation as resolved Outdated
Hans Goudey commented 2023-08-13 00:38:41 +02:00
Outdated
Review
Copy Link

!false could just be written as true

`!false` could just be written as `true`
points_to_dissolve.as_mutable_span().slice(first_range).fill(true);
}
}
}