[CRe] Ensure toggling nightmode invalidates the drawCurrentView cache (#6854)

* Use a CRe set* method when toggling nightmode

This ensures it gets flagged as add_reset by the call cache, and that
CRe will actually re-render, as it's necessary if nightmode_images is
enabled (the default).

Fix #6845

* Prevent ReaderHighlight:onTap from running ReaderHighlight:clear when
it's unnecessary.

Avoiding a clearSelection call in CRe that could invalidate the cache
and cause unnecessary redraws.

* Don't store empty highlight arrays when all HLs on a page have
been deleted

frontend/apps/reader/modules/readerhighlight.lua

@@ -224,7 +224,13 @@ function ReaderHighlight:onClearHighlight()
 end
 
 function ReaderHighlight:onTap(_, ges)
-    if not self:clear() and ges then
+    -- We only actually need to clear if we have something to clear in the first place.
+    -- (We mainly want to avoid CRe's clearSelection,
+    -- which may incur a redraw as it invalidates the cache, c.f., #6854)
+    -- ReaderHighlight:clear can only return true if self.hold_pos was set anyway.
+    local cleared = self.hold_pos and self:clear()
+    -- We only care about potential taps on existing highlights, not on taps that closed a highlight menu.
+    if not cleared and ges then
         if self.ui.document.info.has_pages then
             return self:onTapPageSavedHighlight(ges)
         else
@@ -285,11 +291,18 @@ function ReaderHighlight:onTapXPointerSavedHighlight(ges)
     -- or removed).
     local cur_view_top, cur_view_bottom
     local pos = self.view:screenToPageTransform(ges.pos)
-    for page, _ in pairs(self.view.highlight.saved) do
+    -- NOTE: By now, pos.page is set, but if a highlight spans across multiple pages,
-        local items = self.view.highlight.saved[page]
+    --       it's stored under the hash of its *starting* point,
+    --       so we can't just check the current page, hence the giant hashwalk of death...
+    --       We can't even limit the walk to page <= pos.page,
+    --       because pos.page isn't super accurate in continuous mode
+    --       (it's the page number for what's it the topleft corner of the screen,
+    --       i.e., often a bit earlier)...
+    for page, items in pairs(self.view.highlight.saved) do
         if items then
             for i = 1, #items do
-                local pos0, pos1 = items[i].pos0, items[i].pos1
+                local item = items[i]
+                local pos0, pos1 = item.pos0, item.pos1
                 -- document:getScreenBoxesFromPositions() is expensive, so we
                 -- first check this item is on current page
                 if not cur_view_top then
@@ -1342,7 +1355,12 @@ end
 function ReaderHighlight:deleteHighlight(page, i, bookmark_item)
     self.ui:handleEvent(Event:new("DelHighlight"))
     logger.dbg("delete highlight", page, i)
+    -- The per-page table is a pure array
     local removed = table.remove(self.view.highlight.saved[page], i)
+    -- But the main, outer table is a hash, so clear the table for this page if there are no longer any highlights on it
+    if #self.view.highlight.saved[page] == 0 then
+        self.view.highlight.saved[page] = nil
+    end
     if bookmark_item then
         self.ui.bookmark:removeBookmark(bookmark_item)
     else

frontend/apps/reader/modules/readertypeset.lua

@@ -108,6 +108,7 @@ function ReaderTypeset:onReadSettings(config)
     end
     self:toggleImageScaling(self.smooth_scaling)
 
+    self.ui.document:setNightMode(Screen.night_mode)
     -- default to automagic nightmode-friendly handling of images
     self.nightmode_images = config:readSetting("nightmode_images")
     if self.nightmode_images == nil then

frontend/apps/reader/modules/readerview.lua

@@ -469,20 +469,21 @@ function ReaderView:drawPageSavedHighlight(bb, x, y)
     local pages = self:getCurrentPageList()
     for _, page in pairs(pages) do
         local items = self.highlight.saved[page]
-        if not items then items = {} end
+        if items then
-        for i = 1, #items do
+            for i = 1, #items do
-            local item = items[i]
+                local item = items[i]
-            local pos0, pos1 = item.pos0, item.pos1
+                local pos0, pos1 = item.pos0, item.pos1
-            local boxes = self.ui.document:getPageBoxesFromPositions(page, pos0, pos1)
+                local boxes = self.ui.document:getPageBoxesFromPositions(page, pos0, pos1)
-            if boxes then
+                if boxes then
-                for _, box in pairs(boxes) do
+                    for _, box in pairs(boxes) do
-                    local rect = self:pageToScreenTransform(page, box)
+                        local rect = self:pageToScreenTransform(page, box)
-                    if rect then
+                        if rect then
-                        self:drawHighlightRect(bb, x, y, rect, item.drawer or self.highlight.saved_drawer)
+                            self:drawHighlightRect(bb, x, y, rect, item.drawer or self.highlight.saved_drawer)
-                    end
+                        end
-                end -- end for each box
+                    end -- end for each box
-            end -- end if boxes
+                end -- end if boxes
-        end -- end for each highlight
+            end -- end for each highlight
+        end
     end -- end for each page
 end
 
@@ -492,41 +493,41 @@ function ReaderView:drawXPointerSavedHighlight(bb, x, y)
     -- clear that cache when page layout change or highlights are added
     -- or removed).
     local cur_view_top, cur_view_bottom
