| Message ID | 172150520664.2040.13953223569736513482-0@git.sr.ht |
|---|---|
| State | New |
| Headers | show |
| Series | [qemu] ui/gtk: Reuse input event slots for GdkEventTouch | expand |
Hi Adding Sergio in CC, who wrote that code. I don't have means to test it, which also limits my understanding and ability to check this. On Sat, Jul 20, 2024 at 11:58 PM ~katharine_chui <katharine_chui@git.sr.ht> wrote: > From: Katharine Chui <kwchuiaa@connect.ust.hk> > > There seems to be no guarantee as to how GdkEventTouch.sequence > would progress https://docs.gtk.org/gdk3/struct.EventTouch.html > > True, we also abuse the internal implementation which stores low integers in the sequence pointer. In the case of steam gamescope session, touch input would > increment the number every touch, resulting in all touch inputs > after the 10th touch to get dropped > > ... > qemu: warning: Unexpected touch slot number: 10 >= 10 > qemu: warning: Unexpected touch slot number: 11 >= 10 > qemu: warning: Unexpected touch slot number: 12 >= 10 > qemu: warning: Unexpected touch slot number: 13 >= 10 > qemu: warning: Unexpected touch slot number: 14 >= 10 > ... > > Reuse the slots on gtk to avoid that > But doing modulo like this, there is a chance of conflict with already used slots. Maybe it's time for a better gtk implementation which would handle a proper sequence pointer to slot mapping. > > Signed-off-by: Katharine Chui <kwchuiaa@connect.ust.hk> > --- > ui/gtk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ui/gtk.c b/ui/gtk.c > index bc29f7a1b4..b123c9616d 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -1090,7 +1090,7 @@ static gboolean gd_touch_event(GtkWidget *widget, > GdkEventTouch *touch, > void *opaque) > { > VirtualConsole *vc = opaque; > - uint64_t num_slot = GPOINTER_TO_UINT(touch->sequence); > + uint64_t num_slot = GPOINTER_TO_UINT(touch->sequence) % > INPUT_EVENT_SLOTS_MAX; > int type = -1; > > switch (touch->type) { > -- > 2.43.4 > >
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > Hi > > Adding Sergio in CC, who wrote that code. I don't have means to test it, > which also limits my understanding and ability to check this. > > On Sat, Jul 20, 2024 at 11:58 PM ~katharine_chui <katharine_chui@git.sr.ht> > wrote: > >> From: Katharine Chui <kwchuiaa@connect.ust.hk> >> >> There seems to be no guarantee as to how GdkEventTouch.sequence >> would progress https://docs.gtk.org/gdk3/struct.EventTouch.html >> >> > True, we also abuse the internal implementation which stores low integers > in the sequence pointer. > > In the case of steam gamescope session, touch input would >> increment the number every touch, resulting in all touch inputs >> after the 10th touch to get dropped >> >> ... >> qemu: warning: Unexpected touch slot number: 10 >= 10 >> qemu: warning: Unexpected touch slot number: 11 >= 10 >> qemu: warning: Unexpected touch slot number: 12 >= 10 >> qemu: warning: Unexpected touch slot number: 13 >= 10 >> qemu: warning: Unexpected touch slot number: 14 >= 10 >> ... >> >> Reuse the slots on gtk to avoid that >> > > But doing modulo like this, there is a chance of conflict with already used > slots. > > Maybe it's time for a better gtk implementation which would handle a proper > sequence pointer to slot mapping. The problem with slots vs. sequences is that, from what I can see, there's not way to obtain the slot number from EventTouch, which makes me thing we're a little to high in the abstraction layer to emulate multi-touch properly. And with GTK4 it seems to be even worse, because it tries harder to process gestures on its own (we need them to be processed by the guest instead). Under some compositors, we were lucky enough that indeed slots == sequences, so we could actually pass those events as that and have the guest process and recognize simple gestures (i.e. pinching) properly. The "right" solution would be finding a way to operate at a lower level than what EventTouch provides us today, but I don't know how feasible is that from within the limits of the ui/gtk3.c. In case that's not possible, the modulo workaround is probably as good as we can get. Sergio.
Hi On Mon, Jul 22, 2024 at 3:58 PM Sergio Lopez Pascual <slp@redhat.com> wrote: > Marc-André Lureau <marcandre.lureau@gmail.com> writes: > > > Hi > > > > Adding Sergio in CC, who wrote that code. I don't have means to test it, > > which also limits my understanding and ability to check this. > > > > On Sat, Jul 20, 2024 at 11:58 PM ~katharine_chui < > katharine_chui@git.sr.ht> > > wrote: > > > >> From: Katharine Chui <kwchuiaa@connect.ust.hk> > >> > >> There seems to be no guarantee as to how GdkEventTouch.sequence > >> would progress https://docs.gtk.org/gdk3/struct.EventTouch.html > >> > >> > > True, we also abuse the internal implementation which stores low integers > > in the sequence pointer. > > > > In the case of steam gamescope session, touch input would > >> increment the number every touch, resulting in all touch inputs > >> after the 10th touch to get dropped > >> > >> ... > >> qemu: warning: Unexpected touch slot number: 10 >= 10 > >> qemu: warning: Unexpected touch slot number: 11 >= 10 > >> qemu: warning: Unexpected touch slot number: 12 >= 10 > >> qemu: warning: Unexpected touch slot number: 13 >= 10 > >> qemu: warning: Unexpected touch slot number: 14 >= 10 > >> ... > >> > >> Reuse the slots on gtk to avoid that > >> > > > > But doing modulo like this, there is a chance of conflict with already > used > > slots. > > > > Maybe it's time for a better gtk implementation which would handle a > proper > > sequence pointer to slot mapping. > > The problem with slots vs. sequences is that, from what I can see, > there's not way to obtain the slot number from EventTouch, which makes > me thing we're a little to high in the abstraction layer to emulate > multi-touch properly. And with GTK4 it seems to be even worse, because > it tries harder to process gestures on its own (we need them to be > processed by the guest instead). > > Under some compositors, we were lucky enough that indeed slots == > sequences, so we could actually pass those events as that and have the > guest process and recognize simple gestures (i.e. pinching) properly. > > The "right" solution would be finding a way to operate at a lower level > than what EventTouch provides us today, but I don't know how feasible is > that from within the limits of the ui/gtk3.c. > > In case that's not possible, the modulo workaround is probably as good > as we can get. > Can't we map the sequence pointer to a (reusable) counter? So up to max-slots sequences could be mapped uniquely and we would reject events that do not fit within max-slots.
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > Hi > > On Mon, Jul 22, 2024 at 3:58 PM Sergio Lopez Pascual <slp@redhat.com> wrote: > >> Marc-André Lureau <marcandre.lureau@gmail.com> writes: >> >> > Hi >> > >> > Adding Sergio in CC, who wrote that code. I don't have means to test it, >> > which also limits my understanding and ability to check this. >> > >> > On Sat, Jul 20, 2024 at 11:58 PM ~katharine_chui < >> katharine_chui@git.sr.ht> >> > wrote: >> > >> >> From: Katharine Chui <kwchuiaa@connect.ust.hk> >> >> >> >> There seems to be no guarantee as to how GdkEventTouch.sequence >> >> would progress https://docs.gtk.org/gdk3/struct.EventTouch.html >> >> >> >> >> > True, we also abuse the internal implementation which stores low integers >> > in the sequence pointer. >> > >> > In the case of steam gamescope session, touch input would >> >> increment the number every touch, resulting in all touch inputs >> >> after the 10th touch to get dropped >> >> >> >> ... >> >> qemu: warning: Unexpected touch slot number: 10 >= 10 >> >> qemu: warning: Unexpected touch slot number: 11 >= 10 >> >> qemu: warning: Unexpected touch slot number: 12 >= 10 >> >> qemu: warning: Unexpected touch slot number: 13 >= 10 >> >> qemu: warning: Unexpected touch slot number: 14 >= 10 >> >> ... >> >> >> >> Reuse the slots on gtk to avoid that >> >> >> > >> > But doing modulo like this, there is a chance of conflict with already >> used >> > slots. >> > >> > Maybe it's time for a better gtk implementation which would handle a >> proper >> > sequence pointer to slot mapping. >> >> The problem with slots vs. sequences is that, from what I can see, >> there's not way to obtain the slot number from EventTouch, which makes >> me thing we're a little to high in the abstraction layer to emulate >> multi-touch properly. And with GTK4 it seems to be even worse, because >> it tries harder to process gestures on its own (we need them to be >> processed by the guest instead). >> >> Under some compositors, we were lucky enough that indeed slots == >> sequences, so we could actually pass those events as that and have the >> guest process and recognize simple gestures (i.e. pinching) properly. >> >> The "right" solution would be finding a way to operate at a lower level >> than what EventTouch provides us today, but I don't know how feasible is >> that from within the limits of the ui/gtk3.c. >> >> In case that's not possible, the modulo workaround is probably as good >> as we can get. >> > > Can't we map the sequence pointer to a (reusable) counter? So up to > max-slots sequences could be mapped uniquely and we would reject events > that do not fit within max-slots. A slot is a "contact" in the screen (usually a finger), and should remain the same for the events generated from that contact until it's raised from the screen. That is, for a two-fingers pinch gesture, all the events coming from one finger should have slot == X, and the ones coming from the other finger should slot == Y, where X and Y never change during the gesture. In compositors where slot == sequence, we get exactly this behavior. On compositors where sequence is always increasing, assuming it's kept the same for the duration of a contact, the modulo option can do the trick just fine, as long we don't need to support more than 10 contact points (which seems unlikely to me). Sergio.
diff --git a/ui/gtk.c b/ui/gtk.c index bc29f7a1b4..b123c9616d 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -1090,7 +1090,7 @@ static gboolean gd_touch_event(GtkWidget *widget, GdkEventTouch *touch, void *opaque) { VirtualConsole *vc = opaque; - uint64_t num_slot = GPOINTER_TO_UINT(touch->sequence); + uint64_t num_slot = GPOINTER_TO_UINT(touch->sequence) % INPUT_EVENT_SLOTS_MAX; int type = -1; switch (touch->type) {