Normal Based Automasking Modes
ClosedPublic
Actions

Authored by Joseph Eagar (joeedh) on Jun 27 2022, 3:35 AM.

Details

Reviewers

Julien Kaspar (JulienKaspar)
Jeroen Bakker (jbakker)

Maniphest Tasks

T99983: Store PBVH GPU Batches In Intermediate Nodes
T98593: Paint Mode: Backface Occlusion

Commits

rBbbc69563d070: Sculpt: Normal-based automasking modes

Summary

This patch implements two new automasking modes based on vertex normals. Each mode has an associated angular limit and a falloff.

Brush Normal

Compares vertex normals with the initial normal at the beginning of the brush stroke.

View Normal

Compares vertex normals with the view normal. If "occlusion" is on then rays will be shot from each vertex to test if it is occluded by other geometry (note: this can be very slow). Only geometry inside the sculpt mesh is considered.

Diff Detail

Repository

rB Blender

Branch

temp-sculpt-normals-masking

Build Status

Buildable 22702
Build 22702: arc lint + arc unit

Event Timeline

Joseph Eagar (joeedh) requested review of this revision.Jun 27 2022, 3:35 AM

Joseph Eagar (joeedh) created this revision.

Harbormaster completed remote builds in B22702: Diff 52919.Jun 27 2022, 3:35 AM

Fix compiler error.

Harbormaster completed remote builds in B22704: Diff 52924.Jun 27 2022, 7:35 AM

Julien Kaspar (JulienKaspar) added a task: T98593: Paint Mode: Backface Occlusion.Jun 27 2022, 10:10 AM

Some initial notes:

We should rename the auto-masking options to "Area Normals" and "View Normals" to follow current naming conventions and be more accurate about their functionality
If you try to use the filters Blender crashes
They don't work with any of the sculpting brushes. Only the paint brushes.
The automasking options are missing from the pie menu, and also the brush settings

A big functionality that is missing is actual occlusion masking. The View Normals auto-masking should get an additional setting to mask off any faces that are not visible.

The challenge here might then be to still allow a normal falloff past 90 degrees? We might need to discuss this further.

This revision now requires changes to proceed.Jun 27 2022, 11:32 AM

Update with latest master.

Harbormaster completed remote builds in B22715: Diff 52951.Jun 27 2022, 8:30 PM

Changed name 'Occlusion' to 'View Normal'
Changed name 'Initial Normal' to 'Area Normal'
Added missing RNA for brush automasking.
Added normals automasking modes to brush advanced panel.

Harbormaster completed remote builds in B22758: Diff 53047.Jun 29 2022, 8:45 AM

Automasking code now has access to original normals. This is necassary to avoid exploding mesh artifacts.

This works in a fairly clunky way that preserves the existing automasking API which gets the original
co/normals from the undo data stored per PBVHNode (except for PBVH_BMESH which is a vertex-level lookup).
An alternative is to rewrite the original co/normal API itself to always use vertex indices and not use the node undo data at all. That would be significantly simpler at a modest cost of extra memory use.

Harbormaster completed remote builds in B22932: Diff 53506.Jul 11 2022, 12:38 PM

Fix crash.

Harbormaster completed remote builds in B22933: Diff 53507.Jul 11 2022, 12:42 PM

Merge with master

Harbormaster completed remote builds in B23030: Diff 53720.Jul 16 2022, 1:16 AM

Support normal automasking modes in sculpt filters:

Sculpt filters now sample the area normal on first click.
Small cleanup: there is now a utility function for defining common filter operator properties, SCULPT_mesh_filter_properties.
Filters now update the last location (used for e.g. view centering).

Harbormaster completed remote builds in B23035: Diff 53738.Jul 17 2022, 12:10 AM

Joseph Eagar (joeedh) added a task: T99983: Store PBVH GPU Batches In Intermediate Nodes.Jul 25 2022, 10:47 PM

