New sculpting brush cursor
Needs ReviewPublic

Authored by Pablo Dobarro (pablodp606) on Aug 11 2018, 1:24 AM.
Tokens
"Love" token, awarded by barracuda."Love" token, awarded by sephirothtbm."Love" token, awarded by julperado."Love" token, awarded by Noss."Mountain of Wealth" token, awarded by Zino."Love" token, awarded by JimMorren."100" token, awarded by shader."Love" token, awarded by JulienKaspar."Like" token, awarded by TheRedWaxPolice."Love" token, awarded by plyczkowski."Love" token, awarded by RyanJEC."Love" token, awarded by jonathanl."Love" token, awarded by johnsyed."Love" token, awarded by eoinoneill.yokai."Love" token, awarded by billreynish."Love" token, awarded by andreasus."Like" token, awarded by Schamph."Love" token, awarded by fiendish55."Love" token, awarded by xrg."Love" token, awarded by amonpaike."Love" token, awarded by KINjO."Love" token, awarded by pablovazquez."Love" token, awarded by miclack."Love" token, awarded by Alrob."Party Time" token, awarded by Scaredyfish."Mountain of Wealth" token, awarded by charlie."Love" token, awarded by wo262."Like" token, awarded by fclem."Burninate" token, awarded by DotBow."Like" token, awarded by erickblender."Like" token, awarded by Samirosman.

Details

Summary

This patch adds a new brush cursor with surface normal and vertex preview.

Know issues:

  • There is no option to disable it.

I've been testing this for a while and I think that using the face normal is always a better option, even with high poly meshes. The vertex normal calculation with big brushes doesn't give you any useful information about the surface. On high poly surfaces with noise, the brush moves a lot if you are using the face normal, but I think that is the expected behavior. For now, both methods are implemented in this patch for testing purposes.

Diff Detail

Pablo Dobarro (pablodp606) edited the summary of this revision. (Show Details)

Now face normals are also working for dyntopo

First, if you can use arcanist to upload your patch.

Then here is my review of the drawing code only. Maybe I overlooked something and that's why you did it this way. Please let me know if I misunderstood something.

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

All those cross_v3 does not depends on 'i'. Put them out of the loop.

1180

You should use GPU_SHADER_3D_UNIFORM_COLOR and use imm_draw_circle_wire_3d instead of doing all of this on the CPU.

Just change the model matrix with GPU_matrix_set but don't forget to use GPU_matrix_push before and GPU_matrix_pop after drawing.

If your concern is to always draw on top of the model, just disable depth testing.

Pablo Dobarro (pablodp606) edited the summary of this revision. (Show Details)

Updating D3594: New sculpting brush cursor

Now drawing code uses openGL matrices for cursor transform.

fixed bug with object transforms

Campbell Barton (campbellbarton) requested changes to this revision.Aug 13 2018, 2:20 PM

While this is nice quality-of-life feature, it's not making anything possible that wasn't possible before and it complicates the code further.
Would rather wait until sculpt in 2.8x is as stable and performant as 2.7x before doing further changes which in turn, need development.

Having said this, there is no issues with preparing this patch and addressing issues so its ready to merge later on.


Functionality review:

  • Showing the dot for the nearest vertex seems unnecessary.
  • This is used when it makes no sense. (grab, 2d falloff, smooth, mask... possibly others)
  • It's not showing the brush at all when not over geometry. I think it should show a circle as before.
This revision now requires changes to proceed.Aug 13 2018, 2:20 PM
source/blender/blenkernel/intern/pbvh.c
1700

Shadows previous i, also use an int, no reason to use smaller data type.

I also think that is better to wait until sculpting in 2.8 is complete, this still needs to be tested with multiresolution support. I'll continue working on this fixing the remaining issues.

Regarding functionality:

  • I think having the vertex preview is more useful than the cursor when sculpting, especially when sculpting base meshes, shape keys or low poly surfaces and with brushes like crease or grab. Seeing the hit point in the mesh on the middle of a face doesn't give you any useful information, you need to see a real reference point where the deformation is going to start.
  • Once sculpting in 2.8 is stable I'll go over all sculpting features to check if the cursor is consistent. Even so, I think giving the option to the user to enable/disable this cursor it's a better option. There are some use cases where you still prefer seeing the surface curvature even though all the affected vertices aren't under the cursor preview, like when using the grab brush near the edge of a model to deform the silhouette. Showing both cursors at the same time it these situations could also be an option.
  • I'll add that back. I also need to set the old cursor as default for the rest of the modes that still don't support surface normal features.

