Fix missing undo steps for smooth-view operators

Support pushing undo steps for smooth-view operations that manipulate
the camera. Now V3D_SmoothParams take optional undo arguments.

Used for:

- VIEW3D_OT_view_center_cursor
- VIEW3D_OT_view_center_pick
- VIEW3D_OT_view_orbit
- VIEW3D_OT_view_roll
- VIEW3D_OT_zoom_border

Follow up fix for T92099.

source/blender/editors/include/ED_view3d.h

@@ -1196,6 +1196,14 @@ bool ED_view3d_camera_lock_autokey(struct View3D *v3d,
 
 void ED_view3d_lock_clear(struct View3D *v3d);
 
+/**
+ * Check if creating an undo step should be performed if the viewport moves.
+ * \return true if #ED_view3d_camera_lock_undo_push would do an undo push.
+ */
+bool ED_view3d_camera_lock_undo_test(const View3D *v3d,
+                                     const RegionView3D *rv3d,
+                                     struct bContext *C);
+
 /**
  * Create an undo step when the camera is locked to the view.
  * \param str: The name of the undo step (typically #wmOperatorType.name should be used).

source/blender/editors/space_view3d/view3d_navigate.c

@@ -495,6 +495,8 @@ static void axis_set_view(bContext *C,
                               .camera_old = v3d->camera,
                               .ofs = rv3d->ofs,
                               .quat = quat,
+                              /* No undo because this switches to/from camera. */
+                              .undo_str = NULL,
                           });
   }
   else if (orig_persp == RV3D_CAMOB && v3d->camera) {
@@ -518,6 +520,8 @@ static void axis_set_view(bContext *C,
                               .ofs = ofs,
                               .quat = quat,
                               .dist = &dist,
+                              /* No undo because this switches to/from camera. */
+                              .undo_str = NULL,
                           });
   }
   else {
@@ -540,6 +544,8 @@ static void axis_set_view(bContext *C,
                           &(const V3D_SmoothParams){
                               .quat = quat,
                               .dyn_ofs = dyn_ofs_pt,
+                              /* No undo because this isn't a camera view. */
+                              .undo_str = NULL,
                           });
   }
 }
@@ -694,6 +700,8 @@ static void view3d_from_minmax(bContext *C,
                               .camera_old = v3d->camera,
                               .ofs = new_ofs,
                               .dist = ok_dist ? &new_dist : NULL,
+                              /* The caller needs to use undo begin/end calls. */
+                              .undo_str = NULL,
                           });
   }
   else {
@@ -704,6 +712,8 @@ static void view3d_from_minmax(bContext *C,
                           &(const V3D_SmoothParams){
                               .ofs = new_ofs,
                               .dist = ok_dist ? &new_dist : NULL,
+                              /* The caller needs to use undo begin/end calls. */
+                              .undo_str = NULL,
                           });
   }
 
@@ -736,6 +746,7 @@ static void view3d_from_minmax_multi(bContext *C,
 
 static int view3d_all_exec(bContext *C, wmOperator *op)
 {
+  ScrArea *area = CTX_wm_area(C);
   ARegion *region = CTX_wm_region(C);
   View3D *v3d = CTX_wm_view3d(C);
   RegionView3D *rv3d = CTX_wm_region_view3d(C);
@@ -802,6 +813,7 @@ static int view3d_all_exec(bContext *C, wmOperator *op)
     /* This is an approximation, see function documentation for details. */
     ED_view3d_clipping_clamp_minmax(rv3d, min, max);
   }
+  ED_view3d_smooth_view_undo_begin(C, area);
 
   if (use_all_regions) {
     view3d_from_minmax_multi(C, v3d, min, max, true, smooth_viewtx);
@@ -810,6 +822,8 @@ static int view3d_all_exec(bContext *C, wmOperator *op)
     view3d_from_minmax(C, v3d, region, min, max, true, smooth_viewtx);
   }
 
+  ED_view3d_smooth_view_undo_end(C, area, op->type->name, false);
+
   return OPERATOR_FINISHED;
 }
 
