Dispatcher: avoid multiple cre rerenderings when many settings changed (#9651)

When a gesture/profile was updating multiple cre settings,
each setting handler would emit UpdatePos which each would
force a re-rendering.
When this might be happening, postpone the rerendering
until all are set.
Needs some bit of refactoring to the events at play:
introduce "DocumentRerendered" event, and use it where
we used "UpdatePos" or "UpdateToc" to mean exactly that.

frontend/apps/reader/modules/readerdogear.lua

@@ -101,10 +101,10 @@ function ReaderDogear:updateDogearOffset()
     end
 end
 
-function ReaderDogear:onUpdatePos()
+function ReaderDogear:onDocumentRerendered()
     -- Catching the top status bar toggling with :onSetStatusLine()
-    -- would be too early. But "UpdatePos" is sent after it has
+    -- would be too early. But "DocumentRerendered" is sent after
-    -- been applied
+    -- it has been applied
     self:updateDogearOffset()
 end
 

frontend/apps/reader/modules/readerfooter.lua

@@ -908,7 +908,6 @@ function ReaderFooter:disableFooter()
     self.resetLayout = function() end
     self.updateFooterPage = function() end
     self.updateFooterPos = function() end
-    self.onUpdatePos = function() end
     self.mode = self.mode_list.off
     self.view.footer_visible = false
 end
@@ -2225,6 +2224,8 @@ function ReaderFooter:_updateFooterText(force_repaint, force_recompute)
     end
 end
 
+-- Note: no need for :onDocumentRerendered(), ReaderToc will catch "DocumentRerendered"
+-- and will then emit a "TocReset" after the new ToC is made.
 function ReaderFooter:onTocReset()
     self:setTocMarkers(true)
     if self.view.view_mode == "page" then
@@ -2263,10 +2264,6 @@ function ReaderFooter:onPosUpdate(pos, pageno)
     self:updateFooterPos()
 end
 
--- recalculate footer sizes when document page count is updated
--- see documentation for more info about this event.
-ReaderFooter.onUpdatePos = ReaderFooter.onUpdateFooter
-
 function ReaderFooter:onReaderReady()
     self.ui.menu:registerToMainMenu(self)
     self:setupTouchZones()

frontend/apps/reader/modules/readerrolling.lua

@@ -127,6 +127,7 @@ function ReaderRolling:init()
         self:onRedrawCurrentView()
     end)
     self.ui.menu:registerToMainMenu(self)
+    self.batched_update_count = 0
 end
 
 function ReaderRolling:onReadSettings(config)
@@ -859,12 +860,35 @@ function ReaderRolling:onZoom()
     self:updatePos()
 end
 
+function ReaderRolling:onBatchedUpdate()
+    -- This is called by Dispatcher, and it may be possible to have re-entrant calls
+    self.batched_update_count = self.batched_update_count + 1
+end
+
+function ReaderRolling:onBatchedUpdateDone()
+    self.batched_update_count = self.batched_update_count - 1
+    if self.batched_update_count <= 0 then
+        self.batched_update_count = 0
+        -- Be sure any Notification gets a chance to be painted before
+        -- a blocking rerendering
+        UIManager:nextTick(function()
+            self:onUpdatePos()
+        end)
+    end
+end
+
 --[[
-    remember to signal this event when the document has been zoomed,
+    remember to signal this event when the document layout could
-    font has been changed, or line height has been changed.
+    have changed (ie. font, line height, margin... change)
     Note that xpointer should not be changed.
+    The only handler of this event should be this one.
+    A "DocumentRerendered" event will be then sent if it has changed.
+    Provide force=true to get it emitted even if nothing has changed.
 --]]
