[Webkit-unassigned] [Bug 96114] [chromium] Make prioritized texture manager not touch backings array on the main thread

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=96114





--- Comment #19 from Christopher Cameron <ccameron at chromium.org>  2012-09-10 13:14:04 PST ---
(From update of attachment 163177)
View in context: https://bugs.webkit.org/attachment.cgi?id=163177&action=review

Thanks!  Patch update incoming.

So I have a preference the non-lazy scheme, since it makes somewhat clear when the backings' priorities will be updated in the commit flow, and it was also easier to unit-test (I could do the explicit prioritize and then test the output).  If you feel strongly about going back to the fully-lazy scheme, I can change it back.

All of that said, I'm somewhat curious why we create-then-delete like 12 textures when loading a simple page (maybe if we make that behavior not the norm we can get rid of the aggressive intra-frame recycling).

>> Source/WebCore/platform/graphics/chromium/cc/CCPrioritizedTextureManager.cpp:225
>> +    ASSERT(CCProxy::isImplThread() && CCProxy::isMainThreadBlocked());
> 
> Are the backings possibly not fully sorted when this is called? Could this miss available backings if they were high priority but were since returned?
> 
> In this sense I preferred the old lazy scheme which always detected and kept the backings sorted, or in alternate just insuring sortBackings is called everywhere it needs to be called so this would not be needed.

The backings are guaranteed to be sorted at all times, however, their sorting may be with respect to the last snapshot, but it may be that the main thread has made some changes to the priorities that have not yet been committed.

In acquireBackingTextureIfNeeded, we call this twice -- if the first call doesn't get us a backing and the main thread has since returned some backings, we re-pull the priorities from the textures (to avoid that regression).

>> Source/WebCore/platform/graphics/chromium/cc/CCPrioritizedTextureManager.h:108
>> +        if (priorityA == priorityB) {
> 
> Minor nit, Google usually prefers minimum nesting with early returns, eg:
> if (priorityA != priorityB)
>    return priorityIsLower(priorityA, priorityB)
> if (aboveCuffoffA != aboveCutoffB)
>    return aboveCutoffB;
> return a < b;

Fixed.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.