Curves: Initial brush implementations for curves sculpt mode.
ClosedPublic
Actions

Authored by Jacques Lucke (JacquesLucke) on Feb 23 2022, 1:20 PM.

Details

Reviewers

Hans Goudey (HooglyBoogly)

Maniphest Tasks

T95773: Prototype a grab brush for the hair curves sculpt mode.

Commits

rB226f0c4fef7e: Curves: initial brush implementations for curves sculpt mode

Summary

The main goal here is to add the boilerplate code to make it possible to add the actual sculpt tools more easily. Both brush implementations added by this patch are meant to be prototypes which will be removed or refined in the coming weeks.

Ref T95773.

Diff Detail

Repository: rB Blender

Event Timeline

Jacques Lucke (JacquesLucke) requested review of this revision.Feb 23 2022, 1:20 PM

Jacques Lucke (JacquesLucke) created this revision.

Harbormaster completed remote builds in B20702: Diff 48645.Feb 23 2022, 1:20 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Feb 23 2022, 1:23 PM

Jacques Lucke (JacquesLucke) added a task: T95773: Prototype a grab brush for the hair curves sculpt mode..

cleanup

Harbormaster completed remote builds in B20703: Diff 48646.Feb 23 2022, 1:24 PM

Jacques Lucke (JacquesLucke) added a reviewer: Hans Goudey (HooglyBoogly).Feb 23 2022, 1:25 PM

This looks good to me. I just left some questions and comments inline.

Avoiding commenting about behavior or making brush code more reusable, since I think this is more about getting some initial code included for further iteration.

source/blender/editors/sculpt_paint/curves_sculpt_ops.cc
68	Ideally this would be used by the field evaluator too, for `get_evaluated_as_mask()`. What about `BLI_index_mask_ops.hh`?
73	Extra newline here
113	Don't know your opinion on this, but I find using `.first()` a bit nicer than `[0]`
128–130	I assumed in the past that using a vector in this case over an array would add a bit of overhead. I suppose that must be compiled away?
139	I'm a bit surprised that the nested parallel for loop is worth it, I wouldn't have expected the performance of copying within a single vector to be improved by multithreading. If so (and it probably is so if you added this! :P), it seems like this could be another general utility like `copy_from_threaded` that copies between two spans. Then it could use `slice` too, which would make the copying itself a bit more readable IMO.
194	This could be changed to `curves.curves_range()` now

This revision is now accepted and ready to land.Feb 23 2022, 4:19 PM

Jacques Lucke (JacquesLucke) added inline comments.Feb 23 2022, 4:30 PM

source/blender/editors/sculpt_paint/curves_sculpt_ops.cc
68	Sounds reasonable. Will move it there.
113	Sometimes I use `.first()` when I also use `.last()`, but besides that I don't care too much about which one is used. On that note, I considered adding an int argument to `T last(int64_t n)` to access the nth last element.
128–130	Doesn't really matter here. `Array` is probably a little bit faster in general.
139	Will have to do some more benchmarking here, but I think I measured that this can be faster in the past.