From 1b14ee36b3b401f69d736e813f2bc910cd8f189b Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sun, 4 Sep 2022 23:38:13 +0200 Subject: [PATCH] Kobo: Fix input on Mk. 3 (i.e., Kobo Touch A/B). (#9474) * Kobo: Discriminate between the Touch A/B and the Touch C properly, and implement actual support for the A/B input quirks. This means the clunky touchscreen probe widget shown on fresh installs on those devices is now gone :}. * Input: Fix an off-by-one in most adjustTouchMirrorX/Y callers (only rM was doing it right), and adjust their documentation to avoid similar mistakes in the future. * GestureDetector: Unify logging to always display transformed coordinates for simple gestures. * GestureDetector: Fix two-contact hold lifts to be computed at the midpoint between the two contacts, like their holds counterpart already did. --- frontend/device/cervantes/device.lua | 2 +- frontend/device/gesturedetector.lua | 46 +++++++++++++++++--------- frontend/device/input.lua | 22 ++++++++++--- frontend/device/kobo/device.lua | 49 ++++++++++++---------------- frontend/device/sdl/device.lua | 2 +- spec/unit/device_spec.lua | 21 ++++++------ 6 files changed, 82 insertions(+), 60 deletions(-) diff --git a/frontend/device/cervantes/device.lua b/frontend/device/cervantes/device.lua index 4384f7b65..82b4c1e88 100644 --- a/frontend/device/cervantes/device.lua +++ b/frontend/device/cervantes/device.lua @@ -113,7 +113,7 @@ function Cervantes:initEventAdjustHooks() if self.touch_mirrored_x then self.input:registerEventAdjustHook( self.input.adjustTouchMirrorX, - self.screen:getWidth() + (self.screen:getWidth() - 1) ) end diff --git a/frontend/device/gesturedetector.lua b/frontend/device/gesturedetector.lua index 41568f4a2..80d460d62 100644 --- a/frontend/device/gesturedetector.lua +++ b/frontend/device/gesturedetector.lua @@ -498,17 +498,18 @@ function Contact:tapState(new_tap) h = 0, } local tap_span = pos0:distance(pos1) - logger.dbg("two_finger_tap detected with span", tap_span) + local tap_pos = pos0:midpoint(pos1) + logger.dbg("two_finger_tap detected @", tap_pos.x, tap_pos.y, "with span", tap_span) -- Don't drop buddy, voidState will handle it gesture_detector:dropContact(self) return { ges = "two_finger_tap", - pos = pos0:midpoint(pos1), + pos = tap_pos, span = tap_span, time = tev.timev, } else - logger.dbg("Two-contact tap failed to pass the two_finger_tap constraints") + logger.dbg("Contact:tapState: Two-contact tap failed to pass the two_finger_tap constraints -> single tap @", tev.x, tev.y) -- We blew the gesture position/time constraints, -- neuter buddy and send a single tap on this slot. buddy_contact.state = Contact.voidState @@ -661,7 +662,7 @@ function Contact:handleNonTap(new_tap) -- NOTE: If we happened to have moved enough, holdState will generate a hold_pan on the *next* event, -- but for now, the initial hold is mandatory. -- On the other hand, if we're *already* in pan state, we stay there and *never* switch to hold. - logger.dbg("hold timer detected a hold gesture in slot", slot) + logger.dbg("hold timer tripped a switch to hold state in slot", slot) return self:switchState(Contact.holdState, true) end end @@ -1167,14 +1168,16 @@ function Contact:holdState(new_hold) h = 0, } local tap_span = pos0:distance(pos1) - logger.dbg("two_finger_hold detected with span", tap_span) + local tap_pos = pos0:midpoint(pos1) + logger.dbg("two_finger_hold detected @", tap_pos.x, tap_pos.y, "with span", tap_span) return { ges = "two_finger_hold", - pos = pos0:midpoint(pos1), + pos = tap_pos, span = tap_span, time = tev.timev, } elseif self.down then + logger.dbg("hold detected @", tev.x, tev.y) return { ges = "hold", pos = Geom:new{ @@ -1212,30 +1215,41 @@ function Contact:holdState(new_hold) elseif self.mt_gesture == "hold_pan" or self.mt_gesture == "pan" then self.mt_gesture = "hold_pan_release" buddy_contact.mt_gesture = "hold_pan_release" - logger.dbg("two_finger_hold_pan_release detected") else self.mt_gesture = "hold_release" buddy_contact.mt_gesture = "hold_release" - logger.dbg("two_finger_hold_release detected") end -- Neuter its buddy buddy_contact.state = Contact.voidState -- Don't drop buddy, voidState will handle it gesture_detector:dropContact(self) + + local pos0 = Geom:new{ + x = tev.x, + y = tev.y, + w = 0, + h = 0, + } + local pos1 = Geom:new{ + x = buddy_contact.current_tev.x, + y = buddy_contact.current_tev.y, + w = 0, + h = 0, + } + local ges_type = self.mt_gesture == "hold_pan_release" and "two_finger_hold_pan_release" or "two_finger_hold_release" + local tap_span = pos0:distance(pos1) + local tap_pos = pos0:midpoint(pos1) + logger.dbg(ges_type, "detected @", tap_pos.x, tap_pos.y, "with span", tap_span) return { - ges = self.mt_gesture == "hold_pan_release" and "two_finger_hold_pan_release" or "two_finger_hold_release", - pos = Geom:new{ - x = tev.x, - y = tev.y, - w = 0, - h = 0, - }, + ges = ges_type, + pos = tap_pos, + span = tap_span, time = tev.timev, } elseif self.down then -- Contact lift, emit a hold_release - logger.dbg("hold_release detected") + logger.dbg("hold_release detected @", tev.x, tev.y) gesture_detector:dropContact(self) return { ges = "hold_release", diff --git a/frontend/device/input.lua b/frontend/device/input.lua index c41b2c2fe..f7923653b 100644 --- a/frontend/device/input.lua +++ b/frontend/device/input.lua @@ -341,17 +341,17 @@ function Input:adjustTouchScale(ev, by) end end -function Input:adjustTouchMirrorX(ev, width) +function Input:adjustTouchMirrorX(ev, max_x) if ev.type == C.EV_ABS and (ev.code == C.ABS_X or ev.code == C.ABS_MT_POSITION_X) then - ev.value = width - ev.value + ev.value = max_x - ev.value end end -function Input:adjustTouchMirrorY(ev, height) +function Input:adjustTouchMirrorY(ev, max_y) if ev.type == C.EV_ABS and (ev.code == C.ABS_Y or ev.code == C.ABS_MT_POSITION_Y) then - ev.value = height - ev.value + ev.value = max_y - ev.value end end @@ -847,6 +847,20 @@ function Input:handleTouchEvLegacy(ev) self:setMtSlot(MTSlot.slot, "timev", time.timeval(ev.time)) end + -- On Kobo Mk. 3 devices, the frame that reports a contact lift *actually* does the coordinates transform for us... + -- Unfortunately, our own transforms are not stateful, so, just revert 'em here, + -- since we can't simply avoid not doing 'em for that frame... + -- c.f., https://github.com/koreader/koreader/issues/2128#issuecomment-1236289909 for logs on a Touch B + if self.touch_kobo_mk3_protocol then + if self:getCurrentMtSlotData("id") == -1 then + -- Technically, it's the frame where ABS_PRESSURE is set to 0 ;). + local y = 599 - self:getCurrentMtSlotData("x") -- Mk. 3 devices are all 600x800, so just hard-code it here. + local x = self:getCurrentMtSlotData("y") + self:setCurrentMtSlot("x", x) + self:setCurrentMtSlot("y", y) + end + end + -- feed ev in all slots to state machine local touch_gestures = self.gesture_detector:feedEvent(self.MTSlots) self:newFrame() diff --git a/frontend/device/kobo/device.lua b/frontend/device/kobo/device.lua index 44196b32a..3ba2a12be 100644 --- a/frontend/device/kobo/device.lua +++ b/frontend/device/kobo/device.lua @@ -122,11 +122,16 @@ local Kobo = Generic:new{ unexpected_wakeup_count = 0 } --- Kobo Touch: -local KoboTrilogy = Kobo:new{ - model = "Kobo_trilogy", - needsTouchScreenProbe = yes, - touch_switch_xy = false, +-- Kobo Touch A/B: +local KoboTrilogyAB = Kobo:new{ + model = "Kobo_trilogy_AB", + touch_kobo_mk3_protocol = true, + hasKeys = yes, + hasMultitouch = no, +} +-- Kobo Touch C: +local KoboTrilogyC = Kobo:new{ + model = "Kobo_trilogy_C", hasKeys = yes, hasMultitouch = no, } @@ -646,24 +651,7 @@ function Kobo:init() -- Input handling on Kobo is a thing of nightmares, start by setting up the actual evdev handler... self:setTouchEventHandler() -- And then handle the extra shenanigans if necessary. - if not self.needsTouchScreenProbe() then - self:initEventAdjustHooks() - else - -- If touch probe is required, we postpone EventAdjustHook to *after* it has run, - -- because some of it depends on its results... - self.touchScreenProbe = function() - -- Only run the probe once ;). - if G_reader_settings:hasNot("kobo_touch_switch_xy") then - local TouchProbe = require("tools/kobo_touch_probe") - local UIManager = require("ui/uimanager") - UIManager:show(TouchProbe:new{}) - UIManager:run() - -- If all goes well, we should now have a kobo_touch_switch_xy setting. - end - self.touch_switch_xy = G_reader_settings:readSetting("kobo_touch_switch_xy") - self:initEventAdjustHooks() - end - end + self:initEventAdjustHooks() -- See if the device supports key repeat self:getKeyRepeat() @@ -778,6 +766,9 @@ function Kobo:setTouchEventHandler() self.input.handleTouchEv = self.input.handleTouchEvPhoenix elseif not self:hasMultitouch() then self.input.handleTouchEv = self.input.handleTouchEvLegacy + if self.touch_kobo_mk3_protocol then + self.input.touch_kobo_mk3_protocol = true + end end -- Accelerometer @@ -792,7 +783,7 @@ function Kobo:setTouchEventHandler() end function Kobo:initEventAdjustHooks() - -- NOTE: On trilogy, adjustTouchSwitchXY needs to be called before adjustTouchMirrorX + -- NOTE: adjustTouchSwitchXY needs to be called before adjustTouchMirrorX if self.touch_switch_xy then self.input:registerEventAdjustHook(self.input.adjustTouchSwitchXY) end @@ -801,7 +792,7 @@ function Kobo:initEventAdjustHooks() self.input:registerEventAdjustHook( self.input.adjustTouchMirrorX, --- NOTE: This is safe, we enforce the canonical portrait rotation on startup. - self.screen:getWidth() + (self.screen:getWidth() - 1) ) end end @@ -1252,8 +1243,10 @@ elseif codename == "kraken" then return KoboKraken elseif codename == "phoenix" then return KoboPhoenix -elseif codename == "trilogy" then - return KoboTrilogy +elseif codename == "trilogy" and product_id == "310" then + return KoboTrilogyAB +elseif codename == "trilogy" and product_id == "320" then + return KoboTrilogyC elseif codename == "pixie" then return KoboPixie elseif codename == "alyssum" then @@ -1285,5 +1278,5 @@ elseif codename == "cadmus" then elseif codename == "io" then return KoboIo else - error("unrecognized Kobo model "..codename) + error("unrecognized Kobo model ".. codename .. " with device id " .. product_id) end diff --git a/frontend/device/sdl/device.lua b/frontend/device/sdl/device.lua index 369f3df88..89b197e4d 100644 --- a/frontend/device/sdl/device.lua +++ b/frontend/device/sdl/device.lua @@ -304,7 +304,7 @@ function Device:init() self.input:registerEventAdjustHook(self.input.adjustTouchSwitchXY) self.input:registerEventAdjustHook( self.input.adjustTouchMirrorX, - self.screen:getScreenWidth() + (self.screen:getScreenWidth() - 1) ) end diff --git a/spec/unit/device_spec.lua b/spec/unit/device_spec.lua index aefc1bd81..a3a4c1946 100644 --- a/spec/unit/device_spec.lua +++ b/spec/unit/device_spec.lua @@ -72,10 +72,12 @@ describe("device module", function() assert.is.same("Kobo_dahlia", kobo_dev.model) end) - it("should setup eventAdjustHooks properly for input in trilogy", function() + it("should setup eventAdjustHooks properly for input on trilogy C", function() os.getenv.invokes(function(key) if key == "PRODUCT" then return "trilogy" + elseif key == "MODEL_NUMBER" then + return "320" else return osgetenv(key) end @@ -86,10 +88,8 @@ describe("device module", function() kobo_dev:init() local Screen = kobo_dev.screen - assert.is.same("Kobo_trilogy", kobo_dev.model) - assert.truthy(kobo_dev:needsTouchScreenProbe()) - G_reader_settings:saveSetting("kobo_touch_switch_xy", true) - kobo_dev:touchScreenProbe() + assert.is.same("Kobo_trilogy_C", kobo_dev.model) + assert.falsy(kobo_dev:needsTouchScreenProbe()) local x, y = Screen:getWidth()-5, 10 -- mirror x, then switch_xy local ev_x = { @@ -101,7 +101,7 @@ describe("device module", function() local ev_y = { type = C.EV_ABS, code = C.ABS_Y, - value = Screen:getWidth()-x, + value = Screen:getWidth() - 1 - x, time = TimeVal:realtime(), } @@ -125,6 +125,8 @@ describe("device module", function() os.getenv.invokes(function(key) if key == "PRODUCT" then return "trilogy" + elseif key == "MODEL_NUMBER" then + return "320" else return osgetenv(key) end @@ -135,9 +137,8 @@ describe("device module", function() kobo_dev:init() local Screen = kobo_dev.screen - assert.is.same("Kobo_trilogy", kobo_dev.model) - assert.truthy(kobo_dev:needsTouchScreenProbe()) - kobo_dev:touchScreenProbe() + assert.is.same("Kobo_trilogy_C", kobo_dev.model) + assert.falsy(kobo_dev:needsTouchScreenProbe()) local x, y = Screen:getWidth()-5, 10 local ev_x = { type = C.EV_ABS, @@ -148,7 +149,7 @@ describe("device module", function() local ev_y = { type = C.EV_ABS, code = C.ABS_Y, - value = Screen:getWidth()-x, + value = Screen:getWidth() - 1 - x, time = {sec = 1000} }