Commit 0be7b52f authored by David Grogan's avatar David Grogan Committed by Commit Bot

[LayoutNG] Reland flex-item alignment

We were stretching items that had auto cross margins. This patch sets a
flag on FlexItem when it needs relayout due to stretching. It's not
elegant but is also not intrusive.

This reverts commit 2390edbd.

Bug: 845235, 1015475
Change-Id: Iebefb670f2d3ae4047565077776a6d7a1ff2c2fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1867434
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708455}
parent ebcc4c13
......@@ -89,6 +89,7 @@ FlexItem::FlexItem(LayoutBox* box,
main_axis_border_padding(main_axis_border_padding),
main_axis_margin(main_axis_margin),
frozen(false),
needs_relayout_for_stretch(false),
ng_input_node(/* LayoutBox* */ nullptr) {
DCHECK(!box->IsOutOfFlowPositioned());
DCHECK_GE(min_max_sizes.max_size, LayoutUnit())
......@@ -261,6 +262,54 @@ void FlexItem::ComputeStretchedSize() {
}
}
// static
LayoutUnit FlexItem::AlignmentOffset(LayoutUnit available_free_space,
ItemPosition position,
LayoutUnit ascent,
LayoutUnit max_ascent,
bool is_wrap_reverse,
bool is_deprecated_webkit_box) {
switch (position) {
case ItemPosition::kLegacy:
case ItemPosition::kAuto:
case ItemPosition::kNormal:
NOTREACHED();
break;
case ItemPosition::kStretch:
// Actual stretching must be handled by the caller. Since wrap-reverse
// flips cross start and cross end, stretch children should be aligned
// with the cross end. This matters because applyStretchAlignment
// doesn't always stretch or stretch fully (explicit cross size given, or
// stretching constrained by max-height/max-width). For flex-start and
// flex-end this is handled by alignmentForChild().
if (is_wrap_reverse)
return available_free_space;
break;
case ItemPosition::kFlexStart:
break;
case ItemPosition::kFlexEnd:
return available_free_space;
case ItemPosition::kCenter: {
const LayoutUnit result = (available_free_space / 2);
return is_deprecated_webkit_box ? result.ClampNegativeToZero() : result;
}
case ItemPosition::kBaseline:
// FIXME: If we get here in columns, we want the use the descent, except
// we currently can't get the ascent/descent of orthogonal children.
// https://bugs.webkit.org/show_bug.cgi?id=98076
return max_ascent - ascent;
case ItemPosition::kLastBaseline:
case ItemPosition::kSelfStart:
case ItemPosition::kSelfEnd:
case ItemPosition::kStart:
case ItemPosition::kEnd:
case ItemPosition::kLeft:
case ItemPosition::kRight:
// TODO(jfernandez): Implement these (https://crbug.com/722287).
break;
}
return LayoutUnit();
}
void FlexLine::FreezeViolations(ViolationsVector& violations) {
const ComputedStyle& flex_box_style = algorithm->StyleRef();
for (size_t i = 0; i < violations.size(); ++i) {
......@@ -660,6 +709,79 @@ void FlexLayoutAlgorithm::AlignFlexLines(LayoutUnit cross_axis_content_extent) {
}
}
void FlexLayoutAlgorithm::AlignChildren() {
// Keep track of the space between the baseline edge and the after edge of
// the box for each line.
Vector<LayoutUnit> min_margin_after_baselines;
for (FlexLine& line_context : flex_lines_) {
LayoutUnit min_margin_after_baseline = LayoutUnit::Max();
LayoutUnit max_ascent = line_context.max_ascent;
for (FlexItem& flex_item : line_context.line_items) {
DCHECK(!flex_item.box->IsOutOfFlowPositioned());
if (flex_item.UpdateAutoMarginsInCrossAxis(
std::max(LayoutUnit(), flex_item.AvailableAlignmentSpace()))) {
continue;
}
ItemPosition position = flex_item.Alignment();
if (position == ItemPosition::kStretch) {
flex_item.ComputeStretchedSize();
flex_item.needs_relayout_for_stretch = true;
}
LayoutUnit available_space = flex_item.AvailableAlignmentSpace();
LayoutUnit offset = FlexItem::AlignmentOffset(
available_space, position, flex_item.MarginBoxAscent(), max_ascent,
StyleRef().FlexWrap() == EFlexWrap::kWrapReverse,
StyleRef().IsDeprecatedWebkitBox());
flex_item.desired_location.Move(LayoutUnit(), offset);
if (position == ItemPosition::kBaseline &&
StyleRef().FlexWrap() == EFlexWrap::kWrapReverse) {
min_margin_after_baseline =
std::min(min_margin_after_baseline,
flex_item.AvailableAlignmentSpace() - offset);
}
}
min_margin_after_baselines.push_back(min_margin_after_baseline);
}
if (StyleRef().FlexWrap() != EFlexWrap::kWrapReverse)
return;
// wrap-reverse flips the cross axis start and end. For baseline alignment,
// this means we need to align the after edge of baseline elements with the
// after edge of the flex line.
wtf_size_t line_number = 0;
for (FlexLine& line_context : flex_lines_) {
LayoutUnit min_margin_after_baseline =
min_margin_after_baselines[line_number++];
for (FlexItem& flex_item : line_context.line_items) {
if (flex_item.Alignment() == ItemPosition::kBaseline &&
!flex_item.HasAutoMarginsInCrossAxis() && min_margin_after_baseline) {
flex_item.desired_location.Move(LayoutUnit(),
min_margin_after_baseline);
}
}
}
}
void FlexLayoutAlgorithm::FlipForWrapReverse(
LayoutUnit cross_axis_start_edge,
LayoutUnit cross_axis_content_size) {
DCHECK_EQ(Style()->FlexWrap(), EFlexWrap::kWrapReverse);
for (FlexLine& line_context : flex_lines_) {
LayoutUnit original_offset =
line_context.cross_axis_offset - cross_axis_start_edge;
LayoutUnit new_offset = cross_axis_content_size - original_offset -
line_context.cross_axis_extent;
LayoutUnit wrap_reverse_difference = new_offset - original_offset;
for (FlexItem& flex_item : line_context.line_items)
flex_item.desired_location.Move(LayoutUnit(), wrap_reverse_difference);
}
}
TransformedWritingMode FlexLayoutAlgorithm::GetTransformedWritingMode() const {
return GetTransformedWritingMode(*style_);
}
......
......@@ -171,6 +171,13 @@ class FlexItem {
inline const FlexLine* Line() const;
static LayoutUnit AlignmentOffset(LayoutUnit available_free_space,
ItemPosition position,
LayoutUnit ascent,
LayoutUnit max_ascent,
bool is_wrap_reverse,
bool is_deprecated_webkit_box);
FlexLayoutAlgorithm* algorithm;
wtf_size_t line_number;
LayoutBox* box;
......@@ -189,6 +196,11 @@ class FlexItem {
bool frozen;
// Legacy partially relies on FlexLayoutAlgorithm::AlignChildren to determine
// if the child is eligible for stretching (specifically, checking for auto
// margins). FlexLayoutAlgorithm uses this flag to report back to legacy.
bool needs_relayout_for_stretch;
NGBlockNode ng_input_node;
scoped_refptr<const NGLayoutResult> layout_result;
};
......@@ -378,6 +390,13 @@ class FlexLayoutAlgorithm {
// FlexLine::cross_axis_extent.
void AlignFlexLines(LayoutUnit cross_axis_content_extent);
// Positions flex items by modifying FlexItem::desired_location.
// When lines stretch, also modifies FlexItem::cross_axis_size.
void AlignChildren();
void FlipForWrapReverse(LayoutUnit cross_axis_start_edge,
LayoutUnit cross_axis_content_size);
static TransformedWritingMode GetTransformedWritingMode(const ComputedStyle&);
static const StyleContentAlignmentData& ContentAlignmentNormalBehavior();
......
......@@ -416,10 +416,16 @@ void LayoutFlexibleBox::RepositionLogicalHeightDependentFlexItems(
AlignFlexLines(algorithm);
AlignChildren(line_contexts);
AlignChildren(algorithm);
if (StyleRef().FlexWrap() == EFlexWrap::kWrapReverse)
FlipForWrapReverse(line_contexts, cross_axis_start_edge);
if (StyleRef().FlexWrap() == EFlexWrap::kWrapReverse) {
algorithm.FlipForWrapReverse(cross_axis_start_edge,
CrossAxisContentExtent());
for (FlexLine& line_context : line_contexts) {
for (FlexItem& flex_item : line_context.line_items)
ResetAlignmentForChild(*flex_item.box, flex_item.desired_location.Y());
}
}
// direction:rtl + flex-direction:column means the cross-axis direction is
// flipped.
......@@ -1198,54 +1204,6 @@ void LayoutFlexibleBox::ConstructAndAppendFlexItem(
border_and_padding, margin);
}
static LayoutUnit AlignmentOffset(LayoutUnit available_free_space,
ItemPosition position,
LayoutUnit ascent,
LayoutUnit max_ascent,
bool is_wrap_reverse,
bool is_deprecated_webkit_box) {
switch (position) {
case ItemPosition::kLegacy:
case ItemPosition::kAuto:
case ItemPosition::kNormal:
NOTREACHED();
break;
case ItemPosition::kStretch:
// Actual stretching must be handled by the caller. Since wrap-reverse
// flips cross start and cross end, stretch children should be aligned
// with the cross end. This matters because applyStretchAlignment
// doesn't always stretch or stretch fully (explicit cross size given, or
// stretching constrained by max-height/max-width). For flex-start and
// flex-end this is handled by alignmentForChild().
if (is_wrap_reverse)
return available_free_space;
break;
case ItemPosition::kFlexStart:
break;
case ItemPosition::kFlexEnd:
return available_free_space;
case ItemPosition::kCenter: {
const LayoutUnit result = (available_free_space / 2);
return is_deprecated_webkit_box ? result.ClampNegativeToZero() : result;
}
case ItemPosition::kBaseline:
// FIXME: If we get here in columns, we want the use the descent, except
// we currently can't get the ascent/descent of orthogonal children.
// https://bugs.webkit.org/show_bug.cgi?id=98076
return max_ascent - ascent;
case ItemPosition::kLastBaseline:
case ItemPosition::kSelfStart:
case ItemPosition::kSelfEnd:
case ItemPosition::kStart:
case ItemPosition::kEnd:
case ItemPosition::kLeft:
case ItemPosition::kRight:
// TODO(jferanndez): Implement these (https://crbug.com/722287).
break;
}
return LayoutUnit();
}
void LayoutFlexibleBox::SetOverrideMainAxisContentSizeForChild(FlexItem& item) {
if (MainAxisIsInlineAxis(*item.box)) {
item.box->SetOverrideLogicalWidth(item.FlexedBorderBoxSize());
......@@ -1273,7 +1231,7 @@ LayoutUnit LayoutFlexibleBox::StaticCrossAxisPositionForPositionedChild(
const LayoutBox& child) {
LayoutUnit available_space =
CrossAxisContentExtent() - CrossAxisExtentForChild(child);
return AlignmentOffset(
return FlexItem::AlignmentOffset(
available_space,
FlexLayoutAlgorithm::AlignmentForChild(StyleRef(), child.StyleRef()),
LayoutUnit(), LayoutUnit(),
......@@ -1562,14 +1520,6 @@ void LayoutFlexibleBox::AlignFlexLines(FlexLayoutAlgorithm& algorithm) {
}
}
void LayoutFlexibleBox::AdjustAlignmentForChild(LayoutBox& child,
LayoutUnit delta) {
DCHECK(!child.IsOutOfFlowPositioned());
SetFlowAwareLocationForChild(child, FlowAwareLocationForChild(child) +
LayoutSize(LayoutUnit(), delta));
}
void LayoutFlexibleBox::ResetAlignmentForChild(
LayoutBox& child,
LayoutUnit new_cross_axis_position) {
......@@ -1577,60 +1527,20 @@ void LayoutFlexibleBox::ResetAlignmentForChild(
child, {FlowAwareLocationForChild(child).X(), new_cross_axis_position});
}
void LayoutFlexibleBox::AlignChildren(Vector<FlexLine>& line_contexts) {
// Keep track of the space between the baseline edge and the after edge of
// the box for each line.
// TODO(cbiesinger): This should be stored in FlexLine
Vector<LayoutUnit> min_margin_after_baselines;
for (FlexLine& line_context : line_contexts) {
LayoutUnit min_margin_after_baseline = LayoutUnit::Max();
LayoutUnit max_ascent = line_context.max_ascent;
void LayoutFlexibleBox::AlignChildren(FlexLayoutAlgorithm& algorithm) {
Vector<FlexLine>& line_contexts = algorithm.FlexLines();
algorithm.AlignChildren();
for (unsigned line_number = 0; line_number < line_contexts.size();
++line_number) {
FlexLine& line_context = line_contexts[line_number];
for (FlexItem& flex_item : line_context.line_items) {
DCHECK(!flex_item.box->IsOutOfFlowPositioned());
if (flex_item.UpdateAutoMarginsInCrossAxis(
std::max(LayoutUnit(), flex_item.AvailableAlignmentSpace()))) {
ResetAlignmentForChild(*flex_item.box, flex_item.desired_location.Y());
continue;
}
ItemPosition position = flex_item.Alignment();
if (position == ItemPosition::kStretch) {
flex_item.ComputeStretchedSize();
if (flex_item.needs_relayout_for_stretch) {
DCHECK(flex_item.Alignment() == ItemPosition::kStretch);
ApplyStretchAlignmentToChild(flex_item);
flex_item.needs_relayout_for_stretch = false;
}
LayoutUnit available_space = flex_item.AvailableAlignmentSpace();
LayoutUnit offset = AlignmentOffset(
available_space, position, flex_item.MarginBoxAscent(), max_ascent,
StyleRef().FlexWrap() == EFlexWrap::kWrapReverse,
StyleRef().IsDeprecatedWebkitBox());
AdjustAlignmentForChild(*flex_item.box, offset);
if (position == ItemPosition::kBaseline &&
StyleRef().FlexWrap() == EFlexWrap::kWrapReverse) {
min_margin_after_baseline =
std::min(min_margin_after_baseline,
flex_item.AvailableAlignmentSpace() - offset);
}
}
min_margin_after_baselines.push_back(min_margin_after_baseline);
}
if (StyleRef().FlexWrap() != EFlexWrap::kWrapReverse)
return;
// wrap-reverse flips the cross axis start and end. For baseline alignment,
// this means we need to align the after edge of baseline elements with the
// after edge of the flex line.
wtf_size_t line_number = 0;
for (FlexLine& line_context : line_contexts) {
LayoutUnit min_margin_after_baseline =
min_margin_after_baselines[line_number++];
for (FlexItem& flex_item : line_context.line_items) {
if (flex_item.Alignment() == ItemPosition::kBaseline &&
!flex_item.HasAutoMarginsInCrossAxis() && min_margin_after_baseline)
AdjustAlignmentForChild(*flex_item.box, min_margin_after_baseline);
ResetAlignmentForChild(*flex_item.box, flex_item.desired_location.Y());
}
}
}
......@@ -1684,20 +1594,4 @@ void LayoutFlexibleBox::FlipForRightToLeftColumn(
}
}
void LayoutFlexibleBox::FlipForWrapReverse(
const Vector<FlexLine>& line_contexts,
LayoutUnit cross_axis_start_edge) {
LayoutUnit content_extent = CrossAxisContentExtent();
for (const FlexLine& line_context : line_contexts) {
for (const FlexItem& flex_item : line_context.line_items) {
LayoutUnit line_cross_axis_extent = line_context.cross_axis_extent;
LayoutUnit original_offset =
line_context.cross_axis_offset - cross_axis_start_edge;
LayoutUnit new_offset =
content_extent - original_offset - line_cross_axis_extent;
AdjustAlignmentForChild(*flex_item.box, new_offset - original_offset);
}
}
}
} // namespace blink
......@@ -155,7 +155,6 @@ class CORE_EXPORT LayoutFlexibleBox : public LayoutBlock {
LayoutBox& child,
LayoutUnit main_axis_border_and_padding,
ChildLayoutType = kLayoutIfNeeded);
void AdjustAlignmentForChild(LayoutBox& child, LayoutUnit);
void ResetAlignmentForChild(LayoutBox& child, LayoutUnit);
bool MainAxisLengthIsDefinite(const LayoutBox& child,
const Length& flex_basis,
......@@ -201,11 +200,9 @@ class CORE_EXPORT LayoutFlexibleBox : public LayoutBlock {
LayoutUnit cross_axis_offset,
LayoutUnit available_free_space);
void AlignFlexLines(FlexLayoutAlgorithm&);
void AlignChildren(Vector<FlexLine>&);
void AlignChildren(FlexLayoutAlgorithm&);
void ApplyStretchAlignmentToChild(FlexItem& child);
void FlipForRightToLeftColumn(const Vector<FlexLine>& line_contexts);
void FlipForWrapReverse(const Vector<FlexLine>&,
LayoutUnit cross_axis_start_edge);
float CountIntrinsicSizeForAlgorithmChange(
LayoutUnit max_preferred_width,
......
......@@ -599,60 +599,72 @@ scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::Layout() {
return container_builder_.ToBoxFragment();
}
void NGFlexLayoutAlgorithm::ApplyStretchAlignmentToChild(FlexItem& flex_item) {
WritingMode child_writing_mode =
flex_item.ng_input_node.Style().GetWritingMode();
NGConstraintSpaceBuilder space_builder(ConstraintSpace(), child_writing_mode,
/* is_new_fc */ true);
SetOrthogonalFallbackInlineSizeIfNeeded(Style(), flex_item.ng_input_node,
&space_builder);
LogicalSize available_size(
flex_item.flexed_content_size + flex_item.main_axis_border_padding,
flex_item.cross_axis_size);
if (is_column_) {
available_size.Transpose();
if (!IsColumnContainerMainSizeDefinite() &&
!IsItemMainSizeDefinite(flex_item.ng_input_node)) {
space_builder.SetIsFixedBlockSizeIndefinite(true);
}
}
space_builder.SetAvailableSize(available_size);
space_builder.SetPercentageResolutionSize(content_box_size_);
space_builder.SetIsFixedInlineSize(true);
space_builder.SetIsFixedBlockSize(true);
NGConstraintSpace child_space = space_builder.ToConstraintSpace();
flex_item.layout_result =
flex_item.ng_input_node.Layout(child_space, /* break_token */ nullptr);
}
void NGFlexLayoutAlgorithm::GiveLinesAndItemsFinalPositionAndSize() {
// TODO(dgrogan): This needs to eventually encompass all of the behavior in
// LayoutFlexibleBox::RepositionLogicalHeightDependentFlexItems. It currently
// does AlignFlexLines and the stretch part of AlignChildren.
LayoutUnit final_content_cross_size =
Vector<FlexLine>& line_contexts = algorithm_->FlexLines();
const LayoutUnit cross_axis_start_edge =
line_contexts.IsEmpty() ? LayoutUnit()
: line_contexts[0].cross_axis_offset;
const LayoutUnit final_content_cross_size =
is_column_ ? container_builder_.InlineSize() -
border_scrollbar_padding_.InlineSum()
: container_builder_.BlockSize() -
border_scrollbar_padding_.BlockSum();
if (!algorithm_->IsMultiline() && !algorithm_->FlexLines().IsEmpty())
algorithm_->FlexLines()[0].cross_axis_extent = final_content_cross_size;
// TODO(dgrogan): Implement the behavior from
// LayoutFlexibleBox::LayoutColumnReverse here.
if (!algorithm_->IsMultiline() && !line_contexts.IsEmpty())
line_contexts[0].cross_axis_extent = final_content_cross_size;
algorithm_->AlignFlexLines(final_content_cross_size);
for (FlexLine& line_context : algorithm_->FlexLines()) {
algorithm_->AlignChildren();
if (Style().FlexWrap() == EFlexWrap::kWrapReverse) {
// flex-wrap: wrap-reverse reverses the order of the lines in the container;
// FlipForWrapReverse recalculates each item's cross axis position. We have
// to do that after AlignChildren sets an initial cross axis position.
algorithm_->FlipForWrapReverse(cross_axis_start_edge,
final_content_cross_size);
}
for (FlexLine& line_context : line_contexts) {
for (wtf_size_t child_number = 0;
child_number < line_context.line_items.size(); ++child_number) {
FlexItem& flex_item = line_context.line_items[child_number];
// UpdateAutoMarginsInCrossAxis updates the flex_item's desired_location
// if the auto margins have an effect.
if (!flex_item.UpdateAutoMarginsInCrossAxis(
std::max(LayoutUnit(), flex_item.AvailableAlignmentSpace())) &&
flex_item.Alignment() == ItemPosition::kStretch) {
flex_item.ComputeStretchedSize();
WritingMode child_writing_mode =
flex_item.ng_input_node.Style().GetWritingMode();
NGConstraintSpaceBuilder space_builder(ConstraintSpace(),
child_writing_mode,
/* is_new_fc */ true);
SetOrthogonalFallbackInlineSizeIfNeeded(
Style(), flex_item.ng_input_node, &space_builder);
LogicalSize available_size(
flex_item.flexed_content_size + flex_item.main_axis_border_padding,
flex_item.cross_axis_size);
if (is_column_) {
available_size.Transpose();
if (!IsColumnContainerMainSizeDefinite() &&
!IsItemMainSizeDefinite(flex_item.ng_input_node)) {
space_builder.SetIsFixedBlockSizeIndefinite(true);
}
}
space_builder.SetAvailableSize(available_size);
space_builder.SetPercentageResolutionSize(content_box_size_);
space_builder.SetIsFixedInlineSize(true);
space_builder.SetIsFixedBlockSize(true);
NGConstraintSpace child_space = space_builder.ToConstraintSpace();
flex_item.layout_result = flex_item.ng_input_node.Layout(
child_space, /* break_token */ nullptr);
}
// TODO(dgrogan): Add an extra pass for kColumnReverse containers like
// legacy does in LayoutColumnReverse.
if (DoesItemStretch(flex_item.ng_input_node))
ApplyStretchAlignmentToChild(flex_item);
// TODO(dgrogan): Implement behavior from legacy's
// FlipForRightToLeftColumn here.
// flex_item.desired_location stores the main axis offset in X and the
// cross axis offset in Y. But AddChild wants offset from parent
......
......@@ -50,6 +50,7 @@ class CORE_EXPORT NGFlexLayoutAlgorithm
NGConstraintSpace BuildConstraintSpaceForDeterminingFlexBasis(
const NGBlockNode& flex_item) const;
void ConstructAndAppendFlexItems();
void ApplyStretchAlignmentToChild(FlexItem& flex_item);
void GiveLinesAndItemsFinalPositionAndSize();
// This is same method as FlexItem but we need that logic before FlexItem is
// constructed.
......
<!DOCTYPE html>
<title>Auto cross margins and align-items: stretch</title>
<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-stretch">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<meta name="flags" content="" />
<meta name="assert" content="Auto cross margins prevent an item from stretching." />
<link rel="bookmark" href="https://crbug.com/1015475" />
<style>
x-flexbox {
display: flex;
/* This makes auto margins prevent stretching without affecting position. Without
it the auto margins make the item move to the center of the window. */
width: 100px;
}
.item-with-auto-margin {
background: green;
margin: auto;
}
.pct-height-child {
height: 100%;
width: 100px;
background: red;
}
.second-child {
height: 100px;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<x-flexbox>
<div class=item-with-auto-margin>
<div class="pct-height-child"></div>
<div class="second-child"></div>
</div>
</x-flexbox>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment