Skip to content

Commit

Permalink
Tweak the math in ScrollOffsetAnimationCurve::UpdateTarget.
Browse files Browse the repository at this point in the history 
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}
  • Loading branch information
@skobes-chromium
skobes-chromium authored and Commit bot committed Dec 30, 2015
1 parent 450f237 commit a78fdc9
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 62 deletions.
61 changes: 49 additions & 12 deletions cc/animation/scroll_offset_animation_curve.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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
65 changes: 15 additions & 50 deletions cc/animation/scroll_offset_animation_curve_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,56 +125,6 @@ TEST(ScrollOffsetAnimationCurveTest, Clone) {
}

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(
Expand All @@ -183,9 +133,24 @@ TEST(ScrollOffsetAnimationCurveTest, UpdateTargetConstantDuration) {
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
Expand Down

0 comments on commit a78fdc9

Please sign in to comment.