[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





--- Comment #27 from Sami Kyostila <skyostil at google.com>  2011-12-19 12:12:58 PST ---
(In reply to comment #26)
> (From update of attachment 119683 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=119683&action=review
> 
> High level design looks good to me. Still going through the details, but here's some initial easy-to-resolve feedback.
> 
> > Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:699
> > +        m_scrollableArea->scroll(scrollDelta.width() > 0 ? ScrollRight : ScrollLeft, ScrollByPixel, abs(scrollDelta.width()));
> > +    if (scrollDelta.height())
> > +        m_scrollableArea->scroll(scrollDelta.height() > 0 ? ScrollDown : ScrollUp, ScrollByPixel, abs(scrollDelta.height()));
> 
> I'm pretty sure you need to use the ..WithoutAnimation() variants here or WebKit will attempt to apply animation here, which doesn't make sense for scrolls initiated from the impl thread (which we've presumably already animated in the appropriate way)

Good point, scrollToOffsetWithoutAnimation() sounds like the right thing to do here.

> > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.cpp:32
> > +#include "RenderBox.h"
> 
> you don't actually need this include any more, do you? it's a layering violation

No I don't. This was left over by mistake.

> > Source/WebKit/chromium/src/ChromeClientImpl.h:36
> > +#include "GraphicsLayer.h"
> 
> can GraphicsLayer be forward declared?

Indeed it can. I was using PlatformLayer here before which is a typedef and cannot be forward declared.

> > Source/WebKit/chromium/src/ChromeClientImpl.h:44
> > +class Node;
> 
> don't think you need this

Done.

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