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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 19 16:06:07 PST 2011


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


James Robinson <jamesr at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #119898|review?                     |review+
               Flag|                            |




--- Comment #29 from James Robinson <jamesr at chromium.org>  2011-12-19 16:06:06 PST ---
(From update of attachment 119898)
View in context: https://bugs.webkit.org/attachment.cgi?id=119898&action=review

R=me, but I'd appreciate it if you let aelias@ take a pass over the page scale apply/unapply logic to make sure it's all consistent.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:626
> +    bool didMove = m_scrollLayerImpl && (didScrollSubtree(m_scrollLayerImpl.get()) || m_pageScaleDelta != 1.0f);

webkit style nit: the comparison should be written as "m_pageScaleDelta != 1", without the ".0f" qualifier.

> Source/WebKit/chromium/src/ChromeClientImpl.cpp:68
> +#include "RenderLayer.h"

pretty sure you don't need this #include any more now either (it isn't wrong in terms of code layering, just unnecessary)

> Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:172
> +    RefPtr<CCLayerImpl> root = CCLayerImpl::create(0);
> +    root->setScrollable(false);
> +    RefPtr<CCLayerImpl> child = CCLayerImpl::create(0);

nit: i think it's generally a bad idea to construct CCLayerImpls with overlapping IDs in tests since we maintain the invariant that we never reuse IDs in the normal code.

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



More information about the webkit-unassigned mailing list