Page MenuHome

TopologyRake for dyntopo.
ClosedPublic

Authored by Jean Da Costa (jeacom256) on Jan 10 2019, 4:39 PM.
Tokens
"Love" token, awarded by kursadk."Love" token, awarded by zutxita."Love" token, awarded by shomyshomy."Love" token, awarded by charlie."Love" token, awarded by jonathanl."Like" token, awarded by mixmystery."Love" token, awarded by undo."Love" token, awarded by eoinoneill.yokai."Love" token, awarded by michaelknubben."Love" token, awarded by ebarranco."Love" token, awarded by Loner."Love" token, awarded by amonpaike."Love" token, awarded by ace_dragon."Love" token, awarded by Orv510."Love" token, awarded by Arindam."Love" token, awarded by karmacop."Love" token, awarded by erickblender."Yellow Medal" token, awarded by juang3d.

Details

Summary

Thread on BA: https://blenderartists.org/t/custom-feature-toporake/1142166{F6267154}

Some users request a "Quad based dyntopo", although what is actually needed is just better mesh quality.
The fact that dyntopo doesn't care about the brush strokes and just try ensure a target edge length makes it quite efficient. But we can exploit more of its behavior and implement some more complex mesh dynamics.

This adds the capability of aligning edges to the brush strokes when using dyntopo.
Both increase the stroke quality and feature definition from brushes like Clay, Flatten and Crease.
But theoretically any brush could support it, its just a matter of computing the right direction vectors.

Example Crease brush

Example Snake Hook brush

Example Clay Strips brush

Result after cleaning mesh by dissolving verts connected to 4 edges and merging triangles:

Edit:

I've playing around with this patch for a while, I found no bugs or problems so far.
Even not being quite skilled at sculpting, I ended doing a character.

The hair was so much fun!
It makes dyntopo require way less resolution to be able to define details


Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Number of alignment iterations was too low, set to 3.

Jean Da Costa (jeacom256) set the repository for this revision to rB Blender.
  • Improved a bit the performance, by referencing the BMVerts instead of copying the coordinates from it.
  • Variable number of iterations depending on the brush strength, maximum iterations is now 5.

Note: Since the maximum iterations is now higher, the brushes can yield almost instant topology reconfiguration, in most cases, the better strength is around 0.3 to get more control over the flow.

Juan Gea (juang3d) rescinded a token.
Juan Gea (juang3d) awarded a token.

This is a pretty neat feature for sculpting!

I hope this gets reviewed and implemented sooner than later :)

Yeah, I was willing to implement this for a while, finally I could, lets wait to see if comes a review.

Yeah, I was willing to implement this for a while, finally I could, lets wait to see if comes a review.

Can you provide a build?

  • Direction estimation now uses sculpt_normal rather than just screen space movement.
  • Removed unused if statement.

Yeah, I was willing to implement this for a while, finally I could, lets wait to see if comes a review.

Can you provide a build?

I can provide a 64 bit windows build but my internet is quite slow, I might take a while to upload it.
I will post on the BA thread once its done.

Jean Da Costa (jeacom256) edited the summary of this revision. (Show Details)
Brecht Van Lommel (brecht) requested changes to this revision.Jan 13 2019, 5:25 PM

Functionality seems useful.

The tooltip could get a bit more information about the performance impact and what this is useful for exactly.

release/scripts/startup/bl_ui/space_view3d_toolbar.py
275–278

Maybe just hide the button if context.sculpt_object.use_dynamic_topology_sculpting is False as well? Not sure why it needs to be greyed out then.

source/blender/editors/sculpt_paint/sculpt.c
1602–1604

paralell -> parallel

For the patch overall, please follow comment code style:
https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Comments

1611

Would change the test to this, and avoid the indent for the main case.

if (vfcount < 2) {
    copy_v3_v3(avg, v->co);
    return;
}
1625–1626

ocasionally -> occasionally, requeriments -> requirements, soothing -> smoothing

1634

This is not a code style we use, repeat float instead.

1650

is the edge -> the edge is

1815

Avoid unrelated whitespace changes.

1868

Cancell -> cancel

2154

smooth -> topology rake

Is this check needed at all though? This is not using ss->pmap as far as I can tell.

3875–3876

This seems inconsistent with rna_SculptToolCapabilities_has_topology_rake_get? Under which conditions is this supposed to run exactly?

4634

Add break;

source/blender/makesrna/intern/rna_brush.c
185

suppots -> supports

188

Why not use SCULPT_TOOL_HAS_DYNTOPO() here?

1552

Is the abbreviation necessary, can we just use topology_rake_factor and Topology Rake for clarity?

dyntopo -> dynamic topology

This revision now requires changes to proceed.Jan 13 2019, 5:25 PM
Jean Da Costa (jeacom256) marked 2 inline comments as done.
Jean Da Costa (jeacom256) marked 14 inline comments as done.

Done changes.

last diff was corrupted.

LazyDodo (LazyDodo) added inline comments.
source/blender/editors/sculpt_paint/sculpt.c
1868

looks like you missed this one.

Jean Da Costa (jeacom256) marked an inline comment as done.

looks like you missed this one.

Oops!

Martin Capitanio (capnm) added inline comments.
source/blender/editors/sculpt_paint/sculpt.c
2150

This doesn't compile (gcc) -> max_ff, ?

Jean Da Costa (jeacom256) marked an inline comment as done.

Does it compile now?

Does it compile now?

Yes 8-)

It compiles (without warnings) with implicit conversions as well:

	int iteration;
	const int count = iterations * bstrength + 1;
	const float factor = iterations * bstrength / count;

git apply D4189.diff printed out some whitespace warnings

D4189.diff:8: trailing whitespace.
D4189.diff:14: trailing whitespace.
D4189.diff:18: trailing whitespace.
D4189.diff:72: trailing whitespace.
D4189.diff:151: trailing whitespace.
warning: 5 lines add whitespace errors.

Removed some unnecessary vector normalizations.
I guess there's not much to do to improve this.

@Brecht Van Lommel (brecht), Is it fine now?

I missed to fix the speeling from one comment... again ><

@Campbell Barton (campbellbarton), @Brecht Van Lommel (brecht)
sorry annoying you guys, I wish I could get a second review on this. This is not such a big feature but I really believe it could make blender sculpting more competitive.
For some reason, some reviews neither get rejected nor approved, just stay stagnated.

We are in beta phase, focus is not on new features but stability. So there's things with higher priority and it can take a while to get to this.

I have only made a few formatting adjustments and rebase to master. Do whatever you want with it :-)

I'll commit this with some internal code tweaks.

This revision is now accepted and ready to land.Jan 23 2019, 8:30 PM
This revision was automatically updated to reflect the committed changes.