Last Comment Bug 611780 - gfxFT2Utils prefers measuring "x" over x-height metric of font
: gfxFT2Utils prefers measuring "x" over x-height metric of font
Status: NEW
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All Linux
: -- normal with 2 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-12 11:37 PST by David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
Modified: 2010-11-15 02:58 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.04 KB, patch)
2010-11-12 11:40 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
jfkthame: review-
Details | Diff | Splinter Review

Description David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-11-12 11:37:55 PST
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 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-11-12 11:40:14 PST
Created attachment 490181 [details] [diff] [review]
patch
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-11-12 11:43:24 PST
(Actually, we also measure an 'x' in gfxGDIFont, but we use the font metrics in gfxMacFont and gfxDWriteFonts.)
Comment 3 Karl Tomlinson (:karlt) 2010-11-14 20:46:18 PST
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."

http://www.w3.org/TR/CSS21/syndata.html#length-units

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."

http://www.w3.org/TR/css3-linebox/#inline1

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 4 Jonathan Kew (:jfkthame) 2010-11-15 02:58:59 PST
Comment on attachment 490181 [details] [diff] [review]
patch

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.

Note You need to log in before you can comment on or make changes to this bug.