New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 626748 link

Starred by 3 users
Status: Fixed
Owner:
wangxianzhu@chromium.org
Closed: May 2017
Cc:
█ trchen@chromium.org
pdr@chromium.org
wangxianzhu@chromium.org
schenney@chromium.org
chrishtr@chromium.org
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 619103



Sign in to add a comment

Sometimes under-invalidation of collapsed table borders

Project Member Reported by wangxianzhu@chromium.org, Jul 8 2016 
fast/table/border-collapsing/cached-change-row-border-color.html
fast/table/border-collapsing/cached-change-tbody-border-color.html

The invalidation rects doesn't cover all changed collapsed borders.

Invalidation rects are also incorrect if the cell is transformed or has composited layer.

Fixing the failing tests might be easy by expanding the visual overflow rect of cells to include the collapsed borders.

The issues of transform and composited layer seem not easy. The clean solution may need the new paint invalidation based on paint property tree.

Comment 1 by wangxianzhu@chromium.org, Jul 8 2016

Blocking: 619103
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 30 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eff95516605a04f45c09c603fc247d0da414a003

commit eff95516605a04f45c09c603fc247d0da414a003
Author: schenney <schenney@chromium.org>
Date: Thu Mar 30 16:20:41 2017

Clean up Paint Team TestExpectations

Move a ref test to a block for failing ref tests.

TBR=wangxianzhu@chromium.org
BUG= 626748 

