| Message ID | 20230413152120.53967-3-slp@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement virtio-multitouch and enable GTK3 to use it | expand |
Sergio Lopez <slp@redhat.com> writes: > Add the required infrastructure to support generating multitouch events. > > Signed-off-by: Sergio Lopez <slp@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > include/ui/input.h | 3 +++ > qapi/ui.json | 46 ++++++++++++++++++++++++++++++++++++++++--- > replay/replay-input.c | 18 +++++++++++++++++ > ui/input.c | 6 ++++++ > ui/trace-events | 1 + > 5 files changed, 71 insertions(+), 3 deletions(-) > > diff --git a/include/ui/input.h b/include/ui/input.h > index c86219a1c1..2a3dffd417 100644 > --- a/include/ui/input.h > +++ b/include/ui/input.h > @@ -8,9 +8,12 @@ > #define INPUT_EVENT_MASK_BTN (1<<INPUT_EVENT_KIND_BTN) > #define INPUT_EVENT_MASK_REL (1<<INPUT_EVENT_KIND_REL) > #define INPUT_EVENT_MASK_ABS (1<<INPUT_EVENT_KIND_ABS) > +#define INPUT_EVENT_MASK_MTT (1<<INPUT_EVENT_KIND_MTT) > > #define INPUT_EVENT_ABS_MIN 0x0000 > #define INPUT_EVENT_ABS_MAX 0x7FFF > +#define INPUT_EVENT_SLOTS_MIN 0x0 > +#define INPUT_EVENT_SLOTS_MAX 0xa > > typedef struct QemuInputHandler QemuInputHandler; > typedef struct QemuInputHandlerState QemuInputHandlerState; > diff --git a/qapi/ui.json b/qapi/ui.json > index 98322342f7..83369bdae8 100644 > --- a/qapi/ui.json > +++ b/qapi/ui.json > @@ -1014,7 +1014,7 @@ > ## > { 'enum' : 'InputButton', > 'data' : [ 'left', 'middle', 'right', 'wheel-up', 'wheel-down', 'side', > - 'extra', 'wheel-left', 'wheel-right' ] } > + 'extra', 'wheel-left', 'wheel-right', 'touch' ] } > > ## > # @InputAxis: > @@ -1026,6 +1026,17 @@ > { 'enum' : 'InputAxis', > 'data' : [ 'x', 'y' ] } > > +## > +# @InputMultitouchType: Suggest InputMultiTouchType, because... > +# > +# Type of a multitouch event. ... the common spelling is multi-touch. More of the same below. > +# > +# Since: 8.1 > +## > +{ 'enum' : 'InputMultitouchType', > + 'data' : [ 'begin', 'update', 'end', 'cancel', 'data' ] } > + > + > ## > # @InputKeyEvent: > # > @@ -1069,13 +1080,32 @@ > 'data' : { 'axis' : 'InputAxis', > 'value' : 'int' } } > > +## > +# @InputMultitouchEvent: > +# > +# Multitouch input event. > +# > +# @slot: Which slot has generated the event. Ignorant question: what's a "slot"? > +# @tracking-id: ID to correlate this event with previously generated events. > +# @axis: Which axis is referenced by @value. > +# @value: Contact position. > +# > +# Since: 8.1 > +## > +{ 'struct' : 'InputMultitouchEvent', > + 'data' : { 'type' : 'InputMultitouchType', > + 'slot' : 'int', > + 'tracking-id': 'int', > + 'axis' : 'InputAxis', > + 'value' : 'int' } } > + > ## > # @InputEventKind: > # > # Since: 2.0 > ## > { 'enum': 'InputEventKind', > - 'data': [ 'key', 'btn', 'rel', 'abs' ] } > + 'data': [ 'key', 'btn', 'rel', 'abs', 'mtt' ] } While we generally avoid abbreviations in QAPI, local consistency is a strong argument for this one. Okay. > > ## > # @InputKeyEventWrapper: > @@ -1101,6 +1131,14 @@ > { 'struct': 'InputMoveEventWrapper', > 'data': { 'data': 'InputMoveEvent' } } > > +## > +# @InputMultitouchEventWrapper: > +# > +# Since: 8.1 > +## > +{ 'struct': 'InputMultitouchEventWrapper', > + 'data': { 'data': 'InputMultitouchEvent' } } The only reason for wrapping is consistency with the other branches. Okay. > + > ## > # @InputEvent: > # > @@ -1112,6 +1150,7 @@ # @type: the input type, one of: # # - 'key': Input event of Keyboard > # - 'btn': Input event of pointer buttons > # - 'rel': Input event of relative pointer motion > # - 'abs': Input event of absolute pointer motion > +# - 'mtt': Input event of Multitouch You're imitating the existing "Input event of" pattern, which is fair. But the pattern is bad. The phrasing awkward, and so is the place. By documenting the values of InputEventKind only here, and not in InputEventKind's doc comment, the generated documentation for InputEventKind looks like this: "InputEventKind" (Enum) ----------------------- Values ~~~~~~ "key" Not documented "btn" Not documented "rel" Not documented "abs" Not documented "mtt" Not documented Since ~~~~~ 2.0 We should document them right in InputEventKind's doc comment, roughly like this: ## # @InputEventKind: # # @key: a keyboard input event # @btn: a pointer button input event # @rel: a relative pointer motion input event # @abs: an absolute pointer motion input event # @mtt: a multi-touch input event # # Since: 2.0 ## We can then dumb down the documentation of InputEvent member @type to just # @type: the type of input event What do you think? Many more doc comments neglect to document members in this file, and in others. I'm not asking you to fix them all. > # > # Since: 2.0 > ## > @@ -1121,7 +1160,8 @@ > 'data' : { 'key' : 'InputKeyEventWrapper', > 'btn' : 'InputBtnEventWrapper', > 'rel' : 'InputMoveEventWrapper', > - 'abs' : 'InputMoveEventWrapper' } } > + 'abs' : 'InputMoveEventWrapper', > + 'mtt' : 'InputMultitouchEventWrapper' } } > > ## > # @input-send-event: [...]
On Mon, Apr 17, 2023 at 12:57:08PM +0200, Markus Armbruster wrote: > Sergio Lopez <slp@redhat.com> writes: > > > Add the required infrastructure to support generating multitouch events. > > > > Signed-off-by: Sergio Lopez <slp@redhat.com> > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > include/ui/input.h | 3 +++ > > qapi/ui.json | 46 ++++++++++++++++++++++++++++++++++++++++--- > > replay/replay-input.c | 18 +++++++++++++++++ > > ui/input.c | 6 ++++++ > > ui/trace-events | 1 + > > 5 files changed, 71 insertions(+), 3 deletions(-) > > > > diff --git a/include/ui/input.h b/include/ui/input.h > > index c86219a1c1..2a3dffd417 100644 > > --- a/include/ui/input.h > > +++ b/include/ui/input.h > > @@ -8,9 +8,12 @@ > > #define INPUT_EVENT_MASK_BTN (1<<INPUT_EVENT_KIND_BTN) > > #define INPUT_EVENT_MASK_REL (1<<INPUT_EVENT_KIND_REL) > > #define INPUT_EVENT_MASK_ABS (1<<INPUT_EVENT_KIND_ABS) > > +#define INPUT_EVENT_MASK_MTT (1<<INPUT_EVENT_KIND_MTT) > > > > #define INPUT_EVENT_ABS_MIN 0x0000 > > #define INPUT_EVENT_ABS_MAX 0x7FFF > > +#define INPUT_EVENT_SLOTS_MIN 0x0 > > +#define INPUT_EVENT_SLOTS_MAX 0xa > > > > typedef struct QemuInputHandler QemuInputHandler; > > typedef struct QemuInputHandlerState QemuInputHandlerState; > > diff --git a/qapi/ui.json b/qapi/ui.json > > index 98322342f7..83369bdae8 100644 > > --- a/qapi/ui.json > > +++ b/qapi/ui.json > > @@ -1014,7 +1014,7 @@ > > ## > > { 'enum' : 'InputButton', > > 'data' : [ 'left', 'middle', 'right', 'wheel-up', 'wheel-down', 'side', > > - 'extra', 'wheel-left', 'wheel-right' ] } > > + 'extra', 'wheel-left', 'wheel-right', 'touch' ] } > > > > ## > > # @InputAxis: > > @@ -1026,6 +1026,17 @@ > > { 'enum' : 'InputAxis', > > 'data' : [ 'x', 'y' ] } > > > > +## > > +# @InputMultitouchType: > > Suggest InputMultiTouchType, because... > > > +# > > +# Type of a multitouch event. > > ... the common spelling is multi-touch. > > More of the same below. Ack, let's use this in v4. > > +# > > +# Since: 8.1 > > +## > > +{ 'enum' : 'InputMultitouchType', > > + 'data' : [ 'begin', 'update', 'end', 'cancel', 'data' ] } > > + > > + > > ## > > # @InputKeyEvent: > > # > > @@ -1069,13 +1080,32 @@ > > 'data' : { 'axis' : 'InputAxis', > > 'value' : 'int' } } > > > > +## > > +# @InputMultitouchEvent: > > +# > > +# Multitouch input event. > > +# > > +# @slot: Which slot has generated the event. > > Ignorant question: what's a "slot"? The Multi-touch protocol [1] talks about them without describing them in much detail. In my understanding, the HW has as many slots as simultaneous contact points is able to track. When a new contact is detected is assigned a particular slot, and keeps using that one until the contact is released. > > +# @tracking-id: ID to correlate this event with previously generated events. > > +# @axis: Which axis is referenced by @value. > > +# @value: Contact position. > > +# > > +# Since: 8.1 > > +## > > +{ 'struct' : 'InputMultitouchEvent', > > + 'data' : { 'type' : 'InputMultitouchType', > > + 'slot' : 'int', > > + 'tracking-id': 'int', > > + 'axis' : 'InputAxis', > > + 'value' : 'int' } } > > + > > ## > > # @InputEventKind: > > # > > # Since: 2.0 > > ## > > { 'enum': 'InputEventKind', > > - 'data': [ 'key', 'btn', 'rel', 'abs' ] } > > + 'data': [ 'key', 'btn', 'rel', 'abs', 'mtt' ] } > > While we generally avoid abbreviations in QAPI, local consistency is a > strong argument for this one. Okay. > > > > > ## > > # @InputKeyEventWrapper: > > @@ -1101,6 +1131,14 @@ > > { 'struct': 'InputMoveEventWrapper', > > 'data': { 'data': 'InputMoveEvent' } } > > > > +## > > +# @InputMultitouchEventWrapper: > > +# > > +# Since: 8.1 > > +## > > +{ 'struct': 'InputMultitouchEventWrapper', > > + 'data': { 'data': 'InputMultitouchEvent' } } > > The only reason for wrapping is consistency with the other branches. > Okay. > > > + > > ## > > # @InputEvent: > > # > > @@ -1112,6 +1150,7 @@ > # @type: the input type, one of: > # > # - 'key': Input event of Keyboard > > # - 'btn': Input event of pointer buttons > > # - 'rel': Input event of relative pointer motion > > # - 'abs': Input event of absolute pointer motion > > +# - 'mtt': Input event of Multitouch > > You're imitating the existing "Input event of" pattern, which is fair. > But the pattern is bad. The phrasing awkward, and so is the place. By > documenting the values of InputEventKind only here, and not in > InputEventKind's doc comment, the generated documentation for > InputEventKind looks like this: > > "InputEventKind" (Enum) > ----------------------- > > Values > ~~~~~~ > > "key" > Not documented > > "btn" > Not documented > > "rel" > Not documented > > "abs" > Not documented > > "mtt" > Not documented > > > Since > ~~~~~ > > 2.0 > > We should document them right in InputEventKind's doc comment, roughly > like this: > > ## > # @InputEventKind: > # > # @key: a keyboard input event > # @btn: a pointer button input event > # @rel: a relative pointer motion input event > # @abs: an absolute pointer motion input event > # @mtt: a multi-touch input event > # > # Since: 2.0 > ## > > We can then dumb down the documentation of InputEvent member @type to > just > > # @type: the type of input event > > What do you think? Yeah, this definitely looks better. Fixed in v4. Thanks, Sergio. > Many more doc comments neglect to document members in this file, and in > others. I'm not asking you to fix them all. > > > # > > # Since: 2.0 > > ## > > @@ -1121,7 +1160,8 @@ > > 'data' : { 'key' : 'InputKeyEventWrapper', > > 'btn' : 'InputBtnEventWrapper', > > 'rel' : 'InputMoveEventWrapper', > > - 'abs' : 'InputMoveEventWrapper' } } > > + 'abs' : 'InputMoveEventWrapper', > > + 'mtt' : 'InputMultitouchEventWrapper' } } > > > > ## > > # @input-send-event: > > [...] >
diff --git a/include/ui/input.h b/include/ui/input.h index c86219a1c1..2a3dffd417 100644 --- a/include/ui/input.h +++ b/include/ui/input.h @@ -8,9 +8,12 @@ #define INPUT_EVENT_MASK_BTN (1<<INPUT_EVENT_KIND_BTN) #define INPUT_EVENT_MASK_REL (1<<INPUT_EVENT_KIND_REL) #define INPUT_EVENT_MASK_ABS (1<<INPUT_EVENT_KIND_ABS) +#define INPUT_EVENT_MASK_MTT (1<<INPUT_EVENT_KIND_MTT) #define INPUT_EVENT_ABS_MIN 0x0000 #define INPUT_EVENT_ABS_MAX 0x7FFF +#define INPUT_EVENT_SLOTS_MIN 0x0 +#define INPUT_EVENT_SLOTS_MAX 0xa typedef struct QemuInputHandler QemuInputHandler; typedef struct QemuInputHandlerState QemuInputHandlerState; diff --git a/qapi/ui.json b/qapi/ui.json index 98322342f7..83369bdae8 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -1014,7 +1014,7 @@ ## { 'enum' : 'InputButton', 'data' : [ 'left', 'middle', 'right', 'wheel-up', 'wheel-down', 'side', - 'extra', 'wheel-left', 'wheel-right' ] } + 'extra', 'wheel-left', 'wheel-right', 'touch' ] } ## # @InputAxis: @@ -1026,6 +1026,17 @@ { 'enum' : 'InputAxis', 'data' : [ 'x', 'y' ] } +## +# @InputMultitouchType: +# +# Type of a multitouch event. +# +# Since: 8.1 +## +{ 'enum' : 'InputMultitouchType', + 'data' : [ 'begin', 'update', 'end', 'cancel', 'data' ] } + + ## # @InputKeyEvent: # @@ -1069,13 +1080,32 @@ 'data' : { 'axis' : 'InputAxis', 'value' : 'int' } } +## +# @InputMultitouchEvent: +# +# Multitouch input event. +# +# @slot: Which slot has generated the event. +# @tracking-id: ID to correlate this event with previously generated events. +# @axis: Which axis is referenced by @value. +# @value: Contact position. +# +# Since: 8.1 +## +{ 'struct' : 'InputMultitouchEvent', + 'data' : { 'type' : 'InputMultitouchType', + 'slot' : 'int', + 'tracking-id': 'int', + 'axis' : 'InputAxis', + 'value' : 'int' } } + ## # @InputEventKind: # # Since: 2.0 ## { 'enum': 'InputEventKind', - 'data': [ 'key', 'btn', 'rel', 'abs' ] } + 'data': [ 'key', 'btn', 'rel', 'abs', 'mtt' ] } ## # @InputKeyEventWrapper: @@ -1101,6 +1131,14 @@ { 'struct': 'InputMoveEventWrapper', 'data': { 'data': 'InputMoveEvent' } } +## +# @InputMultitouchEventWrapper: +# +# Since: 8.1 +## +{ 'struct': 'InputMultitouchEventWrapper', + 'data': { 'data': 'InputMultitouchEvent' } } + ## # @InputEvent: # @@ -1112,6 +1150,7 @@ # - 'btn': Input event of pointer buttons # - 'rel': Input event of relative pointer motion # - 'abs': Input event of absolute pointer motion +# - 'mtt': Input event of Multitouch # # Since: 2.0 ## @@ -1121,7 +1160,8 @@ 'data' : { 'key' : 'InputKeyEventWrapper', 'btn' : 'InputBtnEventWrapper', 'rel' : 'InputMoveEventWrapper', - 'abs' : 'InputMoveEventWrapper' } } + 'abs' : 'InputMoveEventWrapper', + 'mtt' : 'InputMultitouchEventWrapper' } } ## # @input-send-event: diff --git a/replay/replay-input.c b/replay/replay-input.c index 1147e3d34e..092f6b5ee9 100644 --- a/replay/replay-input.c +++ b/replay/replay-input.c @@ -22,6 +22,7 @@ void replay_save_input_event(InputEvent *evt) InputKeyEvent *key; InputBtnEvent *btn; InputMoveEvent *move; + InputMultitouchEvent *mtt; replay_put_dword(evt->type); switch (evt->type) { @@ -58,6 +59,14 @@ void replay_save_input_event(InputEvent *evt) replay_put_dword(move->axis); replay_put_qword(move->value); break; + case INPUT_EVENT_KIND_MTT: + mtt = evt->u.mtt.data; + replay_put_dword(mtt->type); + replay_put_qword(mtt->slot); + replay_put_qword(mtt->tracking_id); + replay_put_dword(mtt->axis); + replay_put_qword(mtt->value); + break; case INPUT_EVENT_KIND__MAX: /* keep gcc happy */ break; @@ -73,6 +82,7 @@ InputEvent *replay_read_input_event(void) InputBtnEvent btn; InputMoveEvent rel; InputMoveEvent abs; + InputMultitouchEvent mtt; evt.type = replay_get_dword(); switch (evt.type) { @@ -109,6 +119,14 @@ InputEvent *replay_read_input_event(void) evt.u.abs.data->axis = (InputAxis)replay_get_dword(); evt.u.abs.data->value = replay_get_qword(); break; + case INPUT_EVENT_KIND_MTT: + evt.u.mtt.data = &mtt; + evt.u.mtt.data->type = (InputMultitouchType)replay_get_dword(); + evt.u.mtt.data->slot = replay_get_qword(); + evt.u.mtt.data->tracking_id = replay_get_qword(); + evt.u.mtt.data->axis = (InputAxis)replay_get_dword(); + evt.u.mtt.data->value = replay_get_qword(); + break; case INPUT_EVENT_KIND__MAX: /* keep gcc happy */ break; diff --git a/ui/input.c b/ui/input.c index f2d1e7a3a7..f788db20f7 100644 --- a/ui/input.c +++ b/ui/input.c @@ -212,6 +212,7 @@ static void qemu_input_event_trace(QemuConsole *src, InputEvent *evt) InputKeyEvent *key; InputBtnEvent *btn; InputMoveEvent *move; + InputMultitouchEvent *mtt; if (src) { idx = qemu_console_get_index(src); @@ -250,6 +251,11 @@ static void qemu_input_event_trace(QemuConsole *src, InputEvent *evt) name = InputAxis_str(move->axis); trace_input_event_abs(idx, name, move->value); break; + case INPUT_EVENT_KIND_MTT: + mtt = evt->u.mtt.data; + name = InputAxis_str(mtt->axis); + trace_input_event_mtt(idx, name, mtt->value); + break; case INPUT_EVENT_KIND__MAX: /* keep gcc happy */ break; diff --git a/ui/trace-events b/ui/trace-events index 977577fbba..6747361745 100644 --- a/ui/trace-events +++ b/ui/trace-events @@ -90,6 +90,7 @@ input_event_key_qcode(int conidx, const char *qcode, bool down) "con %d, key qco input_event_btn(int conidx, const char *btn, bool down) "con %d, button %s, down %d" input_event_rel(int conidx, const char *axis, int value) "con %d, axis %s, value %d" input_event_abs(int conidx, const char *axis, int value) "con %d, axis %s, value 0x%x" +input_event_mtt(int conidx, const char *axis, int value) "con %d, axis %s, value 0x%x" input_event_sync(void) "" input_mouse_mode(int absolute) "absolute %d"