Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[LFC] ComputedHorizontalMargin should have optional members
https://bugs.webkit.org/show_bug.cgi?id=193131

Reviewed by Antti Koivisto.

Split HorizontalMargin into UsedHorizontalMargin and ComputedHorizontalMargin. ComputedHorizontalMargin's members are optional.
(see computed vs used values)

* layout/FormattingContext.h:
* layout/FormattingContextGeometry.cpp:
(WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry):
(WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry):
(WebCore::Layout::FormattingContext::Geometry::floatingNonReplacedWidthAndMargin):
(WebCore::Layout::FormattingContext::Geometry::floatingReplacedWidthAndMargin):
(WebCore::Layout::FormattingContext::Geometry::inlineReplacedWidthAndMargin):
(WebCore::Layout::FormattingContext::Geometry::computedHorizontalMargin):
(WebCore::Layout::FormattingContext::Geometry::computedNonCollapsedHorizontalMarginValue): Deleted.
* layout/LayoutUnits.h:
* layout/MarginTypes.h:
* layout/Verification.cpp:
(WebCore::Layout::outputMismatchingBlockBoxInformationIfNeeded):
* layout/blockformatting/BlockFormattingContextGeometry.cpp:
(WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin):
* layout/displaytree/DisplayBox.h:
(WebCore::Display::Box::setHorizontalMargin):
(WebCore::Display::Box::setHorizontalComputedMargin):
(WebCore::Display::Box::computedMarginStart const):
(WebCore::Display::Box::computedMarginEnd const):
* layout/floats/FloatAvoider.h:
(WebCore::Layout::FloatAvoider::marginStart const):
(WebCore::Layout::FloatAvoider::marginEnd const):
* layout/inlineformatting/InlineFormattingContext.cpp:
(WebCore::Layout::InlineFormattingContext::collectInlineContentForSubtree const):
* layout/inlineformatting/InlineFormattingContextGeometry.cpp:
(WebCore::Layout::InlineFormattingContext::Geometry::inlineBlockWidthAndMargin):

Canonical link: https://commits.webkit.org/207637@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239609 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
alanbaradlay committed Jan 4, 2019
1 parent 8f5e466 commit 34f49b8
Show file tree
Hide file tree
Showing 11 changed files with 159 additions and 140 deletions.
38 changes: 38 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,41 @@
2019-01-04 Zalan Bujtas <zalan@apple.com>

[LFC] ComputedHorizontalMargin should have optional members
https://bugs.webkit.org/show_bug.cgi?id=193131

Reviewed by Antti Koivisto.

Split HorizontalMargin into UsedHorizontalMargin and ComputedHorizontalMargin. ComputedHorizontalMargin's members are optional.
(see computed vs used values)

* layout/FormattingContext.h:
* layout/FormattingContextGeometry.cpp:
(WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry):
(WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry):
(WebCore::Layout::FormattingContext::Geometry::floatingNonReplacedWidthAndMargin):
(WebCore::Layout::FormattingContext::Geometry::floatingReplacedWidthAndMargin):
(WebCore::Layout::FormattingContext::Geometry::inlineReplacedWidthAndMargin):
(WebCore::Layout::FormattingContext::Geometry::computedHorizontalMargin):
(WebCore::Layout::FormattingContext::Geometry::computedNonCollapsedHorizontalMarginValue): Deleted.
* layout/LayoutUnits.h:
* layout/MarginTypes.h:
* layout/Verification.cpp:
(WebCore::Layout::outputMismatchingBlockBoxInformationIfNeeded):
* layout/blockformatting/BlockFormattingContextGeometry.cpp:
(WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin):
* layout/displaytree/DisplayBox.h:
(WebCore::Display::Box::setHorizontalMargin):
(WebCore::Display::Box::setHorizontalComputedMargin):
(WebCore::Display::Box::computedMarginStart const):
(WebCore::Display::Box::computedMarginEnd const):
* layout/floats/FloatAvoider.h:
(WebCore::Layout::FloatAvoider::marginStart const):
(WebCore::Layout::FloatAvoider::marginEnd const):
* layout/inlineformatting/InlineFormattingContext.cpp:
(WebCore::Layout::InlineFormattingContext::collectInlineContentForSubtree const):
* layout/inlineformatting/InlineFormattingContextGeometry.cpp:
(WebCore::Layout::InlineFormattingContext::Geometry::inlineBlockWidthAndMargin):

2019-01-04 Zalan Bujtas <zalan@apple.com>

