Fix T94191: correct (time) translation headers not showing DeltaX
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Dec 21 2021, 12:29 PM.

Details

Reviewers

Germano Cavalcante (mano-wii)

Maniphest Tasks

T94191: DeltaX does not show how many frames you are moving your keyframes

Commits

rB66addab27ac7: Fix T94191: correct (time) translation headers not showing DeltaX
rB6db09197242c: Fix T94191: correct (time) translation headers not showing DeltaX

Summary

Caused by rBb0d9e6797fb8: Fix T87173: wrong Auto-Snap in animation editors

For the header (both Graph Editor case in general headerTranslation as
well as headerTimeTranslate) we are only ever interested in deltas
(not absolute values).
Since culprit commit, snapFrameTransform was not working with deltas
anymore (for good reason), but we have to compensate for this.
For the Graph Editor, this only worked "by accident" in rB7192e57d63a5,
since ival is still zero at this point.

So now, be precise and feed only deltas [with a dummy 0.0f as ival] into
snapFrameTransform.

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Dec 21 2021, 12:29 PM

Philipp Oeser (lichtwerk) created this revision.

Philipp Oeser (lichtwerk) added a task: T94191: DeltaX does not show how many frames you are moving your keyframes.

I'm not sure this is really correct.
Some snap modes consider the element's actual position, not the delta.
If I'm not mistaken, to get a correct delta, the initial value needs to be subtracted after the snap.

In D13641#360860, @Germano Cavalcante (mano-wii) wrote:

I'm not sure this is really correct.
Some snap modes consider the element's actual position, not the delta.
If I'm not mistaken, to get a correct delta, the initial value needs to be subtracted after the snap.

Got an example?

In D13641#360864, @Philipp Oeser (lichtwerk) wrote:

Got an example?

Snap of type Nearest Marker seems to use the final value and not the delta X.

I didn't test and compare as it was before. But seeing the description in the header ("Delta X"), I think this is to be expected?

diff --git a/source/blender/editors/transform/transform_mode_timetranslate.c b/source/blender/editors/transform/transform_mode_timetranslate.c
index 5f2a2e472c5..84ca5d3ae54 100644
--- a/source/blender/editors/transform/transform_mode_timetranslate.c
+++ b/source/blender/editors/transform/transform_mode_timetranslate.c
@@ -62,27 +62,28 @@ static void headerTimeTranslate(TransInfo *t, char str[UI_MAX_DRAW_STR])
     float ival = TRANS_DATA_CONTAINER_FIRST_OK(t)->data->ival;
     float val = ival + t->values_final[0];
 
-    float snap_val = val;
-    snapFrameTransform(t, autosnap, ival, val, &snap_val);
+    snapFrameTransform(t, autosnap, ival, val, &val);
+    float delta_x = val - ival;
 
     if (ELEM(autosnap, SACTSNAP_SECOND, SACTSNAP_TSTEP)) {
       /* Convert to seconds. */
       const Scene *scene = t->scene;
       const double secf = FPS;
-      snap_val /= secf;
+      delta_x /= secf;
+      val /= secf;
     }
 
     if (autosnap == SACTSNAP_FRAME) {
-      BLI_snprintf(&tvec[0], NUM_STR_REP_LEN, "%.2f (%.4f)", snap_val, val);
+      BLI_snprintf(&tvec[0], NUM_STR_REP_LEN, "%.2f (%.4f)", delta_x, val);
     }
     else if (autosnap == SACTSNAP_SECOND) {
-      BLI_snprintf(&tvec[0], NUM_STR_REP_LEN, "%.2f sec (%.4f)", snap_val, val);
+      BLI_snprintf(&tvec[0], NUM_STR_REP_LEN, "%.2f sec (%.4f)", delta_x, val);
     }
     else if (autosnap == SACTSNAP_TSTEP) {
-      BLI_snprintf(&tvec[0], NUM_STR_REP_LEN, "%.4f sec", snap_val);
+      BLI_snprintf(&tvec[0], NUM_STR_REP_LEN, "%.4f sec", delta_x);
     }
     else {
-      BLI_snprintf(&tvec[0], NUM_STR_REP_LEN, "%.4f", snap_val);
+      BLI_snprintf(&tvec[0], NUM_STR_REP_LEN, "%.4f", delta_x);
     }
   }
 
diff --git a/source/blender/editors/transform/transform_mode_translate.c b/source/blender/editors/transform/transform_mode_translate.c
index 19d0c6d39a3..9c6e1b01f86 100644
--- a/source/blender/editors/transform/transform_mode_translate.c
+++ b/source/blender/editors/transform/transform_mode_translate.c
@@ -229,7 +229,8 @@ static void headerTranslation(TransInfo *t, const float vec[3], char str[UI_MAX_
       const short autosnap = getAnimEdit_SnapMode(t);
       float ival = TRANS_DATA_CONTAINER_FIRST_OK(t)->data->ival;
       float val = ival + dvec[0];
-      snapFrameTransform(t, autosnap, ival, val, &dvec[0]);
+      snapFrameTransform(t, autosnap, ival, val, &val);
+      dvec[0] = val - ival;
     }
 
     if (t->con.mode & CON_APPLY) {

Will abandon this already, we found some more issues in chat, so there is going to be an updated commit by @Germano Cavalcante (mano-wii) at some point.