| Summary: | Cleanup RenderLayer::currentTransform() | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nikolas Zimmermann <zimmermann> | ||||||
| Component: | Layout and Rendering | Assignee: | Nikolas Zimmermann <zimmermann> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bfulgham, changseok, clopez, darin, esprehn+autocc, ews-watchlist, fred.wang, glenn, kondapallykalyan, pdr, rbuis, simon.fraser, webkit-bug-importer, youennf, zalan | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 90738, 237554, 237701, 237711 | ||||||||
| Attachments: |
|
||||||||
|
Description
Nikolas Zimmermann
2022-03-07 13:30:03 PST
Created attachment 454024 [details]
Patch, v1
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess Comment on attachment 454024 [details] Patch, v1 View in context: https://bugs.webkit.org/attachment.cgi?id=454024&action=review > Source/WebCore/rendering/RenderLayer.cpp:1361 > + std::unique_ptr<RenderStyle> animatedStyle = renderBox.animatedStyle(); Why not move this into the if block? > LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-box-will-change-transform-layer.html:57 > + await waitForNextFrame(); Doesn't this work? await new Promise(requestAnimationFrame); > LayoutTests/platform/mac/TestExpectations:2439 > +webkit.org/b/237166 [ BigSur+ ] webgl/pending/conformance/textures/misc/tex-image-video-repeated.html [ Pass Timeout ] You probably do not want this change. (In reply to Rob Buis from comment #3) > Comment on attachment 454024 [details] > Patch, v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=454024&action=review > > > Source/WebCore/rendering/RenderLayer.cpp:1361 > > + std::unique_ptr<RenderStyle> animatedStyle = renderBox.animatedStyle(); > > Why not move this into the if block? Leftover, fixed -- it should be there, to avoid querying animatedStyle() if not necessary. > > > LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-box-will-change-transform-layer.html:57 > > + await waitForNextFrame(); > > Doesn't this work? await new Promise(requestAnimationFrame); I kept the style from the other tests in that directory, that do it like this. > > > LayoutTests/platform/mac/TestExpectations:2439 > > +webkit.org/b/237166 [ BigSur+ ] webgl/pending/conformance/textures/misc/tex-image-video-repeated.html [ Pass Timeout ] > > You probably do not want this change. Right. Created attachment 454214 [details]
Patch, v2
Comment on attachment 454214 [details] Patch, v2 View in context: https://bugs.webkit.org/attachment.cgi?id=454214&action=review > LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-box.html:25 > + <div id="switchTransformBox" class="block"></div> The name seems not so good. I do not like the 'switch' and not only transform box is changed, right? Maybe something like transformBoxTarget? Committed r291105 (248267@trunk): <https://commits.webkit.org/248267@trunk> (In reply to Rob Buis from comment #6) > Comment on attachment 454214 [details] > Patch, v2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=454214&action=review > > > LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-box.html:25 > > + <div id="switchTransformBox" class="block"></div> > > The name seems not so good. I do not like the 'switch' and not only > transform box is changed, right? Maybe something like transformBoxTarget? Fixed before landing. I'll report about the WPT upstreaming status, haven't contributed so far, and need to tests this process once now :-) |