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

Issue 1375793003: Update error-handling for fe*Lighting and feTurbulence (Closed)

Created:
5 years, 2 months ago by fs
Modified:
5 years, 2 months ago
Reviewers:
Stephen White
CC:
fs, blink-reviews, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj, dshwang, drott+blinkwatch_chromium.org, krit, f(malita), gyuyoung2, jbroman, Justin Novosad, kouhei+svg_chromium.org, pdr+graphicswatchlist_chromium.org, pdr+svgwatchlist_chromium.org, rwlbuis, Stephen Chennney
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Update error-handling for fe*Lighting and feTurbulence Invalid parameters now produce transparent black rather than failing the build step. The new behavior matches Gecko for fe*Lighting but not for feTurbulence (where they seem to allow negative baseFrequency values), so in the latter case we're siding with the "Negative values are unsupported" wording in the spec[1]. [1] https://drafts.fxtf.org/filters/#element-attrdef-feturbulence-basefrequency BUG=533457 Committed: https://crrev.com/2cfb6d58a971b3ca3ba404ef4403394d5ccdc68f Cr-Commit-Position: refs/heads/master@{#351817}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+175 lines, -15 lines) Patch
A third_party/WebKit/LayoutTests/css3/filters/effect-reference-lighting-no-light.html View 1 chunk +28 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/css3/filters/effect-reference-lighting-no-light-expected.html View 1 chunk +11 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/css3/filters/effect-reference-turbulence-invalid.html View 1 chunk +36 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/css3/filters/effect-reference-turbulence-invalid-expected.html View 1 chunk +13 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/svg/filters/feDiffuseLighting-no-light.html View 1 chunk +12 lines, -0 lines 0 comments Download
A + third_party/WebKit/LayoutTests/svg/filters/feDiffuseLighting-no-light-expected.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A third_party/WebKit/LayoutTests/svg/filters/feSpecularLight-no-light.html View 1 chunk +12 lines, -0 lines 0 comments Download
A + third_party/WebKit/LayoutTests/svg/filters/feSpecularLight-no-light-expected.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A third_party/WebKit/LayoutTests/svg/filters/feTurbulence-negative-basefreq.html View 1 chunk +28 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/svg/filters/feTurbulence-negative-basefreq-expected.html View 1 chunk +7 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGFEDiffuseLightingElement.cpp View 2 chunks +7 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGFESpecularLightingElement.cpp View 2 chunks +9 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGFETurbulenceElement.cpp View 2 chunks +8 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/filters/FELighting.cpp View 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/filters/FETurbulence.cpp View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
fs
Last spec-based error-handling change (I hope...)
5 years, 2 months ago (2015-10-01 13:46:44 UTC) #2
Stephen White
LGTM. Thanks very much for cleaning up all these edge cases!
5 years, 2 months ago (2015-10-01 13:55:42 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1375793003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1375793003/1
5 years, 2 months ago (2015-10-01 13:57:59 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 2 months ago (2015-10-01 16:34:15 UTC) #6
commit-bot: I haz the power
5 years, 2 months ago (2015-10-01 16:35:10 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/2cfb6d58a971b3ca3ba404ef4403394d5ccdc68f
Cr-Commit-Position: refs/heads/master@{#351817}

Powered by Google App Engine
This is Rietveld 408576698