@@ -842,6 +856,7 @@ void VIEW3D_OT_view_all(wmOperatorType *ot)
 
 static int viewselected_exec(bContext *C, wmOperator *op)
 {
+  ScrArea *area = CTX_wm_area(C);
   ARegion *region = CTX_wm_region(C);
   View3D *v3d = CTX_wm_view3d(C);
   RegionView3D *rv3d = CTX_wm_region_view3d(C);
@@ -971,6 +986,8 @@ static int viewselected_exec(bContext *C, wmOperator *op)
     ED_view3d_clipping_clamp_minmax(rv3d, min, max);
   }
 
+  ED_view3d_smooth_view_undo_begin(C, area);
+
   if (use_all_regions) {
     view3d_from_minmax_multi(C, v3d, min, max, ok_dist, smooth_viewtx);
   }
@@ -978,6 +995,8 @@ static int viewselected_exec(bContext *C, wmOperator *op)
     view3d_from_minmax(C, v3d, region, min, max, ok_dist, smooth_viewtx);
   }
 
+  ED_view3d_smooth_view_undo_end(C, area, op->type->name, false);
+
   return OPERATOR_FINISHED;
 }
 
@@ -1020,8 +1039,14 @@ static int viewcenter_cursor_exec(bContext *C, wmOperator *op)
     /* non camera center */
     float new_ofs[3];
     negate_v3_v3(new_ofs, scene->cursor.location);
-    ED_view3d_smooth_view(
-        C, v3d, region, smooth_viewtx, &(const V3D_SmoothParams){.ofs = new_ofs});
+    ED_view3d_smooth_view(C,
+                          v3d,
+                          region,
+                          smooth_viewtx,
+                          &(const V3D_SmoothParams){
+                              .ofs = new_ofs,
+                              .undo_str = op->type->name,
+                          });
 
     /* Smooth view does view-lock #RV3D_BOXVIEW copy. */
   }
@@ -1074,8 +1099,14 @@ static int viewcenter_pick_invoke(bContext *C, wmOperator *op, const wmEvent *ev
       ED_view3d_win_to_3d_int(v3d, region, new_ofs, event->mval, new_ofs);
     }
     negate_v3(new_ofs);
-    ED_view3d_smooth_view(
-        C, v3d, region, smooth_viewtx, &(const V3D_SmoothParams){.ofs = new_ofs});
+    ED_view3d_smooth_view(C,
+                          v3d,
+                          region,
+                          smooth_viewtx,
+                          &(const V3D_SmoothParams){
+                              .ofs = new_ofs,
+                              .undo_str = op->type->name,
+                          });
   }
 
   return OPERATOR_FINISHED;
@@ -1318,17 +1349,20 @@ static int view_camera_exec(bContext *C, wmOperator *op)
 
       /* finally do snazzy view zooming */
       rv3d->persp = RV3D_CAMOB;
-      ED_view3d_smooth_view(C,
-                            v3d,
-                            region,
-                            smooth_viewtx,
-                            &(const V3D_SmoothParams){
-                                .camera = v3d->camera,
-                                .ofs = rv3d->ofs,
-                                .quat = rv3d->viewquat,
-                                .dist = &rv3d->dist,
-                                .lens = &v3d->lens,
-                            });
+      ED_view3d_smooth_view(
+          C,
+          v3d,
+          region,
+          smooth_viewtx,
+          &(const V3D_SmoothParams){
+              .camera = v3d->camera,
+              .ofs = rv3d->ofs,
+              .quat = rv3d->viewquat,
+              .dist = &rv3d->dist,
+              .lens = &v3d->lens,
+              /* No undo because this changes cameras (and wont move the camera). */
+              .undo_str = NULL,
+          });
     }
     else {
       /* return to settings of last view */
@@ -1475,6 +1509,9 @@ static int vieworbit_exec(bContext *C, wmOperator *op)
                             &(const V3D_SmoothParams){
                                 .quat = quat_new,
                                 .dyn_ofs = dyn_ofs_pt,
+                                /* Group as successive orbit may run by holding a key. */
+                                .undo_str = op->type->name,
+                                .undo_grouped = true,
                             });
 
       return OPERATOR_FINISHED;

