GNOME Bugzilla – Bug 623223
Handle video out keys in media-keys
Last modified: 2014-07-15 17:45:52 UTC
See patch
Created attachment 164978 [details] [review] Handle video out keys in media-keys Not finished, but gives you an idea. 1) add dbus method to do the "change output" action 2) remove Fn+F7 code from the xrandr backend 3) add hard-coded windows+p/fn+f7 handler in media-keys 4) make those hard-coded keys call the dbus method
The patch add 3), though I didn't check for the actual keys that should be used.
Created attachment 166026 [details] [review] Handle video out keys in media-keys Instead of reimplementing keyboard events capture in xrandr, add D-Bus methods to rotate the screen, and switch video modes to the xrandr plugin, and make the media-keys plugin handle call those methods when the keys are pressed. TODO: 4) make those hard-coded keys call the dbus method
Created attachment 166165 [details] [review] Handle video out keys in media-keys Instead of reimplementing keyboard events capture in xrandr, add D-Bus methods to rotate the screen, and switch video modes to the xrandr plugin, and make the media-keys plugin handle call those methods when the keys are pressed.
Mildly tested on a GNOME 2.28 backport, seems to work as expected :)
I don't think this is going to generally work: + { VIDEO_OUT_KEY, NULL, "<Mod4>p", NULL }, Should be <Super>p right? Presumably, you had <Super>p and that didn't work and then <Mod4>p did work. In that case, maybe g-s-d needs a patch in the same vein as the panel got recently in bug 624572 ?
<Mod4>p is what the keyboard shortcuts app detects, I didn't do any additional work in trying to use Super instead.
The problem is <Super> might be Mod2 or Mod5 on a different machine. Virtual modifiers like super, hyper, meta, etc are arbitrarily assigned to the real modifiers (mod2 to mod5).
Created attachment 166264 [details] [review] Use virtual modifier <Super> for the Windows key By making sure that we convert virtual modifiers to real ones when capturing keys with XGrabKey, and vice-versa when checking for XEvents.
Right. The egg code to parse the shortcuts works fine (eg. <Super> is parsed properly). The problem is that we need to resolve GDK_SUPER_MASK to its real value depending on the keymap for use with XGrabKey (which does not understand the Super modifier), and also when checking for key equality. I'd expect some slight problems if somebody changed the ModX value for the Super key a few times during the gnome-settings-daemon run, but nothing worse than what we have now...
There was some discussion about this video key (super+p) in Ubuntu: https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/539477 One issue we found was that an Enter press was sometimes sent to the system after a delay: https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/539477/comments/39 Are you seeing that on your hardware? Any thought of how to handle that?
(In reply to comment #11) > There was some discussion about this video key (super+p) in Ubuntu: > https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/539477 > > One issue we found was that an Enter press was sometimes sent to the system > after a delay: > https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/539477/comments/39 > > Are you seeing that on your hardware? Any thought of how to handle that? No, and get the manufacturer to fix the BIOS.
Ah, I wish I could make that so. Linux support is not usually top of BIOS makers' lists. As you can see in the Ubuntu bug, Jerone references an internal Dell spec that specifies the Enter behavior. Since we're not seeing it on non-Dell machines, it presumably is a Dell-specific spec. I understand your lack of enthusiasm for working around one manufacturer's behavior. But even if Dell changes it's apparently-desired Windows behavior [1] to accommodate Linux for future machines, there are shipped devices with the Enter issue. The best workaround right now for Dell machines is to tell the BIOS we are not Windows 7 (acpi_osi=\"!Windows 2009\"). Perhaps this should be moved to a different bug about the Enter press on Dell hardware, as it is not really relevant to broader support for the video out key press. [1] The theory is they do that to automatically dismiss the Windows dialog that pops up when you press the Video key if the user just wants to quickly switch displays.
(In reply to comment #13) > Ah, I wish I could make that so. Linux support is not usually top of BIOS > makers' lists. > > As you can see in the Ubuntu bug, Jerone references an internal Dell spec that > specifies the Enter behavior. Since we're not seeing it on non-Dell machines, > it presumably is a Dell-specific spec. > > I understand your lack of enthusiasm for working around one manufacturer's > behavior. But even if Dell changes it's apparently-desired Windows behavior > [1] to accommodate Linux for future machines, there are shipped devices with > the Enter issue. > > The best workaround right now for Dell machines is to tell the BIOS we are not > Windows 7 (acpi_osi=\"!Windows 2009\"). > > Perhaps this should be moved to a different bug about the Enter press on Dell > hardware, as it is not really relevant to broader support for the video out key > press. > > [1] The theory is they do that to automatically dismiss the Windows dialog that > pops up when you press the Video key if the user just wants to quickly switch > displays. Probably, but pressing Enter in the BIOS is just a completely stupid way to do this.
Review of attachment 166264 [details] [review]: ::: plugins/common/gsd-keygrab.c @@ +129,3 @@ + /* XGrabKey requires real modifiers, not virtual ones */ + egg_keymap_resolve_virtual_modifiers (gdk_keymap_get_default (), + cleaned_mask, &mask); This part isn't right. We need to resolve modifiers in the key->state not the ignored modifiers. Basically, we shouldn't ever be passing key->state to grab_key_real, but instead a bitlist of real modifiers
Created attachment 166454 [details] [review] Use virtual modifier <Super> for the Windows key By making sure that we convert virtual modifiers to real ones when capturing keys with XGrabKey, and vice-versa when checking for XEvents. This is based heavily on a patch from Bastien Nocera <hadess@hadess.net> with just minor fixes.
Review of attachment 166165 [details] [review]: ::: plugins/media-keys/gsd-media-keys-manager.c @@ +943,3 @@ + G_TYPE_INT64, timestamp, + G_TYPE_INVALID, + G_TYPE_INVALID); You can't do a blocking call to yourself. It will block until timeout adding a 25 second artificial delay.
Created attachment 166469 [details] [review] Handle video out keys in media-keys Instead of reimplementing keyboard events capture in xrandr, add D-Bus methods to rotate the screen, and switch video modes to the xrandr plugin, and make the media-keys plugin handle call those methods when the keys are pressed. This patch almost entirely written by: Bastien Nocera <hadess@hadess.net> with minor fixes.
So on a downstream report, one user has run into a problem with attachment 166454 [details] [review] They have an xmodmap file that removes Super from their map entirely. This means this call: + /* XGrabKey requires real modifiers, not virtual ones */ + egg_keymap_resolve_virtual_modifiers (gdk_keymap_get_default (), + key->state, &modifiers); + is unable to resolve Super to a real modifier, and so XGrabKey gets called with no modifier at all causing a broken p key.
Created attachment 166715 [details] [review] Use virtual modifier <Super> for the Windows key By making sure that we convert virtual modifiers to real ones when capturing keys with XGrabKey, and vice-versa when checking for XEvents. This is based heavily on a patch from Bastien Nocera <hadess@hadess.net> with just minor fixes.
*** Bug 613155 has been marked as a duplicate of this bug. ***
Very nice! I'm happy to let something else handle the arcana of grabbing keys. Feel free to commit this.
I was talking to Bastien about this yesterday, and we decided we should probably update the patches to gdbus before committing them. I think they're a little bitrotted anyway now, so will need some work regardless.
I added a few parts of the first patch to master, as it was needed to handle bug 633726.
Created attachment 173963 [details] [review] Handle video out keys in media-keys Instead of reimplementing keyboard events capture in xrandr, add D-Bus methods to rotate the screen, and switch video modes to the xrandr plugin, and make the media-keys plugin handle call those methods when the keys are pressed. This patch almost entirely written by: Bastien Nocera <hadess@hadess.net> with minor fixes.
Created attachment 173964 [details] [review] Use virtual modifier <Super> for the Windows key By making sure that we convert virtual modifiers to real ones when capturing keys with XGrabKey, and vice-versa when checking for XEvents. This is based heavily on a patch from Bastien Nocera <hadess@hadess.net> with just minor fixes.
Ported all those to the latest tree, and GDbus. Needs testing. I left the old XRandR code still using dbus-glib, as the additions were fairly minimal. Let's do that separately.
Attachment 173963 [details] pushed as d1a4f8c - Handle video out keys in media-keys Attachment 173964 [details] pushed as 469145f - Use virtual modifier <Super> for the Windows key
Sorry, but using hardcoded keys is just a sloppy work. I upgrade gsd and suddenly "Previous Window" Compiz action makes my screen go blank for a moment and there is no way to find out why. No GUI, no g/dconf knobs. I'll attach the patch I use to make VIDEO_OUT configurable via dconf-editor (against gsd-2.91.93). See also https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/694910.
Created attachment 184767 [details] [review] Make VIDEO_OUT media key configurable via dconf.
(In reply to comment #29) > I'll attach the > patch I use to make VIDEO_OUT configurable via dconf-editor (against > gsd-2.91.93). See also > https://bugs.launchpad.net/ubuntu/+source/gnome-settings-daemon/+bug/694910. FYI, I have requested the patch to be inclued by opening bug 651571, but it was rejected.
FWIW, I used to keep a patch around for debugging purposes, that did exactly that - make the XF86Display hotkey configurable. It was useful for testing the RANDR plugin on my desktop machine, which of course doesn't have the laptop's special keys to switch video modes.
(In reply to comment #32) > FWIW, I used to keep a patch around for debugging purposes, that did exactly > that - make the XF86Display hotkey configurable. It was useful for testing the > RANDR plugin on my desktop machine, which of course doesn't have the laptop's > special keys to switch video modes. But most likely has a Windows, so you could type "Windows+P" and have it do the same thing.
(In reply to comment #33) > But most likely has a Windows, so you could type "Windows+P" and have it do the > same thing. Heh, good point :)
Could you PLEASE apply the patch from comment 30! The current situation is unacceptable, your squatting the shortcut i use since ever to open programs. It's hardcoded in my brain! Also many WMs use <mod>-p per default for this task, eg. wmii, ion, i3, dwm, ... It is a common usecase to run a different WM on top of a gnome-session, so please make this configurable! Having to disable the xrandr-plugin is quite a strange workaround.
Current solution (hardcoding <Super>+p in addition to XF86Display) does not work in case of using some different keyboard layouts - e.g. dvorak. In case of using dvorak the scancodes sent with video out key press convert to <Super>+l and this is not handled by gnome-settings-daemon. I think that either the key combination recognized by settings-daemon should depend on the keyboard layout set or the key combination should be user adjustable.