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

Issue 181943003: Scaling and offset fix for FELighting (software and skia paths) (Closed)

Created:
6 years, 9 months ago by sugoi1
Modified:
6 years, 9 months ago
CC:
blink-reviews, jamesr, krit, dsinclair, jbroman, danakj, Rik, Stephen Chennney, pdr., rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Scaling and offset fix for FELighting (software and skia paths) This fixes scaling (including z-scaling) caused by page zoom and high dpi settings (pinch zoom still todo). This also fixes the light position, since the light origin was misplaced when a non 0 filter effect region x and y parameters were used (which is the case by default). The origin was affected by these values, which shouldn't be the case, as these provide extra space to render an effect, like a blur, for example, but should not changed the effect's coordinate system. BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168526

Patch Set 1 #

Total comments: 6

Patch Set 2 : Fixed related Layout Tests to avoid rebaselining #

Total comments: 2

Patch Set 3 : Changed fix to temporary light creation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+167 lines, -125 lines) Patch
M LayoutTests/svg/dynamic-updates/SVGFESpotLightElement-dom-pointsAtX-attr-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/SVGFESpotLightElement-dom-pointsAtY-attr-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/SVGFESpotLightElement-dom-x-attr-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/SVGFESpotLightElement-dom-y-attr-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/SVGFESpotLightElement-svgdom-pointsAtX-prop-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/SVGFESpotLightElement-svgdom-pointsAtY-prop-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/SVGFESpotLightElement-svgdom-x-prop-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/SVGFESpotLightElement-svgdom-y-prop-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-dom-limitingConeAngle-attr.js View 1 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-dom-pointsAtX-attr.js View 1 2 chunks +7 lines, -7 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-dom-pointsAtY-attr.js View 1 2 chunks +7 lines, -7 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-dom-pointsAtZ-attr.js View 1 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-dom-specularExponent-attr.js View 1 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-dom-x-attr.js View 1 2 chunks +7 lines, -7 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-dom-y-attr.js View 1 2 chunks +7 lines, -7 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-dom-z-attr.js View 1 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-svgdom-limitingConeAngle-prop.js View 1 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-svgdom-pointsAtX-prop.js View 1 2 chunks +7 lines, -7 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-svgdom-pointsAtY-prop.js View 1 2 chunks +7 lines, -7 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-svgdom-pointsAtZ-prop.js View 1 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-svgdom-specularExponent-prop.js View 1 1 chunk +4 lines, -4 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-svgdom-x-prop.js View 1 2 chunks +7 lines, -7 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-svgdom-y-prop.js View 1 2 chunks +7 lines, -7 lines 0 comments Download
M LayoutTests/svg/dynamic-updates/script-tests/SVGFESpotLightElement-svgdom-z-prop.js View 1 1 chunk +4 lines, -4 lines 0 comments Download
M Source/platform/graphics/cpu/arm/filters/FELightingNEON.h View 1 2 2 chunks +5 lines, -5 lines 0 comments Download
M Source/platform/graphics/filters/DistantLightSource.h View 1 2 1 chunk +7 lines, -2 lines 0 comments Download
M Source/platform/graphics/filters/DistantLightSource.cpp View 1 2 chunks +2 lines, -2 lines 0 comments Download
M Source/platform/graphics/filters/FELighting.h View 1 2 2 chunks +3 lines, -0 lines 0 comments Download
M Source/platform/graphics/filters/FELighting.cpp View 1 2 3 chunks +21 lines, -2 lines 0 comments Download
M Source/platform/graphics/filters/LightSource.h View 1 2 1 chunk +4 lines, -2 lines 0 comments Download
M Source/platform/graphics/filters/PointLightSource.h View 1 2 1 chunk +8 lines, -2 lines 0 comments Download
M Source/platform/graphics/filters/PointLightSource.cpp View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/platform/graphics/filters/SpotLightSource.h View 1 2 2 chunks +9 lines, -2 lines 0 comments Download
M Source/platform/graphics/filters/SpotLightSource.cpp View 1 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
sugoi1
There you go, this is a first version of the fix. Seems to work properly ...
6 years, 9 months ago (2014-02-28 19:38:04 UTC) #1
sugoi1
Oh yeah, one more thing : I haven't tested the NEON path and that should ...
6 years, 9 months ago (2014-02-28 19:39:53 UTC) #2
Stephen White
https://codereview.chromium.org/181943003/diff/1/Source/platform/graphics/filters/DistantLightSource.h File Source/platform/graphics/filters/DistantLightSource.h (right): https://codereview.chromium.org/181943003/diff/1/Source/platform/graphics/filters/DistantLightSource.h#newcode44 Source/platform/graphics/filters/DistantLightSource.h:44: virtual void updatePaintingData(PaintingData&, float x, float y, float z) ...
6 years, 9 months ago (2014-03-03 17:55:09 UTC) #3
sugoi1
New version with layout tests fixed. https://codereview.chromium.org/181943003/diff/1/Source/platform/graphics/filters/DistantLightSource.h File Source/platform/graphics/filters/DistantLightSource.h (right): https://codereview.chromium.org/181943003/diff/1/Source/platform/graphics/filters/DistantLightSource.h#newcode44 Source/platform/graphics/filters/DistantLightSource.h:44: virtual void updatePaintingData(PaintingData&, ...
6 years, 9 months ago (2014-03-03 20:29:30 UTC) #4
Stephen White
https://codereview.chromium.org/181943003/diff/20001/Source/platform/graphics/filters/FELighting.cpp File Source/platform/graphics/filters/FELighting.cpp (right): https://codereview.chromium.org/181943003/diff/20001/Source/platform/graphics/filters/FELighting.cpp#newcode190 Source/platform/graphics/filters/FELighting.cpp:190: float transformedLightX = (static_cast<float>(lightX) + data.originOffset.width()) / data.worldScale.x(); Nit: ...
6 years, 9 months ago (2014-03-03 21:00:56 UTC) #5
sugoi1
Here's a new version that shouldn't affect the per pixel performance of the algorithm. https://codereview.chromium.org/181943003/diff/20001/Source/platform/graphics/filters/FELighting.cpp ...
6 years, 9 months ago (2014-03-04 14:45:56 UTC) #6
Stephen White
LGTM
6 years, 9 months ago (2014-03-04 15:01:12 UTC) #7
sugoi1
The CQ bit was checked by sugoi@chromium.org
6 years, 9 months ago (2014-03-04 15:03:30 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sugoi@chromium.org/181943003/40001
6 years, 9 months ago (2014-03-04 15:03:41 UTC) #9
commit-bot: I haz the power
6 years, 9 months ago (2014-03-05 20:04:55 UTC) #10
Message was sent while issue was closed.
Change committed as 168526

Powered by Google App Engine
This is Rietveld 408576698