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

Issue 211193002: Compute correct repaint rect for decorated RenderSVGRoots (Closed)

Created:
6 years, 9 months ago by fs
Modified:
6 years, 8 months ago
CC:
blink-reviews, krit, bemjb+rendering_chromium.org, zoltan1, eae+blinkwatch, leviw+renderwatch, kouhei+svg_chromium.org, ed+blinkwatch_opera.com, gyuyoung.kim_webkit.org, jchaffraix+rendering, rwlbuis, rune+blink
Visibility:
Public.

Description

Compute correct repaint rect for decorated RenderSVGRoots The current implementation of clippedOverflowRectForRepaint() uses the generic repaint-rect calculations in SVGRenderSupport. Those in turn make use of repaintRectInLocalCoordinates(), which for RenderSVGRoot is the union of the painted children (w/ some expansion). If there're no children, or they do not fill the entire content box, then a repaint would not repaint the correct parts. Fix by calculating the union of the border-box and the SVG content when the SVG root is decorated (has background/border/etc.) BUG=354305 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170890 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170960

Patch Set 1 #

Patch Set 2 : TestExpectations #

Patch Set 3 : Comment out conflicting TestExpectations. #

Patch Set 4 : Moar rebaselines. #

Patch Set 5 : Rebase. #

Patch Set 6 : And moar rebaselines. #

Patch Set 7 : Rebase (adds additional TE conflict.) #

Total comments: 1

Patch Set 8 : Only repaint the border-box when we think we have to. #

Total comments: 8

Patch Set 9 : Fix outline-repainting; Add more tests. #

Patch Set 10 : Rebase. #

Total comments: 2

Patch Set 11 : Change tests to repaint tests. #

Patch Set 12 : Adjust some -expected (due to RAL?). #

Patch Set 13 : Track has-box-decorations separately. #

Patch Set 14 : Make border tests less RAL/non-RAL sensitive. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+218 lines, -6 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +9 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/add-background-property-on-root.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +19 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/add-background-property-on-root-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +4 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/add-border-property-on-root.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +18 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/add-border-property-on-root-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +4 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/add-outline-property-on-root.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +20 lines, -0 lines 0 comments Download
A + LayoutTests/svg/repaint/add-outline-property-on-root-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -2 lines 0 comments Download
A LayoutTests/svg/repaint/change-background-color.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +20 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/change-background-color-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +5 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/remove-background-property-on-root.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +19 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/remove-background-property-on-root-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +4 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/remove-border-property-on-root.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +18 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/remove-border-property-on-root-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +4 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/remove-outline-property-on-root.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +19 lines, -0 lines 0 comments Download
A LayoutTests/svg/repaint/remove-outline-property-on-root-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +5 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderBoxModelObject.h View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/rendering/RenderBoxModelObject.cpp View 1 2 3 4 5 6 7 8 9 1 chunk +8 lines, -1 line 0 comments Download
M Source/core/rendering/svg/RenderSVGRoot.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGRoot.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 6 chunks +39 lines, -3 lines 0 comments Download

Messages

