Remove CSS3DTransformInteroperabilityEnabled preference #26017

annevk · 2024-03-17T08:49:41Z

`670a1d8`

Remove CSS3DTransformInteroperabilityEnabled preference
https://bugs.webkit.org/show_bug.cgi?id=271124

Reviewed by Matt Woodrow.

It's been stable for at least a year. The one unfortunate side effect
of this patch is that it more clearly highlights that some underlying
code continues to branch. Arguably those booleans should be turned into
clearly named enum classes.

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/dom/ViewTransition.cpp:
(WebCore::ViewTransition::copyElementBaseProperties):
* Source/WebCore/rendering/LayerOverlapMap.cpp:
(WebCore::LayerOverlapMap::LayerOverlapMap):
* Source/WebCore/rendering/RenderLayer.cpp:
(WebCore::RenderLayer::createLocalTransformState const):
(WebCore::RenderLayer::hitTestLayer):
(WebCore::RenderLayer::hitTestList):
(WebCore::isHitCandidateLegacy): Deleted.
* Source/WebCore/rendering/RenderLayer.h:
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::useCSS3DTransformInteroperability const): Deleted.
* Source/WebCore/rendering/RenderLayerBacking.h:
* Source/WebCore/rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::layerStyleChanged):
* Source/WebCore/rendering/RenderObject.cpp:
(WebCore::RenderObject::localToAbsolute const):
(WebCore::RenderObject::absoluteToLocal const):
(WebCore::RenderObject::absoluteToLocalQuad const):
(WebCore::RenderObject::shouldUseTransformFromContainer const):
(WebCore::RenderObject::getTransformFromContainer const):
(WebCore::RenderObject::pushOntoTransformState const):
(WebCore::RenderObject::pushOntoGeometryMap const):
(WebCore::RenderObject::localToContainerQuad const):
(WebCore::RenderObject::localToContainerPoint const):
(WebCore::RenderObject::participatesInPreserve3D const):
* Source/WebCore/rendering/RenderObject.h:
* Source/WebCore/rendering/RenderView.cpp:
(WebCore::RenderView::mapLocalToContainer const):
(WebCore::RenderView::pushMappingToContainer const):
(WebCore::RenderView::mapAbsoluteToLocalPoint const):
* Source/WebCore/rendering/svg/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::mapLocalToContainer const):
* Source/WebCore/rendering/svg/SVGLayerTransformComputation.h:
(WebCore::SVGLayerTransformComputation::computeAccumulatedTransform const):
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.h:
* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm:
(WebKit::RemoteLayerTreeHost::createLayer):
(WebKit::RemoteLayerTreeHost::css3DTransformInteroperabilityEnabled const): Deleted.

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

ff4f8a7

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
~~🛠 🧪 jsc~~	✅ 🧪 api-ios	~~🧪 mac-wk2~~	✅ 🛠 gtk
~~🛠 🧪 jsc-arm64~~	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

webkit-early-warning-system · 2024-03-17T08:49:47Z

EWS run on previous version of this PR (hash 54d9e26)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	❌ 🛠 ios	❌ 🛠 mac	❌ 🛠 wpe	❌ 🛠 wincairo
✅ 🧪 bindings	❌ 🛠 ios-sim	❌ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	❌ 🧪 api-wpe
	❌ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🛠 wpe-skia
✅ 🛠 🧪 jsc	❌ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	❌ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	❌ 🛠 tv-sim	❌ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	❌ 🛠 watch		✅ 🛠 jsc-armv7
	❌ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

nt1m · 2024-03-17T09:26:09Z

Is this ready to be enabled? I'm not sure every platform has this (although I think older platforms check at runtime? not using this pref?)

webkit-early-warning-system · 2024-03-17T10:52:16Z

EWS run on previous version of this PR (hash 562aaf6)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
❌ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

annevk · 2024-03-17T10:56:28Z

There's also ENABLE_3D_TRANSFORMS which I'm a bit unclear on whether all platforms enable it, but this preference isn't guarded by anything so seems reasonable to remove given how long it's been stable.

mattwoodrow

Thanks for doing this!

mattwoodrow · 2024-03-17T19:27:50Z

