Bug 143402

Summary: [Win] [SVG -> OTF Converter] Support the SVG -> OTF Font Converter
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: Layout and RenderingAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, buildbot, commit-queue, koivisto, rniwa, ryanhaddad
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 151650, 154222    
Bug Blocks: 144693    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews101 for mac-mavericks
none
Archive of layout-test-results from ews106 for mac-mavericks-wk2
none
Patch
darin: review+, buildbot: commit-queue-
Archive of layout-test-results from ews100 for mac-mavericks
none
Archive of layout-test-results from ews106 for mac-mavericks-wk2
none
Patch for landing
commit-queue: commit-queue-
Reduction
none
Reduction
none
Prospective patch
none
Prospective patch
none
Prospective patch
none
Prospective patch
none
Prospective patch
none
Prospective patch
none
Enable on Windows
none
Enable on Windows
none
Patch
none
Patch
mmaxfield: review-
Patch
none
Patch achristensen: review+

Description Myles C. Maxfield 2015-04-04 09:22:37 PDT 
AddFontMemResourceEx is returning failure.
Comment 1 Myles C. Maxfield 2015-04-04 16:03:25 PDT 
Looks like the checksums are being generated using the wrong endianness
Comment 2 Myles C. Maxfield 2015-04-04 22:14:03 PDT 
The CFF private dict is required.
Comment 3 Myles C. Maxfield 2015-04-05 12:24:44 PDT 
Windows also requires a CMAP table in format 4
Comment 4 Myles C. Maxfield 2015-04-05 14:06:19 PDT 
Created attachment 250165 [details]
Patch
Comment 5 WebKit Commit Bot 2015-04-05 14:07:44 PDT 
Attachment 250165 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/SVGToOTFFontConversion.cpp:1368:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Myles C. Maxfield 2015-04-05 14:14:28 PDT 
Created attachment 250166 [details]
Patch
Comment 7 Myles C. Maxfield 2015-04-05 16:14:02 PDT 
Created attachment 250173 [details]
Patch
Comment 8 Build Bot 2015-04-05 17:00:14 PDT 
Comment on attachment 250173 [details]
Patch

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

Number of test failures exceeded the failure limit.
Comment 9 Build Bot 2015-04-05 17:00:17 PDT 
Created attachment 250176 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 10 Build Bot 2015-04-05 17:08:23 PDT 
Comment on attachment 250173 [details]
Patch

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

Number of test failures exceeded the failure limit.
Comment 11 Build Bot 2015-04-05 17:08:26 PDT 
Created attachment 250177 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 12 Myles C. Maxfield 2015-04-05 18:56:56 PDT 
Comment on attachment 250173 [details]
Patch

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

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:339
> +    append32(m_result.size() - startingOffset + 22); // Byte offset of subtable

Nope
Comment 13 Myles C. Maxfield 2015-04-05 19:05:03 PDT 
Created attachment 250186 [details]
Patch
Comment 14 Build Bot 2015-04-05 19:54:01 PDT 
Comment on attachment 250186 [details]
Patch

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

New failing tests:
platform/mac/svg/fonts/svg-font-general.html
Comment 15 Build Bot 2015-04-05 19:54:05 PDT 
Created attachment 250187 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 16 Build Bot 2015-04-05 20:00:21 PDT 
Comment on attachment 250186 [details]
Patch

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

New failing tests:
platform/mac/svg/fonts/svg-font-general.html
Comment 17 Build Bot 2015-04-05 20:00:23 PDT 
Created attachment 250188 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 18 Darin Adler 2015-04-05 20:12:50 PDT 
Comment on attachment 250186 [details]
Patch

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

Looks like this code is crashing on the Mac. I suggest making it not crash before landing ;)

> Source/WebCore/ChangeLog:10
> +        1. Checksums were being calculated with the wrong endianness

That’s the big one!

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:196
> +    void appendFormat12CMAPTable(const Vector<std::pair<UChar32, Glyph>>& mappings);
> +    void appendFormat4CMAPTable(const Vector<std::pair<UChar32, Glyph>>& mappings);

