Bug 133569

Summary: The OpenType MATH table is not read on Mac
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: MathMLAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bfulgham, buildbot, cfleizach, commit-queue, dbarton, gur.trio, mitz, mrobinson, ned, rniwa, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.8   
URL: http://mpeg.chiariglione.org/standards/mpeg-4/open-font-format/text-isoiec-cd-14496-22-3rd-edition
Bug Depends on:    
Bug Blocks: 99614, 122297, 133603    
Attachments:
Description Flags
testcase (includes WOFF as base64 data)
none
Python script used to generate the WOFF file
none
Screenshot of the testcase (WebKitGTK+)
none
Patch to do some logging
none
Screenshot
none
Patch
none
Patch
none
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
none
Patch
none
Patch cfleizach: review+

Description Frédéric Wang (:fredw) 2014-06-06 01:01:28 PDT
At the moment, the OpenType MATH table has been successfully read on EFL & GTK (I tested that) and Windows (Gurpreet tested that). From the screenshots provided by Chris, it seems that this does not work on Mac at the moment (I'm not sure for which version). This would require some debugging, for example:

1) Of the OpenTypeMathData::OpenTypeMathData function, to see if the table is well created and the headers validated.

2) Of the OpenTypeMathData::getMathVariants function, to see if sizeVariants and assemblyParts vectors are well created.

3) Of the OpenTypeMathData::getMathConstant function, to see if the constant is correctly read.

3) Of the RenderMathMLOperator::getDisplayStyleLargeOperator to see if the minheight and size variant is correctly picked.

For a minimal testcase, consider LayoutTests/mathml/opentype/large-operators-LatinModern.html (only one <mo> element is needed).

At the moment I can't do that myself.
Comment 1 Frédéric Wang (:fredw) 2014-06-07 07:19:18 PDT
Created attachment 232658 [details]
testcase (includes WOFF as base64 data)
Comment 2 Frédéric Wang (:fredw) 2014-06-07 07:19:48 PDT
Created attachment 232659 [details]
Python script used to generate the WOFF file
Comment 3 Frédéric Wang (:fredw) 2014-06-07 07:25:38 PDT
Created attachment 232660 [details]
Screenshot of the testcase (WebKitGTK+)

Here is how the testcase renders on WebKitGTK+

The woff font contains (at least) two glyphs

1) a small square for Unicode Character 'N-ARY SUMMATION' (U+2211).
2) a larger square for N-ARY SUMMATION in displaystyle mode.

1) is used in the <mtext>
2) should be used for the <mo> since the operator <math> has displaystyle=true

The DisplayOperatorMinHeight constant is set to the height of the larger square.
Comment 4 Frédéric Wang (:fredw) 2014-06-07 07:57:35 PDT
WebKit/Blink contains the following comment:

// OTTag is native because it's only compared against constants, so we don't
// do endian conversion here but make sure constants are in big-endian order.
// Note that multi-character literal is implementation-defined in C++0x.
typedef uint32_t Tag;
#define OT_MAKE_TAG(ch1, ch2, ch3, ch4) ((((uint32_t)(ch4)) << 24) | (((uint32_t)(ch3)) << 16) | (((uint32_t)(ch2)) << 8) | ((uint32_t)(ch1)))

Interestingly, Gecko does that in a different order:
#define TRUETYPE_TAG(a, b, c, d) ((a) << 24 | (b) << 16 | (c) << 8 | (d))
http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxFontUtils.h#323

Apple doc: https://developer.apple.com/library/ios/documentation/graphicsimaging/Reference/CGFont/Reference/reference.html#//apple_ref/c/func/CGFontCopyTableForTag
Comment 5 Frédéric Wang (:fredw) 2014-06-07 08:34:07 PDT
Created attachment 232662 [details]
Patch to do some logging

Here is a patch to do some logging. I'd appreciate if someone could load the testcase on Mac and tell the output.

As a comparison, on Linux I get the following data:

OpenTypeMathData::OpenTypeMathData
  m_mathBuffer->size(): 360
  m_mathBuffer: 00010000000A00FFFFFFE0012E
  math header valid
  math constants subtable OK
  math variants subtable OK
OpenTypeMathData::getMathConstant
  constant=3
  value=2000
OpenTypeMathData::getMathVariants
  got mathGlyphConstruction
  sizeVariants: [2 3 ]
