[Webkit-unassigned] [Bug 72686] [chromium] Need to prepaint tiles in TiledLayerChromium

[bugzilla-daemon at webkit.org]

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





--- Comment #14 from Adrienne Walker <enne at google.com>  2011-11-29 16:53:13 PST ---
(From update of attachment 116562)
View in context: https://bugs.webkit.org/attachment.cgi?id=116562&action=review

>>> Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:426
>>> +    idlePaintContentRect.inflate(idlePaintPaddingWidth);
>> 
>> I don't think idlePaintPaddingWidth should explicitly be the viewport size, because that won't make any sense for tiled non-root layers.
>> 
>> The contentRect here is the part of the layer that's visible.  For the root layer, this will always be the clip layer, which is sized to the viewport.  For iframes, the contentRect will (most often, unless it's clipped by some other masking layer or is partially offscreen) be the size of its clip rect as well, which is the portion of it that will be visible.  I think it'd be sufficient to use contentRect as an approximation for the viewport size.
> 
> Good point. However, with a single tile of padding the effects on memory management are minimal, so possibly memory priority could be dealt with in another patch this way. I tried to resolve the memory priority issue in this patch but I found it difficult since there are a few conflicting problems:
> 
> - Memory priority taken from visible tiles due to LRU (as you mentioned)
> - Layers that idle paint earlier can evict later layers' pre-painted tiles.
> - Endless paint thrashing can occur if we don't protect existing pre-paint tiles before trying to reserve new ones (eg. we could thrash between two layers, or between top/bottom within one layer). 
> 
> I think I've got a decent solution for all of the above, but using a small padding with high priority gets around the issue for now.

I'd be curious to hear what the eventual solution is.

In the short term, you should at least use m_tiler->tileSize() instead of defaultSize for idlePaintPaddingWidth.  Having a prepainted apron of even a single tile would still be a giant win for scrolling.

>>> Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:447
>>> +                break;
>> 
>> m_skipsIdlePaint seems kind of misnamed.  It's more "idle painting ran out of memory", and it's possible that we still painted a few tiles and that it wasn't skipped at all.  Could you just return a boolean from prepareToUpdateTiles about whether painting occurred or not, rather than setting a flag?
> 
> I did that briefly, but needsIdlePaint needs to know about out-of-memory so we don't keep calling setNeedsCommit. I'm thinking:
> a.) Maybe we should indeed skip the paint if part of the row/column can't be painted in full (I can reset the paintrect etc.)
> b.) I can just rename it to needsIdlePaint.

TiledLayerChromium::updateCompositorResources already asserts that all the textures that the paint rect intersects are reserved, so I think it's reasonable to not set the paint rect and just bail out if you can't reserve a whole row or column.

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