WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch + bug 130572 for testing
(42.95 KB, patch)
2014-03-21 04:11 PDT
,
Frédéric Wang (:fredw)
no flags
Details
Formatted Diff
Diff
Patch + 130572 for testing
(43.90 KB, patch)
2014-03-21 04:34 PDT
,
Frédéric Wang (:fredw)
no flags
Details
Formatted Diff
Diff
Patch + 130572 for testing
(43.40 KB, patch)
2014-03-21 04:52 PDT
,
Frédéric Wang (:fredw)
no flags
Details
Formatted Diff
Diff
Patch
(25.62 KB, patch)
2014-03-21 05:49 PDT
,
Frédéric Wang (:fredw)
no flags
Details
Formatted Diff
Diff
Patch
(26.17 KB, patch)
2014-03-24 09:31 PDT
,
Frédéric Wang (:fredw)
no flags
Details
Formatted Diff
Diff
Patch
(25.78 KB, patch)
2014-03-27 01:00 PDT
,
Frédéric Wang (:fredw)
cfleizach
: review+
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 227420
[details]
Patch
Frédéric Wang (:fredw)
Comment 7
2014-03-24 09:31:20 PDT
Created
attachment 227657
[details]
Patch
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
Created
attachment 227932
[details]
Patch
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
Committed
r166640
: <
http://trac.webkit.org/changeset/166640
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug