Commit fb43a1a6 authored by Oriol Brufau's avatar Oriol Brufau Committed by Commit Bot

[css-grid] Fix referencing implicit grid line after auto repeat()

The indices of both implicit grid lines defined with grid-template-areas
and explicit ones defined with grid-template-rows/columns used to be
stored together in NamedGridColumnLines and NamedGridRowLines.

However, this was problematic, because the former indices already refer
to the final explicit grid so they don't have to be increased when
expanding an auto repeat(), but the latter ones should.

Therefore, this patch stores the indices in separate fields and uses the
correct logic for each one.

BUG=966090

TEST=external/wpt/css/css-grid/placement/grid-placement-using-named-grid-lines-005.html
TEST=external/wpt/css/css-grid/placement/grid-placement-using-named-grid-lines-006.html

Change-Id: I6d423148af0e4dd865f130742f7a927a325cef90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081823Reviewed-by: default avatarJavier Fernandez <jfernandez@igalia.com>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#747669}
parent 3ad7e7db
...@@ -3115,18 +3115,15 @@ void GridTemplateAreas::ApplyValue(StyleResolverState& state, ...@@ -3115,18 +3115,15 @@ void GridTemplateAreas::ApplyValue(StyleResolverState& state,
const NamedGridAreaMap& new_named_grid_areas = const NamedGridAreaMap& new_named_grid_areas =
grid_template_areas_value.GridAreaMap(); grid_template_areas_value.GridAreaMap();
NamedGridLinesMap named_grid_column_lines; NamedGridLinesMap implicit_named_grid_column_lines;
NamedGridLinesMap named_grid_row_lines; NamedGridLinesMap implicit_named_grid_row_lines;
StyleBuilderConverter::ConvertOrderedNamedGridLinesMapToNamedGridLinesMap(
state.Style()->OrderedNamedGridColumnLines(), named_grid_column_lines);
StyleBuilderConverter::ConvertOrderedNamedGridLinesMapToNamedGridLinesMap(
state.Style()->OrderedNamedGridRowLines(), named_grid_row_lines);
StyleBuilderConverter::CreateImplicitNamedGridLinesFromGridArea( StyleBuilderConverter::CreateImplicitNamedGridLinesFromGridArea(
new_named_grid_areas, named_grid_column_lines, kForColumns); new_named_grid_areas, implicit_named_grid_column_lines, kForColumns);
StyleBuilderConverter::CreateImplicitNamedGridLinesFromGridArea( StyleBuilderConverter::CreateImplicitNamedGridLinesFromGridArea(
new_named_grid_areas, named_grid_row_lines, kForRows); new_named_grid_areas, implicit_named_grid_row_lines, kForRows);
state.Style()->SetNamedGridColumnLines(named_grid_column_lines); state.Style()->SetImplicitNamedGridColumnLines(
state.Style()->SetNamedGridRowLines(named_grid_row_lines); implicit_named_grid_column_lines);
state.Style()->SetImplicitNamedGridRowLines(implicit_named_grid_row_lines);
state.Style()->SetNamedGridArea(new_named_grid_areas); state.Style()->SetNamedGridArea(new_named_grid_areas);
state.Style()->SetNamedGridAreaRowCount(grid_template_areas_value.RowCount()); state.Style()->SetNamedGridAreaRowCount(grid_template_areas_value.RowCount());
......
...@@ -939,28 +939,6 @@ void StyleBuilderConverter::ConvertGridTrackList( ...@@ -939,28 +939,6 @@ void StyleBuilderConverter::ConvertGridTrackList(
DCHECK(!track_sizes.IsEmpty() || !auto_repeat_track_sizes.IsEmpty()); DCHECK(!track_sizes.IsEmpty() || !auto_repeat_track_sizes.IsEmpty());
} }
void StyleBuilderConverter::ConvertOrderedNamedGridLinesMapToNamedGridLinesMap(
const OrderedNamedGridLines& ordered_named_grid_lines,
NamedGridLinesMap& named_grid_lines) {
DCHECK_EQ(named_grid_lines.size(), 0u);
if (ordered_named_grid_lines.size() == 0)
return;
for (auto& ordered_named_grid_line : ordered_named_grid_lines) {
for (auto& line_name : ordered_named_grid_line.value) {
NamedGridLinesMap::AddResult start_result =
named_grid_lines.insert(line_name, Vector<size_t>());
start_result.stored_value->value.push_back(ordered_named_grid_line.key);
}
}
for (auto& named_grid_line : named_grid_lines) {
Vector<size_t>& grid_line_indexes = named_grid_line.value;
std::sort(grid_line_indexes.begin(), grid_line_indexes.end());
}
}
void StyleBuilderConverter::CreateImplicitNamedGridLinesFromGridArea( void StyleBuilderConverter::CreateImplicitNamedGridLinesFromGridArea(
const NamedGridAreaMap& named_grid_areas, const NamedGridAreaMap& named_grid_areas,
NamedGridLinesMap& named_grid_lines, NamedGridLinesMap& named_grid_lines,
......
...@@ -231,9 +231,6 @@ class StyleBuilderConverter { ...@@ -231,9 +231,6 @@ class StyleBuilderConverter {
const NamedGridAreaMap&, const NamedGridAreaMap&,
NamedGridLinesMap&, NamedGridLinesMap&,
GridTrackSizingDirection); GridTrackSizingDirection);
static void ConvertOrderedNamedGridLinesMapToNamedGridLinesMap(
const OrderedNamedGridLines&,
NamedGridLinesMap&);
static cc::ScrollSnapType ConvertSnapType(StyleResolverState&, static cc::ScrollSnapType ConvertSnapType(StyleResolverState&,
const CSSValue&); const CSSValue&);
......
...@@ -98,7 +98,8 @@ ...@@ -98,7 +98,8 @@
"NamedGridColumnLines", "NamedGridRowLines", "OrderedNamedGridColumnLines", "NamedGridColumnLines", "NamedGridRowLines", "OrderedNamedGridColumnLines",
"OrderedNamedGridRowLines", "AutoRepeatNamedGridColumnLines", "OrderedNamedGridRowLines", "AutoRepeatNamedGridColumnLines",
"AutoRepeatNamedGridRowLines", "AutoRepeatOrderedNamedGridColumnLines", "AutoRepeatNamedGridRowLines", "AutoRepeatOrderedNamedGridColumnLines",
"AutoRepeatOrderedNamedGridRowLines", "NamedGridArea", "grid-auto-rows", "AutoRepeatOrderedNamedGridRowLines", "ImplicitNamedGridColumnLines",
"ImplicitNamedGridRowLines", "NamedGridArea", "grid-auto-rows",
"grid-template-rows", "grid-template-columns", "grid-auto-columns", "row-gap", "grid-template-rows", "grid-template-columns", "grid-auto-columns", "row-gap",
"NamedGridAreaRowCount", "NamedGridAreaColumnCount", "NamedGridAreaRowCount", "NamedGridAreaColumnCount",
"GridAutoRepeatColumns", "GridAutoRepeatRows", "GridAutoRepeatColumnsInsertionPoint", "GridAutoRepeatColumns", "GridAutoRepeatRows", "GridAutoRepeatColumnsInsertionPoint",
......
...@@ -878,6 +878,22 @@ ...@@ -878,6 +878,22 @@
default_value: "OrderedNamedGridLines()", default_value: "OrderedNamedGridLines()",
include_paths: ["third_party/blink/renderer/core/style/ordered_named_grid_lines.h"], include_paths: ["third_party/blink/renderer/core/style/ordered_named_grid_lines.h"],
}, },
{
name: "ImplicitNamedGridColumnLines",
field_template: "external",
type_name: "NamedGridLinesMap",
field_group: "*->grid",
default_value: "NamedGridLinesMap()",
include_paths: ["third_party/blink/renderer/core/style/named_grid_lines_map.h"],
},
{
name: "ImplicitNamedGridRowLines",
field_template: "external",
type_name: "NamedGridLinesMap",
field_group: "*->grid",
default_value: "NamedGridLinesMap()",
include_paths: ["third_party/blink/renderer/core/style/named_grid_lines_map.h"],
},
{ {
name: "NamedGridArea", name: "NamedGridArea",
field_template: "external", field_template: "external",
......
...@@ -38,6 +38,9 @@ NamedLineCollection::NamedLineCollection( ...@@ -38,6 +38,9 @@ NamedLineCollection::NamedLineCollection(
const NamedGridLinesMap& auto_repeat_grid_line_names = const NamedGridLinesMap& auto_repeat_grid_line_names =
is_row_axis ? grid_container_style.AutoRepeatNamedGridColumnLines() is_row_axis ? grid_container_style.AutoRepeatNamedGridColumnLines()
: grid_container_style.AutoRepeatNamedGridRowLines(); : grid_container_style.AutoRepeatNamedGridRowLines();
const NamedGridLinesMap& implicit_grid_line_names =
is_row_axis ? grid_container_style.ImplicitNamedGridColumnLines()
: grid_container_style.ImplicitNamedGridRowLines();
if (!grid_line_names.IsEmpty()) { if (!grid_line_names.IsEmpty()) {
auto it = grid_line_names.find(named_line); auto it = grid_line_names.find(named_line);
...@@ -50,6 +53,12 @@ NamedLineCollection::NamedLineCollection( ...@@ -50,6 +53,12 @@ NamedLineCollection::NamedLineCollection(
it == auto_repeat_grid_line_names.end() ? nullptr : &it->value; it == auto_repeat_grid_line_names.end() ? nullptr : &it->value;
} }
if (!implicit_grid_line_names.IsEmpty()) {
auto it = implicit_grid_line_names.find(named_line);
implicit_named_lines_indexes_ =
it == implicit_grid_line_names.end() ? nullptr : &it->value;
}
insertion_point_ = insertion_point_ =
is_row_axis ? grid_container_style.GridAutoRepeatColumnsInsertionPoint() is_row_axis ? grid_container_style.GridAutoRepeatColumnsInsertionPoint()
: grid_container_style.GridAutoRepeatRowsInsertionPoint(); : grid_container_style.GridAutoRepeatRowsInsertionPoint();
...@@ -59,10 +68,14 @@ NamedLineCollection::NamedLineCollection( ...@@ -59,10 +68,14 @@ NamedLineCollection::NamedLineCollection(
: grid_container_style.GridAutoRepeatRows().size(); : grid_container_style.GridAutoRepeatRows().size();
} }
bool NamedLineCollection::HasNamedLines() { bool NamedLineCollection::HasExplicitNamedLines() {
return named_lines_indexes_ || auto_repeat_named_lines_indexes_; return named_lines_indexes_ || auto_repeat_named_lines_indexes_;
} }
bool NamedLineCollection::HasNamedLines() {
return HasExplicitNamedLines() || implicit_named_lines_indexes_;
}
bool NamedLineCollection::Contains(size_t line) { bool NamedLineCollection::Contains(size_t line) {
CHECK(HasNamedLines()); CHECK(HasNamedLines());
...@@ -73,6 +86,9 @@ bool NamedLineCollection::Contains(size_t line) { ...@@ -73,6 +86,9 @@ bool NamedLineCollection::Contains(size_t line) {
return indexes && indexes->Find(line) != kNotFound; return indexes && indexes->Find(line) != kNotFound;
}; };
if (find(implicit_named_lines_indexes_, line))
return true;
if (auto_repeat_track_list_length_ == 0LU || line < insertion_point_) if (auto_repeat_track_list_length_ == 0LU || line < insertion_point_)
return find(named_lines_indexes_, line); return find(named_lines_indexes_, line);
...@@ -101,14 +117,14 @@ bool NamedLineCollection::Contains(size_t line) { ...@@ -101,14 +117,14 @@ bool NamedLineCollection::Contains(size_t line) {
auto_repeat_index_in_first_repetition); auto_repeat_index_in_first_repetition);
} }
size_t NamedLineCollection::FirstPosition() { size_t NamedLineCollection::FirstExplicitPosition() {
CHECK(HasNamedLines()); DCHECK(HasExplicitNamedLines());
size_t first_line = 0; size_t first_line = 0;
// If there is no auto repeat(), there must be some named line outside, return // If there is no auto repeat(), there must be some named line outside, return
// the 1st one. Also return it if it precedes the auto-repeat(). // the 1st one. Also return it if it precedes the auto-repeat().
if (auto_repeat_total_tracks_ == 0 || if (auto_repeat_track_list_length_ == 0 ||
(named_lines_indexes_ && (named_lines_indexes_ &&
named_lines_indexes_->at(first_line) <= insertion_point_)) named_lines_indexes_->at(first_line) <= insertion_point_))
return named_lines_indexes_->at(first_line); return named_lines_indexes_->at(first_line);
...@@ -121,6 +137,21 @@ size_t NamedLineCollection::FirstPosition() { ...@@ -121,6 +137,21 @@ size_t NamedLineCollection::FirstPosition() {
return named_lines_indexes_->at(first_line) + auto_repeat_total_tracks_ - 1; return named_lines_indexes_->at(first_line) + auto_repeat_total_tracks_ - 1;
} }
size_t NamedLineCollection::FirstPosition() {
CHECK(HasNamedLines());
size_t first_line = 0;
if (!implicit_named_lines_indexes_)
return FirstExplicitPosition();
if (!HasExplicitNamedLines())
return implicit_named_lines_indexes_->at(first_line);
return std::min(FirstExplicitPosition(),
implicit_named_lines_indexes_->at(first_line));
}
GridPositionSide GridPositionsResolver::InitialPositionSide( GridPositionSide GridPositionsResolver::InitialPositionSide(
GridTrackSizingDirection direction) { GridTrackSizingDirection direction) {
return (direction == kForColumns) ? kColumnStartSide : kRowStartSide; return (direction == kForColumns) ? kColumnStartSide : kRowStartSide;
......
...@@ -38,8 +38,11 @@ class NamedLineCollection { ...@@ -38,8 +38,11 @@ class NamedLineCollection {
bool Contains(size_t line); bool Contains(size_t line);
private: private:
bool HasExplicitNamedLines();
size_t FirstExplicitPosition();
const Vector<size_t>* named_lines_indexes_ = nullptr; const Vector<size_t>* named_lines_indexes_ = nullptr;
const Vector<size_t>* auto_repeat_named_lines_indexes_ = nullptr; const Vector<size_t>* auto_repeat_named_lines_indexes_ = nullptr;
const Vector<size_t>* implicit_named_lines_indexes_ = nullptr;
size_t insertion_point_; size_t insertion_point_;
size_t last_line_; size_t last_line_;
......
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Grid item placement with implicit named line and auto repeat()</title>
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
<link rel="help" href="http://www.w3.org/TR/css-grid-1/#line-placement">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<meta name="assert" content="Grid placement algorithm is able to select the right line when defined implicitly with grid-template-areas.">
<style>
.grid {
display: grid;
width: 300px;
height: 300px;
grid-template-areas: "area";
grid-template-columns: repeat(auto-fill, 100px);
grid-template-rows: repeat(auto-fill, 100px) [area-end] 100px;
}
.grid > div {
grid-area: area;
background: green;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div class="grid">
<div></div>
</div>
<!DOCTYPE html>
<meta charset="utf-8">
<title>CSS Grid Layout Test: Grid item placement with implicit named line, '&lt;integer&gt; && &lt;custom-ident&gt;', and auto repeat()</title>
<link rel="author" title="Oriol Brufau" href="mailto:obrufau@igalia.com">
<link rel="help" href="http://www.w3.org/TR/css-grid-1/#line-placement">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<meta name="assert" content="Grid placement algorithm is able to select the right line when defined implicitly with grid-template-areas, referenced using the '<integer> && <custom-ident>' syntax, and with auto repeat().">
<style>
.grid {
display: grid;
width: 300px;
height: 300px;
margin-left: -100px;
margin-top: -100px;
grid-template-areas: ". ." ". foo";
grid-template-columns: repeat(auto-fill, 100px) [foo-start];
grid-template-rows: repeat(auto-fill, 100px) [foo-start];
}
.grid > div {
grid-area: 1 foo-start / 1 foo-start;
background: green;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div class="grid">
<div></div>
</div>
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