Bug 211068 - [Web Animations] Missing webAnimationsCSSIntegrationEnabled code path in RenderLayer::currentTransform()
Summary: [Web Animations] Missing webAnimationsCSSIntegrationEnabled code path in Rend...
Status: NEW
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:
Depends on:
Blocks:
 
Reported: 2020-04-27 02:50 PDT by Antoine Quint
Modified: 2023-05-10 12:03 PDT (History)
10 users (show)

See Also:


Attachments
Patch (3.57 KB, patch)
2020-04-27 04:30 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (2.17 KB, patch)
2020-04-27 07:55 PDT, Antoine Quint
simon.fraser: review-
Details | Formatted Diff | Diff
Test (1.60 KB, text/html)
2020-04-28 10:08 PDT, Simon Fraser (smfr)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2020-04-27 02:50:16 PDT
We're missing a code path for webAnimationsCSSIntegrationEnabled in the applyOrigin == RenderStyle::ExcludeTransformOrigin case.
Comment 1 Antoine Quint 2020-04-27 04:26:21 PDT
The only call site is RenderLayerBacking::getCurrentTransform().
Comment 2 Antoine Quint 2020-04-27 04:30:22 PDT
Created attachment 397670 [details]
Patch
Comment 3 Antoine Quint 2020-04-27 07:55:30 PDT
Created attachment 397682 [details]
Patch
Comment 4 Simon Fraser (smfr) 2020-04-27 08:57:54 PDT
Comment on attachment 397682 [details]
Patch

This is testable. Please also add an assertion in renderer().animation() or renamed to legacyAnimation().
Comment 5 Simon Fraser (smfr) 2020-04-27 09:00:54 PDT
^ testable like compositing/visible-rect/animated.html with non-default transform origin and an origin-sensitive transform like a rotate.
Comment 6 Antoine Quint 2020-04-27 09:17:12 PDT
This code was added in r251252, the fix to bug 201892.
Comment 7 Antoine Quint 2020-04-27 09:48:44 PDT
I'm renaming the accessor to legacyAnimation() in bug 211082.
Comment 8 Antoine Quint 2020-04-27 14:26:12 PDT
Not sure if we actually need to call animatedStyleForRenderer() here at all, I believe we can't end up in the `if (applyOrigin == RenderStyle::ExcludeTransformOrigin)` code branch with a running `transform` animation since it would be accelerated and thus we'd return earlier on in `RenderLayer::currentTransform()`.
Comment 9 Simon Fraser (smfr) 2020-04-28 10:08:40 PDT
Created attachment 397851 [details]
Test

Here's a test that hits this codepath.
Comment 10 Antoine Quint 2020-04-29 01:38:43 PDT
(In reply to Simon Fraser (smfr) from comment #9)
> Created attachment 397851 [details]
> Test
> 
> Here's a test that hits this codepath.

However it doesn't any behavior different pre and post patch.
Comment 11 Antoine Quint 2020-04-29 01:59:30 PDT
(In reply to Antoine Quint from comment #10)
> (In reply to Simon Fraser (smfr) from comment #9)
> > Created attachment 397851 [details]
> > Test
> > 
> > Here's a test that hits this codepath.
> 
> However it doesn't any behavior different pre and post patch.

I tried to modify the provided test to set a negative delay for the animation to be half-way through but the output is still the same with or without the patch.
Comment 12 Simon Fraser (smfr) 2020-04-29 07:47:44 PDT
It should if you dump visible rects and/or tiling.
Comment 13 Antoine Quint 2023-05-10 12:03:51 PDT
The webAnimationsCSSIntegrationEnabled flag no longer exists, but this may yet still occur.