Summary: | Reversed transform animation not applied alongside other transform animations | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | dino, simon.fraser, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=220653 https://bugs.webkit.org/show_bug.cgi?id=224062 |
||||||||||||
Attachments: |
|
Description
Antoine Quint
2020-11-06 05:05:56 PST
We should also test we switch to software mode if already accelerated and a non-accelerated animation is added. I have the case where we try to add an animation that we think should be accelerated but fails to run accelerated due to the internals of GraphicsLayerCA working fine. I also need to handle the case where we determine the animation cannot be run accelerated before even involving GraphicsLayerCA, such as a steps() timing function. Created attachment 417627 [details]
Patch
Comment on attachment 417627 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417627&action=review > Source/WebCore/animation/DocumentTimeline.cpp:443 > + m_isApplyingPendingAcceleratedAnimations = true; SetForScope<> > Source/WebCore/animation/DocumentTimeline.cpp:466 > + if (keyframeEffect.failedToRunAcceleratedTransformRelatedAnimation()) { It's a bit weird to have the very specific failedToRunAcceleratedTransformRelatedAnimation() function. Could applyPendingAcceleratedActions() instead return an OptionSet<> of what it did? > Source/WebCore/animation/KeyframeEffect.cpp:1773 > + if (downcast<DocumentTimeline>(animation()->timeline())->isApplyingPendingAcceleratedAnimations()) { Not a fan of code like this that needs to know what's on its call stack. (In reply to Simon Fraser (smfr) from comment #5) > Comment on attachment 417627 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=417627&action=review > > > Source/WebCore/animation/DocumentTimeline.cpp:466 > > + if (keyframeEffect.failedToRunAcceleratedTransformRelatedAnimation()) { > > It's a bit weird to have the very specific > failedToRunAcceleratedTransformRelatedAnimation() function. Could > applyPendingAcceleratedActions() instead return an OptionSet<> of what it > did? Yes, I'll do this. > > Source/WebCore/animation/KeyframeEffect.cpp:1773 > > + if (downcast<DocumentTimeline>(animation()->timeline())->isApplyingPendingAcceleratedAnimations()) { > > Not a fan of code like this that needs to know what's on its call stack. Me neither actually, but was a bit lazy there I confess. I'll make it so that a parameter is passed down to effectFailedToRunAcceleratedTransformRelatedAnimation() to indicate whether the change should be made synchronously or as part of an accelerated action. Created attachment 417682 [details]
Patch
Created attachment 417690 [details]
Patch
Committed r271524: <https://trac.webkit.org/changeset/271524> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417690 [details]. |