[Bf-blender-cvs] [d29a720c45e] master: Fix T84002: Sculpt: Masking operations crash if multires is in play.
Bastien Montagne
noreply at git.blender.org
Wed Dec 23 16:06:23 CET 2020
Commit: d29a720c45e55047e4f0e02df4652ba03a49c528
Author: Bastien Montagne
Date: Wed Dec 23 12:19:45 2020 +0100
Branches: master
https://developer.blender.org/rBd29a720c45e55047e4f0e02df4652ba03a49c528
Fix T84002: Sculpt: Masking operations crash if multires is in play.
This fixes the main issue there (essentially a followup to
rB90e12e823ff0: Fix T81854: crash when undoing switch between sculpt and
edit mode).
We basically remove more (hopefully all the remaining!) modifications of
orig mesh from `sculpt_update_object`, as those done here will not be
immediately available in the evaluated data (that specific bug happened
because masking data was added to orig mesh there, but not flushed to
depsgraph evaluated one).
This also goes towards a better separation between handling of evaluated
data and orig one.
Note that modification of orig mesh data can still happen, e.g. values
in some cdlayers, but at least all pointers should now be valid in the
evaluated mesh.
There are still some issues, e.g. we now get an assert/crash in
`multires_reshape_assign_final_coords_from_ccg` when undoing out of the
Sculpt mode, presumably because subdiv_ccg data remains unchanged then
(and hence still has the `has_mask` flag set), while actual mesh data do
not have that cdlayer anymore...
This commit also cleans up/simplifies some code,
`ED_object_sculptmode_enter_ex` was (indirectly) calling
`BKE_sculpt_face_sets_ensure_from_base_mesh_visibility` twice e.g.
===================================================================
M source/blender/blenkernel/BKE_paint.h
M source/blender/blenkernel/intern/paint.c
M source/blender/editors/sculpt_paint/sculpt.c
===================================================================
diff --git a/source/blender/blenkernel/BKE_paint.h b/source/blender/blenkernel/BKE_paint.h
index aaed2649ad9..bd2ac3da21b 100644
--- a/source/blender/blenkernel/BKE_paint.h
+++ b/source/blender/blenkernel/BKE_paint.h
@@ -635,9 +635,6 @@ void BKE_sculpt_sync_face_sets_visibility_to_base_mesh(struct Mesh *mesh);
void BKE_sculpt_sync_face_sets_visibility_to_grids(struct Mesh *mesh,
struct SubdivCCG *subdiv_ccg);
-/* Ensures that a Face Set data-layers exists. If it does not, it creates one respecting the
- * visibility stored in the vertices of the mesh. If it does, it copies the visibility from the
- * mesh to the Face Sets. */
void BKE_sculpt_face_sets_ensure_from_base_mesh_visibility(struct Mesh *mesh);
bool BKE_sculptsession_use_pbvh_draw(const struct Object *ob, const struct View3D *v3d);
diff --git a/source/blender/blenkernel/intern/paint.c b/source/blender/blenkernel/intern/paint.c
index d726a4b1e37..7c2fc40d537 100644
--- a/source/blender/blenkernel/intern/paint.c
+++ b/source/blender/blenkernel/intern/paint.c
@@ -1596,7 +1596,7 @@ static void sculpt_update_object(Depsgraph *depsgraph,
Scene *scene = DEG_get_input_scene(depsgraph);
Sculpt *sd = scene->toolsettings->sculpt;
SculptSession *ss = ob->sculpt;
- Mesh *me = BKE_object_get_original_mesh(ob);
+ const Mesh *me = BKE_object_get_original_mesh(ob);
MultiresModifierData *mmd = BKE_sculpt_multires_active(scene, ob);
const bool use_face_sets = (ob->mode & OB_MODE_SCULPT) != 0;
@@ -1612,20 +1612,13 @@ static void sculpt_update_object(Depsgraph *depsgraph,
if (need_mask) {
if (mmd == NULL) {
- if (!CustomData_has_layer(&me->vdata, CD_PAINT_MASK)) {
- BKE_sculpt_mask_layers_ensure(ob, NULL);
- }
+ BLI_assert(CustomData_has_layer(&me->vdata, CD_PAINT_MASK));
}
else {
- if (!CustomData_has_layer(&me->ldata, CD_GRID_PAINT_MASK)) {
- BKE_sculpt_mask_layers_ensure(ob, mmd);
- }
+ BLI_assert(CustomData_has_layer(&me->ldata, CD_GRID_PAINT_MASK));
}
}
- /* tessfaces aren't used and will become invalid */
- BKE_mesh_tessface_clear(me);
-
ss->shapekey_active = (mmd == NULL) ? BKE_keyblock_from_object(ob) : NULL;
/* NOTE: Weight pPaint require mesh info for loop lookup, but it never uses multires code path,
@@ -1660,12 +1653,7 @@ static void sculpt_update_object(Depsgraph *depsgraph,
/* Sculpt Face Sets. */
if (use_face_sets) {
- if (!CustomData_has_layer(&me->pdata, CD_SCULPT_FACE_SETS)) {
- /* By checking here if the data-layer already exist this avoids copying the visibility from
- * the mesh and looping over all vertices on every sculpt editing operation, using this
- * function only the first time the Face Sets data-layer needs to be created. */
- BKE_sculpt_face_sets_ensure_from_base_mesh_visibility(me);
- }
+ BLI_assert(CustomData_has_layer(&me->pdata, CD_SCULPT_FACE_SETS));
ss->face_sets = CustomData_get_layer(&me->pdata, CD_SCULPT_FACE_SETS);
}
else {
@@ -1928,6 +1916,10 @@ static bool check_sculpt_object_deformed(Object *object, const bool for_construc
return deformed;
}
+/**
+ * Ensures that a Face Set data-layers exists. If it does not, it creates one respecting the
+ * visibility stored in the vertices of the mesh. If it does, it copies the visibility from the
+ * mesh to the Face Sets. */
void BKE_sculpt_face_sets_ensure_from_base_mesh_visibility(Mesh *mesh)
{
const int face_sets_default_visible_id = 1;
diff --git a/source/blender/editors/sculpt_paint/sculpt.c b/source/blender/editors/sculpt_paint/sculpt.c
index 38d2bed7d97..c91ab3f0ee1 100644
--- a/source/blender/editors/sculpt_paint/sculpt.c
+++ b/source/blender/editors/sculpt_paint/sculpt.c
@@ -1188,25 +1188,6 @@ void SCULPT_floodfill_free(SculptFloodFill *flood)
*
* \{ */
-/* Check if there are any active modifiers in stack.
- * Used for flushing updates at enter/exit sculpt mode. */
-static bool sculpt_has_active_modifiers(Scene *scene, Object *ob)
-{
- ModifierData *md;
- VirtualModifierData virtualModifierData;
-
- md = BKE_modifiers_get_virtual_modifierlist(ob, &virtualModifierData);
-
- /* Exception for shape keys because we can edit those. */
- for (; md; md = md->next) {
- if (BKE_modifier_is_enabled(scene, md, eModifierMode_Realtime)) {
- return true;
- }
- }
-
- return false;
-}
-
static bool sculpt_tool_needs_original(const char sculpt_tool)
{
return ELEM(sculpt_tool,
@@ -8320,7 +8301,7 @@ static void sculpt_init_session(Depsgraph *depsgraph, Scene *scene, Object *ob)
/* Here we can detect geometry that was just added to Sculpt Mode as it has the
* SCULPT_FACE_SET_NONE assigned, so we can create a new Face Set for it. */
/* In sculpt mode all geometry that is assigned to SCULPT_FACE_SET_NONE is considered as not
- * initialized, which is used is some operators that modify the mesh topology to preform certain
+ * initialized, which is used is some operators that modify the mesh topology to perform certain
* actions in the new polys. After these operations are finished, all polys should have a valid
* face set ID assigned (different from SCULPT_FACE_SET_NONE) to manage their visibility
* correctly. */
@@ -8334,23 +8315,6 @@ static void sculpt_init_session(Depsgraph *depsgraph, Scene *scene, Object *ob)
ss->face_sets[i] = new_face_set;
}
}
-
- /* Update the Face Sets visibility with the vertex visibility changes that may have been done
- * outside Sculpt Mode */
- Mesh *mesh = ob->data;
- BKE_sculpt_face_sets_ensure_from_base_mesh_visibility(mesh);
-}
-
-static int ed_object_sculptmode_flush_recalc_flag(Scene *scene,
- Object *ob,
- MultiresModifierData *mmd)
-{
- int flush_recalc = 0;
- /* Multires in sculpt mode could have different from object mode subdivision level. */
- flush_recalc |= mmd && mmd->sculptlvl != mmd->lvl;
- /* If object has got active modifiers, its dm could be different in sculpt mode. */
- flush_recalc |= sculpt_has_active_modifiers(scene, ob);
- return flush_recalc;
}
void ED_object_sculptmode_enter_ex(Main *bmain,
@@ -8368,13 +8332,6 @@ void ED_object_sculptmode_enter_ex(Main *bmain,
MultiresModifierData *mmd = BKE_sculpt_multires_active(scene, ob);
- const int flush_recalc = ed_object_sculptmode_flush_recalc_flag(scene, ob, mmd);
-
- if (flush_recalc) {
- DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY);
- BKE_scene_graph_evaluated_ensure(depsgraph, bmain);
- }
-
/* Create sculpt mode session data. */
if (ob->sculpt) {
BKE_sculptsession_free(ob);
@@ -8383,14 +8340,30 @@ void ED_object_sculptmode_enter_ex(Main *bmain,
/* Copy the current mesh visibility to the Face Sets. */
BKE_sculpt_face_sets_ensure_from_base_mesh_visibility(me);
- sculpt_init_session(depsgraph, scene, ob);
+ /* Mask layer is required for Multires. */
+ BKE_sculpt_mask_layers_ensure(ob, mmd);
- /* Mask layer is required. */
- if (mmd) {
- /* XXX, we could attempt to support adding mask data mid-sculpt mode (with multi-res)
- * but this ends up being quite tricky (and slow). */
- BKE_sculpt_mask_layers_ensure(ob, mmd);
- }
+ /* Tessfaces aren't used and will become invalid. */
+ BKE_mesh_tessface_clear(me);
+
+ /* We always need to flush updates from depsgraph here, since at the very least
+ * `BKE_sculpt_face_sets_ensure_from_base_mesh_visibility()` will have updated some data layer of
+ * the mesh.
+ *
+ * All known potential sources of updates:
+ * - Addition of, or changes to, the `CD_SCULPT_FACE_SETS` data layer
+ * (`BKE_sculpt_face_sets_ensure_from_base_mesh_visibility`).
+ * - Addition of a `CD_PAINT_MASK` data layer (`BKE_sculpt_mask_layers_ensure`).
+ * - Object has any active modifier (modifier stack can be different in Sculpt mode).
+ * - Multires:
+ * + Differences of subdiv levels between sculpt and object modes
+ * (`mmd->sculptlvl != mmd->lvl`).
+ * + Addition of a `CD_GRID_PAINT_MASK` data layer (`BKE_sculpt_mask_layers_ensure`).
+ */
+ DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY);
+ BKE_scene_graph_evaluated_ensure(depsgraph, bmain);
+
+ sculpt_init_session(depsgraph, scene, ob);
if (!(fabsf(ob->scale[0] - ob->scale[1]) < 1e-4f &&
fabsf(ob->scale[1] - ob->scale[2]) < 1e-4f)) {
More information about the Bf-blender-cvs
mailing list