Bug 211068

Summary: [Web Animations] Missing webAnimationsCSSIntegrationEnabled code path in RenderLayer::currentTransform()
Product: WebKit Reporter: Antoine Quint <graouts>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: NEW    
Severity: Normal CC: dino, esprehn+autocc, ews-watchlist, fred.wang, glenn, graouts, kondapallykalyan, krit, pdr, simon.fraser
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=201892
Attachments:
Description Flags
Patch
none
Patch
simon.fraser: review-
Test none

Antoine Quint
Reported 2020-04-27 02:50:16 PDT
We're missing a code path for webAnimationsCSSIntegrationEnabled in the applyOrigin == RenderStyle::ExcludeTransformOrigin case.
Attachments
Patch (3.57 KB, patch)
2020-04-27 04:30 PDT, Antoine Quint 		no flags
DetailsFormatted DiffDiff
Patch (2.17 KB, patch)
2020-04-27 07:55 PDT, Antoine Quint 		simon.fraser: review-
DetailsFormatted DiffDiff
Test (1.60 KB, text/html)
2020-04-28 10:08 PDT, Simon Fraser (smfr) 		no flags
Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antoine Quint
Comment 1 2020-04-27 04:26:21 PDT
The only call site is RenderLayerBacking::getCurrentTransform().
Antoine Quint
Comment 2 2020-04-27 04:30:22 PDT
Created attachment 397670 [details] Patch
Antoine Quint
Comment 3 2020-04-27 07:55:30 PDT
Created attachment 397682 [details] Patch
Simon Fraser (smfr)
Comment 4 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().
Simon Fraser (smfr)
Comment 5 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.
Antoine Quint
Comment 6 2020-04-27 09:17:12 PDT
This code was added in r251252, the fix to bug 201892.
Antoine Quint
Comment 7 2020-04-27 09:48:44 PDT
I'm renaming the accessor to legacyAnimation() in bug 211082.
Antoine Quint
Comment 8 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()`.
Simon Fraser (smfr)
Comment 9 2020-04-28 10:08:40 PDT
Created attachment 397851 [details] Test Here's a test that hits this codepath.
Antoine Quint
Comment 10 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.
Antoine Quint
Comment 11 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.
Simon Fraser (smfr)
Comment 12 2020-04-29 07:47:44 PDT
It should if you dump visible rects and/or tiling.
Antoine Quint
Comment 13 2023-05-10 12:03:51 PDT
The webAnimationsCSSIntegrationEnabled flag no longer exists, but this may yet still occur.
Note You need to log in before you can comment on or make changes to this bug.