Argument name doesn’t make sense here.

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:301
> +    uint16_t searchRange = clampTo<uint16_t>(2 * static_cast<uint32_t>(originalSearchRange)); // searchRange: "2 x (2**floor(log2(segCount)))"

The cast to uint32_t is not needed here. The C language guarantees that expression will be done as-if-32-bit since our int is 32-bit, believe it or not.

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:311
> +    append16(0x00); // reserved

This should either be append8, or 0x0000, or just 0.

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:328
> +    overwrite16(subtableLocation + 2, clampTo<uint16_t>(m_result.size() - subtableLocation)); // FIXME: If we ever overrun here, choose a better encoding for the above table.

Confusing FIXME. It’s not like this comment is going to be here at runtime to help us!

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:335
> +    append16(3); // Number subtables

Number of

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:617
> +    const char privateDictIndexKey = 0x12;

Strange that this is hex and the rest is not.

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1363
> +        m_ascent = m_unitsPerEm;

Why are we using m_unitsPerEm when it’s just the constant 0? Also, don’t we need this to be 1, not 0?

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1365
> +        m_xHeight = m_unitsPerEm;

Why are we using m_unitsPerEm when it’s just the constant 0?

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1366
> +        m_capHeight = m_fontFaceElement->ascent();

This will crash!

> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1467
> +        sum += (static_cast<unsigned char>(m_result[offset + 3]))

Extra unneeded parentheses here. We should remove them.
Comment 19 Myles C. Maxfield 2015-04-05 22:16:13 PDT 
Comment on attachment 250186 [details]
Patch

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

>> Source/WebCore/ChangeLog:10
>> +        1. Checksums were being calculated with the wrong endianness
> 
> That’s the big one!

Yeah - turns out that mac doesn't pay attention at all to the checksums, so I didn't even know that it was being done incorrectly. Windows enforces them, though.

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:196
>> +    void appendFormat4CMAPTable(const Vector<std::pair<UChar32, Glyph>>& mappings);
> 
> Argument name doesn’t make sense here.

Done.

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:301
>> +    uint16_t searchRange = clampTo<uint16_t>(2 * static_cast<uint32_t>(originalSearchRange)); // searchRange: "2 x (2**floor(log2(segCount)))"
> 
> The cast to uint32_t is not needed here. The C language guarantees that expression will be done as-if-32-bit since our int is 32-bit, believe it or not.

Oh right, because "2" is an int.

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:311
>> +    append16(0x00); // reserved
> 
> This should either be append8, or 0x0000, or just 0.

Done.

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:328
>> +    overwrite16(subtableLocation + 2, clampTo<uint16_t>(m_result.size() - subtableLocation)); // FIXME: If we ever overrun here, choose a better encoding for the above table.
> 
> Confusing FIXME. It’s not like this comment is going to be here at runtime to help us!

Done.

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:335
>> +    append16(3); // Number subtables
> 
> Number of

Done.

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:617
>> +    const char privateDictIndexKey = 0x12;
> 
> Strange that this is hex and the rest is not.

Done.

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1363
>> +        m_ascent = m_unitsPerEm;
> 
> Why are we using m_unitsPerEm when it’s just the constant 0? Also, don’t we need this to be 1, not 0?

Yes, though if there is no font face element, we're kind of hosed anyway.

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1365
>> +        m_xHeight = m_unitsPerEm;
> 
> Why are we using m_unitsPerEm when it’s just the constant 0?

There isn't really a reason - I don't know what the "right" answer but I've got to initialize it to something. m_unitsPerEm seems about as good as anything else (even if it was just set to 0)

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1366
>> +        m_capHeight = m_fontFaceElement->ascent();
> 
> This will crash!