source/blender/editors/space_view3d/view3d_navigate.h

@@ -231,6 +231,14 @@ typedef struct V3D_SmoothParams {
 
   /** Alternate rotation center, when set `ofs` must be NULL. */
   const float *dyn_ofs;
+
+  /** When non-NULL, perform undo pushes when transforming the camera. */
+  const char *undo_str;
+  /**
+   * When true use grouped undo pushes, use for incremental viewport manipulation
+   * which are likely to be activated by holding a key or from the mouse-wheel.
+   */
+  bool undo_grouped;
 } V3D_SmoothParams;
 
 /**
@@ -251,6 +259,22 @@ void ED_view3d_smooth_view(struct bContext *C,
                            int smooth_viewtx,
                            const V3D_SmoothParams *sview);
 
+/**
+ * Call before multiple smooth-view operations begin to properly handle undo.
+ *
+ * \note Only use explicit undo calls when multiple calls to smooth-view are necessary
+ * or when calling #ED_view3d_smooth_view_ex.
+ * Otherwise pass in #V3D_SmoothParams.undo_str so an undo step is pushed as needed.
+ */
+void ED_view3d_smooth_view_undo_begin(struct bContext *C, struct ScrArea *area);
+/**
+ * Run after multiple smooth-view operations have run to push undo as needed.
+ */
+void ED_view3d_smooth_view_undo_end(struct bContext *C,
+                                    struct ScrArea *area,
+                                    const char *undo_str,
+                                    bool undo_grouped);
+
 /**
  * Apply the smooth-view immediately, use when we need to start a new view operation.
  * (so we don't end up half-applying a view operation when pressing keys quickly).

source/blender/editors/space_view3d/view3d_navigate_roll.c

@@ -202,6 +202,9 @@ static int viewroll_exec(bContext *C, wmOperator *op)
                           &(const V3D_SmoothParams){
                               .quat = quat_new,
                               .dyn_ofs = dyn_ofs_pt,
+                              /* Group as successive roll may run by holding a key. */
+                              .undo_str = op->type->name,
+                              .undo_grouped = true,
                           });
 
     viewops_data_free(C, op->customdata);

source/blender/editors/space_view3d/view3d_navigate_smoothview.c

@@ -8,6 +8,7 @@
 
 #include "MEM_guardedalloc.h"
 
+#include "BLI_listbase.h"
 #include "BLI_math.h"
 
 #include "BKE_context.h"
@@ -21,6 +22,124 @@
 #include "view3d_intern.h"
 #include "view3d_navigate.h" /* own include */
 
