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.

Sign up for GitHub

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

Jump to bottom

RenderLayerBacking::updateGeometry() should update its GraphicsLayer's drawsContent flag #20822

Closed
wants to merge 1 commit into from
Closed

RenderLayerBacking::updateGeometry() should update its GraphicsLayer's drawsContent flag #20822

wants to merge 1 commit into from

Conversation

obyknovenius
Copy link
Contributor

@obyknovenius obyknovenius commented Nov 22, 2023 •
edited by webkit-early-warning-system

6bc9529

`RenderLayerBacking::updateGeometry()` should update its `GraphicsLayer`'s `drawsContent` flag
https://bugs.webkit.org/show_bug.cgi?id=265251

Reviewed by NOBODY (OOPS!).

`RenderLayerBacking::updateGeometry()` may change
`m_requiresOwnBackingStore`, which can affect the result of
`containsPaintedContent()`.

We should call `updateDrawsContent()` so that `GraphicsLayer` reflects
this change.

* LayoutTests/platform/gtk/TestExpectations:
* LayoutTests/platform/gtk/compositing/visibility/layer-visible-content-expected.txt: Removed.
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateGeometry):

6bc9529

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 ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@obyknovenius obyknovenius self-assigned this Nov 22, 2023
@obyknovenius obyknovenius added the Compositing Bugs related to layerization for transforms, video, scrolling etc label Nov 22, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 22, 2023 •
edited

EWS run on previous version of this PR (hash e3b301d)

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 ❌ 🛠 gtk
❌ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
❌ 🛠 tv ❌ 🧪 mac-AS-debug-wk2 ❌ 🧪 api-gtk
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress
❌ 🛠 watch
❌ 🛠 watch-sim

@obyknovenius obyknovenius force-pushed the update-draws-content-during-update-geometry branch from e3b301d to 6c9e939 Compare November 22, 2023 16:06
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 22, 2023 •
edited

EWS run on previous version of this PR (hash 6c9e939)

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 ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 22, 2023
@obyknovenius obyknovenius removed the merging-blocked Applied to prevent a change from being merged label Nov 23, 2023
@obyknovenius obyknovenius force-pushed the update-draws-content-during-update-geometry branch from 6c9e939 to 2d65a6c Compare November 23, 2023 08:57
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 23, 2023 •
edited

EWS run on previous version of this PR (hash 2d65a6c)

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 ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 23, 2023
@obyknovenius
RenderLayerBacking::updateGeometry() should update its `GraphicsLay…
6bc9529 
…er`'s `drawsContent` flag

https://bugs.webkit.org/show_bug.cgi?id=265251

Reviewed by NOBODY (OOPS!).

`RenderLayerBacking::updateGeometry()` may change
`m_requiresOwnBackingStore`, which can affect the result of
`containsPaintedContent()`.

We should call `updateDrawsContent()` so that `GraphicsLayer` reflects
this change.

* LayoutTests/platform/gtk/TestExpectations:
* LayoutTests/platform/gtk/compositing/visibility/layer-visible-content-expected.txt: Removed.
* Source/WebCore/rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateGeometry):
@obyknovenius obyknovenius removed the merging-blocked Applied to prevent a change from being merged label Nov 24, 2023
@obyknovenius obyknovenius force-pushed the update-draws-content-during-update-geometry branch from 2d65a6c to 6bc9529 Compare November 24, 2023 11:00
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 24, 2023 •
edited

EWS run on current version of this PR (hash 6bc9529)

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 ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

smfr
smfr reviewed Nov 24, 2023
View reviewed changes
Source/WebCore/rendering/RenderLayerBacking.cpp
@@ -1594,6 +1594,7 @@ void RenderLayerBacking::updateGeometry(const RenderLayer* compositedAncestor)
LayoutRect ancestorCompositedBounds = compositedAncestor ? compositedAncestor->backing()->compositedBounds() : LayoutRect();
setRequiresOwnBackingStore(compositor().requiresOwnBackingStore(m_owningLayer, compositedAncestor,
LayoutRect(toLayoutPoint(compositedBoundsOffset.fromParentGraphicsLayer()), compositedBounds().size()), ancestorCompositedBounds));
updateDrawsContent();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is called from updateAfterDescendants() which is called on every RenderLayerBacking from RenderLayerCompositor::updateBackingAndHierarchy() so why does calling it here change behavior?

I'm also curious about why this change is needed on GTK but not macOS/iOS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GTK is the only port that doesn't use force compositing mode by default.
When RenderLayerCompositor::updateCompositingLayers() is called the first time with force compositing enabled, we create the root render layer. It will have no children yet. But its drawsContent flag will be set to true by the end of the method.
But with force compositing disabled, we create the root layer (with drawsContent set to false by default) when we already have some children layers. The first child will then decide that it needs its own backing store. And only after that, the root layer will set drawsContent to true. But it may be too late.
I hope I explained it well enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another argument for this change is that the returning value of RenderLayerBacking::containsPaintedContent(), which is used by RenderLayerBacking::updateDrawsContent(), depends on its m_requiresOwnBackingStore value. So, whenever we change it, we should call updateDrawsContent()

@obyknovenius
Copy link
Contributor Author

Closing in favor of #20298.

@obyknovenius obyknovenius deleted the update-draws-content-during-update-geometry branch February 23, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smfr smfr Awaiting requested review from smfr

Assignees

@obyknovenius obyknovenius

Labels
Compositing Bugs related to layerization for transforms, video, scrolling etc
Projects
None yet
Milestone
No milestone
4 participants
@obyknovenius @webkit-early-warning-system @smfr @webkit-ews-buildbot