[SkSVGDevice] Fix whitespace text handling

[SkSVGDevice] Fix whitespace text handling

SVG ignores leading/trailing whitespace and consolidates mid-text
(http://www.w3.org/TR/SVG/text.html#WhiteSpace). This can cause our
x/y position indices to get out of whack.

Implement a text builder which is SVG whitespace-munging-aware and
performs the needed x/y position list adjustments.

R=reed@google.com,mtklein@google.com

Committed: https://skia.googlesource.com/skia/+/8974774c32b5f0365d375a81d5d047c5c2406614

[reed1, 2015-02-13 20:37:53]

sgtm

[mtklein, 2015-02-17 15:37:29]

On 2015/02/13 20:24:12, f(malita) wrote:

What happens if we just use xml:space="preserve" ?

[f(malita), 2015-02-17 15:43:45]

On 2015/02/17 15:37:29, mtklein wrote:
> On 2015/02/13 20:24:12, f(malita) wrote:
> 
> What happens if we just use xml:space="preserve" ?

Certainly an option, but it seems we get us into a more fragile state: the XML
writer needs to change to avoid any text indentation/munging and I'm not sure
how well supported the (non-default) "preserve" mode is across various SVG user
agents. Plus we need to handle pos index adjustments anyway (for \0 and possibly
other tricky cases we haven't run into yet).

[mtklein, 2015-02-18 19:55:44]

This LGTM with small suggestions.

Given the complexity of the new whitespace merging code, I think it might be
time to start up a unit test file for this SVG device.  It seems easy to break
when refactoring.

https://codereview.chromium.org/928583003/diff/1/src/svg/SkSVGDevice.cpp
File src/svg/SkSVGDevice.cpp (right):

https://codereview.chromium.org/928583003/diff/1/src/svg/SkSVGDevice.cpp#newc...
src/svg/SkSVGDevice.cpp:167: switch (scalarsPerPos) {
This might be clearer as two if-statements:

if (scalarsPerPos < 2) { fPosY.appendScalar(offset.y()); }  // DrawText or
DrawPosTextH both have a fixed Y.
if (scalarsPerPos < 1) { fPosX.appendScalar(offset.x()); }  // DrawText has a
fixed X too.

https://codereview.chromium.org/928583003/diff/1/src/svg/SkSVGDevice.cpp#newc...
src/svg/SkSVGDevice.cpp:207: // SkPaint::glyphsToUnichars() returns \0 for
inconcertible glyphs, but these
inconvertible?

[f(malita), 2015-02-19 20:58:21]

New patchsets have been uploaded after l-g-t-m from mtklein@google.com

[f(malita), 2015-02-19 20:58:38]

On 2015/02/18 19:55:44, mtklein wrote:
> Given the complexity of the new whitespace merging code, I think it might be
> time to start up a unit test file for this SVG device.  It seems easy to break
> when refactoring.

SGTM, I'll do that as a follow-up.

https://codereview.chromium.org/928583003/diff/1/src/svg/SkSVGDevice.cpp
File src/svg/SkSVGDevice.cpp (right):

https://codereview.chromium.org/928583003/diff/1/src/svg/SkSVGDevice.cpp#newc...
src/svg/SkSVGDevice.cpp:167: switch (scalarsPerPos) {
On 2015/02/18 19:55:44, mtklein wrote:
> This might be clearer as two if-statements:
> 
> if (scalarsPerPos < 2) { fPosY.appendScalar(offset.y()); }  // DrawText or
> DrawPosTextH both have a fixed Y.
> if (scalarsPerPos < 1) { fPosX.appendScalar(offset.x()); }  // DrawText has a
> fixed X too.

Done.

https://codereview.chromium.org/928583003/diff/1/src/svg/SkSVGDevice.cpp#newc...
src/svg/SkSVGDevice.cpp:207: // SkPaint::glyphsToUnichars() returns \0 for
inconcertible glyphs, but these
On 2015/02/18 19:55:44, mtklein wrote:
> inconvertible?

Done.

[f(malita), 2015-02-19 20:58:43]

The CQ bit was checked by fmalita@chromium.org

[commit-bot: I haz the power, 2015-02-19 20:59:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/928583003/20001

[commit-bot: I haz the power, 2015-02-19 21:01:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-19 21:01:58]

Try jobs failed on following builders:
  Build-Win-VS2013-x86-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-VS2013-x86...)

[f(malita), 2015-02-19 21:03:20]

The CQ bit was checked by fmalita@chromium.org

[commit-bot: I haz the power, 2015-02-19 21:03:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/928583003/20001

[commit-bot: I haz the power, 2015-02-20 02:44:56]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/8974774c32b5f0365d375a81d5d047c5c2406614