+static void view3d_smoothview_apply_ex(bContext *C,
+                                       View3D *v3d,
+                                       ARegion *region,
+                                       bool sync_boxview,
+                                       bool use_autokey,
+                                       const float step,
+                                       const bool finished);
+
+/* -------------------------------------------------------------------- */
+/** \name Smooth View Undo Handling
+ *
+ * When the camera is locked to the viewport smooth-view operations
+ * may need to perform an undo push.
+ *
+ * In this case the smooth-view camera transformation is temporarily completed,
+ * undo is pushed then the change is rewound, and smooth-view completes from it's timer.
+ * In the case smooth-view executed the change immediately - an undo push is called.
+ *
+ * NOTE(@campbellbarton): While this is not ideal it's necessary as making the undo-push
+ * once smooth-view is complete because smooth-view is non-blocking and it's possible other
+ * operations are executed once smooth-view has started.
+ * \{ */
+
+void ED_view3d_smooth_view_undo_begin(bContext *C, ScrArea *area)
+{
+  const View3D *v3d = area->spacedata.first;
+  Object *camera = v3d->camera;
+  if (!camera) {
+    return;
+  }
+
+  /* Tag the camera object so it's known smooth-view is applied to the view-ports camera
+   * (needed to detect when a locked camera is being manipulated).
+   * NOTE: It doesn't matter if the actual object being manipulated is the camera or not. */
+  camera->id.tag &= ~LIB_TAG_DOIT;
+
+  LISTBASE_FOREACH (ARegion *, region, &area->regionbase) {
+    if (region->regiontype != RGN_TYPE_WINDOW) {
+      continue;
+    }
+    RegionView3D *rv3d = region->regiondata;
+    if (ED_view3d_camera_lock_undo_test(v3d, rv3d, C)) {
+      camera->id.tag |= LIB_TAG_DOIT;
+      break;
+    }
+  }
+}
+
+void ED_view3d_smooth_view_undo_end(bContext *C,
+                                    ScrArea *area,
+                                    const char *undo_str,
+                                    const bool undo_grouped)
+{
+  View3D *v3d = area->spacedata.first;
+  Object *camera = v3d->camera;
+  if (!camera) {
+    return;
+  }
+  if (camera->id.tag & LIB_TAG_DOIT) {
+    /* Smooth view didn't touch the camera. */
+    camera->id.tag &= ~LIB_TAG_DOIT;
+    return;
+  }
+
+  if ((U.uiflag & USER_GLOBALUNDO) == 0) {
+    return;
+  }
+
+  /* NOTE(@campbellbarton): It is not possible that a single viewport references different cameras
+   * so even in the case there is a quad-view with multiple camera views set, these will all
+   * reference the same camera. In this case it doesn't matter which region is used.
+   * If in the future multiple cameras are supported, this logic can be extended. */
+  ARegion *region_camera = NULL;
+
+  /* An undo push should be performed. */
+  bool is_interactive = false;
+  LISTBASE_FOREACH (ARegion *, region, &area->regionbase) {
+    if (region->regiontype != RGN_TYPE_WINDOW) {
+      continue;
+    }
+    RegionView3D *rv3d = region->regiondata;
+    if (ED_view3d_camera_lock_undo_test(v3d, rv3d, C)) {
+      region_camera = region;
+      if (rv3d->sms) {
+        is_interactive = true;
+      }
+    }
+  }
+
+  if (region_camera == NULL) {
+    return;
+  }
+
+  /* Arguments to #view3d_smoothview_apply_ex to temporarily apply transformation. */
+  const bool sync_boxview = false;
+  const bool use_autokey = false;
+  const bool finished = false;
+
+  /* Fast forward, undo push, then rewind. */
+  if (is_interactive) {
+    view3d_smoothview_apply_ex(C, v3d, region_camera, sync_boxview, use_autokey, 1.0f, finished);
+  }
+
+  RegionView3D *rv3d = region_camera->regiondata;
+  if (undo_grouped) {
+    ED_view3d_camera_lock_undo_grouped_push(undo_str, v3d, rv3d, C);
+  }
+  else {
+    ED_view3d_camera_lock_undo_push(undo_str, v3d, rv3d, C);
+  }
+
+  if (is_interactive) {
+    view3d_smoothview_apply_ex(C, v3d, region_camera, sync_boxview, use_autokey, 0.0f, finished);
+  }
+}
+
+/** \} */
+
 /* -------------------------------------------------------------------- */
 /** \name Smooth View Operator & Utilities
  *
@@ -86,6 +205,11 @@ void ED_view3d_smooth_view_ex(
     const int smooth_viewtx,
     const V3D_SmoothParams *sview)
 {
+  /* In this case use #ED_view3d_smooth_view_undo_begin & end functions
+   * instead of passing in undo. */
+  BLI_assert_msg(sview->undo_str == NULL,
+                 "Only the 'ED_view3d_smooth_view' version of this function handles undo!");
+
   RegionView3D *rv3d = region->regiondata;
   struct SmoothView3DStore sms = {{0}};
 
@@ -236,6 +360,13 @@ void ED_view3d_smooth_view_ex(
 
     WM_event_add_mousemove(win);
   }
