Skip to content

GitLab

T
tangled
Commit 30df6485 authored by Kunihiko Sakamoto's avatar Kunihiko Sakamoto Committed by Commit Bot
Browse files
Options

Revert "[GridNG] Implement alignment within grid-areas." 

This reverts commit f6a05301.

Reason for revert: virtual/layout-ng-grid/fast/css-grid-layout/mozilla/grid-repeat-auto-fill-fit-*.html are failing in multiple bots.

https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Win10/75331
https://ci.chromium.org/p/chromium/builders/ci/Mac10.14%20Tests/9841
https://ci.chromium.org/p/chromium/builders/ci/linux-trusty-rel/17342


Original change's description:
> [GridNG] Implement alignment within grid-areas.
>
> This patch implements alignment within a grid-item's grid-area.
> When creating the GridItemData we read of the containers, and
> grid-item's style, or determine its alignment axis-edge, and if it
> should be stretched, saving this information on the GridItemData.
>
> When placing the grid-item it will use this to adjust the offset.
>
> This doesn't implement baseline alignment which requires a secondary
> pass to correctly determine the baseline for the column/row.
>
> The other TODO relates to the constraint-space API, and how grid will
> need a slightly new concept in order to do block-axis stretching
> easily. By default non-replaced grid-items will stretch to the available
> space. A new bit on the space can indicate that we should stretch to
> the available space (if possible), this is similar to the shrink-to-fit
> bit we have today.
>
> This adds a self-start/self-end test as after implementing this now new
> tests passed immediately, and I don't believe we had good test
> coverage for inflow content with these keywords.
>
> Bug: 1045599
> Change-Id: I8c9cc51381c3d75519e52b32472b58ec96a6a85a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2492176
> Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
> Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
> Cr-Commit-Position: refs/heads/master@{#821043}

TBR=cbiesinger@chromium.org,ikilpatrick@chromium.org,kschmi@microsoft.com,ansollan@microsoft.com

Change-Id: I0a9398ce4bf339a632c3baed7374754f3c664496
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1045599
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500969Reviewed-by: default avatarKunihiko Sakamoto <ksakamoto@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821074}
parent 08be2d79
Expand all Show whitespace changes
Inline Side-by-side
Showing with 107 additions and 428 deletions
third_party/blink/renderer/core/layout/ng/grid/ng_grid_layout_algorithm.cc
View file @ 30df6485
......@@ -297,105 +297,8 @@ void NGGridLayoutAlgorithm::ConstructAndAppendGridItems() {
reordered_item_indices_.push_back(i);
}
namespace {
using AxisEdge = NGGridLayoutAlgorithm::AxisEdge;
// Given an |item_position| determines the correct |AxisEdge| alignment.
// Additionally will determine if the grid-item should be stretched with the
// |is_stretched| out-parameter.
AxisEdge AxisEdgeFromItemPosition(const ComputedStyle& container_style,
const ComputedStyle& style,
const ItemPosition item_position,
bool is_inline_axis,
bool* is_stretched) {
DCHECK(is_stretched);
*is_stretched = false;
// Auto-margins take precedence over any alignment properties.
if (style.MayHaveMargin()) {
bool start_auto = is_inline_axis
? style.MarginStartUsing(container_style).IsAuto()
: style.MarginBeforeUsing(container_style).IsAuto();
bool end_auto = is_inline_axis
? style.MarginEndUsing(container_style).IsAuto()
: style.MarginAfterUsing(container_style).IsAuto();
if (start_auto && end_auto)
return AxisEdge::kCenter;
else if (start_auto)
return AxisEdge::kEnd;
else if (end_auto)
return AxisEdge::kStart;
}
const auto container_writing_direction =
container_style.GetWritingDirection();
switch (item_position) {
case ItemPosition::kSelfStart:
case ItemPosition::kSelfEnd: {
// In order to determine the correct "self" axis-edge without a
// complicated set of if-branches we use two converters.
// First use the grid-item's writing-direction to convert the logical
// edge into the physical coordinate space.
LogicalToPhysical<AxisEdge> physical(style.GetWritingDirection(),
AxisEdge::kStart, AxisEdge::kEnd,
AxisEdge::kStart, AxisEdge::kEnd);
// Then use the container's writing-direction to convert the physical
// edges, into our logical coordinate space.
PhysicalToLogical<AxisEdge> logical(container_writing_direction,
physical.Top(), physical.Right(),
physical.Bottom(), physical.Left());
if (is_inline_axis) {
return item_position == ItemPosition::kSelfStart ? logical.InlineStart()
: logical.InlineEnd();
}
return item_position == ItemPosition::kSelfStart ? logical.BlockStart()
: logical.BlockEnd();
}
case ItemPosition::kCenter:
return AxisEdge::kCenter;
case ItemPosition::kFlexStart:
case ItemPosition::kStart:
return AxisEdge::kStart;
case ItemPosition::kFlexEnd:
case ItemPosition::kEnd:
return AxisEdge::kEnd;
case ItemPosition::kStretch:
*is_stretched = true;
return AxisEdge::kStart;
case ItemPosition::kBaseline:
case ItemPosition::kLastBaseline:
return AxisEdge::kBaseline;
case ItemPosition::kLeft:
DCHECK(is_inline_axis);
return container_writing_direction.IsLtr() ? AxisEdge::kStart
: AxisEdge::kEnd;
case ItemPosition::kRight:
DCHECK(is_inline_axis);
return container_writing_direction.IsRtl() ? AxisEdge::kStart
: AxisEdge::kEnd;
case ItemPosition::kLegacy:
case ItemPosition::kAuto:
case ItemPosition::kNormal:
NOTREACHED();
break;
}
NOTREACHED();
return AxisEdge::kStart;
}
} // namespace
NGGridLayoutAlgorithm::GridItemData NGGridLayoutAlgorithm::MeasureGridItem(
const NGBlockNode node) {
const auto& container_style = Style();
// Before we take track sizing into account for column width contributions,
// have all child inline and min/max sizes measured for content-based width
// resolution.
......@@ -420,23 +323,6 @@ NGGridLayoutAlgorithm::GridItemData NGGridLayoutAlgorithm::MeasureGridItem(
constraint_space, node, border_padding_in_child_writing_mode);
}
const ItemPosition normal_behaviour =
node.IsReplaced() ? ItemPosition::kStart : ItemPosition::kStretch;
// Determine the alignment for the grid-item ahead of time (we may need to
// know if it stretches ahead of time to correctly determine any block-axis
// contribution).
grid_item.inline_axis_alignment = AxisEdgeFromItemPosition(
container_style, child_style,
child_style.ResolvedJustifySelf(normal_behaviour, &container_style)
.GetPosition(),
/* is_inline_axis */ true, &grid_item.is_inline_axis_stretched);
grid_item.block_axis_alignment = AxisEdgeFromItemPosition(
container_style, child_style,
child_style.ResolvedAlignSelf(normal_behaviour, &container_style)
.GetPosition(),
/* is_inline_axis */ false, &grid_item.is_block_axis_stretched);
grid_item.margins =
ComputeMarginsFor(constraint_space, child_style, ConstraintSpace());
grid_item.min_max_sizes =
......@@ -1167,31 +1053,6 @@ void NGGridLayoutAlgorithm::PlaceGridItems() {
}
}
namespace {
// Returns the alignment offset for either the inline or block direction.
LayoutUnit AlignmentOffset(LayoutUnit container_size,
LayoutUnit size,
LayoutUnit margin_start,
LayoutUnit margin_end,
AxisEdge axis_edge) {
switch (axis_edge) {
case AxisEdge::kStart:
return margin_start;
case AxisEdge::kCenter:
return (container_size - size - margin_start - margin_end) / 2;
case AxisEdge::kEnd:
return container_size - margin_end - size;
case AxisEdge::kBaseline:
// TODO(ikilpatrick): Implement baseline alignment.
return margin_start;
}
NOTREACHED();
return LayoutUnit();
}
} // namespace
void NGGridLayoutAlgorithm::PlaceGridItem(const GridItemData& grid_item,
LogicalOffset offset,
LogicalSize size) {
......@@ -1203,34 +1064,14 @@ void NGGridLayoutAlgorithm::PlaceGridItem(const GridItemData& grid_item,
builder.SetAvailableSize(size);
builder.SetPercentageResolutionSize(size);
builder.SetTextDirection(item_style.Direction());
// TODO(ikilpatrick): We need a slightly different constraint space API now.
// Instead of a "shrink-to-fit" bit, we should have a "is-stretched" bit to
// indicate if 'auto' should stretch to fill the available space. This should
// apply to both the inline, and block axis. E.g.
// IsInlineSizeStretched / IsBlockSizeStretched or similar.
builder.SetIsFixedBlockSize(item_style.LogicalHeight().IsAuto() &&
grid_item.is_block_axis_stretched);
builder.SetIsShrinkToFit(item_style.LogicalWidth().IsAuto() &&
!grid_item.is_inline_axis_stretched);
builder.SetIsShrinkToFit(item_style.LogicalWidth().IsAuto());
NGConstraintSpace constraint_space = builder.ToConstraintSpace();
scoped_refptr<const NGLayoutResult> result =
grid_item.node.Layout(builder.ToConstraintSpace());
const auto& physical_fragment = result->PhysicalFragment();
// Apply the grid-item's alignment (if any).
NGFragment fragment(ConstraintSpace().GetWritingDirection(),
physical_fragment);
offset += LogicalOffset(
AlignmentOffset(size.inline_size, fragment.InlineSize(),
grid_item.margins.inline_start,
grid_item.margins.inline_end,
grid_item.inline_axis_alignment),
AlignmentOffset(
size.block_size, fragment.BlockSize(), grid_item.margins.block_start,
grid_item.margins.block_end, grid_item.block_axis_alignment));
container_builder_.AddChild(physical_fragment, offset);
grid_item.node.Layout(constraint_space);
container_builder_.AddChild(
result->PhysicalFragment(),
{offset.inline_offset + grid_item.margins.inline_start,
offset.block_offset + grid_item.margins.block_start});
}
LayoutUnit NGGridLayoutAlgorithm::GridGap(
......
third_party/blink/renderer/core/layout/ng/grid/ng_grid_layout_algorithm.h
View file @ 30df6485
......@@ -32,7 +32,6 @@ class CORE_EXPORT NGGridLayoutAlgorithm
NGBoxFragmentBuilder,
NGBlockBreakToken> {
public:
enum class AxisEdge { kStart, kCenter, kEnd, kBaseline };
struct GridItemData {
explicit GridItemData(const NGBlockNode node);
......@@ -61,12 +60,6 @@ class CORE_EXPORT NGGridLayoutAlgorithm
wtf_size_t rows_begin_set_index;
wtf_size_t rows_end_set_index;
AxisEdge inline_axis_alignment;
AxisEdge block_axis_alignment;
bool is_inline_axis_stretched;
bool is_block_axis_stretched;
bool is_spanning_flex_track : 1;
bool is_spanning_intrinsic_track : 1;
};
......
third_party/blink/web_tests/virtual/layout-ng-grid/external/wpt/css/css-grid/grid-definition/grid-auto-explicit-rows-001-expected.txt 0 → 100644
View file @ 30df6485
This is a testharness.js-based test.
FAIL .grid 1 assert_equals:
<div class="grid">
<div id="c1" data-expected-width="23" data-expected-height="11"></div>
<div id="c2" data-expected-width="52" data-expected-height="24"></div>
<div id="c3" data-expected-width="83" data-expected-height="41"></div>
<div id="c4" data-expected-width="120" data-expected-height="60"></div>
</div>
width expected 23 but got 784
Harness: the test ran to completion.
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