Fusion: Consistent width calculation and rendering of menu items

The width calculation lacked the addition of the windowItemFrame
constant, which was added in the rendering code. This resulted in
ellision for long items.

To make the code easier to follow, const'ify, rename, and reorder some
local variables, and use constants from QFusionStylePrivate instead of
(now obsolete and removed) constants copied from the Windows style.

Break long lines as a drive-by.

Fixes: QTBUG-94481
Pick-to: 6.2
Change-Id: I6d3e9d0feebee58fc70beaaf29236b8473242083
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

1 files changed, 45 insertions, 39 deletions

diff --git a/src/widgets/styles/qfusionstyle.cpp b/src/widgets/styles/qfusionstyle.cpp
index 5b268aa684..00a80f565a 100644
--- a/src/widgets/styles/qfusionstyle.cpp
+++ b/src/widgets/styles/qfusionstyle.cpp
@@ -104,9 +104,7 @@ enum Direction {
 
 // from windows style
 static const int windowsItemFrame        =  2; // menu item frame width
-static const int windowsItemHMargin      =  3; // menu item hor text margin
 static const int windowsItemVMargin      =  8; // menu item ver text margin
-static const int windowsRightBorder      = 15; // right border on windows
 
 static const int groupBoxBottomMargin    =  0;  // space below the groupbox
 static const int groupBoxTopMargin       =  3;
@@ -1548,34 +1546,36 @@ void QFusionStyle::drawControl(ControlElement element, const QStyleOption *optio
                 const int margin = int(QStyleHelper::dpiScaled(5, option));
                 if (!menuItem->text.isEmpty()) {
                     painter->setFont(menuItem->font);
-                    proxy()->drawItemText(painter, menuItem->rect.adjusted(margin, 0, -margin, 0), Qt::AlignLeft | Qt::AlignVCenter,
+                    proxy()->drawItemText(painter, menuItem->rect.adjusted(margin, 0, -margin, 0),
+                                          Qt::AlignLeft | Qt::AlignVCenter,
                                           menuItem->palette, menuItem->state & State_Enabled, menuItem->text,
                                           QPalette::Text);
                     w = menuItem->fontMetrics.horizontalAdvance(menuItem->text) + margin;
                 }
                 painter->setPen(shadow.lighter(106));
-                bool reverse = menuItem->direction == Qt::RightToLeft;
+                const bool reverse = menuItem->direction == Qt::RightToLeft;
                 painter->drawLine(menuItem->rect.left() + margin + (reverse ? 0 : w), menuItem->rect.center().y(),
                                   menuItem->rect.right() - margin - (reverse ? w : 0), menuItem->rect.center().y());
                 painter->restore();
                 break;
             }
-            bool selected = menuItem->state & State_Selected && menuItem->state & State_Enabled;
+            const bool selected = menuItem->state & State_Selected && menuItem->state & State_Enabled;
             if (selected) {
                 QRect r = option->rect;
                 painter->fillRect(r, highlight);
                 painter->setPen(QPen(highlightOutline));
                 painter->drawRect(QRectF(r).adjusted(0.5, 0.5, -0.5, -0.5));
             }
-            bool checkable = menuItem->checkType != QStyleOptionMenuItem::NotCheckable;
-            bool checked = menuItem->checked;
-            bool sunken = menuItem->state & State_Sunken;
-            bool enabled = menuItem->state & State_Enabled;
+            const bool checkable = menuItem->checkType != QStyleOptionMenuItem::NotCheckable;
+            const bool checked = menuItem->checked;
+            const bool sunken = menuItem->state & State_Sunken;
+            const bool enabled = menuItem->state & State_Enabled;
 
-            bool ignoreCheckMark = false;
-            const int checkColHOffset = windowsItemHMargin + windowsItemFrame - 1;
+            const int checkColHOffset = QFusionStylePrivate::menuItemHMargin + windowsItemFrame - 1;
+            // icon checkbox's highlight column width
             int checkcol = qMax<int>(menuItem->rect.height() * 0.79,
-                                     qMax<int>(menuItem->maxIconWidth, dpiScaled(21, option))); // icon checkbox's highlight column width
+                                     qMax<int>(menuItem->maxIconWidth, dpiScaled(21, option)));
+            bool ignoreCheckMark = false;
             if (
 #if QT_CONFIG(combobox)
                 qobject_cast<const QComboBox*>(widget) ||
@@ -1587,8 +1587,9 @@ void QFusionStyle::drawControl(ControlElement element, const QStyleOption *optio
                 // Check, using qreal and QRectF to avoid error accumulation
                 const qreal boxMargin = dpiScaled(3.5, option);
                 const qreal boxWidth = checkcol - 2 * boxMargin;
-                QRectF checkRectF(option->rect.left() + boxMargin + checkColHOffset, option->rect.center().y() - boxWidth/2 + 1, boxWidth, boxWidth);
-                QRect checkRect = checkRectF.toRect();
+                QRect checkRect = QRectF(option->rect.left() + boxMargin + checkColHOffset,
+                                         option->rect.center().y() - boxWidth/2 + 1, boxWidth,
+                                         boxWidth).toRect();
                 checkRect.setWidth(checkRect.height()); // avoid .toRect() round error results in non-perfect square
                 checkRect = visualRect(menuItem->direction, menuItem->rect, checkRect);
                 if (checkable) {
@@ -1598,8 +1599,10 @@ void QFusionStyle::drawControl(ControlElement element, const QStyleOption *optio
                             painter->setRenderHint(QPainter::Antialiasing);
                             painter->setPen(Qt::NoPen);
 
-                            QPalette::ColorRole textRole = !enabled ? QPalette::Text:
-                                                                      selected ? QPalette::HighlightedText : QPalette::ButtonText;
+                            QPalette::ColorRole textRole = !enabled
+                                                         ? QPalette::Text :
+                                                           selected ? QPalette::HighlightedText
+                                                                    : QPalette::ButtonText;
                             painter->setBrush(option->palette.brush( option->palette.currentColorGroup(), textRole));
                             const int adjustment = checkRect.height() * 0.3;
                             painter->drawEllipse(checkRect.adjusted(adjustment, adjustment, -adjustment, -adjustment));
@@ -1626,8 +1629,8 @@ void QFusionStyle::drawControl(ControlElement element, const QStyleOption *optio
             }
 
             // Text and icon, ripped from windows style
-            bool dis = !(menuItem->state & State_Enabled);
-            bool act = menuItem->state & State_Selected;
+            const bool dis = !(menuItem->state & State_Enabled);
+            const bool act = menuItem->state & State_Selected;
             const QStyleOption *opt = option;
             const QStyleOptionMenuItem *menuitem = menuItem;
 
@@ -1675,36 +1678,38 @@ void QFusionStyle::drawControl(ControlElement element, const QStyleOption *optio
             }
             int x, y, w, h;
             menuitem->rect.getRect(&x, &y, &w, &h);
-            int tab = menuitem->reservedShortcutWidth;
             QColor discol;
             if (dis) {
                 discol = menuitem->palette.text().color();
                 p->setPen(discol);
             }
-            int xm = checkColHOffset + checkcol + windowsItemHMargin;
-            int xpos = menuitem->rect.x() + xm;
+            const int xm = checkColHOffset + checkcol + QFusionStylePrivate::menuItemHMargin;
+            const int xpos = menuitem->rect.x() + xm;
 
-            QRect textRect(xpos, y + windowsItemVMargin, w - xm - windowsRightBorder - tab + 1, h - 2 * windowsItemVMargin);
-            QRect vTextRect = visualRect(opt->direction, menuitem->rect, textRect);
+            const QRect textRect(xpos, y + windowsItemVMargin,
+                                 w - xm - QFusionStylePrivate::menuRightBorder - menuitem->reservedShortcutWidth + 2,
+                                 h - 2 * windowsItemVMargin);
+            const QRect vTextRect = visualRect(opt->direction, menuitem->rect, textRect);
             QStringView s(menuitem->text);
             if (!s.isEmpty()) {                     // draw text
                 p->save();
-                int t = s.indexOf(QLatin1Char('\t'));
+                const int tabIndex = s.indexOf(QLatin1Char('\t'));
                 int text_flags = Qt::AlignVCenter | Qt::TextShowMnemonic | Qt::TextDontClip | Qt::TextSingleLine;
                 if (!proxy()->styleHint(SH_UnderlineShortcut, menuitem, widget))
                     text_flags |= Qt::TextHideMnemonic;
                 text_flags |= Qt::AlignLeft;
-                if (t >= 0) {
+                if (tabIndex >= 0) {
                     QRect vShortcutRect = visualRect(opt->direction, menuitem->rect,
-                                                     QRect(textRect.topRight(), QPoint(menuitem->rect.right(), textRect.bottom())));
-                    const QString textToDraw = s.mid(t + 1).toString();
+                                                     QRect(textRect.topRight(),
+                                                     QPoint(menuitem->rect.right(), textRect.bottom())));
+                    const QString textToDraw = s.mid(tabIndex + 1).toString();
                     if (dis && !act && proxy()->styleHint(SH_EtchDisabledText, option, widget)) {
                         p->setPen(menuitem->palette.light().color());
                         p->drawText(vShortcutRect.adjusted(1, 1, 1, 1), text_flags, textToDraw);
                         p->setPen(discol);
                     }
                     p->drawText(vShortcutRect, text_flags, textToDraw);
-                    s = s.left(t);
+                    s = s.left(tabIndex);
                 }
                 QFont font = menuitem->font;
                 // font may not have any "hard" flags set. We override
@@ -1718,23 +1723,24 @@ void QFusionStyle::drawControl(ControlElement element, const QStyleOption *optio
                     font.setBold(true);
 
                 p->setFont(font);
-                QString textToDraw = s.left(t).toString();
+                const QFontMetrics fontMetrics(font);
+                const QString textToDraw = fontMetrics.elidedText(s.left(tabIndex).toString(),
+                                                                  Qt::ElideMiddle, vTextRect.width());
                 if (dis && !act && proxy()->styleHint(SH_EtchDisabledText, option, widget)) {
                     p->setPen(menuitem->palette.light().color());
                     p->drawText(vTextRect.adjusted(1, 1, 1, 1), text_flags, textToDraw);
                     p->setPen(discol);
                 }
-                textToDraw = menuitem->fontMetrics.elidedText(textToDraw, Qt::ElideMiddle, vTextRect.width());
                 p->drawText(vTextRect, text_flags, textToDraw);
                 p->restore();
             }
 
             // Arrow
             if (menuItem->menuItemType == QStyleOptionMenuItem::SubMenu) {// draw sub menu arrow
-                int dim = (menuItem->rect.height() - 4) / 2;
+                const int dim = (menuItem->rect.height() - 4) / 2;
                 PrimitiveElement arrow;
                 arrow = option->direction == Qt::RightToLeft ? PE_IndicatorArrowLeft : PE_IndicatorArrowRight;
-                int xpos = menuItem->rect.left() + menuItem->rect.width() - 3 - dim;
+                const int xpos = menuItem->rect.left() + menuItem->rect.width() - 3 - dim;
                 QRect  vSubMenuRect = visualRect(option->direction, menuItem->rect,
                                                  QRect(xpos, menuItem->rect.top() + menuItem->rect.height() / 2 - dim / 2, dim, dim));
                 QStyleOptionMenuItem newMI = *menuItem;
@@ -3175,22 +3181,22 @@ QSize QFusionStyle::sizeFromContents(ContentsType type, const QStyleOption *opti
     case CT_MenuItem:
         if (const QStyleOptionMenuItem *menuItem = qstyleoption_cast<const QStyleOptionMenuItem *>(option)) {
             int w = size.width(); // Don't rely of QCommonStyle's width calculation here
-            int maxpmw = menuItem->maxIconWidth;
-            int tabSpacing = 20;
             if (menuItem->text.contains(QLatin1Char('\t')))
-                w += tabSpacing;
+                w += menuItem->reservedShortcutWidth;
             else if (menuItem->menuItemType == QStyleOptionMenuItem::SubMenu)
                 w += 2 * QStyleHelper::dpiScaled(QFusionStylePrivate::menuArrowHMargin, option);
             else if (menuItem->menuItemType == QStyleOptionMenuItem::DefaultItem) {
-                QFontMetrics fm(menuItem->font);
+                const QFontMetrics fm(menuItem->font);
                 QFont fontBold = menuItem->font;
                 fontBold.setBold(true);
-                QFontMetrics fmBold(fontBold);
+                const QFontMetrics fmBold(fontBold);
                 w += fmBold.horizontalAdvance(menuItem->text) - fm.horizontalAdvance(menuItem->text);
             }
             const qreal dpi = QStyleHelper::dpi(option);
-            const int checkcol = qMax<int>(maxpmw, QStyleHelper::dpiScaled(QFusionStylePrivate::menuCheckMarkWidth, dpi)); // Windows always shows a check column
-            w += checkcol;
+             // Windows always shows a check column
+            const int checkcol = qMax<int>(menuItem->maxIconWidth,
+                                           QStyleHelper::dpiScaled(QFusionStylePrivate::menuCheckMarkWidth, dpi));
+            w += checkcol + windowsItemFrame;
             w += QStyleHelper::dpiScaled(int(QFusionStylePrivate::menuRightBorder) + 10, dpi);
             newSize.setWidth(w);
             if (menuItem->menuItemType == QStyleOptionMenuItem::Separator) {