+
+  if (sms.to_camera == false) {
+    /* See comments in #ED_view3d_smooth_view_undo_begin for why this is needed. */
+    if (v3d->camera) {
+      v3d->camera->id.tag &= ~LIB_TAG_DOIT;
+    }
+  }
 }
 
 void ED_view3d_smooth_view(bContext *C,
@@ -249,26 +380,37 @@ void ED_view3d_smooth_view(bContext *C,
   wmWindow *win = CTX_wm_window(C);
   ScrArea *area = CTX_wm_area(C);
 
-  ED_view3d_smooth_view_ex(depsgraph, wm, win, area, v3d, region, smooth_viewtx, sview);
+  /* #ED_view3d_smooth_view_ex asserts this is not set as it doesn't support undo. */
+  struct V3D_SmoothParams sview_no_undo = *sview;
+  sview_no_undo.undo_str = NULL;
+  sview_no_undo.undo_grouped = false;
+
+  const bool do_undo = (sview->undo_str != NULL);
+  if (do_undo) {
+    ED_view3d_smooth_view_undo_begin(C, area);
+  }
+
+  ED_view3d_smooth_view_ex(depsgraph, wm, win, area, v3d, region, smooth_viewtx, &sview_no_undo);
+
+  if (do_undo) {
+    ED_view3d_smooth_view_undo_end(C, area, sview->undo_str, sview->undo_grouped);
+  }
 }
 
-/* only meant for timer usage */
-static void view3d_smoothview_apply(bContext *C, View3D *v3d, ARegion *region, bool sync_boxview)
+static void view3d_smoothview_apply_ex(bContext *C,
+                                       View3D *v3d,
+                                       ARegion *region,
+                                       bool sync_boxview,
+                                       bool use_autokey,
+                                       const float step,
+                                       const bool finished)
 {
   wmWindowManager *wm = CTX_wm_manager(C);
   RegionView3D *rv3d = region->regiondata;
   struct SmoothView3DStore *sms = rv3d->sms;
-  float step, step_inv;
-
-  if (sms->time_allowed != 0.0) {
-    step = (float)((rv3d->smooth_timer->duration) / sms->time_allowed);
-  }
-  else {
-    step = 1.0f;
-  }
 
   /* end timer */
-  if (step >= 1.0f) {
+  if (finished) {
     wmWindow *win = CTX_wm_window(C);
 
     /* if we went to camera, store the original */
@@ -301,9 +443,7 @@ static void view3d_smoothview_apply(bContext *C, View3D *v3d, ARegion *region, b
   }
   else {
     /* ease in/out */
-    step = (3.0f * step * step - 2.0f * step * step * step);
-
-    step_inv = 1.0f - step;
+    const float step_inv = 1.0f - step;
 
     interp_qt_qtqt(rv3d->viewquat, sms->src.quat, sms->dst.quat, step);
 
@@ -320,7 +460,7 @@ static void view3d_smoothview_apply(bContext *C, View3D *v3d, ARegion *region, b
 
     const Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(C);
     ED_view3d_camera_lock_sync(depsgraph, v3d, rv3d);
-    if (ED_screen_animation_playing(wm)) {
+    if (use_autokey && ED_screen_animation_playing(wm)) {
       ED_view3d_camera_lock_autokey(v3d, rv3d, C, true, true);
     }
   }
@@ -342,6 +482,27 @@ static void view3d_smoothview_apply(bContext *C, View3D *v3d, ARegion *region, b
   }
 }
 
+/* only meant for timer usage */
+
+static void view3d_smoothview_apply(bContext *C, View3D *v3d, ARegion *region, bool sync_boxview)
+{
+  RegionView3D *rv3d = region->regiondata;
+  struct SmoothView3DStore *sms = rv3d->sms;
+  float step;
+
+  if (sms->time_allowed != 0.0) {
+    step = (float)((rv3d->smooth_timer->duration) / sms->time_allowed);
+  }
+  else {
+    step = 1.0f;
+  }
+  const bool finished = step >= 1.0f;
+  if (!finished) {
+    step = (3.0f * step * step - 2.0f * step * step * step);
+  }
+  view3d_smoothview_apply_ex(C, v3d, region, sync_boxview, true, step, finished);
+}
+
 static int view3d_smoothview_invoke(bContext *C, wmOperator *UNUSED(op), const wmEvent *event)
 {
   View3D *v3d = CTX_wm_view3d(C);

source/blender/editors/space_view3d/view3d_navigate_zoom_border.c

@@ -173,6 +173,7 @@ static int view3d_zoom_border_exec(bContext *C, wmOperator *op)
                         &(const V3D_SmoothParams){
                             .ofs = new_ofs,
                             .dist = &new_dist,
+                            .undo_str = op->type->name,
                         });
 
   if (RV3D_LOCK_FLAGS(rv3d) & RV3D_BOXVIEW) {

source/blender/editors/space_view3d/view3d_utils.c

@@ -689,6 +689,18 @@ bool ED_view3d_camera_lock_autokey(View3D *v3d,
   return false;
 }
 
+bool ED_view3d_camera_lock_undo_test(const View3D *v3d,
+                                     const RegionView3D *rv3d,
+                                     struct bContext *C)
+{
+  if (ED_view3d_camera_lock_check(v3d, rv3d)) {
+    if (ED_undo_is_memfile_compatible(C)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 /**
  * Create a MEMFILE undo-step for locked camera movement when transforming the view.
  * Edit and texture paint mode don't use MEMFILE undo so undo push is skipped for them.
@@ -699,16 +711,14 @@ bool ED_view3d_camera_lock_autokey(View3D *v3d,
 static bool view3d_camera_lock_undo_ex(
     const char *str, View3D *v3d, RegionView3D *rv3d, struct bContext *C, bool undo_group)
 {
-  if (ED_view3d_camera_lock_check(v3d, rv3d)) {
-    if (ED_undo_is_memfile_compatible(C)) {
-      if (undo_group) {
-        ED_undo_grouped_push(C, str);
-      }
-      else {
-        ED_undo_push(C, str);
-      }
-      return true;
+  if (ED_view3d_camera_lock_undo_test(v3d, rv3d, C)) {
+    if (undo_group) {
+      ED_undo_grouped_push(C, str);
     }
+    else {
+      ED_undo_push(C, str);
+    }
+    return true;
   }
   return false;
 }

source/blender/editors/space_view3d/view3d_view.c

@@ -202,6 +202,8 @@ static void sync_viewport_camera_smoothview(bContext *C,
                                               .quat = other_rv3d->viewquat,
                                               .dist = &other_rv3d->dist,
                                               .lens = &other_v3d->lens,
+                                              /* No undo because this switches cameras. */
+                                              .undo_str = NULL,
                                           });
                   }
                   else {
@@ -256,6 +258,8 @@ static int view3d_setobjectascamera_exec(bContext *C, wmOperator *op)
                                 .quat = rv3d->viewquat,
                                 .dist = &rv3d->dist,
                                 .lens = &v3d->lens,
+                                /* No undo because this switches cameras. */
+                                .undo_str = NULL,
                             });
     }
 
@@ -939,6 +943,8 @@ static bool view3d_localview_init(const Depsgraph *depsgraph,
                                      .quat = rv3d->viewquat,
                                      .dist = ok_dist ? &dist_new : NULL,
                                      .lens = &v3d->lens,
+                                     /* No undo because this doesn't move the camera. */
+                                     .undo_str = NULL,
                                  });
       }
     }
@@ -1008,6 +1014,8 @@ static void view3d_localview_exit(const Depsgraph *depsgraph,
                                      .ofs = rv3d->localvd->ofs,
                                      .quat = rv3d->localvd->viewquat,
                                      .dist = &rv3d->localvd->dist,
+                                     /* No undo because this doesn't move the camera. */
+                                     .undo_str = NULL,
                                  });
       }