Add occlusion mode

Harbormaster completed remote builds in B23139: Diff 54029.Jul 25 2022, 10:51 PM

Revert pbvh leaf limit change.

Harbormaster completed remote builds in B23140: Diff 54030.Jul 25 2022, 10:53 PM

Implement new automasking modes for factor cache mode.

Harbormaster completed remote builds in B23172: Diff 54089.Jul 27 2022, 12:10 AM

I was testing this patch a bit and we discussed this further in the module meeting.
Some notes on what we should do:

Expose "Area Normal" auto-masking option in the pie menu instead of "Occlusion"
Expose Occlusion auto-masking as a sub-setting of "View Normal" auto-masking. If occlusion is toggled it should override the behavior of view normal auto-masking then.
Occlusion auto-masking needs performance optimizations
Occlusion auto-masking needs settings to achieve a less harsh occlusion border
- This could be achieved by adding an addition blurring propagation slider like with the cavity auto-masking patch
- A more desirable solution would be to use the existing Limit and Falloff settings of the View Normal settings to shift the occlusion border and add a soft falloff.

This revision now requires changes to proceed.Jul 28 2022, 2:33 PM

Merge with master.
Make occlusion a suboption of view normal.
Fix bug in automasking factor mode.

Harbormaster completed remote builds in B23213: Diff 54209.Jul 30 2022, 8:33 PM

Remove unwanted formatting changes.

Harbormaster completed remote builds in B23214: Diff 54210.Jul 30 2022, 8:35 PM

Ruslan (Loner) added a subscriber: Ruslan (Loner).Aug 1 2022, 5:19 PM

Ruslan (Loner) removed a subscriber: Ruslan (Loner).

I marked some previous notes as done. Some new issues have been introduced:

Face Set Boundary auto-masking has disappeared from the options panel
Face Set Boundary auto-masking has been replaced in the pie menu by Area Normal auto-masking
The Occlusion sub-toggle is in Area Normal auto-masking of the brush settings instead of the View Normal auto-masking

This revision now requires changes to proceed.Aug 17 2022, 2:24 PM

I also suggest some changes and additions to the tooltips.

source/blender/makesrna/intern/rna_brush.c
3177 ↗	(On Diff #54210)	For consistency and clarity to the user we should change the tooltip to: "Affect only vertices with a similar normal to where the stroke starts."
3185 ↗	(On Diff #54210)	This should also follow other tooltip phrasing: "Affect only vertices with a normal that faces the viewer."
3190 ↗	(On Diff #54210)	We should make this tooltip more clear: "Only affect vertices that are not occluded by other faces. (Slower performance)"
source/blender/makesrna/intern/rna_sculpt_paint.c
909	Here as well: "Affect only vertices with a similar normal to where the stroke starts."
917	Here as well: "Affect only vertices with a normal that faces the viewer."
922	Same here: "Only affect vertices that are not occluded by other faces. (Slower performance)"
931	This could also use a simple tooltip: "The range of angles that will be affected."
935	Same here: "The range of angles that will be affected."
940	A tooltip here as well: "Extend the range of angles with a falloff gradient."
944	Same here: "Extend the range of angles with a falloff gradient."

Joseph Eagar (joeedh) updated this revision to Diff 56028.Sep 22 2022, 8:42 PM

Harbormaster completed remote builds in B23893: Diff 56028.Sep 22 2022, 8:42 PM

Update with latest master.

Harbormaster completed remote builds in B23917: Diff 56080.Sep 24 2022, 7:09 PM

Please change the description of this task See https://wiki.blender.org/wiki/Style_Guide/Commit_Messages#Commit_Types for more information what information to add.

Code looks fine by me. If there are big changes I would like to review it again.

source/blender/editors/sculpt_paint/sculpt_transform.c
70 ↗	(On Diff #56080)	Is this a future task or would it be part of this task and should still be implemented. Future enhancements can better be added as a ticket