Bug 96463 - [chromium] Evict textures through the texture manager instead of the resource provider
Summary: [chromium] Evict textures through the texture manager instead of the resource...
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
: 95057 (view as bug list)
Depends on:
Reported: 2012-09-11 20:10 PDT by Christopher Cameron
Modified: 2012-09-14 15:24 PDT (History)
6 users (show)

See Also:

Patch (34.01 KB, patch)
2012-09-11 20:22 PDT, Christopher Cameron
no flags Details | Formatted Diff | Diff
Patch (34.54 KB, patch)
2012-09-12 11:24 PDT, Christopher Cameron
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Cameron 2012-09-11 20:10:38 PDT
[chromium] Remove last use of CCPrioritizedTextureManager::m_backings array on the main thread
Comment 1 Christopher Cameron 2012-09-11 20:22:47 PDT
Created attachment 163507 [details]
Comment 2 Christopher Cameron 2012-09-11 20:27:22 PDT
This is the second-to-last slice of bug 95057.  The one remaining slice is partial clears of the texture upload queue (which is big enough self-contained enough to warrant a separate change).

Synopsis from ChangeLog:

When deleting contents textures' resources on the impl thread, do the deletion through the CCPrioritizedTextureManager instead of the CCResourceProvider.

This requires traversing the backings list on the impl thread while the main thread is running, so remove the one remaining traversal of  backings list by the main thread. This traversal happens when unlinking textures that were evicted by the impl thread, so explicitly send the list of evicted backings from the impl thread to the main thread.

Unify all resource deletion paths in the CCPrioritizedTextureManager. Always perform the sequence of eviction (deleting the GL resource) and then destruction of evicted backings (deleting the objects).  Also, use the same function (evictBackingsToReduceMemory) to reduce memory consumption both during commit and when done by the impl thread in response to a request by the GPU memory manager.

Note that destroying only some of the resources at a time during texture eviction (as opposed all resources) is still not supported because the texture upload queues cannot be only-partially invalidated yet. Updated tests to take this behavior into account.
Comment 3 James Robinson 2012-09-12 01:52:07 PDT
Comment on attachment 163507 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=163507&action=review

R=me, just some really minor things

> Source/WebCore/platform/graphics/chromium/cc/CCPrioritizedTextureManager.cpp:56
> +    // Each remaining backing is a leaked opengl texture. There sould be none.


> Source/WebCore/platform/graphics/chromium/cc/CCPrioritizedTextureManager.cpp:277
> +    evictBackingsToReduceMemory(0, false, resourceProvider);

i think these callsites would all be more readable if you used a 2-state enum for the second parameter instead of a bool. since the callsites are just literals, i have to go look for the definition of evictBackingsToReduceMemory to understand what "false" means
Comment 4 Christopher Cameron 2012-09-12 11:24:03 PDT
Created attachment 163661 [details]
Comment 5 Christopher Cameron 2012-09-12 11:24:47 PDT
Comment on attachment 163661 [details]

Thanks!  Updated with the changes you suggested.
Comment 6 WebKit Review Bot 2012-09-12 11:49:02 PDT
Comment on attachment 163661 [details]

Clearing flags on attachment: 163661

Committed r128344: <http://trac.webkit.org/changeset/128344>
Comment 7 WebKit Review Bot 2012-09-12 11:49:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Christopher Cameron 2012-09-14 15:24:09 PDT
*** Bug 95057 has been marked as a duplicate of this bug. ***