[Qemu-devel] Qemu does not pass pressed caps lock to client

[Qemu-devel] Qemu does not pass pressed caps lock to client

[Shahar Havivi]

Qemu have a hack for capslock that is not working with Ubuntu.
attached patch that fix it, as describe in this bug:
https://bugs.launchpad.net/qemu/+bug/427612

Signed-off-by: Shahar Havivi <shaharh@gmail.com>

---
 sdl.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/sdl.c b/sdl.c
index cf27ad2..b3d5049 100644
--- a/sdl.c
+++ b/sdl.c
@@ -390,9 +390,10 @@ static void sdl_process_key(SDL_KeyboardEvent *ev)
         break;
     case 0x45: /* num lock */
     case 0x3a: /* caps lock */
-        /* SDL does not send the key up event, so we generate it */
-        kbd_put_keycode(keycode);
-        kbd_put_keycode(keycode | 0x80);
+        if (ev->type == SDL_KEYUP)
+            kbd_put_keycode(keycode | 0x80);
+        else
+            kbd_put_keycode(keycode);
         return;
     }

-- 
1.6.3.3

Re: [Qemu-devel] Qemu does not pass pressed caps lock to client

[Kevin Wolf]

Am 11.02.2010 22:13, schrieb Shahar Havivi:
> Qemu have a hack for capslock that is not working with Ubuntu.
> attached patch that fix it, as describe in this bug:
> https://bugs.launchpad.net/qemu/+bug/427612
> 
> Signed-off-by: Shahar Havivi <shaharh@gmail.com>
> 
> ---
>  sdl.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/sdl.c b/sdl.c
> index cf27ad2..b3d5049 100644
> --- a/sdl.c
> +++ b/sdl.c
> @@ -390,9 +390,10 @@ static void sdl_process_key(SDL_KeyboardEvent *ev)
>          break;
>      case 0x45: /* num lock */
>      case 0x3a: /* caps lock */
> -        /* SDL does not send the key up event, so we generate it */
> -        kbd_put_keycode(keycode);
> -        kbd_put_keycode(keycode | 0x80);
> +        if (ev->type == SDL_KEYUP)
> +            kbd_put_keycode(keycode | 0x80);
> +        else
> +            kbd_put_keycode(keycode);
>          return;
>      }
> 

The previous code explicitly says the SDL doesn't send the key up event.
If you think this is wrong generally, this definitely needs an
explanation in the commit message, Also it could use an explanation of
_why_ it doesn't work with Ubuntu - I assume they use either a newer or
a patched SDL version which does generate these events? As you did not
provide these explanations, I assume that this just happens to work for
your specific Ubuntu version and is wrong for some other systems.

What about always generating both keycodes as we currently do, but
ignoring the event if ev->type == SDL_KEYUP? This should work with any
SDL version.

Kevin

Re: [Qemu-devel] Qemu does not pass pressed caps lock to client

[Shahar Havivi]

It's not true that SDL is not sending up event like the comment say,

On Fedora 12 it behave like a toggle button, first press/release will send
caps-down event second press/release send caps-up event

On Ubuntu 9.10 it work like any other key, i.e. pressing caps will generate two
events down and up.

Shahar.

On Fri, Feb 12, 2010 at 11:54 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 11.02.2010 22:13, schrieb Shahar Havivi:
>> Qemu have a hack for capslock that is not working with Ubuntu.
>> attached patch that fix it, as describe in this bug:
>> https://bugs.launchpad.net/qemu/+bug/427612
>>
>> Signed-off-by: Shahar Havivi <shaharh@gmail.com>
>>
>> ---
>>  sdl.c |    7 ++++---
>>  1 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/sdl.c b/sdl.c
>> index cf27ad2..b3d5049 100644
>> --- a/sdl.c
>> +++ b/sdl.c
>> @@ -390,9 +390,10 @@ static void sdl_process_key(SDL_KeyboardEvent *ev)
>>          break;
>>      case 0x45: /* num lock */
>>      case 0x3a: /* caps lock */
>> -        /* SDL does not send the key up event, so we generate it */
>> -        kbd_put_keycode(keycode);
>> -        kbd_put_keycode(keycode | 0x80);
>> +        if (ev->type == SDL_KEYUP)
>> +            kbd_put_keycode(keycode | 0x80);
>> +        else
>> +            kbd_put_keycode(keycode);
>>          return;
>>      }
>>
>
> The previous code explicitly says the SDL doesn't send the key up event.
> If you think this is wrong generally, this definitely needs an
> explanation in the commit message, Also it could use an explanation of
> _why_ it doesn't work with Ubuntu - I assume they use either a newer or
> a patched SDL version which does generate these events? As you did not
> provide these explanations, I assume that this just happens to work for
> your specific Ubuntu version and is wrong for some other systems.
>
> What about always generating both keycodes as we currently do, but
> ignoring the event if ev->type == SDL_KEYUP? This should work with any
> SDL version.
>
> Kevin
>