[LFC][BFC] Use computedValue and usedValue consistently
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/layout/FormattingContext.h
Expand Up @@ -96,7 +96,7 @@ class FormattingContext {
static Edges computedBorder(const LayoutState&, const Box&);
static Optional<Edges> computedPadding(const LayoutState&, const Box&);

static HorizontalMargin computedNonCollapsedHorizontalMarginValue(const LayoutState&, const Box&);
static ComputedHorizontalMargin computedHorizontalMargin(const LayoutState&, const Box&);
static VerticalMargin::ComputedValues computedNonCollapsedVerticalMarginValue(const LayoutState&, const Box&);

static Optional<LayoutUnit> computedValueIfNotAuto(const Length& geometryProperty, LayoutUnit containingBlockWidth);
Expand Down
148 changes: 62 additions & 86 deletions Source/WebCore/layout/FormattingContextGeometry.cpp

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions Source/WebCore/layout/LayoutUnits.h
Expand Up @@ -103,8 +103,8 @@ struct Edges {

struct WidthAndMargin {
LayoutUnit width;
HorizontalMargin usedMargin;
HorizontalMargin computedMargin;
UsedHorizontalMargin usedMargin;
ComputedHorizontalMargin computedMargin;
};

struct HeightAndMargin {
Expand Down
7 changes: 6 additions & 1 deletion Source/WebCore/layout/MarginTypes.h
Expand Up @@ -56,7 +56,12 @@ struct VerticalMargin {
Optional<CollapsedValues> m_collapsed;
};

struct HorizontalMargin {
struct ComputedHorizontalMargin {
Optional<LayoutUnit> start;
Optional<LayoutUnit> end;
};

struct UsedHorizontalMargin {
LayoutUnit start;
LayoutUnit end;
};
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/layout/Verification.cpp
Expand Up @@ -247,8 +247,8 @@ static bool outputMismatchingBlockBoxInformationIfNeeded(TextStream& stream, con

return Display::Box::Rect {
borderBox.top() - displayBox.nonCollapsedMarginBefore(),
borderBox.left() - displayBox.computedMarginStart(),
displayBox.computedMarginStart() + borderBox.width() + displayBox.computedMarginEnd(),
borderBox.left() - displayBox.computedMarginStart().valueOr(0),
displayBox.computedMarginStart().valueOr(0) + borderBox.width() + displayBox.computedMarginEnd().valueOr(0),
displayBox.nonCollapsedMarginBefore() + borderBox.height() + displayBox.nonCollapsedMarginAfter()
};
};
Expand Down
Expand Up @@ -141,58 +141,58 @@ WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin
auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);

auto width = computedValueIfNotAuto(usedWidth ? Length { usedWidth.value(), Fixed } : style.logicalWidth(), containingBlockWidth);
auto computedMarginStart = computedValueIfNotAuto(style.marginStart(), containingBlockWidth);
auto computedMarginEnd = computedValueIfNotAuto(style.marginEnd(), containingBlockWidth);
auto usedMarginStart = computedMarginStart;
auto usedMarginEnd = computedMarginEnd;
auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, layoutBox);
UsedHorizontalMargin usedHorizontalMargin;
auto borderLeft = displayBox.borderLeft();
auto borderRight = displayBox.borderRight();
auto paddingLeft = displayBox.paddingLeft().valueOr(0);
auto paddingRight = displayBox.paddingRight().valueOr(0);

// #1
if (width) {
auto horizontalSpaceForMargin = containingBlockWidth - (usedMarginStart.valueOr(0) + borderLeft + paddingLeft + *width + paddingRight + borderRight + usedMarginEnd.valueOr(0));
if (horizontalSpaceForMargin < 0) {
usedMarginStart = usedMarginStart.valueOr(0);
usedMarginEnd = usedMarginEnd.valueOr(0);
}
auto horizontalSpaceForMargin = containingBlockWidth - (computedHorizontalMargin.start.valueOr(0) + borderLeft + paddingLeft + *width + paddingRight + borderRight + computedHorizontalMargin.end.valueOr(0));
if (horizontalSpaceForMargin < 0)
usedHorizontalMargin = { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) };
}

// #2
if (width && usedMarginStart && usedMarginEnd) {
if (containingBlock->style().isLeftToRightDirection())
usedMarginEnd = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight);
else
usedMarginStart = containingBlockWidth - (borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd);
if (width && computedHorizontalMargin.start && computedHorizontalMargin.end) {
if (containingBlock->style().isLeftToRightDirection()) {
usedHorizontalMargin.start = *computedHorizontalMargin.start;
usedHorizontalMargin.end = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight);
} else {
usedHorizontalMargin.end = *computedHorizontalMargin.end;
usedHorizontalMargin.start = containingBlockWidth - (borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end);
}
}

