Changeset 122244 in webkit


Ignore:
Timestamp:
Jul 10, 2012 12:14:25 PM (12 years ago)
Author:
hyatt@apple.com
Message:

Source/WebCore: ​https://bugs.webkit.org/show_bug.cgi?id=90646
<rdar://problem/11648478> 3-pass pagination slows down pagination

Improve the logical top estimate function for margin collapsing to be more accurate. In particular
make the basic case of <body><p> or <body><h1> no longer be wrong. This estimate being incorrect
is not a big deal most of the time, but when paginating it is a very big deal, since you have to
relayout everything whenever your vertical placement is wrong.

Improving the estimation exposed a bug in an existing layout test. I had to clean up the buggy
code written for negative margin-related float detection and fix an invalid layout test to
actually be correct.

Reviewed by Simon Fraser.

  • rendering/RenderBlock.cpp:

(WebCore::RenderBlock::collapseMargins):
(WebCore::RenderBlock::marginBeforeEstimateForChild):
(WebCore):
(WebCore::RenderBlock::estimateLogicalTopPosition):
(WebCore::RenderBlock::marginValuesForChild):

  • rendering/RenderBlock.h:

(RenderBlock):

  • rendering/RenderBox.cpp:

(WebCore::RenderBox::shrinkLogicalWidthToAvoidFloats):
(WebCore::RenderBox::computeLogicalWidthInRegionUsing):

LayoutTests: ​https://bugs.webkit.org/show_bug.cgi?id=90646

Fix invalid layout test exposed by my changes to logical top estimation.

