Bug 130324

For the MATH data, see http://mpeg.chiariglione.org/standards/mpeg-4/open-font-format/call-proposals-isoiec-14496-22-open-font-format-color-font I'm not sure if April's Committee Draft will be made public, but anyone interested in the MATH table can ask me a copy of Microsoft's document.

Created attachment 226905 [details] WIP Patch This applies on top of bug 130321.

Created attachment 227410 [details] Patch + bug 130572 for testing

Created attachment 227414 [details] Patch + 130572 for testing

Created attachment 227416 [details] Patch + 130572 for testing

Created attachment 227420 [details] Patch

Created attachment 227657 [details] Patch

Comment on attachment 227657 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227657&action=review > Source/WTF/wtf/Platform.h:1053 > +#if OS(DARWIN) || USE(FREETYPE) || (PLATFORM(WIN) && (USE(CG) || USE(CAIRO))) i think the OS(DARWIN) also needs the USE(CG) macro to go along with it right > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:44 > +#pragma pack(1) are you trying to save space here or is the required for reading math data? how much extra memory will be used by reading in this data? > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:78 > + if (!getCoverageIndex(buffer, coverage, glyph, i) || i>= count) i >= > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:162 > + OpenType::Offset vertGlyphCoverageOffset; my thought is we should just use "vertical" and horizontal instead of these abbreviations. The names are already really long anyway > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:242 > + const OpenType::MathConstants* mathconstants = math->mathConstants(*m_mathBuffer); mathConstants for the name > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:248 > + const OpenType::MathVariants* mathvariants = math->mathVariants(*m_mathBuffer); mathVariants > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:259 > + int32_t value = 0; seems like you could declare this as int16_t and avoid the casts when assigning the value > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:266 > + if (constant <= ScriptScriptPercentScaleDown) for these if/else statements it seems we should also check that constants >= 0 in this case (or rather ScriptPercentScaleDown I believe) and likewise is >= X in the other else if cases > Source/WebCore/platform/graphics/opentype/OpenTypeTypes.h:127 > + getCoverageIndex(const SharedBuffer& buffer, const CoverageTable* coverage, Glyph glyph, uint32_t& coverageIndex) const this should be on the same line as above

(In reply to comment #8) > > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:44 > > +#pragma pack(1) > > are you trying to save space here or is the required for reading math data? > how much extra memory will be used by reading in this data? > No ideas, I just copied that from OpenTypeVerticalData. I'm assuming it was necessary to prevent unexpected data rearrangement but didn't really think about it.

(In reply to comment #8) > > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:259 > > + int32_t value = 0; > > seems like you could declare this as int16_t and avoid the casts when assigning the value Some values are int16_t and others are uint16_t, so that would mean declaring two variables and duplicating the conversion to % / em?

Created attachment 227932 [details] Patch

(In reply to comment #10) > (In reply to comment #8) > > > Source/WebCore/platform/graphics/opentype/OpenTypeMathData.cpp:259 > > > + int32_t value = 0; > > > > seems like you could declare this as int16_t and avoid the casts when assigning the value > > Some values are int16_t and others are uint16_t, so that would mean declaring two variables and duplicating the conversion to % / em? as missed the unit v. int

Committed r166640: <http://trac.webkit.org/changeset/166640>