Source/WebCore/rendering/RenderLayerBacking.h

@@ -245,7 +245,8 @@ class RenderLayerBacking final : public GraphicsLayerClient {
    bool shouldAggressivelyRetainTiles(const GraphicsLayer*) const override;
    bool shouldTemporarilyRetainTileCohorts(const GraphicsLayer*) const override;
    bool useGiantTiles() const override;
-    bool useCSS3DTransformInteroperability() const override;
+    // FIXME: Should this have a different name if we need to preserve this difference indefinitely?
+    bool useCSS3DTransformInteroperability() const override { return true; }


I don't think the difference is intentional, and I believe all the callers of this function will happen on clients that are RenderLayerBacking.

I do see the value in separating the 'remove the preference' changes from anything that could be a functional difference though, but it'd be worth trying to remove this either in this patch or as a follow-up.

Filed https://bugs.webkit.org/show_bug.cgi?id=271141 and will do.

mattwoodrow · 2024-03-17T19:28:54Z

Source/WebCore/rendering/svg/RenderSVGRoot.cpp

@@ -549,7 +549,7 @@ void RenderSVGRoot::mapLocalToContainer(const RenderLayerModelObject* repaintCon
    // For getCTM/getScreenCTM computations the result must be independent of the page zoom factor.
    // To compute these matrices within a non-SVG context (e.g. SVG embedded in HTML -- inline SVG)
    // the scaling needs to be removed from the CSS transform state.
-    TransformState transformStateAboveSVGFragment(settings().css3DTransformInteroperabilityEnabled(), transformState.direction(), transformState.mappedPoint());
+    TransformState transformStateAboveSVGFragment(true, transformState.direction(), transformState.mappedPoint());


I think the only caller of the TransformState constructor that doesn't use the pref currently is just a mistake, so we could/should remove this param.

As with the above, doing so in a separate step is fine :)

Source/WebCore/rendering/RenderLayer.cpp

webkit-early-warning-system · 2024-03-18T07:38:30Z

EWS run on current version of this PR (hash ff4f8a7)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
~~🛠 🧪 jsc~~	✅ 🧪 api-ios	~~🧪 mac-wk2~~	✅ 🛠 gtk
~~🛠 🧪 jsc-arm64~~	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

https://bugs.webkit.org/show_bug.cgi?id=271124 Reviewed by Matt Woodrow. It's been stable for at least a year. The one unfortunate side effect of this patch is that it more clearly highlights that some underlying code continues to branch. Arguably those booleans should be turned into clearly named enum classes. * Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml: * Source/WebCore/dom/ViewTransition.cpp: (WebCore::ViewTransition::copyElementBaseProperties): * Source/WebCore/rendering/LayerOverlapMap.cpp: (WebCore::LayerOverlapMap::LayerOverlapMap): * Source/WebCore/rendering/RenderLayer.cpp: (WebCore::RenderLayer::createLocalTransformState const): (WebCore::RenderLayer::hitTestLayer): (WebCore::RenderLayer::hitTestList): (WebCore::isHitCandidateLegacy): Deleted. * Source/WebCore/rendering/RenderLayer.h: * Source/WebCore/rendering/RenderLayerBacking.cpp: (WebCore::RenderLayerBacking::useCSS3DTransformInteroperability const): Deleted. * Source/WebCore/rendering/RenderLayerBacking.h: * Source/WebCore/rendering/RenderLayerCompositor.cpp: (WebCore::RenderLayerCompositor::layerStyleChanged): * Source/WebCore/rendering/RenderObject.cpp: (WebCore::RenderObject::localToAbsolute const): (WebCore::RenderObject::absoluteToLocal const): (WebCore::RenderObject::absoluteToLocalQuad const): (WebCore::RenderObject::shouldUseTransformFromContainer const): (WebCore::RenderObject::getTransformFromContainer const): (WebCore::RenderObject::pushOntoTransformState const): (WebCore::RenderObject::pushOntoGeometryMap const): (WebCore::RenderObject::localToContainerQuad const): (WebCore::RenderObject::localToContainerPoint const): (WebCore::RenderObject::participatesInPreserve3D const): * Source/WebCore/rendering/RenderObject.h: * Source/WebCore/rendering/RenderView.cpp: (WebCore::RenderView::mapLocalToContainer const): (WebCore::RenderView::pushMappingToContainer const): (WebCore::RenderView::mapAbsoluteToLocalPoint const): * Source/WebCore/rendering/svg/RenderSVGRoot.cpp: (WebCore::RenderSVGRoot::mapLocalToContainer const): * Source/WebCore/rendering/svg/SVGLayerTransformComputation.h: (WebCore::SVGLayerTransformComputation::computeAccumulatedTransform const): * Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.h: * Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm: (WebKit::RemoteLayerTreeHost::createLayer): (WebKit::RemoteLayerTreeHost::css3DTransformInteroperabilityEnabled const): Deleted. Canonical link: https://commits.webkit.org/276257@main

webkit-commit-queue · 2024-03-18T09:00:29Z

Committed 276257@main (670a1d8): https://commits.webkit.org/276257@main

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

annevk requested review from cdumez and rniwa as code owners March 17, 2024 08:49

annevk self-assigned this Mar 17, 2024

annevk added the WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). label Mar 17, 2024

