Chromium Code Reviews
|
|
|
Created:
4 years, 11 months ago by Xianzhu Modified:
4 years, 11 months ago CC:
blink-reviews, blink-reviews-paint_chromium.org, dshwang, slimming-paint-reviews_chromium.org Base URL:
svn://svn.chromium.org/blink/trunk Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
DescriptionLet cull rect of collapsed border contain all painted area
BUG=496823
TEST=run-webkit-tests --additional-driver-flags=SlimmingPaintStrictCullRectClipping fast/table/border-collapsing
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196588
Patch Set 1 #
Total comments: 5
Messages
Total messages: 14 (4 generated)
wangxianzhu@chromium.org changed reviewers: + chrishtr@chromium.org, pdr@chromium.org
https://codereview.chromium.org/1168643006/diff/1/Source/core/paint/TableCell... File Source/core/paint/TableCellPainter.cpp (left): https://codereview.chromium.org/1168643006/diff/1/Source/core/paint/TableCell... Source/core/paint/TableCellPainter.cpp:80: if (drawingCullRect.y() >= paintInfo.rect.maxY()) You're removing a non-SP culling optimization. Was it previously broken?
https://codereview.chromium.org/1168643006/diff/1/Source/core/paint/TableCell... File Source/core/paint/TableCellPainter.cpp (left): https://codereview.chromium.org/1168643006/diff/1/Source/core/paint/TableCell... Source/core/paint/TableCellPainter.cpp:80: if (drawingCullRect.y() >= paintInfo.rect.maxY()) On 2015/06/04 22:21:06, chrishtr wrote: > You're removing a non-SP culling optimization. Was it previously broken? The cullrect was incorrect (which doesn't cover all collapsed border if the used collapsed border is wider than the table's border). We should use borderRect instead. New line 105 is the new culling optimization.
https://codereview.chromium.org/1168643006/diff/1/Source/core/paint/TableCell... File Source/core/paint/TableCellPainter.cpp (right): https://codereview.chromium.org/1168643006/diff/1/Source/core/paint/TableCell... Source/core/paint/TableCellPainter.cpp:99: LayoutRect paintRect = paintBounds(paintOffset, AddOffsetFromParent); This is not used? Why not fix paintBounds?
https://codereview.chromium.org/1168643006/diff/1/Source/core/paint/TableCell... File Source/core/paint/TableCellPainter.cpp (right): https://codereview.chromium.org/1168643006/diff/1/Source/core/paint/TableCell... Source/core/paint/TableCellPainter.cpp:99: LayoutRect paintRect = paintBounds(paintOffset, AddOffsetFromParent); On 2015/06/05 00:31:06, chrishtr wrote: > This is not used? > > Why not fix paintBounds? Here paintRect is used to calculate borderRect below. The result of paintBounds() is for other paintings, not including collapsed borders.
The CQ bit was checked by chrishtr@chromium.org
lgtm https://codereview.chromium.org/1168643006/diff/1/Source/core/paint/TableCell... File Source/core/paint/TableCellPainter.cpp (right): https://codereview.chromium.org/1168643006/diff/1/Source/core/paint/TableCell... Source/core/paint/TableCellPainter.cpp:99: LayoutRect paintRect = paintBounds(paintOffset, AddOffsetFromParent); On 2015/06/05 at 00:40:58, Xianzhu wrote: > On 2015/06/05 00:31:06, chrishtr wrote: > > This is not used? > > > > Why not fix paintBounds? > > Here paintRect is used to calculate borderRect below. Oh right, sorry. My comment was too hasty. > The result of paintBounds() is for other paintings, not including collapsed borders.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1168643006/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/65420)
The CQ bit was checked by wangxianzhu@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1168643006/1
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://src.chromium.org/viewvc/blink?view=rev&revision=196588 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