Re: [Qemu-devel] Qemu does not pass pressed caps lock to client

[Kevin Wolf]

Am 12.02.2010 12:09, schrieb Shahar Havivi:
> It's not true that SDL is not sending up event like the comment say,
> 
> On Fedora 12 it behave like a toggle button, first press/release will send
> caps-down event second press/release send caps-up event
> 
> On Ubuntu 9.10 it work like any other key, i.e. pressing caps will generate two
> events down and up.

So I think we have this situation:

Without patch: F12 works, Ubuntu 9.10 broken
With your patch: F12 broken, Ubuntu 9.10 works

I dont' consider fixing one system by breaking another one a solution...
Do we know why this difference exists? Is there a way to check at
runtime which mode SDL uses?

Have you tested if num lock has the same behaviour or are you just assuming?

Kevin

[Qemu-devel] Re: Qemu does not pass pressed caps lock to client

[Paolo Bonzini]

On 02/12/2010 12:09 PM, Shahar Havivi wrote:
> It's not true that SDL is not sending up event like the comment say,
>
> On Fedora 12 it behave like a toggle button, first press/release will send
> caps-down event second press/release send caps-up event
 >
> On Ubuntu 9.10 it work like any other key, i.e. pressing caps will generate two
> events down and up.

True.  To see why, start at 
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=317010 and 
http://archive.ubuntu.com/ubuntu/pool/main/libs/libsdl1.2/libsdl1.2_1.2.13-4ubuntu4.diff.gz 
-- it's a nice code reading exercise, so I'll suggest a possible 
solution before pointing out the reason for this behavior.  The solution 
would be to put hack after hack, i.e. something like this (untested):

     case 0x3a: /* caps lock */
         /* SDL will usually send only 2 events instead of 4, so we
            generate the missing ones.  However, on Debian/Ubuntu
            systems it may generate 4; in this case we have to discard
            the extra events.  On Debian/Ubuntu ev->key.keysym.mod
            will always be zero, but for other systems we need the
            complicated condition below.  */
	if ((ev->key.keysym.mod & KMOD_CAPS) ==
             (ev->type == SDL_KEYDOWN ? KMOD_CAPS : 0)) {
             kbd_put_keycode(keycode);
             kbd_put_keycode(keycode | 0x80);
         }
         return;
     case 0x45: /* num lock */
         /* Same as above.  */
	if ((ev->key.keysym.mod & KMOD_NUM) ==
             (ev->type == SDL_KEYDOWN ? KMOD_NUM : 0)) {
             kbd_put_keycode(keycode);
             kbd_put_keycode(keycode | 0x80);
         }
         return;

Now, the solution of the riddle.  The patch was correctly submitted as

+	SDL_UseLockKeys = getenv ("SDL_DISABLE_LOCK_KEYS") == NULL;

...

+	use_lock_keys = SDL_UseLockKeys;

...

+				if (!use_lock_keys)
+					break;

(i.e. by default do not change anything) but the maintainer apparently 
morphed it into

+	SDL_UseLockKeys = getenv("SDL_DISABLE_LOCK_KEYS");

...

+ 	use_lock_keys = ( SDL_UseLockKeys && *SDL_UseLockKeys );

...

+ 				if ( ! use_lock_keys )
+ 					break;

which changed the default and the meaning of SDL_DISABLE_LOCK_KEYS.