Total messages: 43 (0 generated)
fs
So this ended up being a fun one... Since this changes what absoluteClippedOverflowRect() returns for ...
6 years, 9 months ago (2014-03-25 14:00:29 UTC) #1
Stephen Chennney
On 2014/03/25 14:00:29, fs wrote: > So this ended up being a fun one... > ...
6 years, 9 months ago (2014-03-25 14:22:49 UTC) #2
fs
On 2014/03/25 14:22:49, Stephen Chenney wrote: > On 2014/03/25 14:00:29, fs wrote: > > So ...
6 years, 9 months ago (2014-03-25 14:37:47 UTC) #3
fs
On 2014/03/25 14:37:47, fs wrote: > On 2014/03/25 14:22:49, Stephen Chenney wrote: > > On ...
6 years, 9 months ago (2014-03-26 15:31:14 UTC) #4
f(malita)
On 2014/03/26 15:31:14, fs wrote: > On 2014/03/25 14:37:47, fs wrote: > > On 2014/03/25 ...
6 years, 9 months ago (2014-03-26 15:38:38 UTC) #5
fs
On 2014/03/26 15:38:38, Florin Malita wrote: > On 2014/03/26 15:31:14, fs wrote: > > On ...
6 years, 9 months ago (2014-03-26 15:58:32 UTC) #6
fs
I hope the TestExpectations are sorted now, so PTAL (at the actual change too! Yes, ...
6 years, 9 months ago (2014-03-26 16:43:31 UTC) #7
fs
On 2014/03/26 16:43:31, fs wrote: > I hope the TestExpectations are sorted now, so PTAL ...
6 years, 9 months ago (2014-03-28 12:00:03 UTC) #8
pdr.
I don't think this change is right, or I need more convincing. This change increases ...
6 years, 8 months ago (2014-03-31 16:51:31 UTC) #9
fs
On 2014/03/31 16:51:31, pdr wrote: > I don't think this change is right, or I ...
6 years, 8 months ago (2014-04-01 12:33:07 UTC) #10
fs
On 2014/04/01 12:33:07, fs wrote: > On 2014/03/31 16:51:31, pdr wrote: > > I don't ...
6 years, 8 months ago (2014-04-01 14:38:03 UTC) #11
pdr.
I think the results of this patch look great--just a handful of tests need updating ...
6 years, 8 months ago (2014-04-01 18:55:47 UTC) #12
pdr.
https://codereview.chromium.org/211193002/diff/220001/Source/core/rendering/svg/RenderSVGRoot.cpp File Source/core/rendering/svg/RenderSVGRoot.cpp (right): https://codereview.chromium.org/211193002/diff/220001/Source/core/rendering/svg/RenderSVGRoot.cpp#newcode373 Source/core/rendering/svg/RenderSVGRoot.cpp:373: // This is an open-coded aggregate of SVGRenderSupport::clippedOverflowRectForRepaint, On ...
6 years, 8 months ago (2014-04-02 06:15:31 UTC) #13
fs
https://codereview.chromium.org/211193002/diff/220001/Source/core/rendering/svg/RenderSVGRoot.cpp File Source/core/rendering/svg/RenderSVGRoot.cpp (right): https://codereview.chromium.org/211193002/diff/220001/Source/core/rendering/svg/RenderSVGRoot.cpp#newcode365 Source/core/rendering/svg/RenderSVGRoot.cpp:365: if (object.isRoot()) On 2014/04/01 18:55:48, pdr wrote: > Should ...
6 years, 8 months ago (2014-04-02 08:01:54 UTC) #14
fs
Fixed repaint w/ outline (not sure that worked previously, hopefully better now) and some other ...
6 years, 8 months ago (2014-04-02 14:04:55 UTC) #15
pdr.
On 2014/04/02 14:04:55, fs wrote: > Fixed repaint w/ outline (not sure that worked previously, ...
6 years, 8 months ago (2014-04-02 19:50:39 UTC) #16
pdr.
On 2014/04/02 08:01:54, fs wrote: > https://codereview.chromium.org/211193002/diff/220001/Source/core/rendering/svg/RenderSVGRoot.cpp > File Source/core/rendering/svg/RenderSVGRoot.cpp (right): > > https://codereview.chromium.org/211193002/diff/220001/Source/core/rendering/svg/RenderSVGRoot.cpp#newcode365 > ...
6 years, 8 months ago (2014-04-02 19:52:36 UTC) #17
Stephen Chennney
On 2014/04/02 19:52:36, pdr wrote: > On 2014/04/02 08:01:54, fs wrote: > > > https://codereview.chromium.org/211193002/diff/220001/Source/core/rendering/svg/RenderSVGRoot.cpp ...
6 years, 8 months ago (2014-04-02 19:59:20 UTC) #18
f(malita)
lgtm https://codereview.chromium.org/211193002/diff/260001/Source/core/rendering/svg/RenderSVGRoot.cpp File Source/core/rendering/svg/RenderSVGRoot.cpp (right): https://codereview.chromium.org/211193002/diff/260001/Source/core/rendering/svg/RenderSVGRoot.cpp#newcode225 Source/core/rendering/svg/RenderSVGRoot.cpp:225: m_overflow.clear(); Not the fault of this CL I ...
6 years, 8 months ago (2014-04-02 20:45:47 UTC) #19
fs
On 2014/04/02 19:59:20, Stephen Chenney wrote: > On 2014/04/02 19:52:36, pdr wrote: > > On ...
6 years, 8 months ago (2014-04-03 07:58:38 UTC) #20
fs
https://codereview.chromium.org/211193002/diff/260001/Source/core/rendering/svg/RenderSVGRoot.cpp File Source/core/rendering/svg/RenderSVGRoot.cpp (right): https://codereview.chromium.org/211193002/diff/260001/Source/core/rendering/svg/RenderSVGRoot.cpp#newcode225 Source/core/rendering/svg/RenderSVGRoot.cpp:225: m_overflow.clear(); On 2014/04/02 20:45:48, Florin Malita wrote: > Not ...
6 years, 8 months ago (2014-04-03 07:58:58 UTC) #21
fs
At a whim I decided to change all tests to (text-based-)repaint tests before landing - ...
6 years, 8 months ago (2014-04-03 09:20:20 UTC) #22
fs
The CQ bit was checked by fs@opera.com
6 years, 8 months ago (2014-04-03 09:20:57 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fs@opera.com/211193002/280001
6 years, 8 months ago (2014-04-03 09:21:07 UTC) #24
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-03 09:55:55 UTC) #25
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_rel
6 years, 8 months ago (2014-04-03 09:55:55 UTC) #26
pdr.
The CQ bit was checked by pdr@chromium.org
6 years, 8 months ago (2014-04-03 17:02:55 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fs@opera.com/211193002/300001
6 years, 8 months ago (2014-04-03 17:03:08 UTC) #28
fs
While trying to come up with a workaround for the 1px difference between the various ...
6 years, 8 months ago (2014-04-03 17:36:47 UTC) #29
pdr.
On 2014/04/03 17:36:47, fs wrote: > While trying to come up with a workaround for ...
6 years, 8 months ago (2014-04-03 20:07:21 UTC) #30
dsinclair
On 2014/04/03 20:07:21, pdr wrote: > On 2014/04/03 17:36:47, fs wrote: > > While trying ...
6 years, 8 months ago (2014-04-04 16:14:46 UTC) #31
pdr.
LGTM as well.
6 years, 8 months ago (2014-04-04 16:28:00 UTC) #32
fs
On 2014/04/04 16:14:46, dsinclair wrote: > On 2014/04/03 20:07:21, pdr wrote: > > On 2014/04/03 ...
6 years, 8 months ago (2014-04-04 16:53:19 UTC) #33
pdr.
On 2014/04/04 16:53:19, fs wrote: > On 2014/04/04 16:14:46, dsinclair wrote: > > On 2014/04/03 ...
6 years, 8 months ago (2014-04-04 17:08:05 UTC) #34
fs
On 2014/04/04 17:08:05, pdr wrote: > On 2014/04/04 16:53:19, fs wrote: > > On 2014/04/04 ...
6 years, 8 months ago (2014-04-04 17:12:24 UTC) #35
fs
The CQ bit was checked by fs@opera.com
6 years, 8 months ago (2014-04-04 17:12:32 UTC) #36
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fs@opera.com/211193002/320001
6 years, 8 months ago (2014-04-04 17:12:34 UTC) #37
commit-bot: I haz the power
Change committed as 170890
6 years, 8 months ago (2014-04-04 18:20:50 UTC) #38
fs
...and what were the odds of this landing at the same time as RAL got ...
6 years, 8 months ago (2014-04-07 10:25:39 UTC) #39
fs
The CQ bit was checked by fs@opera.com
6 years, 8 months ago (2014-04-07 10:25:50 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fs@opera.com/211193002/330001
6 years, 8 months ago (2014-04-07 10:25:58 UTC) #41
fs
The CQ bit was checked by fs@opera.com
6 years, 8 months ago (2014-04-07 10:26:06 UTC) #42
commit-bot: I haz the power
6 years, 8 months ago (2014-04-07 12:54:06 UTC) #43
Message was sent while issue was closed.
Change committed as 170960

Powered by Google App Engine
This is Rietveld 408576698