Fix #113505: Scale strips in nla with snap active seems broken

Caused by bd305c8d18

`snap_transform_data` adds an offset to each element being transformed.

However, this seems to only work for Move and Extend transformations.

Therefore, the solution is to make the NLA snapping system more generic.

Pull Request: https://projects.blender.org/blender/blender/pulls/113554

source/blender/editors/transform/transform_convert_nla.cc

@@ -634,67 +634,12 @@ static void createTransNlaData(bContext *C, TransInfo *t)
   ANIM_animdata_freelist(&anim_data);
 }
 
-static void invert_snap(eSnapMode &snap_mode)
-{
-  if (snap_mode & SCE_SNAP_TO_FRAME) {
-    snap_mode &= ~SCE_SNAP_TO_FRAME;
-    snap_mode |= SCE_SNAP_TO_SECOND;
-  }
-  else if (snap_mode & SCE_SNAP_TO_SECOND) {
-    snap_mode &= ~SCE_SNAP_TO_SECOND;
-    snap_mode |= SCE_SNAP_TO_FRAME;
-  }
-}
-
-static void snap_transform_data(TransInfo *t, TransDataContainer *tc)
-{
-  /* handle auto-snapping
-   * NOTE: only do this when transform is still running, or we can't restore
-   */
-  if (t->state == TRANS_CANCEL) {
-    return;
-  }
-  if ((t->tsnap.flag & SCE_SNAP) == 0) {
-    return;
-  }
-
-  eSnapMode snap_mode = t->tsnap.mode;
-  if (t->modifiers & MOD_SNAP_INVERT) {
-    invert_snap(snap_mode);
-  }
-
-  float offset = 0;
-  float smallest_snap_delta = FLT_MAX;
-
-  /* In order to move the strip in a block and not each end individually,
-   * find the minimal snap offset first and then shift the whole strip by that amount. */
-  for (int i = 0; i < tc->data_len; i++) {
-    TransData td = tc->data[i];
-    float snap_value;
-    transform_snap_anim_flush_data(t, &td, snap_mode, &snap_value);
-
-    /* The snap_delta measures how far from the unsnapped position the value has moved. */
-    const float snap_delta = *td.loc - snap_value;
-    if (fabs(snap_delta) < fabs(smallest_snap_delta)) {
-      offset = snap_value - td.iloc[0];
-      smallest_snap_delta = snap_delta;
-    }
-  }
-
-  for (int i = 0; i < tc->data_len; i++) {
-    TransData td = tc->data[i];
-    *td.loc = td.iloc[0] + offset;
-  }
-}
-
 static void recalcData_nla(TransInfo *t)
 {
   SpaceNla *snla = (SpaceNla *)t->area->spacedata.first;
-
+  
   TransDataContainer *tc = TRANS_DATA_CONTAINER_FIRST_SINGLE(t);
 
-  snap_transform_data(t, tc);
-
   /* 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).

source/blender/editors/transform/transform_mode_timescale.cc

@@ -33,6 +33,15 @@
 /** \name Transform (Animation Time Scale)
  * \{ */
 
+static void timescale_snap_apply_fn(TransInfo *t, float vec[3])
+{
+  float point[3];
+  getSnapPoint(t, point);
+  const float fac = (point[0] - t->center_global[0]) /
+                    (t->tsnap.snap_source[0] - t->center_global[0]);
+  vec[0] = fac;
+}
+
 static void headerTimeScale(TransInfo *t, char str[UI_MAX_DRAW_STR])
 {
   char tvec[NUM_STR_REP_LEN * 3];
@@ -84,6 +93,9 @@ static void applyTimeScale(TransInfo *t)
   /* handle numeric-input stuff */
   t->vec[0] = t->values[0];
   applyNumInput(&t->num, &t->vec[0]);
+
+  transform_snap_mixed_apply(t, &t->vec[0]);
+
   t->values_final[0] = t->vec[0];
   headerTimeScale(t, str);
 
@@ -94,6 +106,15 @@ static void applyTimeScale(TransInfo *t)
   ED_area_status_text(t->area, str);
 }
 
+static void timescale_transform_matrix_fn(TransInfo *t, float mat_xform[4][4])
+{
+  const float i_loc = mat_xform[3][0];
+  const float startx = t->center_global[0];
+  const float fac = t->values_final[0];
+  const float loc = ((i_loc - startx) * fac) + startx;
+  mat_xform[3][0] = loc;
+}
+
 static void initTimeScale(TransInfo *t, wmOperator * /*op*/)
 {
   float center[2];
@@ -141,9 +162,9 @@ TransModeInfo TransMode_timescale = {
     /*flags*/ T_NULL_ONE,
     /*init_fn*/ initTimeScale,
     /*transform_fn*/ applyTimeScale,
-    /*transform_matrix_fn*/ nullptr,
+    /*transform_matrix_fn*/ timescale_transform_matrix_fn,
     /*handle_event_fn*/ nullptr,
     /*snap_distance_fn*/ nullptr,
-    /*snap_apply_fn*/ nullptr,
+    /*snap_apply_fn*/ timescale_snap_apply_fn,
     /*draw_fn*/ nullptr,
 };

source/blender/editors/transform/transform_snap.cc

@@ -68,6 +68,7 @@ static void snap_target_view3d_fn(TransInfo *t, float *vec);
 static void snap_target_uv_fn(TransInfo *t, float *vec);
 static void snap_target_node_fn(TransInfo *t, float *vec);
 static void snap_target_sequencer_fn(TransInfo *t, float *vec);
+static void snap_target_nla_fn(TransInfo *t, float *vec);
 
 static void snap_source_median_fn(TransInfo *t);
 static void snap_source_center_fn(TransInfo *t);
@@ -963,6 +964,11 @@ static void setSnappingCallback(TransInfo *t)
     /* The target is calculated along with the snap point. */
     return;
   }
+  else if (t->spacetype == SPACE_NLA) {
+    t->tsnap.snap_target_fn = snap_target_nla_fn;
+    /* The target is calculated along with the snap point. */
+    return;
+  }
   else {
     return;
   }
@@ -1200,6 +1206,17 @@ static void snap_target_sequencer_fn(TransInfo *t, float * /*vec*/)
   }
 }
 
