Page MenuHome

Fix Pose Brush origin position with large brush size
ClosedPublic

Authored by Pablo Dobarro (pablodp606) on Mon, May 18, 3:31 PM.

Details

Summary

When the brush size is bigger than the entire mesh, fdata.tot_co can be
0, so the pose origin will default to (0,0,0), which does not make much
sense. After this patch, the pose origin will be set to the farthest
vertex from the pose origin, which at least should be in the surface of
the mesh and in most cases in the direction the pose brush was already
detecting the origin.

Diff Detail

Repository
rB Blender

Event Timeline

Pablo Dobarro (pablodp606) requested review of this revision.Mon, May 18, 3:31 PM
Pablo Dobarro (pablodp606) created this revision.
Sergey Sharybin (sergey) requested changes to this revision.Tue, May 19, 10:34 AM
Sergey Sharybin (sergey) added inline comments.
source/blender/editors/sculpt_paint/sculpt_pose.c
376–377

Re-using same field depending on a context is always confusing and error-prone.

It doesn't seem to have any measurable memory footprint if you will add an explicit furthest-away point.

390–395

I don't see initialization of fallback_origin, so it will be {0, 0, 0} here. Depending on the model it might cause very wrong comparison.

It doesn't seem to be any other way than to store an explicit state whether you know furthest away point or not.

This revision now requires changes to proceed.Tue, May 19, 10:34 AM
Pablo Dobarro (pablodp606) marked 2 inline comments as done.
  • Review update

I think this should work. If I initialize the fallback origin to the location the floodfill starts, any vertex in the floodfill should be further, so this should always initialize correctly.

If the initial stroke was done outside of the mesh and brush did not cover any of the mesh vertices then fallback_floodfill_origin will not be a point on a surface. If that's not a problem then initializing to initial location is indeed fine.

The stroke always starts when the cursor is over the mesh, so I think it should be fine

This revision is now accepted and ready to land.Mon, May 25, 10:54 AM