Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LBSE] Enable compositing support for SVG elements #2487

Conversation

nikolaszimmermann
Copy link
Contributor

@nikolaszimmermann nikolaszimmermann commented Jul 16, 2022

5c46074

[LBSE] Enable compositing support for SVG elements
https://bugs.webkit.org/show_bug.cgi?id=242833

Reviewed by Simon Fraser.

Enable compositing / 3D-transforms / z-index for SVG.

To make compositing useful in the context of SVG, a long standing bug needs to be fixed:
webkit.org/b/27684 ("Composited elements appear pixelated when scaled up using transform").
The idea is to include the maximum scale factor a CSS transform can induce into the "content
scale factor" belonging to the backing of the layer. Otherwise CSS transforms have no impact
on the internal GraphicsLayer size --> we'll render at scale=1 and upscale according to the
CSS transform. This results in blurry rendering. This has been attempted by quit a few people
over the years, however there were reservations:
- JS driven, e.g. rAF() animations, that modify scale e.g. from 1 -> 2 in 0.001 steps. Previously
  we never re-renderered, now we'd to it on every scale change. Needs some heuristics, a treshold,
  (|old-new|/old>x?) to enable this or stay with upscaling behavior.

- For WebAnimations/CSS animations/transitions we'd need to figure out the max scale for the whole
  animation timeline, so that we can figure it out once, and can avoid re-rendering during the animation.
  (This is not implemented in this patch yet, but my colleague Miguel has a patch for that...)

The question is: do we want a setting to toggle this at runtime? Or a CSS prop so that authors can
express their intent? (Please never render blurry, or I don't care, but want max. performance...).
SVG definately needs sharp graphics without upscaling artifacts: there are documents defined in a
10px x 10px space that are upscaling to window size (e.g. 800px / 600px) that would look completly
blurry with the current upscaling strategy.
Therefore I decided to introduce a setShouldUpdateRootRelativeScaleFactor() setter for GraphicsLayer,
that's only used by LBSE for now to activate the new strategy, so that I can move on with LBSE
upstreaming. However it would be the right time now to generalize this for all ports and settle
on a future-proof strategy how to deal with this issue. Web Authors already waited way too long
for a solution :-)

3D transformations work out of the box now for every SVG element, tested under various conditions
<svg> embedded in HTML, with CSS box model properties applied, such as border/padding/margin
with/without overflow clipping, with/without non-default transform-origin etc.

To make z-index work, we only have to switch to a new logic to respect z-index for non-positioned
elements (position: static, is the default for SVG elements) in StyleAdjuster, not more.

The bulk of the prepartions were already upstreamed, this smallish patch finally enables the
new fance SVG2+ features.

Add new reftests that cover SVG compositing with 2D/3D transforms (LayoutTests/svg/compositing)
and new z-index reftests (LayoutTests/svg/z-index). These news tests enable LBSE at runtime
such that these features are tested by default on macOS/iOS platforms on EWS. To switch on LBSE
at runtime these tests utilize following marker: <!-- webkit-test-runner [ LayerBasedSVGEngineEnabled=true ] -->

Covered by new and existing tests.

* LayoutTests/platform/glib/TestExpectations:
* LayoutTests/platform/mac-monterey-wk2-lbse-text/TestExpectations:
* LayoutTests/platform/win/TestExpectations:
* LayoutTests/svg/compositing/outermost-svg-directly-composited-group-child-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-group-child-overflow-hidden-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-group-child-overflow-hidden.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-group-child.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-shape-child-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-shape-child.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-transformed-group-child-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-transformed-group-child.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-overflow-visible-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-overflow-visible.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-padding-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-padding-margin-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-padding-margin.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-padding.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border.html: Added.
* LayoutTests/svg/compositing/svg-poster-circle-expected.html: Added.
* LayoutTests/svg/compositing/svg-poster-circle.html: Added.
* LayoutTests/svg/z-index/group-with-zindex-expected.svg: Added.
* LayoutTests/svg/z-index/group-with-zindex.svg: Added.
* LayoutTests/svg/z-index/shapes-with-zindex-expected.svg: Added.
* LayoutTests/svg/z-index/shapes-with-zindex.svg: Added.
* Source/WebCore/platform/graphics/GraphicsLayer.h:
(WebCore::GraphicsLayer::setShouldUpdateRootRelativeScaleFactor):
* Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers):
(WebCore::GraphicsLayerCA::updateRootRelativeScale):
* Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h:
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::createGraphicsLayer):
(WebCore::RenderLayerBacking::paintsContent const):
* Source/WebCore/style/StyleAdjuster.cpp:
(WebCore::Style::Adjuster::adjust const):
(WebCore::Style::Adjuster::adjustSVGElementStyle):

