Fix T82388: Sculpt mode: Unexpected undo behavior.

Issue exposed by rB4c7b1766a7f1.

Main idea is that non-memfile first undo step should check into previous
memfile and tag the ID it is editing as `future_changed`.

That way, when we go back and undo to the memfile, said IDs are properly
detected as changed and re-read from the memfile.

Otherwise, undo system sees them as unchanged, and just re-use the
current data instead.

Note that currently only Sculpt mode seems affected (probably because it
is storing the mode switch itself as a Sculpt undo step instead of a
memfile one), but similar action might be needed in some other cases
too.

Maniphest Tasks: T82388

Differential Revision: https://developer.blender.org/D9510

source/blender/editors/include/ED_undo.h

@@ -87,6 +87,7 @@ void ED_undosys_type_free(void);
 
 /* memfile_undo.c */
 struct MemFile *ED_undosys_stack_memfile_get_active(struct UndoStack *ustack);
+void ED_undosys_stack_memfile_id_changed_tag(struct UndoStack *ustack, struct ID *id);
 
 #ifdef __cplusplus
 }

source/blender/editors/sculpt_paint/paint_hide.c

@@ -366,10 +366,10 @@ static int hide_show_exec(bContext *C, wmOperator *op)
   /* Start undo. */
   switch (action) {
     case PARTIALVIS_HIDE:
-      SCULPT_undo_push_begin("Hide area");
+      SCULPT_undo_push_begin(ob, "Hide area");
       break;
     case PARTIALVIS_SHOW:
-      SCULPT_undo_push_begin("Show area");
+      SCULPT_undo_push_begin(ob, "Show area");
       break;
   }
 

source/blender/editors/sculpt_paint/paint_mask.c

@@ -169,7 +169,7 @@ static int mask_flood_fill_exec(bContext *C, wmOperator *op)
 
   BKE_pbvh_search_gather(pbvh, NULL, NULL, &nodes, &totnode);
 
