Modified: trunk/Source/WebCore/ChangeLog (240474 => 240475)
--- trunk/Source/WebCore/ChangeLog 2019-01-25 16:46:47 UTC (rev 240474)
+++ trunk/Source/WebCore/ChangeLog 2019-01-25 16:50:18 UTC (rev 240475)
@@ -1,5 +1,38 @@
2019-01-25 Zalan Bujtas <za...@apple.com>
+ [LFC][BFC][MarginCollapsing] Add "clear" to static position computation.
+ https://bugs.webkit.org/show_bug.cgi?id=193824
+
+ Reviewed by Antti Koivisto.
+
+ When clear property is set and floats are present, we have to estimate and set the box's vertical position during
+ static positioning to be able to properly layout its subtree.
+
+ <div style="float: left; width: 100px; height: 100px;"></div>
+ <div style="clear: left;">
+ <div style="float: left; width: 100px; height: 100px;"></div>
+ </div>
+
+ In the above example since the second float's parent clears the first float, the second float is positioned below
+ the first float. If we didn't push down (clear) the box, the float child would get placed next to the first float.
+
+ * layout/blockformatting/BlockFormattingContext.cpp:
+ (WebCore::Layout::BlockFormattingContext::layout const):
+ (WebCore::Layout::BlockFormattingContext::layoutFormattingContextRoot const):
+ (WebCore::Layout::BlockFormattingContext::computeStaticPosition const):
+ (WebCore::Layout::BlockFormattingContext::computeEstimatedVerticalPosition const):
+ (WebCore::Layout::BlockFormattingContext::computeEstimatedVerticalPositionForFloatClear const):
+ (WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const):
+ (WebCore::Layout::BlockFormattingContext::verticalPositionWithMargin const):
+ (WebCore::Layout::BlockFormattingContext::computeVerticalPositionForFloatClear const): Deleted.
+ (WebCore::Layout::BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing const): Deleted.
+ * layout/blockformatting/BlockFormattingContext.h:
+ * layout/blockformatting/BlockMarginCollapse.cpp:
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::estimatedMarginBefore):
+ * layout/displaytree/DisplayBox.h:
+
+2019-01-25 Zalan Bujtas <za...@apple.com>
+
[LFC][BFC][MarginCollapsing] Move positive/negative margin value updating to a dedicated function
https://bugs.webkit.org/show_bug.cgi?id=193812
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp (240474 => 240475)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp 2019-01-25 16:46:47 UTC (rev 240474)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp 2019-01-25 16:50:18 UTC (rev 240475)
@@ -90,7 +90,7 @@
LOG_WITH_STREAM(FormattingContextLayout, stream << "[Compute] -> [Position][Border][Padding][Width][Margin] -> for layoutBox(" << &layoutBox << ")");
computeBorderAndPadding(layoutBox);
computeWidthAndMargin(layoutBox);
- computeStaticPosition(layoutBox);
+ computeStaticPosition(floatingContext, layoutBox);
if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowOrFloatingChild())
break;
layoutQueue.append(downcast<Container>(layoutBox).firstInFlowOrFloatingChild());
@@ -105,9 +105,6 @@
// Formatting root boxes are special-cased and they don't come here.
ASSERT(!layoutBox.establishesFormattingContext());
computeHeightAndMargin(layoutBox);
- // Finalize position with clearance.
- if (layoutBox.hasFloatClear())
- computeVerticalPositionForFloatClear(floatingContext, layoutBox);
if (!is<Container>(layoutBox))
continue;
auto& container = downcast<Container>(layoutBox);
@@ -130,7 +127,7 @@
LOG_WITH_STREAM(FormattingContextLayout, stream << "[Compute] -> [Position][Border][Padding][Width][Margin] -> for layoutBox(" << &layoutBox << ")");
computeBorderAndPadding(layoutBox);
computeWidthAndMargin(layoutBox);
- computeStaticPosition(layoutBox);
+ computeStaticPosition(floatingContext, layoutBox);
// Swich over to the new formatting context (the one that the root creates).
auto formattingContext = layoutState().createFormattingContext(layoutBox);
formattingContext->layout();
@@ -143,9 +140,7 @@
if (layoutBox.isFloatingPositioned()) {
computeFloatingPosition(floatingContext, layoutBox);
floatingContext.floatingState().append(layoutBox);
- } else if (layoutBox.hasFloatClear())
- computeVerticalPositionForFloatClear(floatingContext, layoutBox);
- else if (layoutBox.establishesBlockFormattingContext())
+ } else if (layoutBox.establishesBlockFormattingContext())
computePositionToAvoidFloats(floatingContext, layoutBox);
// Now that we computed the root's height, we can go back and layout the out-of-flow descedants (if any).
@@ -176,12 +171,12 @@
LOG_WITH_STREAM(FormattingContextLayout, stream << "End: move in-flow positioned children -> parent: " << &container);
}
-void BlockFormattingContext::computeStaticPosition(const Box& layoutBox) const
+void BlockFormattingContext::computeStaticPosition(const FloatingContext& floatingContext, const Box& layoutBox) const
{
auto& layoutState = this->layoutState();
layoutState.displayBoxForLayoutBox(layoutBox).setTopLeft(Geometry::staticPosition(layoutState, layoutBox));
if (layoutBox.hasFloatClear())
- computeEstimatedVerticalPositionForFloatClear(layoutBox);
+ computeEstimatedVerticalPositionForFloatClear(floatingContext, layoutBox);
else if (layoutBox.establishesFormattingContext())
computeEstimatedVerticalPositionForFormattingRoot(layoutBox);
}
@@ -197,7 +192,7 @@
auto collapsedValues = UsedVerticalMargin::CollapsedValues { estimatedMarginBefore.collapsedValue, { }, estimatedMarginBefore.isCollapsedThrough };
auto verticalMargin = UsedVerticalMargin { nonCollapsedValues, collapsedValues };
displayBox.setVerticalMargin(verticalMargin);
- displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin));
+ displayBox.setTop(verticalPositionWithMargin(layoutBox, verticalMargin));
#if !ASSERT_DISABLED
displayBox.setHasEstimatedMarginBefore();
#endif
@@ -243,8 +238,25 @@
}
}
-void BlockFormattingContext::computeEstimatedVerticalPositionForFloatClear(const Box&) const
+void BlockFormattingContext::computeEstimatedVerticalPositionForFloatClear(const FloatingContext& floatingContext, const Box& layoutBox) const
{
+ ASSERT(layoutBox.hasFloatClear());
+ if (floatingContext.floatingState().isEmpty())
+ return;
+ // The static position with clear requires margin esitmation to see if clearance is needed.
+ computeEstimatedVerticalPosition(layoutBox);
+ computeEstimatedVerticalPositionForAncestors(layoutBox);
+ auto verticalPositionAndClearance = floatingContext.verticalPositionWithClearance(layoutBox);
+ if (!verticalPositionAndClearance.position) {
+ ASSERT(!verticalPositionAndClearance.clearance);
+ return;
+ }
+
+ auto& displayBox = layoutState().displayBoxForLayoutBox(layoutBox);
+ ASSERT(*verticalPositionAndClearance.position >= displayBox.top());
+ displayBox.setTop(*verticalPositionAndClearance.position);
+ if (verticalPositionAndClearance.clearance)
+ displayBox.setHasClearance();
}
#ifndef NDEBUG
@@ -292,54 +304,6 @@
layoutState.displayBoxForLayoutBox(layoutBox).setTopLeft(*adjustedPosition);
}
-void BlockFormattingContext::computeVerticalPositionForFloatClear(const FloatingContext& floatingContext, const Box& layoutBox) const
-{
- ASSERT(layoutBox.hasFloatClear());
- if (floatingContext.floatingState().isEmpty())
- return;
-
- // For formatting roots, we already precomputed final position.
- if (!layoutBox.establishesFormattingContext())
- computeEstimatedVerticalPositionForAncestors(layoutBox);
- ASSERT(hasPrecomputedMarginBefore(layoutBox));
-
- // 1. Compute and adjust the vertical position with clearance.
- // 2. Reset margin collapsing with previous in-flow sibling if clerance is present.
- auto verticalPositionAndClearance = floatingContext.verticalPositionWithClearance(layoutBox);
- if (!verticalPositionAndClearance.position)
- return;
-
- // Adjust vertical position first.
- auto& layoutState = this->layoutState();
- auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
- displayBox.setTop(*verticalPositionAndClearance.position);
- if (!verticalPositionAndClearance.clearance)
- return;
-
- displayBox.setHasClearance();
- // Adjust margin collapsing with clearance.
- auto* previousInFlowSibling = layoutBox.previousInFlowSibling();
- if (!previousInFlowSibling)
- return;
-
- // Clearance inhibits margin collapsing. Let's reset the relevant adjoining margins.
- // Does this box with clearance actually collapse its margin before with the previous inflow box's margin after?
- auto verticalMargin = displayBox.verticalMargin();
- if (!verticalMargin.hasCollapsedValues() || !verticalMargin.collapsedValues().before)
- return;
-
- // Reset previous after and current before margin values to non-collapsing.
- auto& previousInFlowDisplayBox = layoutState.displayBoxForLayoutBox(*previousInFlowSibling);
- auto previousVerticalMargin = previousInFlowDisplayBox.verticalMargin();
- ASSERT(previousVerticalMargin.hasCollapsedValues() && previousVerticalMargin.collapsedValues().after);
-
- previousVerticalMargin.setCollapsedValues({ previousVerticalMargin.collapsedValues().before, { } });
- verticalMargin.setCollapsedValues({ { }, verticalMargin.collapsedValues().after });
-
- previousInFlowDisplayBox.setVerticalMargin(previousVerticalMargin);
- displayBox.setVerticalMargin(verticalMargin);
-}
-
void BlockFormattingContext::computeWidthAndMargin(const Box& layoutBox) const
{
auto& layoutState = this->layoutState();
@@ -428,7 +392,7 @@
ASSERT(!hasEstimatedMarginBefore(layoutBox) || estimatedMarginBefore(layoutBox).usedValue() == verticalMargin.before());
removeEstimatedMarginBefore(layoutBox);
- displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin));
+ displayBox.setTop(verticalPositionWithMargin(layoutBox, verticalMargin));
displayBox.setContentBoxHeight(heightAndMargin.height);
displayBox.setVerticalMargin(verticalMargin);
@@ -501,14 +465,19 @@
return instrinsicWidthConstraints;
}
-LayoutUnit BlockFormattingContext::adjustedVerticalPositionAfterMarginCollapsing(const Box& layoutBox, const UsedVerticalMargin& verticalMargin) const
+LayoutUnit BlockFormattingContext::verticalPositionWithMargin(const Box& layoutBox, const UsedVerticalMargin& verticalMargin) const
{
ASSERT(!layoutBox.isOutOfFlowPositioned());
- // Now that we've computed the final margin before, let's shift the box's vertical position.
- // 1. Check if the margin before collapses with the previous box's margin after. if not -> return previous box's bottom including margin after + marginBefore
- // 2. Check if the previous box's margins collapse through. If not -> return previous box' bottom excluding margin after + marginBefore (they are supposed to be equal)
- // 3. Go to previous box and start from step #1 until we hit the parent box.
+ // Now that we've computed the final margin before, let's shift the box's vertical position if needed.
+ // 1. Check if the box has clearance. If so, we've already precomputed/finalized the top value and vertical margin does not impact it anymore.
+ // 2. Check if the margin before collapses with the previous box's margin after. if not -> return previous box's bottom including margin after + marginBefore
+ // 3. Check if the previous box's margins collapse through. If not -> return previous box' bottom excluding margin after + marginBefore (they are supposed to be equal)
+ // 4. Go to previous box and start from step #1 until we hit the parent box.
auto& layoutState = this->layoutState();
+ auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
+ if (displayBox.hasClearance())
+ return displayBox.top();
+
auto* currentLayoutBox = &layoutBox;
while (currentLayoutBox) {
if (!currentLayoutBox->previousInFlowSibling())
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h (240474 => 240475)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h 2019-01-25 16:46:47 UTC (rev 240474)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h 2019-01-25 16:50:18 UTC (rev 240475)
@@ -58,18 +58,17 @@
void computeWidthAndMargin(const Box&) const;
void computeHeightAndMargin(const Box&) const;
- void computeStaticPosition(const Box&) const;
+ void computeStaticPosition(const FloatingContext&, const Box&) const;
void computeFloatingPosition(const FloatingContext&, const Box&) const;
void computePositionToAvoidFloats(const FloatingContext&, const Box&) const;
- void computeVerticalPositionForFloatClear(const FloatingContext&, const Box&) const;
void computeEstimatedVerticalPosition(const Box&) const;
void computeEstimatedVerticalPositionForAncestors(const Box&) const;
void computeEstimatedVerticalPositionForFormattingRoot(const Box&) const;
- void computeEstimatedVerticalPositionForFloatClear(const Box&) const;
+ void computeEstimatedVerticalPositionForFloatClear(const FloatingContext&, const Box&) const;
InstrinsicWidthConstraints instrinsicWidthConstraints() const override;
- LayoutUnit adjustedVerticalPositionAfterMarginCollapsing(const Box&, const UsedVerticalMargin&) const;
+ LayoutUnit verticalPositionWithMargin(const Box&, const UsedVerticalMargin&) const;
// This class implements positioning and sizing for boxes participating in a block formatting context.
class Geometry : public FormattingContext::Geometry {
Modified: trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp (240474 => 240475)
--- trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp 2019-01-25 16:46:47 UTC (rev 240474)
+++ trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp 2019-01-25 16:50:18 UTC (rev 240475)
@@ -582,11 +582,9 @@
return { };
ASSERT(layoutBox.isBlockLevelBox());
- // Can't estimate vertical margins for out of flow boxes (and we shouldn't need to do it for float boxes).
- ASSERT(layoutBox.isInFlow());
+ // Don't estimate vertical margins for out of flow boxes.
+ ASSERT(layoutBox.isInFlow() || layoutBox.isFloatingPositioned());
ASSERT(!layoutBox.replaced());
- // Can't cross block formatting context boundary.
- ASSERT(!layoutBox.establishesBlockFormattingContext());
auto computedVerticalMargin = Geometry::computedVerticalMargin(layoutState, layoutBox);
auto nonCollapsedMargin = UsedVerticalMargin::NonCollapsedValues { computedVerticalMargin.before.valueOr(0), computedVerticalMargin.after.valueOr(0) };
Modified: trunk/Source/WebCore/layout/displaytree/DisplayBox.h (240474 => 240475)
--- trunk/Source/WebCore/layout/displaytree/DisplayBox.h 2019-01-25 16:46:47 UTC (rev 240474)
+++ trunk/Source/WebCore/layout/displaytree/DisplayBox.h 2019-01-25 16:50:18 UTC (rev 240475)
@@ -242,7 +242,7 @@
Layout::UsedHorizontalMargin m_horizontalMargin;
Layout::UsedVerticalMargin m_verticalMargin;
Layout::ComputedHorizontalMargin m_horizontalComputedMargin;
- bool m_hasClearance;
+ bool m_hasClearance { false };
Layout::Edges m_border;
Optional<Layout::Edges> m_padding;
Modified: trunk/Tools/ChangeLog (240474 => 240475)
--- trunk/Tools/ChangeLog 2019-01-25 16:46:47 UTC (rev 240474)
+++ trunk/Tools/ChangeLog 2019-01-25 16:50:18 UTC (rev 240475)
@@ -1,3 +1,12 @@
+2019-01-25 Zalan Bujtas <za...@apple.com>
+
+ [LFC][BFC][MarginCollapsing] Add "clear" to static position computation.
+ https://bugs.webkit.org/show_bug.cgi?id=193824
+
+ Reviewed by Antti Koivisto.
+
+ * LayoutReloaded/misc/LFC-passing-tests.txt:
+
2019-01-24 Ryan Haddad <ryanhad...@apple.com>
Update macOS JSC bot configurations
Modified: trunk/Tools/LayoutReloaded/misc/LFC-passing-tests.txt (240474 => 240475)
--- trunk/Tools/LayoutReloaded/misc/LFC-passing-tests.txt 2019-01-25 16:46:47 UTC (rev 240474)
+++ trunk/Tools/LayoutReloaded/misc/LFC-passing-tests.txt 2019-01-25 16:50:18 UTC (rev 240475)
@@ -133,6 +133,7 @@
fast/block/margin-collapse/035.html
fast/block/margin-collapse/039.html
fast/block/margin-collapse/040.html
+fast/block/margin-collapse/044.html
fast/block/positioning/003.html
fast/block/positioning/004.html
fast/block/positioning/005.html
