| Summary: | [chromium] Defer makeContextCurrent in compositor until first frame | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | James Robinson <jamesr> | ||||||||
| Component: | New Bugs | Assignee: | James Robinson <jamesr> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | cc-bugs, dglazkov, enne, kbr, nduca, piman, vangelis, webkit.review.bot | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
James Robinson
2012-01-27 19:13:39 PST
Created attachment 124424 [details]
Patch
Created attachment 124477 [details]
Patch
This is half of the fix for http://code.google.com/p/chromium/issues/detail?id=106815, which requires that we be able to initialize the compositor before we're ready to call makeContextCurrent on the compositor context. This defers LayerRendererChromium::initialize() until we're ready to start making the first frame. This depends on the scheduler not attempting to produce frames until the surface is ready, so it only fixes things for the single threaded scheduler. The threaded scheduler needs a way to be notified of when it's safe to call makeContextCurrent(). Without that fix, it's possible (but I think very unlikely in practice) that window.open()'d windows with --force-compositing-mode and the threaded compositor may fail to enter compositing mode correctly. The single threaded scheduler is patched here: https://chromiumcodereview.appspot.com/9225050/ to avoid that issue. Comment on attachment 124477 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124477&action=review Neat stuff! Unofficial LGTM. > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp:311 > + if (!m_layerRendererInitialized) /me wonders how m_layerRendererInitialized and m_contentsTextureManager behave on context lost? Is m_layerRendererInitialized true and m_contentsTextureManager non-null? Wearingmy overddesign hat for a moment, it might be nice to put these inside some inner struct that is m_layerRendererState that was obviously null when layer renderer was lost. But anyway, just thinking out loud. > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.h:157 > Can we make layerRendererCapabilities() pop if !m_layerRendererInitialized? Comment on attachment 124477 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124477&action=review > Source/WebCore/ChangeLog:78 > +2012-01-27 James Robinson <jamesr@chromium.org> Nit: this entry is left over. Possible you've got two bugs' patches in this one? Ah yeah, I had http://trac.webkit.org/changeset/106306 merged into this branch. I can take care of cleaning ChangeLogs up when landing. Comment on attachment 124477 [details]
Patch
LGTM otherwise based on Nat's review.
Created attachment 124803 [details]
Patch for landing
Comment on attachment 124803 [details] Patch for landing Rejecting attachment 124803 [details] from commit-queue. New failing tests: compositing/scroll-painted-composited-content.html platform/chromium/compositing/layout-width-change.html compositing/geometry/fixed-in-composited.html compositing/masks/multiple-masks.html compositing/iframes/overlapped-nested-iframes.html compositing/geometry/vertical-scroll-composited.html compositing/masks/masked-ancestor.html compositing/geometry/ancestor-overflow-change.html compositing/plugins/invalidate_rect.html compositing/overflow/fixed-position-ancestor-clip.html platform/chromium/compositing/img-layer-grow.html compositing/direct-image-compositing.html transforms/3d/point-mapping/3d-point-mapping-deep.html compositing/geometry/tall-page-composited.html compositing/geometry/horizontal-scroll-composited.html compositing/masks/simple-composited-mask.html compositing/geometry/fixed-position.html compositing/iframes/become-composited-nested-iframes.html transforms/3d/point-mapping/3d-point-mapping-origins.html http/tests/inspector/inspect-element.html compositing/scaling/tiled-layer-recursion.html Full output: http://queues.webkit.org/results/11387219 Comment on attachment 124803 [details] Patch for landing Attachment 124803 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11387348 New failing tests: compositing/scroll-painted-composited-content.html platform/chromium/compositing/layout-width-change.html compositing/geometry/fixed-in-composited.html compositing/masks/multiple-masks.html compositing/iframes/overlapped-nested-iframes.html compositing/geometry/vertical-scroll-composited.html compositing/masks/masked-ancestor.html compositing/geometry/ancestor-overflow-change.html compositing/plugins/invalidate_rect.html compositing/overflow/fixed-position-ancestor-clip.html platform/chromium/compositing/img-layer-grow.html compositing/direct-image-compositing.html transforms/3d/point-mapping/3d-point-mapping-deep.html compositing/geometry/tall-page-composited.html compositing/geometry/horizontal-scroll-composited.html compositing/masks/simple-composited-mask.html compositing/geometry/fixed-position.html compositing/iframes/become-composited-nested-iframes.html transforms/3d/point-mapping/3d-point-mapping-origins.html http/tests/inspector/inspect-element.html compositing/scaling/tiled-layer-recursion.html Committed r106700: <http://trac.webkit.org/changeset/106700> |