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

Issue 1041693002: Adjust glyph positions in RTL runs in SVGTextQuery (Closed)

Created:
5 years, 9 months ago by fs
Modified:
5 years, 8 months ago
CC:
blink-reviews, blink-reviews-rendering, Dominik Röttsches, krit, eae+blinkwatch, ed+blinkwatch_opera.com, f(malita), fs, gyuyoung.kim_webkit.org, jchaffraix+rendering, kouhei+svg_chromium.org, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, pdr+svgwatchlist_chromium.org, rwlbuis, Stephen Chennney, zoltan1
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Adjust glyph positions in RTL runs in SVGTextQuery When calculating the glyph position of a glyph in an RTL run/line box, make sure the position is computed from the right of the fragment within the line box. Also adjust calls to SVGTextMetrics::measureCharacterRange to pass the direction of the line box rather than the direction of the layout object. Use the SVGTextMetrics stored in the layout attributes to get the extents for getExtentOfChar to avoid having to redo the somewhat involved extent computation for cursive scripts. With this, glyph extents for cursive scripts are better but still far from perfect/good. This is likely not the fault of the query code though. The test svg/text/bidi-text-query.svg is rewritten to better illustrate the query order and the returned extents. BUG=470326 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=192701

Patch Set 1 #

Patch Set 2 : More TEs. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+158 lines, -20 lines) Patch
M LayoutTests/TestExpectations View 1 1 chunk +3 lines, -0 lines 0 comments Download
M LayoutTests/svg/text/bidi-text-query.svg View 1 chunk +27 lines, -17 lines 0 comments Download
A LayoutTests/svg/text/svgtextcontentelement-glyphqueries-rtl.html View 1 chunk +96 lines, -0 lines 0 comments Download
A LayoutTests/svg/text/svgtextcontentelement-glyphqueries-rtl-expected.txt View 1 chunk +8 lines, -0 lines 0 comments Download
M Source/core/layout/svg/SVGTextQuery.cpp View 2 chunks +24 lines, -3 lines 1 comment Download

Messages

Total messages: 7 (2 generated)
fs
5 years, 9 months ago (2015-03-27 14:35:14 UTC) #2
pdr.
Amazing. LGTM
5 years, 9 months ago (2015-03-27 21:15:27 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1041693002/20001
5 years, 9 months ago (2015-03-27 21:15:46 UTC) #5
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://src.chromium.org/viewvc/blink?view=rev&revision=192701
5 years, 9 months ago (2015-03-27 21:19:00 UTC) #6
fs
5 years, 8 months ago (2015-03-30 12:01:19 UTC) #7
Message was sent while issue was closed.
https://codereview.chromium.org/1041693002/diff/20001/Source/core/layout/svg/...
File Source/core/layout/svg/SVGTextQuery.cpp (right):

https://codereview.chromium.org/1041693002/diff/20001/Source/core/layout/svg/...
Source/core/layout/svg/SVGTextQuery.cpp:436: const SVGTextMetrics& metrics =
textMetricsValues[fragment.characterOffset + startPosition];
This was missing a piece... =/ - Uploaded it as
https://codereview.chromium.org/1045793002

Powered by Google App Engine
This is Rietveld 408576698