I initially thought about removing the caps lock/num lock hack 
altogether and add the following, however it would need SDL 1.2.14 
because SDL_NO_LOCK_KEYS support was added exactly two months after 
1.2.13 was released. :-( :-(

     /* There are two versions around of a Debian patch that changes the
        way Caps Lock and Num Lock are handled.  The first version
        by default sends only one of the KeyDown/KeyUp events, unless
        SDL_DISABLE_LOCK_KEYS is present in the environment.  The second
        version instead by default sends both events, unless
        SDL_DISABLE_LOCK_KEYS is present and not empty.  This version
        is the most commonly found (and a totally braindead idea).

        Upstream instead supports SDL_NO_LOCK_KEYS which, if set to 1,
        will generate all four events---which is what we want.  Luckily,
        there is a combination of environment variable that will satisfy
        all variant.  */

     putenv ("SDL_DISABLE_LOCK_KEYS", "");
     putenv ("SDL_NO_LOCK_KEYS", "1");

Yes, I love Debian.

Paolo

Re: [Qemu-devel] Re: Qemu does not pass pressed caps lock to client

[Anthony Liguori]

On 02/12/2010 06:39 AM, Paolo Bonzini wrote:
>     /* There are two versions around of a Debian patch that changes the
>        way Caps Lock and Num Lock are handled.  The first version
>        by default sends only one of the KeyDown/KeyUp events, unless
>        SDL_DISABLE_LOCK_KEYS is present in the environment.  The second
>        version instead by default sends both events, unless
>        SDL_DISABLE_LOCK_KEYS is present and not empty.  This version
>        is the most commonly found (and a totally braindead idea).
>
>        Upstream instead supports SDL_NO_LOCK_KEYS which, if set to 1,
>        will generate all four events---which is what we want.  Luckily,
>        there is a combination of environment variable that will satisfy
>        all variant.  */
>
>     putenv ("SDL_DISABLE_LOCK_KEYS", "");
>     putenv ("SDL_NO_LOCK_KEYS", "1");
>
> Yes, I love Debian.

So basically, Debian carries a hacked version of SDL that changes the 
key press behaviour?

That's a Debian/Ubuntu bug.  Shame on them for changing the behaviour of 
a library API like that.

Regards,

Anthony Liguori

> Paolo
>
>

Re: [Qemu-devel] Re: Qemu does not pass pressed caps lock to client

[Paolo Bonzini]

On 02/12/2010 04:15 PM, Anthony Liguori wrote:
>
> So basically, Debian carries a hacked version of SDL that changes the
> key press behaviour?

Yes, the patch was submitted to not change the default but the 
maintainer thought he knew better.  Or confused an == with a != more likely.

> That's a Debian/Ubuntu bug.  Shame on them for changing the behaviour of
> a library API like that.

Indeed.  Maybe the Debian/Ubuntu maintainers for QEMU and KVM will read 
this thread and make a fuss.

The only reason it is interesting, is that QEMU is actually going 
through efforts to get the same behavior that the Debian/Ubuntu change 
implements (get a press and a release event whenever you toggle Caps Lock).

Paolo

Re: [Qemu-devel] Re: Qemu does not pass pressed caps lock to client

[Anthony Liguori]

On 02/12/2010 12:17 PM, Paolo Bonzini wrote:
> On 02/12/2010 04:15 PM, Anthony Liguori wrote:
>>
>> So basically, Debian carries a hacked version of SDL that changes the
>> key press behaviour?
>
> Yes, the patch was submitted to not change the default but the 
> maintainer thought he knew better.  Or confused an == with a != more 
> likely.
>
>> That's a Debian/Ubuntu bug.  Shame on them for changing the behaviour of
>> a library API like that.
>
> Indeed.  Maybe the Debian/Ubuntu maintainers for QEMU and KVM will 
> read this thread and make a fuss.

I've already updated the bug report appropriately.  Dustin, the ball's 
in your court :-)

> The only reason it is interesting, is that QEMU is actually going 
> through efforts to get the same behavior that the Debian/Ubuntu change 
> implements (get a press and a release event whenever you toggle Caps 
> Lock).

Yes, but the problem with

> Paolo

Re: [Qemu-devel] Re: Qemu does not pass pressed caps lock to client

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 1453 bytes --]

On Fri, 2010-02-12 at 12:44 -0600, Anthony Liguori wrote:
> On 02/12/2010 12:17 PM, Paolo Bonzini wrote:
> > On 02/12/2010 04:15 PM, Anthony Liguori wrote:
> >>
> >> So basically, Debian carries a hacked version of SDL that changes the
> >> key press behaviour?
> >
> > Yes, the patch was submitted to not change the default but the 
> > maintainer thought he knew better.  Or confused an == with a != more 
> > likely.
> >
> >> That's a Debian/Ubuntu bug.  Shame on them for changing the behaviour of
> >> a library API like that.
> >
> > Indeed.  Maybe the Debian/Ubuntu maintainers for QEMU and KVM will 
> > read this thread and make a fuss.
> 
> I've already updated the bug report appropriately.  Dustin, the ball's 
> in your court :-)

I looked at the original Debian Bug,
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=317010

The libsdl1.2 package in Ubuntu is no longer carrying that patch,
debian/patches/005_lock_keys.diff.  So I don't think that's quite the
cause of this.

As for reproducing the bug, eventually, I was able to get my host and
guest caps-lock keys out of sync, if I went back and forth, between
guest and host, toggling caps-lock.  What's the desired behavior here?
I don't have much of an opinion (other than that the caps-lock key is a
waste of valuable keyboard real estate, that I never actually use on
purpose, only ever hitting it accidentally and causing problems :-)

:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[Qemu-devel] Re: Qemu does not pass pressed caps lock to client

[Paolo Bonzini]

On 02/12/2010 09:49 PM, Dustin Kirkland wrote:
> I looked at the original Debian Bug,
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=317010
>
> The libsdl1.2 package in Ubuntu is no longer carrying that patch,
> debian/patches/005_lock_keys.diff.  So I don't think that's quite the
> cause of this.

The bug was reported in Karmic, which still has the patch with s/005/205/.

Paolo