nicklan / pnmixer

Volume mixer for the system tray
GNU General Public License v3.0
152 stars 32 forks source link

Weird vol_meter width/behavior #136

Closed hasufell closed 6 years ago

hasufell commented 8 years ago

it's currently set in https://github.com/nicklan/pnmixer/blob/master/src/ui-tray-icon.c#L250 to

    vol_meter_width = icon_width / 8;

I can barely see the volume meter bar in my tray icon. I see two solutions here:

  1. change it to icon_width / 6 or icon_width / 4
  2. make it configurable

Making it configurable seems a bit like overkill to me and increases maintenance burden for a really small corner case.

elboulangero commented 8 years ago

It was set to icon_width / 4 before the big rewrite. It looks nicer with icon_width / 8 for my icon set and icon size, so I changed it. I guess you have smaller icons than me.

But the primary goal is that it's usable, not that it looks nice. And making it configurable seems also overkill to me. So I agree that you change it to / 6 or / 8, whatever suits you best.

hasufell commented 8 years ago

Hmmm. I think the fix wasn't the only thing this is really about. It seems that the positioning (offset) of the bar is somewhat messed up and behavior changes significantly when using the system theme.

Something here is wrong.

elboulangero commented 8 years ago

Hey Hasufell, I just tried hard but can't reproduce the issue.

