| Message ID | 4D622DB1-AB63-4FDE-8B40-30FE9897E510@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 18 September 2015 at 22:46, 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 keys when QEMU goes into the > background. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > > --- > ui/cocoa.m | 17 ++++++++++++++++- > 1 files changed, 16 insertions(+), 1 deletions(-) Can you please fix how you're sending patch emails not to send them as HTML? This really does mess up some of the tools we use to process patchmail, and it's a pain to have to handle them manually (also error-prone; last patch of yours I applied I messed up fixing the authorship on). The git send-email tool is one way of doing this. thanks -- PMM
On Sep 19, 2015, at 11:07 AM, Peter Maydell wrote: > On 18 September 2015 at 22:46, 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 keys when QEMU goes into the >> background. >> >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >> >> --- >> ui/cocoa.m | 17 ++++++++++++++++- >> 1 files changed, 16 insertions(+), 1 deletions(-) > > Can you please fix how you're sending patch emails not to send > them as HTML? I just copy and paste them into Apple's Mail application. Didn't know it makes it into html. > This really does mess up some of the tools we > use to process patchmail, and it's a pain to have to handle > them manually (also error-prone; last patch of yours I > applied I messed up fixing the authorship on). > > The git send-email tool is one way of doing this. I will learn this send-email tool today.
On 18 September 2015 at 14:46, 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 keys when QEMU goes into the > background. Looks like maybe you're not wrapping lines early enough in your commit messages, resulting in this ugly effect when they're quoted. It's best to not have lines longer than 75 chars or so. > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > > --- > ui/cocoa.m | 17 ++++++++++++++++- > 1 files changed, 16 insertions(+), 1 deletions(-) > > diff --git a/ui/cocoa.m b/ui/cocoa.m > index 334e6f6..d07b22d 100644 > --- a/ui/cocoa.m > +++ b/ui/cocoa.m > @@ -69,6 +69,7 @@ char **gArgv; > bool stretch_video; > NSTextField *pauseLabel; > NSArray * supportedImageFileTypes; > +int modifiers_state[256]; Rather than making this global, could we have the applicationWillResignActive method call a new raiseAllKeys method on the NSView? > > > > // keymap conversion > int keymap[] = > @@ -274,7 +275,6 @@ static void handleAnyDeviceErrors(Error * err) > NSWindow *fullScreenWindow; > float cx,cy,cw,ch,cdx,cdy; > CGDataProviderRef dataProviderRef; > - int modifiers_state[256]; > BOOL isMouseGrabbed; > BOOL isFullscreen; > BOOL isAbsoluteEnabled; > @@ -933,6 +933,21 @@ QemuCocoaView *cocoaView; > return YES; > } > > > > +/* Called when QEMU goes into the background */ > +- (void) applicationWillResignActive: (NSNotification *)aNotification > +{ > + COCOA_DEBUG("QemuCocoaAppController: applicationWillResignActive\n"); > + int keycode, index; > + const int max_index = 126; /* This is the size of the keymap array */ Hardcoding array sizes is never a good idea. We have an ARRAY_SIZE macro which automatically gets it right. > + > + /* Release all the keys to prevent a stuck key situation */ > + for(index = 0; index <= max_index; index++) { > + keycode = keymap[index]; > + modifiers_state[keycode] = 0; > + qemu_input_event_send_key_number(dcl->con, keycode, false); > + } This will send key-up events even for keys which are already up. Instead you can just send events for only the keys which are down (and avoid the lookup in keymap[] too): for (i = 0; i < ARRAY_SIZE(modifiers_state); i++) { if (modifiers_state[i])) { modifiers_state[i] = 0; qemu_input_event_send_key_number(dcl->con, i, false); } } thanks -- PMM
On Sep 23, 2015, at 2:04 PM, Peter Maydell wrote: > On 18 September 2015 at 14:46, 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 keys when QEMU goes into the >> background. > > Looks like maybe you're not wrapping lines early enough in your > commit messages, resulting in this ugly effect when they're > quoted. It's best to not have lines longer than 75 chars or so. Sorry, will make the lines shorter in the future. > >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >> >> --- >> ui/cocoa.m | 17 ++++++++++++++++- >> 1 files changed, 16 insertions(+), 1 deletions(-) >> >> diff --git a/ui/cocoa.m b/ui/cocoa.m >> index 334e6f6..d07b22d 100644 >> --- a/ui/cocoa.m >> +++ b/ui/cocoa.m >> @@ -69,6 +69,7 @@ char **gArgv; >> bool stretch_video; >> NSTextField *pauseLabel; >> NSArray * supportedImageFileTypes; >> +int modifiers_state[256]; > > Rather than making this global, could we have the applicationWillResignActive > method call a new raiseAllKeys method on the NSView? We could do that, but isn't this more of an app controller function? >> >> >> >> // keymap conversion >> int keymap[] = >> @@ -274,7 +275,6 @@ static void handleAnyDeviceErrors(Error * err) >> NSWindow *fullScreenWindow; >> float cx,cy,cw,ch,cdx,cdy; >> CGDataProviderRef dataProviderRef; >> - int modifiers_state[256]; >> BOOL isMouseGrabbed; >> BOOL isFullscreen; >> BOOL isAbsoluteEnabled; >> @@ -933,6 +933,21 @@ QemuCocoaView *cocoaView; >> return YES; >> } >> >> >> >> +/* Called when QEMU goes into the background */ >> +- (void) applicationWillResignActive: (NSNotification *)aNotification >> +{ >> + COCOA_DEBUG("QemuCocoaAppController: applicationWillResignActive\n"); >> + int keycode, index; >> + const int max_index = 126; /* This is the size of the keymap array */ > > Hardcoding array sizes is never a good idea. We have an ARRAY_SIZE > macro which automatically gets it right. Didn't know about this macro. Will use it in the next patch. > > >> + >> + /* Release all the keys to prevent a stuck key situation */ >> + for(index = 0; index <= max_index; index++) { >> + keycode = keymap[index]; >> + modifiers_state[keycode] = 0; >> + qemu_input_event_send_key_number(dcl->con, keycode, false); >> + } > > This will send key-up events even for keys which are already up. > Instead you can just send events for only the keys which are down > (and avoid the lookup in keymap[] too): > > > for (i = 0; i < ARRAY_SIZE(modifiers_state); i++) { > if (modifiers_state[i])) { > modifiers_state[i] = 0; > qemu_input_event_send_key_number(dcl->con, i, false); > } > } Sounds good. Will use it.
On 23 September 2015 at 14:44, Programmingkid <programmingkidx@gmail.com> wrote: > > On Sep 23, 2015, at 2:04 PM, Peter Maydell wrote: > >> On 18 September 2015 at 14:46, 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 keys when QEMU goes into the >>> background. >> >> Looks like maybe you're not wrapping lines early enough in your >> commit messages, resulting in this ugly effect when they're >> quoted. It's best to not have lines longer than 75 chars or so. > > Sorry, will make the lines shorter in the future. > >> >>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >>> >>> --- >>> ui/cocoa.m | 17 ++++++++++++++++- >>> 1 files changed, 16 insertions(+), 1 deletions(-) >>> >>> diff --git a/ui/cocoa.m b/ui/cocoa.m >>> index 334e6f6..d07b22d 100644 >>> --- a/ui/cocoa.m >>> +++ b/ui/cocoa.m >>> @@ -69,6 +69,7 @@ char **gArgv; >>> bool stretch_video; >>> NSTextField *pauseLabel; >>> NSArray * supportedImageFileTypes; >>> +int modifiers_state[256]; >> >> Rather than making this global, could we have the applicationWillResignActive >> method call a new raiseAllKeys method on the NSView? > > We could do that, but isn't this more of an app controller function? I don't feel strongly about where things should live, globals just make me think we could arrange things better somehow. -- PMM
diff --git a/ui/cocoa.m b/ui/cocoa.m index 334e6f6..d07b22d 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -69,6 +69,7 @@ char **gArgv; bool stretch_video; NSTextField *pauseLabel; NSArray * supportedImageFileTypes; +int modifiers_state[256]; // keymap conversion int keymap[] = @@ -274,7 +275,6 @@ static void handleAnyDeviceErrors(Error * err) NSWindow *fullScreenWindow; float cx,cy,cw,ch,cdx,cdy; CGDataProviderRef dataProviderRef; - int modifiers_state[256]; BOOL isMouseGrabbed; BOOL isFullscreen; BOOL isAbsoluteEnabled; @@ -933,6 +933,21 @@ QemuCocoaView *cocoaView; return YES; } +/* Called when QEMU goes into the background */ +- (void) applicationWillResignActive: (NSNotification *)aNotification +{ + COCOA_DEBUG("QemuCocoaAppController: applicationWillResignActive\n"); + int keycode, index; + const int max_index = 126; /* This is the size of the keymap array */ + + /* Release all the keys to prevent a stuck key situation */ + for(index = 0; index <= max_index; index++) { + keycode = keymap[index]; + modifiers_state[keycode] = 0; + qemu_input_event_send_key_number(dcl->con, keycode, false); + } +} + - (void)startEmulationWithArgc:(int)argc argv:(char**)argv { COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
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 keys when QEMU goes into the background. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- ui/cocoa.m | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-)