+static void snap_target_nla_fn(TransInfo *t, float *vec)
+{
+  BLI_assert(t->spacetype == SPACE_NLA);
+  if (transform_snap_nla_calc(t, vec)) {
+    t->tsnap.status |= (SNAP_TARGET_FOUND | SNAP_SOURCE_FOUND);
+  }
+  else {
+    t->tsnap.status &= ~(SNAP_TARGET_FOUND | SNAP_SOURCE_FOUND);
+  }
+}
+
 /** \} */
 
 /* -------------------------------------------------------------------- */

source/blender/editors/transform/transform_snap.hh

@@ -85,3 +85,4 @@ void transform_snap_anim_flush_data(TransInfo *t,
                                     TransData *td,
                                     eSnapMode autosnap,
                                     float *r_val_final);
+bool transform_snap_nla_calc(TransInfo *t, float *vec);

source/blender/editors/transform/transform_snap_animation.cc

@@ -8,6 +8,9 @@
 
 #include "DNA_anim_types.h"
 
+#include "BLI_math_matrix_types.hh"
+#include "BLI_math_vector.h"
+
 #include "BKE_context.h"
 #include "BKE_nla.h"
 
@@ -17,6 +20,8 @@
 #include "transform.hh"
 #include "transform_snap.hh"
 
+using namespace blender;
+
 /* -------------------------------------------------------------------- */
 /** \name Snapping in Anim Editors
  * \{ */
@@ -68,14 +73,11 @@ void snapFrameTransform(TransInfo *t,
   }
 }
 
