QMenu: Fix margins related display issues

Currently the contents margins and the menu paddings are not considered
for calculating the menu size, the positions and the size of tear-off
bar, scrollers and the positions of the menu items when scrolling the
menu, which results in the following problems when valid contents
margins and/or menu paddings are set:

- The tear off area is displayed in a wrong position. The mouse events
are not handled correctly in the tear off area. For example, when you
click in the tear off area, the menu should be torn off but nothing
happens
- For a multi-column menu, the menu width is not calculated correctly
- For a scrollable menu,
  - the menu width is not calculated correctly
  - the menu items are not displayed in correct positions
  - the scrollers are not displayed in correct positions
  - menu items are displayed on the area of borders and margins when
    scrolling the menu
  - the last menu item is not displayed above the bottom of the content
    area when scrolling the menu to the end.

The changes are to fix the problems above.

Change-Id: I7931e1088dff0029f2d4825e2aa34b4e32fdccd9
Reviewed-by: Gabriel de Dietrich <gabriel.dedietrich@qt.io>

2 files changed, 58 insertions, 21 deletions

diff --git a/src/widgets/widgets/qmenu.cpp b/src/widgets/widgets/qmenu.cpp
index 2b10ae7261..f08f573391 100644
--- a/src/widgets/widgets/qmenu.cpp
+++ b/src/widgets/widgets/qmenu.cpp
@@ -267,9 +267,6 @@ void QMenuPrivate::updateActionRects(const QRect &screen) const
 
     int lastVisibleAction = getLastVisibleAction();
 
-    int max_column_width = 0,
-        dh = screen.height(),
-        y = 0;
     QStyle *style = q->style();
     QStyleOption opt;
     opt.init(q);
@@ -279,6 +276,10 @@ void QMenuPrivate::updateActionRects(const QRect &screen) const
     const int fw = style->pixelMetric(QStyle::PM_MenuPanelWidth, &opt, q);
     const int deskFw = style->pixelMetric(QStyle::PM_MenuDesktopFrameWidth, &opt, q);
     const int tearoffHeight = tearoff ? style->pixelMetric(QStyle::PM_MenuTearoffHeight, &opt, q) : 0;
+    const int base_y = vmargin + fw + topmargin + (scroll ? scroll->scrollOffset : 0) + tearoffHeight;
+    int max_column_width = 0;
+    int dh = screen.height();
+    int y = base_y;
 
     //for compatibility now - will have to refactor this away
     tabWidth = 0;
@@ -356,11 +357,12 @@ void QMenuPrivate::updateActionRects(const QRect &screen) const
             max_column_width = qMax(max_column_width, sz.width());
             //wrapping
             if (!scroll &&
-               y+sz.height()+vmargin > dh - (deskFw * 2)) {
+               y + sz.height() + vmargin + bottommargin + fw > dh - (deskFw * 2)) {
                 ncols++;
-                y = vmargin;
+                y = base_y;
+            } else {
+                y += sz.height();
             }
-            y += sz.height();
             //update the item
             actionRects[i] = QRect(0, 0, sz.width(), sz.height());
         }
@@ -372,9 +374,6 @@ void QMenuPrivate::updateActionRects(const QRect &screen) const
     max_column_width = qMax(min_column_width, max_column_width);
 
     //calculate position
-    const int base_y = vmargin + fw + topmargin +
-        (scroll ? scroll->scrollOffset : 0) +
-        tearoffHeight;
     int x = hmargin + fw + leftmargin;
     y = base_y;
 
@@ -383,7 +382,7 @@ void QMenuPrivate::updateActionRects(const QRect &screen) const
         if (rect.isNull())
             continue;
         if (!scroll &&
-           y+rect.height() > dh - deskFw * 2) {
+           y + rect.height() + vmargin + bottommargin + fw > dh - deskFw * 2) {
             x += max_column_width + hmargin;
             y = base_y;
         }
