- Revision
- 122244
- Author
- [email protected]
- Date
- 2012-07-10 12:14:25 -0700 (Tue, 10 Jul 2012)
Log 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:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (122243 => 122244)
--- trunk/LayoutTests/ChangeLog 2012-07-10 19:10:43 UTC (rev 122243)
+++ trunk/LayoutTests/ChangeLog 2012-07-10 19:14:25 UTC (rev 122244)
@@ -1,3 +1,14 @@
+2012-07-06 David Hyatt <[email protected]>
+
+ 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:
+
2012-07-10 Rafael Weinstein <[email protected]>
Unreviewed gardening. tables/mozilla_expected_failures/other/empty_cells.html marked as flaky.
Modified: trunk/LayoutTests/fast/block/float/previous-sibling-float-002-expected.html (122243 => 122244)
--- trunk/LayoutTests/fast/block/float/previous-sibling-float-002-expected.html 2012-07-10 19:10:43 UTC (rev 122243)
+++ trunk/LayoutTests/fast/block/float/previous-sibling-float-002-expected.html 2012-07-10 19:14:25 UTC (rev 122244)
@@ -8,7 +8,7 @@
{
background-color:green;
width: 400px;
- height: 30px;
+ height: 40px;
}
</style>
</head>
Modified: trunk/LayoutTests/fast/block/float/previous-sibling-float-002.html (122243 => 122244)
--- trunk/LayoutTests/fast/block/float/previous-sibling-float-002.html 2012-07-10 19:10:43 UTC (rev 122243)
+++ trunk/LayoutTests/fast/block/float/previous-sibling-float-002.html 2012-07-10 19:14:25 UTC (rev 122244)
@@ -8,6 +8,8 @@
{
background-color:green;
width: 400px;
+ overflow: hidden;
+ height:40px
}
#div1
{
@@ -16,7 +18,7 @@
#float-right
{
float: right;
- height: 20px;
+ height: 40px;
width: 400px;
background-color: green;
}
Modified: trunk/Source/WebCore/ChangeLog (122243 => 122244)
--- trunk/Source/WebCore/ChangeLog 2012-07-10 19:10:43 UTC (rev 122243)
+++ trunk/Source/WebCore/ChangeLog 2012-07-10 19:14:25 UTC (rev 122244)
@@ -1,3 +1,31 @@
+2012-07-06 David Hyatt <[email protected]>
+
+ 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):
+
2012-07-10 Alexei Filippov <[email protected]>
Web Inspector: Count inspector memory used to traverse DOM in native memory snapshots.
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (122243 => 122244)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-07-10 19:10:43 UTC (rev 122243)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-07-10 19:14:25 UTC (rev 122244)
@@ -2033,7 +2033,7 @@
RenderObject* prev = child->previousSibling();
if (prev && prev->isBlockFlow() && !prev->isFloatingOrOutOfFlowPositioned()) {
RenderBlock* block = toRenderBlock(prev);
- if (block->containsFloats() && block->lowestFloatLogicalBottom() > logicalTop)
+ if (block->containsFloats() && !block->avoidsFloats() && (block->logicalTop() + block->lowestFloatLogicalBottom()) > logicalTop)
addOverhangingFloats(block, false);
}
@@ -2089,14 +2089,60 @@
return yPos + heightIncrease;
}
+void RenderBlock::marginBeforeEstimateForChild(RenderBox* child, LayoutUnit& positiveMarginBefore, LayoutUnit& negativeMarginBefore) const
+{
+ // FIXME: We could get even more quirks mode cases right if we dealt with quirk containers.
+ // FIXME: We should deal with the margin-collapse-* style extensions that prevent collapsing and that discard margins.
+ LayoutUnit beforeChildMargin = marginBeforeForChild(child);
+ positiveMarginBefore = max(positiveMarginBefore, beforeChildMargin);
+ negativeMarginBefore = max(negativeMarginBefore, -beforeChildMargin);
+
+ if (!child->isRenderBlock())
+ return;
+
+ RenderBlock* childBlock = toRenderBlock(child);
+ if (childBlock->childrenInline() || childBlock->isWritingModeRoot())
+ return;
+
+ MarginInfo childMarginInfo(childBlock, childBlock->borderBefore() + childBlock->paddingBefore(), childBlock->borderAfter() + childBlock->paddingAfter());
+ if (!childMarginInfo.canCollapseMarginBeforeWithChildren())
+ return;
+
+ RenderBox* grandchildBox = childBlock->firstChildBox();
+ for ( ; grandchildBox; grandchildBox = grandchildBox->nextSiblingBox()) {
+ if (!grandchildBox->isFloatingOrOutOfFlowPositioned())
+ break;
+ }
+
+ // Give up if there is clearance on the box, since it probably won't collapse into us.
+ if (!grandchildBox || grandchildBox->style()->clear() != CNONE)
+ return;
+
+ // Collapse the margin of the grandchild box with our own to produce an estimate.
+ childBlock->marginBeforeEstimateForChild(grandchildBox, positiveMarginBefore, negativeMarginBefore);
+}
+
LayoutUnit RenderBlock::estimateLogicalTopPosition(RenderBox* child, const MarginInfo& marginInfo, LayoutUnit& estimateWithoutPagination)
{
// FIXME: We need to eliminate the estimation of vertical position, because when it's wrong we sometimes trigger a pathological
// relayout if there are intruding floats.
LayoutUnit logicalTopEstimate = logicalHeight();
if (!marginInfo.canCollapseWithMarginBefore()) {
- LayoutUnit childMarginBefore = child->selfNeedsLayout() ? marginBeforeForChild(child) : collapsedMarginBeforeForChild(child);
- logicalTopEstimate += max(marginInfo.margin(), childMarginBefore);
+ LayoutUnit positiveMarginBefore = ZERO_LAYOUT_UNIT;
+ LayoutUnit negativeMarginBefore = ZERO_LAYOUT_UNIT;
+ if (child->selfNeedsLayout()) {
+ // Try to do a basic estimation of how the collapse is going to go.
+ marginBeforeEstimateForChild(child, positiveMarginBefore, negativeMarginBefore);
+ } else {
+ // Use the cached collapsed margin values from a previous layout. Most of the time they
+ // will be right.
+ MarginValues marginValues = marginValuesForChild(child);
+ positiveMarginBefore = max(positiveMarginBefore, marginValues.positiveMarginBefore());
+ negativeMarginBefore = max(negativeMarginBefore, marginValues.negativeMarginBefore());
+ }
+
+ // Collapse the result with our current margins.
+ logicalTopEstimate += max(marginInfo.positiveMargin(), positiveMarginBefore) - max(marginInfo.negativeMargin(), negativeMarginBefore);
}
// Adjust logicalTopEstimate down to the next page if the margins are so large that we don't fit on the current
@@ -7065,7 +7111,7 @@
return marginAfterForChild(child);
}
-RenderBlock::MarginValues RenderBlock::marginValuesForChild(RenderBox* child)
+RenderBlock::MarginValues RenderBlock::marginValuesForChild(RenderBox* child) const
{
LayoutUnit childBeforePositive = 0;
LayoutUnit childBeforeNegative = 0;
Modified: trunk/Source/WebCore/rendering/RenderBlock.h (122243 => 122244)
--- trunk/Source/WebCore/rendering/RenderBlock.h 2012-07-10 19:10:43 UTC (rev 122243)
+++ trunk/Source/WebCore/rendering/RenderBlock.h 2012-07-10 19:14:25 UTC (rev 122244)
@@ -331,7 +331,7 @@
LayoutUnit m_positiveMarginAfter;
LayoutUnit m_negativeMarginAfter;
};
- MarginValues marginValuesForChild(RenderBox* child);
+ MarginValues marginValuesForChild(RenderBox* child) const;
virtual void scrollbarsChanged(bool /*horizontalScrollbarChanged*/, bool /*verticalScrollbarChanged*/) { };
@@ -921,6 +921,7 @@
LayoutUnit collapseMargins(RenderBox* child, MarginInfo&);
LayoutUnit clearFloatsIfNeeded(RenderBox* child, MarginInfo&, LayoutUnit oldTopPosMargin, LayoutUnit oldTopNegMargin, LayoutUnit yPos);
LayoutUnit estimateLogicalTopPosition(RenderBox* child, const MarginInfo&, LayoutUnit& estimateWithoutPagination);
+ void marginBeforeEstimateForChild(RenderBox* child, LayoutUnit& positiveMarginBefore, LayoutUnit& negativeMarginBefore) const;
void determineLogicalLeftPositionForChild(RenderBox* child);
void handleAfterSideOfBlock(LayoutUnit top, LayoutUnit bottom, MarginInfo&);
void setCollapsedBottomMargin(const MarginInfo&);
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (122243 => 122244)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2012-07-10 19:10:43 UTC (rev 122243)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2012-07-10 19:14:25 UTC (rev 122244)
@@ -1178,6 +1178,7 @@
}
LayoutUnit result = cb->availableLogicalWidthForLine(logicalTopPosition, false, containingBlockRegion, adjustedPageOffsetForContainingBlock) - childMarginStart - childMarginEnd;
+ result = max(result, minPreferredLogicalWidth()); // Don't shrink below our minimum preferred logical width.
// We need to see if margins on either the start side or the end side can contain the floats in question. If they can,
// then just using the line width is inaccurate. In the case where a float completely fits, we don't need to use the line
@@ -1750,7 +1751,7 @@
// shrinkToAvoidFloats() is only true for width: auto so the below code works correctly for
// width: fill-available since no case matches and it returns the logicalWidthResult from above.
if (shrinkToAvoidFloats() && cb->containsFloats())
- logicalWidthResult = shrinkLogicalWidthToAvoidFloats(marginStart, marginEnd, cb, region, offsetFromLogicalTopOfFirstPage);
+ logicalWidthResult = min(logicalWidthResult, shrinkLogicalWidthToAvoidFloats(marginStart, marginEnd, cb, region, offsetFromLogicalTopOfFirstPage));
if (logicalWidth.type() == MinContent)
logicalWidthResult = minPreferredLogicalWidth();