// #3
if (!usedMarginStart && width && usedMarginEnd)
usedMarginStart = containingBlockWidth - (borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd);
else if (usedMarginStart && !width && usedMarginEnd)
width = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + paddingRight + borderRight + *usedMarginEnd);
else if (usedMarginStart && width && !usedMarginEnd)
usedMarginEnd = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight);
if (!computedHorizontalMargin.start && width && computedHorizontalMargin.end) {
usedHorizontalMargin.end = *computedHorizontalMargin.end;
usedHorizontalMargin.start = containingBlockWidth - (borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end);
} else if (computedHorizontalMargin.start && !width && computedHorizontalMargin.end) {
usedHorizontalMargin = { *computedHorizontalMargin.start, *computedHorizontalMargin.end };
width = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + paddingRight + borderRight + usedHorizontalMargin.end);
} else if (computedHorizontalMargin.start && width && !computedHorizontalMargin.end) {
usedHorizontalMargin.start = *computedHorizontalMargin.start;
usedHorizontalMargin.end = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight);
}

// #4
if (!width) {
usedMarginStart = usedMarginStart.valueOr(0);
usedMarginEnd = usedMarginEnd.valueOr(0);
width = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + paddingRight + borderRight + *usedMarginEnd);
usedHorizontalMargin = { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) };
width = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + paddingRight + borderRight + usedHorizontalMargin.end);
}

// #5
if (!usedMarginStart && !usedMarginEnd) {
if (!computedHorizontalMargin.start && !computedHorizontalMargin.end) {
auto horizontalSpaceForMargin = containingBlockWidth - (borderLeft + paddingLeft + *width + paddingRight + borderRight);
usedMarginStart = usedMarginEnd = horizontalSpaceForMargin / 2;
usedHorizontalMargin = { horizontalSpaceForMargin / 2, horizontalSpaceForMargin / 2 };
}

ASSERT(width);
ASSERT(usedMarginStart);
ASSERT(usedMarginEnd);

return WidthAndMargin { *width, { *usedMarginStart, *usedMarginEnd }, { computedMarginStart.valueOr(0), computedMarginEnd.valueOr(0) } };
return WidthAndMargin { *width, usedHorizontalMargin, computedHorizontalMargin };
};

