| Summary: | [chromium] Evict textures through the texture manager instead of the resource provider | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Christopher Cameron <ccameron> | ||||||
| Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cc-bugs, danakj, enne, epenner, jamesr, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Christopher Cameron
2012-09-11 20:10:38 PDT
Created attachment 163507 [details]
Patch
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 on attachment 163507 [details] Patch 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. sould->should > 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 Created attachment 163661 [details]
Patch
Comment on attachment 163661 [details]
Patch
Thanks! Updated with the changes you suggested.
Comment on attachment 163661 [details] Patch Clearing flags on attachment: 163661 Committed r128344: <http://trac.webkit.org/changeset/128344> All reviewed patches have been landed. Closing bug. |