Details

Reviewers

Hans Goudey (HooglyBoogly)

Maniphest Tasks

T96445: Comb brush - curve sculpting

Commits

rBd67f9820b8f8: Curves: improve Comb brush

Summary

New supported features:

3D/spherical brush.
Falloff (custom falloff not yet, because that needs some more ui reorganization which is better done when we have a better understanding of what settings we need exacty).

Currently, the depth of the brush is only sampled once per stroke (when first pressing the mouse button). Sometimes it is expected that the depth of the brush can change. However, implementing that in a good way is not straight forward and might need additional options. Therefore, this will be handled separately (talked about this with @Simon Thommes (simonthommes)). I already did some experiments anyway, which I'll mention here for future reference.
Simply resampling the distance all the time, somewhat works, not super well when the brush is actually changing the "depth".

Here it is not clear whether one wants to brush up or away from the camera.

Diff Detail

Repository

rB Blender

Branch

comb-brush (branched from master)

Build Status

Buildable 21096
Build 21096: arc lint + arc unit

Event Timeline

Jacques Lucke (JacquesLucke) requested review of this revision.Mar 18 2022, 3:22 PM

Jacques Lucke (JacquesLucke) created this revision.

Harbormaster completed remote builds in B21096: Diff 49399.Mar 18 2022, 3:22 PM

Jacques Lucke (JacquesLucke) added a task: T96445: Comb brush - curve sculpting.Mar 18 2022, 3:23 PM

Merge branch 'master' into comb-brush
improve

Harbormaster completed remote builds in B21138: Diff 49472.Mar 21 2022, 1:12 PM

cleanup

Harbormaster completed remote builds in B21139: Diff 49473.Mar 21 2022, 1:17 PM

Jacques Lucke (JacquesLucke) retitled this revision from Curves: Improve Comb brush. (WIP) to Curves: Improve Comb brush..Mar 21 2022, 1:23 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)

Jacques Lucke (JacquesLucke) added a reviewer: Hans Goudey (HooglyBoogly).

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Mar 21 2022, 3:06 PM

Paul Larson (GeorgiaPacific) added a subscriber: Paul Larson (GeorgiaPacific).Mar 21 2022, 5:53 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Mar 21 2022, 6:17 PM

Jacques Lucke (JacquesLucke) added a subscriber: Simon Thommes (simonthommes).

Merge branch 'master' into comb-brush

Harbormaster completed remote builds in B21161: Diff 49517.Mar 22 2022, 9:45 AM

Jacques Lucke (JacquesLucke) added a task: T96448: Snake Hook - curve sculpting.Mar 22 2022, 10:11 AM

Jacques Lucke (JacquesLucke) removed a task: T96448: Snake Hook - curve sculpting.

Merge branch 'master' into comb-brush
extract more of 3d brush sampling into separate file

Harbormaster completed remote builds in B21167: Diff 49526.Mar 22 2022, 12:29 PM

Could the 3D brush utility be used in the add brush too?

The code here is looking good. I have a few smaller suggestions inline, and some random comments.

source/blender/blenlib/BLI_math_geom.h
326–329	Add a `\return` item to the docstring This could be a separate commit Might it make sense to change the 3D version too?
source/blender/editors/sculpt_paint/curves_sculpt_3d_brush.cc
30 ↗	(On Diff #49526)	"under a screen position" maybe?
81 ↗	(On Diff #49526)	Isn't this basically a `parallel_reduce` pattern? It might be nice to write it that way then, instead of this more manual approach.
85 ↗	(On Diff #49526)	`points_range` -> `points`, for consistency
89 ↗	(On Diff #49526)	I suggest moving the comment about naming standards to `curves_sculpt_intern.hh` since it's used in multiple files.
130 ↗	(On Diff #49526)	I know it will require the `&` operator more, but I'd keep it simple and pass `C` as a reference, since it's expect to not be NULL
source/blender/editors/sculpt_paint/curves_sculpt_comb.cc
180–188	I do wonder how it would be to write functions like this as a field network. That would be a really interesting way to open up more optimization opportunities in the future. For example, optimizing for SIMD wouldn't require changing code in so many places, just at the functions used by the field network, which would be a relatively small set of common functions. Obviously not for this patch, but I do have a feeling that it would be good to investigate that in the medium term.

This revision now requires changes to proceed.Mar 22 2022, 10:39 PM

Could the 3D brush utility be used in the add brush too?

Depends on what you want it to do. The add brush samples the surface, while this new utility samples the curves.

source/blender/editors/sculpt_paint/curves_sculpt_3d_brush.cc
89 ↗	(On Diff #49526)	I think until this is more formalized, I rather have the comment in every file to make it easier for readers to stumble over it.
source/blender/editors/sculpt_paint/curves_sculpt_comb.cc
180–188	Yeah, I don't know exactly yet. I think a good next step to make this more reusable would be to find a better abstraction for transforming coordinates between 3d and screen space. One that does not have these dependencies like `ARegion` etc.