Bug 217842 - Support accelerated animation of individual transform CSS properties
Summary: Support accelerated animation of individual transform CSS properties
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks: 178117
  Show dependency tree / graph
 
Reported: 2020-10-16 14:22 PDT by Antoine Quint
Modified: 2021-01-27 12:04 PST (History)
12 users (show)

See Also:

Attachments
Patch (56.42 KB, patch)
2020-10-16 14:50 PDT, Antoine Quint 		dino: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2020-10-16 14:22:29 PDT 
Support accelerated animation of individual transform CSS properties
Comment 1 Radar WebKit Bug Importer 2020-10-16 14:23:32 PDT 
<rdar://problem/70391914>
Comment 2 Antoine Quint 2020-10-16 14:50:17 PDT 
Created attachment 411617 [details]
Patch
Comment 3 Antoine Quint 2020-10-16 14:50:23 PDT 
<rdar://problem/70391914>
Comment 4 Dean Jackson 2020-10-16 15:10:11 PDT 
Comment on attachment 411617 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411617&action=review

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2893
> +        animation.m_beginTime = Seconds::fromNanoseconds(1);

Why not zero? not that it matters
Comment 5 Antoine Quint 2020-10-16 15:11:17 PDT 
(In reply to Dean Jackson from comment #4)
> Comment on attachment 411617 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=411617&action=review
> 
> > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2893
> > +        animation.m_beginTime = Seconds::fromNanoseconds(1);
> 
> Why not zero? not that it matters

Because the CA API gives special meaning to zero in this context, using the current time when the CA transaction occur. We need this time to be as early as possible explicitly.
Comment 6 Antoine Quint 2020-10-16 15:44:03 PDT 
Committed r268615: <https://trac.webkit.org/changeset/268615>
Comment 7 Ryan Haddad 2020-10-16 17:44:29 PDT 
(In reply to Antoine Quint from comment #6)
> Committed r268615: <https://trac.webkit.org/changeset/268615>
As indicated by EWS, these tests are failing on Mojave bots now that the change landed:

webanimations/accelerated-transform-related-animation-property-order.html
webanimations/accelerated-translate-animation-additional-animation-added-in-flight.html
webanimations/accelerated-translate-animation-underlying-transform-changed-in-flight.html
webanimations/accelerated-translate-animation-with-transform.html
webanimations/accelerated-translate-animation.html

https://build.webkit.org/results/Apple-Mojave-Release-WK2-Tests/r268619%20(16992)/results.html
Comment 8 Dean Jackson 2020-10-16 19:12:00 PDT 
Updated Test Expectations to skip these tests on Mojave.

r268627
Comment 9 Ryan Haddad 2020-10-16 19:20:05 PDT 
(In reply to Dean Jackson from comment #8)
> Updated Test Expectations to skip these tests on Mojave.
> 
> r268627
Thanks! These are image failures, so we’ll need to switch this from Failure to ImageOnlyFailure. I can fix it up later tonight.
Comment 10 Simon Fraser (smfr) 2020-10-16 19:55:31 PDT 
Why is it OK to break Mojave? That suggests the patch is wrong.
Comment 11 Simon Fraser (smfr) 2020-10-16 20:00:44 PDT 
Like maybe this breaks the !HAVE_CA_WHERE_ADDITIVE_TRANSFORMS_ARE_REVERSED platforms.
Comment 12 Antoine Quint 2020-10-16 22:18:15 PDT 
Most likely, I’ll install Mojave and fix those ASAP.
Comment 13 Simon Fraser (smfr) 2020-10-17 11:03:31 PDT 
Also Windows is broken.