Summary: | Move updateLayers call to WebViewImpl::layout so that we do not trigger redundant draws | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | John Bates <jbates> | ||||||||||||
Component: | New Bugs | Assignee: | John Bates <jbates> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | fishd, jamesr, nduca, vangelis, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
John Bates
2011-07-08 18:42:21 PDT
Created attachment 100198 [details]
Patch
Associated with chromium patch: http://codereview.chromium.org/7327030/ Fixed bug that caused canvas updates to trigger two compositor paints. LayerRendererChromium::updateLayers was triggering a scheduleComposite because the paint aggregator in RenderWidget was emptied beforehand. This change (in combination with the chromium-side patch) delays the paint aggregator emptying until after updateLayers and before paint. This patch must land before the chromium patch above. popPendingUpdate() is not a very enlightening API name. I've looked at the chromium side and I'm still not completely sure what it is intended to mean. Created attachment 100401 [details]
Patch
Comment on attachment 100401 [details]
Patch
Wait, it's not valid to call LayerRendererChromium::updateLayers() at this point. You want to just call WebViewImpl's updateLayers(), which does a different thing, and remove that call from LayerRendererChromium. Leave doComposite() alone.
Also, this bug needs to be retitled.
Created attachment 100536 [details]
Patch
Comment on attachment 100536 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100536&action=review Closer! Please also rename this bug, preferably to describe the problem this patch fixes (double frames on canvas). > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp:97 > -void CCLayerTreeHost::updateLayers() > +void CCLayerTreeHost::syncCompositingLayers() > { > - m_client->updateLayers(); > + m_client->syncCompositingLayers(); > } delete > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.h:44 > + virtual void syncCompositingLayers() = 0; Just delete this (and the corresponding entry in CCLayerTreeHost), there aren't any callers of this any more. > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.h:65 > + void syncCompositingLayers(); delete > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImplProxy.cpp:153 > m_commitPending = false; > { > - TRACE_EVENT("CCLayerTreeHost::updateLayers", this, 0); > - m_layerTreeHost->updateLayers(); > + TRACE_EVENT("CCLayerTreeHost::syncCompositingLayers", this, 0); > + m_layerTreeHost->syncCompositingLayers(); > } You can delete this section completely, animateAndLayout() takes care of this now (since it calls WebViewImpl::layout()) Created attachment 100547 [details]
Patch
Comment on attachment 100547 [details]
Patch
Cool. One more nit to consider before landing: Could you add a TRACE_EVENT() around syncCompositingLayers()?
Created attachment 100549 [details]
Patch
(In reply to comment #9) > (From update of attachment 100547 [details]) > Cool. One more nit to consider before landing: Could you add a TRACE_EVENT() around syncCompositingLayers()? Done. Comment on attachment 100549 [details]
Patch
Flags away!
Comment on attachment 100549 [details] Patch Clearing flags on attachment: 100549 Committed r90850: <http://trac.webkit.org/changeset/90850> All reviewed patches have been landed. Closing bug. |