Bug 47751

Summary: [chromium] Avoid creating excessively large textures for composited layers
Product: WebKit Reporter: Vangelis Kokkevis <vangelis>
Component: WebCore Misc.Assignee: Vangelis Kokkevis <vangelis>
Status: RESOLVED FIXED    
Severity: Normal CC: enne, jamesr, kbr
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Proposed patch jamesr: review+

Description Vangelis Kokkevis 2010-10-15 16:30:20 PDT
Switch the compositor to "large layer" mode when a layer's size exceeds a reasonable value rather than the max texture size supported by the driver in order to avoid running low on vram on pages that create large layer (zooming in on maps.google.com creates several layers that can be 4Kx4K or bigger).  This is a temporary solution until tiled layers are implemented.
Comment 1 Vangelis Kokkevis 2010-10-15 17:25:21 PDT
Created attachment 70927 [details]
Proposed patch
Comment 2 James Robinson 2010-10-15 17:36:04 PDT
Comment on attachment 70927 [details]
Proposed patch

Will layers >2000 pixels wide still work at all?  Monitors that support resolutions higher than that are still somewhat common
Comment 3 Vangelis Kokkevis 2010-10-15 17:57:42 PDT
(In reply to comment #2)
> (From update of attachment 70927 [details])
> Will layers >2000 pixels wide still work at all?  Monitors that support resolutions higher than that are still somewhat common


They should work, as long as they are smaller than the max texture size supported by your driver. 

If you look at the ContentLayerChromium::requiresClippedUpdateRect() we switch to large layer mode when the layer exceeds the larger of 2000 or the browser window's visible area.
Comment 4 James Robinson 2010-10-18 14:28:20 PDT
Comment on attachment 70927 [details]
Proposed patch

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

R=me

> WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:149
> +    const int maxLayerSize = 2000;

nit: should be static. it'll probably get inlined anyway
Comment 5 Adrienne Walker 2010-10-18 14:36:17 PDT
Comment on attachment 70927 [details]
Proposed patch

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

> WebCore/platform/graphics/chromium/LayerRendererChromium.h:169
>      IntRect m_rootVisibleRect;
>      IntRect m_rootContentRect;
> +    IntRect m_currentScissorRect;
>  

Can we remove m_rootContentRect, then? I think the scissor rect replaces its purpose entirely.
Comment 6 Vangelis Kokkevis 2010-10-18 14:39:13 PDT
(In reply to comment #5)
> (From update of attachment 70927 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=70927&action=review
> 
> > WebCore/platform/graphics/chromium/LayerRendererChromium.h:169
> >      IntRect m_rootVisibleRect;
> >      IntRect m_rootContentRect;
> > +    IntRect m_currentScissorRect;
> >  
> 
> Can we remove m_rootContentRect, then? I think the scissor rect replaces its purpose entirely.

m_currentScissorRect can actually change while drawing the composited layers (for example when we hit a layer that clips its descendants).
Comment 7 Vangelis Kokkevis 2010-10-18 14:43:00 PDT
(In reply to comment #4)
> (From update of attachment 70927 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=70927&action=review
> 
> R=me
> 
> > WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:149
> > +    const int maxLayerSize = 2000;
> 
> nit: should be static. it'll probably get inlined anyway

Good point. I'll fix that before checking in.
Comment 8 Adrienne Walker 2010-10-18 14:57:56 PDT
> Can we remove m_rootContentRect, then? I think the scissor rect replaces its purpose entirely.

Oops, my mistake.  It's still getting used.
Comment 9 Vangelis Kokkevis 2010-10-18 16:54:11 PDT
Committed r70010: <http://trac.webkit.org/changeset/70010>