auto widthAndMargin = compute();
Expand Down
20 changes: 10 additions & 10 deletions Source/WebCore/layout/displaytree/DisplayBox.h
Expand Up @@ -146,8 +146,8 @@ class Box {

LayoutUnit nonCollapsedMarginBefore() const;
LayoutUnit nonCollapsedMarginAfter() const;
LayoutUnit computedMarginStart() const;
LayoutUnit computedMarginEnd() const;
Optional<LayoutUnit> computedMarginStart() const;
Optional<LayoutUnit> computedMarginEnd() const;

Optional<LayoutUnit> estimatedMarginBefore() const { return m_estimatedMarginBefore; }

Expand Down Expand Up @@ -191,9 +191,9 @@ class Box {
void setContentBoxHeight(LayoutUnit);
void setContentBoxWidth(LayoutUnit);

void setHorizontalMargin(Layout::HorizontalMargin);
void setHorizontalMargin(Layout::UsedHorizontalMargin);
void setVerticalMargin(Layout::VerticalMargin);
void setHorizontalComputedMargin(Layout::HorizontalMargin);
void setHorizontalComputedMargin(Layout::ComputedHorizontalMargin);
void setEstimatedMarginBefore(LayoutUnit marginBefore) { m_estimatedMarginBefore = marginBefore; }

void setBorder(Layout::Edges);
Expand Down Expand Up @@ -224,9 +224,9 @@ class Box {
LayoutUnit m_contentWidth;
LayoutUnit m_contentHeight;

Layout::HorizontalMargin m_horizontalMargin;
Layout::UsedHorizontalMargin m_horizontalMargin;
Layout::VerticalMargin m_verticalMargin;
Layout::HorizontalMargin m_horizontalComputedMargin;
Layout::ComputedHorizontalMargin m_horizontalComputedMargin;
Optional<LayoutUnit> m_estimatedMarginBefore;

Layout::Edges m_border;
Expand Down Expand Up @@ -507,7 +507,7 @@ inline LayoutUnit Box::contentBoxWidth() const
return m_contentWidth;
}

inline void Box::setHorizontalMargin(Layout::HorizontalMargin margin)
inline void Box::setHorizontalMargin(Layout::UsedHorizontalMargin margin)
{
#if !ASSERT_DISABLED
setHasValidHorizontalMargin();
Expand All @@ -525,7 +525,7 @@ inline void Box::setVerticalMargin(Layout::VerticalMargin margin)
m_verticalMargin = margin;
}

inline void Box::setHorizontalComputedMargin(Layout::HorizontalMargin margin)
inline void Box::setHorizontalComputedMargin(Layout::ComputedHorizontalMargin margin)
{
#if !ASSERT_DISABLED
setHasValidHorizontalComputedMargin();
Expand Down Expand Up @@ -591,13 +591,13 @@ inline LayoutUnit Box::nonCollapsedMarginAfter() const
return m_verticalMargin.nonCollapsedValues().after;
}

inline LayoutUnit Box::computedMarginStart() const
inline Optional<LayoutUnit> Box::computedMarginStart() const
{
ASSERT(m_hasValidHorizontalComputedMargin);
return m_horizontalComputedMargin.start;
}

inline LayoutUnit Box::computedMarginEnd() const
inline Optional<LayoutUnit> Box::computedMarginEnd() const
{
ASSERT(m_hasValidHorizontalComputedMargin);
return m_horizontalComputedMargin.end;
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/layout/floats/FloatAvoider.h
Expand Up @@ -73,8 +73,8 @@ class FloatAvoider {
LayoutUnit marginBefore() const { return displayBox().marginBefore(); }
LayoutUnit marginAfter() const { return displayBox().marginAfter(); }
// Do not use the used values here because they computed as if this box was not a float avoider.
LayoutUnit marginStart() const { return displayBox().computedMarginStart(); }
LayoutUnit marginEnd() const { return displayBox().computedMarginEnd(); }
LayoutUnit marginStart() const { return displayBox().computedMarginStart().valueOr(0); }
LayoutUnit marginEnd() const { return displayBox().computedMarginEnd().valueOr(0); }

LayoutUnit marginBoxWidth() const { return marginStart() + displayBox().width() + marginEnd(); }

Expand Down
Expand Up @@ -442,10 +442,10 @@ void InlineFormattingContext::collectInlineContentForSubtree(const Box& root, In
createAndAppendInlineItem();
auto& inlineRun = *inlineFormattingState.inlineContent().last();

auto horizontalMargin = Geometry::computedNonCollapsedHorizontalMarginValue(layoutState(), root);
auto horizontalMargin = Geometry::computedHorizontalMargin(layoutState(), root);
inlineRun.addDetachingRule({ InlineItem::DetachingRule::BreakAtStart, InlineItem::DetachingRule::BreakAtEnd });
inlineRun.addNonBreakableStart(horizontalMargin.start);
inlineRun.addNonBreakableEnd(horizontalMargin.end);
inlineRun.addNonBreakableStart(horizontalMargin.start.valueOr(0));
inlineRun.addNonBreakableEnd(horizontalMargin.end.valueOr(0));
// Skip formatting root subtree. They are not part of this inline formatting context.
return;
}
Expand All @@ -465,7 +465,7 @@ void InlineFormattingContext::collectInlineContentForSubtree(const Box& root, In
// FIXME: Revisit this when we figured out how inline boxes fit the display tree.
auto padding = Geometry::computedPadding(layoutState(), root);
auto border = Geometry::computedBorder(layoutState(), root);
auto horizontalMargin = Geometry::computedNonCollapsedHorizontalMarginValue(layoutState(), root);
auto horizontalMargin = Geometry::computedHorizontalMargin(layoutState(), root);
// Setup breaking boundaries for this subtree.
auto* lastDescendantInlineBox = inlineFormattingState.lastInlineItem();
// Empty container?
Expand Down Expand Up @@ -496,15 +496,15 @@ void InlineFormattingContext::collectInlineContentForSubtree(const Box& root, In

ASSERT(firstDescendantInlineBox);
firstDescendantInlineBox->addDetachingRule(InlineItem::DetachingRule::BreakAtStart);
auto startOffset = border.horizontal.left + horizontalMargin.start;
auto startOffset = border.horizontal.left + horizontalMargin.start.valueOr(0);
if (padding)
startOffset += padding->horizontal.left;
firstDescendantInlineBox->addNonBreakableStart(startOffset);
}

if (rootBreaksAtEnd()) {
lastDescendantInlineBox->addDetachingRule(InlineItem::DetachingRule::BreakAtEnd);
auto endOffset = border.horizontal.right + horizontalMargin.end;
auto endOffset = border.horizontal.right + horizontalMargin.end.valueOr(0);
if (padding)
endOffset += padding->horizontal.right;
lastDescendantInlineBox->addNonBreakableEnd(endOffset);
Expand Down
Expand Up @@ -60,9 +60,9 @@ WidthAndMargin InlineFormattingContext::Geometry::inlineBlockWidthAndMargin(Layo
width = shrinkToFitWidth(layoutState, formattingContextRoot);

// #2
auto margin = computedNonCollapsedHorizontalMarginValue(layoutState, formattingContextRoot);
auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, formattingContextRoot);

return WidthAndMargin { *width, margin, margin };
return WidthAndMargin { *width, { computedHorizontalMargin.start.valueOr(0_lu), computedHorizontalMargin.end.valueOr(0_lu) }, computedHorizontalMargin };
}

HeightAndMargin InlineFormattingContext::Geometry::inlineBlockHeightAndMargin(const LayoutState& layoutState, const Box& layoutBox)
Expand Down

0 comments on commit 34f49b8

Please sign in to comment.