Review-Url: https://codereview.chromium.org/2777183008
Cr-Commit-Position: refs/heads/master@{#460787}

[modify] https://crrev.com/eff95516605a04f45c09c603fc247d0da414a003/third_party/WebKit/LayoutTests/TestExpectations

Comment 3 by wangxianzhu@chromium.org, Apr 21 2017

Cc: schenney@chromium.org wangxianzhu@chromium.org
 Issue 680211  has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 27 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb2388b6cee45ca8cba764245b2886727b3fafb8

commit bb2388b6cee45ca8cba764245b2886727b3fafb8
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Thu Apr 27 15:51:46 2017

Don't always invalidate collapsed borders during table layout

Many layout changes e.g. resizing doesn't affect computed collapsed
borders so don't need to invalidate collapsed borders.

We need to invalidate collapsed borders when
1. Table sections are set need recalc (when table structure changes);
2. A cell is appended into a row (which could belong to 1 but we have an
   optimization not to recalc table sections if the added cell is the
   last cell of the table);
3. Border style changes;
4. border-collapse CSS property changes;

This CL will reduce frame time of PerformanceTests/Mutation/large-
table-row-height-change-with-collapsed-border.html (in
https://codereview.chromium.org/2842313002/) by about 35% by avoiding
unnecessary collapsed border recalculations.

BUG= 626748 

Review-Url: https://codereview.chromium.org/2840723005
Cr-Commit-Position: refs/heads/master@{#467694}

[modify] https://crrev.com/bb2388b6cee45ca8cba764245b2886727b3fafb8/third_party/WebKit/Source/core/layout/LayoutTable.cpp
[modify] https://crrev.com/bb2388b6cee45ca8cba764245b2886727b3fafb8/third_party/WebKit/Source/core/layout/LayoutTable.h
[modify] https://crrev.com/bb2388b6cee45ca8cba764245b2886727b3fafb8/third_party/WebKit/Source/core/layout/LayoutTableRow.cpp
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c731464f86c6a5c5c3f2b64890e04b38750896fc

commit c731464f86c6a5c5c3f2b64890e04b38750896fc
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Fri Apr 28 23:14:06 2017

Optimize collapsed border calculation (step 1)

Previously we didn't cache results of collapsed borders computation
during layout or overflow recalculation, so we had to recompute all
collapsed borders before paint invalidation if the table's collapsed
borders are marked invalid.

Now when a table's collapsed borders need to be invalidated, mark all
cells' collapsed borders need to be invalidated [1]. When a cell needs
its collapsed border values (regardless of lifecycle phase), we check
the invalidation flag and update and cache the result when necessary.

As LayoutTableCell::CollapsedBorderValues will be used for both layout
and paint, the include_color paremeter of ComputeCollapsedXXXBorder()
is removed.

As adjacent cells with the same span share collapsed borders, we can
also get the collapsed border from the adjacent cell if its collapsed
borders are valid.

[1] In the next step, we'll only invalidate collapsed borders for
affected cells instead of all cells.

BUG= 626748 , 663208 

Review-Url: https://codereview.chromium.org/2846563002
Cr-Commit-Position: refs/heads/master@{#468174}

[modify] https://crrev.com/c731464f86c6a5c5c3f2b64890e04b38750896fc/third_party/WebKit/Source/core/layout/LayoutTable.cpp
[modify] https://crrev.com/c731464f86c6a5c5c3f2b64890e04b38750896fc/third_party/WebKit/Source/core/layout/LayoutTable.h
[modify] https://crrev.com/c731464f86c6a5c5c3f2b64890e04b38750896fc/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
[modify] https://crrev.com/c731464f86c6a5c5c3f2b64890e04b38750896fc/third_party/WebKit/Source/core/layout/LayoutTableCell.h
[modify] https://crrev.com/c731464f86c6a5c5c3f2b64890e04b38750896fc/third_party/WebKit/Source/core/layout/LayoutTableCellTest.cpp
Project Member

Comment 8 by bugdroid1@chromium.org, May 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dba4d1702934a03d833c91f2c025faf2106b9c76

commit dba4d1702934a03d833c91f2c025faf2106b9c76
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Tue May 09 09:00:11 2017

Let table row and section's self visual overflow cover whole collapsed borders

This ensures correct raster invalidation when row or section's border
style changes and fix under-invalidation.

The new logic in LayoutTableRow::ComputeOverflow() will be also useful when we
let row be the display item client of collapsed borders ( crbug.com/663208 ).
At that time the new logic in LayoutTableSection::ComputeOverflowFromDescendants()
will be replaced by paint invalidation of contained rows on section border
style change.

BUG= 626748 

Review-Url: https://codereview.chromium.org/2861373003
Cr-Commit-Position: refs/heads/master@{#470268}

[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/paint/invalidation/table-outer-border-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/paint/invalidation/table/border-collapse-change-collapse-to-separate-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/paint/invalidation/table/border-collapse-change-separate-to-collapse-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/paint/invalidation/table/cached-69296-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/paint/invalidation/table/cached-change-col-border-width-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/paint/invalidation/table/cached-change-colgroup-border-width-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/paint/invalidation/table/cached-change-row-border-color-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/paint/invalidation/table/cached-change-row-border-width-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/paint/invalidation/table/cached-change-tbody-border-color-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/paint/invalidation/table/cached-change-tbody-border-width-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/composited-table-row-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/composited-table-row-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/table-collapsed-border-expected.txt
[rename] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/composited-table-row-expected.txt
[copy] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/platform/mac/virtual/disable-spinvalidation/paint/invalidation/composited-table-row-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/platform/mac/virtual/disable-spinvalidation/paint/invalidation/table-collapsed-border-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/composited-table-row-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/paint/invalidation/composited-table-row-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/platform/win/virtual/disable-spinvalidation/paint/invalidation/table-collapsed-border-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/virtual/disable-spinvalidation/paint/invalidation/table-outer-border-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/virtual/disable-spinvalidation/paint/invalidation/table/border-collapse-change-collapse-to-separate-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/virtual/disable-spinvalidation/paint/invalidation/table/border-collapse-change-separate-to-collapse-expected.txt
[add] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/LayoutTests/virtual/disable-spinvalidation/paint/invalidation/table/cached-change-row-border-width-expected.txt
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/Source/core/layout/LayoutTable.cpp
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/Source/core/layout/LayoutTableCell.cpp
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/Source/core/layout/LayoutTableCell.h
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/Source/core/layout/LayoutTableCellTest.cpp
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/Source/core/layout/LayoutTableRow.cpp
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/Source/core/layout/LayoutTableRowTest.cpp
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/Source/core/layout/LayoutTableSection.h
[modify] https://crrev.com/dba4d1702934a03d833c91f2c025faf2106b9c76/third_party/WebKit/Source/core/layout/LayoutTableSectionTest.cpp

Comment 9 by wangxianzhu@chromium.org, May 12 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment