From 841e67e0189907279d1698f86b96c31dc0cc5a1a Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Mon, 15 Aug 2022 01:13:36 +0200 Subject: [PATCH] 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 | 54 +++++++++---------- frontend/device/input.lua | 80 +++++++++++++++++------------ 2 files changed, 73 insertions(+), 61 deletions(-) diff --git a/frontend/device/gesturedetector.lua b/frontend/device/gesturedetector.lua index ad5a9511f..cbcd966c5 100644 --- a/frontend/device/gesturedetector.lua +++ b/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{ diff --git a/frontend/device/input.lua b/frontend/device/input.lua index 89aae9464..de697506a 100644 --- a/frontend/device/input.lua +++ b/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)