Input: Make sure GestureDetector:feedEvent will actually consume all

slots

This ensures we won't leave *any* slot in an undefined state because
we skipped parsing 'em because what we consumed first yielded a
gesture.

(In particular, this could leave a few slots dangling in the "hold"
state in corner cases involving spider-hand finger tapping ;p).

Cleans up the slot state clearing in GestureDetector to only clear the
necessary slots (e.g., two-finger gestures now only clear their own two
slots; and holds only clear their own slot).
The fact that every slot will be consumed ensures that every slot will
naturally get their contact up handled, which wasn't the case before,
hence those crappy workarounds.

As far as timerfd callbacks are concerned, this *does* introduce the
possibility of deadline collisions, so, do reimplement minimal safety
checks to ensure we run (and free) the right timerfd callback.

frontend/device/gesturedetector.lua

@@ -126,31 +126,29 @@ end
 --[[--
 Feeds touch events to state machine.
 
-For drivers that bundle multiple slots in the same input frame,
-events are consumed in LIFO order, because of table.remove ;).
-Note that, in a single input frame, if the same slot gets multiple events,
-only the last one is kept.
+Note that, in a single input frame, if the same slot gets multiple events, only the last one is kept.
+Every slot in the input frame is consumed, and that in FIFO order (slot order based on appearance in the frame).
 --]]
 function GestureDetector:feedEvent(tevs)
-    repeat
-        local tev = table.remove(tevs)
-        if tev then
-            local slot = tev.slot
-            if not self.states[slot] then
-                self:clearState(slot) -- initiate state
-            end
-            local ges = self.states[slot](self, tev)
-            if tev.id ~= -1 then
-                -- NOTE: tev is actually a simple reference to Input's self.ev_slots[slot],
-                --       which means self.last_tevs[slot] doesn't actually point to the *previous*
-                --       input frame for a given slot, but always points to the *current* input frame for that slot!
-                --       Compare to self.first_tevs below, which does create a copy...
-                self.last_tevs[slot] = tev
-            end
-            -- return no more than one gesture
-            if ges then return ges end
+    local gestures = {}
+    for _, tev in ipairs(tevs) do
+        local slot = tev.slot
+        if not self.states[slot] then
+            self:clearState(slot) -- initialize slot state
+        end
+        local ges = self.states[slot](self, tev)
+        if tev.id ~= -1 then
+            -- NOTE: tev is actually a simple reference to Input's self.ev_slots[slot],
+            --       which means self.last_tevs[slot] doesn't actually point to the *previous*
+            --       input frame for a given slot, but always points to the *current* input frame for that slot!
+            --       Compare to self.first_tevs below, which does create a copy...
+            self.last_tevs[slot] = tev
         end
-    until tev == nil
+        if ges then
+            table.insert(gestures, ges)
+        end
+    end
+    return gestures
 end
 
 function GestureDetector:deepCopyEv(tev)
@@ -420,7 +418,8 @@ function GestureDetector:tapState(tev)
                 }
                 local tap_span = pos0:distance(pos1)
                 logger.dbg("two-finger tap detected with span", tap_span)
