| Summary: | [LBSE] Fix origin of transformations in SVG | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nikolas Zimmermann <zimmermann> | ||||||||||||
| Component: | SVG | Assignee: | Nikolas Zimmermann <zimmermann> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | changseok, dino, esprehn+autocc, ews-watchlist, fmalita, fred.wang, glenn, gyuyoung.kim, kondapallykalyan, pdr, rbuis, sabouhallawa, schenney, sergio, simon.fraser, webkit-bug-importer, zimmermann | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Bug Depends on: | 237711 | ||||||||||||||
| Bug Blocks: | 90738, 239743, 239764 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Nikolas Zimmermann
2022-04-25 06:46:44 PDT
Created attachment 458267 [details]
Patch, v1
Comment on attachment 458267 [details] Patch, v1 View in context: https://bugs.webkit.org/attachment.cgi?id=458267&action=review > Source/WebCore/rendering/RenderLayer.cpp:3549 > +#endif I think a lambda is nicer here. > Source/WebCore/style/StyleAdjuster.cpp:374 > +#endif I think a lambda is nicer here. Comment on attachment 458267 [details] Patch, v1 View in context: https://bugs.webkit.org/attachment.cgi?id=458267&action=review > Source/WebCore/rendering/RenderElement.cpp:2437 > + if (isSVGLayerAwareRenderer() && !isSVGRoot() && document().settings().layerBasedSVGEngineEnabled()) It seems a bit more logical to do the isSVGRoot check last. Created attachment 458305 [details]
Patch, v2
(In reply to Rob Buis from comment #2) > > Source/WebCore/rendering/RenderLayer.cpp:3549 > > +#endif > > I think a lambda is nicer here. Good idea, fixed. > > Source/WebCore/style/StyleAdjuster.cpp:374 > > +#endif > > I think a lambda is nicer here. Fixed as well. (In reply to Rob Buis from comment #3) > > Source/WebCore/rendering/RenderElement.cpp:2437 > > + if (isSVGLayerAwareRenderer() && !isSVGRoot() && document().settings().layerBasedSVGEngineEnabled()) > > It seems a bit more logical to do the isSVGRoot check last. Hmm, I disagree. isSVGLayerAwareRenderer() contains an explicit '||.isSVGRoot()' condition, so it returns true for RenderSVGRoot. I really meant to check here: "all layer aware renderers, except RenderSVGRoot" (--> "isSVGLayerAwareRenderer() && !isSVGRoot()"). Note, that if 'isSVGRoot()' evaluates to true, that means LBSE must be enabled, otherwise we'd see a LegacyRenderSVGRoot, not a RenderSVGRoot. You could argue about testing: "if (document().settings().layerBasedSVGEngineEnabled() && isSVGLayerAwareRenderer() && !isSVGRoot())" though ;-) I left it as-is, if you're ok with it. Created attachment 458310 [details]
Patch, v3
Created attachment 458314 [details]
Patch, v4
Comment on attachment 458314 [details] Patch, v4 View in context: https://bugs.webkit.org/attachment.cgi?id=458314&action=review > Source/WebCore/rendering/RenderObject.h:377 > + // Their laid out in such a way that transformations do NOT affect layout, as in HTML/CSS world, but take affect during Their -> They are Created attachment 458434 [details]
Patch for landing
Committed r293504 (250035@main): <https://commits.webkit.org/250035@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 458434 [details]. |