The vertex normal calculation with big brushes doesn't give you any useful information about the surface. On high poly surfaces with noise, the brush moves a lot if you are using the face normal, but I think that is the expected behavior.

It's not clear to me which information is missing from vertex normals and what it is needed for?

If we could use just the face normals that would be simpler and faster, so from that point of view I'm all for it. But I would expect users want to see which direction the sculpting operation will work on?

source/blender/blenkernel/intern/pbvh.c
1697–1699

Replace 3 lines with madd_v3_v3v3fl.

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

What is the purpose of computing the nearest vertex location, rather than using the location where the ray intersects the mesh? The code would be simpler without it, and I don't think the nearest vertex location has any special meaning for sculpting?

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

Code style:

}
else {

I think the main purpose of the cursor should be previewing the position of the brush over the mesh and not previewing the result of the sculpting operation.

When you are working with the lowest subdivision level of a model or when you are tweaking very small details (like mouth corners or fingers) you often work like this:

As you can see, the brush is often smaller than the average face, so using the vertex normal in this situation is not useful.

Furthermore, having the preview point in the middle of the cursor gives you wrong information about the cursor position. A point in a face of the model it's not a valid deformation point and that's why the vertex preview it's hidden when there aren't any vertices inside the area of influence of the brush. In the part where I tweak the eye and face silhouette, the real hit point below the cursor will be in the mesh behind the eye when I start the operation, but the vertex preview shows me that I will be affecting the vertex on the corner of the eye. This still applies to high poly models in any sculpting operation that requires tweaking the silhouette of the model.

Also, if in the future the topograb brush is merged vertex preview it's a must. You need to see which vertex is taken into account for masking the model before starting the operation.

Regarding using the vertex or face normal, I think it's better to always use face normal. In a situation like this:

Using the face normal gives the user a lot more information about the cursor placement over the surface, using the vertex normal with this brush size will no take into account the detail of the nose and it will seem that the surface is flat. I still need to test it with multiresolution and models with high-frequency detail but I think face normal it's more useful in any situation.

Made some requested changes.
I also removed the vertex normal code. Now everything is simpler.

Pablo Dobarro (pablodp606) marked 4 inline comments as done.
Pablo Dobarro (pablodp606) edited the summary of this revision. (Show Details)
  • Added support for murtilresolution.
  • Now the cursor previews symmetry on XYZ axis.
srikanth (Alrob) awarded a token.

Thanks for the update!

The brush system is currently getting an overhaul, this feature would fit perfectly. It's true it doesn't *add* anything new that wasn't possible before (except previewing where the symmetry will happen) but it's one of those quality-of-life usability features that Blender lacks in so many areas. Luckily this is getting tackled with features like this and the new awesome gizmos.

Let's hope now that sculpting tools are more mature in 2.8 and being used in production (Spring) this can get reviewed again.

I'm still doubtful if using the normal of a single face instead of the actual averaged normal that will be used for sculpting is good enough. The example videos are on low poly and smooth surfaces, where this might be ok, but on a more detailed mesh I expect it is quite common to get a preview normal that is totally different.

Some sculptors could test it and give their feedback though.

I don't know if this makes any sense but, shouldn't it be possible to draw the vertex and symmetry cursor as it is now and use a shader to draw the normal cursor using the same normal information that is being used to draw the matcaps?

For reference, zbrush uses flat face normals. not smoothed or vertex normals, but the edges of those face normals seem blurred or averaged on a constant screen space radius. For example, the brush remains flat on a face until you get close to the edge and it starts making a smooth rolling instead of a constant snap as you get to the other face. If faces on screen space are small, it doesn't matter if they are too noisy. But at the right scale of noise on screen, the problem of the brush rocking back and forth still happens. Blender could improve on that or be the same

I really like this change. It should dramatically improve the sculpting experience in Blender!