GestureDetector: Full refactor for almost-sane(TM) MT gesture handling (#9463)

Should hopefully make two-contact gestures *much* more reliable, among other things.

See the PR for all the details ;).

.luacheckrc

@@ -41,8 +41,6 @@ read_globals = {
     "DTAP_ZONE_BOTTOM_RIGHT",
     "DDOUBLE_TAP_ZONE_NEXT_CHAPTER",
     "DDOUBLE_TAP_ZONE_PREV_CHAPTER",
-    "DCHANGE_WEST_SWIPE_TO_EAST",
-    "DCHANGE_EAST_SWIPE_TO_WEST",
     "DKOPTREADER_CONFIG_FONT_SIZE",
     "DKOPTREADER_CONFIG_TEXT_WRAP",
     "DKOPTREADER_CONFIG_TRIM_PAGE",

base

@@ -1 +1 @@
-Subproject commit 7fb47e46054ebf0499131db8f6fc006729795f3e
+Subproject commit 78510e49888dbb723e313b79fd99121cd04ab185

defaults.lua

@@ -91,10 +91,6 @@ DTAP_ZONE_BOTTOM_RIGHT = {x = 7/8, y = 7/8, w = 1/8, h = 1/8}
 DDOUBLE_TAP_ZONE_NEXT_CHAPTER = {x = 1/4, y = 0, w = 3/4, h = 1}
 DDOUBLE_TAP_ZONE_PREV_CHAPTER = {x = 0, y = 0, w = 1/4, h = 1}
 
--- behaviour of swipes
-DCHANGE_WEST_SWIPE_TO_EAST = false
-DCHANGE_EAST_SWIPE_TO_WEST = false
-
 -- koptreader config defaults
 DKOPTREADER_CONFIG_FONT_SIZE = 1.0        -- range from 0.1 to 3.0
 DKOPTREADER_CONFIG_TEXT_WRAP = 0        -- 1 = on, 0 = off

frontend/apps/reader/modules/readerfooter.lua

@@ -134,7 +134,7 @@ local PROGRESS_BAR_STYLE_THICK_DEFAULT_HEIGHT = 7
 local PROGRESS_BAR_STYLE_THIN_DEFAULT_HEIGHT = 3
 
 -- android: guidelines for rounded corner margins
-local material_pixels = 16 * math.floor(Screen:getDPI() / 160)
+local material_pixels = Screen:scaleByDPI(16)
 
 -- functions that generates footer text for each mode
 local footerTextGeneratorMap = {

frontend/device/devicelistener.lua

@@ -242,9 +242,10 @@ if not Device:isAlwaysFullscreen() then
     end
 end
 
-function DeviceListener:onIterateRotation()
-    -- Simply rotate by 90° CW
-    local arg = bit.band(Screen:getRotationMode() + 1, 3)
+function DeviceListener:onIterateRotation(ccw)
+    -- Simply rotate by 90° CW or CCW
+    local step = ccw and -1 or 1
+    local arg = bit.band(Screen:getRotationMode() + step, 3)
     self.ui:handleEvent(Event:new("SetRotationMode", arg))
     return true
 end

frontend/device/input.lua

@@ -454,7 +454,7 @@ end
 -- Reset the gesture parsing state to a blank slate
 function Input:resetState()
     if self.gesture_detector then
-        self.gesture_detector:clearStates()
+        self.gesture_detector:dropContacts()
         -- Resets the clock source probe
         self.gesture_detector:resetClockSource()
     end
@@ -1139,7 +1139,9 @@ function Input:waitEvent(now, deadline)
                         -- Deadline hasn't been blown yet, honor it.
                         poll_timeout = poll_deadline - now
                     else
-                        -- We've already blown the deadline: make select return immediately (most likely straight to timeout)
+                        -- We've already blown the deadline: make select return immediately (most likely straight to timeout).
+                        -- NOTE: With the timerfd backend, this is sometimes a tad optimistic,
+                        --       as we may in fact retry for a few iterations while waiting for the timerfd to actually expire.
                         poll_timeout = 0
                     end
                 end
@@ -1192,19 +1194,14 @@ function Input:waitEvent(now, deadline)
                             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: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
-                            -- That leaves explicit cleanup to every other case (i.e., nil or every other gesture)
-                            if timerfd then
-                                input.clearTimer(timerfd)
-                            end
-                            table.remove(self.timer_callbacks, timer_idx)
+                        -- Cleanup after the timer callback.
+                        -- GestureDetector has guards in place to avoid double frees in case the callback itself
+                        -- affected the timerfd or timer_callbacks list (e.g., by dropping a contact).
+                        if timerfd then
+                            input.clearTimer(timerfd)
                         end
+                        table.remove(self.timer_callbacks, timer_idx)
+
                         if touch_ges then
                             self:gestureAdjustHook(touch_ges)
                             return {

frontend/device/kobo/device.lua

@@ -35,7 +35,7 @@ local function checkStandby()
     end
     local mode = f:read()
     logger.dbg("Kobo: available power states", mode)
-    if mode:find("standby") then
+    if mode and mode:find("standby") then
         logger.dbg("Kobo: standby state allowed")
         return yes
     end

frontend/dispatcher.lua

@@ -82,7 +82,8 @@ local settingsList = {
     toggle_hold_corners = {category="none", event="IgnoreHoldCorners", title=_("Toggle hold corners"), device=true, separator=true},
     toggle_rotation = {category="none", event="SwapRotation", title=_("Toggle orientation"), device=true},
     invert_rotation = {category="none", event="InvertRotation", title=_("Invert rotation"), device=true},
-    iterate_rotation = {category="none", event="IterateRotation", title=_("Rotate by 90° CW"), device=true, separator=true},
+    iterate_rotation = {category="none", event="IterateRotation", title=_("Rotate by 90° CW"), device=true},
+    iterate_rotation_ccw = {category="none", event="IterateRotation", arg=true, title=_("Rotate by 90° CCW"), device=true, separator=true},
 
     -- General
     reading_progress = {category="none", event="ShowReaderProgress", title=_("Reading progress"), general=true},
@@ -235,6 +236,7 @@ local dispatcher_menu_order = {
     "toggle_rotation",
     "invert_rotation",
     "iterate_rotation",
+    "iterate_rotation_ccw",
 
     "wifi_on",
     "wifi_off",

frontend/ui/uimanager.lua

@@ -1590,17 +1590,16 @@ function UIManager:widgetInvert(widget, x, y, w, h)
 end
 
 function UIManager:setInputTimeout(timeout)
-    self.INPUT_TIMEOUT = timeout or 200*1000
+    self.INPUT_TIMEOUT = timeout or (200*1000)
 end
 
 function UIManager:resetInputTimeout()
     self.INPUT_TIMEOUT = nil
 end
 
+-- NOTE: The Event hook mechanism used to dispatch for *every* event, and would actually pass the event along.
+--       We've simplified that to once per input frame, and without passing anything (as we, in fact, have never made use of it).
 function UIManager:handleInputEvent(input_event)
-    if input_event.handler ~= "onInputError" then
-        self.event_hook:execute("InputEvent", input_event)
-    end
     local handler = self.event_handlers[input_event]
     if handler then
         handler(input_event)
@@ -1611,6 +1610,9 @@ end
 
 -- Process all pending events on all registered ZMQs.
 function UIManager:processZMQs()
+    if self._zeromqs[1] then
+        self.event_hook:execute("InputEvent")
+    end
     for _, zeromq in ipairs(self._zeromqs) do
         for input_event in zeromq.waitEvent, zeromq do
             self:handleInputEvent(input_event)
@@ -1689,6 +1691,11 @@ function UIManager:handleInput()
 
     -- delegate each input event to handler
     if input_events then
+        -- Dispatch event hooks first, as some plugins (*cough* AutoSuspend *cough*)
+        -- rely on it to react properly to the actual event...
+        if input_events[1] then
+            self.event_hook:execute("InputEvent")
+        end
         -- Handle the full batch of events
         for __, ev in ipairs(input_events) do
             self:handleInputEvent(ev)
@@ -1853,12 +1860,12 @@ end
 function UIManager:_standbyTransition()
     if self._prevent_standby_count == 0 and self._prev_prevent_standby_count > 0 then
         -- edge prevent->allow
-        logger.dbg("allow standby")
+        logger.dbg("UIManager:_standbyTransition -> AllowStandby")
         Device:setAutoStandby(true)
         self:broadcastEvent(Event:new("AllowStandby"))
     elseif self._prevent_standby_count > 0 and self._prev_prevent_standby_count == 0 then
         -- edge allow->prevent
-        logger.dbg("prevent standby")
+        logger.dbg("UIManager:_standbyTransition -> PreventStandby")
         Device:setAutoStandby(false)
         self:broadcastEvent(Event:new("PreventStandby"))
     end

frontend/ui/widget/imageviewer.lua

@@ -624,10 +624,12 @@ function ImageViewer:onZoomIn(inc)
         self.scale_factor = self._image_wg:getScaleFactor()
     end
     if not inc then inc = 0.2 end -- default for key zoom event
-    if self.scale_factor + inc < 100 then -- avoid excessive zoom
-        self.scale_factor = self.scale_factor + inc
-        self:update()
+    self.scale_factor = self.scale_factor + inc
+    -- Avoid excessive zoom by halving the increase if we go too high, and clamp the result
+    if self.scale_factor > 100 then
+        self.scale_factor = math.min((self.scale_factor - inc) + inc/2, 100)
     end
+    self:update()
     return true
 end
 
@@ -637,10 +639,12 @@ function ImageViewer:onZoomOut(dec)
         self.scale_factor = self._image_wg:getScaleFactor()
     end
     if not dec then dec = 0.2 end -- default for key zoom event
-    if self.scale_factor - dec > 0.01 then -- avoid excessive unzoom
-        self.scale_factor = self.scale_factor - dec
-        self:update()
+    self.scale_factor = self.scale_factor - dec
+    -- Avoid excessive unzoom by halving the decrease if we go too low, and clamp the result
+    if self.scale_factor < 0.01 then
+        self.scale_factor = math.max(0.01, (self.scale_factor + dec) - dec/2)
     end
+    self:update()
     return true
 end
 

frontend/ui/widget/imagewidget.lua

@@ -31,8 +31,8 @@ local logger = require("logger")
 
 -- DPI_SCALE can't change without a restart, so let's compute it now
 local function get_dpi_scale()
-    local size_scale = math.min(Screen:getWidth(), Screen:getHeight())/600
-    local dpi_scale = Screen:getDPI() / 167
+    local size_scale = math.min(Screen:getWidth(), Screen:getHeight()) / 600
+    local dpi_scale = Screen:scaleByDPI(1)
     return math.pow(2, math.max(0, math.log((size_scale+dpi_scale)/2)/0.69))
 end
 local DPI_SCALE = get_dpi_scale()

plugins/gestures.koplugin/defaults.lua

@@ -66,6 +66,8 @@ return {
         two_finger_swipe_southwest = nil,
         spread_gesture = nil,
         pinch_gesture = nil,
+        rotate_cw = nil,
+        rotate_ccw = nil,
     },
     gesture_reader = {
         tap_top_left_corner = {toggle_page_flipping = true,},
@@ -131,6 +133,8 @@ return {
         two_finger_swipe_southwest = nil,
         spread_gesture = {increase_font = 0,},
         pinch_gesture = {decrease_font = 0,},
+        rotate_cw = nil,
+        rotate_ccw = nil,
     },
     custom_multiswipes = {},
 }

plugins/gestures.koplugin/main.lua

@@ -74,6 +74,8 @@ local gestures_list = {
     two_finger_swipe_southwest = "⇙",
     spread_gesture = _("Spread"),
     pinch_gesture = _("Pinch"),
+    rotate_cw = _("Rotate ⤸ 90°"),
+    rotate_ccw = _("Rotate ⤹ 90°"),
     multiswipe = "", -- otherwise registerGesture() won't pick up on multiswipes
     multiswipe_west_east = "⬅ ➡",
     multiswipe_east_west = "➡ ⬅",
@@ -694,6 +696,10 @@ function Gestures:addToMainMenu(menu_items)
             text = _("Spread and pinch"),
             sub_item_table = self:genSubItemTable({"spread_gesture", "pinch_gesture"}),
         })
+        table.insert(menu_items.gesture_manager.sub_item_table, {
+            text = _("Rotation"),
+            sub_item_table = self:genSubItemTable({"rotate_cw", "rotate_ccw"}),
+        })
     end
 
     self:addIntervals(menu_items)
@@ -1027,6 +1033,14 @@ function Gestures:setupGesture(ges)
     elseif ges == "pinch_gesture" then
         ges_type = "pinch"
         zone = zone_fullscreen
+    elseif ges == "rotate_cw" then
+        ges_type = "rotate"
+        zone = zone_fullscreen
+        direction = {cw = true}
+    elseif ges == "rotate_ccw" then
+        ges_type = "rotate"
+        zone = zone_fullscreen
+        direction = {ccw = true}
     else return
     end
     self:registerGesture(ges, ges_type, zone, overrides, direction, distance)

plugins/gestures.koplugin/migration.lua

@@ -188,7 +188,7 @@ function Migration:migrateGestures(caller)
             G_reader_settings:delSetting(ges_mode)
         end
     end
-    --custom multiswipes
+    -- custom multiswipes
     if custom_multiswipes_table then
         for k, v in pairs(custom_multiswipes_table) do
             local multiswipe = "multiswipe_" .. caller:safeMultiswipeName(v)

spec/unit/defaults_spec.lua

@@ -8,7 +8,7 @@ describe("defaults module", function()
 
     it("should load all defaults from defaults.lua", function()
         Defaults:init()
-        assert.is_same(101, #Defaults.defaults_name)
+        assert.is_same(99, #Defaults.defaults_name)
     end)
 
     it("should save changes to defaults.persistent.lua", function()
@@ -16,18 +16,18 @@ describe("defaults module", function()
         os.remove(persistent_filename)
 
         -- To see indices and help updating this when new settings are added:
-        -- for i=1, 101 do print(i.." ".. Defaults.defaults_name[i]) end
+        -- for i=1, 99 do print(i.." ".. Defaults.defaults_name[i]) end
 
         -- not in persistent but checked in defaults
-        Defaults.changed[20] = true
-        Defaults.changed[50] = true
-        Defaults.changed[56] = true
-        Defaults.changed[85] = true
+        Defaults.changed[18] = true
+        Defaults.changed[48] = true
+        Defaults.changed[54] = true
+        Defaults.changed[83] = true
         Defaults:saveSettings()
-        assert.is_same(101, #Defaults.defaults_name)
-        assert.is_same("DTAP_ZONE_BACKWARD", Defaults.defaults_name[86])
-        assert.is_same("DCREREADER_CONFIG_WORD_SPACING_LARGE", Defaults.defaults_name[50])
-        assert.is_same("DCREREADER_CONFIG_H_MARGIN_SIZES_XXX_LARGE", Defaults.defaults_name[20])
+        assert.is_same(99, #Defaults.defaults_name)
+        assert.is_same("DTAP_ZONE_BACKWARD", Defaults.defaults_name[84])
+        assert.is_same("DCREREADER_CONFIG_WORD_SPACING_LARGE", Defaults.defaults_name[48])
+        assert.is_same("DCREREADER_CONFIG_H_MARGIN_SIZES_XXX_LARGE", Defaults.defaults_name[18])
         dofile(persistent_filename)
         assert.is_same(DCREREADER_CONFIG_WORD_SPACING_LARGE, { [1] = 100, [2] = 90 })
         assert.is_same(DTAP_ZONE_BACKWARD, { ["y"] = 0, ["x"] = 0, ["h"] = 1, ["w"] = 0.25 })
@@ -36,15 +36,15 @@ describe("defaults module", function()
 
         -- in persistent
         Defaults:init()
-        Defaults.changed[56] = true
-        Defaults.defaults_value[56] = {
+        Defaults.changed[54] = true
+        Defaults.defaults_value[54] = {
             y = 0,
             x = 0,
             h = 0.25,
             w = 0.75
         }
-        Defaults.changed[86] = true
-        Defaults.defaults_value[86] = {
+        Defaults.changed[84] = true
+        Defaults.defaults_value[84] = {
             y = 10,
             x = 10.125,
             h = 20.25,
@@ -85,8 +85,8 @@ DHINTCOUNT = 2
 
         -- in persistent
         Defaults:init()
-        Defaults.changed[58] = true
-        Defaults.defaults_value[58] = 1
+        Defaults.changed[56] = true
+        Defaults.defaults_value[56] = 1
         Defaults:saveSettings()
         dofile(persistent_filename)
         assert.Equals(DCREREADER_VIEW_MODE, "page")

spec/unit/device_spec.lua

@@ -16,6 +16,7 @@ describe("device module", function()
                     getRotationMode = function() return 0 end,
                     getScreenMode = function() return "portrait" end,
                     setRotationMode = function() end,
+                    scaleByDPI = function(this, dp) return math.ceil(dp * this:getDPI() / 160) end,
                 }
             end
         }