RESOLVED FIXED 137463
Text drawn with an SVG font has no spaces when word-rounding hacks are enabled
https://bugs.webkit.org/show_bug.cgi?id=137463
Summary Text drawn with an SVG font has no spaces when word-rounding hacks are enabled
Myles C. Maxfield
Reported 2014-10-06 14:19:18 PDT
Text drawn with an SVG font has no spaces when word-rounding hacks are enabled
Attachments
Patch (4.49 KB, patch)
2014-10-06 15:59 PDT, Myles C. Maxfield
no flags
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (540.23 KB, application/zip)
2014-10-06 17:17 PDT, Build Bot
no flags
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (563.41 KB, application/zip)
2014-10-06 17:46 PDT, Build Bot
no flags
Patch (6.39 KB, patch)
2014-10-07 14:22 PDT, Myles C. Maxfield
no flags
Patch (6.69 KB, patch)
2014-10-08 11:31 PDT, Myles C. Maxfield
darin: review+
Myles C. Maxfield
Comment 1 2014-10-06 15:59:19 PDT
Myles C. Maxfield
Comment 2 2014-10-06 15:59:56 PDT
Build Bot
Comment 3 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
Build Bot
Comment 4 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
Build Bot
Comment 5 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
Build Bot
Comment 6 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
Myles C. Maxfield
Comment 7 2014-10-07 14:22:40 PDT
Andrei Bucur
Comment 8 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?
Darin Adler
Comment 9 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?
Myles C. Maxfield
Comment 10 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.
Myles C. Maxfield
Comment 11 2014-10-08 11:31:57 PDT
Antti Koivisto
Comment 12 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.
Myles C. Maxfield
Comment 13 2014-10-08 11:56:42 PDT
Note You need to log in before you can comment on or make changes to this bug.