[Qemu-devel] [PATCH v2] ui/cocoa.m: prevent stuck key situation

[Qemu-devel] [PATCH v2] ui/cocoa.m: prevent stuck key situation

[Programmingkid]

When the user puts QEMU in the background while holding
down a key, QEMU will not receive the keyup event when
the user lets go of the key. When the user goes back to
QEMU, QEMU will think the key is still down causing
stuck key symptoms. This patch fixes this problem by
releasing all down keys when QEMU goes into the
background. 

Signed-off-by: John Arbuckle <programmingkidx@gmail.com>

---
Removed the modifiers_state global variable.
Added a raiseAllKeys method.

 ui/cocoa.m |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 334e6f6..4d15553 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -304,6 +304,7 @@ static void handleAnyDeviceErrors(Error * err)
 - (float) cdx;
 - (float) cdy;
 - (QEMUScreen) gscreen;
+- (void) raiseAllKeys;
 @end
 
 QemuCocoaView *cocoaView;
@@ -798,6 +799,23 @@ QemuCocoaView *cocoaView;
 - (float) cdx {return cdx;}
 - (float) cdy {return cdy;}
 - (QEMUScreen) gscreen {return screen;}
+
+/*
+ * Makes the target think all down keys are being released.
+ * This prevents a stuck key problem.
+ */
+- (void) raiseAllKeys
+{
+    int index;
+    const int max_index = 220; /* This is the highest value key */
+
+   for (index = 0; index <= max_index; index++) {
+       if (modifiers_state[index]) {
+           modifiers_state[index] = 0;
+           qemu_input_event_send_key_number(dcl->con, index, false);
+       }
+   }
+}
 @end
 
 
@@ -933,6 +951,14 @@ QemuCocoaView *cocoaView;
     return YES;
 }
 
+/* Called when QEMU goes into the background */
+- (void) applicationWillResignActive: (NSNotification *)aNotification
+{
+    COCOA_DEBUG("QemuCocoaAppController: applicationWillResignActive\n");
+
+    [cocoaView raiseAllKeys];
+}
+
 - (void)startEmulationWithArgc:(int)argc argv:(char**)argv
 {
     COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH v2] ui/cocoa.m: prevent stuck key situation

[Peter Maydell]

