Page MenuHome

Sculpt: Mesh filter tool
ClosedPublic

Authored by Pablo Dobarro (pablodp606) on Aug 17 2019, 6:00 PM.
Tags
None
Tokens
"Yellow Medal" token, awarded by zutxita."Love" token, awarded by johnsyed."Like" token, awarded by michaelknubben."Like" token, awarded by amonpaike."Like" token, awarded by brilliant_ape.

Details

Summary

Mesh filter tool from the sculpt branch. I also added support for locking deformation axis.


I included some code from D3594 to try it without crashing.
It should work with regular meshes, dyntopo, EEVEE and modifiers enabled. The mode where masked nodes are filtered works only when EEVEE and modifiers are disabled.

Diff Detail

Repository
rB Blender

Event Timeline

Pablo Dobarro (pablodp606) updated this revision to Diff 17643.
  • Fix shape key compatibility, fix performance issue
  • Fix warnings, fix bug with invalid original normals
Brecht Van Lommel (brecht) requested changes to this revision.
Brecht Van Lommel (brecht) added inline comments.
release/scripts/startup/bl_ui/space_toolsystem_toolbar.py
1003

This icon will need to be created.

source/blender/editors/sculpt_paint/sculpt.c
7286–7293

It's not obvious to me why this tool can't work similar to the grab brush, which uses sculpt_orig_vert_data_init rather than allocating its own array for original coordinates and normals.

Is there a specific reason for that?

7487

You can use something like BLI_hash_int_01(vd.index ^ random_seed) rather than a table with a fixed amount of random numbers.

Just using modulo can lead to visible patterns depending on the mesh topology.

7518

Better to remove this here and use bool use_pbvh_draw = BKE_sculptsession_use_pbvh_draw(ob, v3d); below.

In general initializing immediately avoids use of uninitialized variables.

7533–7536

These 4 lines could be wrapped into a sculpt_vertex_random_access_init function as part of the abstraction API.

Is BM_mesh_elem_table_ensure needed?

7541

This shouldn't happen as far as I know, was this to fix an actual issue?

7547

Why does this depend on the usage of PBVH drawing? Does the PBVH get destroyed and rebuilt in that case?

These are not obviously related, so that makes this test prone to break in the future or may not cover all cases already.

7592–7595

Same comment as above.

7598

true -> false

There is no need to create a mask for this tool, it only needs to use it if it's already there.

7640–7642

Instead of 3 separate booleans, define this as an array like e.g. sculpt symmetry mirror axes.

This revision now requires changes to proceed.Thu, Aug 29, 7:05 PM
Pablo Dobarro (pablodp606) marked 9 inline comments as done.Fri, Aug 30, 12:49 AM
Pablo Dobarro (pablodp606) updated this revision to Diff 17674.
  • Review update

When I did the first version of this tool something was rebuilding the PBVH when EEVEE was enabled, so the nodes in the cache were no longer valid. Also, the ss->pbvh was NULL randomly for a few seconds after saving the file or after changing the viewport render mode. I can't reproduce that now, so I removed all those checks.

Brecht Van Lommel (brecht) requested changes to this revision.Fri, Sep 6, 3:15 PM

The "Sphere" option seems to be broken in this revision, just trying with a simple Monkey mesh. It seems to scale down to a point or something?

Otherwise looks good to me.

For the icon, can you create this? Or do you need help from @William Reynish (billreynish)?

release/scripts/startup/bl_ui/space_toolsystem_toolbar.py
991–996

This layout looks bad in the tool settings tab in the properties editor. I think you can just do:

props = tool.operator_properties("sculpt.mesh_filter")
layout.prop(props, "type", expand=False)
layout.prop(props, "strength")
layout.prop(props, "deform_axis")
This revision now requires changes to proceed.Fri, Sep 6, 3:15 PM
  • Fix layout, fix sphere mesh filter, rebase

I don't think I can create icons that look decent, maybe we should leave that to @William Reynish (billreynish). We should probably make a list of all the icons that are needed for the new tools and options.

Don't worry about the icon - I can easily help with that after a merge. There's a certain complicated process needed to make and add them. I will gladly add any required icons before release, with help from @Andrzej Ambroz (jendrzych) and Aslam Cader.

The lack of an icon should not block these kinds of patches IMO.

This revision is now accepted and ready to land.Fri, Sep 6, 6:01 PM

Ok, we can commit these tools without icons.

This revision was automatically updated to reflect the committed changes.