-    for page, _ in pairs(self.highlight.saved) do
+    for page, items in pairs(self.highlight.saved) do
-        local items = self.highlight.saved[page]
+        if items then
-        if not items then items = {} end
+            for j = 1, #items do
-        for j = 1, #items do
+                local item = items[j]
-            local item = items[j]
+                local pos0, pos1 = item.pos0, item.pos1
-            local pos0, pos1 = item.pos0, item.pos1
+                -- document:getScreenBoxesFromPositions() is expensive, so we
-            -- document:getScreenBoxesFromPositions() is expensive, so we
+                -- first check this item is on current page
-            -- first check this item is on current page
+                if not cur_view_top then
-            if not cur_view_top then
+                    -- Even in page mode, it's safer to use pos and ui.dimen.h
-                -- Even in page mode, it's safer to use pos and ui.dimen.h
+                    -- than pages' xpointers pos, even if ui.dimen.h is a bit
-                -- than pages' xpointers pos, even if ui.dimen.h is a bit
+                    -- larger than pages' heights
-                -- larger than pages' heights
+                    cur_view_top = self.ui.document:getCurrentPos()
-                cur_view_top = self.ui.document:getCurrentPos()
+                    if self.view_mode == "page" and self.ui.document:getVisiblePageCount() > 1 then
-                if self.view_mode == "page" and self.ui.document:getVisiblePageCount() > 1 then
+                        cur_view_bottom = cur_view_top + 2 * self.ui.dimen.h
-                    cur_view_bottom = cur_view_top + 2 * self.ui.dimen.h
+                    else
-                else
+                        cur_view_bottom = cur_view_top + self.ui.dimen.h
-                    cur_view_bottom = cur_view_top + self.ui.dimen.h
+                    end
                 end
-            end
+                local spos0 = self.ui.document:getPosFromXPointer(pos0)
-            local spos0 = self.ui.document:getPosFromXPointer(pos0)
+                local spos1 = self.ui.document:getPosFromXPointer(pos1)
-            local spos1 = self.ui.document:getPosFromXPointer(pos1)
+                local start_pos = math.min(spos0, spos1)
-            local start_pos = math.min(spos0, spos1)
+                local end_pos = math.max(spos0, spos1)
-            local end_pos = math.max(spos0, spos1)
+                if start_pos <= cur_view_bottom and end_pos >= cur_view_top then
-            if start_pos <= cur_view_bottom and end_pos >= cur_view_top then
+                    local boxes = self.ui.document:getScreenBoxesFromPositions(pos0, pos1, true) -- get_segments=true
-                local boxes = self.ui.document:getScreenBoxesFromPositions(pos0, pos1, true) -- get_segments=true
+                    if boxes then
-                if boxes then
+                        for _, box in pairs(boxes) do
-                    for _, box in pairs(boxes) do
+                            local rect = self:pageToScreenTransform(page, box)
-                        local rect = self:pageToScreenTransform(page, box)
+                            if rect then
-                        if rect then
+                                self:drawHighlightRect(bb, x, y, rect, item.drawer or self.highlight.saved_drawer)
-                            self:drawHighlightRect(bb, x, y, rect, item.drawer or self.highlight.saved_drawer)
+                            end
-                        end
+                        end -- end for each box
-                    end -- end for each box
+                    end -- end if boxes
-                end -- end if boxes
+                end
-            end
+            end -- end for each highlight
-        end -- end for each highlight
+        end
     end -- end for all saved highlight
 end
 

frontend/device/devicelistener.lua

@@ -26,6 +26,10 @@ end
 function DeviceListener:onToggleNightMode()
     local night_mode = G_reader_settings:isTrue("night_mode")
     Screen:toggleNightMode()
+    -- Make sure CRe will bypass the call cache
+    if self.ui and self.ui.document and self.ui.document.provider == "crengine" then
+        self.ui.document:setNightMode(not night_mode)
+    end
     UIManager:setDirty("all", "full")
     UIManager:ToggleNightMode(not night_mode)
     G_reader_settings:saveSetting("night_mode", not night_mode)

frontend/document/credocument.lua

@@ -25,6 +25,7 @@ local CreDocument = Document:new{
     _view_mode = nil,
     _smooth_scaling = false,
     _nightmode_images = true,
+    _nightmode = false,
 
     line_space_percent = 100,
     default_font = "Noto Serif",
@@ -423,7 +424,7 @@ function CreDocument:drawCurrentView(target, x, y, rect, pos)
 
     --local start_ts = FFIUtil.getTimestamp()
     self._drawn_images_count, self._drawn_images_surface_ratio =
-        self._document:drawCurrentPage(self.buffer, self.render_color, Screen.night_mode and self._nightmode_images, self._smooth_scaling, Screen.sw_dithering)
+        self._document:drawCurrentPage(self.buffer, self.render_color, self._nightmode and self._nightmode_images, self._smooth_scaling, Screen.sw_dithering)
     --local end_ts = FFIUtil.getTimestamp()
     --print(string.format("CreDocument:drawCurrentView: Rendering took %9.3f ms", (end_ts - start_ts) * 1000))
 
@@ -877,6 +878,11 @@ function CreDocument:setNightmodeImages(toggle)
     self._nightmode_images = toggle
 end
 
+function CreDocument:setNightMode(toggle)
+    logger.dbg("CreDocument: set nightmode", toggle)
+    self._nightmode = toggle
+end
+
 function CreDocument:setFloatingPunctuation(enabled)
     --- @fixme occasional segmentation fault when toggling floating punctuation
     logger.dbg("CreDocument: set floating punctuation", enabled)