<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[208973] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/208973">208973</a></dd>
<dt>Author</dt> <dd>svillar@igalia.com</dd>
<dt>Date</dt> <dd>2016-11-24 07:08:11 -0800 (Thu, 24 Nov 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[css-grid] Convert grid representation into a class
https://bugs.webkit.org/show_bug.cgi?id=165042
Reviewed by Manuel Rego Casasnovas.
So far grids are represented as Vectors of Vectors. There are a couple of issues associated
to that decision. First or all, the source code in RenderGrid assumes the existence of that
data structure, meaning that we cannot eventually change it without changing a lot of
code. Apart from the coupling there is another issue, RenderGrid is full of methods to
access and manipulate that data structure.
Instead, it'd be much better to have a Grid class encapsulating both the data structures and
the methods required to access/manipulate it. Note that follow-up patches will move even
more data and procedures into this new class from the RenderGrid code.
No new tests required as this is a refactoring.
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::Grid::ensureGridSize): Moved from RenderGrid.
(WebCore::RenderGrid::Grid::insert): Ditto.
(WebCore::RenderGrid::Grid::clear): Ditto.
(WebCore::RenderGrid::GridIterator::GridIterator):
(WebCore::RenderGrid::gridColumnCount): Use Grid's methods.
(WebCore::RenderGrid::gridRowCount): Ditto.
(WebCore::RenderGrid::placeItemsOnGrid): Use Grid's methods to insert children.
(WebCore::RenderGrid::populateExplicitGridAndOrderIterator): Ditto.
(WebCore::RenderGrid::placeSpecifiedMajorAxisItemsOnGrid): Ditto.
(WebCore::RenderGrid::placeAutoMajorAxisItemOnGrid): Ditto.
(WebCore::RenderGrid::numTracks): Use Grid's methods.
(WebCore::RenderGrid::ensureGridSize): Deleted. Moved to Grid class.
(WebCore::RenderGrid::insertItemIntoGrid): Deleted. Moved to Grid class.
* rendering/RenderGrid.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderGridcpp">trunk/Source/WebCore/rendering/RenderGrid.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderGridh">trunk/Source/WebCore/rendering/RenderGrid.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (208972 => 208973)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-11-24 13:35:34 UTC (rev 208972)
+++ trunk/Source/WebCore/ChangeLog 2016-11-24 15:08:11 UTC (rev 208973)
</span><span class="lines">@@ -1,3 +1,38 @@
</span><ins>+2016-11-23 Sergio Villar Senin <svillar@igalia.com>
+
+ [css-grid] Convert grid representation into a class
+ https://bugs.webkit.org/show_bug.cgi?id=165042
+
+ Reviewed by Manuel Rego Casasnovas.
+
+ So far grids are represented as Vectors of Vectors. There are a couple of issues associated
+ to that decision. First or all, the source code in RenderGrid assumes the existence of that
+ data structure, meaning that we cannot eventually change it without changing a lot of
+ code. Apart from the coupling there is another issue, RenderGrid is full of methods to
+ access and manipulate that data structure.
+
+ Instead, it'd be much better to have a Grid class encapsulating both the data structures and
+ the methods required to access/manipulate it. Note that follow-up patches will move even
+ more data and procedures into this new class from the RenderGrid code.
+
+ No new tests required as this is a refactoring.
+
+ * rendering/RenderGrid.cpp:
+ (WebCore::RenderGrid::Grid::ensureGridSize): Moved from RenderGrid.
+ (WebCore::RenderGrid::Grid::insert): Ditto.
+ (WebCore::RenderGrid::Grid::clear): Ditto.
+ (WebCore::RenderGrid::GridIterator::GridIterator):
+ (WebCore::RenderGrid::gridColumnCount): Use Grid's methods.
+ (WebCore::RenderGrid::gridRowCount): Ditto.
+ (WebCore::RenderGrid::placeItemsOnGrid): Use Grid's methods to insert children.
+ (WebCore::RenderGrid::populateExplicitGridAndOrderIterator): Ditto.
+ (WebCore::RenderGrid::placeSpecifiedMajorAxisItemsOnGrid): Ditto.
+ (WebCore::RenderGrid::placeAutoMajorAxisItemOnGrid): Ditto.
+ (WebCore::RenderGrid::numTracks): Use Grid's methods.
+ (WebCore::RenderGrid::ensureGridSize): Deleted. Moved to Grid class.
+ (WebCore::RenderGrid::insertItemIntoGrid): Deleted. Moved to Grid class.
+ * rendering/RenderGrid.h:
+
</ins><span class="cx"> 2016-11-24 Antti Koivisto <antti@apple.com>
</span><span class="cx">
</span><span class="cx"> Remove unused bool return from Element::willRecalcStyle
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderGridcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (208972 => 208973)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderGrid.cpp 2016-11-24 13:35:34 UTC (rev 208972)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp 2016-11-24 15:08:11 UTC (rev 208973)
</span><span class="lines">@@ -46,6 +46,38 @@
</span><span class="cx"> ForbidInfinity,
</span><span class="cx"> };
</span><span class="cx">
</span><ins>+void RenderGrid::Grid::ensureGridSize(unsigned maximumRowSize, unsigned maximumColumnSize)
+{
+ const size_t oldColumnSize = numColumns();
+ const size_t oldRowSize = numRows();
+ if (maximumRowSize > oldRowSize) {
+ m_grid.grow(maximumRowSize);
+ for (size_t row = oldRowSize; row < maximumRowSize; ++row)
+ m_grid[row].grow(oldColumnSize);
+ }
+
+ if (maximumColumnSize > oldColumnSize) {
+ for (size_t row = 0; row < numRows(); ++row)
+ m_grid[row].grow(maximumColumnSize);
+ }
+}
+
+void RenderGrid::Grid::insert(RenderBox& child, const GridArea& area)
+{
+ ASSERT(area.rows.isTranslatedDefinite() && area.columns.isTranslatedDefinite());
+ ensureGridSize(area.rows.endLine(), area.columns.endLine());
+
+ for (const auto& row : area.rows) {
+ for (const auto& column : area.columns)
+ m_grid[row][column].append(&child);
+ }
+}
+
+void RenderGrid::Grid::clear()
+{
+ m_grid.resize(0);
+}
+
</ins><span class="cx"> class GridTrack {
</span><span class="cx"> public:
</span><span class="cx"> GridTrack() {}
</span><span class="lines">@@ -149,8 +181,8 @@
</span><span class="cx"> public:
</span><span class="cx"> // |direction| is the direction that is fixed to |fixedTrackIndex| so e.g
</span><span class="cx"> // GridIterator(m_grid, ForColumns, 1) will walk over the rows of the 2nd column.
</span><del>- GridIterator(const Vector<Vector<Vector<RenderBox*, 1>>>& grid, GridTrackSizingDirection direction, unsigned fixedTrackIndex, unsigned varyingTrackIndex = 0)
- : m_grid(grid)
</del><ins>+ GridIterator(const Grid& grid, GridTrackSizingDirection direction, unsigned fixedTrackIndex, unsigned varyingTrackIndex = 0)
+ : m_grid(grid.m_grid)
</ins><span class="cx"> , m_direction(direction)
</span><span class="cx"> , m_rowIndex((direction == ForColumns) ? varyingTrackIndex : fixedTrackIndex)
</span><span class="cx"> , m_columnIndex((direction == ForColumns) ? fixedTrackIndex : varyingTrackIndex)
</span><span class="lines">@@ -227,7 +259,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> private:
</span><del>- const Vector<Vector<Vector<RenderBox*, 1>>>& m_grid;
</del><ins>+ const GridAsMatrix& m_grid;
</ins><span class="cx"> GridTrackSizingDirection m_direction;
</span><span class="cx"> unsigned m_rowIndex;
</span><span class="cx"> unsigned m_columnIndex;
</span><span class="lines">@@ -387,13 +419,13 @@
</span><span class="cx"> unsigned RenderGrid::gridColumnCount() const
</span><span class="cx"> {
</span><span class="cx"> ASSERT(!m_gridIsDirty);
</span><del>- return m_grid.size() ? m_grid[0].size() : 0;
</del><ins>+ return m_grid.numColumns();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> unsigned RenderGrid::gridRowCount() const
</span><span class="cx"> {
</span><span class="cx"> ASSERT(!m_gridIsDirty);
</span><del>- return m_grid.size();
</del><ins>+ return m_grid.numRows();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> LayoutUnit RenderGrid::computeTrackBasedLogicalHeight(const GridSizingData& sizingData) const
</span><span class="lines">@@ -1510,32 +1542,6 @@
</span><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span><del>-void RenderGrid::ensureGridSize(unsigned maximumRowSize, unsigned maximumColumnSize)
-{
- const unsigned oldRowCount = gridRowCount();
- if (maximumRowSize > oldRowCount) {
- m_grid.grow(maximumRowSize);
- for (unsigned row = oldRowCount; row < gridRowCount(); ++row)
- m_grid[row].grow(gridColumnCount());
- }
-
- if (maximumColumnSize > gridColumnCount()) {
- for (unsigned row = 0; row < gridRowCount(); ++row)
- m_grid[row].grow(maximumColumnSize);
- }
-}
-
-void RenderGrid::insertItemIntoGrid(RenderBox& child, const GridArea& area)
-{
- ASSERT(area.rows.isTranslatedDefinite() && area.columns.isTranslatedDefinite());
- ensureGridSize(area.rows.endLine(), area.columns.endLine());
-
- for (auto row : area.rows) {
- for (auto column : area.columns)
- m_grid[row][column].append(&child);
- }
-}
-
</del><span class="cx"> unsigned RenderGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection direction, SizingOperation sizingOperation) const
</span><span class="cx"> {
</span><span class="cx"> bool isRowAxis = direction == ForColumns;
</span><span class="lines">@@ -1679,7 +1685,7 @@
</span><span class="cx"> specifiedMajorAxisAutoGridItems.append(child);
</span><span class="cx"> continue;
</span><span class="cx"> }
</span><del>- insertItemIntoGrid(*child, GridArea(area.rows, area.columns));
</del><ins>+ m_grid.insert(*child, { area.rows, area.columns });
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if ENABLE(ASSERT)
</span><span class="lines">@@ -1743,9 +1749,7 @@
</span><span class="cx"> m_gridItemArea.set(child, GridArea(rowPositions, columnPositions));
</span><span class="cx"> }
</span><span class="cx">
</span><del>- m_grid.grow(maximumRowIndex + std::abs(m_smallestRowStart));
- for (auto& column : m_grid)
- column.grow(maximumColumnIndex + std::abs(m_smallestColumnStart));
</del><ins>+ m_grid.ensureGridSize(maximumRowIndex + std::abs(m_smallestRowStart), maximumColumnIndex + std::abs(m_smallestColumnStart));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> std::unique_ptr<GridArea> RenderGrid::createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(const RenderBox& gridItem, GridTrackSizingDirection specifiedDirection, const GridSpan& specifiedPositions) const
</span><span class="lines">@@ -1780,7 +1784,7 @@
</span><span class="cx"> emptyGridArea = createEmptyGridAreaAtSpecifiedPositionsOutsideGrid(*autoGridItem, autoPlacementMajorAxisDirection(), majorAxisPositions);
</span><span class="cx">
</span><span class="cx"> m_gridItemArea.set(autoGridItem, *emptyGridArea);
</span><del>- insertItemIntoGrid(*autoGridItem, *emptyGridArea);
</del><ins>+ m_grid.insert(*autoGridItem, *emptyGridArea);
</ins><span class="cx">
</span><span class="cx"> if (!isGridAutoFlowDense)
</span><span class="cx"> minorAxisCursors.set(majorAxisInitialPosition, isForColumns ? emptyGridArea->rows.startLine() : emptyGridArea->columns.startLine());
</span><span class="lines">@@ -1853,7 +1857,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> m_gridItemArea.set(&gridItem, *emptyGridArea);
</span><del>- insertItemIntoGrid(gridItem, *emptyGridArea);
</del><ins>+ m_grid.insert(gridItem, *emptyGridArea);
</ins><span class="cx"> autoPlacementCursor.first = emptyGridArea->rows.startLine();
</span><span class="cx"> autoPlacementCursor.second = emptyGridArea->columns.startLine();
</span><span class="cx"> }
</span><span class="lines">@@ -2718,9 +2722,9 @@
</span><span class="cx"> // because not having rows implies that there are no "normal" children (out-of-flow children are
</span><span class="cx"> // not stored in m_grid).
</span><span class="cx"> if (direction == ForRows)
</span><del>- return m_grid.size();
</del><ins>+ return m_grid.numRows();
</ins><span class="cx">
</span><del>- return m_grid.size() ? m_grid[0].size() : GridPositionsResolver::explicitGridColumnCount(style(), m_autoRepeatColumns);
</del><ins>+ return m_grid.numRows() ? m_grid.numColumns() : GridPositionsResolver::explicitGridColumnCount(style(), m_autoRepeatColumns);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void RenderGrid::paintChildren(PaintInfo& paintInfo, const LayoutPoint& paintOffset, PaintInfo& forChild, bool usePrintRect)
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderGridh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderGrid.h (208972 => 208973)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderGrid.h 2016-11-24 13:35:34 UTC (rev 208972)
+++ trunk/Source/WebCore/rendering/RenderGrid.h 2016-11-24 15:08:11 UTC (rev 208973)
</span><span class="lines">@@ -88,9 +88,6 @@
</span><span class="cx"> LayoutUnit computeUsedBreadthOfMaxLength(const GridTrackSize&, LayoutUnit usedBreadth, LayoutUnit maxSize) const;
</span><span class="cx"> void resolveContentBasedTrackSizingFunctions(GridTrackSizingDirection, GridSizingData&) const;
</span><span class="cx">
</span><del>- void ensureGridSize(unsigned maximumRowSize, unsigned maximumColumnSize);
- void insertItemIntoGrid(RenderBox&, const GridArea&);
-
</del><span class="cx"> unsigned computeAutoRepeatTracksCount(GridTrackSizingDirection, SizingOperation) const;
</span><span class="cx">
</span><span class="cx"> typedef ListHashSet<size_t> OrderedTrackIndexSet;
</span><span class="lines">@@ -200,7 +197,30 @@
</span><span class="cx"> bool isOrthogonalChild(const RenderBox&) const;
</span><span class="cx"> GridTrackSizingDirection flowAwareDirectionForChild(const RenderBox&, GridTrackSizingDirection) const;
</span><span class="cx">
</span><del>- Vector<Vector<Vector<RenderBox*, 1>>> m_grid;
</del><ins>+ typedef Vector<RenderBox*, 1> GridCell;
+ typedef Vector<Vector<GridCell>> GridAsMatrix;
+ class Grid final {
+ public:
+ Grid() { }
+
+ unsigned numColumns() const { return m_grid.size() ? m_grid[0].size() : 0; }
+ unsigned numRows() const { return m_grid.size(); }
+
+ void ensureGridSize(unsigned maximumRowSize, unsigned maximumColumnSize);
+ void insert(RenderBox&, const GridArea&);
+
+ const GridCell& cell(unsigned row, unsigned column) const { return m_grid[row][column]; }
+
+ void shrinkToFit() { m_grid.shrinkToFit(); }
+
+ void clear();
+
+ private:
+ friend class GridIterator;
+ GridAsMatrix m_grid;
+ };
+ Grid m_grid;
+
</ins><span class="cx"> Vector<LayoutUnit> m_columnPositions;
</span><span class="cx"> Vector<LayoutUnit> m_rowPositions;
</span><span class="cx"> LayoutUnit m_offsetBetweenColumns;
</span></span></pre>
</div>
</div>
</body>
</html>