[Bf-blender-cvs] [26c34a2a3ab] master: Tweak comment regarding Sculpt mode undo issues in object update code.

[Bastien Montagne]

Commit: 26c34a2a3abb14884328ef347963cec2062fc0c2
Author: Bastien Montagne
Date:   Mon Dec 28 14:16:14 2020 +0100
Branches: master
https://developer.blender.org/rB26c34a2a3abb14884328ef347963cec2062fc0c2

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

Original comment from rB8a9dedf82954. See also T84084.

===================================================================

M	source/blender/blenkernel/intern/multires_reshape_ccg.c

===================================================================

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);
         }
       }
     }