Operator stretching: expose a math data API

• Component: MathML
• Status: RESOLVED FIXED

[Frédéric Wang (:fredw)]

This is essentially the work of bug 130321, without the hardcoded data.

I think we can forget the hardcoded data, since the CGFontCopyTableForTag seems to be what we want to load a table on Mac. 
I'm not sure what are the other platforms that are missing the feature to load OpenType, but I think we can ignore them for now. The parsing of the MATH table and everything that requires adding a compile flag will be handled in bug 130324.

[Frédéric Wang (:fredw)]

Created attachment 227408 [details]
Patch

[Frédéric Wang (:fredw)]

> > Source/WebCore/platform/graphics/FontPlatformData.cpp:30
> > +#if OS(DARWIN)
>
> OS(DARWIN) might be correct, but my first guess would have been to use
> PLATFORM(COCOA)

If I read Source/WTF/wtf/Platform.h correctly, OS(DARWIN) implies PLATFORM(COCOA). OS(DARWIN) is what is used in FontPlaformData.h.

[chris fleizach]

(In reply to comment #2)
> > > Source/WebCore/platform/graphics/FontPlatformData.cpp:30
> > > +#if OS(DARWIN)
> >
> > OS(DARWIN) might be correct, but my first guess would have been to use
> > PLATFORM(COCOA)
> 
> If I read Source/WTF/wtf/Platform.h correctly, OS(DARWIN) implies PLATFORM(COCOA). OS(DARWIN) is what is used in FontPlaformData.h.

Ok

[Frédéric Wang (:fredw)]

Created attachment 227629 [details]
Patch

This patch addresses bug 130321 comment 13 (except the #if OS(DARWIN)).

[chris fleizach]

Comment on attachment 227629 [details]
Patch

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

> Source/WebCore/platform/graphics/opentype/OpenTypeMathData.h:44
> +    bool hasMathData() const { return !!m_mathBuffer; }

should this just be return m_mathBuffer.get(), instead of !!

[Frédéric Wang (:fredw)]

Committed r166169: <http://trac.webkit.org/changeset/166169>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 130682

[Frédéric Wang (:fredw)]

So it seems that some platform/font can not generate a hash(). I'll need to add some special handling to ignore these cases.

[Frédéric Wang (:fredw)]

Created attachment 227733 [details]
Patch

[Frédéric Wang (:fredw)]

Created attachment 227734 [details]
Difference from the previous commit

So I think openTypeTable should only be defined for "OS(DARWIN) && USE(CG)", as cgFont() is not available otherwise.

I've modified the code to leave m_mathData uninitialized when m_font is undefined on Mac/Win, since otherwise the hash() asserts or return 0.

[chris fleizach]

Comment on attachment 227734 [details]
Difference from the previous commit

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

> Source/WebCore/platform/graphics/SimpleFontData.cpp:66
> +    bool tryMathData = true;

should this be initialized with false so that it's explicit who is testing for math?

[Frédéric Wang (:fredw)]

Created attachment 227931 [details]
Patch

I've refined tryMathData so that only platforms that will have ENABLE(OPENTYPE_MATH) may set it to true. Also for freetype, there is an ASSERT(hash()) when creating the verticalData. I'm not sure when the hash() can be zero on freetype, but I leave tryMathData to false in that case too.

[Frédéric Wang (:fredw)]

Committed r166364: <http://trac.webkit.org/changeset/166364>

[Simon Fraser (smfr)]

This caused a new assertion
http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r166371%20(4179)/svg/text/text-fonts-02-t-crash-log.txt

[WebKit Commit Bot]

Re-opened since this is blocked by bug 130872

[Frédéric Wang (:fredw)]

Created attachment 228073 [details]
Patch

So the new assertion was

ASSERT(!lookupForWriting(Extractor::extract(entry)).second)

which is, IIUC, that the entry already exists in the hash table.

I now realize that what FontCache does for vertical data is very weird: it uses the hash() of the FontPlatformData as a key for the hash table (and so of course this key is itself hashed again!). So two fonts with different Open Type tables are likely to get the same key...

Open Type tables are encoded in the font file, so they should really be per FontPlatformData and I'm even assuming per SimpleFontData (unless two SimpleFontData's can share the same FontPlatformData). So I'm not really sure it is relevant to use a cache for the Open Type tables.

Also, there are not many fonts with Open Type MATH tables and the math data are likely to only be used in the MathML code. So trying to initialize the data only the first time the mathData() is called is probably a better strategy.

In this patch, I've removed the cache and moved the initialization to mathData().

[chris fleizach]

Comment on attachment 228073 [details]
Patch

Please watch the bots after landing

[Frédéric Wang (:fredw)]

Committed r166633: <http://trac.webkit.org/changeset/166633>