RESOLVED FIXED 130324
Operator stretching: read the Open Type MATH table
https://bugs.webkit.org/show_bug.cgi?id=130324
Summary Operator stretching: read the Open Type MATH table
Frédéric Wang (:fredw)
Reported 2014-03-17 00:11:57 PDT
Third step for attachment 226844 [details]. On some platforms (Linux/Windows), we can directly read the MATH data from the font and thus get a generic support. On other platforms (Mac), we will still use the hardcoded data introduced in bug 130321. I'll add an ENABLE_OPENTYPE_MATH compilation flag for that purpose.
Attachments
WIP Patch (33.36 KB, patch)
2014-03-17 06:13 PDT, Frédéric Wang (:fredw)
no flags
Patch + bug 130572 for testing (42.95 KB, patch)
2014-03-21 04:11 PDT, Frédéric Wang (:fredw)
no flags
Patch + 130572 for testing (43.90 KB, patch)
2014-03-21 04:34 PDT, Frédéric Wang (:fredw)
no flags
Patch + 130572 for testing (43.40 KB, patch)
2014-03-21 04:52 PDT, Frédéric Wang (:fredw)
no flags
Patch (25.62 KB, patch)
2014-03-21 05:49 PDT, Frédéric Wang (:fredw)
no flags
Patch (26.17 KB, patch)
2014-03-24 09:31 PDT, Frédéric Wang (:fredw)
no flags
Patch (25.78 KB, patch)
2014-03-27 01:00 PDT, Frédéric Wang (:fredw)
cfleizach: review+
Frédéric Wang (:fredw)
Comment 1 2014-03-17 02:07:13 PDT
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.
Frédéric Wang (:fredw)
Comment 2 2014-03-17 06:13:14 PDT
Created attachment 226905 [details] WIP Patch This applies on top of bug 130321.
Frédéric Wang (:fredw)
Comment 3 2014-03-21 04:11:53 PDT
Created attachment 227410 [details] Patch + bug 130572 for testing
Frédéric Wang (:fredw)
Comment 4 2014-03-21 04:34:50 PDT
Created attachment 227414 [details] Patch + 130572 for testing
Frédéric Wang (:fredw)
Comment 5 2014-03-21 04:52:20 PDT
Created attachment 227416 [details] Patch + 130572 for testing
Frédéric Wang (:fredw)
Comment 6 2014-03-21 05:49:22 PDT
Frédéric Wang (:fredw)
Comment 7 2014-03-24 09:31:20 PDT
chris fleizach
Comment 8 2014-03-26 09:03:47 PDT
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
Frédéric Wang (:fredw)
Comment 9 2014-03-26 09:11:10 PDT
(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.
Frédéric Wang (:fredw)
Comment 10 2014-03-27 00:48:45 PDT
(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?
Frédéric Wang (:fredw)
Comment 11 2014-03-27 01:00:31 PDT
chris fleizach
Comment 12 2014-03-27 09:04:51 PDT
(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
Frédéric Wang (:fredw)
Comment 13 2014-04-02 06:02:07 PDT
Note You need to log in before you can comment on or make changes to this bug.