Fix T75881: Animation, limitation of Bézier Handles

Relax limits of FCurve Bézier handles during evaluation. FCurve handles
can be scaled down to avoid the curve looping backward in time. This
scaling was done correctly but over-carefully, posing unnecessary
limitations on the possible slope of FCurves. This commit changes the
scaling approach such that the FCurve can become near-vertical.

Bump Blender's subversion from 291.0.1 to 291.0.2 to ensure that older
animation files are correctly updated.

Reviewed By: sybren

Differential Revision: https://developer.blender.org/D8752

4 files changed, 89 insertions, 24 deletions

diff --git a/source/blender/blenkernel/BKE_blender_version.h b/source/blender/blenkernel/BKE_blender_version.h
index 5ad903a0119..ef5ccc3696d 100644
--- a/source/blender/blenkernel/BKE_blender_version.h
+++ b/source/blender/blenkernel/BKE_blender_version.h
@@ -39,7 +39,7 @@ extern "C" {
 
 /* Blender file format version. */
 #define BLENDER_FILE_VERSION BLENDER_VERSION
-#define BLENDER_FILE_SUBVERSION 1
+#define BLENDER_FILE_SUBVERSION 2
 
 /* Minimum Blender version that supports reading file written with the current
  * version. Older Blender versions will test this and show a warning if the file
diff --git a/source/blender/blenkernel/intern/fcurve.c b/source/blender/blenkernel/intern/fcurve.c
index 8a5ad483ffd..c1233a5ea61 100644
--- a/source/blender/blenkernel/intern/fcurve.c
+++ b/source/blender/blenkernel/intern/fcurve.c
@@ -1308,7 +1308,7 @@ bool test_time_fcurve(FCurve *fcu)
 /** \name F-Curve Calculations
  * \{ */
 
-/* The total length of the handles is not allowed to be more
+/* The length of each handle is not allowed to be more
  * than the horizontal distance between (v1-v4).
  * This is to prevent curve loops.
  */
@@ -1337,15 +1337,17 @@ void correct_bezpart(const float v1[2], float v2[2], float v3[2], const float v4
     return;
   }
 
-  /* the two handles cross over each other, so force them
-   * apart using the proportion they overlap
+  /* To prevent looping or rewinding, handles cannot
+   * exceed the adjacent's keyframe time position.
    */
-  if ((len1 + len2) > len) {
-    fac = len / (len1 + len2);
-
+  if (len1 > len) {
+    fac = len / len1;
     v2[0] = (v1[0] - fac * h1[0]);
     v2[1] = (v1[1] - fac * h1[1]);
+  }
 
+  if (len2 > len) {
+    fac = len / len2;
     v3[0] = (v4[0] - fac * h2[0]);
     v3[1] = (v4[1] - fac * h2[1]);
   }
diff --git a/source/blender/blenloader/intern/versioning_290.c b/source/blender/blenloader/intern/versioning_290.c
index c9cb6930da3..2db2bb3c29e 100644
--- a/source/blender/blenloader/intern/versioning_290.c
+++ b/source/blender/blenloader/intern/versioning_290.c
@@ -25,6 +25,7 @@
 #include "BLI_string.h"
 #include "BLI_utildefines.h"
 
+#include "DNA_anim_types.h"
 #include "DNA_brush_types.h"
 #include "DNA_cachefile_types.h"
 #include "DNA_constraint_types.h"
@@ -272,6 +273,54 @@ static void do_versions_point_attributes(CustomData *pdata)
   }
 }
 
+/* Move FCurve handles towards the control point in such a way that the curve itself doesn't
+ * change. Since 2.91 FCurves are computed slightly differently, which requires this update to keep
+ * the same animation result. Previous versions scaled down overlapping handles during evaluation.
+ * This function applies the old correction to the actual animation data instead. */
+static void do_versions_291_fcurve_handles_limit(FCurve *fcu)
+{
+  uint i = 1;
+  for (BezTriple *bezt = fcu->bezt; i < fcu->totvert; i++, bezt++) {
+    /* Only adjust bezier keyframes. */
+    if (bezt->ipo != BEZT_IPO_BEZ) {
+      continue;
+    }
+
+    BezTriple *nextbezt = bezt + 1;
+    const float v1[2] = {bezt->vec[1][0], bezt->vec[1][1]};
+    const float v2[2] = {bezt->vec[2][0], bezt->vec[2][1]};
+    const float v3[2] = {nextbezt->vec[0][0], nextbezt->vec[0][1]};
+    const float v4[2] = {nextbezt->vec[1][0], nextbezt->vec[1][1]};
+
+    /* If the handles have no length, no need to do any corrections. */
+    if (v1[0] == v2[0] && v3[0] == v4[0]) {
+      continue;
+    }
+
+    /* Calculate handle deltas. */
+    float delta1[2], delta2[2];
+    sub_v2_v2v2(delta1, v1, v2);
+    sub_v2_v2v2(delta2, v4, v3);
+
+    const float len1 = fabsf(delta1[0]); /* Length of handle of first key. */
+    const float len2 = fabsf(delta2[0]); /* Length of handle of second key. */
+
+    /* Overlapping handles used to be internally scaled down in previous versions.
+     * We bake the handles onto these previously virtual values. */
+    const float time_delta = v4[0] - v1[0];
+    const float total_len = len1 + len2;
+    if (total_len <= time_delta) {
+      continue;
+    }
+
+    const float factor = time_delta / total_len;
+    /* Current keyframe's right handle: */
+    madd_v2_v2v2fl(bezt->vec[2], v1, delta1, -factor); /* vec[2] = v1 - factor * delta1 */
+    /* Next keyframe's left handle: */
+    madd_v2_v2v2fl(nextbezt->vec[0], v4, delta2, -factor); /* vec[0] = v4 - factor * delta2 */
+  }
+}
+
 void blo_do_versions_290(FileData *fd, Library *UNUSED(lib), Main *bmain)
 {
   UNUSED_VARS(fd);
@@ -528,16 +577,7 @@ void blo_do_versions_290(FileData *fd, Library *UNUSED(lib), Main *bmain)
     }
   }
 
