Commit 26664074 authored by Yi Gu's avatar Yi Gu Committed by Commit Bot

Fix AnimationTiming calculation regarding active time and phase

Our codes do not match the spec in calculating the animation effect
phases [1] and its active time [2]. This patch updated the current
implementation by spec and fixed the related layout tests.

[1] https://drafts.csswg.org/web-animations/#before-active-boundary-time
[2] https://drafts.csswg.org/web-animations/#calculating-the-active-time

Change-Id: Ia9ccea246835ddd2e2a3970d9279e008de9e699c
Bug: 909785
Reviewed-on: https://chromium-review.googlesource.com/c/1352710
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613163}
parent 583d3a0c
...@@ -180,9 +180,15 @@ void AnimationEffect::UpdateInheritedTime(double inherited_time, ...@@ -180,9 +180,15 @@ void AnimationEffect::UpdateInheritedTime(double inherited_time,
double time_to_next_iteration = std::numeric_limits<double>::infinity(); double time_to_next_iteration = std::numeric_limits<double>::infinity();
if (needs_update) { if (needs_update) {
const double active_duration = RepeatedDuration(); const double active_duration = RepeatedDuration();
// TODO(yigu): Direction of WorkletAnimation is always forwards based on
// the calculation. Need to unify the logic to handle it correctly.
// https://crbug.com/896249.
const AnimationDirection direction =
(GetAnimation() && GetAnimation()->playbackRate() < 0) ? kBackwards
: kForwards;
const Phase current_phase = const Phase current_phase =
CalculatePhase(active_duration, local_time, timing_); CalculatePhase(active_duration, local_time, direction, timing_);
// FIXME: parentPhase depends on groups being implemented. // FIXME: parentPhase depends on groups being implemented.
const AnimationEffect::Phase kParentPhase = AnimationEffect::kPhaseActive; const AnimationEffect::Phase kParentPhase = AnimationEffect::kPhaseActive;
const double active_time = CalculateActiveTime( const double active_time = CalculateActiveTime(
...@@ -227,12 +233,21 @@ void AnimationEffect::UpdateInheritedTime(double inherited_time, ...@@ -227,12 +233,21 @@ void AnimationEffect::UpdateInheritedTime(double inherited_time,
const double local_active_duration = const double local_active_duration =
kLocalIterationDuration * timing_.iteration_count; kLocalIterationDuration * timing_.iteration_count;
DCHECK_GE(local_active_duration, 0); DCHECK_GE(local_active_duration, 0);
const double end_time = std::max(
timing_.start_delay + active_duration + timing_.end_delay, 0.0);
const double before_active_boundary_time =
std::max(std::min(timing_.start_delay, end_time), 0.0);
// local_local_time should be greater than or equal to the
// before_active_boundary_time once the local_time goes past the start
// delay.
const double local_local_time = const double local_local_time =
local_time < timing_.start_delay local_time < timing_.start_delay
? local_time ? local_time
: local_active_duration + timing_.start_delay; : std::max(local_active_duration + timing_.start_delay,
const AnimationEffect::Phase local_current_phase = before_active_boundary_time);
CalculatePhase(local_active_duration, local_local_time, timing_);
const AnimationEffect::Phase local_current_phase = CalculatePhase(
local_active_duration, local_local_time, direction, timing_);
const double local_active_time = CalculateActiveTime( const double local_active_time = CalculateActiveTime(
local_active_duration, local_active_duration,
ResolvedFillMode(timing_.fill_mode, IsKeyframeEffect()), ResolvedFillMode(timing_.fill_mode, IsKeyframeEffect()),
......
...@@ -82,6 +82,12 @@ class CORE_EXPORT AnimationEffect : public ScriptWrappable { ...@@ -82,6 +82,12 @@ class CORE_EXPORT AnimationEffect : public ScriptWrappable {
kPhaseAfter, kPhaseAfter,
kPhaseNone, kPhaseNone,
}; };
// Represents the animation direction from the Web Animations spec, see
// https://drafts.csswg.org/web-animations-1/#animation-direction.
enum AnimationDirection {
kForwards,
kBackwards,
};
class EventDelegate : public GarbageCollectedFinalized<EventDelegate> { class EventDelegate : public GarbageCollectedFinalized<EventDelegate> {
public: public:
......
...@@ -50,18 +50,31 @@ static inline double MultiplyZeroAlwaysGivesZero(AnimationTimeDelta x, ...@@ -50,18 +50,31 @@ static inline double MultiplyZeroAlwaysGivesZero(AnimationTimeDelta x,
return x.is_zero() || y == 0 ? 0 : (x * y).InSecondsF(); return x.is_zero() || y == 0 ? 0 : (x * y).InSecondsF();
} }
static inline AnimationEffect::Phase CalculatePhase(double active_duration, // https://drafts.csswg.org/web-animations-1/#animation-effect-phases-and-states
double local_time, static inline AnimationEffect::Phase CalculatePhase(
const Timing& specified) { double active_duration,
double local_time,
AnimationEffect::AnimationDirection direction,
const Timing& specified) {
DCHECK_GE(active_duration, 0); DCHECK_GE(active_duration, 0);
if (IsNull(local_time)) if (IsNull(local_time))
return AnimationEffect::kPhaseNone; return AnimationEffect::kPhaseNone;
double end_time = double end_time = std::max(
specified.start_delay + active_duration + specified.end_delay; specified.start_delay + active_duration + specified.end_delay, 0.0);
if (local_time < std::min(specified.start_delay, end_time)) double before_active_boundary_time =
std::max(std::min(specified.start_delay, end_time), 0.0);
if (local_time < before_active_boundary_time ||
(local_time == before_active_boundary_time &&
direction == AnimationEffect::AnimationDirection::kBackwards)) {
return AnimationEffect::kPhaseBefore; return AnimationEffect::kPhaseBefore;
if (local_time >= std::min(specified.start_delay + active_duration, end_time)) }
double active_after_boundary_time = std::max(
std::min(specified.start_delay + active_duration, end_time), 0.0);
if (local_time > active_after_boundary_time ||
(local_time == active_after_boundary_time &&
direction == AnimationEffect::AnimationDirection::kForwards)) {
return AnimationEffect::kPhaseAfter; return AnimationEffect::kPhaseAfter;
}
return AnimationEffect::kPhaseActive; return AnimationEffect::kPhaseActive;
} }
...@@ -89,13 +102,12 @@ static inline double CalculateActiveTime(double active_duration, ...@@ -89,13 +102,12 @@ static inline double CalculateActiveTime(double active_duration,
AnimationEffect::Phase phase, AnimationEffect::Phase phase,
const Timing& specified) { const Timing& specified) {
DCHECK_GE(active_duration, 0); DCHECK_GE(active_duration, 0);
DCHECK_EQ(phase, CalculatePhase(active_duration, local_time, specified));
switch (phase) { switch (phase) {
case AnimationEffect::kPhaseBefore: case AnimationEffect::kPhaseBefore:
if (fill_mode == Timing::FillMode::BACKWARDS || if (fill_mode == Timing::FillMode::BACKWARDS ||
fill_mode == Timing::FillMode::BOTH) fill_mode == Timing::FillMode::BOTH)
return 0; return std::max(local_time - specified.start_delay, 0.0);
return NullValue(); return NullValue();
case AnimationEffect::kPhaseActive: case AnimationEffect::kPhaseActive:
if (IsActiveInParentPhase(parent_phase, fill_mode)) if (IsActiveInParentPhase(parent_phase, fill_mode))
...@@ -103,9 +115,10 @@ static inline double CalculateActiveTime(double active_duration, ...@@ -103,9 +115,10 @@ static inline double CalculateActiveTime(double active_duration,
return NullValue(); return NullValue();
case AnimationEffect::kPhaseAfter: case AnimationEffect::kPhaseAfter:
if (fill_mode == Timing::FillMode::FORWARDS || if (fill_mode == Timing::FillMode::FORWARDS ||
fill_mode == Timing::FillMode::BOTH) fill_mode == Timing::FillMode::BOTH) {
return std::max(0.0, std::min(active_duration, return std::max(
active_duration + specified.end_delay)); 0.0, std::min(active_duration, local_time - specified.start_delay));
}
return NullValue(); return NullValue();
case AnimationEffect::kPhaseNone: case AnimationEffect::kPhaseNone:
DCHECK(IsNull(local_time)); DCHECK(IsNull(local_time));
......
...@@ -55,6 +55,10 @@ TEST(AnimationTimingCalculationsTest, ActiveTime) { ...@@ -55,6 +55,10 @@ TEST(AnimationTimingCalculationsTest, ActiveTime) {
EXPECT_EQ(0, CalculateActiveTime(20, Timing::FillMode::BOTH, 0, EXPECT_EQ(0, CalculateActiveTime(20, Timing::FillMode::BOTH, 0,
AnimationEffect::kPhaseActive, AnimationEffect::kPhaseActive,
AnimationEffect::kPhaseBefore, timing)); AnimationEffect::kPhaseBefore, timing));
timing.start_delay = -10;
EXPECT_EQ(5, CalculateActiveTime(20, Timing::FillMode::BACKWARDS, -5,
AnimationEffect::kPhaseActive,
AnimationEffect::kPhaseBefore, timing));
// Active Phase // Active Phase
timing.start_delay = 10; timing.start_delay = 10;
......
This is a testharness.js-based test.
PASS Phase calculation for a simple animation effect
PASS Phase calculation for an animation effect with a positive start delay
FAIL Phase calculation for an animation effect with a negative start delay assert_equals: Animation effect is in before phase when current time is -1ms (progress is null with 'none' fill mode) expected (object) null but got (number) 0
PASS Phase calculation for an animation effect with a positive end delay
PASS Phase calculation for an animation effect with a negative end delay lesser in magnitude than the active duration
PASS Phase calculation for an animation effect with a negative end delay equal in magnitude to the active duration
FAIL Phase calculation for an animation effect with a negative end delay greater in magnitude than the active duration assert_not_equals: Animation effect is in before phase when current time is -1ms (progress is non-null with appropriate fill mode) got disallowed value null
PASS Phase calculation for an animation effect with a positive start delay and a negative end delay lesser in magnitude than the active duration
FAIL Phase calculation for an animation effect with a negative start delay and a negative end delay equal in magnitude to the active duration assert_not_equals: Animation effect is in before phase when current time is -1ms (progress is non-null with appropriate fill mode) got disallowed value null
FAIL Phase calculation for an animation effect with a negative start delay and a negative end delay equal greater in magnitude than the active duration assert_not_equals: Animation effect is in before phase when current time is -2ms (progress is non-null with appropriate fill mode) got disallowed value null
FAIL Phase calculation for a simple animation effect with negative playback rate assert_not_equals: Animation effect is in active phase when current time is 1ms got disallowed value null
Harness: the test ran to completion.
...@@ -46,7 +46,7 @@ FAIL Test end delay: iterations:2 duration:100 delay:1 fill:both endDelay:-100 a ...@@ -46,7 +46,7 @@ FAIL Test end delay: iterations:2 duration:100 delay:1 fill:both endDelay:-100 a
PASS Test end delay: iterations:1 iterationStart:2 duration:100 delay:1 fill:both endDelay:-50 PASS Test end delay: iterations:1 iterationStart:2 duration:100 delay:1 fill:both endDelay:-50
FAIL Test end delay: iterations:1 iterationStart:2 duration:100 delay:1 fill:both endDelay:-100 assert_approx_equals: Value of progress in the after phase expected 0 +/- 0.001 but got 1 FAIL Test end delay: iterations:1 iterationStart:2 duration:100 delay:1 fill:both endDelay:-100 assert_approx_equals: Value of progress in the after phase expected 0 +/- 0.001 but got 1
PASS Test negative playback rate: duration:1 delay:1 fill:both playbackRate:-1 PASS Test negative playback rate: duration:1 delay:1 fill:both playbackRate:-1
FAIL Test negative playback rate: duration:0 delay:1 fill:both playbackRate:-1 assert_approx_equals: Value of progress in the before phase expected 0 +/- 0.001 but got 1 FAIL Test negative playback rate: duration:0 delay:1 fill:both playbackRate:-1 assert_approx_equals: Value of progress in the before phase expected 0 +/- 0.001 but got 0.9999999999999999
PASS Test negative playback rate: duration:0 iterations:0 delay:1 fill:both playbackRate:-1 PASS Test negative playback rate: duration:0 iterations:0 delay:1 fill:both playbackRate:-1
Harness: the test ran to completion. Harness: the test ran to completion.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment