Changeset 152998 in webkit


Ignore:
Timestamp:
Jul 22, 2013, 2:32:39 PM (12 years ago)
Author:
Beth Dakin
Message:

StickyPositionContraints should not need to change to account for a RenderLayer's
scrollOffset
​https://bugs.webkit.org/show_bug.cgi?id=118958
-and corresponding-
<rdar://problem/12469203>

Reviewed by Simon Fraser.

Source/WebCore:

Before this patch, to get sticky offsets right in overflow areas, the
StickyPositionConstraints changed on every scroll to factor it in. This will be a
problem once we can scroll overflow areas on the scrolling thread. The constraints
should never have to change to account for the scroll position. This patch fixes
that issue by changing the StickyPositionViewportConstraints’s containerBlockRect
and stickyBoxRect to be in a coordinate space that is relative to the scrolling
ancestor rather than being absolute. This patch also removes ‘absolute’ from those
variable names since they are no longer absolute.

A few re-names in the StickyPositionViewportConstraints class. The parameter to
computeStickyOffset() used to be called viewportRect, and is now called
constrainingRect. m_absoluteStickyBoxRect is now m_stickyBoxRect, and
m_absoluteContainingBlockRect is now m_containingBlockRect. And finally,
layerPositionForViewportRect() is now layerPositionForConstrainingRect()

  • page/scrolling/ScrollingConstraints.cpp:

(WebCore::StickyPositionViewportConstraints::computeStickyOffset):
(WebCore::StickyPositionViewportConstraints::layerPositionForConstrainingRect):

  • page/scrolling/ScrollingConstraints.h:

(WebCore::StickyPositionViewportConstraints::StickyPositionViewportConstraints):
(WebCore::StickyPositionViewportConstraints::containingBlockRect):
(WebCore::StickyPositionViewportConstraints::setContainingBlockRect):
(WebCore::StickyPositionViewportConstraints::stickyBoxRect):
(WebCore::StickyPositionViewportConstraints::setStickyBoxRect):
(WebCore::StickyPositionViewportConstraints::operator==):

Accounting for the re-names.

  • page/scrolling/ScrollingStateStickyNode.cpp:

(WebCore::ScrollingStateStickyNode::syncLayerPositionForViewportRect):
(WebCore::ScrollingStateStickyNode::dumpProperties):

  • page/scrolling/mac/ScrollingTreeStickyNode.mm:

(WebCore::ScrollingTreeStickyNode::parentScrollPositionDidChange):

Compute all values relative to the scrolling ancestor. This requires some juggling
in the overflow case to factor border and padding in or out.

  • rendering/RenderBoxModelObject.cpp:

(WebCore::RenderBoxModelObject::computeStickyPositionConstraints):

This is where the scrollOffset should be factored in.
(WebCore::RenderBoxModelObject::stickyPositionOffset):

LayoutTests:

