5 files changed, 31 insertions, 11 deletions

diff --git a/source/blender/blenkernel/BKE_undo_system.h b/source/blender/blenkernel/BKE_undo_system.h
index efac5d9097f..2973a432723 100644
--- a/source/blender/blenkernel/BKE_undo_system.h
+++ b/source/blender/blenkernel/BKE_undo_system.h
@@ -162,6 +162,13 @@ typedef enum UndoTypeFlags {
    * \note Callback is still supposed to properly deal with a NULL context pointer.
    */
   UNDOTYPE_FLAG_NEED_CONTEXT_FOR_ENCODE = 1 << 0,
+
+  /**
+   * When the active undo step is of this type, it must be read before loading other undo steps.
+   *
+   * This is typically used for undo systems that store both before/after states.
+   */
+  UNDOTYPE_FLAG_DECODE_ACTIVE_STEP = 1 << 1,
 } UndoTypeFlags;
 
 /* Expose since we need to perform operations on specific undo types (rarely). */
diff --git a/source/blender/blenkernel/intern/undo_system.c b/source/blender/blenkernel/intern/undo_system.c
index fe2aa701c63..03bd9323f39 100644
--- a/source/blender/blenkernel/intern/undo_system.c
+++ b/source/blender/blenkernel/intern/undo_system.c
@@ -722,6 +722,24 @@ eUndoStepDir BKE_undosys_step_calc_direction(const UndoStack *ustack,
 }
 
 /**
+ * When reading undo steps for undo/redo,
+ * some extra checks are needed when so the correct undo step is decoded.
+ */
+static UndoStep *undosys_step_iter_first(UndoStep *us_reference, const eUndoStepDir undo_dir)
+{
+  if (us_reference->type->flags & UNDOTYPE_FLAG_DECODE_ACTIVE_STEP) {
+    /* Reading this step means an undo action reads undo twice.
+     * This should be avoided where possible, however some undo systems require it.
+     *
+     * Redo skips the current state as this represents the currently loaded state. */
+    return (undo_dir == -1) ? us_reference : us_reference->next;
+  }
+
+  /* Typical case, skip reading the current undo step. */
+  return (undo_dir == -1) ? us_reference->prev : us_reference->next;
+}
+
+/**
  * Undo/Redo until the given `us_target` step becomes the active (currently loaded) one.
  *
  * \note Unless `us_target` is a 'skipped' one and `use_skip` is true, `us_target` will become the
@@ -786,15 +804,10 @@ bool BKE_undosys_step_load_data_ex(UndoStack *ustack,
             us_target->type->name,
             undo_dir);
 
-  /* Undo/Redo steps until we reach given target step (or beyond if it has to be skipped), from
-   * given reference step.
-   *
-   * NOTE: Unlike with redo case, where we can expect current active step to fully reflect current
-   * data status, in undo case we also do reload the active step.
-   * FIXME: this feels weak, and should probably not be actually needed? Or should also be done in
-   * redo case? */
+  /* Undo/Redo steps until we reach given target step (or beyond if it has to be skipped),
+   * from given reference step. */
   bool is_processing_extra_skipped_steps = false;
-  for (UndoStep *us_iter = (undo_dir == -1) ? us_reference : us_reference->next; us_iter != NULL;
+  for (UndoStep *us_iter = undosys_step_iter_first(us_reference, undo_dir); us_iter != NULL;
        us_iter = (undo_dir == -1) ? us_iter->prev : us_iter->next) {
     BLI_assert(us_iter != NULL);
 
diff --git a/source/blender/editors/sculpt_paint/sculpt_undo.c b/source/blender/editors/sculpt_paint/sculpt_undo.c
index fe7029c7457..f21c900941b 100644
--- a/source/blender/editors/sculpt_paint/sculpt_undo.c
+++ b/source/blender/editors/sculpt_paint/sculpt_undo.c
@@ -1608,7 +1608,7 @@ void ED_sculpt_undosys_type(UndoType *ut)
   ut->step_decode = sculpt_undosys_step_decode;
   ut->step_free = sculpt_undosys_step_free;
 
-  ut->flags = 0;
+  ut->flags = UNDOTYPE_FLAG_DECODE_ACTIVE_STEP;
 
   ut->step_size = sizeof(SculptUndoStep);
 }
diff --git a/source/blender/editors/space_image/image_undo.c b/source/blender/editors/space_image/image_undo.c
index 082f66b57af..cc6effd0f71 100644
--- a/source/blender/editors/space_image/image_undo.c
+++ b/source/blender/editors/space_image/image_undo.c
@@ -1006,7 +1006,7 @@ void ED_image_undosys_type(UndoType *ut)
    * specific case, see `image_undosys_step_encode` code. We cannot specify
    * `UNDOTYPE_FLAG_NEED_CONTEXT_FOR_ENCODE` though, as it can be called with a NULL context by
    * current code. */
-  ut->flags = 0;
+  ut->flags = UNDOTYPE_FLAG_DECODE_ACTIVE_STEP;
 
   ut->step_size = sizeof(ImageUndoStep);
 }
diff --git a/source/blender/editors/space_text/text_undo.c b/source/blender/editors/space_text/text_undo.c
index f55db8c3cc9..80af7d8c9f6 100644
--- a/source/blender/editors/space_text/text_undo.c
+++ b/source/blender/editors/space_text/text_undo.c
@@ -265,7 +265,7 @@ void ED_text_undosys_type(UndoType *ut)
 
   ut->step_foreach_ID_ref = text_undosys_foreach_ID_ref;
 
-  ut->flags = UNDOTYPE_FLAG_NEED_CONTEXT_FOR_ENCODE;
+  ut->flags = UNDOTYPE_FLAG_NEED_CONTEXT_FOR_ENCODE | UNDOTYPE_FLAG_DECODE_ACTIVE_STEP;
 
   ut->step_size = sizeof(TextUndoStep);
 }