Whoops :(

>> Source/WebCore/svg/SVGToOTFFontConversion.cpp:1467
>> +        sum += (static_cast<unsigned char>(m_result[offset + 3]))
> 
> Extra unneeded parentheses here. We should remove them.

Done.
Comment 20 Myles C. Maxfield 2015-04-05 22:21:20 PDT 
Created attachment 250193 [details]
Patch for landing
Comment 21 WebKit Commit Bot 2015-04-06 08:01:07 PDT 
Comment on attachment 250193 [details]
Patch for landing

Rejecting attachment 250193 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 250193, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/6415266074329088
Comment 22 Myles C. Maxfield 2015-04-06 08:31:43 PDT 
Committed r182423: <http://trac.webkit.org/changeset/182423>
Comment 23 Myles C. Maxfield 2015-05-06 16:22:49 PDT 
Rolled out in r182443
Comment 24 Myles C. Maxfield 2015-05-06 16:23:33 PDT 
Blocking https://bugs.webkit.org/show_bug.cgi?id=144693
Comment 25 Myles C. Maxfield 2015-05-15 19:19:02 PDT 
A simple test, for a good place to start: fast\text\font-with-no-space-glyph.html

Another test that this causes to fail: fast\text\svg-font-face-with-kerning.html
Comment 26 Myles C. Maxfield 2015-05-15 23:04:04 PDT 
Looks like these are the svg/ tests which are failing:

svg\W3C-SVG-1.1-SE\color-prop-05-t.svg
svg\W3C-SVG-1.1-SE\coords-dom-01-f.svg
svg\W3C-SVG-1.1-SE\coords-dom-02-f.svg
svg\W3C-SVG-1.1-SE\coords-dom-03-f.svg
svg\W3C-SVG-1.1-SE\coords-dom-04-f.svg
svg\W3C-SVG-1.1-SE\coords-units-03-b.svg
svg\W3C-SVG-1.1-SE\filters-felem-01-b.svg
svg\W3C-SVG-1.1-SE\filters-image-03-f.svg
svg\W3C-SVG-1.1-SE\interact-pointer-03-t.svg
svg\W3C-SVG-1.1-SE\painting-marker-07-f.svg
svg\W3C-SVG-1.1-SE\paths-dom-02-f.svg
svg\W3C-SVG-1.1-SE\pservers-grad-17-b.svg
svg\W3C-SVG-1.1-SE\pservers-grad-20-b.svg
svg\W3C-SVG-1.1-SE\pservers-pattern-03-f.svg
svg\W3C-SVG-1.1-SE\pservers-pattern-04-f.svg
svg\W3C-SVG-1.1-SE\struct-use-14-f.svg
svg\W3C-SVG-1.1-SE\styling-css-04-f.svg
svg\W3C-SVG-1.1-SE\styling-pres-02-f.svg
svg\W3C-SVG-1.1-SE\svgdom-over-01-f.svg
svg\W3C-SVG-1.1-SE\text-tref-03-b.svg
svg\W3C-SVG-1.1-SE\text-tspan-02-b.svg
svg\W3C-SVG-1.1-SE\types-dom-02-f.svg
svg\W3C-SVG-1.1-SE\types-dom-03-b.svg
svg\W3C-SVG-1.1-SE\types-dom-04-b.svg
svg\W3C-SVG-1.1-SE\types-dom-05-b.svg
svg\W3C-SVG-1.1-SE\types-dom-06-f.svg
svg\W3C-SVG-1.1-SE\types-dom-07-f.svg
svg\W3C-SVG-1.1\animate-elem-03-t.svg
svg\W3C-SVG-1.1\animate-elem-24-t.svg
svg\W3C-SVG-1.1\animate-elem-36-t.svg
svg\W3C-SVG-1.1\animate-elem-40-t.svg
svg\W3C-SVG-1.1\filters-light-04-f.svg
svg\W3C-SVG-1.1\filters-turb-02-f.svg
svg\W3C-SVG-1.1\fonts-desc-02-t.svg
svg\W3C-SVG-1.1\fonts-elem-01-t.svg
svg\W3C-SVG-1.1\fonts-elem-02-t.svg
svg\W3C-SVG-1.1\fonts-elem-03-b.svg
svg\W3C-SVG-1.1\fonts-elem-04-b.svg
svg\W3C-SVG-1.1\fonts-elem-07-b.svg
svg\W3C-SVG-1.1\fonts-glyph-02-t.svg
svg\W3C-SVG-1.1\fonts-glyph-03-t.svg
svg\W3C-SVG-1.1\fonts-kern-01-t.svg
svg\W3C-SVG-1.1\masking-mask-01-b.svg
svg\W3C-SVG-1.1\pservers-grad-08-b.svg
svg\W3C-SVG-1.1\render-elems-06-t.svg
svg\W3C-SVG-1.1\render-elems-07-t.svg
svg\W3C-SVG-1.1\render-elems-08-t.svg
svg\W3C-SVG-1.1\render-groups-01-b.svg
svg\W3C-SVG-1.1\render-groups-03-t.svg
svg\W3C-SVG-1.1\text-align-08-b.svg
svg\W3C-SVG-1.1\text-altglyph-01-b.svg
svg\W3C-SVG-1.1\text-fonts-03-t.svg
svg\W3C-SVG-1.1\text-intro-01-t.svg
svg\W3C-SVG-1.1\text-intro-02-b.svg
svg\W3C-SVG-1.1\text-intro-03-b.svg
svg\W3C-SVG-1.1\text-intro-04-t.svg
svg\W3C-SVG-1.1\text-text-04-t.svg
svg\W3C-SVG-1.1\text-text-05-t.svg
svg\W3C-SVG-1.1\text-text-06-t.svg
svg\batik\text\textEffect.svg
svg\batik\text\textEffect3.svg
svg\batik\text\textPosition2.svg
svg\batik\text\xmlSpace.svg
svg\custom\altglyph.svg
svg\custom\glyph-selection-arabic-forms.svg
svg\custom\glyph-selection-lang-attribute.svg
svg\custom\glyph-selection-non-bmp.svg
svg\custom\glyph-setting-d-attribute.svg
svg\custom\glyph-transformation-with-hkern.svg
svg\custom\svg-fonts-in-html.html
svg\custom\svg-fonts-in-text-controls.html
svg\custom\svg-fonts-no-latin-glyph.html
svg\custom\svg-fonts-segmented.xhtml
svg\custom\svg-fonts-with-no-element-reference.html
svg\custom\svg-fonts-without-missing-glyph.xhtml
svg\custom\svg-fonts-word-spacing.html
svg\foreignObject\text-tref-02-b.svg
svg\text\kerning.svg
svg\text\multichar-glyph.svg
svg\text\svg-font-word-rounding-hacks-spaces.html
svg\text\text-altglyph-01-b.svg
svg\text\text-hkern-on-vertical-text.svg
svg\text\text-hkern.svg
svg\text\text-overflow-ellipsis-svgfont-kerning-ligatures.html
svg\text\text-overflow-ellipsis-svgfont.html
svg\text\text-text-04-t.svg
svg\text\text-text-05-t.svg
svg\text\text-text-06-t.svg
svg\text\text-vkern-on-horizontal-text.svg
svg\text\text-vkern.svg
svg\transforms\text-with-mask-with-svg-transform.svg
svg\wicd\test-rightsizing-b.xhtml
Comment 27 Myles C. Maxfield 2015-05-15 23:35:53 PDT 
After looking over the list of failures, these are the tests which aren't simply a rebaseline:

svg\W3C-SVG-1.1-SE\types-dom-06-f.svg
svg\W3C-SVG-1.1\animate-elem-03-t.svg
svg\W3C-SVG-1.1\animate-elem-24-t.svg
svg\W3C-SVG-1.1\animate-elem-36-t.svg
svg\W3C-SVG-1.1\animate-elem-40-t.svg
svg\W3C-SVG-1.1\fonts-desc-02-t.svg
+svg\W3C-SVG-1.1\fonts-elem-01-t.svg
+svg\W3C-SVG-1.1\fonts-elem-02-t.svg
+svg\W3C-SVG-1.1\fonts-elem-03-b.svg
+svg\W3C-SVG-1.1\fonts-elem-04-b.svg
+svg\W3C-SVG-1.1\fonts-elem-07-b.svg
+svg\W3C-SVG-1.1\fonts-glyph-02-t.svg
+svg\W3C-SVG-1.1\fonts-glyph-03-t.svg
svg\W3C-SVG-1.1\fonts-kern-01-t.svg
svg\W3C-SVG-1.1\masking-mask-01-b.svg
svg\W3C-SVG-1.1\pservers-grad-08-b.svg
svg\W3C-SVG-1.1\render-elems-06-t.svg
svg\W3C-SVG-1.1\render-elems-07-t.svg
svg\W3C-SVG-1.1\render-elems-08-t.svg
svg\W3C-SVG-1.1\render-groups-01-b.svg
svg\W3C-SVG-1.1\render-groups-03-t.svg
svg\W3C-SVG-1.1\text-align-08-b.svg
svg\W3C-SVG-1.1\text-altglyph-01-b.svg
svg\W3C-SVG-1.1\text-fonts-03-t.svg
svg\W3C-SVG-1.1\text-intro-01-t.svg
svg\W3C-SVG-1.1\text-intro-02-b.svg
svg\W3C-SVG-1.1\text-intro-03-b.svg
svg\W3C-SVG-1.1\text-intro-04-t.svg
svg\W3C-SVG-1.1\text-text-04-t.svg
svg\W3C-SVG-1.1\text-text-05-t.svg
svg\W3C-SVG-1.1\text-text-06-t.svg
svg\batik\text\textEffect.svg
svg\batik\text\textEffect3.svg
svg\batik\text\textPosition2.svg
svg\batik\text\xmlSpace.svg
svg\custom\altglyph.svg
svg\custom\glyph-selection-arabic-forms.svg
svg\custom\glyph-selection-lang-attribute.svg
svg\custom\glyph-selection-non-bmp.svg
svg\custom\glyph-setting-d-attribute.svg
svg\custom\glyph-transformation-with-hkern.svg
svg\custom\svg-fonts-in-text-controls.html
svg\custom\svg-fonts-no-latin-glyph.html
svg\custom\svg-fonts-segmented.xhtml
svg\text\kerning.svg
svg\text\multichar-glyph.svg
svg\text\svg-font-word-rounding-hacks-spaces.html
svg\text\text-altglyph-01-b.svg
svg\text\text-hkern-on-vertical-text.svg
svg\text\text-hkern.svg
svg\text\text-overflow-ellipsis-svgfont-kerning-ligatures.html
svg\text\text-overflow-ellipsis-svgfont.html
svg\text\text-text-04-t.svg
svg\text\text-text-05-t.svg
svg\text\text-text-06-t.svg
svg\text\text-vkern-on-horizontal-text.svg
svg\text\text-vkern.svg
svg\transforms\text-with-mask-with-svg-transform.svg
svg\wicd\test-rightsizing-b.xhtml
Comment 28 Myles C. Maxfield 2015-05-20 18:29:24 PDT 
Looks like external SVG fonts work, but in-document SVG fonts don't.
Comment 29 Myles C. Maxfield 2015-05-20 18:36:06 PDT 
Created attachment 253487 [details]
Reduction
Comment 30 Myles C. Maxfield 2015-05-20 18:43:02 PDT 
Created attachment 253489 [details]
Reduction
Comment 31 Myles C. Maxfield 2015-05-20 18:46:06 PDT 
(In reply to comment #28)
> Looks like external SVG fonts work, but in-document SVG fonts don't.

Nevermind. This is incorrect.
Comment 32 Myles C. Maxfield 2015-05-20 19:11:10 PDT 
It's a caching problem. If you visit the reduction, change the contents of the font data inside the .html file, then reload the page, the page does not update to include your edits. (And if you change the content in the rest of the .html file, it does update)
Comment 33 Myles C. Maxfield 2015-11-15 13:13:06 PST 
Created attachment 265561 [details]
Prospective patch
Comment 34 Myles C. Maxfield 2015-11-15 16:19:09 PST 
(In reply to comment #32)
> It's a caching problem. If you visit the reduction, change the contents of
> the font data inside the .html file, then reload the page, the page does not
> update to include your edits. (And if you change the content in the rest of
> the .html file, it does update)

This is true, but it's a red herring. The font is simply cached in our memory cache.
Comment 35 Myles C. Maxfield 2015-11-15 17:55:29 PST 
Created attachment 265563 [details]
Prospective patch
Comment 36 Myles C. Maxfield 2015-11-15 18:07:06 PST 
Created attachment 265564 [details]
Prospective patch
Comment 37 Myles C. Maxfield 2015-11-15 18:34:16 PST 
Created attachment 265565 [details]
Prospective patch
Comment 38 Myles C. Maxfield 2015-11-15 18:46:11 PST 
Created attachment 265566 [details]
Prospective patch
Comment 39 Myles C. Maxfield 2015-11-15 19:31:06 PST 
Created attachment 265569 [details]
Prospective patch
Comment 40 Myles C. Maxfield 2015-11-15 19:57:45 PST 
svg/W3C-SVG-1.1/fonts-elem-03-b.svg shows the glyphs being rendered too large.
Comment 41 Myles C. Maxfield 2015-11-21 18:51:47 PST 
We have a #if ENABLE(SVG_FONTS) in CSSFontSelector::getFontFace() which could be deleted when SVG fonts go away.
Comment 42 Myles C. Maxfield 2016-02-06 15:39:41 PST 
r192930 fixed the glyph sizing issue.

svg\W3C-SVG-1.1-SE\types-dom-06-f.svg should just be a rebaseline.
Comment 43 Alex Christensen 2016-02-06 17:23:04 PST 
Comment on attachment 250186 [details]
Patch

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

> WebKitLibraries/win/tools/vsprops/FeatureDefines.props:77
> -    <ENABLE_SVG_OTF_CONVERTER />
> +    <ENABLE_SVG_OTF_CONVERTER>ENABLE_SVG_OTF_CONVERTER</ENABLE_SVG_OTF_CONVERTER>

This is great, but changing Source/cmake/PlatformWin.cmake will change whether the feature is enabled or not.
Comment 44 Myles C. Maxfield 2016-02-06 21:11:20 PST 
Created attachment 270814 [details]
Enable on Windows
Comment 45 Myles C. Maxfield 2016-02-06 21:16:25 PST 
Attachment 270814 [details] fixes the svg\W3C-SVG-1.1\animate* tests.
Comment 46 Myles C. Maxfield 2016-02-06 21:39:50 PST 
(In reply to comment #45)
> Attachment 270814 [details] fixes the svg\W3C-SVG-1.1\animate* tests.

I've made good progress. Here is the list of remaining failures. Note that I'm not including altglyph tests (because we don't support altglyph) and kerning tests (because Windows doesn't respect the 'kern' table, in favor of 'GPOS').

svg\W3C-SVG-1.1\fonts-desc-02-t.svg
svg\W3C-SVG-1.1\fonts-glyph-03-t.svg
svg\W3C-SVG-1.1\masking-mask-01-b.svg
svg\W3C-SVG-1.1\pservers-grad-08-b.svg
svg\W3C-SVG-1.1\text-align-08-b.svg
svg\W3C-SVG-1.1\text-intro-01-t.svg
svg\W3C-SVG-1.1\text-intro-02-b.svg
svg\W3C-SVG-1.1\text-intro-03-b.svg
svg\W3C-SVG-1.1\text-intro-04-t.svg
svg\W3C-SVG-1.1\text-text-06-t.svg
svg\batik\text\textEffect.svg
svg\batik\text\textEffect3.svg
svg\custom\glyph-selection-lang-attribute.svg
svg\custom\glyph-selection-non-bmp.svg
svg\text\multichar-glyph.svg
svg\text\text-overflow-ellipsis-svgfont.html
svg\text\text-text-06-t.svg
Comment 47 Myles C. Maxfield 2016-02-06 21:51:26 PST 
A better list (this one omits tests which are marked as failing on Mac, because both platforms use the same converter, there's no reason why anything would pass on Windows which fails on Mac)

svg\W3C-SVG-1.1\fonts-glyph-03-t.svg
svg\W3C-SVG-1.1\masking-mask-01-b.svg
svg\W3C-SVG-1.1\pservers-grad-08-b.svg
svg\W3C-SVG-1.1\text-align-08-b.svg
svg\W3C-SVG-1.1\text-intro-01-t.svg
svg\W3C-SVG-1.1\text-intro-02-b.svg
svg\W3C-SVG-1.1\text-intro-03-b.svg
svg\W3C-SVG-1.1\text-intro-04-t.svg
svg\batik\text\textEffect3.svg
svg\custom\glyph-selection-lang-attribute.svg
svg\custom\glyph-selection-non-bmp.svg
svg\text\multichar-glyph.svg
svg\text\text-overflow-ellipsis-svgfont.html
Comment 48 Myles C. Maxfield 2016-02-06 22:07:03 PST 
We also don't support the lang attribute, so let's omit those tests as well.

Here is the new list of failing tests:
svg\W3C-SVG-1.1\masking-mask-01-b.svg
svg\W3C-SVG-1.1\pservers-grad-08-b.svg
svg\W3C-SVG-1.1\text-align-08-b.svg
svg\W3C-SVG-1.1\text-intro-01-t.svg
svg\W3C-SVG-1.1\text-intro-02-b.svg
svg\W3C-SVG-1.1\text-intro-03-b.svg
svg\W3C-SVG-1.1\text-intro-04-t.svg
svg\batik\text\textEffect3.svg
svg\custom\glyph-selection-non-bmp.svg
svg\text\multichar-glyph.svg
svg\text\text-overflow-ellipsis-svgfont.html
Comment 49 Myles C. Maxfield 2016-02-06 22:16:55 PST 
I've sorted these failing tests into buckets:

Multichar characters are invisible:
svg\W3C-SVG-1.1\text-align-08-b.svg
svg\W3C-SVG-1.1\text-intro-01-t.svg
svg\W3C-SVG-1.1\text-intro-02-b.svg
svg\W3C-SVG-1.1\text-intro-04-t.svg
svg\custom\glyph-selection-non-bmp.svg
svg\text\multichar-glyph.svg

Vertical advances are not being calculated correctly:
svg\W3C-SVG-1.1\text-intro-03-b.svg

Unknown:
svg\W3C-SVG-1.1\masking-mask-01-b.svg
svg\W3C-SVG-1.1\pservers-grad-08-b.svg
svg\batik\text\textEffect3.svg
svg\text\text-overflow-ellipsis-svgfont.html
Comment 50 Myles C. Maxfield 2016-02-13 12:25:30 PST 
Created attachment 271293 [details]
Enable on Windows
Comment 51 Myles C. Maxfield 2016-02-13 15:59:14 PST 
(In reply to comment #50)
> Created attachment 271293 [details]
> Enable on Windows

https://bugs.webkit.org/show_bug.cgi?id=154222 fixes all the tests above labelled "Unknown."

I think we're at the point where I'm comfortable filing bugs for the known issues and turning this on for Windows.
Comment 52 Myles C. Maxfield 2016-02-19 14:35:15 PST 
Created attachment 271802 [details]
Patch
Comment 53 Myles C. Maxfield 2016-02-22 11:19:56 PST 
Ryan: I plan to turn this on for Windows sometime in the next few days. There may (will almost certainly be) some test fallout (on Windows only) so I'm giving you a heads up. I'm trying to do everything I can to minimize the problems but we'll see what happens!!

Very exciting time!!
Comment 54 Myles C. Maxfield 2016-02-25 13:44:30 PST 
Created attachment 272232 [details]
Patch
Comment 55 Alex Christensen 2016-02-25 13:57:24 PST 
Comment on attachment 272232 [details]
Patch

I don't understand why you only have one change in OptionsWin.cmake, but you have efl expectations changes and mac TestExpectations changes.
Comment 56 Myles C. Maxfield 2016-02-25 14:10:34 PST 
Created attachment 272236 [details]
Patch
Comment 57 Myles C. Maxfield 2016-02-25 14:11:56 PST 
Comment on attachment 272236 [details]
Patch

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

> LayoutTests/ChangeLog:18
> +        * platform/mac/TestExpectations:

platform/win/TestExpectations
Comment 58 Myles C. Maxfield 2016-02-25 14:38:11 PST 
Created attachment 272242 [details]
Patch
Comment 59 Myles C. Maxfield 2016-02-25 16:33:39 PST 
Committed r197145: <http://trac.webkit.org/changeset/197145>