From 21210800c1dd0e715a0396851d48dce764cce153 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Thu, 2 Feb 2023 00:29:23 +0100 Subject: [PATCH] ReaderFooter/Header: Refine autorefresh repaint-or-not checks (#10045) Use both a whitelist for targeted widget repaints, a blacklist for no repaint at all, and a fallback for a full in-order ReaderUI repaint when unsure. Use a similar approach in ReaderHeader (i.e., prevent explicit refreshes while ReaderMenu is open). Re #9979, re #9768 --- .../reader/modules/readercoptlistener.lua | 22 +++---- .../reader/modules/readerdevicestatus.lua | 3 +- frontend/apps/reader/modules/readerfooter.lua | 57 ++++++++++--------- frontend/apps/reader/modules/readermenu.lua | 1 + frontend/apps/reader/modules/readerstatus.lua | 3 +- frontend/ui/uimanager.lua | 41 ++++++------- frontend/ui/widget/sortwidget.lua | 3 - plugins/autodim.koplugin/main.lua | 1 + plugins/autoturn.koplugin/main.lua | 3 +- plugins/vocabbuilder.koplugin/main.lua | 3 - 10 files changed, 67 insertions(+), 70 deletions(-) diff --git a/frontend/apps/reader/modules/readercoptlistener.lua b/frontend/apps/reader/modules/readercoptlistener.lua index 2c06947cd..406ebf4a8 100644 --- a/frontend/apps/reader/modules/readercoptlistener.lua +++ b/frontend/apps/reader/modules/readercoptlistener.lua @@ -91,18 +91,18 @@ function ReaderCoptListener:onTimeFormatChanged() end function ReaderCoptListener:shouldHeaderBeRepainted() - local n = 1 - local widget = UIManager:getNthTopWidget(n) - while widget do - if widget.name == "ReaderUI" then - return true - elseif widget.covers_fullscreen then - return false - end - n = n + 1 - widget = UIManager:getNthTopWidget(n) + local top_wg = UIManager:getTopmostVisibleWidget() or {} + if top_wg.name == "ReaderUI" then + -- We're on display, go ahead + return true + elseif top_wg.covers_fullscreen or top_wg.covers_header then + -- We're hidden behind something that definitely covers us, don't do anything + return false + else + -- There's something on top of us, but we might still be visible, request a ReaderUI repaint, + -- UIManager will sort it out. + return true end - return false end function ReaderCoptListener:updateHeader() diff --git a/frontend/apps/reader/modules/readerdevicestatus.lua b/frontend/apps/reader/modules/readerdevicestatus.lua index 6f1a3f9e2..e057a30b9 100644 --- a/frontend/apps/reader/modules/readerdevicestatus.lua +++ b/frontend/apps/reader/modules/readerdevicestatus.lua @@ -70,7 +70,8 @@ function ReaderDeviceStatus:init() UIManager:close(self.memory_confirm_box) end if Device:canRestart() then - if UIManager:getNthTopWidget().name == "ReaderUI" + local top_wg = UIManager:getTopmostVisibleWidget() or {} + if top_wg.name == "ReaderUI" and G_reader_settings:isTrue("device_status_memory_auto_restart") then UIManager:show(InfoMessage:new{ text = _("High memory usage!\n\nKOReader is restarting…"), diff --git a/frontend/apps/reader/modules/readerfooter.lua b/frontend/apps/reader/modules/readerfooter.lua index 30ca36a3c..0a23a5ba5 100644 --- a/frontend/apps/reader/modules/readerfooter.lua +++ b/frontend/apps/reader/modules/readerfooter.lua @@ -771,19 +771,23 @@ function ReaderFooter:unscheduleFooterAutoRefresh() end function ReaderFooter:shouldBeRepainted() - local n = 1 - local widget = UIManager:getNthTopWidget(n) - while widget do - if widget.name == "ReaderUI" then - return true - elseif widget.covers_fullscreen or widget.covers_footer then - -- (e.g. the virtual keyboard sets widget_covers_footer == true) - return false - end - n = n + 1 - widget = UIManager:getNthTopWidget(n) + if not self.view.footer_visible then + return false + end + + local top_wg = UIManager:getTopmostVisibleWidget() or {} + if top_wg.name == "ReaderUI" then + -- No overlap possible, it's safe to request a targeted widget repaint + return true + elseif top_wg.covers_fullscreen or top_wg.covers_footer then + -- No repaint necessary at all + return false end - return false + + -- The topmost visible widget might overlap with us, but dimen isn't reliable enough to do a proper bounds check + -- (as stuff often just sets it to the Screen dimensions), + -- so request a full ReaderUI repaint to avoid out-of-order repaints. + return true, true end function ReaderFooter:rescheduleFooterAutoRefreshIfNeeded() @@ -794,7 +798,7 @@ function ReaderFooter:rescheduleFooterAutoRefreshIfNeeded() -- (We want to avoid the footer to be painted over a widget covering it - we would -- be fine refreshing it if the widget is not covering it, but this is hard to -- guess from here.) - self:onUpdateFooter(self.view.footer_visible and self:shouldBeRepainted()) + self:onUpdateFooter(self:shouldBeRepainted()) self:rescheduleFooterAutoRefreshIfNeeded() -- schedule (or not) next refresh end @@ -2102,15 +2106,15 @@ function ReaderFooter:getDataFromStatistics(title, pages) return title .. sec end -function ReaderFooter:onUpdateFooter(force_repaint, force_recompute) +function ReaderFooter:onUpdateFooter(force_repaint, full_repaint) if self.pageno then - self:updateFooterPage(force_repaint, force_recompute) + self:updateFooterPage(force_repaint, full_repaint) else - self:updateFooterPos(force_repaint, force_recompute) + self:updateFooterPos(force_repaint, full_repaint) end end -function ReaderFooter:updateFooterPage(force_repaint, force_recompute) +function ReaderFooter:updateFooterPage(force_repaint, full_repaint) if type(self.pageno) ~= "number" then return end if self.ui.document:hasHiddenFlows() then local flow = self.ui.document:getPageFlow(self.pageno) @@ -2120,24 +2124,24 @@ function ReaderFooter:updateFooterPage(force_repaint, force_recompute) else self.progress_bar.percentage = self.pageno / self.pages end - self:updateFooterText(force_repaint, force_recompute) + self:updateFooterText(force_repaint, full_repaint) end -function ReaderFooter:updateFooterPos(force_repaint, force_recompute) +function ReaderFooter:updateFooterPos(force_repaint, full_repaint) if type(self.position) ~= "number" then return end self.progress_bar.percentage = self.position / self.doc_height - self:updateFooterText(force_repaint, force_recompute) + self:updateFooterText(force_repaint, full_repaint) end -- updateFooterText will start as a noop. After onReaderReady event is -- received, it will initialized as _updateFooterText below -function ReaderFooter:updateFooterText(force_repaint, force_recompute) +function ReaderFooter:updateFooterText(force_repaint, full_repaint) end -- only call this function after document is fully loaded -function ReaderFooter:_updateFooterText(force_repaint, force_recompute) +function ReaderFooter:_updateFooterText(force_repaint, full_repaint) -- footer is invisible, we need neither a repaint nor a recompute, go away. - if not self.view.footer_visible and not force_repaint and not force_recompute then + if not self.view.footer_visible and not force_repaint and not full_repaint then return end local text = self:genFooterText() @@ -2215,7 +2219,7 @@ function ReaderFooter:_updateFooterText(force_repaint, force_recompute) refresh_dim.y = self._saved_screen_height - refresh_dim.h end -- If we're making the footer visible (or it already is), we don't need to repaint ReaderUI behind it - if self.view.footer_visible then + if self.view.footer_visible and not full_repaint then -- Unfortunately, it's not a modal (we never show() it), so it's not in the window stack, -- instead, it's baked inside ReaderUI, so it gets slightly trickier... -- NOTE: self.view.footer -> self ;). @@ -2227,6 +2231,7 @@ function ReaderFooter:_updateFooterText(force_repaint, force_recompute) return self.view.currently_scrolling and "fast" or "ui", self.footer_content.dimen end) else + -- If the footer is invisible or might be hidden behind another widget, we need to repaint the full ReaderUI stack. UIManager:setDirty(self.view.dialog, function() return self.view.currently_scrolling and "fast" or "ui", refresh_dim end) @@ -2472,12 +2477,12 @@ end -- Used by event handlers that can trip without direct UI interaction... function ReaderFooter:maybeUpdateFooter() -- ...so we need to avoid stomping over unsuspecting widgets (usually, ScreenSaver). - self:onUpdateFooter(self.view.footer_visible and self:shouldBeRepainted()) + self:onUpdateFooter(self:shouldBeRepainted()) end -- is the same as maybeUpdateFooter function ReaderFooter:onFrontlightStateChanged() - self:onUpdateFooter(self.view.footer_visible and self:shouldBeRepainted()) + self:onUpdateFooter(self:shouldBeRepainted()) end function ReaderFooter:onNetworkConnected() diff --git a/frontend/apps/reader/modules/readermenu.lua b/frontend/apps/reader/modules/readermenu.lua index db8d43fcd..4f5d15c5d 100644 --- a/frontend/apps/reader/modules/readermenu.lua +++ b/frontend/apps/reader/modules/readermenu.lua @@ -414,6 +414,7 @@ function ReaderMenu:onShowMenu(tab_index) end local menu_container = CenterContainer:new{ + covers_header = true, ignore = "height", dimen = Screen:getSize(), } diff --git a/frontend/apps/reader/modules/readerstatus.lua b/frontend/apps/reader/modules/readerstatus.lua index 7788d86f1..434de62eb 100644 --- a/frontend/apps/reader/modules/readerstatus.lua +++ b/frontend/apps/reader/modules/readerstatus.lua @@ -54,7 +54,8 @@ function ReaderStatus:onEndOfBook() self:onMarkBook(true) end - if (settings == "pop-up" or settings == nil) and UIManager:getNthTopWidget().name ~= "end_document" then + local top_wg = UIManager:getTopmostVisibleWidget() or {} + if (settings == "pop-up" or settings == nil) and top_wg.name ~= "end_document" then local buttons = { { { diff --git a/frontend/ui/uimanager.lua b/frontend/ui/uimanager.lua index a65798e44..ecc94210b 100644 --- a/frontend/ui/uimanager.lua +++ b/frontend/ui/uimanager.lua @@ -729,36 +729,29 @@ function UIManager:getNthTopWidget(n) return widget end ---[[-- -Get the *second* topmost widget, if there is one (name if possible, ref otherwise). - -Useful when VirtualKeyboard is involved, as it *always* steals the top spot ;). - -NOTE: Will skip over VirtualKeyboard instances, plural, in case there are multiple (because, apparently, we can do that.. ugh). ---]] -function UIManager:getSecondTopmostWidget() - if #self._window_stack < 2 then - -- Not enough widgets in the stack, bye! - return nil +--- Top-to-bottom widgets iterator +--- NOTE: VirtualKeyboard can be instantiated multiple times, and is a modal, +-- so don't be suprised if you find a couple of instances of it at the top ;). +function UIManager:topdown_widgets_iter() + local n = #self._window_stack + local i = n + 1 + return function() + i = i - 1 + if i > 0 then + return self._window_stack[i].widget + end end +end - -- Because everything is terrible, you can actually instantiate multiple VirtualKeyboards, - -- and they'll stack at the top, so, loop until we get something that *isn't* VK... - for i = #self._window_stack - 1, 1, -1 do +--- Get the topmost visible widget +function UIManager:getTopmostVisibleWidget() + for i = #self._window_stack, 1, -1 do local widget = self._window_stack[i].widget - - if widget.name then - if widget.name ~= "VirtualKeyboard" then - return widget.name - end - -- Meaning if name is set, and is set to VK => continue, as we want the *next* widget. - -- I *really* miss the continue keyword, Lua :/. - else + -- Skip invisible widgets (e.g., TrapWidget) + if not widget.invisible then return widget end end - - return nil end --- Check if a widget is still in the window stack, or is a subwidget of a widget still in the window stack. diff --git a/frontend/ui/widget/sortwidget.lua b/frontend/ui/widget/sortwidget.lua index 2a215a845..d2d86c19b 100644 --- a/frontend/ui/widget/sortwidget.lua +++ b/frontend/ui/widget/sortwidget.lua @@ -146,9 +146,6 @@ function SortWidget:init() w = self.width or Screen:getWidth(), h = self.height or Screen:getHeight(), } - if self.dimen.h == Screen:getHeight() then - self.covers_footer = true - end if Device:hasKeys() then self.key_events.Close = { { Device.input.group.Back } } diff --git a/plugins/autodim.koplugin/main.lua b/plugins/autodim.koplugin/main.lua index d52baa181..b603f5950 100644 --- a/plugins/autodim.koplugin/main.lua +++ b/plugins/autodim.koplugin/main.lua @@ -290,6 +290,7 @@ function AutoDim:autodim_task() end self.trap_widget = TrapWidget:new{ + name = "AutoDim", dismiss_callback = function() self:restoreFrontlight() self.trap_widget = nil diff --git a/plugins/autoturn.koplugin/main.lua b/plugins/autoturn.koplugin/main.lua index 1184968ef..5e97fdfc5 100644 --- a/plugins/autoturn.koplugin/main.lua +++ b/plugins/autoturn.koplugin/main.lua @@ -32,7 +32,8 @@ function AutoTurn:_schedule() local delay = self.last_action_time + time.s(self.autoturn_sec) - UIManager:getTime() if delay <= 0 then - if UIManager:getNthTopWidget().name == "ReaderUI" then + local top_wg = UIManager:getTopmostVisibleWidget() or {} + if top_wg.name == "ReaderUI" then logger.dbg("AutoTurn: go to next page") self.ui:handleEvent(Event:new("GotoViewRel", self.autoturn_distance)) self.last_action_time = UIManager:getTime() diff --git a/plugins/vocabbuilder.koplugin/main.lua b/plugins/vocabbuilder.koplugin/main.lua index 96381a26e..aabfc689c 100644 --- a/plugins/vocabbuilder.koplugin/main.lua +++ b/plugins/vocabbuilder.koplugin/main.lua @@ -1191,9 +1191,6 @@ function VocabularyBuilderWidget:init() w = self.width or Screen:getWidth(), h = self.height or Screen:getHeight(), } - if self.dimen.h == Screen:getHeight() then - self.covers_footer = true - end if Device:hasKeys() then self.key_events.Close = { { Device.input.group.Back } }