From 37b45fe0415bd6695640b26eb7545280b2e8ffe1 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Fri, 7 May 2010 16:23:10 +0200 Subject: Fix off-by-one in text layouts and widget size hints on Mac The mac font engine was made to return fractional values. This has exposed a few bugs where the rounded height of the font was used for text layouts and widget size hints. Since a fractional part of the font's height also creates pixels, rounding the height down in these cases is always wrong. The biggest culprit was QScriptLine::height. This is used to lay out text lines and returning a fractional height created problems all over the place. Rather than ceil the number in all places where it is used, we simply return a ceiled value for the line's height. In 4.6, this value would be qRound(ascent)+qRound(descent)+1, which would in some cases cause the previous and current line to overlap by one pixel. By ceiling the sum of the two, we guarantee that the text line contains the complete bounds of the font. A note about the removal of the code that gets the font metrics for QSmallFont: The condition would never be true since this is inside a branch which is only evaluated if fontIsSet is true. Task-number: QTBUG-9971 Reviewed-by: Gunnar --- src/gui/styles/qmacstyle_mac.mm | 20 +++++++++++--------- src/gui/text/qtextengine_p.h | 2 +- src/gui/widgets/qcombobox.cpp | 3 ++- src/gui/widgets/qcommandlinkbutton.cpp | 3 ++- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/gui/styles/qmacstyle_mac.mm b/src/gui/styles/qmacstyle_mac.mm index f5b0b0c..e065bcc 100644 --- a/src/gui/styles/qmacstyle_mac.mm +++ b/src/gui/styles/qmacstyle_mac.mm @@ -97,6 +97,7 @@ #include #include #include +#include #include #include #include @@ -2142,6 +2143,9 @@ int QMacStyle::pixelMetric(PixelMetric metric, const QStyleOption *opt, const QW // The combo box popup has no frame. if (qstyleoption_cast(opt) != 0) ret = 0; + // Frame of mac style line edits is two pixels on top and one on the bottom + else if (qobject_cast(widget) != 0) + ret = 2; else ret = 1; break; @@ -5437,14 +5441,12 @@ QRect QMacStyle::subControlRect(ComplexControl cc, const QStyleOptionComplex *op CGFloat height; QCFString groupText = qt_mac_removeMnemonics(groupBox->text); HIThemeGetTextDimensions(groupText, 0, &tti, &width, &height, 0); - tw = int(width); - h = int(height); + tw = qRound(width); + h = qCeil(height); } else { - QFontMetrics fm = groupBox->fontMetrics; - if (!checkable && !fontIsSet) - fm = QFontMetrics(qt_app_fonts_hash()->value("QSmallFont", QFont())); - h = fm.height(); - tw = fm.size(Qt::TextShowMnemonic, groupBox->text).width(); + QFontMetricsF fm = QFontMetricsF(groupBox->fontMetrics); + h = qCeil(fm.height()); + tw = qCeil(fm.size(Qt::TextShowMnemonic, groupBox->text).width()); } ret.setHeight(h); @@ -5491,10 +5493,10 @@ QRect QMacStyle::subControlRect(ComplexControl cc, const QStyleOptionComplex *op fm = QFontMetrics(qt_app_fonts_hash()->value("QSmallFont", QFont())); yOffset = 5; if (hasNoText) - yOffset = -fm.height(); + yOffset = -qCeil(QFontMetricsF(fm).height()); } - ret = opt->rect.adjusted(0, fm.height() + yOffset, 0, 0); + ret = opt->rect.adjusted(0, qCeil(QFontMetricsF(fm).height()) + yOffset, 0, 0); if (sc == SC_GroupBoxContents) ret.adjust(3, 3, -3, -4); // guess } diff --git a/src/gui/text/qtextengine_p.h b/src/gui/text/qtextengine_p.h index 5054b66..d92148f 100644 --- a/src/gui/text/qtextengine_p.h +++ b/src/gui/text/qtextengine_p.h @@ -389,7 +389,7 @@ struct Q_AUTOTEST_EXPORT QScriptLine mutable uint gridfitted : 1; uint hasTrailingSpaces : 1; uint leadingIncluded : 1; - QFixed height() const { return ascent + descent + 1 + QFixed height() const { return (ascent + descent).ceil() + 1 + (leadingIncluded? qMax(QFixed(),leading) : QFixed()); } QFixed base() const { return ascent + (leadingIncluded ? qMax(QFixed(),leading) : QFixed()); } diff --git a/src/gui/widgets/qcombobox.cpp b/src/gui/widgets/qcombobox.cpp index ca58e6d..e0b09aa 100644 --- a/src/gui/widgets/qcombobox.cpp +++ b/src/gui/widgets/qcombobox.cpp @@ -56,6 +56,7 @@ #include #include #include +#include #ifndef QT_NO_IM #include "qinputcontext.h" #endif @@ -328,7 +329,7 @@ QSize QComboBoxPrivate::recomputeSizeHint(QSize &sh) const // height - sh.setHeight(qMax(fm.height(), 14) + 2); + sh.setHeight(qMax(qCeil(QFontMetricsF(fm).height()), 14) + 2); if (hasIcon) { sh.setHeight(qMax(sh.height(), iconSize.height() + 2)); } diff --git a/src/gui/widgets/qcommandlinkbutton.cpp b/src/gui/widgets/qcommandlinkbutton.cpp index 6919bc0..e8fe299 100644 --- a/src/gui/widgets/qcommandlinkbutton.cpp +++ b/src/gui/widgets/qcommandlinkbutton.cpp @@ -46,6 +46,7 @@ #include "qtextlayout.h" #include "qcolor.h" #include "qfont.h" +#include #include "private/qpushbutton_p.h" @@ -242,7 +243,7 @@ int QCommandLinkButtonPrivate::descriptionHeight(int widgetWidth) const } layout.endLayout(); } - return qRound(descriptionheight); + return qCeil(descriptionheight); } /*! -- cgit v0.12