On Mon, Apr 17, 2023 at 12:57:08PM +0200, Markus Armbruster wrote:
> Sergio Lopez <s...@redhat.com> writes:
>
> > Add the required infrastructure to support generating multitouch events.
> >
> > Signed-off-by: Sergio Lopez <s...@redhat.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lur...@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:
>
> [...]
>
signature.asc
Description: PGP signature
