Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(228)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2380403002: Use enclosingLayoutRect instead of enclosingIntRect in SVGLayoutSupport::transformPaintInvalidation… (Closed)

Created:
4 years, 2 months ago by Xianzhu
Modified:
4 years, 2 months ago
Reviewers:
pdr., fs
CC:
fs, ajuma+watch_chromium.org, blink-reviews, blink-reviews-layout_chromium.org, blink-reviews-paint_chromium.org, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, eae+blinkwatch, f(malita), gyuyoung2, jbroman, jchaffraix+rendering, Justin Novosad, kouhei+svg_chromium.org, leviw+renderwatch, pdr+graphicswatchlist_chromium.org, pdr+svgwatchlist_chromium.org, pdr+renderingwatchlist_chromium.org, rwlbuis, Stephen Chennney, slimming-paint-reviews_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Use enclosingLayoutRect instead of enclosingIntRect in SVGLayoutSupport::transformPaintInvalidationRect() https://bugs.chromium.org/p/chromium/issues/detail?id=651521#c2: The purpose of the original enclosingIntRect() might be to avoid unwanted inward-snapping due to FloatRect -> LayoutRect conversion. Change it to enclosingLayoutRect. Also changed PaintInvalidator to use enclosingLayoutRect when converting from FloatRect to LayoutRect. BUG=651521 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Committed: https://crrev.com/97024f931ff54c93b9b0633c3406b4f9ff9779b7 Cr-Commit-Position: refs/heads/master@{#422710}

Patch Set 1 #

Patch Set 2 : - #

Total comments: 2

Patch Set 3 : - #

Patch Set 4 : Rebase #

Patch Set 5 : Remove FloatRectTest.cpp from this CL (may create another CL) #

Patch Set 6 : Remove LayoutRectTest.cpp from this CL (may create another CL) #

Patch Set 7 : Rebaseline #

Patch Set 8 : Rebase #

Patch Set 9 : Remove an unrelated test rebaseline #

Unified diffs Side-by-side diffs Delta from patch set Stats (+103 lines, -90 lines) Patch
M third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=slimmingPaintInvalidation View 2 chunks +7 lines, -10 lines 0 comments Download
M third_party/WebKit/LayoutTests/paint/invalidation/svg/absolute-sized-document-no-scrollbars-expected.txt View 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/svg/text-selection-text-05-t-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/small-fonts-in-html5-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-align-01-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-align-02-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-align-03-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-align-04-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-align-05-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-deco-01-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-fonts-01-t-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-fonts-02-t-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-intro-05-t-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-path-01-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-text-03-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-text-04-t-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-text-06-t-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-text-07-t-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-text-08-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-tselect-01-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-tselect-02-f-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-tspan-01-b-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-ws-01-t-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/text/text-selection-ws-02-t-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/transforms/svg-css-transforms-expected.png View Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/transforms/svg-css-transforms-expected.txt View 2 chunks +5 lines, -5 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/zoom/page/zoom-clip-path-expected.txt View 1 chunk +4 lines, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/svg/zoom/page/zoom-svg-float-border-padding-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/linux/transforms/svg-vs-css-expected.txt View 1 3 chunks +6 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac-mac10.10/svg/text/text-selection-fonts-01-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac-mac10.9/svg/text/text-selection-fonts-01-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac-mac10.9/svg/text/text-selection-intro-05-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/svg/text-selection-text-05-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/custom/embedding-external-svgs-expected.txt View 1 1 chunk +10 lines, -10 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/bbox-with-glyph-overflow-zoomed-expected.txt View 1 1 chunk +3 lines, -3 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/small-fonts-in-html5-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-align-01-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-align-02-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-align-05-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-deco-01-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-fonts-01-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-fonts-02-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-intro-05-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-text-03-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-text-04-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-text-06-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-text-07-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-text-08-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-tselect-01-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-tselect-02-f-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-tspan-01-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-ws-01-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/text/text-selection-ws-02-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/transforms/svg-css-transforms-expected.txt View 1 2 chunks +5 lines, -5 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/svg/zoom/page/zoom-svg-float-border-padding-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/transforms/svg-vs-css-expected.txt View 1 3 chunks +6 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/paint/invalidation/svg/text-selection-text-05-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/custom/embedding-external-svgs-expected.txt View 1 1 chunk +10 lines, -10 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/bbox-with-glyph-overflow-zoomed-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/small-fonts-in-html5-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-align-01-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-align-02-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-align-05-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-deco-01-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-fonts-01-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-fonts-02-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-intro-05-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-text-03-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-text-04-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-text-06-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-text-07-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-text-08-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-tselect-01-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-tselect-02-f-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-tspan-01-b-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-ws-01-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/text/text-selection-ws-02-t-expected.png View 1 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/transforms/svg-css-transforms-expected.txt View 1 2 chunks +5 lines, -5 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/svg/zoom/page/zoom-svg-float-border-padding-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/transforms/svg-vs-css-expected.txt View 1 3 chunks +6 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/win7/svg/text/bbox-with-glyph-overflow-zoomed-expected.txt View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/win7/svg/text/text-selection-fonts-01-t-expected.png View 1 2 3 4 5 6 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win7/svg/text/text-selection-intro-05-t-expected.png View 1 2 3 4 5 6 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/platform/win7/svg/text/text-selection-text-08-b-expected.png View 1 2 3 4 5 6 Binary file 0 comments Download
M third_party/WebKit/LayoutTests/svg/zoom/page/zoom-clip-path-expected.txt View 1 1 chunk +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/paint/PaintInvalidator.cpp View 1 2 3 4 5 6 7 6 chunks +18 lines, -7 lines 0 comments Download