On 23 September 2015 at 17:17, Programmingkid <programmingkidx@gmail.com> wrote:
> When the user puts QEMU in the background while holding
> down a key, QEMU will not receive the keyup event when
> the user lets go of the key. When the user goes back to
> QEMU, QEMU will think the key is still down causing
> stuck key symptoms. This patch fixes this problem by
> releasing all down keys when QEMU goes into the
> background.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>
> ---
> Removed the modifiers_state global variable.
> Added a raiseAllKeys method.
>
>  ui/cocoa.m |   26 ++++++++++++++++++++++++++
>  1 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 334e6f6..4d15553 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -304,6 +304,7 @@ static void handleAnyDeviceErrors(Error * err)
>  - (float) cdx;
>  - (float) cdy;
>  - (QEMUScreen) gscreen;
> +- (void) raiseAllKeys;
>  @end
>
>  QemuCocoaView *cocoaView;
> @@ -798,6 +799,23 @@ QemuCocoaView *cocoaView;
>  - (float) cdx {return cdx;}
>  - (float) cdy {return cdy;}
>  - (QEMUScreen) gscreen {return screen;}
> +
> +/*
> + * Makes the target think all down keys are being released.
> + * This prevents a stuck key problem.

", since we will not see key up events for those keys after we
have lost focus."

> + */
> +- (void) raiseAllKeys
> +{
> +    int index;
> +    const int max_index = 220; /* This is the highest value key */

No, you need to use ARRAY_SIZE.

> +
> +   for (index = 0; index <= max_index; index++) {
> +       if (modifiers_state[index]) {
> +           modifiers_state[index] = 0;
> +           qemu_input_event_send_key_number(dcl->con, index, false);
> +       }
> +   }

-- PMM

Re: [Qemu-devel] [PATCH v2] ui/cocoa.m: prevent stuck key situation

[Programmingkid]

On Sep 23, 2015, at 8:34 PM, Peter Maydell wrote:

> On 23 September 2015 at 17:17, Programmingkid <programmingkidx@gmail.com> wrote:
>> When the user puts QEMU in the background while holding
>> down a key, QEMU will not receive the keyup event when
>> the user lets go of the key. When the user goes back to
>> QEMU, QEMU will think the key is still down causing
>> stuck key symptoms. This patch fixes this problem by
>> releasing all down keys when QEMU goes into the
>> background.
>> 
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> 
>> ---
>> Removed the modifiers_state global variable.
>> Added a raiseAllKeys method.
>> 
>> ui/cocoa.m |   26 ++++++++++++++++++++++++++
>> 1 files changed, 26 insertions(+), 0 deletions(-)
>> 
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 334e6f6..4d15553 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -304,6 +304,7 @@ static void handleAnyDeviceErrors(Error * err)
>> - (float) cdx;
>> - (float) cdy;
>> - (QEMUScreen) gscreen;
>> +- (void) raiseAllKeys;
>> @end
>> 
>> QemuCocoaView *cocoaView;
>> @@ -798,6 +799,23 @@ QemuCocoaView *cocoaView;
>> - (float) cdx {return cdx;}
>> - (float) cdy {return cdy;}
>> - (QEMUScreen) gscreen {return screen;}
>> +
>> +/*
>> + * Makes the target think all down keys are being released.
>> + * This prevents a stuck key problem.
> 
> ", since we will not see key up events for those keys after we
> have lost focus."

If that is all the changes that are required, I wouldn't mind if you just
added it to the patch.

> 
>> + */
>> +- (void) raiseAllKeys
>> +{
>> +    int index;
>> +    const int max_index = 220; /* This is the highest value key */
> 
> No, you need to use ARRAY_SIZE.

I did use it, it didn't work. The command key would still stay down.
This is because the command key has a value of 220. The keymap
array size is only 126. I know it is confusing. I was thinking of using
cocoa_keycode_to_qemu() to translate the index to the qemu (pc xt)
values, but that would be expensive in terms of cpu usage. 

> 
>> +
>> +   for (index = 0; index <= max_index; index++) {
>> +       if (modifiers_state[index]) {
>> +           modifiers_state[index] = 0;
>> +           qemu_input_event_send_key_number(dcl->con, index, false);
>> +       }
>> +   }
> 
> -- PMM

Re: [Qemu-devel] [PATCH v2] ui/cocoa.m: prevent stuck key situation

[Peter Maydell]

On 23 September 2015 at 17:44, Programmingkid <programmingkidx@gmail.com> wrote:
>
> On Sep 23, 2015, at 8:34 PM, Peter Maydell wrote:

>>> + */
>>> +- (void) raiseAllKeys
>>> +{
>>> +    int index;
>>> +    const int max_index = 220; /* This is the highest value key */
>>
>> No, you need to use ARRAY_SIZE.
>
> I did use it, it didn't work. The command key would still stay down.
> This is because the command key has a value of 220. The keymap
> array size is only 126. I know it is confusing. I was thinking of using
> cocoa_keycode_to_qemu() to translate the index to the qemu (pc xt)
> values, but that would be expensive in terms of cpu usage.

We don't care about the keymap array size. ARRAY_SIZE(modifiers_state)
is 256.

If you're iterating through an array, as we are here, then the
correct upper bound is always ARRAY_SIZE(array).

>>
>>> +
>>> +   for (index = 0; index <= max_index; index++) {
>>> +       if (modifiers_state[index]) {
>>> +           modifiers_state[index] = 0;
>>> +           qemu_input_event_send_key_number(dcl->con, index, false);
>>> +       }
>>> +   }

-- PMM