Skip to content

Commit

Permalink
SDA: Fix start time calculation when changing timelines.
Browse files Browse the repository at this point in the history
We don't necessarily know the correct start time when changing
timelines. Thus, if the animation is already running, we should
fall back to using a deferred start time similar to when playing
an animation attached to a scroll timeline.

While fixing this issue, it became apparent that there was an error
in how we normalized time based animations.  The full timeline
duration is used even if the animation range aligns with only a
portion of the timeline.  This bug does not manifest if using duration
auto, but does appear if using an explicit duration.

(cherry picked from commit 2b175a5)

Bug: 1450710
Change-Id: I82ea3279af1ecaaa914614d6f07460511634d8a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4582749
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1152602}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4608623
Cr-Commit-Position: refs/branch-heads/5790@{#687}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
kevers-google authored and Chromium LUCI CQ committed Jun 13, 2023
1 parent 0e1f7d9 commit 7a222ed
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 30 deletions.
17 changes: 7 additions & 10 deletions third_party/blink/renderer/core/animation/animation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -959,26 +959,23 @@ void Animation::setTimeline(AnimationTimeline* timeline) {
}

if (timeline && !timeline->IsMonotonicallyIncreasing()) {
ApplyPendingPlaybackRate();
AnimationTimeDelta boundary_time =
(playback_rate_ > 0) ? AnimationTimeDelta() : EffectEnd();
switch (old_play_state) {
case kIdle:
break;

case kRunning:
case kFinished:
// A non-monotonic timeline has a fixed start time at the beginning or
// end of the timeline.
start_time_ = boundary_time;
break;
if (old_current_time) {
start_time_ = absl::nullopt;
hold_time_ = progress * EffectEnd();
}
PlayInternal(AutoRewind::kEnabled, ASSERT_NO_EXCEPTION);
return;

case kPaused:
if (old_current_time) {
start_time_ = absl::nullopt;
hold_time_ = progress * EffectEnd();
} else if (PendingInternal()) {
start_time_ = boundary_time;
}
break;

Expand Down Expand Up @@ -1557,7 +1554,7 @@ void Animation::PlayInternal(AutoRewind auto_rewind,
// start time.
if (has_finite_timeline && auto_rewind == AutoRewind::kEnabled) {
auto_align_start_time_ = true;
hold_time_ = CalculateCurrentTime();
hold_time_ = CurrentTimeInternal();
}

// 9. If animation’s hold time is resolved, let its start time be unresolved.
Expand Down
19 changes: 13 additions & 6 deletions third_party/blink/renderer/core/animation/animation_effect.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,23 @@ void AnimationEffect::EnsureNormalizedTiming() const {
} else {
// End time is not 0 or infinite.
// Convert to percentages then multiply by the timeline_duration

// TODO(kevers): Revisit once % delays are supported. At present,
// % delays are zero and the following product aligns with the animation
// range. Note the range duration will need to be plumbed through to
// InertEffect via CSSAnimationProxy. One more reason to try and get rid
// of InertEffect.
AnimationTimeDelta range_duration =
IntrinsicIterationDuration() * timing_.iteration_count;

normalized_->start_delay =
(timing_.start_delay.AsTimeValue() / end_time) *
normalized_->timeline_duration.value();
(timing_.start_delay.AsTimeValue() / end_time) * range_duration;

normalized_->end_delay = (timing_.end_delay.AsTimeValue() / end_time) *
normalized_->timeline_duration.value();
normalized_->end_delay =
(timing_.end_delay.AsTimeValue() / end_time) * range_duration;

normalized_->iteration_duration =
(timing_.iteration_duration.value() / end_time) *
normalized_->timeline_duration.value();
(timing_.iteration_duration.value() / end_time) * range_duration;
}
} else {
// Default (auto) duration with a non-monotonic timeline case.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,71 +67,89 @@
element.setAttribute('id', 'element');
container.append(element);
});
func();
await func();
} finally {
while (container.firstChild)
container.firstChild.remove();
}
}, description);
}

