There exist X fonts where max_bounds.ascent and max_bounds.descent are smaller than the (overall) ascent and descent in the XFontStruct. These fonts are currently rendered with a narrower line spacing than what is desireable (in my opinion), and certainly a narrower line spacing than Netscape 4.x renders them. One such font is Chris Cannam's Bitmap Garamond BDF font, which I use as my default browser font. You can find it at the URL http://www.all-day-breakfast.com/cannam/garamond.html Mozilla renders lines in this font two pixels closer to each other than Netscape 4.x does, and as a result not insignificantly reduces the readability of the resulting pages. The X font -cannam-garamond-medium-r-normal--14-140-75-75-p-90-iso8859-1 has ascent 14, descent 5, max_bounds.ascent 13, and max_bounds.descent 4. (As shown by 'xlsfonts -ll'.) The manpages (and Xlib documentation) for XQueryFont() strongly implies that the ascent and descent values are what you should use for interline spacing (while the max_bounds versions merely serve as the top and bottom of the area outside of which the font will never draw pixels). At the moment nsFontMetricsGTK::RealizeFont primarily drives interline spacing through mHeight, which is currently calculated off of the max_bounds values instead of the straight values. This appears to be wrong. There is another problem in the woods: various pieces of code use mMaxAscent (and presumably mMaxDescent), via GetMaxAscent et al. Quite possibly these routines, or the callers of these routines, suffer from the same confusion that the calculation of mHeight has. The real solution may be to split things out so that callers explicitly ask for either the font's inter-line spacing (either ascent or descent as appropriate in the face of changing fonts), which would use font ascent and descent, or for bounding box information on the characters involved, which would use max_bounds.ascent and max_bounds.descent as the quick approach. At the advice of people on #mozilla on irc.mozilla.org, this bug report is being cc:'d to a couple of Netscape people.
Created attachment 7770 [details] [diff] [review] Obvious patch to fix the mHeight calculation (compiles, works on my test case)
Summary: Mozilla may use incorrect line spacing due to GTK confusion between font max_bounds.ascent vs font ascent (and ditto descent) → Mozilla may use incorrect line spacing due to GTK confusion between font max_bounds.ascent vs font ascent (and ditto descent)
Kevin, this is a gfx problem
Assignee: troy → pavlov
Component: Layout → GTK Embedding Widget
QA Contact: petersen → pavlov
Pav, I don't know whether this is a the right component either. I usually assign these bugs to Kevin, but I changed the component and it went to you
Yes, this is a GFX problem, and I normally file those under the Layout component. Also, I'm the owner of X font height issues, so I'm reassigning it to myself.
Assignee: pavlov → erik
Status: UNCONFIRMED → NEW
Component: GTK Embedding Widget → Layout
Ever confirmed: true
Target Milestone: --- → M17
The problem is that Adobe's "times" on Unix has a smaller (font ascent + descent) than (max ascent + descent). Mozilla's default is adobe-times, so it's quite important to us. Windows's Times New Roman gives a max height of 19 when the 16-pixel font is requested, and Mozilla uses the max height for line-height "normal". Times New Roman reports 20 for the recommended line spacing, but Mozilla ignores that (and Nav4 and IE ignore it too). Adobe's times on Unix reports a max height of 19 for the 17-pixel font, so I chose to use the max height for the line spacing. This is indeed different from UnixNav4. In short, X fonts, as usual, are a mess. It's infuriating. Maybe we need to come up with some hack again. How about using the (font ascent + descent) if it's larger than the (max ascent + descent)?
Status: NEW → ASSIGNED
Looking at http://slashdot.org/ I can see that just ascent + descent causes problems (I wish I knew what NS 4.xx does, because its approach doesn't). It looks better with mHeight being the maximum of ascent + descent and max_bounds.ascent + max_bounds.descent.
Further testing with mHeight being the maximum of the max_bounds ascent and descent and the regular one has shown that this has problems too. Some fonts (especially some TrueType fonts) seem to have max_bounds that are far out on their regular ascent/descent figures, and cause the linespacing to be uncomfortably wide. Times New Roman at relatively smallish sizes (as used on Salon.com, for example) seems prone to this in my configuration. (You'll need to patch the GTK code to allow you to use Times New Roman at all to see this, which is another already-existing bug.) So a plain MAX() doesn't seem to be the solution. I'm experimenting with allowing things to grow somewhat larger based on the difference between the max_bounds and the normal values. Erik, do you have any figures on what the typical and maximum differences are between the max_bounds and the regular values that you've seen?
I'm beginning to think that we should Just Say No. Instead of taking the max ascent + max descent just because that makes adobe-times on X look like Times New Roman on Windows, maybe we should just use font ascent + font descent, as prescribed by the XLFD spec, and then if people complain, refer them to the X people (e.g. XFree86) and have them fix the fonts themselves. It's probably not a very good idea to put circumstantial hacks into Mozilla, since they might bite us hard later.
I agree with ignoring max_bounds.ascent/descent. After some more experimentation, I've found that the important thing for making something like http://slashdot.org/ look right with using plain ascent/descent seems to be to have mMaxAscent and (to a lesser extent) mMaxDescent synchronized with the line height information in mHeight, which argues that they should be set to be the same as mAscent and mDescent. Another (potential) nit: mUnderlineOffset and mUnderlineSize duplicate the calculation of interline height (and at current do it differently from how mHeight itself is calculated, although if mHeight is changed to be ascent+descent they'll become right). I think that the thing to do with them is pull the height calculation out to a single point and drive mHeight and everything else from this single value.
I think we should fix this when we clean up the nsIFontMetrics API and its callers. Currently, the layout engine treats line-height "normal" as 1.0, and multiplies line-height by GetHeight, which is actually the bounding box, not the em square. This needs to be fixed anyway, and so we might as well fix this bug at that time too. See bug 27874.
Target Milestone: M17 → M16
See bug 27164. If people disagree with my plan for that bug, then we should at least correct the Unix version of Mozilla, to return font ascent + descent instead of max ascent + descent (from GetHeight).
Fixed bug 27164, so this is fixed now too.
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.