-                self:clearStates()
+                self:clearState(s1)
+                self:clearState(s2)
                 return {
                     ges = "two_finger_tap",
                     pos = pos0:midpoint(pos1),
@@ -577,7 +576,8 @@ function GestureDetector:panState(tev)
             local s2 = self.input.main_finger_slot + 1
             if self.detectings[s1] and self.detectings[s2] then
                 local ges_ev = self:handleTwoFingerPan(tev)
-                self:clearStates()
+                self:clearState(s1)
+                self:clearState(s2)
                 if ges_ev then
                     if ges_ev.ges == "two_finger_pan" then
                         ges_ev.ges = "two_finger_swipe"
@@ -812,7 +812,8 @@ function GestureDetector:handlePanRelease(tev)
     if self.detectings[s1] and self.detectings[s2] then
         logger.dbg("two finger pan release detected")
         pan_ev.ges = "two_finger_pan_release"
-        self:clearStates()
+        self:clearState(s1)
+        self:clearState(s2)
     else
         logger.dbg("pan release detected in slot", slot)
         self:clearState(slot)
@@ -840,8 +841,7 @@ function GestureDetector:holdState(tev, hold)
         logger.dbg("hold_release detected in slot", slot)
         local last_x = self.last_tevs[slot].x
         local last_y = self.last_tevs[slot].y
-        -- NOTE: Don't leave multiple slots "stuck" in hold state, as we've cleared their timeouts in the main input loop anyway.
-        self:clearStates()
+        self:clearState(slot)
         return {
             ges = "hold_release",
             pos = Geom:new{

frontend/device/input.lua

@@ -742,14 +742,14 @@ function Input:handleTouchEv(ev)
                 self:setMtSlot(MTSlot.slot, "timev", time.timeval(ev.time))
             end
             -- feed ev in all slots to state machine
-            local touch_ges = self.gesture_detector:feedEvent(self.MTSlots)
+            local touch_gestures = self.gesture_detector:feedEvent(self.MTSlots)
             self:newFrame()
-            if touch_ges then
+            local ges_evs = {}
+            for _, touch_ges in ipairs(touch_gestures) do
                 self:gestureAdjustHook(touch_ges)
-                return Event:new("Gesture",
-                    self.gesture_detector:adjustGesCoordinate(touch_ges)
-                )
+                table.insert(ges_evs, Event:new("Gesture", self.gesture_detector:adjustGesCoordinate(touch_ges)))
             end
+            return ges_evs
         end
     end
 end
@@ -801,14 +801,14 @@ function Input:handleTouchEvPhoenix(ev)
                 self:setMtSlot(MTSlot.slot, "timev", time.timeval(ev.time))
             end
             -- feed ev in all slots to state machine
-            local touch_ges = self.gesture_detector:feedEvent(self.MTSlots)
+            local touch_gestures = self.gesture_detector:feedEvent(self.MTSlots)
             self:newFrame()
-            if touch_ges then
+            local ges_evs = {}
+            for _, touch_ges in ipairs(touch_gestures) do
                 self:gestureAdjustHook(touch_ges)
-                return Event:new("Gesture",
-                    self.gesture_detector:adjustGesCoordinate(touch_ges)
-                )
+                table.insert(ges_evs, Event:new("Gesture", self.gesture_detector:adjustGesCoordinate(touch_ges)))
             end
+            return ges_evs
         end
     end
 end
@@ -837,14 +837,14 @@ function Input:handleTouchEvLegacy(ev)
             end
 
             -- feed ev in all slots to state machine
-            local touch_ges = self.gesture_detector:feedEvent(self.MTSlots)
+            local touch_gestures = self.gesture_detector:feedEvent(self.MTSlots)
             self:newFrame()
-            if touch_ges then
+            local ges_evs = {}
+            for _, touch_ges in ipairs(touch_gestures) do
                 self:gestureAdjustHook(touch_ges)
-                return Event:new("Gesture",
-                    self.gesture_detector:adjustGesCoordinate(touch_ges)
-                )
+                table.insert(ges_evs, Event:new("Gesture", self.gesture_detector:adjustGesCoordinate(touch_ges)))
             end
+            return ges_evs
         end
     end
 end
@@ -1162,17 +1162,30 @@ function Input:waitEvent(now, deadline)
                     end
 
                     if consume_callback then
-                        local touch_ges = self.timer_callbacks[1].callback()
-                        -- If it was a timerfd, we *may* also need to close the fd.
-                        -- NOTE: The fact that deadlines are sorted *should* ensure that the timerfd that expired
-                        --       is actually at the head of the list, without us having to double-check that...
-                        --       In the rare case we would get a storm of events, we rely on the fact that each event frame
-                        --       *should* have a slightly different timestamp to make sure we never have a deadline collision,
-                        --       combined with the fact that GestureDetector:feedEvent will only ever return a *single* gesture
-                        --       per input frame, even if a frame contains multiple slots on the same timestamp.
-                        -- NOTE: GestureDetector only calls Input:setTimeout for "hold" & "double_tap" gestures.
+                        local touch_ges
+                        local timer_idx = 1
+                        if timerfd then
+                            -- If there's a deadline collision, make sure we call the callback that matches the timerfd returned.
+                            -- We'll handle the next one on the next iteration, as an expired timerfd will ensure
+                            -- that select will return immediately.
+                            for i, item in ipairs(self.timer_callbacks) do
+                                if item.timerfd == timerfd then
+                                    -- In the vast majority of cases, we should find our match on the first entry ;).
+                                    timer_idx = i
+                                    touch_ges = item.callback()
+                                    break
+                                end
+                            end
+                        else
+                            -- If there's a deadline collision, we'll just handle the next one on the next iteration,
+                            -- because the blown deadline means we'll have asked waitForEvent to return immediately.
+                            touch_ges = self.timer_callbacks[1].callback()
+                        end
+
+                        -- NOTE: If it was a timerfd, we *may* also need to close the fd.
+                        --       GestureDetector only calls Input:setTimeout for "hold" & "double_tap" gestures.
                         --       For double taps, the callback itself doesn't interact with the timer_callbacks list,
-                        --       but for holds, it *will* call GestureDetector:clearStates on "hold_release" (and *only* then),
+                        --       but for holds, it *will* call GestureDetector:clearState on "hold_release" (and *only* then),
                         --       and *that* already takes care of pop'ping the (hold) timer and closing the fd,
                         --       via Input:clearTimeout(slot, "hold")...
                         if not touch_ges or touch_ges.ges ~= "hold_release" then
@@ -1180,13 +1193,9 @@ function Input:waitEvent(now, deadline)
                             if timerfd then
                                 input.clearTimer(timerfd)
                             end
-                            table.remove(self.timer_callbacks, 1)
+                            table.remove(self.timer_callbacks, timer_idx)
                         end
                         if touch_ges then
-                            -- The timers we'll encounter are for finalizing a hold or (if enabled) double tap gesture,
-                            -- as such, it makes no sense to try to detect *multiple* subsequent gestures.
-                            -- This is why we clear the full list of timers on the first match ;).
-                            self:clearTimeouts()
                             self:gestureAdjustHook(touch_ges)
                             return {
                                 Event:new("Gesture", self.gesture_detector:adjustGesCoordinate(touch_ges))
@@ -1303,10 +1312,13 @@ function Input:waitEvent(now, deadline)
                     table.insert(handled, handled_ev)
                 end
             elseif event.type == C.EV_ABS or event.type == C.EV_SYN then
-                local handled_ev = self:handleTouchEv(event)
-                -- We don't generate an Event for *every* input event, so, make sure we don't push nil values to the array
-                if handled_ev then
-                    table.insert(handled, handled_ev)
+                local handled_evs = self:handleTouchEv(event)
+                -- handleTouchEv only returns an array of Events once it gets a SYN_REPORT,
+                -- so more often than not, we just get a nil here ;).
+                if handled_evs then
+                    for _, handled_ev in ipairs(handled_evs) do
+                        table.insert(handled, handled_ev)
+                    end
                 end
             elseif event.type == C.EV_MSC then
                 local handled_ev = self:handleMiscEv(event)