Fixes: MenuItem size fixes and missing separator with Gtk+

Task:     QTBUG-6522
RevBy:    thorbjorn
Details:

This fixes missing separator line in the Dust theme.

The patch also significantly improves the accuracy of menu item
size metrics by making use of gtk_widget_size_request to query
things like default heights.

- More accurate offset and size of separators
- Fixed item height of menu items using size_request
- Fixed vertical offset of label and check boxes
- Fixed clipping issue on last menu item

3 files changed, 33 insertions, 28 deletions

diff --git a/src/gui/styles/qgtkstyle.cpp b/src/gui/styles/qgtkstyle.cpp
index 41efaa3..920a0f7 100644
--- a/src/gui/styles/qgtkstyle.cpp
+++ b/src/gui/styles/qgtkstyle.cpp
@@ -2495,7 +2495,6 @@ void QGtkStyle::drawControl(ControlElement element,
             const int windowsItemHMargin      =  3; // menu item hor text margin
             const int windowsItemVMargin      = 26; // menu item ver text margin
             const int windowsRightBorder      = 15; // right border on windows
-            GtkWidget *gtkMenu = d->gtkWidget(QLS("GtkMenu"));
             GtkWidget *gtkMenuItem = menuItem->checked ? d->gtkWidget(QLS("GtkMenu.GtkCheckMenuItem")) :
                                      d->gtkWidget(QLS("GtkMenu.GtkMenuItem"));
 
@@ -2509,6 +2508,7 @@ void QGtkStyle::drawControl(ControlElement element,
                 gboolean wide_separators = 0;
                 gint     separator_height = 0;
                 guint    horizontal_padding = 3;
+                QRect separatorRect = option->rect;
                 if (!d->gtk_check_version(2, 10, 0)) {
                     d->gtk_widget_style_get(gtkMenuSeparator,
                                            "wide-separators",    &wide_separators,
@@ -2516,13 +2516,16 @@ void QGtkStyle::drawControl(ControlElement element,
                                            "horizontal-padding", &horizontal_padding,
                                            NULL);
                 }
+                separatorRect.setHeight(option->rect.height() - 2 * gtkMenuSeparator->style->ythickness);
+                separatorRect.setWidth(option->rect.width() - 2 * (horizontal_padding + gtkMenuSeparator->style->xthickness));
+                separatorRect.moveCenter(option->rect.center());
                 if (wide_separators)
-                    gtkPainter.paintBox( gtkMenuSeparator, "hseparator",
-                                         option->rect.adjusted(0, 0, 0, -1), GTK_STATE_NORMAL, GTK_SHADOW_NONE, gtkMenu->style);
+                   gtkPainter.paintBox( gtkMenuSeparator, "hseparator",
+                                        separatorRect, GTK_STATE_NORMAL, GTK_SHADOW_NONE, gtkMenuSeparator->style);
                 else
                     gtkPainter.paintHline( gtkMenuSeparator, "hseparator",
-                                           menuItem->rect, GTK_STATE_NORMAL, gtkMenu->style,
-                                           option->rect.left() + horizontal_padding, option->rect.width() - 2*horizontal_padding, 2);
+                                           separatorRect, GTK_STATE_NORMAL, gtkMenuSeparator->style,
+                                           0, option->rect.right() - 1, 1);
                 painter->restore();
                 break;
             }
@@ -2530,7 +2533,7 @@ void QGtkStyle::drawControl(ControlElement element,
             bool selected = menuItem->state & State_Selected && menuItem->state & State_Enabled;
 
             if (selected) {
-                QRect rect = option->rect.adjusted(0, 0, 0, -1);
+                QRect rect = option->rect;
 #ifndef QT_NO_COMBOBOX
                 if (qobject_cast<const QComboBox*>(widget))
                     rect = option->rect;
@@ -2556,7 +2559,7 @@ void QGtkStyle::drawControl(ControlElement element,
 #endif
             if (!ignoreCheckMark) {
                 // Check
-                QRect checkRect(option->rect.left() + 7, option->rect.center().y() - checkSize/2, checkSize, checkSize);
+                QRect checkRect(option->rect.left() + 7, option->rect.center().y() - checkSize/2 + 1, checkSize, checkSize);
                 checkRect = visualRect(menuItem->direction, menuItem->rect, checkRect);
 
                 if (checkable && menuItem->icon.isNull()) {
@@ -2680,7 +2683,7 @@ void QGtkStyle::drawControl(ControlElement element,
             int tab = menuitem->tabWidth;
             int xm = windowsItemFrame + checkcol + windowsItemHMargin;
             int xpos = menuitem->rect.x() + xm + 1;
-            QRect textRect(xpos, y + windowsItemVMargin - 1, w - xm - windowsRightBorder - tab + 1, h - 2 * windowsItemVMargin);
+            QRect textRect(xpos, y + windowsItemVMargin, w - xm - windowsRightBorder - tab + 1, h - 2 * windowsItemVMargin);
             QRect vTextRect = visualRect(opt->direction, menuitem->rect, textRect);
             QString s = menuitem->text;
 
@@ -3150,44 +3153,38 @@ QSize QGtkStyle::sizeFromContents(ContentsType type, const QStyleOption *option,
                 newSize += QSize(6, 0);
         }
         break;
-
     case CT_MenuItem:
         if (const QStyleOptionMenuItem *menuItem = qstyleoption_cast<const QStyleOptionMenuItem *>(option)) {
             int textMargin = 8;
 
             if (menuItem->menuItemType == QStyleOptionMenuItem::Separator) {
                 GtkWidget *gtkMenuSeparator = d->gtkWidget(QLS("GtkMenu.GtkSeparatorMenuItem"));
-                gboolean wide_separators;
-                gint     separator_height;
-                d->gtk_widget_style_get(gtkMenuSeparator,
-                                       "wide-separators",    &wide_separators,
-                                       "separator-height",   &separator_height,
-                                       NULL);
-                newSize = QSize(size.width(), wide_separators ? separator_height - 1 : 7 );
-
+                GtkRequisition sizeReq = {0, 0};
+                d->gtk_widget_size_request(gtkMenuSeparator, &sizeReq);
+                newSize = QSize(size.width(), sizeReq.height);
                 break;
             }
 
-            GtkWidget *gtkMenuItem = d->gtkWidget(QLS("GtkMenu.GtkMenuItem"));
+            GtkWidget *gtkMenuItem = d->gtkWidget(QLS("GtkMenu.GtkCheckMenuItem"));
             GtkStyle* style = gtkMenuItem->style;
-            newSize += QSize(textMargin + style->xthickness - 1, style->ythickness - 3);
+
+            // Note we get the perfect height for the default font since we
+            // set a fake text label on the gtkMenuItem
+            // But if custom fonts are used on the widget we need a minimum size
+            GtkRequisition sizeReq = {0, 0};
+            d->gtk_widget_size_request(gtkMenuItem, &sizeReq);
+            newSize.setHeight(qMax(newSize.height() - 4, sizeReq.height));
+            newSize += QSize(textMargin + style->xthickness - 1, 0);
 
             // Cleanlooks assumes a check column of 20 pixels so we need to
             // expand it a bit
             gint checkSize;
-            d->gtk_widget_style_get(d->gtkWidget(QLS("GtkMenu.GtkCheckMenuItem")), "indicator-size", &checkSize, NULL);
-            newSize.setHeight(qMax(newSize.height(), checkSize + 2));
+            d->gtk_widget_style_get(gtkMenuItem, "indicator-size", &checkSize, NULL);
             newSize.setWidth(newSize.width() + qMax(0, checkSize - 20));
         }
 
         break;
 
-    case CT_Menu:
-        // This is evil, but QMenu adds 1 pixel too much
-        newSize -= QSize(0, 1);
-
-        break;
-
     case CT_SpinBox:
         // QSpinBox does some nasty things that depends on CT_LineEdit
         newSize = size + QSize(0, -d->gtkWidget(QLS("GtkSpinButton"))->style->ythickness * 2);
diff --git a/src/gui/styles/qgtkstyle_p.cpp b/src/gui/styles/qgtkstyle_p.cpp
index 5060be2..a033407 100644
--- a/src/gui/styles/qgtkstyle_p.cpp
+++ b/src/gui/styles/qgtkstyle_p.cpp
@@ -151,6 +151,7 @@ Ptr_gtk_menu_item_set_submenu QGtkStylePrivate::gtk_menu_item_set_submenu = 0;
 Ptr_gtk_settings_get_default QGtkStylePrivate::gtk_settings_get_default = 0;
 Ptr_gtk_separator_menu_item_new QGtkStylePrivate::gtk_separator_menu_item_new = 0;
 Ptr_gtk_widget_size_allocate QGtkStylePrivate::gtk_widget_size_allocate = 0;
+Ptr_gtk_widget_size_request QGtkStylePrivate::gtk_widget_size_request = 0;
 Ptr_gtk_widget_set_direction QGtkStylePrivate::gtk_widget_set_direction = 0;
 Ptr_gtk_widget_path QGtkStylePrivate::gtk_widget_path = 0;
 Ptr_gtk_container_get_type QGtkStylePrivate::gtk_container_get_type = 0;
@@ -159,7 +160,6 @@ Ptr_gtk_widget_get_type QGtkStylePrivate::gtk_widget_get_type = 0;
 Ptr_gtk_rc_get_style_by_paths QGtkStylePrivate::gtk_rc_get_style_by_paths = 0;
 Ptr_gtk_check_version QGtkStylePrivate::gtk_check_version = 0;
 Ptr_gtk_border_free QGtkStylePrivate::gtk_border_free = 0;
-
 Ptr_pango_font_description_get_size QGtkStylePrivate::pango_font_description_get_size = 0;
 Ptr_pango_font_description_get_weight QGtkStylePrivate::pango_font_description_get_weight = 0;
 Ptr_pango_font_description_get_family QGtkStylePrivate::pango_font_description_get_family = 0;
@@ -410,11 +410,13 @@ void QGtkStylePrivate::resolveGtk() const
     gtk_combo_box_entry_new = (Ptr_gtk_combo_box_entry_new)libgtk.resolve("gtk_combo_box_entry_new");
     gtk_container_forall = (Ptr_gtk_container_forall)libgtk.resolve("gtk_container_forall");
     gtk_widget_size_allocate =(Ptr_gtk_widget_size_allocate)libgtk.resolve("gtk_widget_size_allocate");
+    gtk_widget_size_request =(Ptr_gtk_widget_size_request)libgtk.resolve("gtk_widget_size_request");
     gtk_widget_set_direction =(Ptr_gtk_widget_set_direction)libgtk.resolve("gtk_widget_set_direction");
     gtk_widget_path =(Ptr_gtk_widget_path)libgtk.resolve("gtk_widget_path");
     gtk_container_get_type =(Ptr_gtk_container_get_type)libgtk.resolve("gtk_container_get_type");
     gtk_window_get_type =(Ptr_gtk_window_get_type)libgtk.resolve("gtk_window_get_type");
     gtk_widget_get_type =(Ptr_gtk_widget_get_type)libgtk.resolve("gtk_widget_get_type");
+
     gtk_rc_get_style_by_paths =(Ptr_gtk_rc_get_style_by_paths)libgtk.resolve("gtk_rc_get_style_by_paths");
     gtk_check_version =(Ptr_gtk_check_version)libgtk.resolve("gtk_check_version");
     gtk_border_free =(Ptr_gtk_border_free)libgtk.resolve("gtk_border_free");
@@ -447,10 +449,13 @@ void QGtkStylePrivate::initGtkMenu() const
     gtk_widget_realize(gtkMenu);
 
     GtkWidget *gtkMenuItem = QGtkStylePrivate::gtk_menu_item_new();
+    g_object_set(gtkMenuItem, "label", "X", NULL);
+
     gtk_menu_shell_append((GtkMenuShell*)gtkMenu, gtkMenuItem);
     gtk_widget_realize(gtkMenuItem);
 
     GtkWidget *gtkCheckMenuItem = QGtkStylePrivate::gtk_check_menu_item_new();
+    g_object_set(gtkCheckMenuItem, "label", "X", NULL);
     gtk_menu_shell_append((GtkMenuShell*)gtkMenu, gtkCheckMenuItem);
     gtk_widget_realize(gtkCheckMenuItem);
 
diff --git a/src/gui/styles/qgtkstyle_p.h b/src/gui/styles/qgtkstyle_p.h
index ad672db..db5b9b9 100644
--- a/src/gui/styles/qgtkstyle_p.h
+++ b/src/gui/styles/qgtkstyle_p.h
@@ -147,8 +147,10 @@ typedef void   (*Ptr_gtk_paint_vline) (GtkStyle *, GdkWindow *, GtkStateType, co
 typedef void (*Ptr_gtk_menu_item_set_submenu) (GtkMenuItem *, GtkWidget *);
 typedef void (*Ptr_gtk_container_forall) (GtkContainer *, GtkCallback, gpointer);
 typedef void (*Ptr_gtk_widget_size_allocate) (GtkWidget *, GtkAllocation*);
+typedef void (*Ptr_gtk_widget_size_request) (GtkWidget *widget, GtkRequisition *requisition);
 typedef void (*Ptr_gtk_widget_set_direction) (GtkWidget *, GtkTextDirection);
 typedef void (*Ptr_gtk_widget_path) (GtkWidget *, guint *, gchar **, gchar**);
+
 typedef void (*Ptr_gtk_toolbar_insert) (GtkToolbar *toolbar, GtkToolItem *item, int pos);
 typedef void (*Ptr_gtk_menu_shell_append)(GtkMenuShell *, GtkWidget *);
 typedef GtkType (*Ptr_gtk_container_get_type) (void);
@@ -365,6 +367,7 @@ public:
     static Ptr_gtk_settings_get_default gtk_settings_get_default;
     static Ptr_gtk_separator_menu_item_new gtk_separator_menu_item_new;
     static Ptr_gtk_widget_size_allocate gtk_widget_size_allocate;
+    static Ptr_gtk_widget_size_request gtk_widget_size_request;
     static Ptr_gtk_widget_set_direction gtk_widget_set_direction;
     static Ptr_gtk_widget_path gtk_widget_path;
     static Ptr_gtk_container_get_type gtk_container_get_type;