Chromium Code Reviews

Issue 307643003: Don't render empty shapes with non-scaling-stroke (Closed)

Created:
6 years, 6 months ago by fs
Modified:
6 years, 6 months ago
Reviewers:
pdr.
CC:
blink-reviews, blink-reviews-rendering, zoltan1, eae+blinkwatch, leviw+renderwatch, kouhei+svg_chromium.org, ed+blinkwatch_opera.com, krit, f(malita), gyuyoung.kim_webkit.org, jchaffraix+rendering, pdr., rwlbuis, Stephen Chennney, rune+blink
Visibility:
Public.

Description

Don't render empty shapes with non-scaling-stroke Compute and validate the shape parameters before deciding which code-path should be used (path vs. primitive). Don't use the fallback when the bounding box is empty. Based on WebKit patch: https://bugs.webkit.org/attachment.cgi?id=232191&action=prettypatch by: Nikos Andronikos <nikos.andronikos-webkit@cisra.canon.com.au>; BUG=377610 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175147

Patch Set 1 #

Patch Set 2 : Update bbox tests. #

Total comments: 2

Patch Set 3 : Use isEmpty where applicable. #

Patch Set 4 : Update TEs. #

Unified diffs Side-by-side diffs Stats (+194 lines, -141 lines)
M LayoutTests/TestExpectations View 1 chunk +4 lines, -0 lines 0 comments
M LayoutTests/svg/custom/getBBox-circle-has-one-zero-value.html View 1 chunk +38 lines, -39 lines 0 comments
D LayoutTests/svg/custom/getBBox-circle-has-one-zero-value-expected.txt View 1 chunk +0 lines, -1 line 0 comments
M LayoutTests/svg/custom/getBBox-ellipse-has-one-zero-value.html View 1 chunk +38 lines, -39 lines 0 comments
D LayoutTests/svg/custom/getBBox-ellipse-has-one-zero-value-expected.txt View 1 chunk +0 lines, -1 line 0 comments
M LayoutTests/svg/custom/getBBox-rect-has-one-zero-value.html View 1 chunk +70 lines, -41 lines 0 comments
D LayoutTests/svg/custom/getBBox-rect-has-one-zero-value-expected.txt View 1 chunk +0 lines, -1 line 0 comments
A LayoutTests/svg/custom/non-scaling-stroke-on-empty-shapes.html View 1 chunk +17 lines, -0 lines 0 comments
A LayoutTests/svg/custom/non-scaling-stroke-on-empty-shapes-expected.html View 1 chunk +3 lines, -0 lines 0 comments
M Source/core/rendering/svg/RenderSVGEllipse.cpp View 1 chunk +11 lines, -9 lines 0 comments
M Source/core/rendering/svg/RenderSVGRect.cpp View 1 chunk +13 lines, -10 lines 0 comments

Messages

Total messages: 9 (0 generated)
fs
6 years, 6 months ago (2014-05-28 16:32:32 UTC) #1
Erik Dahlström (inactive)
There's a typo in the description: s/renderer/render/
6 years, 6 months ago (2014-05-28 16:38:58 UTC) #2
fs
On 2014/05/28 16:38:58, Erik Dahlström wrote: > There's a typo in the description: s/renderer/render/ Corrected.
6 years, 6 months ago (2014-05-28 16:53:36 UTC) #3
fs
Since this modifies (fixes) what getBBox returns in some cases, I updated (uhm, or, rewrote) ...
6 years, 6 months ago (2014-05-28 17:03:59 UTC) #4
pdr.
LGTM https://codereview.chromium.org/307643003/diff/20001/Source/core/rendering/svg/RenderSVGEllipse.cpp File Source/core/rendering/svg/RenderSVGEllipse.cpp (right): https://codereview.chromium.org/307643003/diff/20001/Source/core/rendering/svg/RenderSVGEllipse.cpp#newcode62 Source/core/rendering/svg/RenderSVGEllipse.cpp:62: if (m_radii.width() > 0 && m_radii.height() > 0) ...
6 years, 6 months ago (2014-05-28 18:21:41 UTC) #5
fs
https://codereview.chromium.org/307643003/diff/20001/Source/core/rendering/svg/RenderSVGEllipse.cpp File Source/core/rendering/svg/RenderSVGEllipse.cpp (right): https://codereview.chromium.org/307643003/diff/20001/Source/core/rendering/svg/RenderSVGEllipse.cpp#newcode62 Source/core/rendering/svg/RenderSVGEllipse.cpp:62: if (m_radii.width() > 0 && m_radii.height() > 0) { ...
6 years, 6 months ago (2014-05-30 07:43:45 UTC) #6
fs
The CQ bit was checked by fs@opera.com
6 years, 6 months ago (2014-05-30 14:34:23 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fs@opera.com/307643003/50001
6 years, 6 months ago (2014-05-30 14:34:49 UTC) #8
commit-bot: I haz the power
6 years, 6 months ago (2014-05-30 15:58:09 UTC) #9
Message was sent while issue was closed. 
Change committed as 175147

Powered by Google App Engine