Bug#524062: Incorrectly reports font metrics, results in excess vertical spacing
Package: libqtgui4
Version: 4.4.3-2
Severity: normal
Tags: patch
Hi,
Qt 4 appears to incorrectly report font metrics, or at least, the behavior
deviates from that of Qt 3.3 which I believe it was intended to emulate.
In particular, a miscalculation of the size of a font's descent results in
an incorrect reporting of the font's height.
This is most apparent in bitmapped fonts used in Konsole, where box drawing
characters that should be column-contiguous are separated by unexpected
blank space between rows. Image [1] illustrates this phenomenon in Konsole
using the VGA bitmap font "console8x16" distributed with KDE 3.5. Image
[2] illustrates a rendering with correct metrics which is consistent with
both KDE 3.5's Konsole and xterm with the same font.
I believe this issue is the same as that reported in [3], where it was
suggested that the problem was due to a change in Qt 4's font rendering
engine, but it was unknown exactly where.
Looking into the issue, I think I've sorted out where the problem lies:
Konsole determines row spacing as a funciton of the font height as reported
by QFontMetrics::height. Both Qt 3.3 and 4.4 implement
QFontMetrics::height using the formula "ascent + descent + 1" where the
ascent and descent values are determined by the underlying font engines,
and the "+1" is to account for the base line.
The code that reports descent for the Xft font engine in Qt 3.3 is:
int QFontEngineXft::descent() const
{
return qRound((_font->descent-1)*_scale);
}
where the "_font->descent" value ultimately comes from the font itself.
There's no comment to explain what's intended by the "-1" here, but I
believe what's happening is that, at least for bitmapped fonts, the font
height is the sum of the font's ascent and descent values which includes
the base line. Qt defines "descent" as not including the base line, so the
above method must explicitly subtract it before scaling. The
QFontMetrics::height method then adds the base line back in when
determining the final height.
The net effect of all this is to create a ceiling round function, where for
unscaled bitmap fonts, QFontMetrics::height returns a value identical to
the font's "ascent+descent", and for scaled (outline) fonts, it returns
ceil((ascent+descent)*scale). This is desirable behavior since it
guarantes sufficient pixel space for scaled fonts, but maintains exact
spacing for bitmapped fonts.
In contrast, the code that reports descent for the FT font engine in Qt 4.4
is:
QFixed QFontEngineFT::descent() const
{
return QFixed::fromFixed(-metrics.descender + (1<<6));
}
Here, "metrics.descender" comes from FreeType which uses power-of-two
units, whereby 64 units comprise a single pixel. Thus it seems to me that
the "1<<6" factor is intended to subtract the base line (1<<6 == 64),
emulating the behavior of Qt 3.3. But there's a sign error here, and
instead it actually doubly adds the base line. This results in bitmapped
fonts being reported with descents and heights that are two pixels too
large. Since these factors serve to implement a ceiling round, the effect
of this error lessens for scaled fonts as the scaling factor increases.
Attached is a patch which corrects this sign error. With it, Konsole
renders fonts correctly. The patch does not have a noticable effect on
font _spacing_ in other applications, but correcting the "descent"
definition does alter font _position_. In particular, Qt now appears to
render glyphs one pixel lower in their bounding box.
As an example, here's illustrations of Konqueror's font rendering before
[4] and after [5] applying the patch. As far as I can tell, the only
difference is that every glyph is shifted down one pixel. As it turns out,
I believe this may actually be desired behavior. If you notice, the
location of underlines in the hyperlinks _don't_ change. Meaning, before
the patch there's two pixels of white-space between the font base line and
the underline, and after the patch there's only one. As it turns out, Qt
3.3 renders underlines the same way, with Konqueror 3.5 using single pixel
spaces for underlines too [6]. So in the end, I believe that this
"side-effect" is actually Qt 4's intended behavior, and nobody noticed the
bug previously.
Sorry for the wall-of-text explanation, but there's few comments in Qt's
source code that explains the intention behind font descent/height
calculations, so I had to infer the intended behavior based on what they
appear to do.
Thanks!
[1] http://www.club.cc.cmu.edu/~mkasick/libqtgui4/libQtGui_bad_metrics.png
[2] http://www.club.cc.cmu.edu/~mkasick/libqtgui4/libQtGui_good_metrics.png
[3] http://www.nabble.com/Ridiculously-large-Konsole-line-spacing-to21803579.html#a21803579
[4] http://www.club.cc.cmu.edu/~mkasick/libqtgui4/libQtGui_konq_old.png
[5] http://www.club.cc.cmu.edu/~mkasick/libqtgui4/libQtGui_konq_new.png
[6] http://www.club.cc.cmu.edu/~mkasick/libqtgui4/libQtGui_konq_35.png
diff -uNr qt4-x11-4.4.3.orig/src/gui/text/qfontengine_ft.cpp qt4-x11-4.4.3/src/gui/text/qfontengine_ft.cpp
--- qt4-x11-4.4.3.orig/src/gui/text/qfontengine_ft.cpp 2008-09-27 04:58:47.000000000 -0400
+++ qt4-x11-4.4.3/src/gui/text/qfontengine_ft.cpp 2009-04-14 09:35:44.398583558 -0400
@@ -1005,7 +1005,7 @@
QFixed QFontEngineFT::descent() const
{
- return QFixed::fromFixed(-metrics.descender + (1<<6));
+ return QFixed::fromFixed(-metrics.descender - (1<<6));
}
QFixed QFontEngineFT::leading() const
Reply to: