Fix T87173: wrong Auto-Snap in animation editors

This was partially broken with {rBde9ea94fc6f}.

The `Frame Step` and `Second Step` snapping options were working as if
they were `Nearest Frame` and `Nearest Second` respectively in the
`Dope Sheet` and `NLA` editors.

In the `Graph Editor` the problem was more serious:
"Second Step: ... The keyframe itself moves along as though in snapping
were active at all, while its handles 'stay behind' until it reaches
the next second boundary, at which point the teleport handles to
'catch up'".

The snapping code for these modes was spread across the transform
mode code and `recalcData` of each data type. Therefore, create a
unified snapping code for these options so that all issues are fixed in
one place.

Differetial Revision: https://developer.blender.org/D12241

1 files changed, 18 insertions, 53 deletions

diff --git a/source/blender/editors/transform/transform_convert_nla.c b/source/blender/editors/transform/transform_convert_nla.c
index 4d98b7a489f..7e5b80c2453 100644
--- a/source/blender/editors/transform/transform_convert_nla.c
+++ b/source/blender/editors/transform/transform_convert_nla.c
@@ -41,6 +41,8 @@
 #include "RNA_access.h"
 
 #include "transform.h"
+#include "transform_snap.h"
+
 #include "transform_convert.h"
 #include "transform_snap.h"
 
@@ -292,21 +294,30 @@ void createTransNlaData(bContext *C, TransInfo *t)
 void recalcData_nla(TransInfo *t)
 {
   SpaceNla *snla = (SpaceNla *)t->area->spacedata.first;
-  Scene *scene = t->scene;
-  double secf = FPS;
-  int i;
 
   TransDataContainer *tc = TRANS_DATA_CONTAINER_FIRST_SINGLE(t);
-  TransDataNla *tdn = tc->custom.type.data;
+
+  /* handle auto-snapping
+   * NOTE: only do this when transform is still running, or we can't restore
+   */
+  if (t->state != TRANS_CANCEL) {
+    const short autosnap = getAnimEdit_SnapMode(t);
+    if (autosnap != SACTSNAP_OFF) {
+      TransData *td = tc->data;
+      for (int i = 0; i < tc->data_len; i++, td++) {
+        transform_snap_anim_flush_data(t, td, autosnap, td->loc);
+      }
+    }
+  }
 
   /* For each strip we've got, perform some additional validation of the values
    * that got set before using RNA to set the value (which does some special
    * operations when setting these values to make sure that everything works ok).
    */
-  for (i = 0; i < tc->data_len; i++, tdn++) {
+  TransDataNla *tdn = tc->custom.type.data;
+  for (int i = 0; i < tc->data_len; i++, tdn++) {
     NlaStrip *strip = tdn->strip;
     PointerRNA strip_ptr;
-    short iter;
     int delta_y1, delta_y2;
 
     /* if this tdn has no handles, that means it is just a dummy that should be skipped */
@@ -370,8 +381,7 @@ void recalcData_nla(TransInfo *t)
       next = next->next;
     }
 
-    for (iter = 0; iter < 5; iter++) {
-
+    for (short iter = 0; iter < 5; iter++) {
       const bool pExceeded = (prev != NULL) && (tdn->h1[0] < prev->end);
       const bool nExceeded = (next != NULL) && (tdn->h2[0] > next->start);
 
@@ -410,51 +420,6 @@ void recalcData_nla(TransInfo *t)
       }
     }
 
-    /* handle auto-snapping
-     * NOTE: only do this when transform is still running, or we can't restore
-     */
-    if (t->state != TRANS_CANCEL) {
-      const short autosnap = getAnimEdit_SnapMode(t);
-      switch (autosnap) {
-        case SACTSNAP_FRAME: /* snap to nearest frame */
-        case SACTSNAP_STEP:  /* frame step - this is basically the same,
-                              * since we don't have any remapping going on */
-        {
-          tdn->h1[0] = floorf(tdn->h1[0] + 0.5f);
-          tdn->h2[0] = floorf(tdn->h2[0] + 0.5f);
-          break;
-        }
-
-        case SACTSNAP_SECOND: /* snap to nearest second */
-        case SACTSNAP_TSTEP:  /* second step - this is basically the same,
-                               * since we don't have any remapping going on */
-        {
-          /* This case behaves differently from the rest, since lengths of strips
-           * may not be multiples of a second. If we just naively resize adjust
-           * the handles, things may not work correctly. Instead, we only snap
-           * the first handle, and move the other to fit.
-           *
-           * FIXME: we do run into problems here when user attempts to negatively
-           *        scale the strip, as it then just compresses down and refuses
-           *        to expand out the other end.
-           */
-          float h1_new = (float)(floor(((double)tdn->h1[0] / secf) + 0.5) * secf);
-          float delta = h1_new - tdn->h1[0];
-
-          tdn->h1[0] = h1_new;
-          tdn->h2[0] += delta;
-          break;
-        }
-
-        case SACTSNAP_MARKER: /* snap to nearest marker */
-        {
-          tdn->h1[0] = (float)ED_markers_find_nearest_marker_time(&t->scene->markers, tdn->h1[0]);
-          tdn->h2[0] = (float)ED_markers_find_nearest_marker_time(&t->scene->markers, tdn->h2[0]);
-          break;
-        }
-      }
-    }
-
     /* Use RNA to write the values to ensure that constraints on these are obeyed
      * (e.g. for transition strips, the values are taken from the neighbors)
      *