Canonical link: https://commits.webkit.org/253054@main

@nikolaszimmermann
Copy link
Contributor Author

This depends on PR #1623 and therefore includes it, to be able to test this patch with the CI.

@nikolaszimmermann nikolaszimmermann changed the title [LBSE] [LBSE] Enable compositing support for SVG elements Jul 16, 2022
@nikolaszimmermann nikolaszimmermann force-pushed the eng/LBSE-Enable-compositing-support-for-SVG-elements branch from 1f82998 to 747fef7 Compare July 16, 2022 16:19
@nikolaszimmermann nikolaszimmermann changed the title [LBSE] Enable compositing support for SVG elements [LBSE] Jul 16, 2022
@nikolaszimmermann nikolaszimmermann changed the title [LBSE] [LBSE] Enable compositing support for SVG elements Jul 16, 2022
@nikolaszimmermann
Copy link
Contributor Author

Please check it out, once PR #1623 is in, this is the only patch remaining to enable a 3D Poster Circle made with SVG in WebKit nightlies :-)

@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jul 16, 2022
@nikolaszimmermann nikolaszimmermann removed the merging-blocked Applied to prevent a change from being merged label Jul 20, 2022
@nikolaszimmermann nikolaszimmermann force-pushed the eng/LBSE-Enable-compositing-support-for-SVG-elements branch from 747fef7 to f4b76e0 Compare July 20, 2022 14:20
@nikolaszimmermann
Copy link
Contributor Author

Ready for review, now includes z-index tests as well. Finally! I'll post another issue on my WebKitIgalia repo soon that describes the overall LBSE status, besides the already existing upstreaming history.

Possibly the reftest pixel tolerances still need to be tuned, will check that once EWS is ready.

@nikolaszimmermann
Copy link
Contributor Author

@smfr - I'd like you to review this one, because of the GraphicsLayer/RenderLayerBacking changes. Would be great if you could have some time to check the code changes and new compositing / z-index reftests.

@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jul 20, 2022
@nikolaszimmermann nikolaszimmermann removed the merging-blocked Applied to prevent a change from being merged label Aug 1, 2022
@nikolaszimmermann nikolaszimmermann force-pushed the eng/LBSE-Enable-compositing-support-for-SVG-elements branch from f4b76e0 to da3a726 Compare August 1, 2022 08:06
@nikolaszimmermann
Copy link
Contributor Author

@smfr friendly ping :-)

@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Aug 1, 2022
@nikolaszimmermann
Copy link
Contributor Author

Need to skip svg/z-index tests (besides svg/compositing) for glib/win to fix EWS issues, will push a fix.

@nikolaszimmermann nikolaszimmermann removed the merging-blocked Applied to prevent a change from being merged label Aug 1, 2022
@nikolaszimmermann nikolaszimmermann force-pushed the eng/LBSE-Enable-compositing-support-for-SVG-elements branch from da3a726 to 398f404 Compare August 1, 2022 11:04
@nikolaszimmermann
Copy link
Contributor Author

Aw, also forgot to include the maxPixel diff in svg-poster-circle.html from 8916 -> 8904, should fix Mac WK1/WK2, too.

@nikolaszimmermann nikolaszimmermann force-pushed the eng/LBSE-Enable-compositing-support-for-SVG-elements branch from 398f404 to c00b37b Compare August 1, 2022 11:14
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Aug 1, 2022
Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h Outdated Show resolved Hide resolved
Source/WebCore/rendering/RenderLayerBacking.cpp Outdated Show resolved Hide resolved
@nikolaszimmermann
Copy link
Contributor Author

