Looking at the CSS 2.1 test suite on Linux, I'm seeing a lot of tests that are affected by what seems likely to be a rounding bug in our font code. There are a significant number of font sizes at which the max extents of the Ahem font (i.e., max ascent + max descent, which we use for the size of the background on an inline element) come out 1px larger than the actual size. This isn't supposed to happen for Ahem, but it does. See, for example: http://software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20HTML%3E%0A%3Cstyle%3E%0Aspan%20%7B%20background%3A%20red%3B%20color%3A%20green%3B%20font%3A%2032px%2F1%20Ahem%20%7D%0A%3C%2Fstyle%3E%0A%3Cspan%3EHello%3C%2Fspan%3E There, it happens with a 32px font size, but it also happens at other sizes. I might poke into this in a bit.
Well, I've traced this down to gfxFT2LockedFace::GetMetrics, where the numbers we get out of freetype already appear to be rounded-up: for 32px Ahem freetype gives us ascender of exactly 26 and descender of exactly 7. (Really they should be 25.6 and 6.4.)
Created attachment 484164 [details] [diff] [review] possible patch This fixes it, although I really don't know if it's something we'd want to do. I'd note there is a bunch of rounding at the bottom of the function, though, which might make this a little less scary.
I'd note that I tried using height/ascender and height/descender instead of ascender/descender; that didn't work because of the code checking that maxDescent wasn't smaller than emDescent, etc.
This patch causes underlines to be too close to the text some portion of the time. I think I can make my other idea work, though.
Created attachment 490164 [details] [diff] [review] less problematic patch This patch seems less problematic. Thoughts?
Actually, I think this is still wrong since we end up drawing the text 1px higher than we should in the cases where this reduces the size by 1px.
Hmmm, I think the observed behavior suggests we're positioning the glyphs relative to their baseline, so both ways of allocating the reduction break some of the time. (In any case, both ways break some of the time.)
Comment on attachment 490164 [details] [diff] [review] less problematic patch AFAIK ftMetrics.height is a line-height here and that's not what is wanted for maxAscent and maxDescent (because it "height" includes leading). http://freetype.sourceforge.net/freetype2/docs/reference/ft2-base_interface.html#FT_Size_Metrics http://freetype.sourceforge.net/freetype2/docs/reference/ft2-base_interface.html#FT_FaceRec
In a pixellated world it's not generally possible to achieve all the desirable properties and it's not clear which are the preferred properties to maintain. And, as you mention, the baseline alignment makes choosing ascent and descent difficult, and in reality the number of affected pixels will be the ceiling of (absolute value of) each in each direction. It might be worth experimenting with unrounded maxAscent and maxDescent, which could be obtained from the FT_FaceRec mFace (as underline_position is used). It may be best not to round ascent in GetMetrics if rounding to nearest as glyphs will also be snapped to pixels based on non-pixel layout offsets. Rounding absolute positions and snapping can add the errors (sometimes, and cancel others). A risk with not using the ceiling of ascent and descent comes from the fact that we don't measure glyph bounds for normal text sizes and instead rely on maxAscent and maxDescent. Perhaps we should use a font bbox ceiling for maximum glyph extents, and probably not call the layout metrics *max*, because they are not. I /think/ the underline offset really should be rounded because it is the distance *between* two snapped items, though I don't see that happening. When the distance between them is integer, both snapped items get snapped in the same direction (assuming consistent pixel snapping). Maybe that was part of the problem with underlines mentioned in comment 4? Though there is also the problem that glyphs get hinted and at small sizes and those order-1-pixel changes are significant. Rounding metrics up tends to get lucky and compensate for this. I wonder whether it is unfair for a test with non-pixel-aligned font metrics to expect floating point accuracy. The glyphs will cover an integer number of pixels.
Comment on attachment 490164 [details] [diff] [review] less problematic patch As per the preceding few comments, I don't think it's clear exactly how we should proceed here at this point. What I'd like to do would be to re-examine how we obtain and use font metrics across all platforms/backends, with a view to achieving more consistent behavior, as we currently have a number of unfortunate discrepancies. (See, for example, bug 598900 and bug 657864.) I think we should try reading metrics directly from the 'sfnt' tables for TrueType/OpenType fonts, in order to give us well-understood, consistent results, and fall back to other metrics APIs such as FT2, GDI, DWrite, Core Text, etc only for the (increasingly rare) case where we're using some other kind of font. However, this will involve a more extensive patch cutting across all backends, and will obviously need to be carefully tested. Currently, there is code used on OS X to read metrics from sfnt tables, and we could reuse much of this across other platforms; however, I'm not sure that it is entirely correct at this point (as illustrated by bug 657864 comment #0, where the Mac metrics appear to be significantly off).
Test: http://test.csswg.org/source/approved/css2.1/src/css1/c526-font-sz-001.xht Its reftest: http://test.csswg.org/source/approved/css2.1/src/css1/c526-font-sz-001-ref.xht OS -> Linux
Jonathan: I'd like to reach out to the Linux OS text engine developers if we need their help to fix this. Can you describe the desired API for the OS bug? If we start reading metrics directly from the 'sfnt' tables on Linux, is that preferable to using the OS API?
Hello all, I believe this bug is very important to fix. It annoys me tremendously and every single day. One consequence of this bug is that it makes Firefox's DOM inspector NOT reliable: what I can see rendered on a page is not what DOM inspector will report. I understand that Firefox developers can not fix this bug and why. I would like to know if this bug has been filed UPSTREAM to Linux font/text rendering engine developers. If so, then what is the bug report URL. Thank you, Gérard
(In reply to Jonathan Kew (:jfkthame) from comment #10) > What I'd like to do would be to re-examine how we obtain and use font > metrics across all platforms/backends, with a view to achieving more > consistent behavior, as we currently have a number of unfortunate > discrepancies. (See, for example, bug 598900 and bug 657864.) > > I think we should try reading metrics directly from the 'sfnt' tables for > TrueType/OpenType fonts, in order to give us well-understood, consistent > results, and fall back to other metrics APIs such as FT2, GDI, DWrite, Core > Text, etc only for the (increasingly rare) case where we're using some other > kind of font. However, this will involve a more extensive patch cutting > across all backends, and will obviously need to be carefully tested. This makes a lot of sense. I think how to test this needs to be thought out carefully, so that we know where differences vs. platform API's exist and why.
ought to look into whether this was fixed by https://hg.mozilla.org/mozilla-central/rev/1bd6b04a535f (bug 883997, despite the commit message).
I don't believe so. That patch just avoids the case where we sometimes use an unrounded dimension for one or other of ascent/descent, but it won't prevent them -both- being rounded up, and giving a sum that is greater than the (rounded) em-height.