I tried with the system theme (though I don't think the icon theme matters). I tried with a tray icon size ranging from very small (16) to huge (56). I tried in a horizontal/vertical panel (tint2 version 0.12.12-1). I tried with different vol_meter_width dividers: / 8, / 6, / 4. I tried with an offset ranging from 0 to 100. I tried with both Gtk2 and Gtk3. It all works flawlessly here.

Could it be a Gtk version problem ? Here's some info about my system.

$ pkg-config --modversion gtk+-2.0 
2.24.30
$ pkg-config --modversion gtk+-3.0 
3.21.5

Cheers

hasufell commented 8 years ago

I doubt this is a gtk+ version issue:

# pkg-config --modversion gtk+-2.0
2.24.30
# pkg-config --modversion gtk+-3.0
3.20.9

offset 10: offset-10

offset 0: offset-0

window manager is i3. I've never seen a "cut icon", so I don't think this is an icon display problem either

Similar in xfce (with system icon): offset 10: xfce-offset-10

offset 0: xfce-offset-0

elboulangero commented 8 years ago

Just tried and reproduced the problem with XFCE4. The problem was that, for small icons, we allocated a pixbuf under the legal size required by Gtk3. I fixed the warning yesterday, without knowing that it fixed the problem, in 399f78b77de779fbb5156b41c5556df14ab92c2b. Please give it a try.

hasufell commented 8 years ago

Doesn't seem to fix it for me

hasufell commented 8 years ago

Also, this seems to not be a regression, it's similar in v0.6.1. Something must be off with icon width detection I guess.

Do you think this should block the 0.7 release?

elboulangero commented 8 years ago

Ok, here are more investigation on this problem. I ran a series of tests with xfce4-panel, using an offset of 0% for the volume meter. I changed the size of the icons displayed in the notification area of the panel (Panel), and looked for the result:

First test, with the PNMixer icons.

Panel   Pixbuf  Warn    VM

13      128     16x16   none
14      128     16x16   ok
15      128     16x16   ok
16      128             ok
17      128             ok
18      128             ok
19      128             ok
20      128             ok
21      128      24x24  ok
22      128      24x24  ok
23      128      24x24  ok
24      128             ok
25      128             ok
26      128             ok
27      128             ok
28      128             ok
29      128      32x32  ok
30      128      32x32  ok
31      128      32x32  ok
32      128             ok

Quick analysis:

Second test, with the system theme icons.

Panel   Pixbuf  Warn    VM

13      16      16x16   none
14      16      16x16   ok
15      16      16x16   ok
16      16              ok
17      22              ok
18      22              ok
19      22              ok
20      22              ok
21      22      24x24   split
22      22      24x24   split
23      24      24x24   ok
24      24              ok
25      32              ok
26      32              ok
27      32              ok
28      32              ok
29      32      32x32   ok
30      32      32x32   ok
31      32      32x32   ok
32      32              ok

Quick analysis:

My understanding would be that somehow, the 22x22 icons are screwed, or the resizing from 22x22 is screwed. But where is the resizing when we pick up a 22x22 icon to display it 22x22 ?

I tried to remove the 22x22 icons on my theme. In such case, the 24x24 icons are picked up instead, and it works like a charm.

As an additional test, I tried to replace the 22x22 icons on my theme by a gradient from black to white (attached). With such icon, you can see clearly that the display is screwed: one line from the left of the icon is displayed to the right. Which prove that the Volume Meter is drawn at the right position.

audio-volume-off

elboulangero commented 8 years ago

I can now confirm that I can reproduce the problem with tint2 as a panel. The volume meter is split only for 21x21 and 22x22 icon size, and only if using an icon theme that provide 22x22 icons.

If you want to give it a try, please ensure using the last commits, since I added debug messages to see which icons are picked up. For system icons, you can see the size of the icons in the path.

hasufell commented 8 years ago

I normally just use the pnmixer provided icons.

$ src/pnmixer -d
debug: ui-popup-menu.c: Building from ui file './data/ui/popup-menu-gtk3.glade'
debug: ui-popup-window.c: Building from ui file './data/ui/popup-window-vertical-gtk3.glade'
debug: ui-tray-icon.c: Creating tray icon
debug: ui-tray-icon.c: Building pixbuf array (requesting size 16)
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-muted.png' from './data/pixmaps/pnmixer-muted.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-off.png' from './data/pixmaps/pnmixer-off.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-low.png' from './data/pixmaps/pnmixer-low.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-medium.png' from './data/pixmaps/pnmixer-medium.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-high.png' from './data/pixmaps/pnmixer-high.png'
debug: hotkeys.c: Creating hotkeys control
debug: audio.c: Hooking soundcard '(default) (Master)' to the audio system
debug: alsa.c: 'default': Opening mixer
debug: alsa.c: 'default': Looking for playable mixer element 'Master'
debug: alsa.c: 'default': 1 poll descriptors are now watched
debug: alsa.c: 'default': Card '(default)' with channel 'Master' initialized !
debug: audio.c: Soundcard successfully hooked (scroll step: 5, normalize: true)
debug: audio.c: ** Dispatching signal 'card initialized' from 'unknown', vol=59.8101, muted=no
debug: ui-tray-icon.c: Allocating vol meter row (width 21)
debug: main.c: ---- Running main loop ----
debug: ui-tray-icon.c: Tray icon size is now 21
debug: ui-tray-icon.c: Building pixbuf array (requesting size 21)
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-muted.png' from './data/pixmaps/pnmixer-muted.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-off.png' from './data/pixmaps/pnmixer-off.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-low.png' from './data/pixmaps/pnmixer-low.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-medium.png' from './data/pixmaps/pnmixer-medium.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-high.png' from './data/pixmaps/pnmixer-high.png'
debug: ui-tray-icon.c: Allocating vol meter row (width 21)
debug: ui-prefs-dialog.c: Building from ui file './data/ui/prefs-dialog-gtk3.glade'
debug: ui-prefs-dialog.c: Populating prefs dialog values
debug: ui-prefs-dialog.c: Filling cards ComboBox
debug: alsa.c: 'default': Opening mixer
debug: alsa.c: 'default': Closing mixer
debug: alsa.c: 'hw:0': Opening mixer
debug: alsa.c: 'hw:0': Closing mixer
debug: alsa.c: 'hw:13': Opening mixer
debug: alsa.c: 'hw:13': Closing mixer
debug: ui-prefs-dialog.c: Filling channels ComboBox for card '(default)'
debug: alsa.c: 'default': Opening mixer
debug: alsa.c: 'default': Closing mixer
debug: ui-prefs-dialog.c: Retrieving prefs dialog values
debug: ui-prefs-dialog.c: Destroying
debug: ui-popup-window.c: Destroying
debug: ui-popup-window.c: Building from ui file './data/ui/popup-window-vertical-gtk3.glade'
debug: ui-tray-icon.c: Building pixbuf array (requesting size 21)
debug: ui-tray-icon.c: Loading stock icon 'audio-volume-muted' from '/usr/share/icons/Tango/22x22/status/audio-volume-muted.png'
warning: ui-tray-icon.c: Unable to lookup icon 'audio-volume-off'
debug: ui-tray-icon.c: Loading stock icon 'audio-volume-low' from '/usr/share/icons/Tango/22x22/status/audio-volume-low.png'
debug: ui-tray-icon.c: Loading stock icon 'audio-volume-medium' from '/usr/share/icons/Tango/22x22/status/audio-volume-medium.png'
debug: ui-tray-icon.c: Loading stock icon 'audio-volume-high' from '/usr/share/icons/Tango/22x22/status/audio-volume-high.png'
debug: ui-tray-icon.c: Loading stock icon 'audio-volume-low' from '/usr/share/icons/Tango/22x22/status/audio-volume-low.png'
debug: ui-tray-icon.c: Allocating vol meter row (width 3)
debug: audio.c: Unhooking soundcard from the audio system
debug: alsa.c: 'default': Closing mixer
debug: audio.c: ** Dispatching signal 'card cleaned up' from 'unknown', vol=0, muted=yes
debug: audio.c: Hooking soundcard '(default) (Master)' to the audio system
debug: alsa.c: 'default': Opening mixer
debug: alsa.c: 'default': Looking for playable mixer element 'Master'
debug: alsa.c: 'default': 1 poll descriptors are now watched
debug: alsa.c: 'default': Card '(default)' with channel 'Master' initialized !
debug: audio.c: Soundcard successfully hooked (scroll step: 5, normalize: true)
debug: audio.c: ** Dispatching signal 'card initialized' from 'unknown', vol=59.8101, muted=no
elboulangero commented 8 years ago

These are not the PNMixer icons, these are the icons provided by your icon theme. See these lines:

debug: ui-tray-icon.c: Loading stock icon 'audio-volume-muted' from '/usr/share/icons/Tango/22x22/status/audio-volume-muted.png'
warning: ui-tray-icon.c: Unable to lookup icon 'audio-volume-off'
debug: ui-tray-icon.c: Loading stock icon 'audio-volume-low' from '/usr/share/icons/Tango/22x22/status/audio-volume-low.png'
debug: ui-tray-icon.c: Loading stock icon 'audio-volume-medium' from '/usr/share/icons/Tango/22x22/status/audio-volume-medium.png'
debug: ui-tray-icon.c: Loading stock icon 'audio-volume-high' from '/usr/share/icons/Tango/22x22/status/audio-volume-high.png'
debug: ui-tray-icon.c: Loading stock icon 'audio-volume-low' from '/usr/share/icons/Tango/22x22/status/audio-volume-low.png'

And the way your panel is configured for the icon size, the 22x22 icons are picked up. My icon theme is Vibrancy, your icon theme is Tango. We both have the problem. I believe that if you slightly increase the icon size in your panel so that the 24x24 icons are picked up, that will solve the problem.

hasufell commented 8 years ago

Yes, they are pnmixer icons, see the first half of the debug output. I just switched the icons to have complete output for both pnmixer and system icons.

debug: ui-popup-menu.c: Building from ui file './data/ui/popup-menu-gtk3.glade'
debug: ui-popup-window.c: Building from ui file './data/ui/popup-window-vertical-gtk3.glade'
debug: ui-tray-icon.c: Creating tray icon
debug: ui-tray-icon.c: Building pixbuf array (requesting size 16)
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-muted.png' from './data/pixmaps/pnmixer-muted.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-off.png' from './data/pixmaps/pnmixer-off.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-low.png' from './data/pixmaps/pnmixer-low.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-medium.png' from './data/pixmaps/pnmixer-medium.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-high.png' from './data/pixmaps/pnmixer-high.png'
debug: hotkeys.c: Creating hotkeys control
debug: audio.c: Hooking soundcard '(default) (Master)' to the audio system
debug: alsa.c: 'default': Opening mixer
debug: alsa.c: 'default': Looking for playable mixer element 'Master'
debug: alsa.c: 'default': 1 poll descriptors are now watched
debug: alsa.c: 'default': Card '(default)' with channel 'Master' initialized !
debug: audio.c: Soundcard successfully hooked (scroll step: 5, normalize: true)
debug: audio.c: ** Dispatching signal 'card initialized' from 'unknown', vol=59.8101, muted=no
debug: ui-tray-icon.c: Allocating vol meter row (width 21)
debug: main.c: ---- Running main loop ----
debug: ui-tray-icon.c: Tray icon size is now 21
debug: ui-tray-icon.c: Building pixbuf array (requesting size 21)
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-muted.png' from './data/pixmaps/pnmixer-muted.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-off.png' from './data/pixmaps/pnmixer-off.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-low.png' from './data/pixmaps/pnmixer-low.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-medium.png' from './data/pixmaps/pnmixer-medium.png'
debug: ui-tray-icon.c: Loading PNMixer icon 'pnmixer-high.png' from './data/pixmaps/pnmixer-high.png'
debug: ui-tray-icon.c: Allocating vol meter row (width 21)
debug: ui-prefs-dialog.c: Building from ui file './data/ui/prefs-dialog-gtk3.glade'
elboulangero commented 8 years ago

Ah ok, didn't see that. And you have the bug with both icon set ? On your screenshot it seems that with PNMixer icons the volume meter is not split, only thiner, which is weird actually...

hasufell commented 8 years ago

And you have the bug with both icon set ? On your screenshot it seems that with PNMixer icons the volume meter is not split, only thiner, which is weird actually...

Yeah, the bug is similar, but not 100% the same.

pnmixer icons:

debug: ui-tray-icon.c: Allocating vol meter row (width 21)

system icons:

debug: ui-tray-icon.c: Allocating vol meter row (width 3)
elboulangero commented 8 years ago

That's ok. That's because we work on the pixbuf before it's resized to fit in the panel.

For PNMixer icons, the pixbuf is always 128x128. So the vol meter meter width is 128 / 6 = 21.333.

For system icons, GTk+ picks up icons which size is >= to the size the panel wants to display. If you panel wants to display 20x20 icons, and your icon set doesn't provide this size, then Gtk+ picks up the next size above, which is 22x22 in your case. Then we work on this pixbuf, and the vol meter width is 22 / 6 = 3.666

elboulangero commented 8 years ago

BTW, I just run a quick test with GTk2, only for the two icon sizes that create the problem on my setup, and I don't reproduce the problem. So I believe more and more that this is a Gtk3 bug, and that they screw something when resizing 22x22 icons...

elboulangero commented 8 years ago

I just pushed a bugfix for Gtk2 btw

hasufell commented 8 years ago

I just run a quick test with GTk2, only for the two icon sizes that create the problem on my setup, and I don't reproduce the problem.

I can confirm that

elboulangero commented 8 years ago

Just for the reference, I setup a little test to reproduce the bug, and therefore remove any suspicion from PNMixer. The bug seems to lie somewhere in Gtk+3, and maybe within this deprecated GtkStatusICon code. I won't dig further, I'll live with that.

Test available here:

git clone https://github.com/elboulangero/gtk-status-icon-size-test.git

Just executing the test is not enough to show the bug, you also need to play with your system tray icon size. I personnally can reproduce the bug only if my system tray icon size is 21x21 or 22x22, and the pixbuf used is 22x22.