Reviewed by Simon Fraser.

  • fast/block/float/previous-sibling-float-002-expected.html:
  • fast/block/float/previous-sibling-float-002.html:
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r122240 r122244  
     12012-07-06  David Hyatt  <hyatt@apple.com>
     2
     3        https://bugs.webkit.org/show_bug.cgi?id=90646
     4
     5        Fix invalid layout test exposed by my changes to logical top estimation.
     6
     7        Reviewed by Simon Fraser.
     8
     9        * fast/block/float/previous-sibling-float-002-expected.html:
     10        * fast/block/float/previous-sibling-float-002.html:
     11
    1122012-07-10  Rafael Weinstein  <rafaelw@chromium.org>
    213

  • trunk/LayoutTests/fast/block/float/previous-sibling-float-002-expected.html

    r120844 r122244  
    99                background-color:green;
    1010                width: 400px;
    11                 height: 30px;
     11                height: 40px;
    1212            }
    1313        </style>

  • trunk/LayoutTests/fast/block/float/previous-sibling-float-002.html

    r120844 r122244  
    99                background-color:green;
    1010                width: 400px;
     11                overflow: hidden;
     12                height:40px
    1113            }
    1214            #div1
    … …  
    1719            {
    1820                float: right;
    19                 height: 20px;
     21                height: 40px;
    2022                width: 400px;
    2123                background-color: green;

  • trunk/Source/WebCore/ChangeLog

    r122243 r122244  
     12012-07-06  David Hyatt  <hyatt@apple.com>
     2
     3        https://bugs.webkit.org/show_bug.cgi?id=90646
     4        <rdar://problem/11648478> 3-pass pagination slows down pagination
     5
     6        Improve the logical top estimate function for margin collapsing to be more accurate. In particular
     7        make the basic case of <body><p> or <body><h1> no longer be wrong. This estimate being incorrect
     8        is not a big deal most of the time, but when paginating it is a very big deal, since you have to
     9        relayout everything whenever your vertical placement is wrong.
     10
     11        Improving the estimation exposed a bug in an existing layout test. I had to clean up the buggy
     12        code written for negative margin-related float detection and fix an invalid layout test to
     13        actually be correct.
     14
     15        Reviewed by Simon Fraser.
     16
     17        * rendering/RenderBlock.cpp:
     18        (WebCore::RenderBlock::collapseMargins):
     19        (WebCore::RenderBlock::marginBeforeEstimateForChild):
     20        (WebCore):
     21        (WebCore::RenderBlock::estimateLogicalTopPosition):
     22        (WebCore::RenderBlock::marginValuesForChild):
     23        * rendering/RenderBlock.h:
     24        (RenderBlock):
     25        * rendering/RenderBox.cpp:
     26        (WebCore::RenderBox::shrinkLogicalWidthToAvoidFloats):
     27        (WebCore::RenderBox::computeLogicalWidthInRegionUsing):
     28
    1292012-07-10  Alexei Filippov  <alexeif@chromium.org>
    230

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

    r121917 r122244  
    20342034    if (prev && prev->isBlockFlow() && !prev->isFloatingOrOutOfFlowPositioned()) {
    20352035        RenderBlock* block = toRenderBlock(prev);
    2036         if (block->containsFloats() && block->lowestFloatLogicalBottom() > logicalTop)
     2036        if (block->containsFloats() && !block->avoidsFloats() && (block->logicalTop() + block->lowestFloatLogicalBottom()) > logicalTop)
    20372037            addOverhangingFloats(block, false);
    20382038    }
    … …  
    20902090}
    20912091
     2092void RenderBlock::marginBeforeEstimateForChild(RenderBox* child, LayoutUnit& positiveMarginBefore, LayoutUnit& negativeMarginBefore) const
     2093{
     2094    // FIXME: We could get even more quirks mode cases right if we dealt with quirk containers.
     2095    // FIXME: We should deal with the margin-collapse-* style extensions that prevent collapsing and that discard margins.
     2096    LayoutUnit beforeChildMargin = marginBeforeForChild(child);
     2097    positiveMarginBefore = max(positiveMarginBefore, beforeChildMargin);
     2098    negativeMarginBefore = max(negativeMarginBefore, -beforeChildMargin);
     2099
     2100    if (!child->isRenderBlock())
     2101        return;
     2102   
     2103    RenderBlock* childBlock = toRenderBlock(child);
     2104    if (childBlock->childrenInline() || childBlock->isWritingModeRoot())
     2105        return;
     2106
     2107    MarginInfo childMarginInfo(childBlock, childBlock->borderBefore() + childBlock->paddingBefore(), childBlock->borderAfter() + childBlock->paddingAfter());
     2108    if (!childMarginInfo.canCollapseMarginBeforeWithChildren())
     2109        return;
     2110
     2111    RenderBox* grandchildBox = childBlock->firstChildBox();
     2112    for ( ; grandchildBox; grandchildBox = grandchildBox->nextSiblingBox()) {
     2113        if (!grandchildBox->isFloatingOrOutOfFlowPositioned())
     2114            break;
     2115    }
     2116   
     2117    // Give up if there is clearance on the box, since it probably won't collapse into us.
     2118    if (!grandchildBox || grandchildBox->style()->clear() != CNONE)
     2119        return;
     2120
     2121    // Collapse the margin of the grandchild box with our own to produce an estimate.
     2122    childBlock->marginBeforeEstimateForChild(grandchildBox, positiveMarginBefore, negativeMarginBefore);
     2123}
     2124
    20922125LayoutUnit RenderBlock::estimateLogicalTopPosition(RenderBox* child, const MarginInfo& marginInfo, LayoutUnit& estimateWithoutPagination)
    20932126{
    … …  
    20962129    LayoutUnit logicalTopEstimate = logicalHeight();
    20972130    if (!marginInfo.canCollapseWithMarginBefore()) {
    2098         LayoutUnit childMarginBefore = child->selfNeedsLayout() ? marginBeforeForChild(child) : collapsedMarginBeforeForChild(child);
    2099         logicalTopEstimate += max(marginInfo.margin(), childMarginBefore);
     2131        LayoutUnit positiveMarginBefore = ZERO_LAYOUT_UNIT;
     2132        LayoutUnit negativeMarginBefore = ZERO_LAYOUT_UNIT;
     2133        if (child->selfNeedsLayout()) {
     2134            // Try to do a basic estimation of how the collapse is going to go.
     2135            marginBeforeEstimateForChild(child, positiveMarginBefore, negativeMarginBefore);
     2136        } else {
     2137            // Use the cached collapsed margin values from a previous layout. Most of the time they
     2138            // will be right.
     2139            MarginValues marginValues = marginValuesForChild(child);
     2140            positiveMarginBefore = max(positiveMarginBefore, marginValues.positiveMarginBefore());
     2141            negativeMarginBefore = max(negativeMarginBefore, marginValues.negativeMarginBefore());
     2142        }
     2143
     2144        // Collapse the result with our current margins.
     2145        logicalTopEstimate += max(marginInfo.positiveMargin(), positiveMarginBefore) - max(marginInfo.negativeMargin(), negativeMarginBefore);
    21002146    }
    21012147
    … …  
    70667112}
    70677113
    7068 RenderBlock::MarginValues RenderBlock::marginValuesForChild(RenderBox* child)
     7114RenderBlock::MarginValues RenderBlock::marginValuesForChild(RenderBox* child) const
    70697115{
    70707116    LayoutUnit childBeforePositive = 0;

  • trunk/Source/WebCore/rendering/RenderBlock.h

    r121789 r122244  
    332332        LayoutUnit m_negativeMarginAfter;
    333333    };
    334     MarginValues marginValuesForChild(RenderBox* child);
     334    MarginValues marginValuesForChild(RenderBox* child) const;
    335335
    336336    virtual void scrollbarsChanged(bool /*horizontalScrollbarChanged*/, bool /*verticalScrollbarChanged*/) { };
    … …  
    922922    LayoutUnit clearFloatsIfNeeded(RenderBox* child, MarginInfo&, LayoutUnit oldTopPosMargin, LayoutUnit oldTopNegMargin, LayoutUnit yPos);
    923923    LayoutUnit estimateLogicalTopPosition(RenderBox* child, const MarginInfo&, LayoutUnit& estimateWithoutPagination);
     924    void marginBeforeEstimateForChild(RenderBox* child, LayoutUnit& positiveMarginBefore, LayoutUnit& negativeMarginBefore) const;
    924925    void determineLogicalLeftPositionForChild(RenderBox* child);
    925926    void handleAfterSideOfBlock(LayoutUnit top, LayoutUnit bottom, MarginInfo&);

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

    r121789 r122244  
    11791179
    11801180    LayoutUnit result = cb->availableLogicalWidthForLine(logicalTopPosition, false, containingBlockRegion, adjustedPageOffsetForContainingBlock) - childMarginStart - childMarginEnd;
     1181    result = max(result, minPreferredLogicalWidth()); // Don't shrink below our minimum preferred logical width.
    11811182
    11821183    // We need to see if margins on either the start side or the end side can contain the floats in question. If they can,
    … …  
    17511752        // width: fill-available since no case matches and it returns the logicalWidthResult from above.
    17521753        if (shrinkToAvoidFloats() && cb->containsFloats())
    1753             logicalWidthResult = shrinkLogicalWidthToAvoidFloats(marginStart, marginEnd, cb, region, offsetFromLogicalTopOfFirstPage);
     1754            logicalWidthResult = min(logicalWidthResult, shrinkLogicalWidthToAvoidFloats(marginStart, marginEnd, cb, region, offsetFromLogicalTopOfFirstPage));
    17541755
    17551756        if (logicalWidth.type() == MinContent)
Note: See TracChangeset for help on using the changeset viewer.