Tweak the math in ScrollOffsetAnimationCurve::UpdateTarget.

The existing division-by-0 guard was not correct, since MaximumDimension can
return negative values.  This led to occasional wild jumps when scrolling up and
down quickly (because new_velocity had the wrong sign).

This patch addresses the sign issue and improves handling of cases where we have
a large current velocity and a small delta to the new target.

BUG=575
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1553603003

Cr-Commit-Position: refs/heads/master@{#367200}

2 files changed, 64 insertions, 62 deletions

diff --git a/cc/animation/scroll_offset_animation_curve.cc b/cc/animation/scroll_offset_animation_curve.cc
index 6cd823f..371f4f6 100644
--- a/cc/animation/scroll_offset_animation_curve.cc
+++ b/cc/animation/scroll_offset_animation_curve.cc
@@ -19,6 +19,8 @@ namespace cc {
 
 namespace {
 
+const double kEpsilon = 0.01f;
+
 static float MaximumDimension(const gfx::Vector2dF& delta) {
   return std::abs(delta.x()) > std::abs(delta.y()) ? delta.x() : delta.y();
 }
@@ -125,6 +127,33 @@ scoped_ptr<AnimationCurve> ScrollOffsetAnimationCurve::Clone() const {
   return std::move(curve_clone);
 }
 
+static double VelocityBasedDurationBound(gfx::Vector2dF old_delta,
+                                         double old_normalized_velocity,
+                                         double old_duration,
+                                         gfx::Vector2dF new_delta) {
+  double old_delta_max_dimension = MaximumDimension(old_delta);
+  double new_delta_max_dimension = MaximumDimension(new_delta);
+
+  // If we are already at the target, stop animating.
+  if (std::abs(new_delta_max_dimension) < kEpsilon)
+    return 0;
+
+  // Guard against division by zero.
+  if (std::abs(old_delta_max_dimension) < kEpsilon ||
+      std::abs(old_normalized_velocity) < kEpsilon) {
+    return std::numeric_limits<double>::infinity();
+  }
+
+  // Estimate how long it will take to reach the new target at our present
+  // velocity, with some fudge factor to account for the "ease out".
+  double old_true_velocity =
+      old_normalized_velocity * old_delta_max_dimension / old_duration;
+  double bound = (new_delta_max_dimension / old_true_velocity) * 2.5f;
+
+  // If bound < 0 we are moving in the opposite direction.
+  return bound < 0 ? std::numeric_limits<double>::infinity() : bound;
+}
+
 void ScrollOffsetAnimationCurve::UpdateTarget(
     double t,
     const gfx::ScrollOffset& new_target) {
@@ -135,28 +164,36 @@ void ScrollOffsetAnimationCurve::UpdateTarget(
 
   double old_duration =
       (total_animation_duration_ - last_retarget_).InSecondsF();
-  double new_duration =
-      SegmentDuration(new_delta, duration_behavior_).InSecondsF();
-
-  double old_velocity = timing_function_->Velocity(
+  double old_normalized_velocity = timing_function_->Velocity(
       (t - last_retarget_.InSecondsF()) / old_duration);
 
+  // Use the velocity-based duration bound when it is less than the constant
+  // segment duration. This minimizes the "rubber-band" bouncing effect when
+  // old_normalized_velocity is large and new_delta is small.
+  double new_duration =
+      std::min(SegmentDuration(new_delta, duration_behavior_).InSecondsF(),
+               VelocityBasedDurationBound(old_delta, old_normalized_velocity,
+                                          old_duration, new_delta));
+
+  if (new_duration < kEpsilon) {
+    // We are already at or very close to the new target. Stop animating.
+    target_value_ = new_target;
+    total_animation_duration_ = base::TimeDelta::FromSecondsD(t);
+    return;
+  }
+
   // TimingFunction::Velocity gives the slope of the curve from 0 to 1.
   // To match the "true" velocity in px/sec we must adjust this slope for
   // differences in duration and scroll delta between old and new curves.
-  const double kEpsilon = 0.01f;
-  double new_delta_max_dimension = MaximumDimension(new_delta);
-  double new_velocity =
-      new_delta_max_dimension < kEpsilon  // Guard against division by 0.
-          ? old_velocity
-          : old_velocity * (new_duration / old_duration) *
-                (MaximumDimension(old_delta) / new_delta_max_dimension);
+  double new_normalized_velocity =
+      old_normalized_velocity * (new_duration / old_duration) *
+      (MaximumDimension(old_delta) / MaximumDimension(new_delta));
 
   initial_value_ = current_position;
   target_value_ = new_target;
   total_animation_duration_ = base::TimeDelta::FromSecondsD(t + new_duration);
   last_retarget_ = base::TimeDelta::FromSecondsD(t);
-  timing_function_ = EaseOutWithInitialVelocity(new_velocity);
+  timing_function_ = EaseOutWithInitialVelocity(new_normalized_velocity);
 }
 
 }  // namespace cc
diff --git a/cc/animation/scroll_offset_animation_curve_unittest.cc b/cc/animation/scroll_offset_animation_curve_unittest.cc
index 4638ecf..7044b03a 100644
--- a/cc/animation/scroll_offset_animation_curve_unittest.cc
+++ b/cc/animation/scroll_offset_animation_curve_unittest.cc
@@ -128,64 +128,29 @@ TEST(ScrollOffsetAnimationCurveTest, UpdateTarget) {
   gfx::ScrollOffset initial_value(0.f, 0.f);
   gfx::ScrollOffset target_value(0.f, 3600.f);
   scoped_ptr<ScrollOffsetAnimationCurve> curve(
-      ScrollOffsetAnimationCurve::Create(target_value,
-                                         EaseInOutTimingFunction::Create()));
-  curve->SetInitialValue(initial_value);
-  EXPECT_EQ(1.0, curve->Duration().InSecondsF());
-  EXPECT_EQ(1800.0, curve->GetValue(base::TimeDelta::FromSecondsD(0.5)).y());
-  EXPECT_EQ(3600.0, curve->GetValue(base::TimeDelta::FromSecondsD(1.0)).y());
-
-  curve->UpdateTarget(0.5, gfx::ScrollOffset(0.0, 9900.0));
-
-  EXPECT_EQ(2.0, curve->Duration().InSecondsF());
-  EXPECT_EQ(1800.0, curve->GetValue(base::TimeDelta::FromSecondsD(0.5)).y());
-  EXPECT_NEAR(5410.05, curve->GetValue(base::TimeDelta::FromSecondsD(1.0)).y(),
-              0.01);
-  EXPECT_EQ(9900.0, curve->GetValue(base::TimeDelta::FromSecondsD(2.0)).y());
-
-  curve->UpdateTarget(1.0, gfx::ScrollOffset(0.0, 7200.0));
-
-  EXPECT_NEAR(1.705, curve->Duration().InSecondsF(), 0.01);
-  EXPECT_NEAR(5410.05, curve->GetValue(base::TimeDelta::FromSecondsD(1.0)).y(),
-              0.01);
-  EXPECT_EQ(7200.0, curve->GetValue(base::TimeDelta::FromSecondsD(1.705)).y());
-}
-
-TEST(ScrollOffsetAnimationCurveTest, UpdateTargetWithLargeVelocity) {
-  gfx::ScrollOffset initial_value(0.f, 0.f);
-  gfx::ScrollOffset target_value(0.f, 900.f);
-  scoped_ptr<ScrollOffsetAnimationCurve> curve(
-      ScrollOffsetAnimationCurve::Create(target_value,
-                                         EaseInOutTimingFunction::Create()));
-  curve->SetInitialValue(initial_value);
-  EXPECT_EQ(0.5, curve->Duration().InSecondsF());
-
-  EXPECT_EQ(450.0, curve->GetValue(base::TimeDelta::FromSecondsD(0.25)).y());
-
-  // This leads to a new computed velocity larger than 5000.
-  curve->UpdateTarget(0.25, gfx::ScrollOffset(0.0, 450.0001));
-
-  EXPECT_NEAR(0.25015, curve->Duration().InSecondsF(), 0.0001);
-  EXPECT_NEAR(450.0,
-              curve->GetValue(base::TimeDelta::FromSecondsD(0.22501)).y(),
-              0.001);
-  EXPECT_NEAR(450.0,
-              curve->GetValue(base::TimeDelta::FromSecondsD(0.225015)).y(),
-              0.001);
-}
-
-TEST(ScrollOffsetAnimationCurveTest, UpdateTargetConstantDuration) {
-  gfx::ScrollOffset initial_value(0.f, 0.f);
-  gfx::ScrollOffset target_value(0.f, 3600.f);
-  scoped_ptr<ScrollOffsetAnimationCurve> curve(
       ScrollOffsetAnimationCurve::Create(
           target_value, EaseInOutTimingFunction::Create(),
           ScrollOffsetAnimationCurve::DurationBehavior::CONSTANT));
   curve->SetInitialValue(initial_value);
   EXPECT_EQ(0.2, curve->Duration().InSecondsF());
+  EXPECT_EQ(1800.0, curve->GetValue(base::TimeDelta::FromSecondsD(0.1)).y());
+  EXPECT_EQ(3600.0, curve->GetValue(base::TimeDelta::FromSecondsD(0.2)).y());
 
   curve->UpdateTarget(0.1, gfx::ScrollOffset(0.0, 9900.0));
+
   EXPECT_EQ(0.3, curve->Duration().InSecondsF());
+  EXPECT_EQ(1800.0, curve->GetValue(base::TimeDelta::FromSecondsD(0.1)).y());
+  EXPECT_NEAR(6827.59, curve->GetValue(base::TimeDelta::FromSecondsD(0.2)).y(),
+              0.01);
+  EXPECT_EQ(9900.0, curve->GetValue(base::TimeDelta::FromSecondsD(0.3)).y());
+
+  curve->UpdateTarget(0.2, gfx::ScrollOffset(0.0, 7200.0));
+
+  // A closer target at high velocity reduces the duration.
+  EXPECT_NEAR(0.22, curve->Duration().InSecondsF(), 0.01);
+  EXPECT_NEAR(6827.59, curve->GetValue(base::TimeDelta::FromSecondsD(0.2)).y(),
+              0.01);
+  EXPECT_EQ(7200.0, curve->GetValue(base::TimeDelta::FromSecondsD(0.22)).y());
 }
 
 }  // namespace