Diff
Modified: trunk/Source/WebCore/ChangeLog (240252 => 240253)
--- trunk/Source/WebCore/ChangeLog 2019-01-22 16:59:15 UTC (rev 240252)
+++ trunk/Source/WebCore/ChangeLog 2019-01-22 17:25:59 UTC (rev 240253)
@@ -1,3 +1,23 @@
+2019-01-22 Zalan Bujtas <za...@apple.com>
+
+ [LFC][Floats] Decouple clearance computation and margin collapsing reset.
+ https://bugs.webkit.org/show_bug.cgi?id=193670
+
+ Reviewed by Antti Koivisto.
+
+ Move margin collapsing reset logic from FloatingContext to BlockFormattingContext. It's the BlockFormattingContext's job to do.
+ This is also in preparation for adding clear to static position.
+
+ * layout/FormattingContext.cpp:
+ (WebCore::Layout::FormattingContext::mapTopToAncestor):
+ (WebCore::Layout::FormattingContext::mapTopLeftToAncestor): Deleted.
+ * layout/FormattingContext.h:
+ * layout/blockformatting/BlockFormattingContext.cpp:
+ (WebCore::Layout::BlockFormattingContext::computeVerticalPositionForFloatClear const):
+ * layout/floats/FloatingContext.cpp:
+ (WebCore::Layout::FloatingContext::verticalPositionWithClearance const):
+ * layout/floats/FloatingContext.h:
+
2019-01-22 Frederic Wang <fw...@igalia.com>
Minor refactoring of the scrolling code
Modified: trunk/Source/WebCore/layout/FormattingContext.cpp (240252 => 240253)
--- trunk/Source/WebCore/layout/FormattingContext.cpp 2019-01-22 16:59:15 UTC (rev 240252)
+++ trunk/Source/WebCore/layout/FormattingContext.cpp 2019-01-22 17:25:59 UTC (rev 240253)
@@ -184,10 +184,14 @@
return mappedDisplayBox;
}
-Point FormattingContext::mapTopLeftToAncestor(const LayoutState& layoutState, const Box& layoutBox, const Container& ancestor)
+LayoutUnit FormattingContext::mapTopToAncestor(const LayoutState& layoutState, const Box& layoutBox, const Container& ancestor)
{
ASSERT(layoutBox.isDescendantOf(ancestor));
- return mapCoordinateToAncestor(layoutState, layoutState.displayBoxForLayoutBox(layoutBox).topLeft(), *layoutBox.containingBlock(), ancestor);
+ auto top = layoutState.displayBoxForLayoutBox(layoutBox).top();
+ auto* container = layoutBox.containingBlock();
+ for (; container && container != &ancestor; container = container->containingBlock())
+ top += layoutState.displayBoxForLayoutBox(*container).top();
+ return top;
}
Point FormattingContext::mapCoordinateToAncestor(const LayoutState& layoutState, Point position, const Container& containingBlock, const Container& ancestor)
Modified: trunk/Source/WebCore/layout/FormattingContext.h (240252 => 240253)
--- trunk/Source/WebCore/layout/FormattingContext.h 2019-01-22 16:59:15 UTC (rev 240252)
+++ trunk/Source/WebCore/layout/FormattingContext.h 2019-01-22 17:25:59 UTC (rev 240253)
@@ -59,7 +59,7 @@
virtual InstrinsicWidthConstraints instrinsicWidthConstraints() const = 0;
static Display::Box mapBoxToAncestor(const LayoutState&, const Box&, const Container& ancestor);
- static Point mapTopLeftToAncestor(const LayoutState&, const Box&, const Container& ancestor);
+ static LayoutUnit mapTopToAncestor(const LayoutState&, const Box&, const Container& ancestor);
static Point mapCoordinateToAncestor(const LayoutState&, Point, const Container& containingBlock, const Container& ancestor);
protected:
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp (240252 => 240253)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp 2019-01-22 16:59:15 UTC (rev 240252)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp 2019-01-22 17:25:59 UTC (rev 240253)
@@ -291,14 +291,46 @@
if (floatingContext.floatingState().isEmpty())
return;
- auto& layoutState = this->layoutState();
// For formatting roots, we already precomputed final position.
if (!layoutBox.establishesFormattingContext())
computeEstimatedMarginBeforeForAncestors(layoutBox);
ASSERT(hasPrecomputedMarginBefore(layoutBox));
- if (auto verticalPositionWithClearance = floatingContext.verticalPositionWithClearance(layoutBox))
- layoutState.displayBoxForLayoutBox(layoutBox).setTop(*verticalPositionWithClearance);
+ // 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
Modified: trunk/Source/WebCore/layout/floats/FloatingContext.cpp (240252 => 240253)
--- trunk/Source/WebCore/layout/floats/FloatingContext.cpp 2019-01-22 16:59:15 UTC (rev 240252)
+++ trunk/Source/WebCore/layout/floats/FloatingContext.cpp 2019-01-22 17:25:59 UTC (rev 240253)
@@ -191,7 +191,7 @@
return { floatAvoider.rectInContainingBlock().topLeft() };
}
-Optional<Position> FloatingContext::verticalPositionWithClearance(const Box& layoutBox) const
+FloatingContext::ClearancePosition FloatingContext::verticalPositionWithClearance(const Box& layoutBox) const
{
ASSERT(layoutBox.hasFloatClear());
ASSERT(layoutBox.isBlockLevelBox());
@@ -200,7 +200,7 @@
if (m_floatingState.isEmpty())
return { };
- auto bottom = [&](Optional<PositionInContextRoot> floatBottom) -> Optional<Position> {
+ auto bottom = [&](Optional<PositionInContextRoot> floatBottom) -> ClearancePosition {
// 'bottom' is in the formatting root's coordinate system.
if (!floatBottom)
return { };
@@ -210,37 +210,23 @@
//
// 1. The amount necessary to place the border edge of the block even with the bottom outer edge of the lowest float that is to be cleared.
// 2. The amount necessary to place the top border edge of the block at its hypothetical position.
-
auto& layoutState = this->layoutState();
- auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
- auto rootRelativeTop = FormattingContext::mapTopLeftToAncestor(layoutState, layoutBox, downcast<Container>(m_floatingState.root())).y;
+ auto rootRelativeTop = FormattingContext::mapTopToAncestor(layoutState, layoutBox, downcast<Container>(m_floatingState.root()));
auto clearance = *floatBottom - rootRelativeTop;
if (clearance <= 0)
return { };
- displayBox.setHasClearance();
- // Clearance inhibits margin collapsing. Let's reset the relevant adjoining margins.
+ // Clearance inhibits margin collapsing.
if (auto* previousInFlowSibling = layoutBox.previousInFlowSibling()) {
- auto& previousInFlowDisplayBox = layoutState.displayBoxForLayoutBox(*previousInFlowSibling);
// Does this box with clearance actually collapse its margin before with the previous inflow box's margin after?
- auto verticalMargin = displayBox.verticalMargin();
+ auto verticalMargin = layoutState.displayBoxForLayoutBox(layoutBox).verticalMargin();
if (verticalMargin.hasCollapsedValues() && verticalMargin.collapsedValues().before) {
- // Reset previous bottom after and current margin before to non-collapsing.
- auto previousVerticalMargin = previousInFlowDisplayBox.verticalMargin();
- ASSERT(previousVerticalMargin.hasCollapsedValues() && previousVerticalMargin.collapsedValues().after);
-
+ auto previousVerticalMargin = layoutState.displayBoxForLayoutBox(*previousInFlowSibling).verticalMargin();
auto collapsedMargin = *verticalMargin.collapsedValues().before;
- previousVerticalMargin.setCollapsedValues({ previousVerticalMargin.collapsedValues().before, { } });
- previousInFlowDisplayBox.setVerticalMargin(previousVerticalMargin);
-
- verticalMargin.setCollapsedValues({ { }, verticalMargin.collapsedValues().after });
- displayBox.setVerticalMargin(verticalMargin);
-
auto nonCollapsedMargin = previousVerticalMargin.after() + verticalMargin.before();
auto marginDifference = nonCollapsedMargin - collapsedMargin;
// Move the box to the position where it would be with non-collapsed margins.
rootRelativeTop += marginDifference;
-
// Having negative clearance is also normal. It just means that the box with the non-collapsed margins is now lower than it needs to be.
clearance -= marginDifference;
}
@@ -251,10 +237,10 @@
// The return vertical position is in the containing block's coordinate system. Convert it to the formatting root's coordinate system if needed.
if (layoutBox.containingBlock() == &m_floatingState.root())
- return Position { rootRelativeTop };
+ return { Position { rootRelativeTop }, clearance };
- auto containingBlockRootRelativeTop = FormattingContext::mapTopLeftToAncestor(layoutState, *layoutBox.containingBlock(), downcast<Container>(m_floatingState.root())).y;
- return Position { rootRelativeTop - containingBlockRootRelativeTop };
+ auto containingBlockRootRelativeTop = FormattingContext::mapTopToAncestor(layoutState, *layoutBox.containingBlock(), downcast<Container>(m_floatingState.root()));
+ return { Position { rootRelativeTop - containingBlockRootRelativeTop }, clearance };
};
auto clear = layoutBox.style().clear();
Modified: trunk/Source/WebCore/layout/floats/FloatingContext.h (240252 => 240253)
--- trunk/Source/WebCore/layout/floats/FloatingContext.h 2019-01-22 16:59:15 UTC (rev 240252)
+++ trunk/Source/WebCore/layout/floats/FloatingContext.h 2019-01-22 17:25:59 UTC (rev 240253)
@@ -52,8 +52,13 @@
Point positionForFloat(const Box&) const;
Optional<Point> positionForFloatAvoiding(const Box&) const;
- Optional<Position> verticalPositionWithClearance(const Box&) const;
+ struct ClearancePosition {
+ Optional<Position> position;
+ Optional<LayoutUnit> clearance;
+ };
+ ClearancePosition verticalPositionWithClearance(const Box&) const;
+
private:
LayoutState& layoutState() const { return m_floatingState.layoutState(); }
