| Message ID | 1484132292-21458-1-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11 January 2017 at 10:58, Gerd Hoffmann <kraxel@redhat.com> wrote: > No need to go the indirect route with a bitfield and mapping the bits to > INPUT_BUTTON_*. We can use a bool array and INPUT_BUTTON_* directly > instead. > > Untested, not even compiled, due to lack of a osx^Wmacos machine. > Test results are very welcome. So what does the patch gain us? We'll now loop round that button array for every event rather than only the ones where the button state changed, which I don't suppose is a big deal but does somewhat incline me towards "just leave the code the way it is"... thanks -- PMM
On Mo, 2017-01-16 at 18:35 +0000, Peter Maydell wrote: > On 11 January 2017 at 10:58, Gerd Hoffmann <kraxel@redhat.com> wrote: > > No need to go the indirect route with a bitfield and mapping the bits to > > INPUT_BUTTON_*. We can use a bool array and INPUT_BUTTON_* directly > > instead. > > > > Untested, not even compiled, due to lack of a osx^Wmacos machine. > > Test results are very welcome. > > So what does the patch gain us? I can drop MOUSE_EVENT_WHEEL* without breaking the osx build. > We'll now loop round > that button array for every event rather than only > the ones where the button state changed, Well, we loop round anyway, you just don't see it directly b/c it is hidden in the qemu_input_update_buttons() helper function. cheers, Gerd
On 17 January 2017 at 08:20, Gerd Hoffmann <kraxel@redhat.com> wrote: > On Mo, 2017-01-16 at 18:35 +0000, Peter Maydell wrote: >> On 11 January 2017 at 10:58, Gerd Hoffmann <kraxel@redhat.com> wrote: >> > No need to go the indirect route with a bitfield and mapping the bits to >> > INPUT_BUTTON_*. We can use a bool array and INPUT_BUTTON_* directly >> > instead. >> > >> > Untested, not even compiled, due to lack of a osx^Wmacos machine. >> > Test results are very welcome. >> >> So what does the patch gain us? > > I can drop MOUSE_EVENT_WHEEL* without breaking the osx build. Why do you want to drop it, though? It's a valid mouse event type, and you're not going to drop the whole MOUSE_EVENT_* set of defines, I assume, because they're used in lots of devices. >> We'll now loop round >> that button array for every event rather than only >> the ones where the button state changed, > > Well, we loop round anyway, you just don't see it directly b/c it is > hidden in the qemu_input_update_buttons() helper function. No, we only call qemu_input_update_buttons() if last_buttons != buttons. thanks -- PMM
On Di, 2017-01-17 at 10:44 +0000, Peter Maydell wrote: > On 17 January 2017 at 08:20, Gerd Hoffmann <kraxel@redhat.com> wrote: > > On Mo, 2017-01-16 at 18:35 +0000, Peter Maydell wrote: > >> On 11 January 2017 at 10:58, Gerd Hoffmann <kraxel@redhat.com> wrote: > >> > No need to go the indirect route with a bitfield and mapping the bits to > >> > INPUT_BUTTON_*. We can use a bool array and INPUT_BUTTON_* directly > >> > instead. > >> > > >> > Untested, not even compiled, due to lack of a osx^Wmacos machine. > >> > Test results are very welcome. > >> > >> So what does the patch gain us? > > > > I can drop MOUSE_EVENT_WHEEL* without breaking the osx build. > > Why do you want to drop it, though? It's a valid mouse event > type, and you're not going to drop the whole MOUSE_EVENT_* set > of defines, I assume, because they're used in lots of devices. Dunno where the MOUSE_EVENT_* #defines originally come from, I suspect from old ps2 mouse protocols, for historical reasons, simliar to the numeric key codes we have in qemu which are pc scancodes too. So, yes, I considered dropping them all and use hardware-specific #defines instead. But it seems at least for the left/right/middle buttons most (all?) hardware agrees on which bit is which. So the attempt to drop the button defines looks like pointless churn. For the wheel that isn't the case though, that is either not present at all (older devices) or implemented in different ways (virtual buttons, wheel axis, ...). cheers, Gerd
diff --git a/ui/cocoa.m b/ui/cocoa.m index 26d4a1c..599b899 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -66,7 +66,7 @@ typedef struct { NSWindow *normalWindow, *about_window; static DisplayChangeListener *dcl; -static int last_buttons; +static bool last_buttons[INPUT_BUTTON__MAX]; int gArgc; char **gArgv; @@ -511,7 +511,9 @@ QemuCocoaView *cocoaView; { COCOA_DEBUG("QemuCocoaView: handleEvent\n"); - int buttons = 0; + bool buttons[INPUT_BUTTON__MAX] = { + [ 0 ... (INPUT_BUTTON__MAX-1) ] = false; + }; int keycode; bool mouse_event = false; NSPoint p = [event locationInWindow]; @@ -638,34 +640,34 @@ QemuCocoaView *cocoaView; break; case NSLeftMouseDown: if ([event modifierFlags] & NSCommandKeyMask) { - buttons |= MOUSE_EVENT_RBUTTON; + buttons[INPUT_BUTTON_RIGHT] = true; } else { - buttons |= MOUSE_EVENT_LBUTTON; + buttons[INPUT_BUTTON_LEFT] = true; } mouse_event = true; break; case NSRightMouseDown: - buttons |= MOUSE_EVENT_RBUTTON; + buttons[INPUT_BUTTON_RIGHT] = true; mouse_event = true; break; case NSOtherMouseDown: - buttons |= MOUSE_EVENT_MBUTTON; + buttons[INPUT_BUTTON_MIDDLE] = true; mouse_event = true; break; case NSLeftMouseDragged: if ([event modifierFlags] & NSCommandKeyMask) { - buttons |= MOUSE_EVENT_RBUTTON; + buttons[INPUT_BUTTON_RIGHT] = true; } else { - buttons |= MOUSE_EVENT_LBUTTON; + buttons[INPUT_BUTTON_LEFT] = true; } mouse_event = true; break; case NSRightMouseDragged: - buttons |= MOUSE_EVENT_RBUTTON; + buttons[INPUT_BUTTON_RIGHT] = true; mouse_event = true; break; case NSOtherMouseDragged: - buttons |= MOUSE_EVENT_MBUTTON; + buttons[INPUT_BUTTON_MIDDLE] = true; mouse_event = true; break; case NSLeftMouseUp: @@ -684,8 +686,11 @@ QemuCocoaView *cocoaView; break; case NSScrollWheel: if (isMouseGrabbed) { - buttons |= ([event deltaY] < 0) ? - MOUSE_EVENT_WHEELUP : MOUSE_EVENT_WHEELDN; + if ([event deltaY] < 0) { + buttons[INPUT_BUTTON_WHEEL_UP] = true; + } else { + buttons[INPUT_BUTTON_WHEEL_DOWN] = true; + } } mouse_event = true; break; @@ -703,15 +708,14 @@ QemuCocoaView *cocoaView; */ if ((isMouseGrabbed || [[self window] isKeyWindow]) && (last_buttons != buttons)) { - static uint32_t bmap[INPUT_BUTTON__MAX] = { - [INPUT_BUTTON_LEFT] = MOUSE_EVENT_LBUTTON, - [INPUT_BUTTON_MIDDLE] = MOUSE_EVENT_MBUTTON, - [INPUT_BUTTON_RIGHT] = MOUSE_EVENT_RBUTTON, - [INPUT_BUTTON_WHEEL_UP] = MOUSE_EVENT_WHEELUP, - [INPUT_BUTTON_WHEEL_DOWN] = MOUSE_EVENT_WHEELDN, - }; - qemu_input_update_buttons(dcl->con, bmap, last_buttons, buttons); - last_buttons = buttons; + InputButton btn; + + for (btn = 0; btn < INPUT_BUTTON__MAX; btn++) { + if (last_button[btn] != button[btn]) { + qemu_input_queue_btn(dcl->con, btn, button[btn]); + last_button[btn] = button[btn]; + } + } } if (isMouseGrabbed) { if (isAbsoluteEnabled) {
No need to go the indirect route with a bitfield and mapping the bits to INPUT_BUTTON_*. We can use a bool array and INPUT_BUTTON_* directly instead. Untested, not even compiled, due to lack of a osx^Wmacos machine. Test results are very welcome. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- ui/cocoa.m | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-)