Page MenuHome
Differential D9728

Fix persistent base using invalid data in dyntopo
Needs ReviewPublic
Actions

Authored by Pablo Dobarro (pablodp606) on Dec 3 2020, 12:24 AM.

Details

Reviewers
Sergey Sharybin (sergey)
Summary

When modifying the topology in dyntopo, the persistent base created for
the previous mesh state will become invalid, so it needs to be freed.
In some situations this could crash.

This can be considered again after merging the dyntopo updates. The
persistent base in dyntopo could become a datalayer and preserved during
the topology modifications, like vertex colors and face sets.

Diff Detail

Repository
rB Blender
Branch
fix-persistent-base (branched from master)
Build Status
Buildable 11533
Build 11533: arc lint + arc unit

Event Timeline

Pablo Dobarro (pablodp606) requested review of this revision.Dec 3 2020, 12:24 AM
Pablo Dobarro (pablodp606) created this revision.
Harbormaster completed remote builds in B11533: Diff 31555.Dec 3 2020, 12:25 AM
Pablo Dobarro (pablodp606) added a reviewer: Sergey Sharybin (sergey).Dec 3 2020, 12:25 AM
Pablo Dobarro (pablodp606) edited the summary of this revision. (Show Details)
Pablo Dobarro (pablodp606) mentioned this in D9620: Sculpt: Ensure that persistent base always exist for strokes.Dec 3 2020, 12:30 AM
Sergey Sharybin (sergey) added a comment.Dec 3 2020, 10:23 AM

In some situations this could crash.

What are the steps to reproduce the crash?

This can be considered again after merging the dyntopo updates.

What "this" refers to?

Pablo Dobarro (pablodp606) added a comment.Dec 3 2020, 8:48 PM

What are the steps to reproduce the crash?

If dyntopo splits edges and creates new vertices after creating the persistent base, it is possible to access the persistent base array out of bounds.

What "this" refers to?

Deleting the persistent base before dyntopo changes the topology.

Sergey Sharybin (sergey) added a comment.Dec 4 2020, 10:14 AM

Please put all disambiguations to the patch description (and commit message) as well.

From the behavior here I am not sure this is proper place or thing to do. To me it doesn't make sense to create persistent base which will be tossed away after first stroke iteration. To me it seems that the creation of persistent base is to be forbidden for dynamic topology instead.

Revision Contents
Changeset List

PathSize
source/
blender/
editors/
sculpt_paint/
2 lines

Diff 31555

View Options

source/blender/editors/sculpt_paint/sculpt.c

Loading...