gfxFT2Utils prefers measuring "x" over x-height metric of font




7 years ago
7 years ago


(Reporter: dbaron, Unassigned)



Firefox Tracking Flags

(Not tracked)



(1 attachment)



7 years ago
gfxFT2Utils's code for getting the x-height of a font prefers measuring an "x" over the metric from the font.  This is different from what we do on other platforms.

It leads to accumulation of rounding error that causes a number of tests in the CSS 2.1 test suite to fail on Linux, since the measurement of the "x" ends up rounded.

Comment 1

7 years ago
Created attachment 490181 [details] [diff] [review]
Attachment #490181 - Flags: review?(jfkthame)

Comment 2

7 years ago
(Actually, we also measure an 'x' in gfxGDIFont, but we use the font metrics in gfxMacFont and gfxDWriteFonts.)
I assume this is the relevant text:

"The 'ex' unit is defined by the element's first available font. The 'x-height' is so called because it is often equal to the height of the lowercase "x". However, an 'ex' is defined even for fonts that do not contain an "x".

The x-height of a font can be found in different ways. Some fonts contain reliable metrics for the x-height. If reliable font metrics are not available, UAs may determine the x-height from the height of a lowercase glyph. One possible heuristic is to look at how far the glyph for the lowercase "o" extends below the baseline, and subtract that value from the top of its bounding box. In the cases where it is impossible or impractical to determine the x-height, a value of 0.5em should be used."

I thought I recalled reading that em units should be the em of the font actually used, which may only be a nearest match to the requested font-size (which differs significantly with bitmap fonts), though I don't see that explicitly spelled out.

It may be implied here:

"When more than one font-size is used (this could happen when glyphs are found in different fonts), it is recommended that the largest font-size provides the em square and the cell-height."

font-size-adjust can also cause the actual font-size to differ from requested.

If em units should be actual font-sizes then perhaps ex units should too.

Do Quartz and DWrite do grid-fitting/hinting of horizontal glyph stems?
(I know the whole glyph is snapped vertically, but I don't recall what happens with parts of glyphs.)  I don't notice fitting of vertical stems (but glyphs are not snapped horizontally - at least, any snapping is subpixel).  Using raw font metrics makes more sense with systems that don't hint glyphs.
Comment on attachment 490181 [details] [diff] [review]

This looks fine as far as it goes, but I think we should consider some additional issues while we're here.

>     // Prefering a measured x over sxHeight because sxHeight doesn't consider

The comment here is no longer true with this patch... we'll be preferring sxHeight.

>     if (GetCharExtents('x', &extents) && extents.y_bearing < 0.0) {
>         aMetrics->xHeight = -extents.y_bearing;
>         aMetrics->aveCharWidth = extents.x_advance;
>     } else {

I'd expect the common case is going to be that OS/2 metrics are available - this will be true for (virtually) all OpenType/TrueType fonts, which are becoming increasingly prevalent. Therefore, I think it would make sense to reorder the code so that we try setting xHeight and aveCharWidth from the OS/2 metrics first, and only call GetCharExtents() at all if the OS/2 metrics are not available.

>+        // CSS 2.1, section 4.3.2 Lengths: "In the cases where it is
>+        // impossible or impractical to determine the x-height, a value of
>+        // 0.5em should be used."
>+        aMetrics->xHeight = 0.5 * emHeight;

Hmmm. Yes, I see that's what CSS 2.1 says, but our code on other platforms uses (0.56 * ascent), which will typically evaluate to somewhat less than (0.5 * emHeight). I'm not sure where that number originated, but I'd be strongly in favor of harmonizing this across platforms one way or the other. More specifically, I'd prefer that we use 0.56 * ascent everywhere as I think it's a more sensible guess.

So minusing for now, pending consideration of the above.
Attachment #490181 - Flags: review?(jfkthame) → review-
You need to log in before you can comment on or make changes to this bug.