Cleanup/refactor: Undosys: Get rid of the magic values for undo direction.

Move `eUndoStepDir` to `BKE_undo_system.h` and use its values
everywhere.

Note that this also introduce the `STEP_INVALID` value in that enum.

Finally, kept the matching struct members in some lower-level readfile
code as an `int` to avoid having to include `BKE_undo_system.h` in a lot
of unrelated files.

source/blender/blenkernel/BKE_blender_undo.h

@@ -27,12 +27,14 @@ struct Main;
 struct MemFileUndoData;
 struct bContext;
 
+enum eUndoStepDir;
+
 #define BKE_UNDO_STR_MAX 64
 
 struct MemFileUndoData *BKE_memfile_undo_encode(struct Main *bmain,
                                                 struct MemFileUndoData *mfu_prev);
 bool BKE_memfile_undo_decode(struct MemFileUndoData *mfu,
-                             const int undo_direction,
+                             const enum eUndoStepDir undo_direction,
                              const bool use_old_bmain_data,
                              struct bContext *C);
 void BKE_memfile_undo_free(struct MemFileUndoData *mfu);

source/blender/blenkernel/BKE_undo_system.h

@@ -96,6 +96,12 @@ typedef struct UndoStep {
   /* Over alloc 'type->struct_size'. */
 } UndoStep;
 