-  /**
-   * Versioning code until next subversion bump goes here.
-   *
-   * \note Be sure to check when bumping the version:
-   * - "versioning_userdef.c", #BLO_version_defaults_userpref_blend
-   * - "versioning_userdef.c", #do_versions_theme
-   *
-   * \note Keep this message at the bottom of the function.
-   */
-  {
+  if (!MAIN_VERSION_ATLEAST(bmain, 291, 2)) {
     for (Scene *scene = bmain->scenes.first; scene; scene = scene->id.next) {
       RigidBodyWorld *rbw = scene->rigidbody_world;
 
@@ -598,6 +638,28 @@ void blo_do_versions_290(FileData *fd, Library *UNUSED(lib), Main *bmain)
       }
     }
 
+    /* Fix fcurves to allow for new bezier handles behaviour (T75881 and D8752). */
+    for (bAction *act = bmain->actions.first; act; act = act->id.next) {
+      for (FCurve *fcu = act->curves.first; fcu; fcu = fcu->next) {
+        /* Only need to fix Bezier curves with at least 2 keyframes. */
+        if (fcu->totvert < 2 || fcu->bezt == NULL) {
+          return;
+        }
+        do_versions_291_fcurve_handles_limit(fcu);
+      }
+    }
+  }
+
+  /**
+   * Versioning code until next subversion bump goes here.
+   *
+   * \note Be sure to check when bumping the version:
+   * - "versioning_userdef.c", #BLO_version_defaults_userpref_blend
+   * - "versioning_userdef.c", #do_versions_theme
+   *
+   * \note Keep this message at the bottom of the function.
+   */
+  {
     /* Keep this block, even when empty. */
   }
 }
diff --git a/source/blender/blenloader/intern/versioning_userdef.c b/source/blender/blenloader/intern/versioning_userdef.c
index de4cff0101d..955725f0ea7 100644
--- a/source/blender/blenloader/intern/versioning_userdef.c
+++ b/source/blender/blenloader/intern/versioning_userdef.c
@@ -216,6 +216,14 @@ static void do_versions_theme(const UserDef *userdef, bTheme *btheme)
     FROM_DEFAULT_V4_UCHAR(tui.transparent_checker_secondary);
     btheme->tui.transparent_checker_size = U_theme_default.tui.transparent_checker_size;
   }
+  if (!USER_VERSION_ATLEAST(291, 2)) {
+    /* The new defaults for the file browser theme are the same as
+     * the outliner's, and it's less disruptive to just copy them. */
+    copy_v4_v4_uchar(btheme->space_file.back, btheme->space_outliner.back);
+    copy_v4_v4_uchar(btheme->space_file.row_alternate, btheme->space_outliner.row_alternate);
+
+    FROM_DEFAULT_V4_UCHAR(space_image.grid);
+  }
 
   /**
    * Versioning code until next subversion bump goes here.
@@ -228,13 +236,6 @@ static void do_versions_theme(const UserDef *userdef, bTheme *btheme)
    */
   {
     /* Keep this block, even when empty. */
-
-    /* The new defaults for the file browser theme are the same as
-     * the outliner's, and it's less disruptive to just copy them. */
-    copy_v4_v4_uchar(btheme->space_file.back, btheme->space_outliner.back);
-    copy_v4_v4_uchar(btheme->space_file.row_alternate, btheme->space_outliner.row_alternate);
-
-    FROM_DEFAULT_V4_UCHAR(space_image.grid);
   }
 
 #undef FROM_DEFAULT_V4_UCHAR