Bug 264521 - oxygen-gtk has too much space between menu items
Summary: oxygen-gtk has too much space between menu items
Status: CLOSED UPSTREAM
Alias: None
Product: Oxygen
Classification: Plasma
Component: gtk2-engine (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Hugo Pereira Da Costa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-27 19:53 UTC by Ivo Anjo
Modified: 2011-07-29 23:32 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Screenshot showing gajim (gtk) and konsole (qt) menu spacing (43.51 KB, image/png)
2011-01-27 19:53 UTC, Ivo Anjo
Details
gajim menu vs gwenview (60.29 KB, image/png)
2011-01-27 21:36 UTC, Hugo Pereira Da Costa
Details
aligned gajim vs gwenview comparison (55.00 KB, image/png)
2011-01-27 22:34 UTC, Ivo Anjo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ivo Anjo 2011-01-27 19:53:52 UTC
Created attachment 56527 [details]
Screenshot showing gajim (gtk) and konsole (qt) menu spacing

Version:           unspecified (using KDE 4.5.5) 
OS:                Linux

Oxygen-gtk seems to have a very big space between menu items, when drawing menus. I've tested with firefox, gajim and tomboy and all show this issue.

Attached screenshot shows gajim vs konsole.

P.s.: Thanks for all your hard work on oxygen-gtk :)

Reproducible: Always
Comment 1 Hugo Pereira Da Costa 2011-01-27 21:36:53 UTC
Created attachment 56534 [details]
gajim menu vs gwenview

Can't reproduce:

Thats what I have here. 
Gajim on the left,
Gwenview menu on the right.
Spacing is just the same.
Comment 2 Hugo Pereira Da Costa 2011-01-27 21:38:26 UTC
In fact, your konsole looks quite narrower than my gwenview (and konsole)
Which kde version do you have ?
Comment 3 Ivo Anjo 2011-01-27 22:34:04 UTC
(In reply to comment #2)
> In fact, your konsole looks quite narrower than my gwenview (and konsole)
> Which kde version do you have ?

The screenshot was taken on my 4.5.5 machine, but I get pretty much the same thing on my 4.6 machine (altough it's less noticeable, maybe because of DPI setting?).

I was just looking really closely at your screenshot, and I think it suffers the same problem. I've re-attached a modified version of it that I think shows the problem.
Comment 4 Ivo Anjo 2011-01-27 22:34:39 UTC
Created attachment 56542 [details]
aligned gajim vs gwenview comparison
Comment 5 Hugo Pereira Da Costa 2011-01-27 22:50:17 UTC
Well so on this second screenshot, spacing between items in gajim (left) is _smaller_ than between items in gwenview (right), which is Qt.
That's pretty much the opposite of your statement, no ?
Or do I completely miss something ? 

What's your font size ?
Comment 6 Ivo Anjo 2011-01-27 23:03:40 UTC
(In reply to comment #5)
> Well so on this second screenshot, spacing between items in gajim (left) is
> _smaller_ than between items in gwenview (right), which is Qt.
> That's pretty much the opposite of your statement, no ?
> Or do I completely miss something ? 
> 
> What's your font size ?

You are right, but it does show that they differ, so the balance might "shift" with different font settings.
For menus, I use Sans Serif 10, with antialiasing, no sub-pixel and no hinting (I think antialiasing settings don't mess with vertical height, but just in case), and XServer dpi is 104x104.
Comment 7 nucleo 2011-01-28 16:06:32 UTC
Large space between wireshark menu items with menu font size 8
http://img96.imageshack.us/img96/3053/wiresharkoxygen1.png

and with font size 10:
http://img825.imageshack.us/img825/3161/wiresharkoxygen.png

How it looks with QtCurve:
http://img593.imageshack.us/img593/9381/wiresharkqtcurve.png
Comment 8 Hugo Pereira Da Costa 2011-01-28 16:13:21 UTC
Shit. My last comment posted on the bug got lost.
Ok. Rewritting.

So the issue is that the spacing between items in gtk changes when you change the font size, whereas it does not in kde, as long as the font is smaller than the icon.

So there is no way to make them match for all font sizes.
I think we adjusted them to match for font-size 9. 
This explain that font-size 8 gives smaller spacing in gtk than KDE
and font size 10 gives larger spacing in gtk than KDE.

I'll keep the bug open to look for a workaround, but don't have too much hope. 
This is a gtk bug (or feature) which is intrinsically inconsistent with Qt.

On the wireshark screenshots on top of that, there is another *wireshark* bug, in the sense that they use generic GtkMenuItem to render separators, and not GtkSeparatorMenuItem, which we use to make them narrower.
Comment 9 nucleo 2011-01-30 16:54:44 UTC
There is also problem with large separators in pidgin:
http://img156.imageshack.us/img156/3367/pidgin1.png

With clerlooks no problem:
http://img638.imageshack.us/img638/6148/pidgin2.png
Comment 10 Hugo Pereira Da Costa 2011-01-30 16:58:00 UTC
Large separators are application bugs. Not oxygen.
No need to report them here.
Please report downstream (to the app). They should be GtkSeparatorMenuItem and not GtkMenuItem objects.
Comment 11 Ivo Anjo 2011-01-30 17:55:06 UTC
Would it be possible to make space between menu items a bit more tight, even if it doesn't match oxygen/qt at all at any font size?

At least for me, I think what bothers me the most is that they are too spaced apart, not that they pixel-match. For example in your gwenview/gajim picture, they weren't pixel-matched but it was hardly noticeable.
Comment 12 Hugo Pereira Da Costa 2011-01-30 18:00:31 UTC
As I tried to explain, with the current code, the space changes with the font size, so what you call large space is due to using "large" font. Making it smaller for your font size will make it too small for smaller font size. So unless the issue above gets fixed, (by us, in due time, or by gtk), we won't change anything. (as you noticed: the spacing is already too small for a font 8 computer. Which is what I use).
Comment 13 Hugo Pereira Da Costa 2011-01-30 18:03:18 UTC
PS: making the spacing font dependent, to compensate from the gtk bug, is actually doable (in oxygenqtsettings), and should fix the issue. It has not been done yet only for a question of limited manpower.
Comment 14 Ivo Anjo 2011-01-30 18:22:44 UTC
Yes, you are right, but my suggestion was more in the lines of: could the spacing just be fixed to a reasonable size?

I'm guessing that due to the limitations you pointed out, most other GTK styles probably do something like this, because they don't have a lot of spacing, even at large font sizes, and my reasoning was that this would feel much more "natural" than having a big space between items.

I'm also thinking here about things like netbooks: I have a 8.9" 1024x600 screen, and I have to use bigger fonts otherwise it's very straining on the eye, but with that configuration, menus would be very big vertically using oxygen-gtk, which really sucks on a netbook with limited vertical height. Of course you could just not-use oxygen-gtk in such a case, but I like it so much I want to use it everywhere :)
Comment 15 Hugo Pereira Da Costa 2011-01-30 18:30:32 UTC
Lets try be clear again.
I will not make changes that introduces differences between Qt and Gtk versions of oxygen for default settings. Especially not to accomodate an upstream bug. Period.
Comment 16 Hugo Pereira Da Costa 2011-01-30 19:43:57 UTC
Git commit 4ced733a5368f714ee50c5c0f5dbd1037a01e1b4 by Hugo Pereira Da Costa.
Pushed by hpereiradacosta into branch '1.0'.

Moved MenuItem spacing from gtkrc to oxygenqtsettings
Adjust menuitem spacing based on font size.
CCBUG: 264521

M  +2    -14   rc/gtkrc     
M  +15   -0    src/oxygenqtsettings.cpp     

http://commits.kde.org/d1a86b4e/4ced733a5368f714ee50c5c0f5dbd1037a01e1b4
Comment 17 Hugo Pereira Da Costa 2011-01-30 19:46:14 UTC
That fixes it.
Note that now gtk menu items are 1pixels too close with respect to qt.
This is due to an even/odd issue between the two toolkits.
In Qt you can set an odd spacing between items. In Gtk you cannot.

This was already the case before the bug fix.
Comment 18 Hugo Pereira Da Costa 2011-01-30 22:15:55 UTC
Git commit c5d5ee7bec7c54c5cefab32cb6d91bfa1d3650ab by Hugo Pereira Da Costa.
Pushed by hpereiradacosta into branch '1.0'.

Revert "Moved MenuItem spacing from gtkrc to oxygenqtsettings"

This reverts commit 4ced733a5368f714ee50c5c0f5dbd1037a01e1b4.
CCBUG: 264521

M  +14   -2    rc/gtkrc     
M  +0    -15   src/oxygenqtsettings.cpp     

http://commits.kde.org/d1a86b4e/c5d5ee7bec7c54c5cefab32cb6d91bfa1d3650ab
Comment 19 Hugo Pereira Da Costa 2011-01-30 22:16:03 UTC
Git commit 75ac485c7ff97223cdb1537369348e5734da3111 by Hugo Pereira Da Costa.
Pushed by hpereiradacosta into branch 'master'.

Revert "Moved MenuItem spacing from gtkrc to oxygenqtsettings"

This reverts commit 4ced733a5368f714ee50c5c0f5dbd1037a01e1b4.

Conflicts:

	src/oxygenqtsettings.cpp

CCBUG: 264521

M  +14   -2    rc/gtkrc     
M  +0    -15   src/oxygenqtsettings.cpp     

http://commits.kde.org/d1a86b4e/75ac485c7ff97223cdb1537369348e5734da3111
Comment 20 Hugo Pereira Da Costa 2011-01-30 22:19:06 UTC
ok. After some discussion with co-developper Ruslan, we decided

- to revert the commit that fixed the issue, cause too buggy
- to mark the bug as resolved UPSTREAM. This is really a gtk bug:
"as long as the font size is smaller than the icons, setting a larger font size should not make menus larger"

So we won't fix it.

As a fallback solution, it is fairly easy for users to actually change the spacing between items.

1/ edit oxygen's gtkrc file (usually in /usr/share/themes/oxygen-gtk/gtk-2.0/gtkrc)

2/ look for the following lines:
  style "oxygen-menuitem" = "oxygen-default"
  {
      xthickness = 1
      ythickness = 5
  }

3/ change the ythickness to your liking.
Comment 21 Hugo Pereira Da Costa 2011-01-30 22:49:45 UTC
Git commit 0113e08c0a4bff47a9f7ca2cb173f2320605b220 by Hugo Pereira Da Costa.
Pushed by hpereiradacosta into branch 'master'.

Revert "Moved MenuItem spacing from gtkrc to oxygenqtsettings"

This reverts commit 4ced733a5368f714ee50c5c0f5dbd1037a01e1b4.
CCBUG: 264521

M  +14   -2    rc/gtkrc     
M  +0    -15   src/oxygenqtsettings.cpp     

http://commits.kde.org/d1a86b4e/0113e08c0a4bff47a9f7ca2cb173f2320605b220
Comment 22 nucleo 2011-01-31 00:33:42 UTC
(In reply to comment #20)
> ok. After some discussion with co-developper Ruslan, we decided
> 
> - to revert the commit that fixed the issue, cause too buggy
> - to mark the bug as resolved UPSTREAM. This is really a gtk bug:
> "as long as the font size is smaller than the icons, setting a larger font size
> should not make menus larger"
> 
> So we won't fix it.
> 
> As a fallback solution, it is fairly easy for users to actually change the
> spacing between items.
> 
> 1/ edit oxygen's gtkrc file (usually in
> /usr/share/themes/oxygen-gtk/gtk-2.0/gtkrc)
> 
> 2/ look for the following lines:
>   style "oxygen-menuitem" = "oxygen-default"
>   {
>       xthickness = 1
>       ythickness = 5
>   }
> 
> 3/ change the ythickness to your liking.

When I tested fix for this bug I never see this in gimp:
http://simplest-image-hosting.net/png-0-huge-menuitems

Gimp menus was fine in GNOME, LXDE, XFCE4, KDE.
Comment 23 Ruslan Kabatsayev 2011-01-31 00:43:07 UTC
> Gimp menus was fine in GNOME, LXDE, XFCE4, KDE.
But not when kde4-config is not installed (i.e. when no KDE on the system).
Comment 24 nucleo 2011-01-31 01:41:22 UTC
(In reply to comment #23)
> > Gimp menus was fine in GNOME, LXDE, XFCE4, KDE.
> But not when kde4-config is not installed (i.e. when no KDE on the system).

Yes, if kde4-config is not installed XFCE menu also too large
http://img546.imageshack.us/img546/7492/xfce.png
Comment 25 Hugo Pereira Da Costa 2011-01-31 06:28:17 UTC
So please file bug report upstream.
Comment 26 Ivo Anjo 2011-01-31 09:17:57 UTC
Thanks for explaining how to manually work-around this issue.

Just a question: has a bug report for this been created upstream? It would seem that with all the cleanups due on gtk 3, it might be the right time to fix this.