test_animation_timeline(() => {
test_animation_timeline(async () => {
let animation = element.getAnimations()[0];
assert_equals(getComputedStyle(element).width, '120px');
element.style = 'animation-timeline:--timeline2';
await animation.ready;

assert_equals(getComputedStyle(element).width, '140px');
}, 'Changing animation-timeline changes the timeline (sanity check)');

test_animation_timeline(() => {
test_animation_timeline(async () => {
let animation = element.getAnimations()[0];
assert_equals(getComputedStyle(element).width, '120px');

// Set a (non-CSS) ScrollTimeline on the CSSAnimation.
let timeline4 = new ScrollTimeline({ source: scroller4 });

element.getAnimations()[0].timeline = timeline4;
animation.timeline = timeline4;
await animation.ready;
assert_equals(getComputedStyle(element).width, '180px');

// Changing the animation-timeline property should have no effect.
element.style = 'animation-timeline:--timeline2';
await animation.ready;

assert_equals(getComputedStyle(element).width, '180px');
}, 'animation-timeline ignored after setting timeline with JS ' +
'(ScrollTimeline from JS)');

test_animation_timeline(() => {
test_animation_timeline(async () => {
let animation = element.getAnimations()[0];
assert_equals(getComputedStyle(element).width, '120px');

let animation = element.getAnimations()[0];
let timeline1 = animation.timeline;
element.style = 'animation-timeline:--timeline2';
await animation.ready;
assert_equals(getComputedStyle(element).width, '140px');

animation.timeline = timeline1;
await animation.ready;

assert_equals(getComputedStyle(element).width, '120px');

// Should have no effect.
element.style = 'animation-timeline:--timeline3';
await animation.ready;

assert_equals(getComputedStyle(element).width, '120px');
}, 'animation-timeline ignored after setting timeline with JS ' +
'(ScrollTimeline from CSS)');

test_animation_timeline(() => {
test_animation_timeline(async () => {
let animation = element.getAnimations()[0];
assert_equals(getComputedStyle(element).width, '120px');
element.getAnimations()[0].timeline = document.timeline;
animation.timeline = document.timeline;
await animation.ready;

// (The animation continues from where the previous timeline left it).
assert_equals(getComputedStyle(element).width, '120px');

// Changing the animation-timeline property should have no effect.
element.style = 'animation-timeline:--timeline2';
await animation.ready;
assert_equals(getComputedStyle(element).width, '120px');
}, 'animation-timeline ignored after setting timeline with JS (document timeline)');

test_animation_timeline(() => {
test_animation_timeline(async () => {
let animation = element.getAnimations()[0];
assert_equals(getComputedStyle(element).width, '120px');
element.getAnimations()[0].timeline = null;
animation.timeline = null;
assert_false(animation.pending);
assert_equals(getComputedStyle(element).width, '120px');

// Changing the animation-timeline property should have no effect.
element.style = 'animation-timeline:--timeline2';
assert_false(animation.pending);
assert_equals(getComputedStyle(element).width, '120px');
}, 'animation-timeline ignored after setting timeline with JS (null)');
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<!doctype html>
<meta charset=utf-8>
<title>Scroll based animation: AnimationEffect.getComputedTiming</title>
<link rel="help" href="https://www.w3.org/TR/web-animations-2/#dom-animationeffect-getcomputedtiming">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/web-animations/testcommon.js"></script>
<script src="testcommon.js"></script>
<style>
.scroller {
overflow: auto;
height: 100px;
width: 100px;
will-change: transform;
}
.contents {
height: 1000px;
width: 100%;
}
</style>
<body>
<div id="log"></div>
<script type="text/javascript">

//------------------------------------
// Time-based duration
//------------------------------------

test(t => {
const anim = createScrollLinkedAnimationWithTiming(t, {duration: 1000 });
assert_equals(anim.effect.getTiming().duration, 1000);
assert_percents_equal(anim.effect.getComputedTiming().duration, 100);
}, 'Computed duration in percent even when specified in ms');

test(t => {
const anim = createScrollLinkedAnimationWithTiming(t, { duration: 1000 });
anim.rangeStart = { offset: CSS.percent(20) };
anim.rangeEnd = { offset: CSS.percent(80) };
assert_equals(anim.effect.getTiming().duration, 1000);
assert_percents_equal(anim.effect.getComputedTiming().duration, 60);
}, 'Time-based duration normalized to fill animation range.');

test(t => {
const anim =
createScrollLinkedAnimationWithTiming(
t, {duration: 700, delay: 100, endDelay: 200 });
assert_equals(anim.effect.getTiming().duration, 700);
assert_percents_equal(anim.effect.getComputedTiming().duration, 70);
}, 'Time-based duration normalized to preserve proportional delays.');

//-------------------------------------------------
// Duration 'auto' = Intrinsic iteration duration
//-------------------------------------------------

test(t => {
const anim = createScrollLinkedAnimationWithTiming(t, {});
assert_equals(anim.effect.getTiming().duration, 'auto');
assert_percents_equal(anim.effect.getComputedTiming().duration, 100);
}, 'Intrinsic iteration duration fills timeline.');

test(t => {
const anim = createScrollLinkedAnimationWithTiming(t, {});
anim.rangeStart = { offset: CSS.percent(30) };
anim.rangeEnd = { offset: CSS.percent(90) };
assert_equals(anim.effect.getTiming().duration, 'auto');
assert_percents_equal(anim.effect.getComputedTiming().duration, 60);
}, 'Intrinsic iteration duration accounts for animation range.');

test(t => {
const anim =
createScrollLinkedAnimationWithTiming(
t, { iterations: 4 });
assert_equals(anim.effect.getTiming().duration, 'auto');
assert_percents_equal(anim.effect.getComputedTiming().duration, 25);
}, 'Intrinsic iteration duration accounts for number of iterations');

</script>
</body>
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
<script src="testcommon.js"></script>
<style>
.scroller {
overflow: auto;
overflow-x: hidden;
overflow-y: auto;
height: 200px;
width: 100px;
will-change: transform;
Expand All @@ -26,6 +27,12 @@
.anim {
animation: anim 1s paused linear;
}
#target {
height: 100px;
width: 100px;
background-color: green;
margin-top: -1000px;
}
</style>
<body>
<script>
Expand Down Expand Up @@ -91,6 +98,17 @@
}
}

function createViewTimeline(t) {
const parent = document.querySelector('.scroller');
const elem = document.createElement('div');
elem.id = 'target';
t.add_cleanup(() => {
elem.remove();
});
parent.appendChild(elem);
return new ViewTimeline({ subject: elem });
}

promise_test(async t => {
const scrollTimeline = createScrollTimeline(t);
await updateScrollPosition(scrollTimeline, 100);
Expand Down Expand Up @@ -153,8 +171,9 @@
await animation.ready;

animation.timeline = scrollTimeline;
assert_false(animation.pending);
assert_true(animation.pending);
assert_equals(animation.playState, 'running');
await animation.ready;
assert_scroll_synced_times(animation, 10, 10);
}, 'Setting a scroll timeline on a running animation synchronizes the ' +
'currentTime of the animation with the scroll position.');
Expand Down Expand Up @@ -278,6 +297,8 @@

animation.timeline = scrollTimeline;
assert_equals(animation.playState, 'running');
await animation.ready;

assert_percents_equal(animation.currentTime, 10);
}, 'Switching from a null timeline to a scroll timeline on an animation with ' +
'a resolved start time preserved the play state');
Expand All @@ -295,6 +316,8 @@
assert_percents_equal(animation.currentTime, 10);

animation.timeline = secondScrollTimeline;
await animation.ready;

assert_percents_equal(animation.currentTime, 20);
}, 'Switching from one scroll timeline to another updates currentTime');

Expand Down Expand Up @@ -368,5 +391,39 @@
}, 'Switching from a document timeline to a scroll timeline on an infinite ' +
'duration animation.');


promise_test(async t => {
const scrollTimeline = createScrollTimeline(t);
const view_timeline = createViewTimeline(t);
await updateScrollPosition(scrollTimeline, 100);
const animation = createAnimation(t);
animation.timeline = scrollTimeline;
// Range name is ignored while attached to a non-view scroll-timeline.
// Offsets are still applied to the scroll-timeline.
animation.rangeStart = { rangeName: 'contain', offset: CSS.percent(10) };
animation.rangeEnd = { rangeName: 'contain', offset: CSS.percent(90) };
await animation.ready;

assert_scroll_synced_times(animation, 10, 0);
assert_percents_equal(animation.startTime, 10);

animation.timeline = view_timeline;
assert_true(animation.pending);
await animation.ready;

// Cover range is [0px, 300px]
// Contain range is [100px, 200px]
// start time = (contain 10% pos - cover start pos) / cover range * 100%
const expected_start_time = 110 / 300 * 100;
// timeline time = (scroll pos - cover start pos) / cover range * 100%
const expected_timeline_time = 100 / 300 * 100;
// current time = timeline time - start time.
const expected_current_time = expected_timeline_time - expected_start_time;

assert_percents_equal(animation.startTime, expected_start_time);
assert_percents_equal(animation.timeline.currentTime, expected_timeline_time);
assert_percents_equal(animation.currentTime, expected_current_time);
}, 'Changing from a scroll-timeline to a view-timeline updates start time.');

</script>
</body>
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,13 @@
assert_opacity_equals(1, `Opacity with document timeline`);

anim.timeline = new ViewTimeline( { subject: target });
await waitForNextFrame();
await anim.ready;

assert_progress_equals(anim, 0.5, `Progress at contain 50%`);
assert_opacity_equals(0.5, `Opacity at contain 50%`);

anim.timeline = document.timeline;
assert_false(anim.pending);
await waitForNextFrame();
assert_opacity_equals(1, `Opacity after resetting timeline`);

Expand Down Expand Up @@ -251,7 +252,7 @@
assert_progress_equals(
anim, 0.5, `Progress at contain 50% after effect change`);
assert_opacity_equals(0.5, `Opacity at contain 50% after effect change`);
}, 'Timeline offsets in programmetic keyframes resolved when updating ' +
}, 'Timeline offsets in programmatic keyframes resolved when updating ' +
'the animation effect');
}

Expand Down

0 comments on commit 7a222ed

Please sign in to comment.