Messages

Total messages: 54 (32 generated)
Xianzhu
4 years, 2 months ago (2016-09-30 21:58:14 UTC) #9
fs
lgtm https://codereview.chromium.org/2380403002/diff/20001/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp File third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp (right): https://codereview.chromium.org/2380403002/diff/20001/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp#newcode111 third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp:111: return LayoutRect(enclosingLayoutRect(adjustedRect)); Nit: Drop LayoutRect cast?
4 years, 2 months ago (2016-09-30 22:16:43 UTC) #10
Xianzhu
https://codereview.chromium.org/2380403002/diff/20001/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp File third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp (right): https://codereview.chromium.org/2380403002/diff/20001/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp#newcode111 third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp:111: return LayoutRect(enclosingLayoutRect(adjustedRect)); On 2016/09/30 22:16:43, fs wrote: > Nit: ...
4 years, 2 months ago (2016-09-30 22:19:00 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2380403002/40001
4 years, 2 months ago (2016-10-01 00:22:57 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/271213)
4 years, 2 months ago (2016-10-01 00:34:31 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2380403002/40001
4 years, 2 months ago (2016-10-01 01:23:19 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/271240)
4 years, 2 months ago (2016-10-01 01:34:10 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2380403002/60001
4 years, 2 months ago (2016-10-01 01:46:00 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/271253)
4 years, 2 months ago (2016-10-01 02:00:18 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2380403002/60001
4 years, 2 months ago (2016-10-03 00:07:11 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2380403002/80001
4 years, 2 months ago (2016-10-03 15:43:33 UTC) #31
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/271924)
4 years, 2 months ago (2016-10-03 15:52:41 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2380403002/100001
4 years, 2 months ago (2016-10-03 16:20:24 UTC) #36
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/304028)
4 years, 2 months ago (2016-10-03 19:54:23 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2380403002/120001
4 years, 2 months ago (2016-10-03 23:05:05 UTC) #41
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/304624)
4 years, 2 months ago (2016-10-04 00:41:27 UTC) #43
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2380403002/140001
4 years, 2 months ago (2016-10-04 03:02:30 UTC) #46
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2380403002/160001
4 years, 2 months ago (2016-10-04 03:25:16 UTC) #49
commit-bot: I haz the power
Committed patchset #9 (id:160001)
4 years, 2 months ago (2016-10-04 04:59:17 UTC) #50
commit-bot: I haz the power
Patchset 9 (id:??) landed as https://crrev.com/97024f931ff54c93b9b0633c3406b4f9ff9779b7 Cr-Commit-Position: refs/heads/master@{#422710}
4 years, 2 months ago (2016-10-04 05:01:45 UTC) #52
Xianzhu
A revert of this CL (patchset #9 id:160001) has been created in https://codereview.chromium.org/2410153007/ by wangxianzhu@chromium.org. ...
4 years, 2 months ago (2016-10-12 16:57:55 UTC) #53
Xianzhu
4 years, 2 months ago (2016-10-12 16:58:12 UTC) #54
Message was sent while issue was closed. 
A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/2413783002/ by wangxianzhu@chromium.org.

The reason for reverting is: The CL might be incorrect. See
https://bugs.chromium.org/p/chromium/issues/detail?id=651521#c7 for details..

Powered by Google App Engine
This is Rietveld 408576698