| Message ID | 1411726999-26513-1-git-send-email-akong@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fr, 2014-09-26 at 18:23 +0800, Amos Kong wrote: > Currently we emit press events of combined keys first, then emit > release events by reverse order. But it doesn't match with physical > keyboard if the keys contain continued & repeated keys. > > For example, (qemu) sendkey a-b-b Hmm, somehow I don't feel like building too much magic into this. If you want send Ctrl-somekey twice just use two sendkey commands ... cheers, Gerd
On Fri, Sep 26, 2014 at 12:36:50PM +0200, Gerd Hoffmann wrote: > On Fr, 2014-09-26 at 18:23 +0800, Amos Kong wrote: > > Currently we emit press events of combined keys first, then emit > > release events by reverse order. But it doesn't match with physical > > keyboard if the keys contain continued & repeated keys. > > > > For example, (qemu) sendkey a-b-b > > Hmm, somehow I don't feel like building too much magic into this. > If you want send Ctrl-somekey twice just use two sendkey commands ... Before this patch, If 'sendkey r-o-o-t', only 'rot' can be inputted. People want to panic windows by sending Ctrl-Scrool-Scrool http://msdn.microsoft.com/en-us/library/windows/hardware/ff545499(v=vs.85).aspx But current events order doesn't work. In physical keyboard. We can prese Ctrl first, then press & release Scroll twice, then release Ctrl. It's very common behavior. So this fix just reference the physical implement, if you want to input same key twice, you have to release it before second pressing. (here we ignore the auto-repeat feature) > cheers, > Gerd >
On Fr, 2014-09-26 at 18:53 +0800, Amos Kong wrote: > On Fri, Sep 26, 2014 at 12:36:50PM +0200, Gerd Hoffmann wrote: > > On Fr, 2014-09-26 at 18:23 +0800, Amos Kong wrote: > > > Currently we emit press events of combined keys first, then emit > > > release events by reverse order. But it doesn't match with physical > > > keyboard if the keys contain continued & repeated keys. > > > > > > For example, (qemu) sendkey a-b-b > > > > Hmm, somehow I don't feel like building too much magic into this. > > If you want send Ctrl-somekey twice just use two sendkey commands ... > > > Before this patch, If 'sendkey r-o-o-t', only 'rot' can be inputted. To type 'root' you should use sendkey r sendkey o sendkey o sendkey t Multiple keys in sendkey is meant for multiple keys pressed at the same time, i.e. ctrl-alt-del, not for sending key sequences and typing words. > People want to panic windows by sending Ctrl-Scrool-Scrool > http://msdn.microsoft.com/en-us/library/windows/hardware/ff545499(v=vs.85).aspx > But current events order doesn't work. sendkey Ctrl-Scroll sendkey Ctrl-Scroll > In physical keyboard. We can prese Ctrl first, then press & release > Scroll twice, then release Ctrl. It's very common behavior. In most cases it doesn't matter whenever you release the modifier key or not. The windows panic hotkey might be the exception from the rule though. > So this fix just reference the physical implement, if you want to > input same key twice, you have to release it before second pressing. > (here we ignore the auto-repeat feature) sendkey doesn't cover that use case indeed. /me wonders what happened to the input-send-event patch from marcelo, see http://patchwork.ozlabs.org/patch/360649/ According to patchwork I've picked it up. But it is neither upstream nor in my local input branch. And I can't remember what happened :( Marcelo, any clue? Maybe I should just re-queue it ... The input-send-event gives you fine-grained control about the exact input event sequence and it can handle your use case without problems. cheers, Gerd
On 09/26/2014 04:53 AM, Amos Kong wrote: > On Fri, Sep 26, 2014 at 12:36:50PM +0200, Gerd Hoffmann wrote: >> On Fr, 2014-09-26 at 18:23 +0800, Amos Kong wrote: >>> Currently we emit press events of combined keys first, then emit >>> release events by reverse order. But it doesn't match with physical >>> keyboard if the keys contain continued & repeated keys. >>> >>> For example, (qemu) sendkey a-b-b >> >> Hmm, somehow I don't feel like building too much magic into this. >> If you want send Ctrl-somekey twice just use two sendkey commands ... > > > Before this patch, If 'sendkey r-o-o-t', only 'rot' can be inputted. sendkey with multiple keys is designed for shift modifiers, NOT for repeated keys. 'sendkey r-o-o-t' makes no sense; it is the equivalent of mashing down the 'r', 'o', and 't' keys at once. To send four separate key events, you need to do 'sendkey r; sendkey o; sendkey o; sendkey t'. (Or libvirt should learn some syntactic sugar so you could do 'sendkey r -- o -- o -- t' and sequence it automatically). I don't think this patch is worth the effort. > > People want to panic windows by sending Ctrl-Scrool-Scrool s/Scrool/Scroll/ > http://msdn.microsoft.com/en-us/library/windows/hardware/ff545499(v=vs.85).aspx > But current events order doesn't work. > > In physical keyboard. We can prese Ctrl first, then press & release > Scroll twice, then release Ctrl. It's very common behavior. > Hmm. This almost argues that we need a way to send a press without a release, or a release without a press, rather than trying to make 'sendkey Ctrl-Scroll-Scroll' emulate double scroll pushes all while ctrl is still held down. But this example is MUCH more realistic than 'r-o-o-t', so maybe there is merit to your patch after all. > So this fix just reference the physical implement, if you want to > input same key twice, you have to release it before second pressing. > (here we ignore the auto-repeat feature) >
On Fri, Sep 26, 2014 at 01:24:05PM +0200, Gerd Hoffmann wrote: > On Fr, 2014-09-26 at 18:53 +0800, Amos Kong wrote: > > On Fri, Sep 26, 2014 at 12:36:50PM +0200, Gerd Hoffmann wrote: > > > On Fr, 2014-09-26 at 18:23 +0800, Amos Kong wrote: > > > > Currently we emit press events of combined keys first, then emit > > > > release events by reverse order. But it doesn't match with physical > > > > keyboard if the keys contain continued & repeated keys. > > > > > > > > For example, (qemu) sendkey a-b-b > > > > > > Hmm, somehow I don't feel like building too much magic into this. > > > If you want send Ctrl-somekey twice just use two sendkey commands ... > > > > > > Before this patch, If 'sendkey r-o-o-t', only 'rot' can be inputted. > > To type 'root' you should use > > sendkey r > sendkey o > sendkey o > sendkey t > > Multiple keys in sendkey is meant for multiple keys pressed at the same > time, i.e. ctrl-alt-del, not for sending key sequences and typing words. > > > People want to panic windows by sending Ctrl-Scrool-Scrool > > http://msdn.microsoft.com/en-us/library/windows/hardware/ff545499(v=vs.85).aspx > > But current events order doesn't work. > > sendkey Ctrl-Scroll > sendkey Ctrl-Scroll > > > In physical keyboard. We can prese Ctrl first, then press & release > > Scroll twice, then release Ctrl. It's very common behavior. > > In most cases it doesn't matter whenever you release the modifier key or > not. The windows panic hotkey might be the exception from the rule > though. > > > So this fix just reference the physical implement, if you want to > > input same key twice, you have to release it before second pressing. > > (here we ignore the auto-repeat feature) > > sendkey doesn't cover that use case indeed. > > /me wonders what happened to the input-send-event patch from marcelo, > see http://patchwork.ozlabs.org/patch/360649/ > > According to patchwork I've picked it up. But it is neither upstream > nor in my local input branch. And I can't remember what happened :( > Marcelo, any clue? Maybe I should just re-queue it ... I was wondering the same... just requeue please. Let me know if it fails to apply and i'll rebase/resend. > The input-send-event gives you fine-grained control about the exact > input event sequence and it can handle your use case without problems. > > cheers, > Gerd
On Fri, Sep 26, 2014 at 01:24:05PM +0200, Gerd Hoffmann wrote: > On Fr, 2014-09-26 at 18:53 +0800, Amos Kong wrote: > > On Fri, Sep 26, 2014 at 12:36:50PM +0200, Gerd Hoffmann wrote: > > > On Fr, 2014-09-26 at 18:23 +0800, Amos Kong wrote: > > > > Currently we emit press events of combined keys first, then emit > > > > release events by reverse order. But it doesn't match with physical > > > > keyboard if the keys contain continued & repeated keys. > > > > > > > > For example, (qemu) sendkey a-b-b > > > > > > Hmm, somehow I don't feel like building too much magic into this. > > > If you want send Ctrl-somekey twice just use two sendkey commands ... > > > > > > Before this patch, If 'sendkey r-o-o-t', only 'rot' can be inputted. > > To type 'root' you should use > > sendkey r > sendkey o > sendkey o > sendkey t > > Multiple keys in sendkey is meant for multiple keys pressed at the same > time, i.e. ctrl-alt-del, not for sending key sequences and typing words. > > > People want to panic windows by sending Ctrl-Scrool-Scrool > > http://msdn.microsoft.com/en-us/library/windows/hardware/ff545499(v=vs.85).aspx > > But current events order doesn't work. > > sendkey Ctrl-Scroll > sendkey Ctrl-Scroll > > > In physical keyboard. We can prese Ctrl first, then press & release > > Scroll twice, then release Ctrl. It's very common behavior. > > In most cases it doesn't matter whenever you release the modifier key or > not. The windows panic hotkey might be the exception from the rule > though. It doesn't matter, so users might release the modifier key or not. we should make both works 1) sendkey Ctrl-Scroll sendkey Ctrl-Scroll 2) sendkey Ctrl-Scroll-Scroll > > So this fix just reference the physical implement, if you want to > > input same key twice, you have to release it before second pressing. > > (here we ignore the auto-repeat feature) > > sendkey doesn't cover that use case indeed. original sendkey didn't cover it, but my change isn't added a split feature, but try to adapt native behavior by a little change. It doesn't effect original right behavior, but process repeated case in native way. > /me wonders what happened to the input-send-event patch from marcelo, > see http://patchwork.ozlabs.org/patch/360649/ > > According to patchwork I've picked it up. But it is neither upstream > nor in my local input branch. And I can't remember what happened :( > Marcelo, any clue? Maybe I should just re-queue it ... > > The input-send-event gives you fine-grained control about the exact > input event sequence and it can handle your use case without problems. > > cheers, > Gerd
Hi, > > /me wonders what happened to the input-send-event patch from marcelo, > > see http://patchwork.ozlabs.org/patch/360649/ > > > > According to patchwork I've picked it up. But it is neither upstream > > nor in my local input branch. And I can't remember what happened :( > > Marcelo, any clue? Maybe I should just re-queue it ... > > I was wondering the same... just requeue please. Let me know if it fails > to apply and i'll rebase/resend. Doesn't apply any more, so yes, please rebase & resend. thanks, Gerd
Hi, > It doesn't matter, so users might release the modifier key or not. > we should make both works > > 1) > sendkey Ctrl-Scroll > sendkey Ctrl-Scroll Good to know this works. > 2) > sendkey Ctrl-Scroll-Scroll Why? This tries to squeeze something into the sendkey interface which it doesn't was designed for, and IMO this isn't a good idea, especially if we have something better at hand (marcelos patch). cheers, Gerd
On Mon, Sep 29, 2014 at 11:09:56AM +0200, Gerd Hoffmann wrote: > Hi, > > > It doesn't matter, so users might release the modifier key or not. > > we should make both works > > > > 1) > > sendkey Ctrl-Scroll > > sendkey Ctrl-Scroll > > Good to know this works. > > > 2) > > sendkey Ctrl-Scroll-Scroll > > Why? Quote from http://msdn.microsoft.com/en-us/library/windows/hardware/ff545499(v=vs.85).aspx The keyboard crash can be initiated by using the following hotkey sequence: _Hold down_ the rightmost CTRL key, and press the SCROLL LOCK key twice. > This tries to squeeze something into the sendkey interface which > it doesn't was designed for, and IMO this isn't a good idea, especially > if we have something better at hand (marcelos patch). My patch fixed this problem, but it's a hack. Marcelo's patch works in (press/release) event level, it's more clear. Thanks. > cheers, > Gerd >
diff --git a/ui/input-legacy.c b/ui/input-legacy.c index a698a34..3fd3e83 100644 --- a/ui/input-legacy.c +++ b/ui/input-legacy.c @@ -95,9 +95,17 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time, for (p = keys; p != NULL; p = p->next) { qemu_input_event_send_key(NULL, copy_key_value(p->value), true); qemu_input_event_send_key_delay(hold_time); - up = g_realloc(up, sizeof(*up) * (count+1)); - up[count] = copy_key_value(p->value); - count++; + + /* release event will be emitted immediately if next key is repeated */ + if (p->next && p->value->kind == p->next->value->kind && + p->value->qcode == p->next->value->qcode) { + qemu_input_event_send_key(NULL, copy_key_value(p->value), false); + qemu_input_event_send_key_delay(hold_time); + } else { + up = g_realloc(up, sizeof(*up) * (count+1)); + up[count] = copy_key_value(p->value); + count++; + } } while (count) { count--;
Currently we emit press events of combined keys first, then emit release events by reverse order. But it doesn't match with physical keyboard if the keys contain continued & repeated keys. For example, (qemu) sendkey a-b-b Current emited events: (actually the second 'presse b' and 'release b' can't be identified by guest, the interval is too short) press a press b press b release b release b release a Correct events order of physical keyboard: press a press b release b press b release b release a This patch fixed the event order if keys contain continued & repeated keys. his patch based on: http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg05150.html Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Amos Kong <akong@redhat.com> --- V2: rebase this patch on Gerd's better fix of release order --- ui/input-legacy.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)