Diff
Modified: trunk/Source/WebCore/ChangeLog (257689 => 257690)
--- trunk/Source/WebCore/ChangeLog 2020-03-01 17:19:57 UTC (rev 257689)
+++ trunk/Source/WebCore/ChangeLog 2020-03-01 17:55:58 UTC (rev 257690)
@@ -1,3 +1,25 @@
+2020-03-01 Zalan Bujtas <[email protected]>
+
+ [LFC][MarginCollapsing] Do not re-compute PositiveAndNegativeVerticalMargin values
+ https://bugs.webkit.org/show_bug.cgi?id=208419
+ <rdar://problem/59923666>
+
+ Reviewed by Antti Koivisto.
+
+ MarginCollapse::collapsedVerticalValues() already computes the positive/negative before/after pairs.
+
+ * layout/MarginTypes.h:
+ * layout/blockformatting/BlockFormattingContext.cpp:
+ (WebCore::Layout::BlockFormattingContext::precomputeVerticalPositionForBoxAndAncestors):
+ (WebCore::Layout::BlockFormattingContext::computeHeightAndMargin):
+ * layout/blockformatting/BlockFormattingContext.h:
+ * layout/blockformatting/BlockFormattingContextQuirks.cpp:
+ (WebCore::Layout::BlockFormattingContext::Quirks::stretchedInFlowHeight):
+ * layout/blockformatting/BlockMarginCollapse.cpp:
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::positiveNegativeValues const):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedVerticalValues):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::resolvedPositiveNegativeMarginValues): Deleted.
+
2020-03-01 Simon Fraser <[email protected]>
Convert m_scrollingNodeToLayerMap to use WeakPtr<RenderLayer>
Modified: trunk/Source/WebCore/layout/MarginTypes.h (257689 => 257690)
--- trunk/Source/WebCore/layout/MarginTypes.h 2020-03-01 17:19:57 UTC (rev 257689)
+++ trunk/Source/WebCore/layout/MarginTypes.h 2020-03-01 17:55:58 UTC (rev 257690)
@@ -86,8 +86,8 @@
// This only matters in case of collapse through margins when they collapse into another sibling box.
// <div style="margin: 1px"></div><div style="margin: 10px"></div> <- the second div's before/after marings collapse through and the same time they collapse into
// the first div. When the parent computes its before margin, it should see the second div's collapsed through margin as the value to collapse width (adjoining margin value).
-// So while the first div's before margin is not 10px, the cached value is 10px so that when we compute the parent's margin we just need to check the fist
-// inflow child's cached margin value.
+// So while the first div's before margin is not 10px, the cached value is 10px so that when we compute the parent's margin we just need to check the first
+// inflow child's cached margin values.
struct PositiveAndNegativeVerticalMargin {
struct Values {
bool isNonZero() const { return positive.valueOr(0) || negative.valueOr(0); }
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp (257689 => 257690)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp 2020-03-01 17:19:57 UTC (rev 257689)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp 2020-03-01 17:55:58 UTC (rev 257690)
@@ -423,13 +423,16 @@
// 1. Compute collapsed margins.
// 2. Adjust vertical position using the collapsed values
// 3. Adjust previous in-flow sibling margin after using this margin.
- auto collapsedMargin = marginCollapse().collapsedVerticalValues(layoutBox, contentHeightAndMargin.nonCollapsedMargin);
- auto verticalMargin = UsedVerticalMargin { contentHeightAndMargin.nonCollapsedMargin, collapsedMargin };
- auto& displayBox = formattingState().displayBox(layoutBox);
+ auto marginCollapse = this->marginCollapse();
+ auto collapsedAndPositiveNegativeValues = marginCollapse.collapsedVerticalValues(layoutBox, contentHeightAndMargin.nonCollapsedMargin);
+ // Cache the computed positive and negative margin value pair.
+ formattingState().setPositiveAndNegativeVerticalMargin(layoutBox, collapsedAndPositiveNegativeValues.positiveAndNegativeVerticalValues);
+ auto verticalMargin = UsedVerticalMargin { contentHeightAndMargin.nonCollapsedMargin, collapsedAndPositiveNegativeValues.collapsedValues };
// Out of flow boxes don't need vertical adjustment after margin collapsing.
if (layoutBox.isOutOfFlowPositioned()) {
ASSERT(!hasPrecomputedMarginBefore(layoutBox));
+ auto& displayBox = formattingState().displayBox(layoutBox);
displayBox.setContentBoxHeight(contentHeightAndMargin.contentHeight);
displayBox.setVerticalMargin(verticalMargin);
return;
@@ -437,12 +440,10 @@
ASSERT(!hasPrecomputedMarginBefore(layoutBox) || precomputedMarginBefore(layoutBox).usedValue() == verticalMargin.before());
removePrecomputedMarginBefore(layoutBox);
+ auto& displayBox = formattingState().displayBox(layoutBox);
displayBox.setTop(verticalPositionWithMargin(layoutBox, verticalMargin, verticalConstraints));
displayBox.setContentBoxHeight(contentHeightAndMargin.contentHeight);
displayBox.setVerticalMargin(verticalMargin);
-
- auto marginCollapse = this->marginCollapse();
- formattingState().setPositiveAndNegativeVerticalMargin(layoutBox, marginCollapse.resolvedPositiveNegativeMarginValues(layoutBox, contentHeightAndMargin.nonCollapsedMargin));
// Adjust the previous sibling's margin bottom now that this box's vertical margin is computed.
MarginCollapse::updateMarginAfterForPreviousSibling(*this, marginCollapse, layoutBox);
}
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h (257689 => 257690)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h 2020-03-01 17:19:57 UTC (rev 257689)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h 2020-03-01 17:55:58 UTC (rev 257690)
@@ -101,12 +101,15 @@
// This class implements margin collapsing for block formatting context.
class MarginCollapse {
public:
- UsedVerticalMargin::CollapsedValues collapsedVerticalValues(const Box&, UsedVerticalMargin::NonCollapsedValues);
+ struct CollapsedAndPositiveNegativeValues {
+ UsedVerticalMargin::CollapsedValues collapsedValues;
+ PositiveAndNegativeVerticalMargin positiveAndNegativeVerticalValues;
+ };
+ CollapsedAndPositiveNegativeValues collapsedVerticalValues(const Box&, UsedVerticalMargin::NonCollapsedValues);
PrecomputedMarginBefore precomputedMarginBefore(const Box&, UsedVerticalMargin::NonCollapsedValues);
LayoutUnit marginBeforeIgnoringCollapsingThrough(const Box&, UsedVerticalMargin::NonCollapsedValues);
static void updateMarginAfterForPreviousSibling(BlockFormattingContext&, const MarginCollapse&, const Box&);
- PositiveAndNegativeVerticalMargin resolvedPositiveNegativeMarginValues(const Box&, const UsedVerticalMargin::NonCollapsedValues&);
bool marginBeforeCollapsesWithParentMarginBefore(const Box&) const;
bool marginBeforeCollapsesWithFirstInFlowChildMarginBefore(const Box&) const;
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp (257689 => 257690)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp 2020-03-01 17:19:57 UTC (rev 257689)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp 2020-03-01 17:55:58 UTC (rev 257690)
@@ -88,7 +88,7 @@
bodyBoxContentHeight -= bodyBoxGeometry.verticalBorder() + bodyBoxGeometry.verticalPadding().valueOr(0);
// Body box never collapses its vertical margins with the document box but it might collapse its margin with its descendants.
auto nonCollapsedMargin = contentHeightAndMargin.nonCollapsedMargin;
- auto collapsedMargin = formattingContext.marginCollapse().collapsedVerticalValues(layoutBox, nonCollapsedMargin);
+ auto collapsedMargin = formattingContext.marginCollapse().collapsedVerticalValues(layoutBox, nonCollapsedMargin).collapsedValues;
auto usedVerticalMargin = collapsedMargin.before.valueOr(nonCollapsedMargin.before);
usedVerticalMargin += collapsedMargin.isCollapsedThrough ? nonCollapsedMargin.after : collapsedMargin.after.valueOr(nonCollapsedMargin.after);
bodyBoxContentHeight -= usedVerticalMargin;
Modified: trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp (257689 => 257690)
--- trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp 2020-03-01 17:19:57 UTC (rev 257689)
+++ trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp 2020-03-01 17:55:58 UTC (rev 257690)
@@ -508,8 +508,8 @@
PositiveAndNegativeVerticalMargin::Values BlockFormattingContext::MarginCollapse::positiveNegativeValues(const Box& layoutBox, MarginType marginType) const
{
auto& blockFormattingState = downcast<BlockFormattingState>(layoutState().formattingStateForBox(layoutBox));
- // By the time we get here in BFC layout to gather positive and negative margin values for a either a previous sibling or a child box,
- // we mush have computed and cached those valued.
+ // By the time we get here in BFC layout to gather positive and negative margin values for either a previous sibling or a child box,
+ // we mush have computed and cached those values.
ASSERT(blockFormattingState.hasPositiveAndNegativeVerticalMargin(layoutBox));
auto positiveAndNegativeVerticalMargin = blockFormattingState.positiveAndNegativeVerticalMargin(layoutBox);
return marginType == MarginType::Before ? positiveAndNegativeVerticalMargin.before : positiveAndNegativeVerticalMargin.after;
@@ -570,43 +570,29 @@
return marginValue(positiveNegativeMarginBefore(layoutBox, nonCollapsedValues)).valueOr(nonCollapsedValues.before);
}
-PositiveAndNegativeVerticalMargin BlockFormattingContext::MarginCollapse::resolvedPositiveNegativeMarginValues(const Box& layoutBox, const UsedVerticalMargin::NonCollapsedValues& nonCollapsedValues)
+BlockFormattingContext::MarginCollapse::CollapsedAndPositiveNegativeValues BlockFormattingContext::MarginCollapse::collapsedVerticalValues(const Box& layoutBox, UsedVerticalMargin::NonCollapsedValues nonCollapsedValues)
{
ASSERT(layoutBox.isBlockLevelBox());
- auto positiveNegativeMarginBefore = this->positiveNegativeMarginBefore(layoutBox, nonCollapsedValues);
- auto positiveNegativeMarginAfter = this->positiveNegativeMarginAfter(layoutBox, nonCollapsedValues);
-
- if (marginsCollapseThrough(layoutBox)) {
- positiveNegativeMarginBefore = computedPositiveAndNegativeMargin(positiveNegativeMarginBefore, positiveNegativeMarginAfter);
- positiveNegativeMarginAfter = positiveNegativeMarginBefore;
- }
- return { positiveNegativeMarginBefore, positiveNegativeMarginAfter };
-}
-
-UsedVerticalMargin::CollapsedValues BlockFormattingContext::MarginCollapse::collapsedVerticalValues(const Box& layoutBox, UsedVerticalMargin::NonCollapsedValues nonCollapsedValues)
-{
- ASSERT(layoutBox.isBlockLevelBox());
// 1. Get min/max margin top values from the first in-flow child if we are collapsing margin top with it.
// 2. Get min/max margin top values from the previous in-flow sibling, if we are collapsing margin top with it.
// 3. Get this layout box's computed margin top value.
// 4. Update the min/max value and compute the final margin.
- auto positiveNegativeMarginBefore = this->positiveNegativeMarginBefore(layoutBox, nonCollapsedValues);
- auto positiveNegativeMarginAfter = this->positiveNegativeMarginAfter(layoutBox, nonCollapsedValues);
+ auto positiveAndNegativeVerticalMargin = PositiveAndNegativeVerticalMargin { this->positiveNegativeMarginBefore(layoutBox, nonCollapsedValues), this->positiveNegativeMarginAfter(layoutBox, nonCollapsedValues) };
auto marginsCollapseThrough = this->marginsCollapseThrough(layoutBox);
if (marginsCollapseThrough) {
- positiveNegativeMarginBefore = computedPositiveAndNegativeMargin(positiveNegativeMarginBefore, positiveNegativeMarginAfter);
- positiveNegativeMarginAfter = positiveNegativeMarginBefore;
+ positiveAndNegativeVerticalMargin.before = computedPositiveAndNegativeMargin(positiveAndNegativeVerticalMargin.before, positiveAndNegativeVerticalMargin.after);
+ positiveAndNegativeVerticalMargin.after = positiveAndNegativeVerticalMargin.before;
}
- auto beforeMarginIsCollapsedValue = marginBeforeCollapsesWithFirstInFlowChildMarginBefore(layoutBox) || marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutBox);
- auto afterMarginIsCollapsedValue = marginAfterCollapsesWithLastInFlowChildMarginAfter(layoutBox);
+ auto hasCollapsedMarginBefore = marginBeforeCollapsesWithFirstInFlowChildMarginBefore(layoutBox) || marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutBox);
+ auto hasCollapsedMarginAfter = marginAfterCollapsesWithLastInFlowChildMarginAfter(layoutBox);
- if ((beforeMarginIsCollapsedValue && afterMarginIsCollapsedValue) || marginsCollapseThrough)
- return { marginValue(positiveNegativeMarginBefore), marginValue(positiveNegativeMarginAfter), marginsCollapseThrough };
- if (beforeMarginIsCollapsedValue)
- return { marginValue(positiveNegativeMarginBefore), { }, false };
- return { { }, marginValue(positiveNegativeMarginAfter), false };
+ if ((hasCollapsedMarginBefore && hasCollapsedMarginAfter) || marginsCollapseThrough)
+ return { { marginValue(positiveAndNegativeVerticalMargin.before), marginValue(positiveAndNegativeVerticalMargin.after), marginsCollapseThrough }, positiveAndNegativeVerticalMargin };
+ if (hasCollapsedMarginBefore)
+ return { { marginValue(positiveAndNegativeVerticalMargin.before), { }, false }, positiveAndNegativeVerticalMargin };
+ return { { { }, marginValue(positiveAndNegativeVerticalMargin.after), false }, positiveAndNegativeVerticalMargin };
}
}
