diff options
author | Bastien Montagne <bastien@blender.org> | 2020-12-28 16:16:14 +0300 |
---|---|---|
committer | Bastien Montagne <bastien@blender.org> | 2020-12-28 16:16:14 +0300 |
commit | 26c34a2a3abb14884328ef347963cec2062fc0c2 (patch) | |
tree | 9573e5d4cc635f5e30a32d1a481c2ea212914ea1 | |
parent | 823f0da702f1144fd87863f6b3dc3e082889d19c (diff) |
Tweak comment regarding Sculpt mode undo issues in object update code.
Original comment from rB8a9dedf82954. See also T84084.
-rw-r--r-- | source/blender/blenkernel/intern/multires_reshape_ccg.c | 43 |
1 files changed, 24 insertions, 19 deletions
diff --git a/source/blender/blenkernel/intern/multires_reshape_ccg.c b/source/blender/blenkernel/intern/multires_reshape_ccg.c index aa003909bb0..9159c5c03cd 100644 --- a/source/blender/blenkernel/intern/multires_reshape_ccg.c +++ b/source/blender/blenkernel/intern/multires_reshape_ccg.c @@ -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), sizeof(float[3])); - if (reshape_level_key.has_mask) { - /* Assert about a non-NULL `grid_element.mask` may fail here, this code may be called - * from cleanup code during COW evaluation phase by depsgraph (e.g. - * `object_update_from_subsurf_ccg` call in `BKE_object_free_derived_caches`). - * - * `reshape_level_key.has_mask` is ultimately set from MultiRes modifier apply code - * (through `multires_as_ccg` -> `multires_ccg_settings_init`), when object is in sculpt - * mode only, and there is matching loop cdlayer. - * - * `grid_element.mask` is directly set from existing matching loop cdlayer during - * initialization of `MultiresReshapeContext` struct. - * - * Since ccg data is preserved during undos, we may end up with a state where there is no - * mask data in mesh loops' cdlayer, while ccg's `has_mask` is still set to true. - */ - // BLI_assert(grid_element.mask != NULL); - if (grid_element.mask != NULL) { - *grid_element.mask = *CCG_grid_elem_mask(&reshape_level_key, ccg_grid, x, y); - } + /* NOTE: The sculpt mode might have SubdivCCG's data out of sync from what is stored in + * the original object. This happens upon the following scenario: + * + * - User enters sculpt mode of the default cube object. + * - Sculpt mode creates new `layer` + * - User does some strokes. + * - User used undo until sculpt mode is exited. + * + * In an ideal world the sculpt mode will take care of keeping CustomData and CCG layers in + * sync by doing proper pushes to a local sculpt undo stack. + * + * Since the proper solution needs time to be implemented, consider the target object + * the source of truth of which data layers are to be updated during reshape. This means, + * for example, that if the undo system says object does not have paint mask layer, it is + * not to be updated. + * + * This is a fragile logic, and is only working correctly because the code path is only + * 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); } } } |