[Qemu-devel] wrong behaviour of caps lock

[Qemu-devel] wrong behaviour of caps lock

[Benjamin Drung]

[-- Attachment #1.1: Type: text/plain, Size: 366 bytes --]

Hi,

We applied the attached patch in Ubuntu to fix the wrong behavior of
caps lock.

Initial bug report: https://launchpad.net/bugs/427612

Testcase: Select German NEO 2 as keyboard layout and press "caps lock" +
"l". Then a "-" should appear instead of a "t".

-- 
Benjamin Drung
Ubuntu Developer (www.ubuntu.com) | Debian Maintainer (www.debian.org)

[-- Attachment #1.2: SDL-wrong-behaviour-of-caps-lock.patch --]
[-- Type: text/x-patch, Size: 892 bytes --]

From bb212d2b23bee1abe52db53231caccc1a6a27791 Mon Sep 17 00:00:00 2001
From: Shahar Havivi <shaharh@redhat.com>
Date: Fri, 12 Feb 2010 00:00:44 +0200
Subject: [PATCH] Qemu does not pass pressed capslock to client

---
 sdl.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/sdl.c b/sdl.c
index cf27ad2..9074641 100644
--- a/sdl.c
+++ b/sdl.c
@@ -390,9 +390,11 @@ 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


[-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] wrong behaviour of caps lock

[Stefan Weil]

Benjamin Drung schrieb:
> Hi,
>
> We applied the attached patch in Ubuntu to fix the wrong behavior of
> caps lock.
>
> Initial bug report: https://launchpad.net/bugs/427612
>
> Testcase: Select German NEO 2 as keyboard layout and press "caps lock" +
> "l". Then a "-" should appear instead of a "t".

>From bb212d2b23bee1abe52db53231caccc1a6a27791 Mon Sep 17 00:00:00 2001
From: Shahar Havivi <shaharh@redhat.com>
Date: Fri, 12 Feb 2010 00:00:44 +0200
Subject: [PATCH] Qemu does not pass pressed capslock to client

---
sdl.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/sdl.c b/sdl.c
index cf27ad2..9074641 100644
--- a/sdl.c
+++ b/sdl.c
@@ -390,9 +390,11 @@ 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

Hi,

I can confirm that caps lock shows wrong behavior in
the vga console. It works in monitor and serial console.

The patch improves the wrong behavior for vga
without changing the behavior for monitor and serial
console.

Nevertheless, some problems remain:

* The patch does not apply cleanly to current git master.

* Macro SCANCODE_UP should be used instead of 0x80.
  (That's already a problem in the current code).

* Removing the special cases for num lock and caps lock
  and the related code has the same effect as the patch
  (and is therefor the better solution).

Even with the patch applied, there remains an additional
problem:

QEMU changes num lock / caps lock states only when the
key is pressed / released while QEMU's sdl windows has
the input focus.

Users expect that any application window respects the
systems num lock / caps lock state, so we still need a
patch which synchronizes QEMU's state with the host's
state in any case - even without input focus.

Regards,
Stefan

Re: [Qemu-devel] wrong behaviour of caps lock

[Jamie Lokier]

Benjamin Drung wrote:
> -        /* SDL does not send the key up event, so we generate it */

Was the original comment just plain wrong?

> -        kbd_put_keycode(keycode);
> -        kbd_put_keycode(keycode | 0x80);
> +        if (ev->type == SDL_KEYUP) {
> +            kbd_put_keycode(keycode | 0x80);
> +        } else {
> +            kbd_put_keycode(keycode);
> +        }

The patch implies that SDL *does* send the key up event.

Somebody obviously thought that it doesn't, hence the comment.

So what has changed?  Is it different versions of SDL, or does the
patch only work on some hosts / distros?

-- Jamie

Re: [Qemu-devel] wrong behaviour of caps lock

[Kevin Wolf]

Am 19.04.2010 03:23, schrieb Jamie Lokier:
> Benjamin Drung wrote:
>> -        /* SDL does not send the key up event, so we generate it */
> 
> Was the original comment just plain wrong?
> 
>> -        kbd_put_keycode(keycode);
>> -        kbd_put_keycode(keycode | 0x80);
>> +        if (ev->type == SDL_KEYUP) {
>> +            kbd_put_keycode(keycode | 0x80);
>> +        } else {
>> +            kbd_put_keycode(keycode);
>> +        }
> 
> The patch implies that SDL *does* send the key up event.
> 
> Somebody obviously thought that it doesn't, hence the comment.
> 
> So what has changed?  Is it different versions of SDL, or does the
> patch only work on some hosts / distros?

I think we already have had a discussion on this and it turned out that
Ubuntu had a "special" version of SDL which changed this behaviour. So
it is considered an Ubuntu SDL bug. Googled the old discussion for you:

http://www.mail-archive.com/qemu-devel@nongnu.org/msg25246.html

Kevin

Re: [Qemu-devel] wrong behaviour of caps lock

[Stefan Weil]

Kevin Wolf schrieb:
> Am 19.04.2010 03:23, schrieb Jamie Lokier:
>> Benjamin Drung wrote:
>>> - /* SDL does not send the key up event, so we generate it */
>> Was the original comment just plain wrong?
>>
>>> - kbd_put_keycode(keycode);
>>> - kbd_put_keycode(keycode | 0x80);
>>> + if (ev->type == SDL_KEYUP) {
>>> + kbd_put_keycode(keycode | 0x80);
>>> + } else {
>>> + kbd_put_keycode(keycode);
>>> + }
>> The patch implies that SDL *does* send the key up event.
>>
>> Somebody obviously thought that it doesn't, hence the comment.
>>
>> So what has changed? Is it different versions of SDL, or does the
>> patch only work on some hosts / distros?
>
> I think we already have had a discussion on this and it turned out that
> Ubuntu had a "special" version of SDL which changed this behaviour. So
> it is considered an Ubuntu SDL bug. Googled the old discussion for you:
>
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg25246.html
>
> Kevin

My report was based on the Debian testing distribution
with libsdl components version 1.2.13-5.

So if it's a bug, both Ubuntu and Debian share it
(which is not too surprising).

And as I explained in my previous mail on this thread
QEMU's caps lock handling is buggy on any distribution.

Maybe I'll find the time to write a patch until the end
of this week.

Stefan

Re: [Qemu-devel] wrong behaviour of caps lock

[Benjamin Drung]

[-- Attachment #1.1: Type: text/plain, Size: 2166 bytes --]

Am Montag, den 19.04.2010, 18:07 +0200 schrieb Stefan Weil:
> Kevin Wolf schrieb:
> > Am 19.04.2010 03:23, schrieb Jamie Lokier:
> >> Benjamin Drung wrote:
> >>> - /* SDL does not send the key up event, so we generate it */
> >> Was the original comment just plain wrong?
> >>
> >>> - kbd_put_keycode(keycode);
> >>> - kbd_put_keycode(keycode | 0x80);
> >>> + if (ev->type == SDL_KEYUP) {
> >>> + kbd_put_keycode(keycode | 0x80);
> >>> + } else {
> >>> + kbd_put_keycode(keycode);
> >>> + }
> >> The patch implies that SDL *does* send the key up event.
> >>
> >> Somebody obviously thought that it doesn't, hence the comment.
> >>
> >> So what has changed? Is it different versions of SDL, or does the
> >> patch only work on some hosts / distros?
> >
> > I think we already have had a discussion on this and it turned out that
> > Ubuntu had a "special" version of SDL which changed this behaviour. So
> > it is considered an Ubuntu SDL bug. Googled the old discussion for you:
> >
> > http://www.mail-archive.com/qemu-devel@nongnu.org/msg25246.html
> >
> > Kevin
> 
> My report was based on the Debian testing distribution
> with libsdl components version 1.2.13-5.
> 
> So if it's a bug, both Ubuntu and Debian share it
> (which is not too surprising).
> 
> And as I explained in my previous mail on this thread
> QEMU's caps lock handling is buggy on any distribution.
> 
> Maybe I'll find the time to write a patch until the end
> of this week.

I did more investigation, because kvm has this issue in Debian testing
and Ubuntu 10.10 (maverick). The German NEO2 layout maps the caps lock
key to alt gr. I discovered that libsdl sends a key down and key up
event for caps lock if it is mapped to something else. Therefore you
should only send a key up event if caps lock is really used as caps
lock. Patch for kvm 0.12.5 attached. This works for me. Does it work for
your keyboard layout, too?

You find more Details in the Ubuntu bug #427612 [1] beginning with
comment #29.

[1] https://launchpad.net/bugs/427612

-- 
Benjamin Drung
Ubuntu Developer (www.ubuntu.com) | Debian Maintainer (www.debian.org)

[-- Attachment #1.2: caps-key-up-event.patch --]
[-- Type: text/x-patch, Size: 688 bytes --]

=== modified file 'sdl.c'
--- sdl.c	2010-02-26 16:26:00 +0000
+++ sdl.c	2010-11-18 22:47:20 +0000
@@ -390,10 +390,13 @@
         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);
-        return;
+        /* SDL does not send the key up event for real caps lock, so we generate it */
+        if (ev->keysym.sym == 0x12d) {
+            kbd_put_keycode(keycode);
+            kbd_put_keycode(keycode | 0x80);
+            return;
+        }
+        break;
     }
 
     /* now send the key code */


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