+typedef enum eUndoStepDir {
+  STEP_REDO = 1,
+  STEP_UNDO = -1,
+  STEP_INVALID = 0,
+} eUndoStepDir;
+
 typedef enum UndoPushReturn {
   UNDO_PUSH_RET_FAILURE = 0,
   UNDO_PUSH_RET_SUCCESS = (1 << 0),
@@ -127,7 +133,7 @@ typedef struct UndoType {
 
   bool (*step_encode)(struct bContext *C, struct Main *bmain, UndoStep *us);
   void (*step_decode)(
-      struct bContext *C, struct Main *bmain, UndoStep *us, int dir, bool is_final);
+      struct bContext *C, struct Main *bmain, UndoStep *us, const eUndoStepDir dir, bool is_final);
 
   /**
    * \note When freeing all steps,
@@ -203,9 +209,9 @@ UndoStep *BKE_undosys_step_find_by_name_with_type(UndoStack *ustack,
 UndoStep *BKE_undosys_step_find_by_type(UndoStack *ustack, const UndoType *ut);
 UndoStep *BKE_undosys_step_find_by_name(UndoStack *ustack, const char *name);
 
-int BKE_undosys_step_calc_direction(const UndoStack *ustack,
-                                    const UndoStep *us_target,
-                                    const UndoStep *us_reference);
+eUndoStepDir BKE_undosys_step_calc_direction(const UndoStack *ustack,
+                                             const UndoStep *us_target,
+                                             const UndoStep *us_reference);
 
 bool BKE_undosys_step_load_data_ex(UndoStack *ustack,
                                    struct bContext *C,

source/blender/blenkernel/intern/blender_undo.c

@@ -48,6 +48,7 @@
 #include "BKE_context.h"
 #include "BKE_global.h"
 #include "BKE_main.h"
+#include "BKE_undo_system.h"
 
 #include "BLO_readfile.h"
 #include "BLO_undofile.h"
@@ -62,7 +63,7 @@
 #define UNDO_DISK 0
 
 bool BKE_memfile_undo_decode(MemFileUndoData *mfu,
-                             const int undo_direction,
+                             const eUndoStepDir undo_direction,
                              const bool use_old_bmain_data,
                              bContext *C)
 {
@@ -80,7 +81,7 @@ bool BKE_memfile_undo_decode(MemFileUndoData *mfu,
   }
   else {
     struct BlendFileReadParams params = {0};
-    params.undo_direction = undo_direction > 0 ? 1 : -1;
+    params.undo_direction = undo_direction;
     if (!use_old_bmain_data) {
       params.skip_flags |= BLO_READ_SKIP_UNDO_OLD_MAIN;
     }

source/blender/blenkernel/intern/blendfile.c

@@ -57,6 +57,7 @@
 #include "BKE_scene.h"
 #include "BKE_screen.h"
 #include "BKE_studiolight.h"
+#include "BKE_undo_system.h"
 #include "BKE_workspace.h"
 
 #include "BLO_readfile.h"
@@ -148,7 +149,7 @@ static void setup_app_data(bContext *C,
     LOAD_UNDO,
   } mode;
 
-  if (params->undo_direction != 0) {
+  if (params->undo_direction != STEP_INVALID) {
     BLI_assert(bfd->curscene != NULL);
     mode = LOAD_UNDO;
   }

source/blender/blenkernel/intern/undo_system.c

@@ -176,8 +176,12 @@ static bool undosys_step_encode(bContext *C, Main *bmain, UndoStack *ustack, Und
   return ok;
 }
 
-static void undosys_step_decode(
-    bContext *C, Main *bmain, UndoStack *ustack, UndoStep *us, int dir, bool is_final)
+static void undosys_step_decode(bContext *C,
+                                Main *bmain,
+                                UndoStack *ustack,
+                                UndoStep *us,
+                                const eUndoStepDir dir,
+                                bool is_final)
 {
   CLOG_INFO(&LOG, 2, "addr=%p, name='%s', type='%s'", us, us->name, us->type->name);
 
@@ -677,9 +681,9 @@ UndoStep *BKE_undosys_step_find_by_type(UndoStack *ustack, const UndoType *ut)
  *
  * \return -1 for undo, 1 for redo, 0 in case of error.
  */
-int BKE_undosys_step_calc_direction(const UndoStack *ustack,
-                                    const UndoStep *us_target,
-                                    const UndoStep *us_reference)
+eUndoStepDir BKE_undosys_step_calc_direction(const UndoStack *ustack,
+                                             const UndoStep *us_target,
+                                             const UndoStep *us_reference)
 {
   if (us_reference == NULL) {
     us_reference = ustack->step_active;
@@ -694,26 +698,26 @@ int BKE_undosys_step_calc_direction(const UndoStack *ustack,
    *    to the end of the list, rather than its start. */
   /* NOTE: in case target step is the active one, we assume we are in an undo case... */
   if (ELEM(us_target, us_reference, us_reference->prev)) {
-    return -1;
+    return STEP_UNDO;
   }
   if (us_target == us_reference->next) {
-    return 1;
+    return STEP_REDO;
   }
 
   /* Search forward, and then backward. */
   for (UndoStep *us_iter = us_reference->next; us_iter != NULL; us_iter = us_iter->next) {
     if (us_iter == us_target) {
-      return 1;
+      return STEP_REDO;
     }
   }
   for (UndoStep *us_iter = us_reference->prev; us_iter != NULL; us_iter = us_iter->prev) {
     if (us_iter == us_target) {
-      return -1;
+      return STEP_UNDO;
     }
   }
 
   BLI_assert(!"Target undo step not found, this should not happen and may indicate an undo stack corruption");
-  return 0;
+  return STEP_INVALID;
 }
 
 /**
@@ -752,8 +756,8 @@ bool BKE_undosys_step_load_data_ex(UndoStack *ustack,
   }
 
   /* This considers we are in undo case if both `us_target` and `us_reference` are the same. */
-  const int undo_dir = BKE_undosys_step_calc_direction(ustack, us_target, us_reference);
-  BLI_assert(undo_dir != 0);
+  const eUndoStepDir undo_dir = BKE_undosys_step_calc_direction(ustack, us_target, us_reference);
+  BLI_assert(undo_dir != STEP_INVALID);
 
   /* This will be the active step once the undo process is complete.
    *

source/blender/blenloader/BLO_readfile.h

@@ -84,8 +84,8 @@ struct BlendFileReadParams {
   uint skip_flags : 3; /* eBLOReadSkip */
   uint is_startup : 1;
 
-  /** Whether we are reading the memfile for an undo (< 0) or a redo (> 0). */
-  int undo_direction : 2;
+  /** Whether we are reading the memfile for an undo or a redo. */
+  int undo_direction; /* eUndoStepDir */
 };
 
 /* skip reading some data-block types (may want to skip screen data too). */

source/blender/blenloader/intern/readfile.c

@@ -94,6 +94,7 @@
 #include "BKE_report.h"
 #include "BKE_scene.h"
 #include "BKE_screen.h"
+#include "BKE_undo_system.h"
 #include "BKE_workspace.h"
 
 #include "DRW_engine.h"
@@ -1294,12 +1295,12 @@ static ssize_t fd_read_from_memfile(FileData *filedata,
       seek += readsize;
       if (r_is_memchunck_identical != NULL) {
         /* `is_identical` of current chunk represents whether it changed compared to previous undo
-         * step. this is fine in redo case (filedata->undo_direction > 0), but not in undo case,
-         * where we need an extra flag defined when saving the next (future) step after the one we
-         * want to restore, as we are supposed to 'come from' that future undo step, and not the
-         * one before current one. */
-        *r_is_memchunck_identical &= filedata->undo_direction > 0 ? chunk->is_identical :
-                                                                    chunk->is_identical_future;
+         * step. this is fine in redo case, but not in undo case, where we need an extra flag
+         * defined when saving the next (future) step after the one we want to restore, as we are
+         * supposed to 'come from' that future undo step, and not the one before current one. */
+        *r_is_memchunck_identical &= filedata->undo_direction == STEP_REDO ?
+                                         chunk->is_identical :
+                                         chunk->is_identical_future;
       }
     } while (totread < size);
 
@@ -2400,11 +2401,12 @@ static int direct_link_id_restore_recalc(const FileData *fd,
 
     /* Tags that were set between the target state and the current state,
      * that we need to perform again. */
-    if (fd->undo_direction < 0) {
+    if (fd->undo_direction == STEP_UNDO) {
       /* Undo: tags from target to the current state. */
       recalc |= id_current->recalc_up_to_undo_push;
     }
     else {
+      BLI_assert(fd->undo_direction == STEP_REDO);
       /* Redo: tags from current to the target state. */
       recalc |= id_target->recalc_up_to_undo_push;
     }

source/blender/blenloader/intern/readfile.h

@@ -91,7 +91,7 @@ typedef struct FileData {
   struct MemFile *memfile;
   /** Whether we are undoing (< 0) or redoing (> 0), used to choose which 'unchanged' flag to use
    * to detect unchanged data from memfile. */
-  short undo_direction;
+  int undo_direction; /* eUndoStepDir */
 
   /** Variables needed for reading from file. */
   gzFile gzfiledes;

source/blender/editors/armature/editarmature_undo.c

@@ -178,8 +178,11 @@ static bool armature_undosys_step_encode(struct bContext *C, struct Main *bmain,
   return true;
 }
 
-static void armature_undosys_step_decode(
-    struct bContext *C, struct Main *bmain, UndoStep *us_p, int UNUSED(dir), bool UNUSED(is_final))
+static void armature_undosys_step_decode(struct bContext *C,
+                                         struct Main *bmain,
+                                         UndoStep *us_p,
+                                         const eUndoStepDir UNUSED(dir),
+                                         bool UNUSED(is_final))
 {
   ArmatureUndoStep *us = (ArmatureUndoStep *)us_p;
 

source/blender/editors/curve/editcurve_undo.c

@@ -238,8 +238,11 @@ static bool curve_undosys_step_encode(struct bContext *C, struct Main *bmain, Un
   return true;
 }
 
-static void curve_undosys_step_decode(
-    struct bContext *C, struct Main *bmain, UndoStep *us_p, int UNUSED(dir), bool UNUSED(is_final))
+static void curve_undosys_step_decode(struct bContext *C,
+                                      struct Main *bmain,
+                                      UndoStep *us_p,
+                                      const eUndoStepDir UNUSED(dir),
+                                      bool UNUSED(is_final))
 {
   CurveUndoStep *us = (CurveUndoStep *)us_p;
 

source/blender/editors/curve/editfont_undo.c

@@ -356,8 +356,11 @@ static bool font_undosys_step_encode(struct bContext *C, struct Main *bmain, Und
   return true;
 }
 
-static void font_undosys_step_decode(
-    struct bContext *C, struct Main *bmain, UndoStep *us_p, int UNUSED(dir), bool UNUSED(is_final))
+static void font_undosys_step_decode(struct bContext *C,
+                                     struct Main *bmain,
+                                     UndoStep *us_p,
+                                     const eUndoStepDir UNUSED(dir),
+                                     bool UNUSED(is_final))
 {
   /* TODO(campbell): undo_system: use low-level API to set mode. */
   ED_object_mode_set_ex(C, OB_MODE_EDIT, false, NULL);

source/blender/editors/gpencil/gpencil_undo.c

@@ -36,6 +36,7 @@
 #include "BKE_blender_undo.h"
 #include "BKE_context.h"
 #include "BKE_gpencil.h"
+#include "BKE_undo_system.h"
 
 #include "ED_gpencil.h"
 
@@ -61,19 +62,19 @@ int ED_gpencil_session_active(void)
   return (BLI_listbase_is_empty(&undo_nodes) == false);
 }
 
-int ED_undo_gpencil_step(bContext *C, const int step)
+int ED_undo_gpencil_step(bContext *C, const eUndoStepDir step)
 {
   bGPdata **gpd_ptr = NULL, *new_gpd = NULL;
 
   gpd_ptr = ED_gpencil_data_get_pointers(C, NULL);
 
-  if (step == -1) { /* undo */
+  if (step == STEP_UNDO) {
     if (cur_node->prev) {
       cur_node = cur_node->prev;
       new_gpd = cur_node->gpd;
     }
   }
-  else if (step == 1) {
+  else if (step == STEP_REDO) {
     if (cur_node->next) {
       cur_node = cur_node->next;
       new_gpd = cur_node->gpd;

source/blender/editors/include/ED_gpencil.h

@@ -62,6 +62,8 @@ struct bAnimContext;
 struct wmKeyConfig;
 struct wmOperator;
 
+enum eUndoStepDir;
+
 #define GPENCIL_MINIMUM_JOIN_DIST 20.0f
 
 /* Reproject stroke modes. */
@@ -213,7 +215,7 @@ bool ED_gpencil_anim_copybuf_paste(struct bAnimContext *ac, const short copy_mod
 
 /* ------------ Grease-Pencil Undo System ------------------ */
 int ED_gpencil_session_active(void);
-int ED_undo_gpencil_step(struct bContext *C, const int step);
+int ED_undo_gpencil_step(struct bContext *C, const enum eUndoStepDir step);
 
 /* ------------ Grease-Pencil Armature ------------------ */
 bool ED_gpencil_add_armature(const struct bContext *C,

source/blender/editors/lattice/editlattice_undo.c

@@ -212,8 +212,11 @@ static bool lattice_undosys_step_encode(struct bContext *C, Main *bmain, UndoSte
   return true;
 }
 
-static void lattice_undosys_step_decode(
-    struct bContext *C, struct Main *bmain, UndoStep *us_p, int UNUSED(dir), bool UNUSED(is_final))
+static void lattice_undosys_step_decode(struct bContext *C,
+                                        struct Main *bmain,
+                                        UndoStep *us_p,
+                                        const eUndoStepDir UNUSED(dir),
+                                        bool UNUSED(is_final))
 {
   LatticeUndoStep *us = (LatticeUndoStep *)us_p;
 

source/blender/editors/mesh/editmesh_undo.c

@@ -744,8 +744,11 @@ static bool mesh_undosys_step_encode(struct bContext *C, struct Main *bmain, Und
   return true;
 }
 
-static void mesh_undosys_step_decode(
-    struct bContext *C, struct Main *bmain, UndoStep *us_p, int UNUSED(dir), bool UNUSED(is_final))
+static void mesh_undosys_step_decode(struct bContext *C,
+                                     struct Main *bmain,
+                                     UndoStep *us_p,
+                                     const eUndoStepDir UNUSED(dir),
+                                     bool UNUSED(is_final))
 {
   MeshUndoStep *us = (MeshUndoStep *)us_p;
 

source/blender/editors/metaball/editmball_undo.c

@@ -187,8 +187,11 @@ static bool mball_undosys_step_encode(struct bContext *C, struct Main *bmain, Un
   return true;
 }
 
-static void mball_undosys_step_decode(
-    struct bContext *C, struct Main *bmain, UndoStep *us_p, int UNUSED(dir), bool UNUSED(is_final))
+static void mball_undosys_step_decode(struct bContext *C,
+                                      struct Main *bmain,
+                                      UndoStep *us_p,
+                                      const eUndoStepDir UNUSED(dir),
+                                      bool UNUSED(is_final))
 {
   MBallUndoStep *us = (MBallUndoStep *)us_p;
 

source/blender/editors/physics/particle_edit_undo.c

@@ -247,7 +247,7 @@ static bool particle_undosys_step_encode(struct bContext *C,
 static void particle_undosys_step_decode(struct bContext *C,
                                          struct Main *UNUSED(bmain),
                                          UndoStep *us_p,
-                                         int UNUSED(dir),
+                                         const eUndoStepDir UNUSED(dir),
                                          bool UNUSED(is_final))
 {
   Depsgraph *depsgraph = CTX_data_depsgraph_pointer(C);

source/blender/editors/sculpt_paint/paint_curve_undo.c

@@ -126,7 +126,7 @@ static bool paintcurve_undosys_step_encode(struct bContext *C,
 static void paintcurve_undosys_step_decode(struct bContext *UNUSED(C),
                                            struct Main *UNUSED(bmain),
                                            UndoStep *us_p,
-                                           int UNUSED(dir),
+                                           const eUndoStepDir UNUSED(dir),
                                            bool UNUSED(is_final))
 {
   PaintCurveUndoStep *us = (PaintCurveUndoStep *)us_p;

source/blender/editors/sculpt_paint/sculpt_undo.c

@@ -1527,9 +1527,14 @@ static void sculpt_undosys_step_decode_redo(struct bContext *C,
   }
 }
 
-static void sculpt_undosys_step_decode(
-    struct bContext *C, struct Main *bmain, UndoStep *us_p, int dir, bool UNUSED(is_final))
+static void sculpt_undosys_step_decode(struct bContext *C,
+                                       struct Main *bmain,
+                                       UndoStep *us_p,
+                                       const eUndoStepDir dir,
+                                       bool UNUSED(is_final))
 {
+  BLI_assert(dir != STEP_INVALID);
+
   Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(C);
 
   /* Ensure sculpt mode. */
@@ -1568,10 +1573,10 @@ static void sculpt_undosys_step_decode(
   }
 
   SculptUndoStep *us = (SculptUndoStep *)us_p;
-  if (dir < 0) {
+  if (dir == STEP_UNDO) {
     sculpt_undosys_step_decode_undo(C, depsgraph, us);
   }
-  else {
+  else if (dir == STEP_REDO) {
     sculpt_undosys_step_decode_redo(C, depsgraph, us);
   }
 }

source/blender/editors/space_image/image_undo.c

@@ -947,13 +947,15 @@ static void image_undosys_step_decode_redo(ImageUndoStep *us)
 }
 
 static void image_undosys_step_decode(
-    struct bContext *C, struct Main *bmain, UndoStep *us_p, int dir, bool is_final)
+    struct bContext *C, struct Main *bmain, UndoStep *us_p, const eUndoStepDir dir, bool is_final)
 {
+  BLI_assert(dir != STEP_INVALID);
+
   ImageUndoStep *us = (ImageUndoStep *)us_p;
-  if (dir < 0) {
+  if (dir == STEP_UNDO) {
     image_undosys_step_decode_undo(us, is_final);
   }
-  else {
+  else if (dir == STEP_REDO) {
     image_undosys_step_decode_redo(us);
   }
 

source/blender/editors/space_text/text_undo.c

@@ -196,14 +196,19 @@ static bool text_undosys_step_encode(struct bContext *C,
   return true;
 }
 
-static void text_undosys_step_decode(
-    struct bContext *C, struct Main *UNUSED(bmain), UndoStep *us_p, int dir, bool is_final)
+static void text_undosys_step_decode(struct bContext *C,
+                                     struct Main *UNUSED(bmain),
+                                     UndoStep *us_p,
+                                     const eUndoStepDir dir,
+                                     bool is_final)
 {
+  BLI_assert(dir != STEP_INVALID);
+
   TextUndoStep *us = (TextUndoStep *)us_p;
   Text *text = us->text_ref.ptr;
 
   TextState *state;
-  if ((us->states[0].buf_array_state != NULL) && (dir == -1) && !is_final) {
+  if ((us->states[0].buf_array_state != NULL) && (dir == STEP_UNDO) && !is_final) {
     state = &us->states[0];
   }
   else {

source/blender/editors/undo/ed_undo.c

@@ -70,15 +70,6 @@
 /** We only need this locally. */
 static CLG_LogRef LOG = {"ed.undo"};
 
-/**
- * \warning Values are used in #ED_undo_gpencil_step,
- * which should eventually be replaced with the undo-system.
- */
-enum eUndoStepDir {
-  STEP_REDO = 1,
-  STEP_UNDO = -1,
-};
-
 /* -------------------------------------------------------------------- */
 /** \name Generic Undo System Access
  *
@@ -276,7 +267,7 @@ static int ed_undo_step_direction(bContext *C, enum eUndoStepDir step, ReportLis
    * FIXME: However, it seems to never be used in current code (`ED_gpencil_session_active` seems
    * to always return false). */
   if (ED_gpencil_session_active()) {
-    return ED_undo_gpencil_step(C, (int)step);
+    return ED_undo_gpencil_step(C, step);
   }
 
   wmWindowManager *wm = CTX_wm_manager(C);

source/blender/editors/undo/memfile_undo.c

@@ -145,19 +145,18 @@ static int memfile_undosys_step_id_reused_cb(LibraryIDLinkCallbackData *cb_data)
 static void memfile_undosys_step_decode(struct bContext *C,
                                         struct Main *bmain,
                                         UndoStep *us_p,
-                                        int undo_direction,
+                                        const eUndoStepDir undo_direction,
                                         bool UNUSED(is_final))
 {
-  BLI_assert(undo_direction != 0);
+  BLI_assert(undo_direction != STEP_INVALID);
 
   bool use_old_bmain_data = true;
 
   if (USER_EXPERIMENTAL_TEST(&U, use_undo_legacy)) {
     use_old_bmain_data = false;
   }
-  else if (undo_direction > 0) {
-    /* Redo case.
-     * The only time we should have to force a complete redo is when current step is tagged as a
+  else if (undo_direction == STEP_REDO) {
+    /* The only time we should have to force a complete redo is when current step is tagged as a
      * redo barrier.
      * If previous step was not a memfile one should not matter here, current data in old bmain
      * should still always be valid for unchanged data-blocks. */
@@ -165,9 +164,8 @@ static void memfile_undosys_step_decode(struct bContext *C,
       use_old_bmain_data = false;
     }
   }
-  else {
-    /* Undo case.
-     * Here we do not care whether current step is an undo barrier, since we are coming from
+  else if (undo_direction == STEP_UNDO) {
+    /* Here we do not care whether current step is an undo barrier, since we are coming from
      * 'the future' we can still re-use old data. However, if *next* undo step
      * (i.e. the one immediately in the future, the one we are coming from)
      * is a barrier, then we have to force a complete undo.