Summary: | [Chromium] Purge Skia's GPU texture cache using GL_CHROMIUM_gpu_memory_manager | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jeff Timanus <twiz> | ||||||
Component: | Canvas | Assignee: | Jeff Timanus <twiz> | ||||||
Status: | RESOLVED DUPLICATE | ||||||||
Severity: | Normal | CC: | bsalomon, jamesr, mdelaney7, mmocny, nduca, vangelis | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 77155 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Jeff Timanus
2012-02-01 13:05:28 PST
Created attachment 125141 [details]
Patch
(In reply to comment #1) > Created an attachment (id=125141) [details] > Patch This patch implements a prototype of the integration required plumb the Ganesh context through to the GL_CHROMIUM_gpu_memory_manager extension. The implementation is not quite complete, as I'm waiting for the extension in issue 77155 to land. At this point, I'd appreciate some feedback on the approach taken to connect the WebView to the GrContext used by Ganesh. During WebViewImpl::composite, I unregister all of the affecting contexts, and then during Canvas2DLayerChromium::pushPropertiesTo, the GraphicsContext3D is registered with the WebViewImpl via the CCLayerTreeHostClient. All of the collected contexts are then to be registered with the extension. Comment on attachment 125141 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125141&action=review > Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:85 > +namespace { I assume this is going to be part of Michal's patch, not yours? > Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:158 > + if (extensions3d->supports("gpu_memory_management_CHROMIUM")) { I dont think this is right... the person owning the context should be installing the extension. I think Michal has a patch that makes Extensions3DChromium support an allocation callback > Source/WebKit/chromium/src/WebLayerTreeViewImpl.cpp:116 > +void WebLayerTreeViewImpl::registerAffectingContext(WebCore::GraphicsContext3D* context) Lets use the same words we're using on the extension. addAffectedContext/etc... > Source/WebKit/chromium/src/WebLayerTreeViewImpl.cpp:118 > +} i assume this is going to make the same call up to m_client (which is WebViewImpl?) > Source/WebKit/chromium/src/WebViewImpl.cpp:1267 > +#if USE(SKIA) Lets make this unconditional on skia. Its a good concept to have anyways. > Source/WebKit/chromium/src/WebViewImpl.cpp:1289 > + So this composite function isn't called in threaded mode. Lets add two methods in CCLayerTreeHostClient called willCommit didCommit CCSingleThreadProxy should call that in its commitIfneeded code CCThreadProxy should too Comment on attachment 125141 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125141&action=review After offline discussion with mmocny@ and nduca@, some changes are required. See inline comments. > Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:160 > + } Constructing a callback for every context is unnecessary. A callback is only needed for contexts with a GrContext. This code path should be moved to GraphicsContext3DPrivate::GrContext(). > Source/WebKit/chromium/src/WebViewImpl.cpp:1276 > + Resetting all of the affected widgets during every composite is unnecessarily expensive. Instead, the set deltas between successive composite operations need to be processed. This will minimize the number of calls into the extension. Created attachment 125396 [details]
Patch
Comment on attachment 125141 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125141&action=review >> Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:85 >> +namespace { > > I assume this is going to be part of Michal's patch, not yours? Michal's patch introduces the interface, but my patch will still need the implementation that purges Ganesh's texture cache. >> Source/WebKit/chromium/src/GraphicsContext3DChromium.cpp:158 >> + if (extensions3d->supports("gpu_memory_management_CHROMIUM")) { > > I dont think this is right... the person owning the context should be installing the extension. I think Michal has a patch that makes Extensions3DChromium support an allocation callback I moved the installation of the extension to the shared graphics context. It's safe to assume that a shared graphics context will be associated with a GrContext. >> Source/WebKit/chromium/src/WebLayerTreeViewImpl.cpp:116 >> +void WebLayerTreeViewImpl::registerAffectingContext(WebCore::GraphicsContext3D* context) > > Lets use the same words we're using on the extension. addAffectedContext/etc... Changed to addAffectingContext. The extension is for adding affected web-views. >> Source/WebKit/chromium/src/WebLayerTreeViewImpl.cpp:118 >> +} > > i assume this is going to make the same call up to m_client (which is WebViewImpl?) I'm uncertain how this class should interact with the extension. m_client is not a WebViewImpl. It is a WebLayerTreeViewClient, which is implemented by the Compositor class (ui/gfx/compositor/compositor.h) >> Source/WebKit/chromium/src/WebViewImpl.cpp:1289 >> + > > So this composite function isn't called in threaded mode. > > Lets add two methods in CCLayerTreeHostClient called > willCommit > didCommit > > CCSingleThreadProxy should call that in its commitIfneeded code > > CCThreadProxy should too Done. CCThreadProxy does not have a commitIfNeeded routine. Instead I added it to the compositeAndReadback(...) routine. Is this safe? From an offline discussion with bsalomon, we decided that it would be beneficial if this patch set the texture cache size to 0 upon background notification. If the cache is only purged to 0 when backgrounded, then the GPU memory footprint could still increase as non-raf rendering events are fired. This change will require the GL_CHROMIUM_gpu_memory_manager to provide foregrounded notifications as well as backgrounded. |