Tweak comment regarding Sculpt mode undo issues in object update code.

Original comment from rB8a9dedf82954. See also T84084.
This commit is contained in:
Bastien Montagne 2020-12-28 14:16:14 +01:00
parent 823f0da702
commit 26c34a2a3a

View File

@ -60,25 +60,30 @@ bool multires_reshape_assign_final_coords_from_ccg(const MultiresReshapeContext
CCG_grid_elem_co(&reshape_level_key, ccg_grid, x, y), CCG_grid_elem_co(&reshape_level_key, ccg_grid, x, y),
sizeof(float[3])); sizeof(float[3]));
if (reshape_level_key.has_mask) { /* NOTE: The sculpt mode might have SubdivCCG's data out of sync from what is stored in
/* Assert about a non-NULL `grid_element.mask` may fail here, this code may be called * the original object. This happens upon the following scenario:
* from cleanup code during COW evaluation phase by depsgraph (e.g. *
* `object_update_from_subsurf_ccg` call in `BKE_object_free_derived_caches`). * - User enters sculpt mode of the default cube object.
* * - Sculpt mode creates new `layer`
* `reshape_level_key.has_mask` is ultimately set from MultiRes modifier apply code * - User does some strokes.
* (through `multires_as_ccg` -> `multires_ccg_settings_init`), when object is in sculpt * - User used undo until sculpt mode is exited.
* mode only, and there is matching loop cdlayer. *
* * In an ideal world the sculpt mode will take care of keeping CustomData and CCG layers in
* `grid_element.mask` is directly set from existing matching loop cdlayer during * sync by doing proper pushes to a local sculpt undo stack.
* initialization of `MultiresReshapeContext` struct. *
* * Since the proper solution needs time to be implemented, consider the target object
* Since ccg data is preserved during undos, we may end up with a state where there is no * the source of truth of which data layers are to be updated during reshape. This means,
* mask data in mesh loops' cdlayer, while ccg's `has_mask` is still set to true. * for example, that if the undo system says object does not have paint mask layer, it is
*/ * not to be updated.
// BLI_assert(grid_element.mask != NULL); *
if (grid_element.mask != NULL) { * This is a fragile logic, and is only working correctly because the code path is only
*grid_element.mask = *CCG_grid_elem_mask(&reshape_level_key, ccg_grid, x, y); * used by sculpt changes. In other usecases the code might not catch inconsistency and
} * silently do wrong decision. */
/* NOTE: There is a known bug in Undo code that results in first Sculpt step
* after a Memfile one to never be undone (see T83806). This might be the root cause of
* this inconsistency. */
if (reshape_level_key.has_mask && grid_element.mask != NULL) {
*grid_element.mask = *CCG_grid_elem_mask(&reshape_level_key, ccg_grid, x, y);
} }
} }
} }