Details

Reviewers

Maniphest Tasks

T101203: Regression: Issues with undoing the visibility of face sets in sculpt mode

Commits

rBa777c09d5f86: Sculpt: Standardize face set undo steps, optimize memory usage
rBdf788ecfd95f: Sculpt: Standardize face set undo steps, optimize memory usage

Summary

Currently the face set of every single face is saved for every sculpt undo step.
When only changing the face sets of a small section of the mesh, this can be quite
wasteful. It also makes face sets a special case compare to all other sculpt undo step
types, which makes the whole system more complex and harder to improve.

This aims to fix T101203, though currently the patch introduces more glitches than it solves.

Diff Detail

Repository: rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Oct 11 2022, 10:56 PM

Hans Goudey (HooglyBoogly) created this revision.

Joseph Eagar (joeedh) commandeered this revision.Oct 11 2022, 11:21 PM

Joseph Eagar (joeedh) edited reviewers, added: Hans Goudey (HooglyBoogly); removed: Joseph Eagar (joeedh).

Use new face set iterators.

Torrent (Torrent) awarded a token.Oct 12 2022, 1:25 AM

Torrent (Torrent) added a subscriber: Torrent (Torrent).

Hans Goudey (HooglyBoogly) added a parent revision: D16225: Sculpt face iterators.Oct 14 2022, 8:15 PM

Fix multires crash.

Harbormaster completed remote builds in B24268: Diff 56826.Oct 15 2022, 7:46 AM

Merge with latest master

Harbormaster completed remote builds in B24528: Diff 57334.Nov 7 2022, 7:45 PM

Joe pointed out that before this patch can land we need to review and commit D16225: Sculpt face iterators.

In D16224#447353, @Julien Kaspar (JulienKaspar) wrote:

Joe pointed out that before this patch can land we need to review and commit D16225: Sculpt face iterators.

Hey @Julien Kaspar (JulienKaspar), in case you didn't know about this, you can see this information as part of the diff in the "Stack" view:

@Hans Goudey (HooglyBoogly) I did not! Thanks for pointing it out.

Refactor BKE_pbvh_num_prims to BKE_pbvh_num_faces. Remember that "faces" in sculpt mode always refers to base mesh faces, not loop tris or grids.
Fix undo for box face sets.
Added SCULPT_face_set_set and SCULPT_face_set_get API methods.

Harbormaster completed remote builds in B24752: Diff 57853.Nov 20 2022, 6:13 PM

The code looks fine to me now, just a few small comments inline.

source/blender/blenkernel/intern/pbvh.c
2141–2143	Tiny thing, but IMO this white space (new lines) isn't helpful here
source/blender/editors/sculpt_paint/sculpt.cc
3363 ↗	(On Diff #57853)	What about `BKE_pbvh_node_mark_update_face_sets` here?
source/blender/editors/sculpt_paint/sculpt_undo.c
937–939	Unnecessary change here
1054	Unnecessary newline here
2121	Unnecessary change

This revision is now accepted and ready to land.Nov 20 2022, 6:31 PM

Code cleanup.

Harbormaster completed remote builds in B24765: Diff 57889.Nov 21 2022, 8:01 PM

Sync with latest master

Harbormaster completed remote builds in B24787: Diff 57922.Nov 22 2022, 8:19 PM

Closed by commit rBdf788ecfd95f: Sculpt: Standardize face set undo steps, optimize memory usage (authored by Joseph Eagar (joeedh)). · Explain WhyNov 22 2022, 9:15 PM

This revision was automatically updated to reflect the committed changes.

Joseph Eagar (joeedh) added a commit: rBdf788ecfd95f: Sculpt: Standardize face set undo steps, optimize memory usage.

Joseph Eagar (joeedh) added a commit: rBa777c09d5f86: Sculpt: Standardize face set undo steps, optimize memory usage.

Sculpt: Standardize face set undo steps, optimize memory usage
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 57928

.clang-format

source/blender/blenkernel/BKE_pbvh.h

source/blender/blenkernel/intern/pbvh.c

source/blender/blenkernel/intern/pbvh_intern.h

source/blender/editors/sculpt_paint/paint_mask.c

source/blender/editors/sculpt_paint/sculpt.c

source/blender/editors/sculpt_paint/sculpt_automasking.cc

source/blender/editors/sculpt_paint/sculpt_expand.c

source/blender/editors/sculpt_paint/sculpt_face_set.cc

source/blender/editors/sculpt_paint/sculpt_intern.h

source/blender/editors/sculpt_paint/sculpt_mask_expand.c

source/blender/editors/sculpt_paint/sculpt_undo.c

Sculpt: Standardize face set undo steps, optimize memory usageClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 57928

.clang-format

source/blender/blenkernel/BKE_pbvh.h

source/blender/blenkernel/intern/pbvh.c

source/blender/blenkernel/intern/pbvh_intern.h

source/blender/editors/sculpt_paint/paint_mask.c

source/blender/editors/sculpt_paint/sculpt.c

source/blender/editors/sculpt_paint/sculpt_automasking.cc

source/blender/editors/sculpt_paint/sculpt_expand.c

source/blender/editors/sculpt_paint/sculpt_face_set.cc

source/blender/editors/sculpt_paint/sculpt_intern.h

source/blender/editors/sculpt_paint/sculpt_mask_expand.c

source/blender/editors/sculpt_paint/sculpt_undo.c

Sculpt: Standardize face set undo steps, optimize memory usage
ClosedPublic
Actions

Revision Contents
Changeset List