[Webkit-unassigned] [Bug 70103] [chromium] Implement position:fixed in compositor thread

[bugzilla-daemon at webkit.org]

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





--- Comment #17 from Vangelis Kokkevis <vangelis at chromium.org>  2011-10-31 14:29:02 PST ---
(From update of attachment 113052)
View in context: https://bugs.webkit.org/attachment.cgi?id=113052&action=review

Patch looks good.  My main concern is the naming for the GraphicsLayer addition.  I was thinking of splitting this up to a WebKit common patch that just sets the properties on GraphicsLayer and an additional chromium-specific patch that handles scrolling.

> Source/WebCore/platform/graphics/GraphicsLayer.h:362
> +    bool anchorLayer() const { return m_anchorLayer; }

The word "anchor" is already used by "anchorPoint" which makes it confusing to reuse for a different purpose. Let's see what we're able to check into webkit.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:107
> +template<typename LayerType> static IntSize enclosingAncestorScrollDelta(LayerType* layer) {

The name of this function should reflect that it adds up deltas up to the enclosing ancestor.  Maybe something like: cummulativeScrollDeltasFromEnclosingAncestor() ? 

Also, have you considered implementing it as a while() loop instead of a recursion?  It will be faster.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:209
> +    // and the anchor layer.

It would be useful to add a short blurb here as to why there are no other transformations in between.

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