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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 956553004: Use Length for baselineShiftValue in SVGLayoutStyle (Closed)

Created:
5 years, 10 months ago by fs
Modified:
5 years, 10 months ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-animation_chromium.org, blink-reviews-css, blink-reviews-rendering, blink-reviews-style_chromium.org, dglazkov+blink, Dominik Röttsches, krit, dstockwell, eae+blinkwatch, ed+blinkwatch_opera.com, Eric Willigers, f(malita), fs, gyuyoung.kim_webkit.org, jchaffraix+rendering, kouhei+svg_chromium.org, leviw+renderwatch, Mike Lawther (Google), pdr+svgwatchlist_chromium.org, pdr+renderingwatchlist_chromium.org, rjwright, rwlbuis, Stephen Chennney, shans, Steve Block, Timothy Loh, zoltan1
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Use Length for baselineShiftValue in SVGLayoutStyle This replaces the SVGLength used for baselineShiftValue in SVGLayoutStyle. The value is stored in zoom-adjusted form, and is then "unzoomed" by the client. This means that very little needs to be changed outside of SVG layout code. Also update the handling of the 'baseline' value to compute it to '0px' per the current css-inline draft [1]. This simplifies the animation handling slightly. [1] http://dev.w3.org/csswg/css-inline/#baseline-shift-property BUG=461375 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190903

Patch Set 1 #

Patch Set 2 : Drop SVGLengthContext changes for now. #

Total comments: 2

Patch Set 3 : Fix animations. #

Patch Set 4 : A few more TEs. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+182 lines, -200 lines) Patch
M LayoutTests/animations/interpolation/svg-baseline-shift-interpolation.html View 1 2 2 chunks +13 lines, -13 lines 0 comments Download
M LayoutTests/animations/interpolation/svg-baseline-shift-interpolation-expected.txt View 1 2 2 chunks +24 lines, -24 lines 0 comments Download
M LayoutTests/fast/css/getComputedStyle/computed-style-expected.txt View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/css/getComputedStyle/computed-style-without-renderer-expected.txt View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/svg/css/baseline-shift-inherit.html View 1 2 1 chunk +9 lines, -3 lines 0 comments Download
M LayoutTests/svg/css/getComputedStyle-basic-expected.txt View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M LayoutTests/svg/css/scientific-numbers-expected.txt View 1 chunk +86 lines, -86 lines 0 comments Download
M LayoutTests/svg/css/script-tests/scientific-numbers.js View 1 chunk +6 lines, -6 lines 0 comments Download
M Source/core/animation/SVGLengthStyleInterpolation.cpp View 1 2 3 chunks +3 lines, -18 lines 0 comments Download
M Source/core/animation/StringKeyframe.cpp View 1 2 3 chunks +5 lines, -1 line 0 comments Download
M Source/core/animation/css/CSSAnimatableValueFactory.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/animation/css/CSSPropertyEquality.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/LayoutStyleCSSValueMapping.cpp View 1 2 1 chunk +1 line, -3 lines 0 comments Download
M Source/core/css/resolver/AnimatedStyleBuilder.cpp View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/css/resolver/StyleBuilderCustom.cpp View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M Source/core/layout/style/LayoutStyle.h View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/layout/style/SVGLayoutStyle.h View 1 2 4 chunks +6 lines, -12 lines 0 comments Download
M Source/core/layout/style/SVGLayoutStyle.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/style/SVGLayoutStyleDefs.h View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/layout/style/SVGLayoutStyleDefs.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/layout/style/SVGLayoutStyleTest.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/svg/SVGTextLayoutEngine.cpp View 2 chunks +1 line, -3 lines 0 comments Download
M Source/core/layout/svg/SVGTextLayoutEngineBaseline.h View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/layout/svg/SVGTextLayoutEngineBaseline.cpp View 1 2 2 chunks +7 lines, -12 lines 2 comments Download

Messages

Total messages: 11 (3 generated)
Erik Dahlström (inactive)
https://codereview.chromium.org/956553004/diff/10019/LayoutTests/svg/css/script-tests/scientific-numbers.js File LayoutTests/svg/css/script-tests/scientific-numbers.js (left): https://codereview.chromium.org/956553004/diff/10019/LayoutTests/svg/css/script-tests/scientific-numbers.js#oldcode14 LayoutTests/svg/css/script-tests/scientific-numbers.js:14: text.setAttribute("baseline-shift", "baseline"); Is there another test for making sure ...
5 years, 10 months ago (2015-02-25 09:50:55 UTC) #2
pdr.
https://codereview.chromium.org/956553004/diff/50001/Source/core/layout/svg/SVGTextLayoutEngineBaseline.cpp File Source/core/layout/svg/SVGTextLayoutEngineBaseline.cpp (right): https://codereview.chromium.org/956553004/diff/50001/Source/core/layout/svg/SVGTextLayoutEngineBaseline.cpp#newcode45 Source/core/layout/svg/SVGTextLayoutEngineBaseline.cpp:45: return floatValueForLength(svgStyle.baselineShiftValue(), m_font.fontDescription().computedPixelSize() * zoom) / zoom; What do ...
5 years, 10 months ago (2015-02-25 19:21:15 UTC) #4
fs
@ericwilligers: Could you take a look at the animation changes? https://codereview.chromium.org/956553004/diff/10019/LayoutTests/svg/css/script-tests/scientific-numbers.js File LayoutTests/svg/css/script-tests/scientific-numbers.js (left): https://codereview.chromium.org/956553004/diff/10019/LayoutTests/svg/css/script-tests/scientific-numbers.js#oldcode14 ...
5 years, 10 months ago (2015-02-25 21:21:12 UTC) #5
pdr.
LGTM here! Could you wait until tomorrow your time so ed and ericwilligers (UTC+10:00) have ...
5 years, 10 months ago (2015-02-25 21:47:51 UTC) #6
Eric Willigers
> @ericwilligers: Could you take a look at the animation changes? LGTM. I'll retire the ...
5 years, 10 months ago (2015-02-26 05:28:23 UTC) #7
Erik Dahlström (inactive)
lgtm, the baseline-shift as attribute is covered by the existing svg tests AFAICT.
5 years, 10 months ago (2015-02-26 08:20:45 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/956553004/50001
5 years, 10 months ago (2015-02-26 10:28:29 UTC) #10
commit-bot: I haz the power
5 years, 10 months ago (2015-02-26 10:31:43 UTC) #11
Message was sent while issue was closed. 
Committed patchset #4 (id:50001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190903

Powered by Google App Engine
This is Rietveld 408576698