Page MenuHome

Sculpt: Pose brush origin offset
ClosedPublic

Authored by Pablo Dobarro (pablodp606) on Wed, Sep 18, 4:28 PM.
Tags
None
Tokens
"Love" token, awarded by brilliant_ape."Love" token, awarded by ThomasJohann."100" token, awarded by Frozen_Death_Knight."Like" token, awarded by erickblender.

Details

Summary

With the previous behavior, it was impossible to manipulate areas with a lot of complex shapes like fingers, as the pose origin was calculated only with the topology inside the radius.
With pose offset, the previous method is used to calculate the direction of the "bone", and an extra offset is added on top of it. This way you can set the pose origin in the correct place in this kind of situations. The pose factor grows to fit the new rotation origin.

Diff Detail

Repository
rB Blender

Event Timeline

release/scripts/startup/bl_ui/space_view3d_toolbar.py
427

Should be elif

source/blender/editors/sculpt_paint/sculpt.c
3609

Note for future investigation: Check if spinlock would fit better.

3792

Use madd_v3_v3fl

3850

This is still part of calculating the pose_factor. should be called from inside the sculpt_pose_calc_pose_data
but put this in its own function as it is isolated and complex.

Need additional look in how to make this faster. It seems like in large meshes there is a lot of overhead towards checking nodes that are actually not interested and also have not been interested in the previous iteration.

Have you checked how many iterations you typically get and how many hits vs misses?

3852

grow_next_iteration?

3860

Can this be done outside the while loop?

Pablo Dobarro (pablodp606) marked 6 inline comments as done.
  • Review update

This brush is intended to be used in the lower levels of a multires or a low poly mesh. I often get 4/5 iterations and I didn't notice any performance difference.
Iterating over all nodes is an issue in the mask filter. The code is almost the same, but if you are working in a high poly mesh you often want to do about 100 iterations in one step. The only optimization I'm doing is to only tag to redraw nodes where the mask data changed.
The big bottleneck in sculpt mode happens when you tag to redraw a lot of nodes at once, so just by doing this performance is not that bad.

Jeroen Bakker (jbakker) requested changes to this revision.Fri, Sep 20, 1:06 PM
Jeroen Bakker (jbakker) added inline comments.
source/blender/editors/sculpt_paint/sculpt.c
3645

Move the init and end of the mutex to outside the while loop.

3820

This is not safe! 50% of the reads are using the new values. reading and updating values in the same loop.

ss->cache->pose_factor[vd.index] is uninitialized when total is 0, should it be set to 0?

This revision now requires changes to proceed.Fri, Sep 20, 1:06 PM
Pablo Dobarro (pablodp606) marked 2 inline comments as done.
  • Rebase
  • Move mutex out of the while loop
Jeroen Bakker (jbakker) requested changes to this revision.Tue, Sep 24, 8:23 AM
Jeroen Bakker (jbakker) added inline comments.
release/scripts/startup/bl_ui/space_view3d_toolbar.py
427

I don't get it :-) Should still be done.

source/blender/editors/sculpt_paint/sculpt_intern.h
220

This code is unused, please remove.

This revision now requires changes to proceed.Tue, Sep 24, 8:23 AM
source/blender/editors/sculpt_paint/sculpt.c
3577–3584

In stead of the mutex we can use the tls parameter and store there the accumulations that this thread has handled.
When finished we can use a TaskParallelSettings.func_finalize to accumulate over all the sub results.

Pablo Dobarro (pablodp606) marked 3 inline comments as done.
  • Use func_finalize instead of mutex
Jeroen Bakker (jbakker) requested changes to this revision.Wed, Sep 25, 10:14 AM
Jeroen Bakker (jbakker) added inline comments.
source/blender/editors/sculpt_paint/sculpt.c
3592

This resets the count + average when a worker/thread is reused.

3605

'gftd[n].pos_avg' => gftd->pos_avg

the userdata_chunk is already localized

3619

This method is called per task, no need to loop.

3642

No need to allocate for every task here. this is done in task.c#1227-1235
Also no need to use calloc. A local variable is sufficient as it is used as template.

3647

only the size of a single element needed.

This revision now requires changes to proceed.Wed, Sep 25, 10:14 AM
Pablo Dobarro (pablodp606) marked 5 inline comments as done.
  • Fix finalize implementation
Jeroen Bakker (jbakker) requested changes to this revision.Wed, Sep 25, 4:43 PM

Just a final thing! great work. Also check if clang is configured correctly.

This revision now requires changes to proceed.Wed, Sep 25, 4:43 PM
  • Rebase
  • Reset gftd on each iteration
Jeroen Bakker (jbakker) requested changes to this revision.Fri, Sep 27, 5:14 PM

Seems to be that rebasing introduced more changes...

source/blender/editors/sculpt_paint/paint_cursor.c
1355

prev_active_vertex_index can be read uninitialized. This is undefined behavior, best set to a useful default.

This revision now requires changes to proceed.Fri, Sep 27, 5:14 PM
Pablo Dobarro (pablodp606) marked an inline comment as done.
  • Init prev_active_vertex_index

Before commit please initialize to -1; 0 is an actual index.

This revision is now accepted and ready to land.Fri, Sep 27, 5:35 PM
This revision was automatically updated to reflect the committed changes.