WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
84393
Avoid calling updateCompositingLayers() more than once before each render
https://bugs.webkit.org/show_bug.cgi?id=84393
Summary
Avoid calling updateCompositingLayers() more than once before each render
Simon Fraser (smfr)
Reported
2012-04-19 15:36:12 PDT
Pages that repeatedly call offsetWidth then change style will force a lot of unnecessary compositing updates. We should avoid them, and figure out a way to only update compositing layers before painting.
Attachments
Patch
(9.87 KB, patch)
2019-08-29 21:42 PDT
,
Simon Fraser (smfr)
no flags
Details
Formatted Diff
Diff
Updated WIP patch, for EWS testing
(4.42 KB, patch)
2023-11-13 17:37 PST
,
Matt Woodrow
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Updated WIP patch, for EWS testing
(15.33 KB, patch)
2023-11-14 14:50 PST
,
Matt Woodrow
mattwoodrow
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1
2012-04-19 17:22:00 PDT
Maybe we could use the rAF mechanism here too
James Robinson
Comment 2
2012-04-19 17:30:56 PDT
One difficulty design-wise is that currently WebCore doesn't generally know if it's doing layout because the host wants to make a frame or if it's doing layout for some other reason (like JS wanted it to or the host wants it to do layout to query some values). If we knew that in FrameView::layout then we could only update the compositing tree when it was a layout-because-it's-make-a-frame-time. We could use the rAF mechanism since it's (hopefully) tied in to the host's "I want to make a frame now" scheduling mechanism.
Simon Fraser (smfr)
Comment 3
2012-04-19 17:34:04 PDT
I don't think knowing the "reason for layout" in FrameView::layout is enough; you need to know if this is the last layout you'll get before painting happens. We also have the "sync compositing layer" stuff that is based on a run loop observer but also fires before painting.
Simon Fraser (smfr)
Comment 4
2012-04-19 18:11:36 PDT
Just saw this when scrolling macnn.com, via JS calling pageYOffset. Lots of time in absBounds = layer->renderer()->localToAbsoluteQuad(FloatRect(layer->localBoundingBox())).enclosingBoundingBox(); under RenderLayerCompositor::computeCompositingRequirements().
Radar WebKit Bug Importer
Comment 5
2012-05-01 16:45:59 PDT
<
rdar://problem/11359525
>
Simon Fraser (smfr)
Comment 6
2012-11-29 16:43:21 PST
Site that would benefit from this:
http://www.milwaukeepolicenews.com/
James Robinson
Comment 7
2012-11-29 17:04:22 PST
(In reply to
comment #6
)
> Site that would benefit from this:
http://www.milwaukeepolicenews.com/
Can we add a platform hook that the embedder has to call when they wanna make a frame? I imagine it'd tie to the run loop observer for you, and I've have to hook it in somewhere for chromium (WebKit::WebViewImpl::layout() would be a good approximation). I don't know where to put it for other ports.
Simon Fraser (smfr)
Comment 8
2012-11-29 17:45:53 PST
I think we should enhance RunLoop for this kind of thing. We would just need a callback for things that want to run near the end of the runloop (after timers, before layer committing).
James Robinson
Comment 9
2012-11-29 17:48:40 PST
The RunLoop can spin many many times in between frames (hopefully each operation isn't blocking for 16ms), so I think just tracking that by itself will still produce a lot of unnecessary uCL() calls.
Simon Fraser (smfr)
Comment 10
2012-11-29 17:55:05 PST
Maybe we should leverage requestAnimationFrame underpinnings then. I already do that for TiledLayer flushing.
Simon Fraser (smfr)
Comment 11
2012-11-29 21:28:46 PST
We can't just not update compositing layers, because otherwise GraphicsLayers will be left pointing at RenderLayerBackings which might have gone away.
Simon Fraser (smfr)
Comment 12
2013-07-11 11:12:56 PDT
***
Bug 118555
has been marked as a duplicate of this bug. ***
Darin Adler
Comment 13
2015-04-15 09:21:02 PDT
Would it hurt compositing too much to make updating the compositing layers a completely separate pass from layout?
Simon Fraser (smfr)
Comment 14
2015-04-15 09:31:50 PDT
(In reply to
comment #13
)
> Would it hurt compositing too much to make updating the compositing layers a > completely separate pass from layout?
No, that would be ideal. The problem is that we can't easily disassociate compositing layer updates from layout without losing repaints. If we stored repaints differently this would be easier to do.
Darin Adler
Comment 15
2015-04-15 09:44:18 PDT
So we need an in-WebKit data structure for tracking repaints and then we’d communicate any repainting to the OS only after updating compositing layers.
Simon Fraser (smfr)
Comment 16
2015-04-15 09:50:30 PDT
(In reply to
comment #15
)
> So we need an in-WebKit data structure for tracking repaints
We have this already (we accumulate dirty rects in GraphicsLayers). The problems are: 1. we dirty during layout, and dirtying assumes that you can find the relevant repaintContainer (which is a compositing layer) and compute a dirty rect relative to it. 2. RenderLayers cache repaint rects, which will also go stale if their repaint container changes because of compositing updates. So we need a data structure that hangs off the render tree that tracks repaints, and can track them sensibly across multiple layouts, in a compositing-independent way.
Simon Fraser (smfr)
Comment 17
2019-08-29 21:32:33 PDT
I hacked this up and it mostly worked, but some tests fail because of tree statefulness.
Simon Fraser (smfr)
Comment 18
2019-08-29 21:42:15 PDT
Created
attachment 377678
[details]
Patch
Matt Woodrow
Comment 19
2023-11-13 17:37:42 PST
Created
attachment 468589
[details]
Updated WIP patch, for EWS testing
Matt Woodrow
Comment 20
2023-11-14 14:50:09 PST
Created
attachment 468595
[details]
Updated WIP patch, for EWS testing
Matt Woodrow
Comment 21
2023-11-15 13:16:05 PST
Pull request:
https://github.com/WebKit/WebKit/pull/20556
EWS
Comment 22
2023-11-28 20:36:52 PST
Committed
271261@main
(b6b74556b8e6): <
https://commits.webkit.org/271261@main
> Reviewed commits have been landed. Closing PR #20556 and removing active labels.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug