[Webkit-unassigned] [Bug 132535] Top content inset: Margin tiles should not display in the inset area when pinned to the top of the page

[bugzilla-daemon at webkit.org]

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


Simon Fraser (smfr) <simon.fraser at apple.com> changed:

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




--- Comment #4 from Simon Fraser (smfr) <simon.fraser at apple.com>  2014-05-03 16:11:14 PST ---
(From update of attachment 230768)
View in context: https://bugs.webkit.org/attachment.cgi?id=230768&action=review

r+ but it would be nice to try to share more of the layer position math, and fix UI-side compositing (at least encode/decode the new values).

> Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp:223
> +                positionForInsetClipLayer = FloatPoint();

No need, it's already 0,0

> Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp:226
> +                positionForInsetClipLayer = FloatPoint(0, topContentInset - scrollY);

positionForInsetClipLayer.setY(...)?

> Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp:235
> +                if (insetClipLayer)
> +                    insetClipLayer->setPosition(positionForInsetClipLayer);

Why did we spend all that time computing positionForInsetClipLayer when we don't have one :(

> Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:128
> +    // This is a clipping layer that will scroll with the page for all y-delta scroll values between 0
> +    // and topContentInset(). Once the y-deltas get beyond the content inset point, this layer no longer
> +    // needs to move.

Comment only applies to some configurations.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1603
> +FloatPoint RenderLayerCompositor::positionForClipLayer() const
> +{
> +    float contentInset = m_renderView.frameView().topContentInset();
> +    if (contentInset == 0)
> +        return FloatPoint();
> +
> +    int scrollY = std::max(0, m_renderView.frameView().scrollPosition().y());
> +
> +    if (scrollY >= contentInset)
> +        return FloatPoint();
> +
> +    return FloatPoint(0, contentInset - scrollY);
> +}
> +
> +float RenderLayerCompositor::topContentOffsetForRootLayer() const
> +{
> +    float contentInset = m_renderView.frameView().topContentInset();
> +    if (contentInset == 0)
> +        return 0;
> +
> +    int scrollY = std::max(0, m_renderView.frameView().scrollPosition().y());
> +
> +    if (scrollY >= contentInset)
> +        return contentInset;
> +
> +    return scrollY;
> +}

Doesn't this duplicate code in RenderLayerCompositor and teh scrolling tree node? Can we factor into a static FrameView function?

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