-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace proxied animations in getAnimations reusing proxied CSS animations #89
Changes from all commits
02954b5
00ae9df
1db7fe1
c6daadc
e262ddb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,10 +79,10 @@ FAIL /scroll-animations/css/animation-timeline-deferred.html Animation.timeline | |
FAIL /scroll-animations/css/animation-timeline-deferred.html Animation.timeline returns null for inactive deferred timeline | ||
FAIL /scroll-animations/css/animation-timeline-deferred.html Animation.timeline returns null for inactive (overattached) deferred timeline | ||
FAIL /scroll-animations/css/animation-timeline-ignored.tentative.html Changing animation-timeline changes the timeline (sanity check) | ||
FAIL /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (ScrollTimeline from JS) | ||
PASS /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (ScrollTimeline from JS) | ||
FAIL /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (ScrollTimeline from CSS) | ||
PASS /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (document timeline) | ||
PASS /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (null) | ||
FAIL /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (document timeline) | ||
FAIL /scroll-animations/css/animation-timeline-ignored.tentative.html animation-timeline ignored after setting timeline with JS (null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked and these two new failures are really just exposing existing issues. The first is failing because the animated position is ever so slightly off from the position we had with the scroll timeline - perhaps we need to allow for some skew? The second is that we simply don't currently support setting timeline = null, but before this change we were setting |
||
FAIL /scroll-animations/css/animation-timeline-in-keyframe.html The animation-timeline property may not be used in keyframes | ||
PASS /scroll-animations/css/animation-timeline-multiple.html animation-timeline works with multiple timelines | ||
FAIL /scroll-animations/css/animation-timeline-none.html Animation with animation-timeline:none holds current time at zero | ||
|
@@ -957,4 +957,4 @@ FAIL /scroll-animations/view-timelines/view-timeline-sticky-block.html View time | |
FAIL /scroll-animations/view-timelines/view-timeline-sticky-inline.html View timeline with sticky target, block axis. | ||
FAIL /scroll-animations/view-timelines/view-timeline-subject-size-changes.html View timeline with subject size change after the creation of the animation | ||
FAIL /scroll-animations/view-timelines/zero-intrinsic-iteration-duration.tentative.html Intrinsic iteration duration is non-negative | ||
Passed 392 of 959 tests. | ||
Passed 391 of 959 tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
result may be null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Canceling and restarting a CSS animation will unconditionally create a new ScrollTimeline. Possibly not a big performance hit, but it looked like this was one of the things the previous cache PR was trying to avoid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on
result
possibly beingnull
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, done!
I think createScrollTimeline is pretty inexpensive. Scroll driven animations themselves will generally start once and run forever. Non-scroll driven animations should quickly be determined not to match any of the rules in getAnimationTimeline (and we didn't used to cache misses anyways). There are other easy optimizations we could do here if it became a bottleneck.