I'll have to update to fuzzy tag again for another test according to EWS. Will fix that soon.

@nikolaszimmermann nikolaszimmermann removed the merging-blocked Applied to prevent a change from being merged label Aug 1, 2022
@nikolaszimmermann nikolaszimmermann force-pushed the eng/LBSE-Enable-compositing-support-for-SVG-elements branch from c00b37b to 42c6445 Compare August 1, 2022 20:04
@nikolaszimmermann nikolaszimmermann added the merge-queue Applied to send a pull request to merge-queue label Aug 1, 2022
@webkit-early-warning-system webkit-early-warning-system added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Aug 1, 2022
@nikolaszimmermann nikolaszimmermann removed the merging-blocked Applied to prevent a change from being merged label Aug 2, 2022
@nikolaszimmermann nikolaszimmermann force-pushed the eng/LBSE-Enable-compositing-support-for-SVG-elements branch from 42c6445 to a0d5e42 Compare August 2, 2022 20:18
@nikolaszimmermann nikolaszimmermann added the merge-queue Applied to send a pull request to merge-queue label Aug 2, 2022
https://bugs.webkit.org/show_bug.cgi?id=242833

Reviewed by Simon Fraser.

Enable compositing / 3D-transforms / z-index for SVG.

To make compositing useful in the context of SVG, a long standing bug needs to be fixed:
webkit.org/b/27684 ("Composited elements appear pixelated when scaled up using transform").
The idea is to include the maximum scale factor a CSS transform can induce into the "content
scale factor" belonging to the backing of the layer. Otherwise CSS transforms have no impact
on the internal GraphicsLayer size --> we'll render at scale=1 and upscale according to the
CSS transform. This results in blurry rendering. This has been attempted by quit a few people
over the years, however there were reservations:
- JS driven, e.g. rAF() animations, that modify scale e.g. from 1 -> 2 in 0.001 steps. Previously
  we never re-renderered, now we'd to it on every scale change. Needs some heuristics, a treshold,
  (|old-new|/old>x?) to enable this or stay with upscaling behavior.

- For WebAnimations/CSS animations/transitions we'd need to figure out the max scale for the whole
  animation timeline, so that we can figure it out once, and can avoid re-rendering during the animation.
  (This is not implemented in this patch yet, but my colleague Miguel has a patch for that...)

The question is: do we want a setting to toggle this at runtime? Or a CSS prop so that authors can
express their intent? (Please never render blurry, or I don't care, but want max. performance...).
SVG definately needs sharp graphics without upscaling artifacts: there are documents defined in a
10px x 10px space that are upscaling to window size (e.g. 800px / 600px) that would look completly
blurry with the current upscaling strategy.
Therefore I decided to introduce a setShouldUpdateRootRelativeScaleFactor() setter for GraphicsLayer,
that's only used by LBSE for now to activate the new strategy, so that I can move on with LBSE
upstreaming. However it would be the right time now to generalize this for all ports and settle
on a future-proof strategy how to deal with this issue. Web Authors already waited way too long
for a solution :-)

3D transformations work out of the box now for every SVG element, tested under various conditions
<svg> embedded in HTML, with CSS box model properties applied, such as border/padding/margin
with/without overflow clipping, with/without non-default transform-origin etc.

To make z-index work, we only have to switch to a new logic to respect z-index for non-positioned
elements (position: static, is the default for SVG elements) in StyleAdjuster, not more.

The bulk of the prepartions were already upstreamed, this smallish patch finally enables the
new fance SVG2+ features.

Add new reftests that cover SVG compositing with 2D/3D transforms (LayoutTests/svg/compositing)
and new z-index reftests (LayoutTests/svg/z-index). These news tests enable LBSE at runtime
such that these features are tested by default on macOS/iOS platforms on EWS. To switch on LBSE
at runtime these tests utilize following marker: <!-- webkit-test-runner [ LayerBasedSVGEngineEnabled=true ] -->

Covered by new and existing tests.

