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

Issue 1158323005: Include 'outline' in LayoutSVGBlock::visualOverflowRect (Closed)

Created:
5 years, 6 months ago by fs
Modified:
5 years, 6 months ago
Reviewers:
chrishtr, pdr.
CC:
blink-reviews, blink-reviews-rendering, krit, eae+blinkwatch, f(malita), gyuyoung2, jchaffraix+rendering, kouhei+svg_chromium.org, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, pdr+svgwatchlist_chromium.org, rwlbuis, Stephen Chennney, szager+layoutwatch_chromium.org, zoltan1
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Include 'outline' in LayoutSVGBlock::visualOverflowRect LayoutSVGBlock doesn't maintain overflow via LayoutBox::m_overflow, but rather overrides LayoutBox::visualOverflowRect. Instead of doing that, add visual effect overflow during layout of LayoutSVGText and stop overriding said method. This will include outsets for border-image (which doesn't apply to any of <text> or <foreignObject>) too in the unlikely event it's specified. TEST=svg/text/text-outline-2.html (S.P. strict cull rect) BUG=495368 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196591

Patch Set 1 #

Total comments: 3

Patch Set 2 : Move fix to layout-side. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -13 lines) Patch
M Source/core/layout/svg/LayoutSVGBlock.h View 1 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/layout/svg/LayoutSVGBlock.cpp View 1 1 chunk +0 lines, -10 lines 0 comments Download
M Source/core/layout/svg/LayoutSVGText.cpp View 1 2 chunks +3 lines, -1 line 2 comments Download

Messages

Total messages: 20 (2 generated)
fs
5 years, 6 months ago (2015-06-02 15:30:46 UTC) #2
chrishtr
You should also be able to test this CL in a similar way to https://codereview.chromium.org/1158193011 ...
5 years, 6 months ago (2015-06-02 15:51:05 UTC) #3
fs
On 2015/06/02 15:51:05, chrishtr wrote: > You should also be able to test this CL ...
5 years, 6 months ago (2015-06-02 16:23:06 UTC) #4
chrishtr
On 2015/06/02 at 16:23:06, fs wrote: > On 2015/06/02 15:51:05, chrishtr wrote: > > You ...
5 years, 6 months ago (2015-06-02 16:27:46 UTC) #5
fs
On 2015/06/02 16:27:46, chrishtr wrote: > On 2015/06/02 at 16:23:06, fs wrote: > > On ...
5 years, 6 months ago (2015-06-02 16:39:58 UTC) #6
chrishtr
On 2015/06/02 at 16:39:58, fs wrote: > On 2015/06/02 16:27:46, chrishtr wrote: > > On ...
5 years, 6 months ago (2015-06-02 16:49:45 UTC) #7
fs
On 2015/06/02 16:49:45, chrishtr wrote: > On 2015/06/02 at 16:39:58, fs wrote: > > On ...
5 years, 6 months ago (2015-06-02 18:28:11 UTC) #8
chrishtr
On 2015/06/02 at 18:28:11, fs wrote: > On 2015/06/02 16:49:45, chrishtr wrote: > > On ...
5 years, 6 months ago (2015-06-02 20:48:05 UTC) #9
fs
On 2015/06/02 20:48:05, chrishtr wrote: > On 2015/06/02 at 18:28:11, fs wrote: > > On ...
5 years, 6 months ago (2015-06-03 13:58:30 UTC) #10
chrishtr
On 2015/06/03 at 13:58:30, fs wrote: > On 2015/06/02 20:48:05, chrishtr wrote: > > On ...
5 years, 6 months ago (2015-06-03 16:41:22 UTC) #11
fs
On 2015/06/03 16:41:22, chrishtr wrote: > On 2015/06/03 at 13:58:30, fs wrote: > > On ...
5 years, 6 months ago (2015-06-03 16:56:04 UTC) #12
chrishtr
https://codereview.chromium.org/1158323005/diff/1/Source/core/layout/svg/LayoutSVGBlock.cpp File Source/core/layout/svg/LayoutSVGBlock.cpp (right): https://codereview.chromium.org/1158323005/diff/1/Source/core/layout/svg/LayoutSVGBlock.cpp#newcode39 Source/core/layout/svg/LayoutSVGBlock.cpp:39: LayoutRect LayoutSVGBlock::visualOverflowRect() const Is this only ever used in ...
5 years, 6 months ago (2015-06-03 22:12:54 UTC) #13
fs
I've transplanted the fix to the layout-side, PTAL. https://codereview.chromium.org/1158323005/diff/1/Source/core/layout/svg/LayoutSVGBlock.cpp File Source/core/layout/svg/LayoutSVGBlock.cpp (right): https://codereview.chromium.org/1158323005/diff/1/Source/core/layout/svg/LayoutSVGBlock.cpp#newcode39 Source/core/layout/svg/LayoutSVGBlock.cpp:39: LayoutRect ...
5 years, 6 months ago (2015-06-04 09:13:31 UTC) #14
chrishtr
https://codereview.chromium.org/1158323005/diff/20001/Source/core/layout/svg/LayoutSVGText.cpp File Source/core/layout/svg/LayoutSVGText.cpp (right): https://codereview.chromium.org/1158323005/diff/20001/Source/core/layout/svg/LayoutSVGText.cpp#newcode400 Source/core/layout/svg/LayoutSVGText.cpp:400: addVisualEffectOverflow(); So now LayoutSVGBlock inherits behavior from LayoutBlockFlow and ...
5 years, 6 months ago (2015-06-04 16:57:03 UTC) #15
fs
https://codereview.chromium.org/1158323005/diff/20001/Source/core/layout/svg/LayoutSVGText.cpp File Source/core/layout/svg/LayoutSVGText.cpp (right): https://codereview.chromium.org/1158323005/diff/20001/Source/core/layout/svg/LayoutSVGText.cpp#newcode400 Source/core/layout/svg/LayoutSVGText.cpp:400: addVisualEffectOverflow(); On 2015/06/04 16:57:03, chrishtr wrote: > So now ...
5 years, 6 months ago (2015-06-05 09:01:47 UTC) #16
chrishtr
lgtm
5 years, 6 months ago (2015-06-05 16:37:47 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1158323005/20001
5 years, 6 months ago (2015-06-05 16:38:55 UTC) #19
commit-bot: I haz the power
5 years, 6 months ago (2015-06-05 17:39:04 UTC) #20
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196591

Powered by Google App Engine
This is Rietveld 408576698