Comment 6 Tim Horton 2014-06-20 00:56:20 PDT
(In reply to comment #5)
> Created an attachment (id=232662) [details]
> Patch to do some logging
> 
> Here is a patch to do some logging. I'd appreciate if someone could load the testcase on Mac and tell the output.

I've emailed you the output.
Comment 7 Frédéric Wang (:fredw) 2014-06-20 01:35:10 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > Created an attachment (id=232662) [details] [details]
> > Patch to do some logging
> > 
> > Here is a patch to do some logging. I'd appreciate if someone could load the testcase on Mac and tell the output.
> 
> I've emailed you the output.

Thank you very much, Tim. So apparently, the MATH table is not loaded at all.

Reading the Mac code, I see that Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm has a static function

static CFDataRef copyFontTableForTag(FontPlatformData& platformData, FourCharCode tableName)
{
    return CGFontCopyTableForTag(platformData.cgFont(), tableName);
}

that is used to read tables that way:

RetainPtr<CFDataRef> os2Table = adoptCF(copyFontTableForTag(m_platformData, 'OS/2'));

Are you able to copy copyFontTableForTag into Source/WebCore/platform/graphics/FontPlatformData.cpp and then use it in FontPlatformData::openTypeTable with a hardcoded 'MATH' string:

if (RetainPtr<CFDataRef> data = adoptCF(copyFontTableForTag(cgFont(), 'MATH'))
  return ...;

...just to see if the MATH table is loaded?
Comment 8 Frédéric Wang (:fredw) 2014-06-20 02:07:01 PDT
Created attachment 233418 [details]
Screenshot

So it seems that it was just some kind of endianness issue. I'll try to work on a patch to fix that.

FYI, here is a screenshot of http://www.maths-informatique-jeux.com/ulule/mathml_torture_test/latinmodern2math.html provided by Tim.
Comment 9 Frédéric Wang (:fredw) 2014-06-20 02:48:20 PDT
Created attachment 233419 [details]
Patch

So I'm not sure I understand the comment about "OTTag is native because it's only compared against constants" (comment 4). Anyway, MATHTag is only used in OpenTypeMathData.cpp so I guess it is safe to take this small patch to switch to FourCharCode on Mac. If the vertical data code is enabled on Mac or if more Open Type tables are read, I guess we should find a coherent way to handle all the platforms & endianness.

@Tim, can you please try again this patch?
Comment 10 Tim Horton 2014-06-20 19:29:48 PDT
(In reply to comment #9)
> Created an attachment (id=233419) [details]
> Patch
> 
> So I'm not sure I understand the comment about "OTTag is native because it's only compared against constants" (comment 4). Anyway, MATHTag is only used in OpenTypeMathData.cpp so I guess it is safe to take this small patch to switch to FourCharCode on Mac. If the vertical data code is enabled on Mac or if more Open Type tables are read, I guess we should find a coherent way to handle all the platforms & endianness.
> 
> @Tim, can you please try again this patch?

It works.
Comment 11 Frédéric Wang (:fredw) 2014-06-20 23:40:20 PDT
Created attachment 233512 [details]
Patch

Could someone please update the text and image references for

mathml/opentype/opentype-stretchy.html
mathml/opentype/opentype-stretchy-horizontal.html

?
Comment 12 Alexey Proskuryakov 2014-06-21 00:17:44 PDT
Comment on attachment 233512 [details]
Patch

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

> LayoutTests/ChangeLog:9
> +        Other tests require a font with a MATH table to be installed on Mac bots and to be used by default (bug 133603).

The way we normally achieve this is by loading fonts dynamically (as with Ahem).
Comment 13 Frédéric Wang (:fredw) 2014-06-21 00:26:54 PDT
(In reply to comment #12)
> The way we normally achieve this is by loading fonts dynamically (as with Ahem).

See the summary of my IRC discussion with Martin on bug 134094. I already used a small "fake font" to test basic stretching (horizontal, vertical and largeop), but to test "real examples" with several math operators, it's probably not a good idea to keep Latin Modern Math in the source (add a license, size of ~500ko).

Anyway, it will be necessary for Apple to have an Open Type MATH table installed on Mac/iOS if you want to benefit from the improved MathML rendering, without requiring Web authors to use Web fonts (compare attachment 233418 [details] with https://drfhlmcehrc34.cloudfront.net/cache/67/2f/672f5b477badfeccc6a2c06619a0c2c2.png).
Comment 14 Frédéric Wang (:fredw) 2014-06-21 03:27:27 PDT
Comment on attachment 233512 [details]
Patch

This was supposed to be a review request.
Comment 15 Build Bot 2014-06-21 05:58:56 PDT
Comment on attachment 233512 [details]
Patch

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

New failing tests:
mathml/opentype/opentype-stretchy-horizontal.html
mathml/opentype/opentype-stretchy.html
Comment 16 Build Bot 2014-06-21 05:59:00 PDT
Created attachment 233531 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 17 Build Bot 2014-06-21 06:21:12 PDT
Comment on attachment 233512 [details]
Patch

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

New failing tests:
mathml/opentype/opentype-stretchy-horizontal.html
mathml/opentype/opentype-stretchy.html
Comment 18 Build Bot 2014-06-21 06:21:17 PDT
Created attachment 233532 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 19 Frédéric Wang (:fredw) 2014-06-22 01:48:09 PDT
Created attachment 233561 [details]
Patch
Comment 20 Frédéric Wang (:fredw) 2014-06-22 01:53:22 PDT
Created attachment 233563 [details]
Patch
Comment 21 Frédéric Wang (:fredw) 2014-07-01 23:44:35 PDT
Committed r170699: <http://trac.webkit.org/changeset/170699>
Comment 22 Frédéric Wang (:fredw) 2014-07-02 02:16:15 PDT
I see new MathML failures that can safely be ignored for now as well as new tests that pass (although I think that's unrelated to this patch). Last time, someone told me on IRC that mac/win gardening should only be done by people doing mac/win development, so feel free to update the TestExpectations for Apple Win / Mac.