* LayoutTests/platform/glib/TestExpectations:
* LayoutTests/platform/mac-monterey-wk2-lbse-text/TestExpectations:
* LayoutTests/platform/win/TestExpectations:
* LayoutTests/svg/compositing/outermost-svg-directly-composited-group-child-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-group-child-overflow-hidden-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-group-child-overflow-hidden.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-group-child.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-shape-child-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-shape-child.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-transformed-group-child-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-directly-composited-transformed-group-child.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-overflow-visible-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-overflow-visible.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-padding-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-padding-margin-expected.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-padding-margin.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border-padding.html: Added.
* LayoutTests/svg/compositing/outermost-svg-with-border.html: Added.
* LayoutTests/svg/compositing/svg-poster-circle-expected.html: Added.
* LayoutTests/svg/compositing/svg-poster-circle.html: Added.
* LayoutTests/svg/z-index/group-with-zindex-expected.svg: Added.
* LayoutTests/svg/z-index/group-with-zindex.svg: Added.
* LayoutTests/svg/z-index/shapes-with-zindex-expected.svg: Added.
* LayoutTests/svg/z-index/shapes-with-zindex.svg: Added.
* Source/WebCore/platform/graphics/GraphicsLayer.h:
(WebCore::GraphicsLayer::setShouldUpdateRootRelativeScaleFactor):
* Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers):
(WebCore::GraphicsLayerCA::updateRootRelativeScale):
* Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h:
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::createGraphicsLayer):
(WebCore::RenderLayerBacking::paintsContent const):
* Source/WebCore/style/StyleAdjuster.cpp:
(WebCore::Style::Adjuster::adjust const):
(WebCore::Style::Adjuster::adjustSVGElementStyle):

Canonical link: https://commits.webkit.org/253054@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/LBSE-Enable-compositing-support-for-SVG-elements branch from a0d5e42 to 5c46074 Compare August 2, 2022 21:38
@webkit-commit-queue
Copy link
Collaborator

Committed 253054@main (5c46074): https://commits.webkit.org/253054@main

Reviewed commits have been landed. Closing PR #2487 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 5c46074 into WebKit:main Aug 2, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 2, 2022
@nikolaszimmermann nikolaszimmermann deleted the eng/LBSE-Enable-compositing-support-for-SVG-elements branch August 2, 2022 22:47
@getflourish
Copy link

getflourish commented Aug 3, 2022

😍 Thank you so much @nikolaszimmermann.

How applicable is your approach to the more general issue that you’ve mentioned (“Composited elements appear pixelated when scaled up using transform”)? Will it eventually improve animation and scaling of text, too?

@nikolaszimmermann
Copy link
Contributor Author

heart_eyes Thank you so much @nikolaszimmermann.

How applicable is your approach to the more general issue that you’ve mentioned (“Composited elements appear pixelated when scaled up using transform”)? Will it eventually improve animation and scaling of text, too?

Thanks Florian for the warm words. The solution is generic, just not enabled at present out of the new SVG engine, due to the reservations I mentioned in the commit message, that e.g. @smfr raised in the past, which are very valid. WebKit doesn't ship performance regressions - so we at least need fine-grained heuristics when to enable the new behavior (IMHO "the only right one" -- however we have to live with what we shipped and can't ignore our legacy).

The main point is not to slow down all JS driven transform animations in the Web, by e.g. re-rendering each 'absolute scale change' of 0.0001. My colleague @magomez has a patch that tackles the CSS animations (+ CSS transitions / WebAnimations) pixelation issues by computing the max-scale factor upfront across the entire animation timeline. One needs to avoid to change the content scale factor during the animation, since you'd otherwise re-allocate backing memory (due to the graphic layer size changes), need to re-render etc. The patch is currently WPE specific (solved within Nicosia), and will need to be generalized on top of this work. This fixes half of the remaining problem.

The other half, JS driven animations, at least need some heuristics before it can enabled. IMHO the whole problem boils down to identify "significant" transform changes, that "deserve" a re-rendering.

Unfortunately I won't work on this, since I'm focusing on LBSE, but I can definitely help to review / push this forward -- do you have plans to work on this @magomez?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SVG For bugs in the SVG implementation.
Projects
None yet
5 participants