-function ReaderRolling:onUpdatePos()
+function ReaderRolling:onUpdatePos(force)
+    if self.batched_update_count > 0 then
+        return
+    end
     if self.ui.postReaderCallback ~= nil then -- ReaderUI:init() not yet done
         -- Don't schedule any updatePos as long as ReaderUI:init() is
         -- not finished (one will be called in the ui.postReaderCallback
@@ -885,7 +909,7 @@ function ReaderRolling:onUpdatePos()
     -- previously because of some bad setDirty() in ConfigDialog widgets
     -- that were triggering a full repaint of crengine (so, the needed
     -- rerendering) before updatePos() is called.
-    self:updatePos()
+    self:updatePos(force)
 
     Device:setIgnoreInput(false) -- Allow processing of events (on Android).
     Input:inhibitInputUntil(0.2) -- Discard input events, which might have occurred (double tap).
@@ -893,24 +917,26 @@ function ReaderRolling:onUpdatePos()
     -- to allow for quicker setting changes and rendering comparisons.
 end
 
-function ReaderRolling:updatePos()
+function ReaderRolling:updatePos(force)
     if not self.ui.document then
         -- document closed since we were scheduleIn'ed
         return
     end
     -- Check if the document has been re-rendered
     local new_rendering_hash = self.ui.document:getDocumentRenderingHash()
-    if new_rendering_hash ~= self.rendering_hash then
+    if new_rendering_hash ~= self.rendering_hash or force then
         logger.dbg("rendering hash changed:", self.rendering_hash, ">", new_rendering_hash)
         self.rendering_hash = new_rendering_hash
         -- A few things like page numbers may have changed
         self.ui.document:resetCallCache() -- be really sure this cache is reset
         self.ui.document:_readMetadata() -- get updated document height and nb of pages
-        if self.hide_nonlinear_flows then
+        if self.hide_nonlinear_flows or force then
             self.ui.document:cacheFlows()
         end
+        -- Note: ReaderStatistics needs to get these in this order
+        -- ("PageUpdate" event first, and then "DocumentRerendered").
         self:_gotoXPointer(self.xpointer)
-        self.ui:handleEvent(Event:new("UpdateToc"))
+        self.ui:handleEvent(Event:new("DocumentRerendered"))
     end
     self:onUpdateTopStatusBarMarkers()
     UIManager:setDirty(self.view.dialog, "partial")
@@ -930,7 +956,7 @@ function ReaderRolling:onChangeViewMode()
         if self.visible_pages == 2 then
             -- Switching from 2-pages page mode to scroll mode has crengine switch to 1-page,
             -- and we need to notice this re-rendering and keep things sane
-            self.ui:handleEvent(Event:new("UpdatePos"))
+            self:onUpdatePos()
         end
         self:_gotoXPointer(self.xpointer)
         -- Ensure a whole screen refresh is always enqueued
@@ -1090,7 +1116,7 @@ function ReaderRolling:onSetVisiblePages(visible_pages)
     self.ui.document:setVisiblePageCount(visible_pages)
     local cur_visible_pages = self.ui.document:getVisiblePageCount()
     if cur_visible_pages ~= prev_visible_pages then
-        self.ui:handleEvent(Event:new("UpdatePos"))
+        self:onUpdatePos()
     end
 end
 
@@ -1102,7 +1128,7 @@ function ReaderRolling:onSetStatusLine(status_line)
     -- (We used to toggle the footer when toggling the top status bar,
     -- but people seem to like having them both, and it feels more
     -- practicable to have the independant.)
-    self.ui:handleEvent(Event:new("UpdatePos"))
+    self:onUpdatePos()
 end
 
 function ReaderRolling:onUpdateTopStatusBarMarkers()
@@ -1476,13 +1502,12 @@ function ReaderRolling:onToggleHideNonlinear()
     self.ui.document:setHideNonlinearFlows(self.hide_nonlinear_flows)
     -- The document may change due to forced pagebreaks between flows being
     -- added or removed, so we need to find our location
-    self:onUpdatePos()
     -- Even if the document doesn't change, we must ensure that the
     -- flow and call caches are cleared, to get the right page numbers,
     -- which may have changed, and the correct flow structure. Also,
     -- the footer needs updating, and TOC markers may come or go.
-    self.ui.document:cacheFlows()
+    -- So, provide force=true.
-    self.ui:handleEvent(Event:new("UpdateToc"))
+    self:onUpdatePos(true)
 end
 
 return ReaderRolling

frontend/apps/reader/modules/readerthumbnail.lua

@@ -499,7 +499,7 @@ function ReaderThumbnail:onColorRenderingUpdate()
 end
 
 -- CRE: emitted after a re-rendering
-ReaderThumbnail.onTocReset = ReaderThumbnail.resetCache
+ReaderThumbnail.onDocumentRerendered = ReaderThumbnail.resetCache
 -- Emitted When adding/removing/updating bookmarks and highlights
 ReaderThumbnail.onBookmarkAdded = ReaderThumbnail.resetCachedPagesForBookmarks
 ReaderThumbnail.onBookmarkRemoved = ReaderThumbnail.resetCachedPagesForBookmarks

frontend/apps/reader/modules/readertoc.lua

@@ -99,11 +99,12 @@ end
 function ReaderToc:onUpdateToc()
     self:resetToc()
     self.ui:handleEvent(Event:new("TocReset"))
-
+    return true
-    --- @note: Let this propagate, plugins/statistics uses it to react to changes in document pagination
-    --return true
 end
 
+-- Be sure to update the ToC after a CRE rerendering
+ReaderToc.onDocumentRerendered = ReaderToc.onUpdateToc
+
 function ReaderToc:onPageUpdate(pageno)
     if UIManager.FULL_REFRESH_COUNT == -1 or G_reader_settings:isTrue("refresh_on_chapter_boundaries") then
         local flash_on_second = G_reader_settings:nilOrFalse("no_refresh_on_second_chapter_page")

frontend/dispatcher.lua

@@ -931,6 +931,10 @@ function Dispatcher:execute(settings, gesture)
     if settings.settings ~= nil and settings.settings.show_as_quickmenu == true then
         return Dispatcher:_showAsMenu(settings)
     end
+    local has_many = util.tableSize(settings) > (settings.settings ~= nil and 2 or 1)
+    if has_many then
+        UIManager:broadcastEvent(Event:new("BatchedUpdate"))
+    end
     for k, v in iter_func(settings) do
         if type(k) == "number" then
             k = v
@@ -972,6 +976,9 @@ function Dispatcher:execute(settings, gesture)
         end
         Notification:resetNotifySource()
     end
+    if has_many then
+        UIManager:broadcastEvent(Event:new("BatchedUpdateDone"))
+    end
 end
 
 return Dispatcher

plugins/statistics.koplugin/main.lua

@@ -243,7 +243,7 @@ function ReaderStatistics:isEnabled()
 end
 
 -- Reset the (volatile) stats on page count changes (e.g., after a font size update)
-function ReaderStatistics:onUpdateToc()
+function ReaderStatistics:onDocumentRerendered()
     -- Note: this is called *after* onPageUpdate(new current page in new page count), which
     -- has updated the duration for (previous current page in old page count) and created
     -- a tuple for (new current page) with a 0-duration.
@@ -259,7 +259,7 @@ function ReaderStatistics:onUpdateToc()
     --   - it adds/creates self.page_stat[153]={..., {now, 0}}
     --   - it sets self.curr_page=153
     --   - (at this point, we don't know the new page count is 254)
-    -- - OnUpdateToc() is called:
+    -- - OnDocumentRerendered() is called:
     --   - insertDB() is called, which will still use the previous self.data.pages=200 as the
     --     page count, and will go at inserting or not in the DB:
     --       - (127, now-35s, 35, 200) inserted