@@ -761,7 +760,7 @@ QWidget *QMenuPrivate::topCausedWidget() const
 
 QAction *QMenuPrivate::actionAt(QPoint p) const
 {
-    if (!q_func()->rect().contains(p))     //sanity check
+    if (!rect().contains(p))     //sanity check
        return 0;
 
     for(int i = 0; i < actionRects.count(); i++) {
@@ -855,6 +854,19 @@ void QMenuPrivate::drawTearOff(QPainter *painter, const QRect &rect)
     q->style()->drawControl(QStyle::CE_MenuTearoff, &menuOpt, painter, q);
 }
 
+QRect QMenuPrivate::rect() const
+{
+    Q_Q(const QMenu);
+    QStyle *style = q->style();
+    QStyleOption opt(0);
+    opt.init(q);
+    const int hmargin = style->pixelMetric(QStyle::PM_MenuHMargin, &opt, q);
+    const int vmargin = style->pixelMetric(QStyle::PM_MenuVMargin, &opt, q);
+    const int fw = style->pixelMetric(QStyle::PM_MenuPanelWidth, &opt, q);
+    return (q->rect().adjusted(hmargin + fw + leftmargin, vmargin + fw + topmargin,
+                               -(hmargin + fw + rightmargin), -(vmargin + fw + bottommargin)));
+}
+
 QMenuPrivate::ScrollerTearOffItem::ScrollerTearOffItem(QMenuPrivate::ScrollerTearOffItem::Type type, QMenuPrivate *mPrivate, QWidget *parent, Qt::WindowFlags f)
     : QWidget(parent, f), menuPrivate(mPrivate), scrollType(type)
 {
@@ -989,7 +1001,9 @@ void QMenuPrivate::scrollMenu(QAction *action, QMenuScroller::ScrollLocation loc
     }
 
     if (!(newScrollFlags & QMenuScroller::ScrollDown) && (scroll->scrollFlags & QMenuScroller::ScrollDown)) {
-        newOffset = q->height() - (saccum - newOffset) - fw*2 - vmargin;    //last item at bottom
+        newOffset = q->height() - (saccum - newOffset) - fw*2 - vmargin - topmargin - bottommargin;    //last item at bottom
+        if (tearoff)
+            newOffset -= q->style()->pixelMetric(QStyle::PM_MenuTearoffHeight, 0, q);
     }
 
     if (!(newScrollFlags & QMenuScroller::ScrollUp) && (scroll->scrollFlags & QMenuScroller::ScrollUp)) {
@@ -1141,15 +1155,23 @@ bool QMenuPrivate::mouseEventTaken(QMouseEvent *e)
 {
     Q_Q(QMenu);
     QPoint pos = q->mapFromGlobal(e->globalPos());
+
+    QStyle *style = q->style();
+    QStyleOption opt(0);
+    opt.init(q);
+    const int hmargin = style->pixelMetric(QStyle::PM_MenuHMargin, &opt, q);
+    const int vmargin = style->pixelMetric(QStyle::PM_MenuVMargin, &opt, q);
+    const int fw = style->pixelMetric(QStyle::PM_MenuPanelWidth, &opt, q);
+
     if (scroll && !activeMenu) { //let the scroller "steal" the event
         bool isScroll = false;
         if (pos.x() >= 0 && pos.x() < q->width()) {
-            for(int dir = QMenuScroller::ScrollUp; dir <= QMenuScroller::ScrollDown; dir = dir << 1) {
+        for (int dir = QMenuScroller::ScrollUp; dir <= QMenuScroller::ScrollDown; dir = dir << 1) {
                 if (scroll->scrollFlags & dir) {
                     if (dir == QMenuScroller::ScrollUp)
-                        isScroll = (pos.y() <= scrollerHeight());
+                        isScroll = (pos.y() <= scrollerHeight() + fw + vmargin + topmargin);
                     else if (dir == QMenuScroller::ScrollDown)
-                        isScroll = (pos.y() >= q->height() - scrollerHeight());
+                        isScroll = (pos.y() >= q->height() - scrollerHeight() - fw - vmargin - bottommargin);
                     if (isScroll) {
                         scroll->scrollDirection = dir;
                         break;
@@ -1166,7 +1188,8 @@ bool QMenuPrivate::mouseEventTaken(QMouseEvent *e)
     }
 
     if (tearoff) { //let the tear off thingie "steal" the event..
-        QRect tearRect(0, 0, q->width(), q->style()->pixelMetric(QStyle::PM_MenuTearoffHeight, 0, q));
+        QRect tearRect(leftmargin + hmargin + fw, topmargin + vmargin + fw, q->width() - fw * 2 - hmargin * 2 -leftmargin - rightmargin,
+                       q->style()->pixelMetric(QStyle::PM_MenuTearoffHeight, &opt, q));
         if (scroll && scroll->scrollFlags & QMenuPrivate::QMenuScroller::ScrollUp)
             tearRect.translate(0, scrollerHeight());
         q->update(tearRect);
@@ -2615,21 +2638,28 @@ void QMenu::paintEvent(QPaintEvent *e)
 
     //calculate the scroll up / down rect
     const int fw = style()->pixelMetric(QStyle::PM_MenuPanelWidth, 0, this);
+    const int hmargin = style()->pixelMetric(QStyle::PM_MenuHMargin,0, this);
+    const int vmargin = style()->pixelMetric(QStyle::PM_MenuVMargin, 0, this);
+
     QRect scrollUpRect, scrollDownRect;
+    const int leftmargin = fw + hmargin + d->leftmargin;
+    const int topmargin = fw + vmargin + d->topmargin;
+    const int bottommargin = fw + vmargin + d->bottommargin;
+    const int contentWidth = width() - (fw + hmargin) * 2 - d->leftmargin - d->rightmargin;
     if (d->scroll) {
         if (d->scroll->scrollFlags & QMenuPrivate::QMenuScroller::ScrollUp)
-            scrollUpRect.setRect(fw, fw, width() - (fw * 2), d->scrollerHeight());
+            scrollUpRect.setRect(leftmargin, topmargin, contentWidth, d->scrollerHeight());
 
         if (d->scroll->scrollFlags & QMenuPrivate::QMenuScroller::ScrollDown)
-            scrollDownRect.setRect(fw, height() - d->scrollerHeight() - fw, width() - (fw * 2),
-                                     d->scrollerHeight());
+            scrollDownRect.setRect(leftmargin, height() - d->scrollerHeight() - bottommargin,
+                                   contentWidth, d->scrollerHeight());
     }
 
     //calculate the tear off rect
     QRect tearOffRect;
     if (d->tearoff) {
-        tearOffRect.setRect(fw, fw, width() - (fw * 2),
-                             style()->pixelMetric(QStyle::PM_MenuTearoffHeight, 0, this));
+        tearOffRect.setRect(leftmargin, topmargin, contentWidth,
+                            style()->pixelMetric(QStyle::PM_MenuTearoffHeight, 0, this));
         if (d->scroll && d->scroll->scrollFlags & QMenuPrivate::QMenuScroller::ScrollUp)
             tearOffRect.translate(0, d->scrollerHeight());
     }
@@ -2646,6 +2676,12 @@ void QMenu::paintEvent(QPaintEvent *e)
         emptyArea -= QRegion(actionRect);
 
         QRect adjustedActionRect = actionRect;
+        if (!scrollUpTearOffRect.isEmpty() && adjustedActionRect.bottom() <= scrollUpTearOffRect.top())
+            continue;
+
+        if (!scrollDownRect.isEmpty() && adjustedActionRect.top() >=  scrollDownRect.bottom())
+            continue;
+
         if (adjustedActionRect.intersects(scrollUpTearOffRect)) {
             if (adjustedActionRect.bottom() <= scrollUpTearOffRect.bottom())
                 continue;
diff --git a/src/widgets/widgets/qmenu_p.h b/src/widgets/widgets/qmenu_p.h
index 898fa3161b..a160a2c7d4 100644
--- a/src/widgets/widgets/qmenu_p.h
+++ b/src/widgets/widgets/qmenu_p.h
@@ -463,6 +463,7 @@ public:
 
     void drawScroller(QPainter *painter, ScrollerTearOffItem::Type type, const QRect &rect);
     void drawTearOff(QPainter *painter, const QRect &rect);
+    QRect rect() const;
 };
 
 #endif // QT_NO_MENU