annevk requested a review from mattwoodrow March 17, 2024 08:52

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 17, 2024

annevk removed the merging-blocked Applied to prevent a change from being merged label Mar 17, 2024

annevk force-pushed the eng/Remove-CSS3DTransformInteroperabilityEnabled-preference branch from 54d9e26 to 562aaf6 Compare March 17, 2024 10:52

mattwoodrow approved these changes Mar 17, 2024

View reviewed changes

annevk force-pushed the eng/Remove-CSS3DTransformInteroperabilityEnabled-preference branch from 562aaf6 to ff4f8a7 Compare March 18, 2024 07:35

annevk added the merge-queue Applied to send a pull request to merge-queue label Mar 18, 2024

webkit-commit-queue force-pushed the eng/Remove-CSS3DTransformInteroperabilityEnabled-preference branch from ff4f8a7 to 670a1d8 Compare March 18, 2024 09:00

webkit-commit-queue merged commit 670a1d8 into WebKit:main Mar 18, 2024

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 18, 2024

annevk deleted the eng/Remove-CSS3DTransformInteroperabilityEnabled-preference branch March 18, 2024 09:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove CSS3DTransformInteroperabilityEnabled preference #26017

Remove CSS3DTransformInteroperabilityEnabled preference #26017

annevk commented Mar 17, 2024 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Mar 17, 2024 •

edited

Loading

nt1m commented Mar 17, 2024

webkit-early-warning-system commented Mar 17, 2024 •

edited

Loading

annevk commented Mar 17, 2024

mattwoodrow left a comment

mattwoodrow Mar 17, 2024

annevk Mar 18, 2024

mattwoodrow Mar 17, 2024

webkit-early-warning-system commented Mar 18, 2024 •

edited

Loading

webkit-commit-queue commented Mar 18, 2024

Remove CSS3DTransformInteroperabilityEnabled preference #26017

Remove CSS3DTransformInteroperabilityEnabled preference #26017

Conversation

annevk commented Mar 17, 2024 • edited by webkit-early-warning-system Loading

670a1d8

webkit-early-warning-system commented Mar 17, 2024 • edited Loading

nt1m commented Mar 17, 2024

webkit-early-warning-system commented Mar 17, 2024 • edited Loading

annevk commented Mar 17, 2024

mattwoodrow left a comment

Choose a reason for hiding this comment

mattwoodrow Mar 17, 2024

Choose a reason for hiding this comment

annevk Mar 18, 2024

Choose a reason for hiding this comment

mattwoodrow Mar 17, 2024

Choose a reason for hiding this comment

webkit-early-warning-system commented Mar 18, 2024 • edited Loading

webkit-commit-queue commented Mar 18, 2024

annevk commented Mar 17, 2024 •

edited by webkit-early-warning-system

Loading

`670a1d8`

webkit-early-warning-system commented Mar 17, 2024 •

edited

Loading

webkit-early-warning-system commented Mar 17, 2024 •

edited

Loading

webkit-early-warning-system commented Mar 18, 2024 •

edited

Loading