Bug 84393 - Avoid calling updateCompositingLayers() more than once before each render
Summary: Avoid calling updateCompositingLayers() more than once before each render
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Compositing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Matt Woodrow
URL:
Keywords: InRadar
: 118555 (view as bug list)
Depends on: 201330
Blocks: 84410 98800
  Show dependency treegraph
 
Reported: 2012-04-19 15:36 PDT by Simon Fraser (smfr)
Modified: 2024-04-22 11:26 PDT (History)
15 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 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.
Comment 1 Simon Fraser (smfr) 2012-04-19 17:22:00 PDT
Maybe we could use the rAF mechanism here too
Comment 2 James Robinson 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.
Comment 3 Simon Fraser (smfr) 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.
Comment 4 Simon Fraser (smfr) 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().
Comment 5 Radar WebKit Bug Importer 2012-05-01 16:45:59 PDT
<rdar://problem/11359525>
Comment 6 Simon Fraser (smfr) 2012-11-29 16:43:21 PST
Site that would benefit from this: http://www.milwaukeepolicenews.com/
Comment 7 James Robinson 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.
Comment 8 Simon Fraser (smfr) 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).
Comment 9 James Robinson 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.
Comment 10 Simon Fraser (smfr) 2012-11-29 17:55:05 PST
Maybe we should leverage requestAnimationFrame underpinnings then. I already do that for TiledLayer flushing.
Comment 11 Simon Fraser (smfr) 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.
Comment 12 Simon Fraser (smfr) 2013-07-11 11:12:56 PDT
*** Bug 118555 has been marked as a duplicate of this bug. ***
Comment 13 Darin Adler 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?
Comment 14 Simon Fraser (smfr) 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.
Comment 15 Darin Adler 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.
Comment 16 Simon Fraser (smfr) 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.
Comment 17 Simon Fraser (smfr) 2019-08-29 21:32:33 PDT
I hacked this up and it mostly worked, but some tests fail because of tree statefulness.
Comment 18 Simon Fraser (smfr) 2019-08-29 21:42:15 PDT
Created attachment 377678 [details]
Patch
Comment 19 Matt Woodrow 2023-11-13 17:37:42 PST
Created attachment 468589 [details]
Updated WIP patch, for EWS testing
Comment 20 Matt Woodrow 2023-11-14 14:50:09 PST
Created attachment 468595 [details]
Updated WIP patch, for EWS testing
Comment 21 Matt Woodrow 2023-11-15 13:16:05 PST
Pull request: https://github.com/WebKit/WebKit/pull/20556
Comment 22 EWS 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.