Skip to content

GitLab

T
tangled
Commit b2b9dee1 authored by Javier Fernandez's avatar Javier Fernandez Committed by Commit Bot
Browse files
Options

[css-grid] Move the pre-layout logic out of the PlaceItemsOnGrid func 

The Grid Layout code needs to run a pre-layout for some specific case,
which include orthogonal and baseline aligned grid items. We decided to
execute this logic as part of the PlaceItemsOnGrid function, aiming for
a better performance and simpler and more elegant code.

However, the PlaceItemsOnGrid function is not executed on re-layouts if
no item has changed its position. Hence, there may be cases where we
need to repeat the layout of the grid and some of the items involved in
baseline alignment items are marked for layout as well.

In those cases, like the one described in the bug this CL tries to fix,
the grid item is assumed as not participating in baseline alignment, but
we resolve that it actually participates during the layout phase.

This CL tries to solve the issue my moving the pre-layout logic out of
the PlaceItemOnGrid function, so we ensure both orthogonal and baseline
alignment are laid out before running the intrinsic size and layout
operations.

Bug: 853427
Change-Id: I31d0357e647cccf728376a4abc12e8ea19983822
Reviewed-on: https://chromium-review.googlesource.com/1104425
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: default avatarManuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#568783}
parent 48d2aa2c
Show whitespace changes
Inline Side-by-side
Showing with 64 additions and 50 deletions
third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-self-baseline-and-item-relayout-should-not-crash.html 0 → 100644
View file @ b2b9dee1
<!doctype html>
<script>
if (window.testRunner)
testRunner.dumpAsText();
</script>
<p>This test should not crash</p>
<div style="display: grid;">
<div id="item" style="align-self: baseline;">PASS</div>
</div>
<script>
let item = document.getElementById("item");
item.offsetLeft;
item.style.width = "20px";
</script>
third_party/blink/renderer/core/layout/grid.cc
View file @ b2b9dee1
......@@ -142,10 +142,6 @@ GridSpan Grid::GridItemSpan(const LayoutBox& grid_item,
return direction == kForColumns ? area.columns : area.rows;
}
void Grid::SetHasAnyOrthogonalGridItem(bool has_any_orthogonal_grid_item) {
has_any_orthogonal_grid_item_ = has_any_orthogonal_grid_item;
}
void Grid::SetNeedsItemsPlacement(bool needs_items_placement) {
needs_items_placement_ = needs_items_placement;
......@@ -157,13 +153,14 @@ void Grid::SetNeedsItemsPlacement(bool needs_items_placement) {
ClearGridDataStructure();
grid_item_area_.clear();
grid_items_indexes_map_.clear();
has_any_orthogonal_grid_item_ = false;
smallest_row_start_ = 0;
smallest_column_start_ = 0;
auto_repeat_columns_ = 0;
auto_repeat_rows_ = 0;
auto_repeat_empty_columns_ = nullptr;
auto_repeat_empty_rows_ = nullptr;
baseline_grid_items_.resize(0);
orthogonal_grid_items_.resize(0);
}
Grid::GridIterator::GridIterator(GridTrackSizingDirection direction,
......
third_party/blink/renderer/core/layout/grid.h
View file @ b2b9dee1
......@@ -46,10 +46,21 @@ class Grid {
// Note that out of flow children are not grid items.
bool HasGridItems() const { return !grid_item_area_.IsEmpty(); }
void AddBaselineAlignedItem(LayoutBox& item) {
baseline_grid_items_.push_back(&item);
}
void AddOrthogonalItem(LayoutBox& item) {
orthogonal_grid_items_.push_back(&item);
}
bool HasAnyOrthogonalGridItem() const {
return has_any_orthogonal_grid_item_;
return !orthogonal_grid_items_.IsEmpty();
}
const Vector<LayoutBox*>& OrthogonalGridItems() const {
return orthogonal_grid_items_;
}
const Vector<LayoutBox*>& BaselineGridItems() const {
return baseline_grid_items_;
}
void SetHasAnyOrthogonalGridItem(bool);
GridArea GridItemArea(const LayoutBox&) const;
void SetGridItemArea(const LayoutBox&, GridArea);
......@@ -131,7 +142,6 @@ class Grid {
size_t auto_repeat_columns_{0};
size_t auto_repeat_rows_{0};
bool has_any_orthogonal_grid_item_{false};
bool needs_items_placement_{true};
HashMap<const LayoutBox*, GridArea> grid_item_area_;
......@@ -139,6 +149,9 @@ class Grid {
std::unique_ptr<OrderedTrackIndexSet> auto_repeat_empty_columns_{nullptr};
std::unique_ptr<OrderedTrackIndexSet> auto_repeat_empty_rows_{nullptr};
Vector<LayoutBox*> orthogonal_grid_items_;
Vector<LayoutBox*> baseline_grid_items_;
};
class VectorGrid final : public Grid {
......
third_party/blink/renderer/core/layout/layout_grid.cc
View file @ b2b9dee1
......@@ -304,6 +304,8 @@ void LayoutGrid::UpdateBlockLayout(bool relayout_children) {
LayoutUnit available_space_for_columns = AvailableLogicalWidth();
PlaceItemsOnGrid(track_sizing_algorithm_, available_space_for_columns);
PerformGridItemsPreLayout(track_sizing_algorithm_);
track_sizing_algorithm_.ComputeBaselineAlignmentContext();
// 1- First, the track sizing algorithm is used to resolve the sizes of the
......@@ -499,6 +501,8 @@ void LayoutGrid::ComputeIntrinsicLogicalWidths(
GridTrackSizingAlgorithm algorithm(this, *grid);
PlaceItemsOnGrid(algorithm, base::nullopt);
PerformGridItemsPreLayout(algorithm);
algorithm.ComputeBaselineAlignmentContext();
ComputeTrackSizesForIndefiniteSize(algorithm, kForColumns, min_logical_width,
max_logical_width);
......@@ -770,8 +774,6 @@ void LayoutGrid::PlaceItemsOnGrid(
Vector<LayoutBox*> auto_major_axis_auto_grid_items;
Vector<LayoutBox*> specified_major_axis_auto_grid_items;
Vector<LayoutBox*> orthogonal_grid_items;
Vector<LayoutBox*> baseline_grid_items;
#if DCHECK_IS_ON()
DCHECK(!grid.HasAnyGridItemPaintOrder());
#endif
......@@ -791,9 +793,9 @@ void LayoutGrid::PlaceItemsOnGrid(
child->SetOverrideContainingBlockContentLogicalHeight(LayoutUnit(-1));
if (GridLayoutUtils::IsOrthogonalChild(*this, *child))
orthogonal_grid_items.push_back(child);
grid.AddOrthogonalItem(*child);
if (IsBaselineAlignmentForChild(*child))
baseline_grid_items.push_back(child);
grid.AddBaselineAlignedItem(*child);
grid.SetGridItemPaintOrder(*child, child_index++);
GridArea area = grid.GridItemArea(*child);
......@@ -815,7 +817,6 @@ void LayoutGrid::PlaceItemsOnGrid(
}
grid.insert(*child, area);
}
grid.SetHasAnyOrthogonalGridItem(!orthogonal_grid_items.IsEmpty());
#if DCHECK_IS_ON()
if (grid.HasGridItems()) {
......@@ -839,19 +840,6 @@ void LayoutGrid::PlaceItemsOnGrid(
grid.SetNeedsItemsPlacement(false);
// Blink does a pre-layout of all the orthogonal boxes in the layout
// tree (see how LocalFrameView::PerformLayout calls its
// LayoutOrthogonalWritingModeRoots function). However, grid items
// don't participate in this process (see the function
// PrepareOrthogonalWritingModeRootForLayout) because it's useless
// and even wrong if they don't have their corresponding Grid Area.
LayoutOrthogonalWritingModeRoots(algorithm, orthogonal_grid_items);
// We need to layout the item to know whether it must synthesize its
// baseline or not, which may imply a cyclic sizing dependency.
// TODO (jfernandez): Can we avoid it ?
LayoutBaselineAlignedItems(algorithm, baseline_grid_items);
#if DCHECK_IS_ON()
for (LayoutBox* child = grid.GetOrderIterator().First(); child;
child = grid.GetOrderIterator().Next()) {
......@@ -876,32 +864,34 @@ static bool PrepareOrthogonalWritingModeRootForLayout(LayoutObject& root) {
return true;
}
// TODO(lajava): Consider rafactoring this code with
// LocalFrameView::LayoutOrthogonalWritingModeRoots
void LayoutGrid::LayoutOrthogonalWritingModeRoots(
const GridTrackSizingAlgorithm& algorithm,
const Vector<LayoutBox*>& orthogonal_grid_items) const {
void LayoutGrid::PerformGridItemsPreLayout(
const GridTrackSizingAlgorithm& algorithm) const {
DCHECK(!algorithm.GetGrid().NeedsItemsPlacement());
if (!GetDocument().View()->IsInPerformLayout())
return;
for (auto* root : orthogonal_grid_items) {
DCHECK(GridLayoutUtils::IsOrthogonalChild(*this, *root));
if (PrepareOrthogonalWritingModeRootForLayout(*root)) {
// Blink does a pre-layout of all the orthogonal boxes in the layout
// tree (see how LocalFrameView::PerformLayout calls its
// LayoutOrthogonalWritingModeRoots function). However, grid items
// don't participate in this process (see the function
// PrepareOrthogonalWritingModeRootForLayout) because it's useless
// and even wrong if they don't have their corresponding Grid Area.
// TODO(jfernandez): Consider rafactoring this code with
// LocalFrameView::LayoutOrthogonalWritingModeRoots
for (auto* item : algorithm.GetGrid().OrthogonalGridItems()) {
DCHECK(GridLayoutUtils::IsOrthogonalChild(*this, *item));
if (PrepareOrthogonalWritingModeRootForLayout(*item)) {
UpdateGridAreaLogicalSize(
*root, algorithm.EstimatedGridAreaBreadthForChild(*root));
root->LayoutIfNeeded();
*item, algorithm.EstimatedGridAreaBreadthForChild(*item));
item->LayoutIfNeeded();
}
}
}
void LayoutGrid::LayoutBaselineAlignedItems(
const GridTrackSizingAlgorithm& algorithm,
const Vector<LayoutBox*>& items_list) const {
if (!GetDocument().View()->IsInPerformLayout())
return;
for (auto* item : items_list) {
// We need to layout the item to know whether it must synthesize its
// baseline or not, which may imply a cyclic sizing dependency.
// TODO (jfernandez): Can we avoid it ?
for (auto* item : algorithm.GetGrid().BaselineGridItems()) {
DCHECK(IsBaselineAlignmentForChild(*item));
if (GridLayoutUtils::IsOrthogonalChild(*this, *item))
continue;
if (item->HasRelativeLogicalWidth() || item->HasRelativeLogicalHeight()) {
UpdateGridAreaLogicalSize(
*item, algorithm.EstimatedGridAreaBreadthForChild(*item));
......
third_party/blink/renderer/core/layout/layout_grid.h
View file @ b2b9dee1
......@@ -172,10 +172,7 @@ class LayoutGrid final : public LayoutBlock {
Grid&,
GridTrackSizingDirection) const;
void LayoutOrthogonalWritingModeRoots(const GridTrackSizingAlgorithm&,
const Vector<LayoutBox*>&) const;
void LayoutBaselineAlignedItems(const GridTrackSizingAlgorithm&,
const Vector<LayoutBox*>&) const;
void PerformGridItemsPreLayout(const GridTrackSizingAlgorithm&) const;
void PlaceItemsOnGrid(
GridTrackSizingAlgorithm&,
......
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