-  SCULPT_undo_push_begin("Mask flood fill");
+  SCULPT_undo_push_begin(ob, "Mask flood fill");
 
   MaskTaskData data = {
       .ob = ob,
@@ -707,7 +707,7 @@ static bool sculpt_gesture_is_vertex_effected(SculptGestureContext *sgcontext, P
 static void sculpt_gesture_apply(bContext *C, SculptGestureContext *sgcontext)
 {
   SculptGestureOperation *operation = sgcontext->operation;
-  SCULPT_undo_push_begin("Sculpt Gesture Apply");
+  SCULPT_undo_push_begin(CTX_data_active_object(C), "Sculpt Gesture Apply");
 
   operation->sculpt_gesture_begin(C, sgcontext);
 

source/blender/editors/sculpt_paint/sculpt.c

@@ -7698,7 +7698,7 @@ static bool sculpt_stroke_test_start(bContext *C, struct wmOperator *op, const f
 
     sculpt_update_cache_invariants(C, sd, ss, op, mouse);
 
-    SCULPT_undo_push_begin(sculpt_tool_name(sd));
+    SCULPT_undo_push_begin(ob, sculpt_tool_name(sd));
 
     return true;
   }
@@ -8057,7 +8057,7 @@ static int sculpt_symmetrize_exec(bContext *C, wmOperator *op)
        * as deleted, then after symmetrize operation all BMesh elements
        * are logged as added (as opposed to attempting to store just the
        * parts that symmetrize modifies). */
-      SCULPT_undo_push_begin("Dynamic topology symmetrize");
+      SCULPT_undo_push_begin(ob, "Dynamic topology symmetrize");
       SCULPT_undo_push_node(ob, NULL, SCULPT_UNDO_DYNTOPO_SYMMETRIZE);
       BM_log_before_all_removed(ss->bm, ss->bm_log);
 
@@ -8297,7 +8297,7 @@ void ED_object_sculptmode_enter_ex(Main *bmain,
       bool has_undo = wm->undo_stack != NULL;
       /* Undo push is needed to prevent memory leak. */
       if (has_undo) {
-        SCULPT_undo_push_begin("Dynamic topology enable");
+        SCULPT_undo_push_begin(ob, "Dynamic topology enable");
       }
       SCULPT_dynamic_topology_enable_ex(bmain, depsgraph, scene, ob);
       if (has_undo) {
@@ -8413,7 +8413,7 @@ static int sculpt_mode_toggle_exec(bContext *C, wmOperator *op)
          * while it works it causes lag when undoing the first undo step, see T71564. */
         wmWindowManager *wm = CTX_wm_manager(C);
         if (wm->op_undo_depth <= 1) {
-          SCULPT_undo_push_begin(op->type->name);
+          SCULPT_undo_push_begin(ob, op->type->name);
         }
       }
     }
@@ -9206,7 +9206,7 @@ static int sculpt_mask_by_color_invoke(bContext *C, wmOperator *op, const wmEven
   mouse[1] = event->mval[1];
   SCULPT_cursor_geometry_info_update(C, &sgi, mouse, false);
 
-  SCULPT_undo_push_begin("Mask by color");
+  SCULPT_undo_push_begin(ob, "Mask by color");
 
   const int active_vertex = SCULPT_active_vertex_get(ss);
   const float threshold = RNA_float_get(op->ptr, "threshold");

source/blender/editors/sculpt_paint/sculpt_cloth.c

@@ -1548,7 +1548,7 @@ static int sculpt_cloth_filter_invoke(bContext *C, wmOperator *op, const wmEvent
   /* Needs mask data to be available as it is used when solving the constraints. */
   BKE_sculpt_update_object_for_edit(depsgraph, ob, true, true, false);
 
-  SCULPT_undo_push_begin("Cloth filter");
+  SCULPT_undo_push_begin(ob, "Cloth filter");
   SCULPT_filter_cache_init(C, ob, sd, SCULPT_UNDO_COORDS);
 
   ss->filter_cache->automasking = SCULPT_automasking_cache_init(sd, NULL, ob);

source/blender/editors/sculpt_paint/sculpt_detail.c

@@ -122,7 +122,7 @@ static int sculpt_detail_flood_fill_exec(bContext *C, wmOperator *UNUSED(op))
   float object_space_constant_detail = 1.0f / (sd->constant_detail * mat4_to_scale(ob->obmat));
   BKE_pbvh_bmesh_detail_size_set(ss->pbvh, object_space_constant_detail);
 
-  SCULPT_undo_push_begin("Dynamic topology flood fill");
+  SCULPT_undo_push_begin(ob, "Dynamic topology flood fill");
   SCULPT_undo_push_node(ob, NULL, SCULPT_UNDO_COORDS);
 
   while (BKE_pbvh_bmesh_update_topology(

source/blender/editors/sculpt_paint/sculpt_dyntopo.c

@@ -291,7 +291,7 @@ void sculpt_dynamic_topology_disable_with_undo(Main *bmain,
     /* May be false in background mode. */
     const bool use_undo = G.background ? (ED_undo_stack_get() != NULL) : true;
     if (use_undo) {
-      SCULPT_undo_push_begin("Dynamic topology disable");
+      SCULPT_undo_push_begin(ob, "Dynamic topology disable");
       SCULPT_undo_push_node(ob, NULL, SCULPT_UNDO_DYNTOPO_END);
     }
     SCULPT_dynamic_topology_disable_ex(bmain, depsgraph, scene, ob, NULL);
@@ -311,7 +311,7 @@ static void sculpt_dynamic_topology_enable_with_undo(Main *bmain,
     /* May be false in background mode. */
     const bool use_undo = G.background ? (ED_undo_stack_get() != NULL) : true;
     if (use_undo) {
-      SCULPT_undo_push_begin("Dynamic topology enable");
+      SCULPT_undo_push_begin(ob, "Dynamic topology enable");
     }
     SCULPT_dynamic_topology_enable_ex(bmain, depsgraph, scene, ob);
     if (use_undo) {

source/blender/editors/sculpt_paint/sculpt_face_set.c

@@ -328,7 +328,7 @@ static int sculpt_face_set_create_exec(bContext *C, wmOperator *op)
     return OPERATOR_CANCELLED;
   }
 
-  SCULPT_undo_push_begin("face set change");
+  SCULPT_undo_push_begin(ob, "face set change");
   SCULPT_undo_push_node(ob, nodes[0], SCULPT_UNDO_FACE_SETS);
 
   const int next_face_set = SCULPT_face_set_next_available_get(ss);
@@ -684,7 +684,7 @@ static int sculpt_face_set_init_exec(bContext *C, wmOperator *op)
     return OPERATOR_CANCELLED;
   }
 
-  SCULPT_undo_push_begin("face set change");
+  SCULPT_undo_push_begin(ob, "face set change");
   SCULPT_undo_push_node(ob, nodes[0], SCULPT_UNDO_FACE_SETS);
 
   const float threshold = RNA_float_get(op->ptr, "threshold");
@@ -829,7 +829,7 @@ static int sculpt_face_sets_change_visibility_exec(bContext *C, wmOperator *op)
   const int mode = RNA_enum_get(op->ptr, "mode");
   const int active_face_set = SCULPT_active_face_set_get(ss);
 
-  SCULPT_undo_push_begin("Hide area");
+  SCULPT_undo_push_begin(ob, "Hide area");
 
   PBVH *pbvh = ob->sculpt->pbvh;
   PBVHNode **nodes;
@@ -1276,7 +1276,7 @@ static void sculpt_face_set_edit_modify_face_sets(Object *ob,
   if (!nodes) {
     return;
   }
-  SCULPT_undo_push_begin("face set edit");
+  SCULPT_undo_push_begin(ob, "face set edit");
   SCULPT_undo_push_node(ob, nodes[0], SCULPT_UNDO_FACE_SETS);
   sculpt_face_set_apply_edit(ob, abs(active_face_set), mode, modify_hidden);
   SCULPT_undo_push_end();

source/blender/editors/sculpt_paint/sculpt_filter_color.c

@@ -290,7 +290,7 @@ static int sculpt_color_filter_invoke(bContext *C, wmOperator *op, const wmEvent
     return OPERATOR_CANCELLED;
   }
 
-  SCULPT_undo_push_begin("color filter");
+  SCULPT_undo_push_begin(ob, "color filter");
 
   BKE_sculpt_color_layer_create_if_needed(ob);
 

source/blender/editors/sculpt_paint/sculpt_filter_mask.c

@@ -212,7 +212,7 @@ static int sculpt_mask_filter_exec(bContext *C, wmOperator *op)
   int num_verts = SCULPT_vertex_count_get(ss);
 
   BKE_pbvh_search_gather(pbvh, NULL, NULL, &nodes, &totnode);
-  SCULPT_undo_push_begin("Mask filter");
+  SCULPT_undo_push_begin(ob, "Mask filter");
 
   for (int i = 0; i < totnode; i++) {
     SCULPT_undo_push_node(ob, nodes[i], SCULPT_UNDO_MASK);
@@ -440,7 +440,7 @@ static int sculpt_dirty_mask_exec(bContext *C, wmOperator *op)
   }
 
   BKE_pbvh_search_gather(pbvh, NULL, NULL, &nodes, &totnode);
-  SCULPT_undo_push_begin("Dirty Mask");
+  SCULPT_undo_push_begin(ob, "Dirty Mask");
 
   for (int i = 0; i < totnode; i++) {
     SCULPT_undo_push_node(ob, nodes[i], SCULPT_UNDO_MASK);

source/blender/editors/sculpt_paint/sculpt_filter_mesh.c

@@ -703,7 +703,7 @@ static int sculpt_mesh_filter_invoke(bContext *C, wmOperator *op, const wmEvent
     SCULPT_boundary_info_ensure(ob);
   }
 
-  SCULPT_undo_push_begin("Mesh Filter");
+  SCULPT_undo_push_begin(ob, "Mesh Filter");
 
   SCULPT_filter_cache_init(C, ob, sd, SCULPT_UNDO_COORDS);
 

source/blender/editors/sculpt_paint/sculpt_intern.h

@@ -1085,7 +1085,7 @@ void SCULPT_cache_free(StrokeCache *cache);
 SculptUndoNode *SCULPT_undo_push_node(Object *ob, PBVHNode *node, SculptUndoType type);
 SculptUndoNode *SCULPT_undo_get_node(PBVHNode *node);
 SculptUndoNode *SCULPT_undo_get_first_node(void);
-void SCULPT_undo_push_begin(const char *name);
+void SCULPT_undo_push_begin(struct Object *ob, const char *name);
 void SCULPT_undo_push_end(void);
 void SCULPT_undo_push_end_ex(const bool use_nested_undo);
 

source/blender/editors/sculpt_paint/sculpt_mask_expand.c

@@ -378,7 +378,7 @@ static int sculpt_mask_expand_invoke(bContext *C, wmOperator *op, const wmEvent
 
   BKE_pbvh_search_gather(pbvh, NULL, NULL, &ss->filter_cache->nodes, &ss->filter_cache->totnode);
 
-  SCULPT_undo_push_begin("Mask Expand");
+  SCULPT_undo_push_begin(ob, "Mask Expand");
 
   if (create_face_set) {
     SCULPT_undo_push_node(ob, ss->filter_cache->nodes[0], SCULPT_UNDO_FACE_SETS);

source/blender/editors/sculpt_paint/sculpt_transform.c

@@ -70,7 +70,7 @@ void ED_sculpt_init_transform(struct bContext *C)
   copy_v3_v3(ss->init_pivot_pos, ss->pivot_pos);
   copy_v4_v4(ss->init_pivot_rot, ss->pivot_rot);
 
-  SCULPT_undo_push_begin("Transform");
+  SCULPT_undo_push_begin(ob, "Transform");
   BKE_sculpt_update_object_for_edit(depsgraph, ob, false, false, false);
 
   ss->pivot_rot[3] = 1.0f;

source/blender/editors/sculpt_paint/sculpt_undo.c

@@ -1334,10 +1334,17 @@ SculptUndoNode *SCULPT_undo_push_node(Object *ob, PBVHNode *node, SculptUndoType
   return unode;
 }
 
-void SCULPT_undo_push_begin(const char *name)
+void SCULPT_undo_push_begin(Object *ob, const char *name)
 {
   UndoStack *ustack = ED_undo_stack_get();
 
+  if (ob != NULL) {
+    /* If possible, we need to tag the object and its geometry data as 'changed in the future' in
+     * the previous undo step if it's a memfile one. */
+    ED_undosys_stack_memfile_id_changed_tag(ustack, &ob->id);
+    ED_undosys_stack_memfile_id_changed_tag(ustack, ob->data);
+  }
+
   /* Special case, we never read from this. */
   bContext *C = NULL;
 
@@ -1525,7 +1532,7 @@ static void sculpt_undosys_step_free(UndoStep *us_p)
 
 void ED_sculpt_undo_geometry_begin(struct Object *ob, const char *name)
 {
-  SCULPT_undo_push_begin(name);
+  SCULPT_undo_push_begin(ob, name);
   SCULPT_undo_push_node(ob, NULL, SCULPT_UNDO_GEOMETRY);
 }
 
@@ -1638,7 +1645,7 @@ void ED_sculpt_undo_push_multires_mesh_begin(bContext *C, const char *str)
 
   Object *object = CTX_data_active_object(C);
 
-  SCULPT_undo_push_begin(str);
+  SCULPT_undo_push_begin(object, str);
 
   SculptUndoNode *geometry_unode = SCULPT_undo_push_node(object, NULL, SCULPT_UNDO_GEOMETRY);
   geometry_unode->geometry_clear_pbvh = false;

source/blender/editors/undo/memfile_undo.c

@@ -24,7 +24,9 @@
 #include "BLI_utildefines.h"
 
 #include "BLI_ghash.h"
+#include "BLI_listbase.h"
 
+#include "DNA_ID.h"
 #include "DNA_node_types.h"
 #include "DNA_object_enums.h"
 #include "DNA_object_types.h"
@@ -52,6 +54,8 @@
 
 #include "undo_intern.h"
 
+#include <stdio.h>
+
 /* -------------------------------------------------------------------- */
 /** \name Implements ED Undo System
  * \{ */
@@ -307,4 +311,35 @@ struct MemFile *ED_undosys_stack_memfile_get_active(UndoStack *ustack)
   return NULL;
 }
 
+/**
+ * If the last undo step is a memfile one, find the first memchunk matching given ID (using its
+ * seesion uuid), and tag it as 'changed in the future'.
+ *
+ * Since non-memfile undos cannot automatically set this flag in the previous step as done with
+ * memfile ones, this has to be called manually by relevant undo code.
+ *
+ * \note Only current known case for this is undoing a switch from Pbject to Sculpt mode (see
+ * T82388).
+ *
+ * \note Calling this ID by ID is not optimal, as it will loop over all memchunks until it find
+ * expected one. If this becomes an issue we'll have to add a mapping from session uuid to first
+ * memchunk in #MemFile itself (currently we only do that in #MemFileWriteData when writing a new
+ * step).
+ */
+void ED_undosys_stack_memfile_id_changed_tag(UndoStack *ustack, ID *id)
+{
+  UndoStep *us = ustack->step_active;
+  if (id == NULL || us == NULL || us->type != BKE_UNDOSYS_TYPE_MEMFILE) {
+    return;
+  }
+
+  MemFile *memfile = &((MemFileUndoStep *)us)->data->memfile;
+  LISTBASE_FOREACH (MemFileChunk *, mem_chunk, &memfile->chunks) {
+    if (mem_chunk->id_session_uuid == id->session_uuid) {
+      mem_chunk->is_identical_future = false;
+      break;
+    }
+  }
+}
+
 /** \} */