[Webkit-unassigned] [Bug 73350] [chromium] Allow scrolling non-root layers in the compositor thread

[bugzilla-daemon at webkit.org]

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


James Robinson <jamesr at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #122229|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #43 from James Robinson <jamesr at chromium.org>  2012-01-19 14:13:45 PST ---
(From update of attachment 122229)
View in context: https://bugs.webkit.org/attachment.cgi?id=122229&action=review

Overall I think this is nearly ready to go. I think we need to wait for https://bugs.webkit.org/show_bug.cgi?id=76663 to land to clean up the delegate stuff, and I'm hoping that the layer iterators can clean up some of the iteration logic. Otherwise we should be good to go.

> Source/WebCore/platform/graphics/chromium/LayerChromium.h:63
> +    virtual void scrollBy(const IntSize& scrollDelta) = 0;

I'd really prefer that you not grow the interface required for every LayerChromium.  In https://bugs.webkit.org/show_bug.cgi?id=76663 i'm getting rid of CCLayerDelegate and moving the paintContents() down to ContentLayerChromium. I think you can do the same thing with scrollBy(), the only layers that we actually support scrolling are GraphicsLayerChromium::m_layer's, right? That way you won't have to provide empty implementations in so many classes (like WebLayerImpl) where scrollBy() makes no sense

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h:141
> +    static void collectLayersInPaintOrder(CCLayerList&, CCRenderSurface*);
> +    static void collectNonDrawingScrollableLayers(CCLayerList&, CCLayerImpl*);
> +    static void collectAllLayers(CCLayerList&, CCLayerImpl*);

now that layer iterators have landed (http://trac.webkit.org/changeset/104626) can we use those instead of adding a set of new utility functions?

if we do need these, please separate the static functions from the member functions and add some documentation of what they do

> Source/WebKit/chromium/src/ChromeClientImpl.cpp:877
> +    PlatformLayer* platformLayer = layer->platformLayer();

since we're in chromium specific code here the PlatformLayer typedef is unnecessary abstraction here - just use LayerChromium here

> Source/WebKit/chromium/src/ChromeClientImpl.cpp:888
> +    PlatformLayer* platformLayer = layer->platformLayer();

same here re: using LayerChromium

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