From 03e9fac15617e63f99dab71fd1ac0d36acf2e148 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sat, 3 Apr 2021 01:48:35 +0200 Subject: [PATCH] Input: Process input events in batches (#7483) Requires https://github.com/koreader/koreader-base/pull/1344 & https://github.com/koreader/koreader-base/pull/1346 (fix #7485) Assorted input fixes: * Actually handle errors in the "there's a callback timer" input polling loop. * Don't break timerfd when the clock probe was inconclusive. Not directly related, but noticed because of duplicate onInputEvent handlers: * HookContainer: Fix deregistration to actually deregister properly. "Regression" extant since its inception in #2933 (!). * Made sure the three plugins (basically the trio of AutoThingies ;p) that were using HookContainer actually unschedule their task on teardown. --- base | 2 +- frontend/device/input.lua | 140 ++++++++++++++++---------- frontend/device/kobo/device.lua | 3 +- frontend/pluginloader.lua | 8 +- frontend/ui/hook_container.lua | 8 +- frontend/ui/uimanager.lua | 17 ++-- plugins/autostandby.koplugin/main.lua | 5 + plugins/autosuspend.koplugin/main.lua | 8 ++ plugins/autoturn.koplugin/main.lua | 5 + spec/unit/device_spec.lua | 16 +-- 10 files changed, 134 insertions(+), 78 deletions(-) diff --git a/base b/base index 528aa30b4..b3337e535 160000 --- a/base +++ b/base @@ -1 +1 @@ -Subproject commit 528aa30b42b61596e1fbe4bdf553621a67efb7d5 +Subproject commit b3337e5352e7af58759031f218232cbcff0afa84 diff --git a/frontend/device/input.lua b/frontend/device/input.lua index b0b12cf70..e1b9bedaf 100644 --- a/frontend/device/input.lua +++ b/frontend/device/input.lua @@ -361,6 +361,7 @@ function Input:setTimeout(slot, ges, cb, origin, delay) -- If GestureDetector's clock source probing was inconclusive, do this on the UI timescale instead. if clock_id == -1 then deadline = TimeVal:now() + delay + clock_id = C.CLOCK_MONOTONIC else deadline = origin + delay end @@ -934,15 +935,16 @@ end function Input:waitEvent(now, deadline) -- On the first iteration of the loop, we don't need to update now, we're following closely (a couple ms at most) behind UIManager. local ok, ev - -- Wrapper around the platform-specific input.waitForEvent (which itself is generally poll-like). + -- Wrapper around the platform-specific input.waitForEvent (which itself is generally poll-like, and supposed to poll *once*). -- Speaking of input.waitForEvent, it can return: - -- * true, ev: When an input event was read. ev is a table mapped after the input_event struct. + -- * true, ev: When a batch of input events was read. + -- ev is an array of event tables, themselves mapped after the input_event struct. -- * false, errno, timerfd: When no input event was read, possibly for benign reasons. - -- One such common case is after a polling timeout, in which case errno is C.ETIME. - -- If the timerfd backend is in use, and the early return was caused by a timerfd expiring, - -- it returns false, C.ETIME, timerfd; where timerfd is a C pointer (i.e., light userdata) - -- to the timerfd node that expired (so as to be able to free it later, c.f., input/timerfd-callbacks.h). - -- Otherwise, errno is the actual error code from the backend (e.g., select's errno for the C backend). + -- One such common case is after a polling timeout, in which case errno is C.ETIME. + -- If the timerfd backend is in use, and the early return was caused by a timerfd expiring, + -- it returns false, C.ETIME, timerfd; where timerfd is a C pointer (i.e., light userdata) + -- to the timerfd node that expired (so as to be able to free it later, c.f., input/timerfd-callbacks.h). + -- Otherwise, errno is the actual error code from the backend (e.g., select's errno for the C backend). -- * nil: When something terrible happened (e.g., fatal poll/read failure). We abort in such cases. while true do if #self.timer_callbacks > 0 then @@ -1028,14 +1030,17 @@ function Input:waitEvent(now, deadline) -- 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) - ) + return { + Event:new("Gesture", self.gesture_detector:adjustGesCoordinate(touch_ges)) + } end -- if touch_ges end -- if poll_deadline reached + else + -- Something went wrong, jump to error handling *now* + break end -- if poll returned ETIME - -- Refresh now on the next iteration (e.g., when we have multiple timers to check) + -- Refresh now on the next iteration (e.g., when we have multiple timers to check, and we've just timed out) now = nil end -- while #timer_callbacks > 0 else @@ -1071,7 +1076,7 @@ function Input:waitEvent(now, deadline) -- Retry on EINTR else -- Warn, report, and go back to UIManager - logger.warn("Polling for input events returned an error:", ev) + logger.warn("Polling for input events returned an error:", ev, "->", ffi.string(C.strerror(ev))) break end elseif ok == nil then @@ -1087,55 +1092,80 @@ function Input:waitEvent(now, deadline) end if ok and ev then - if DEBUG.is_on then - DEBUG:logEv(ev) - if ev.type == C.EV_KEY then - logger.dbg(string.format( - "key event => code: %d (%s), value: %s, time: %d.%d", - ev.code, self.event_map[ev.code] or linux_evdev_key_code_map[ev.code], ev.value, - ev.time.sec, ev.time.usec)) - elseif ev.type == C.EV_SYN then - logger.dbg(string.format( - "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d", - ev.type, linux_evdev_type_map[ev.type], ev.code, linux_evdev_syn_code_map[ev.code], ev.value, - ev.time.sec, ev.time.usec)) - elseif ev.type == C.EV_ABS then - logger.dbg(string.format( - "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d", - ev.type, linux_evdev_type_map[ev.type], ev.code, linux_evdev_abs_code_map[ev.code], ev.value, - ev.time.sec, ev.time.usec)) - elseif ev.type == C.EV_MSC then - logger.dbg(string.format( - "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d", - ev.type, linux_evdev_type_map[ev.type], ev.code, linux_evdev_msc_code_map[ev.code], ev.value, - ev.time.sec, ev.time.usec)) + local handled = {} + -- We're guaranteed that ev is an array of event tables. Might be an array of *one* event, but an array nonetheless ;). + for __, event in ipairs(ev) do + if DEBUG.is_on then + DEBUG:logEv(event) + if event.type == C.EV_KEY then + logger.dbg(string.format( + "key event => code: %d (%s), value: %s, time: %d.%d", + event.code, self.event_map[event.code] or linux_evdev_key_code_map[event.code], event.value, + event.time.sec, event.time.usec)) + elseif event.type == C.EV_SYN then + logger.dbg(string.format( + "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d", + event.type, linux_evdev_type_map[event.type], event.code, linux_evdev_syn_code_map[event.code], event.value, + event.time.sec, event.time.usec)) + elseif event.type == C.EV_ABS then + logger.dbg(string.format( + "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d", + event.type, linux_evdev_type_map[event.type], event.code, linux_evdev_abs_code_map[event.code], event.value, + event.time.sec, event.time.usec)) + elseif event.type == C.EV_MSC then + logger.dbg(string.format( + "input event => type: %d (%s), code: %d (%s), value: %s, time: %d.%d", + event.type, linux_evdev_type_map[event.type], event.code, linux_evdev_msc_code_map[event.code], event.value, + event.time.sec, event.time.usec)) + else + logger.dbg(string.format( + "input event => type: %d (%s), code: %d, value: %s, time: %d.%d", + event.type, linux_evdev_type_map[event.type], event.code, event.value, + event.time.sec, event.time.usec)) + end + end + self:eventAdjustHook(event) + if event.type == C.EV_KEY then + local handled_ev = self:handleKeyBoardEv(event) + if handled_ev then + table.insert(handled, handled_ev) + end + elseif event.type == C.EV_ABS and event.code == ABS_OASIS_ORIENTATION then + local handled_ev = self:handleOasisOrientationEv(event) + if handled_ev then + 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 gnerate 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) + end + elseif event.type == C.EV_MSC then + local handled_ev = self:handleMiscEv(event) + if handled_ev then + table.insert(handled, handled_ev) + end + elseif event.type == C.EV_SDL then + local handled_ev = self:handleSdlEv(event) + if handled_ev then + table.insert(handled, handled_ev) + end else - logger.dbg(string.format( - "input event => type: %d (%s), code: %d, value: %s, time: %d.%d", - ev.type, linux_evdev_type_map[ev.type], ev.code, ev.value, - ev.time.sec, ev.time.usec)) + -- Received some other kind of event that we do not know how to specifically handle yet + table.insert(handled, Event:new("GenericInput", event)) end end - self:eventAdjustHook(ev) - if ev.type == C.EV_KEY then - return self:handleKeyBoardEv(ev) - elseif ev.type == C.EV_ABS and ev.code == ABS_OASIS_ORIENTATION then - return self:handleOasisOrientationEv(ev) - elseif ev.type == C.EV_ABS or ev.type == C.EV_SYN then - return self:handleTouchEv(ev) - elseif ev.type == C.EV_MSC then - return self:handleMiscEv(ev) - elseif ev.type == C.EV_SDL then - return self:handleSdlEv(ev) - else - -- Received some other kind of event that we do not know how to specifically handle yet - return Event:new("GenericInput", ev) - end + return handled elseif ok == false and ev then - return Event:new("InputError", ev) + return { + Event:new("InputError", ev) + } elseif ok == nil then -- No ok and no ev? Hu oh... - return Event:new("InputError", "Catastrophic") + return { + Event:new("InputError", "Catastrophic") + } end end diff --git a/frontend/device/kobo/device.lua b/frontend/device/kobo/device.lua index 3732c212c..d74321027 100644 --- a/frontend/device/kobo/device.lua +++ b/frontend/device/kobo/device.lua @@ -94,7 +94,8 @@ local KoboDahlia = Kobo:new{ model = "Kobo_dahlia", hasFrontlight = yes, touch_phoenix_protocol = true, - -- There's no slot 0, the first finger gets assigned slot 1, and the second slot 2 + -- There's no slot 0, the first finger gets assigned slot 1, and the second slot 2. + -- NOTE: Could be queried at runtime via EVIOCGABS on ABS_MT_TRACKING_ID (minimum field). main_finger_slot = 1, display_dpi = 265, -- the bezel covers the top 11 pixels: diff --git a/frontend/pluginloader.lua b/frontend/pluginloader.lua index 651eb7a67..9ace7539f 100644 --- a/frontend/pluginloader.lua +++ b/frontend/pluginloader.lua @@ -84,8 +84,8 @@ function PluginLoader:loadPlugins() for element in pairs(OBSOLETE_PLUGINS) do plugins_disabled[element] = true end - for _,lookup_path in ipairs(lookup_path_list) do - logger.info('Loading plugins from directory:', lookup_path) + for _, lookup_path in ipairs(lookup_path_list) do + logger.info("Loading plugins from directory:", lookup_path) for entry in lfs.dir(lookup_path) do local plugin_root = lookup_path.."/"..entry local mode = lfs.attributes(plugin_root, "mode") @@ -124,7 +124,7 @@ function PluginLoader:loadPlugins() end -- set package path for all loaded plugins - for _,plugin in ipairs(self.enabled_plugins) do + for _, plugin in ipairs(self.enabled_plugins) do package.path = string.format("%s;%s/?.lua", package.path, plugin.path) package.cpath = string.format("%s;%s/lib/?.so", package.cpath, plugin.path) end @@ -193,7 +193,7 @@ function PluginLoader:createPluginInstance(plugin, attr) if ok then -- re is a plugin instance return ok, re else -- re is the error message - logger.err('Failed to initialize', plugin.name, 'plugin: ', re) + logger.err("Failed to initialize", plugin.name, "plugin: ", re) return nil, re end end diff --git a/frontend/ui/hook_container.lua b/frontend/ui/hook_container.lua index bc74b9b79..1ec3e019b 100644 --- a/frontend/ui/hook_container.lua +++ b/frontend/ui/hook_container.lua @@ -49,16 +49,18 @@ end function HookContainer:registerWidget(name, widget) self:_assertIsValidName(name) assert(type(widget) == "table") - self:register(name, function(args) + -- *That* is the function we actually register and need to unregister later, so keep a ref to it... + local hook_func = function(args) local f = widget["on" .. name] self:_assertIsValidFunction(f) f(widget, args) - end) + end + self:register(name, hook_func) local original_close_widget = widget.onCloseWidget self:_assertIsValidFunctionOrNil(original_close_widget) widget.onCloseWidget = function() if original_close_widget then original_close_widget(widget) end - self:unregister(name, widget["on" .. name]) + self:unregister(name, hook_func) end end diff --git a/frontend/ui/uimanager.lua b/frontend/ui/uimanager.lua index 9b4304ca9..81de4f1d5 100644 --- a/frontend/ui/uimanager.lua +++ b/frontend/ui/uimanager.lua @@ -1549,7 +1549,7 @@ end -- Process all pending events on all registered ZMQs. function UIManager:processZMQs() for _, zeromq in ipairs(self._zeromqs) do - for input_event in zeromq.waitEvent,zeromq do + for input_event in zeromq.waitEvent, zeromq do self:handleInputEvent(input_event) end end @@ -1611,12 +1611,15 @@ function UIManager:handleInput() -- Anywhere else breaks preventStandby/allowStandby invariants used by background jobs while UI is left running. self:_standbyTransition() - -- wait for next event - local input_event = Input:waitEvent(now, deadline) + -- wait for next batch of events + local input_events = Input:waitEvent(now, deadline) - -- delegate input_event to handler - if input_event then - self:handleInputEvent(input_event) + -- delegate each input event to handler + if input_events then + -- Handle the full batch of events + for __, ev in ipairs(input_events) do + self:handleInputEvent(ev) + end end if self.looper then @@ -1637,7 +1640,7 @@ end function UIManager:onRotation() - self:setDirty('all', 'full') + self:setDirty("all", "full") self:forceRePaint() end diff --git a/plugins/autostandby.koplugin/main.lua b/plugins/autostandby.koplugin/main.lua index 0d2e50a8a..b90cb462a 100644 --- a/plugins/autostandby.koplugin/main.lua +++ b/plugins/autostandby.koplugin/main.lua @@ -44,6 +44,11 @@ function AutoStandby:init() self.ui.menu:registerToMainMenu(self) end +function AutoStandby:onCloseWidget() + logger.dbg("AutoStandby:onCloseWidget() instance=", tostring(self)) + UIManager:unschedule(AutoStandby.allow) +end + function AutoStandby:addToMainMenu(menu_items) menu_items.autostandby = { sorting_hint = "device", diff --git a/plugins/autosuspend.koplugin/main.lua b/plugins/autosuspend.koplugin/main.lua index be1dd4ba1..87bec3d5b 100644 --- a/plugins/autosuspend.koplugin/main.lua +++ b/plugins/autosuspend.koplugin/main.lua @@ -100,6 +100,7 @@ function AutoSuspend:_restart() end function AutoSuspend:init() + logger.dbg("AutoSuspend: init") if Device:isPocketBook() and not Device:canSuspend() then return end UIManager.event_hook:registerWidget("InputEvent", self) self:_unschedule() @@ -109,6 +110,13 @@ function AutoSuspend:init() self.ui.menu:registerToMainMenu(self) end +-- For event_hook automagic deregistration purposes +function AutoSuspend:onCloseWidget() + logger.dbg("AutoSuspend: onCloseWidget") + if Device:isPocketBook() and not Device:canSuspend() then return end + self:_unschedule() +end + function AutoSuspend:onInputEvent() logger.dbg("AutoSuspend: onInputEvent") self.last_action_tv = UIManager:getTime() diff --git a/plugins/autoturn.koplugin/main.lua b/plugins/autoturn.koplugin/main.lua index ccf1fd679..2e7431606 100644 --- a/plugins/autoturn.koplugin/main.lua +++ b/plugins/autoturn.koplugin/main.lua @@ -94,6 +94,11 @@ function AutoTurn:init() self:_start() end +function AutoTurn:onCloseWidget() + logger.dbg("AutoTurn: onCloseWidget") + self:_deprecateLastTask() +end + function AutoTurn:onCloseDocument() logger.dbg("AutoTurn: onCloseDocument") self:_deprecateLastTask() diff --git a/spec/unit/device_spec.lua b/spec/unit/device_spec.lua index 4e7562bf4..191e47493 100644 --- a/spec/unit/device_spec.lua +++ b/spec/unit/device_spec.lua @@ -274,13 +274,15 @@ describe("device module", function() mock_ffi_input = require('ffi/input') stub(mock_ffi_input, "waitForEvent") mock_ffi_input.waitForEvent.returns(true, { - type = 3, - time = { - usec = 450565, - sec = 1471081881 - }, - code = 24, - value = 16 + { + type = 3, + time = { + usec = 450565, + sec = 1471081881 + }, + code = 24, + value = 16 + } }) local UIManager = require("ui/uimanager")