-void transform_snap_anim_flush_data(TransInfo *t,
-                                    TransData *td,
-                                    const eSnapMode snap_mode,
-                                    float *r_val_final)
+static void transform_snap_anim_flush_data_ex(
+    TransInfo *t, TransData *td, float val, const eSnapMode snap_mode, float *r_val_final)
 {
   BLI_assert(t->tsnap.flag);
 
-  float val = td->loc[0];
   float ival = td->iloc[0];
   AnimData *adt = static_cast<AnimData *>(!ELEM(t->spacetype, SPACE_NLA, SPACE_SEQ) ? td->extra :
                                                                                       nullptr);
@@ -96,4 +98,85 @@ void transform_snap_anim_flush_data(TransInfo *t,
   *r_val_final = val;
 }
 
+void transform_snap_anim_flush_data(TransInfo *t,
+                                    TransData *td,
+                                    const eSnapMode snap_mode,
+                                    float *r_val_final)
+{
+  transform_snap_anim_flush_data_ex(t, td, td->loc[0], snap_mode, r_val_final);
+}
+
+static void invert_snap(eSnapMode &snap_mode)
+{
+  if (snap_mode & SCE_SNAP_TO_FRAME) {
+    snap_mode &= ~SCE_SNAP_TO_FRAME;
+    snap_mode |= SCE_SNAP_TO_SECOND;
+  }
+  else if (snap_mode & SCE_SNAP_TO_SECOND) {
+    snap_mode &= ~SCE_SNAP_TO_SECOND;
+    snap_mode |= SCE_SNAP_TO_FRAME;
+  }
+}
+
+/* WORKAROUND: The source position is based on the transformed elements.
+ * However, at this stage, the transformation has not yet been applied.
+ * So apply the transformation here. */
+static float2 nla_transform_apply(TransInfo *t, float *vec, float2 &ival)
+{
+  float4x4 mat = float4x4::identity();
+
+  float values_final_prev[4];
+  const size_t values_final_size = sizeof(*t->values_final) * size_t(t->idx_max + 1);
+  memcpy(values_final_prev, t->values_final, values_final_size);
+  memcpy(t->values_final, vec, values_final_size);
+
+  mat[3][0] = ival[0];
+  mat[3][1] = ival[1];
+  transform_apply_matrix(t, mat.ptr());
+
+  memcpy(t->values_final, values_final_prev, values_final_size);
+
+  return mat.location().xy();
+}
+
+bool transform_snap_nla_calc(TransInfo *t, float *vec)
+{
+  TransDataContainer *tc = TRANS_DATA_CONTAINER_FIRST_SINGLE(t);
+
+  eSnapMode snap_mode = t->tsnap.mode;
+  if (t->modifiers & MOD_SNAP_INVERT) {
+    invert_snap(snap_mode);
+  }
+
+  float best_dist = FLT_MAX;
+  float2 best_source = float2(0);
+  float2 best_target = float2(0);
+  bool found = false;
+
+  for (int i = 0; i < tc->data_len; i++) {
+    TransData *td = &tc->data[i];
+    float2 snap_source = td->iloc;
+    float2 snap_target = nla_transform_apply(t, vec, snap_source);
+
+    transform_snap_anim_flush_data_ex(t, td, snap_target[0], snap_mode, &snap_target[0]);
+    const int dist = abs(snap_target[0] - snap_source[0]);
+    if (dist < best_dist) {
+      if (dist != 0) {
+        /* Prioritize non-zero dist for scale. */
+        best_dist = dist;
+      }
+      else if (found) {
+        continue;
+      }
+      best_source = snap_source;
+      best_target = snap_target;
+      found = true;
+    }
+  }
+
+  copy_v2_v2(t->tsnap.snap_source, best_source);
+  copy_v2_v2(t->tsnap.snap_target, best_target);
+  return found;
+}
+
 /** \} */