Bug 137463

Summary: Text drawn with an SVG font has no spaces when word-rounding hacks are enabled
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, dino, d-r, fmalita, gyuyoung.kim, jonlee, mitz, pdr, rniwa, schenney, sergio, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
none
Patch
none
Patch darin: review+

Description Myles C. Maxfield 2014-10-06 14:19:18 PDT
Text drawn with an SVG font has no spaces when word-rounding hacks are enabled
Comment 1 Myles C. Maxfield 2014-10-06 15:59:19 PDT
Created attachment 239364 [details]
Patch
Comment 2 Myles C. Maxfield 2014-10-06 15:59:56 PDT
<rdar://problem/18506282>
Comment 3 Build Bot 2014-10-06 17:17:44 PDT
Comment on attachment 239364 [details]
Patch

Attachment 239364 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4849734556581888

New failing tests:
fast/text/svg-font-word-rounding-hacks-spaces.html
Comment 4 Build Bot 2014-10-06 17:17:50 PDT
Created attachment 239368 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 5 Build Bot 2014-10-06 17:45:59 PDT
Comment on attachment 239364 [details]
Patch

Attachment 239364 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5961705079177216

New failing tests:
fast/text/svg-font-word-rounding-hacks-spaces.html
Comment 6 Build Bot 2014-10-06 17:46:06 PDT
Created attachment 239371 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 7 Myles C. Maxfield 2014-10-07 14:22:40 PDT
Created attachment 239433 [details]
Patch
Comment 8 Andrei Bucur 2014-10-08 09:04:52 PDT
Comment on attachment 239433 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=239433&action=review

> Source/WebCore/svg/SVGFontData.cpp:111
> +    fontData->setSpaceWidth(spaceWidth);

Should't setAdjustedSpaceWidth(0) be called in the if (!glyphPageZero) above, especially if it's initialized with undefined?
Comment 9 Darin Adler 2014-10-08 09:07:49 PDT
Comment on attachment 239433 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=239433&action=review

> Source/WebCore/platform/graphics/SimpleFontData.h:163
>      void setSpaceWidth(float spaceWidth) { m_spaceWidth = spaceWidth; }
> +    void setAdjustedSpaceWidth(float adjustedSpaceWidth) { m_adjustedSpaceWidth = adjustedSpaceWidth; }

Maybe we should offer a setter that sets both?
Comment 10 Myles C. Maxfield 2014-10-08 11:30:51 PDT
Comment on attachment 239433 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=239433&action=review

>> Source/WebCore/platform/graphics/SimpleFontData.h:163
>> +    void setAdjustedSpaceWidth(float adjustedSpaceWidth) { m_adjustedSpaceWidth = adjustedSpaceWidth; }
> 
> Maybe we should offer a setter that sets both?

Good idea. Done.

>> Source/WebCore/svg/SVGFontData.cpp:111
>> +    fontData->setSpaceWidth(spaceWidth);
> 
> Should't setAdjustedSpaceWidth(0) be called in the if (!glyphPageZero) above, especially if it's initialized with undefined?

Yep.
Comment 11 Myles C. Maxfield 2014-10-08 11:31:57 PDT
Created attachment 239482 [details]
Patch
Comment 12 Antti Koivisto 2014-10-08 11:42:50 PDT
Comment on attachment 239482 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=239482&action=review

> Source/WebCore/platform/graphics/SimpleFontData.h:163
>      void setSpaceWidth(float spaceWidth) { m_spaceWidth = spaceWidth; }
> +    void setAllSpaceWidths(float spaceWidth)

setSpaceWidth won't have clients anymore. I think you can just have one setter called setSpaceWidths() that sets both.
Comment 13 Myles C. Maxfield 2014-10-08 11:56:42 PDT
http://trac.webkit.org/changeset/174466