Summary: | Operator stretching: read the Open Type MATH table | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Frédéric Wang (:fredw) <fred.wang> | ||||||||||||||||
Component: | MathML | Assignee: | Frédéric Wang (:fredw) <fred.wang> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | benjamin, bunhere, cfleizach, cmarcelo, commit-queue, gyuyoung.kim, mrobinson, rakuco, sergio | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | All | ||||||||||||||||||
OS: | All | ||||||||||||||||||
URL: | http://www.ntg.nl/maps/38/03.pdf | ||||||||||||||||||
Bug Depends on: | 130572 | ||||||||||||||||||
Bug Blocks: | 122297, 130322, 130325 | ||||||||||||||||||
Attachments: |
|
Description
Frédéric Wang (:fredw)
2014-03-17 00:11:57 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. 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> |