This tests stick in overflow areas where the sticky’s containing block overflows
the overflow area. The sticky object should not extend beyond the overflow area in
that case.

  • fast/css/sticky/sticky-top-overflow-container-overflow-expected.html: Added.
  • fast/css/sticky/sticky-top-overflow-container-overflow.html: Added.
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r152987 r152998  
     12013-07-22  Beth Dakin  <bdakin@apple.com>
     2
     3        StickyPositionContraints should not need to change to account for a RenderLayer's
     4        scrollOffset
     5        https://bugs.webkit.org/show_bug.cgi?id=118958
     6        -and corresponding-
     7        <rdar://problem/12469203>
     8
     9        Reviewed by Simon Fraser.
     10
     11        This tests stick in overflow areas where the sticky’s containing block overflows
     12        the overflow area. The sticky object should not extend beyond the overflow area in
     13        that case.
     14
     15        * fast/css/sticky/sticky-top-overflow-container-overflow-expected.html: Added.
     16        * fast/css/sticky/sticky-top-overflow-container-overflow.html: Added.
     17
    1182013-07-22  Joone Hur  <joone.hur@intel.com>
    219

  • trunk/Source/WebCore/ChangeLog

    r152989 r152998  
     12013-07-22  Beth Dakin  <bdakin@apple.com>
     2
     3        StickyPositionContraints should not need to change to account for a RenderLayer's
     4        scrollOffset
     5        https://bugs.webkit.org/show_bug.cgi?id=118958
     6        -and corresponding-
     7        <rdar://problem/12469203>
     8
     9        Reviewed by Simon Fraser.
     10
     11        Before this patch, to get sticky offsets right in overflow areas, the
     12        StickyPositionConstraints changed on every scroll to factor it in. This will be a
     13        problem once we can scroll overflow areas on the scrolling thread. The constraints
     14        should never have to change to account for the scroll position. This patch fixes
     15        that issue by changing the StickyPositionViewportConstraints’s containerBlockRect
     16        and stickyBoxRect to be in a coordinate space that is relative to the scrolling
     17        ancestor rather than being absolute. This patch also removes ‘absolute’ from those
     18        variable names since they are no longer absolute.
     19
     20        A few re-names in the StickyPositionViewportConstraints class. The parameter to
     21        computeStickyOffset() used to be called viewportRect, and is now called
     22        constrainingRect. m_absoluteStickyBoxRect is now m_stickyBoxRect, and
     23        m_absoluteContainingBlockRect is now m_containingBlockRect. And finally,
     24        layerPositionForViewportRect() is now layerPositionForConstrainingRect()
     25        * page/scrolling/ScrollingConstraints.cpp:
     26        (WebCore::StickyPositionViewportConstraints::computeStickyOffset):
     27        (WebCore::StickyPositionViewportConstraints::layerPositionForConstrainingRect):
     28        * page/scrolling/ScrollingConstraints.h:
     29        (WebCore::StickyPositionViewportConstraints::StickyPositionViewportConstraints):
     30        (WebCore::StickyPositionViewportConstraints::containingBlockRect):
     31        (WebCore::StickyPositionViewportConstraints::setContainingBlockRect):
     32        (WebCore::StickyPositionViewportConstraints::stickyBoxRect):
     33        (WebCore::StickyPositionViewportConstraints::setStickyBoxRect):
     34        (WebCore::StickyPositionViewportConstraints::operator==):
     35
     36        Accounting for the re-names.
     37        * page/scrolling/ScrollingStateStickyNode.cpp:
     38        (WebCore::ScrollingStateStickyNode::syncLayerPositionForViewportRect):
     39        (WebCore::ScrollingStateStickyNode::dumpProperties):
     40        * page/scrolling/mac/ScrollingTreeStickyNode.mm:
     41        (WebCore::ScrollingTreeStickyNode::parentScrollPositionDidChange):
     42
     43        Compute all values relative to the scrolling ancestor. This requires some juggling
     44        in the overflow case to factor border and padding in or out.
     45        * rendering/RenderBoxModelObject.cpp:
     46        (WebCore::RenderBoxModelObject::computeStickyPositionConstraints):
     47
     48        This is where the scrollOffset should be factored in.
     49        (WebCore::RenderBoxModelObject::stickyPositionOffset):
     50
    1512013-07-22  Dean Jackson  <dino@apple.com>
    252

  • trunk/Source/WebCore/page/scrolling/ScrollingConstraints.cpp

    r132968 r152998  
    4646}
    4747
    48 FloatSize StickyPositionViewportConstraints::computeStickyOffset(const FloatRect& viewportRect) const
     48FloatSize StickyPositionViewportConstraints::computeStickyOffset(const FloatRect& constrainingRect) const
    4949{
    50     FloatRect boxRect = m_absoluteStickyBoxRect;
     50    FloatRect boxRect = m_stickyBoxRect;
    5151   
    5252    if (hasAnchorEdge(AnchorEdgeRight)) {
    53         float rightLimit = viewportRect.maxX() - m_rightOffset;
    54         float rightDelta = std::min<float>(0, rightLimit - m_absoluteStickyBoxRect.maxX());
    55         float availableSpace = std::min<float>(0, m_absoluteContainingBlockRect.x() - m_absoluteStickyBoxRect.x());
     53        float rightLimit = constrainingRect.maxX() - m_rightOffset;
     54        float rightDelta = std::min<float>(0, rightLimit - m_stickyBoxRect.maxX());
     55        float availableSpace = std::min<float>(0, m_containingBlockRect.x() - m_stickyBoxRect.x());
    5656        if (rightDelta < availableSpace)
    5757            rightDelta = availableSpace;
    … …  
    6161
    6262    if (hasAnchorEdge(AnchorEdgeLeft)) {
    63         float leftLimit = viewportRect.x() + m_leftOffset;
    64         float leftDelta = std::max<float>(0, leftLimit - m_absoluteStickyBoxRect.x());
    65         float availableSpace = std::max<float>(0, m_absoluteContainingBlockRect.maxX() - m_absoluteStickyBoxRect.maxX());
     63        float leftLimit = constrainingRect.x() + m_leftOffset;
     64        float leftDelta = std::max<float>(0, leftLimit - m_stickyBoxRect.x());
     65        float availableSpace = std::max<float>(0, m_containingBlockRect.maxX() - m_stickyBoxRect.maxX());
    6666        if (leftDelta > availableSpace)
    6767            leftDelta = availableSpace;
    … …  
    7171   
    7272    if (hasAnchorEdge(AnchorEdgeBottom)) {
    73         float bottomLimit = viewportRect.maxY() - m_bottomOffset;
    74         float bottomDelta = std::min<float>(0, bottomLimit - m_absoluteStickyBoxRect.maxY());
    75         float availableSpace = std::min<float>(0, m_absoluteContainingBlockRect.y() - m_absoluteStickyBoxRect.y());
     73        float bottomLimit = constrainingRect.maxY() - m_bottomOffset;
     74        float bottomDelta = std::min<float>(0, bottomLimit - m_stickyBoxRect.maxY());
     75        float availableSpace = std::min<float>(0, m_containingBlockRect.y() - m_stickyBoxRect.y());
    7676        if (bottomDelta < availableSpace)
    7777            bottomDelta = availableSpace;
    … …  
    8181
    8282    if (hasAnchorEdge(AnchorEdgeTop)) {
    83         float topLimit = viewportRect.y() + m_topOffset;
    84         float topDelta = std::max<float>(0, topLimit - m_absoluteStickyBoxRect.y());
    85         float availableSpace = std::max<float>(0, m_absoluteContainingBlockRect.maxY() - m_absoluteStickyBoxRect.maxY());
     83        float topLimit = constrainingRect.y() + m_topOffset;
     84        float topDelta = std::max<float>(0, topLimit - m_stickyBoxRect.y());
     85        float availableSpace = std::max<float>(0, m_containingBlockRect.maxY() - m_stickyBoxRect.maxY());
    8686        if (topDelta > availableSpace)
    8787            topDelta = availableSpace;
    … …  
    9090    }
    9191
    92     return boxRect.location() - m_absoluteStickyBoxRect.location();
     92    return boxRect.location() - m_stickyBoxRect.location();
    9393}
    9494
    95 FloatPoint StickyPositionViewportConstraints::layerPositionForViewportRect(const FloatRect& viewportRect) const
     95FloatPoint StickyPositionViewportConstraints::layerPositionForConstrainingRect(const FloatRect& constrainingRect) const
    9696{
    97     FloatSize offset = computeStickyOffset(viewportRect);
     97    FloatSize offset = computeStickyOffset(constrainingRect);
    9898    return m_layerPositionAtLastLayout + offset - m_stickyOffsetAtLastLayout;
    9999}

  • trunk/Source/WebCore/page/scrolling/ScrollingConstraints.h

    r144687 r152998  
    126126        , m_topOffset(other.m_topOffset)
    127127        , m_bottomOffset(other.m_bottomOffset)
    128         , m_absoluteContainingBlockRect(other.m_absoluteContainingBlockRect)
    129         , m_absoluteStickyBoxRect(other.m_absoluteStickyBoxRect)
     128        , m_containingBlockRect(other.m_containingBlockRect)
     129        , m_stickyBoxRect(other.m_stickyBoxRect)
    130130        , m_stickyOffsetAtLastLayout(other.m_stickyOffsetAtLastLayout)
    131131        , m_layerPositionAtLastLayout(other.m_layerPositionAtLastLayout)
    132132    { }
    133133
    134     FloatSize computeStickyOffset(const FloatRect& viewportRect) const;
     134    FloatSize computeStickyOffset(const FloatRect& constrainingRect) const;
    135135
    136136    const FloatSize stickyOffsetAtLastLayout() const { return m_stickyOffsetAtLastLayout; }
    137137    void setStickyOffsetAtLastLayout(const FloatSize& offset) { m_stickyOffsetAtLastLayout = offset; }
    138138
    139     FloatPoint layerPositionForViewportRect(const FloatRect& viewportRect) const;
     139    FloatPoint layerPositionForConstrainingRect(const FloatRect& constrainingRect) const;
    140140
    141141    const FloatPoint& layerPositionAtLastLayout() const { return m_layerPositionAtLastLayout; }
    … …  
    152152    void setBottomOffset(float offset) { m_bottomOffset = offset; }
    153153
    154     FloatRect absoluteContainingBlockRect() const { return m_absoluteContainingBlockRect; }
    155     void setAbsoluteContainingBlockRect(const FloatRect& rect) { m_absoluteContainingBlockRect = rect; }
     154    // containingBlockRect() is in the scrolling ancestor's coordinate space.
     155    FloatRect containingBlockRect() const { return m_containingBlockRect; }
     156    void setContainingBlockRect(const FloatRect& rect) { m_containingBlockRect = rect; }
    156157
    157     FloatRect absoluteStickyBoxRect() const { return m_absoluteStickyBoxRect; }
    158     void setAbsoluteStickyBoxRect(const FloatRect& rect) { m_absoluteStickyBoxRect = rect; }
     158    // stickyBoxRect() is in the scrolling ancestor's coordinate space.
     159    FloatRect stickyBoxRect() const { return m_stickyBoxRect; }
     160    void setStickyBoxRect(const FloatRect& rect) { m_stickyBoxRect = rect; }
    159161
    160162    bool operator==(const StickyPositionViewportConstraints& other) const
    … …  
    164166            && m_topOffset == other.m_topOffset
    165167            && m_bottomOffset == other.m_bottomOffset
    166             && m_absoluteContainingBlockRect == other.m_absoluteContainingBlockRect
    167             && m_absoluteStickyBoxRect == other.m_absoluteStickyBoxRect
     168            && m_containingBlockRect == other.m_containingBlockRect
     169            && m_stickyBoxRect == other.m_stickyBoxRect
    168170            && m_stickyOffsetAtLastLayout == other.m_stickyOffsetAtLastLayout
    169171            && m_layerPositionAtLastLayout == other.m_layerPositionAtLastLayout;
    … …  
    179181    float m_topOffset;
    180182    float m_bottomOffset;
    181     FloatRect m_absoluteContainingBlockRect;
    182     FloatRect m_absoluteStickyBoxRect;
     183    FloatRect m_containingBlockRect;
     184    FloatRect m_stickyBoxRect;
    183185    FloatSize m_stickyOffsetAtLastLayout;
    184186    FloatPoint m_layerPositionAtLastLayout;

  • trunk/Source/WebCore/page/scrolling/ScrollingStateStickyNode.cpp

    r148298 r152998  
    7373void ScrollingStateStickyNode::syncLayerPositionForViewportRect(const LayoutRect& viewportRect)
    7474{
    75     FloatPoint position = m_constraints.layerPositionForViewportRect(viewportRect);
     75    FloatPoint position = m_constraints.layerPositionForConstrainingRect(viewportRect);
    7676    graphicsLayer()->syncPosition(position);
    7777}
    … …  
    113113
    114114    writeIndent(ts, indent + 1);
    115     FloatRect r = m_constraints.absoluteContainingBlockRect();
     115    FloatRect r = m_constraints.containingBlockRect();
    116116    ts << "(containing block rect " << r.x() << ", " << r.y() << " " << r.width() << " x " << r.height() << ")\n";
    117117
    118118    writeIndent(ts, indent + 1);
    119     r = m_constraints.absoluteStickyBoxRect();
     119    r = m_constraints.stickyBoxRect();
    120120    ts << "(sticky box rect " << r.x() << " " << r.y() << " " << r.width() << " " << r.height() << ")\n";
    121121
    122122    writeIndent(ts, indent + 1);
    123     r = m_constraints.absoluteStickyBoxRect();
     123    r = m_constraints.stickyBoxRect();
    124124    ts << "(sticky box rect " << r.x() << " " << r.y() << " " << r.width() << " " << r.height() << ")\n";
    125125

  • trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeStickyNode.mm

    r142520 r152998  
    6666void ScrollingTreeStickyNode::parentScrollPositionDidChange(const IntRect& viewportRect, const FloatSize& cumulativeDelta)
    6767{
    68     FloatPoint layerPosition = m_constraints.layerPositionForViewportRect(viewportRect);
     68    FloatPoint layerPosition = m_constraints.layerPositionForConstrainingRect(viewportRect);
    6969
    7070    // FIXME: Subtracting the cumulativeDelta is not totally sufficient to get the new position right for nested
    71     // sticky objects. We probably need a way to modify the absoluteContainingBlockRect in the ViewportContraints
     71    // sticky objects. We probably need a way to modify the containingBlockRect in the ViewportContraints
    7272    // to get this right in all cases.
    7373    layerPosition -= cumulativeDelta;

  • trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

    r152122 r152998  
    528528{
    529529    RenderBlock* containingBlock = this->containingBlock();
    530 
    531     LayoutRect containerContentRect = containingBlock->contentBoxRect();
     530    RenderLayer* enclosingClippingLayer = layer()->enclosingOverflowClipLayer(ExcludeSelf);
     531    RenderBox* enclosingClippingBox = enclosingClippingLayer ? toRenderBox(enclosingClippingLayer->renderer()) : view();
     532
     533    LayoutRect containerContentRect;
     534    if (!enclosingClippingLayer || (containingBlock != enclosingClippingBox))
     535        containerContentRect = containingBlock->contentBoxRect();
     536    else {
     537        containerContentRect = containingBlock->layoutOverflowRect();
     538        LayoutPoint containerLocation = containerContentRect.location() + LayoutPoint(containingBlock->borderLeft() + containingBlock->paddingLeft(),
     539            containingBlock->borderTop() + containingBlock->paddingTop());
     540        containerContentRect.setLocation(containerLocation);
     541    }
     542
    532543    LayoutUnit maxWidth = containingBlock->availableLogicalWidth();
    533544
    … …  
    541552    // Compute the container-relative area within which the sticky element is allowed to move.
    542553    containerContentRect.contract(minMargin);
    543     // Map to the view to avoid including page scale factor.
    544     constraints.setAbsoluteContainingBlockRect(containingBlock->localToContainerQuad(FloatRect(containerContentRect), view()).boundingBox());
    545 
     554
     555    // Finally compute container rect relative to the scrolling ancestor.
     556    FloatRect containerRectRelativeToScrollingAncestor = containingBlock->localToContainerQuad(FloatRect(containerContentRect), enclosingClippingBox).boundingBox();
     557    if (enclosingClippingLayer) {
     558        FloatPoint containerLocationRelativeToScrollingAncestor = containerRectRelativeToScrollingAncestor.location() -
     559            FloatSize(enclosingClippingBox->borderLeft() + enclosingClippingBox->paddingLeft(),
     560            enclosingClippingBox->borderTop() + enclosingClippingBox->paddingTop());
     561        if (enclosingClippingBox != containingBlock)
     562            containerLocationRelativeToScrollingAncestor += enclosingClippingLayer->scrollOffset();
     563        containerRectRelativeToScrollingAncestor.setLocation(containerLocationRelativeToScrollingAncestor);
     564    }
     565    constraints.setContainingBlockRect(containerRectRelativeToScrollingAncestor);
     566
     567    // Now compute the sticky box rect, also relative to the scrolling ancestor.
    546568    LayoutRect stickyBoxRect = frameRectForStickyPositioning();
    547569    LayoutRect flippedStickyBoxRect = stickyBoxRect;
    548570    containingBlock->flipForWritingMode(flippedStickyBoxRect);
    549     LayoutPoint stickyLocation = flippedStickyBoxRect.location();
    550 
    551     // FIXME: sucks to call localToAbsolute again, but we can't just offset from the previously computed rect if there are transforms.
     571    FloatRect stickyBoxRelativeToScrollingAnecstor = flippedStickyBoxRect;
     572
     573    // FIXME: sucks to call localToContainerQuad again, but we can't just offset from the previously computed rect if there are transforms.
    552574    // Map to the view to avoid including page scale factor.
    553     FloatRect absContainerFrame = containingBlock->localToContainerQuad(FloatRect(FloatPoint(), containingBlock->size()), view()).boundingBox();
    554 
    555     if (containingBlock->hasOverflowClip()) {
    556         IntSize scrollOffset = containingBlock->layer()->scrollOffset();
    557         stickyLocation -= scrollOffset;
    558     }
    559 
    560     // We can't call localToAbsolute on |this| because that will recur. FIXME: For now, assume that |this| is not transformed.
    561     FloatRect absoluteStickyBoxRect(absContainerFrame.location() + stickyLocation, flippedStickyBoxRect.size());
    562     constraints.setAbsoluteStickyBoxRect(absoluteStickyBoxRect);
     575    FloatPoint stickyLocationRelativeToScrollingAncestor = flippedStickyBoxRect.location() + containingBlock->localToContainerQuad(FloatRect(FloatPoint(), containingBlock->size()), enclosingClippingBox).boundingBox().location();
     576    if (enclosingClippingLayer) {
     577        stickyLocationRelativeToScrollingAncestor -= FloatSize(enclosingClippingBox->borderLeft() + enclosingClippingBox->paddingLeft(),
     578            enclosingClippingBox->borderTop() + enclosingClippingBox->paddingTop());
     579        if (enclosingClippingBox != containingBlock)
     580            stickyLocationRelativeToScrollingAncestor += enclosingClippingLayer->scrollOffset();
     581    }
     582    // FIXME: For now, assume that |this| is not transformed.
     583    stickyBoxRelativeToScrollingAnecstor.setLocation(stickyLocationRelativeToScrollingAncestor);
     584    constraints.setStickyBoxRect(stickyBoxRelativeToScrollingAnecstor);
    563585
    564586    if (!style()->left().isAuto()) {
    … …  
    593615        LayoutRect clipRect = enclosingClippingBox->overflowClipRect(LayoutPoint(), 0); // FIXME: make this work in regions.
    594616        constrainingRect = enclosingClippingBox->localToContainerQuad(FloatRect(clipRect), view()).boundingBox();
     617
     618        FloatPoint scrollOffset = FloatPoint() + enclosingClippingLayer->scrollOffset();
     619        constrainingRect.setLocation(scrollOffset);
    595620    } else {
    596621        LayoutRect viewportRect = view()->frameView()->viewportConstrainedVisibleContentRect();
Note: See TracChangeset for help on using the changeset viewer.