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

frontend/apps/reader/modules/readercoptlistener.lua

@@ -91,18 +91,18 @@ function ReaderCoptListener:onTimeFormatChanged()
 end
 
 function ReaderCoptListener:shouldHeaderBeRepainted()
-    local n = 1
+    local top_wg = UIManager:getTopmostVisibleWidget() or {}
-    local widget = UIManager:getNthTopWidget(n)
+    if top_wg.name == "ReaderUI" then
-    while widget do
+        -- We're on display, go ahead
-        if widget.name == "ReaderUI"  then
+        return true
-            return true
+    elseif top_wg.covers_fullscreen or top_wg.covers_header then
-        elseif widget.covers_fullscreen then
+        -- We're hidden behind something that definitely covers us, don't do anything
-            return false
+        return false
-        end
+    else
-        n = n + 1
+        -- There's something on top of us, but we might still be visible, request a ReaderUI repaint,
-        widget = UIManager:getNthTopWidget(n)
+        -- UIManager will sort it out.
+        return true
     end
-    return false
 end
 
 function ReaderCoptListener:updateHeader()

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…"),

frontend/apps/reader/modules/readerfooter.lua

@@ -771,19 +771,23 @@ function ReaderFooter:unscheduleFooterAutoRefresh()
 end
 
 function ReaderFooter:shouldBeRepainted()
-    local n = 1
+    if not self.view.footer_visible then
-    local widget = UIManager:getNthTopWidget(n)
+        return false
-    while widget do
+    end
-        if widget.name == "ReaderUI" then
+
-            return true
+    local top_wg = UIManager:getTopmostVisibleWidget() or {}
-        elseif widget.covers_fullscreen or widget.covers_footer then
+    if top_wg.name == "ReaderUI" then
-            -- (e.g. the virtual keyboard sets widget_covers_footer == true)
+        -- No overlap possible, it's safe to request a targeted widget repaint
-            return false
+        return true
-        end
+    elseif top_wg.covers_fullscreen or top_wg.covers_footer then
-        n = n + 1
+        -- No repaint necessary at all
-        widget = UIManager:getNthTopWidget(n)
+        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()

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(),
     }

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 = {
             {
                 {

frontend/ui/uimanager.lua

@@ -729,36 +729,29 @@ function UIManager:getNthTopWidget(n)
     return widget
 end
 
---[[--
+--- Top-to-bottom widgets iterator
-Get the *second* topmost widget, if there is one (name if possible, ref otherwise).
+--- 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 ;).
-Useful when VirtualKeyboard is involved, as it *always* steals the top spot ;).
+function UIManager:topdown_widgets_iter()
-
+    local n = #self._window_stack
-NOTE: Will skip over VirtualKeyboard instances, plural, in case there are multiple (because, apparently, we can do that.. ugh).
+    local i = n + 1
---]]
+    return function()
-function UIManager:getSecondTopmostWidget()
+        i = i - 1
-    if #self._window_stack < 2 then
+        if i > 0 then
-        -- Not enough widgets in the stack, bye!
+            return self._window_stack[i].widget
-        return nil
+        end
     end
+end
 
-    -- Because everything is terrible, you can actually instantiate multiple VirtualKeyboards,
+--- Get the topmost visible widget
-    -- and they'll stack at the top, so, loop until we get something that *isn't* VK...
+function UIManager:getTopmostVisibleWidget()
-    for i = #self._window_stack - 1, 1, -1 do
+    for i = #self._window_stack, 1, -1 do
         local widget = self._window_stack[i].widget
-
+        -- Skip invisible widgets (e.g., TrapWidget)
-        if widget.name then
+        if not widget.invisible 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
             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.

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 } }

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

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()

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 } }