From 4caf8f281d6ee1c781719446332c47aa8ec9a8ce Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sun, 8 Nov 2020 02:18:50 +0100 Subject: [PATCH] [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 --- .../apps/reader/modules/readerhighlight.lua | 26 ++++- .../apps/reader/modules/readertypeset.lua | 1 + frontend/apps/reader/modules/readerview.lua | 97 ++++++++++--------- frontend/device/devicelistener.lua | 4 + frontend/document/credocument.lua | 8 +- 5 files changed, 83 insertions(+), 53 deletions(-) diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua index 03269f6f6..f936faeb0 100644 --- a/frontend/apps/reader/modules/readerhighlight.lua +++ b/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 - local items = self.view.highlight.saved[page] + -- NOTE: By now, pos.page is set, but if a highlight spans across multiple pages, + -- 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 diff --git a/frontend/apps/reader/modules/readertypeset.lua b/frontend/apps/reader/modules/readertypeset.lua index 2acc67e4a..8991d80fd 100644 --- a/frontend/apps/reader/modules/readertypeset.lua +++ b/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 diff --git a/frontend/apps/reader/modules/readerview.lua b/frontend/apps/reader/modules/readerview.lua index 49c10a5d6..2ab578861 100644 --- a/frontend/apps/reader/modules/readerview.lua +++ b/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 - for i = 1, #items do - local item = items[i] - local pos0, pos1 = item.pos0, item.pos1 - local boxes = self.ui.document:getPageBoxesFromPositions(page, pos0, pos1) - if boxes then - for _, box in pairs(boxes) do - local rect = self:pageToScreenTransform(page, box) - if rect then - self:drawHighlightRect(bb, x, y, rect, item.drawer or self.highlight.saved_drawer) - end - end -- end for each box - end -- end if boxes - end -- end for each highlight + if items then + for i = 1, #items do + local item = items[i] + local pos0, pos1 = item.pos0, item.pos1 + local boxes = self.ui.document:getPageBoxesFromPositions(page, pos0, pos1) + if boxes then + for _, box in pairs(boxes) do + local rect = self:pageToScreenTransform(page, box) + if rect then + self:drawHighlightRect(bb, x, y, rect, item.drawer or self.highlight.saved_drawer) + end + end -- end for each box + end -- end if boxes + 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 - local items = self.highlight.saved[page] - if not items then items = {} end - for j = 1, #items do - local item = items[j] - 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 - -- 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 - -- larger than pages' heights - cur_view_top = self.ui.document:getCurrentPos() - if self.view_mode == "page" and self.ui.document:getVisiblePageCount() > 1 then - cur_view_bottom = cur_view_top + 2 * self.ui.dimen.h - else - cur_view_bottom = cur_view_top + self.ui.dimen.h + for page, items in pairs(self.highlight.saved) do + if items then + for j = 1, #items do + local item = items[j] + 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 + -- 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 + -- larger than pages' heights + cur_view_top = self.ui.document:getCurrentPos() + if self.view_mode == "page" and self.ui.document:getVisiblePageCount() > 1 then + cur_view_bottom = cur_view_top + 2 * self.ui.dimen.h + else + cur_view_bottom = cur_view_top + self.ui.dimen.h + end end - end - local spos0 = self.ui.document:getPosFromXPointer(pos0) - local spos1 = self.ui.document:getPosFromXPointer(pos1) - local start_pos = math.min(spos0, spos1) - local end_pos = math.max(spos0, spos1) - 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 - if boxes then - for _, box in pairs(boxes) do - local rect = self:pageToScreenTransform(page, box) - if rect then - self:drawHighlightRect(bb, x, y, rect, item.drawer or self.highlight.saved_drawer) - end - end -- end for each box - end -- end if boxes - end - end -- end for each highlight + local spos0 = self.ui.document:getPosFromXPointer(pos0) + local spos1 = self.ui.document:getPosFromXPointer(pos1) + local start_pos = math.min(spos0, spos1) + local end_pos = math.max(spos0, spos1) + 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 + if boxes then + for _, box in pairs(boxes) do + local rect = self:pageToScreenTransform(page, box) + if rect then + self:drawHighlightRect(bb, x, y, rect, item.drawer or self.highlight.saved_drawer) + end + end -- end for each box + end -- end if boxes + end + end -- end for each highlight + end end -- end for all saved highlight end diff --git a/frontend/device/devicelistener.lua b/frontend/device/devicelistener.lua index e4325c7b2..d841e92cf 100644 --- a/frontend/device/devicelistener.lua +++ b/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) diff --git a/frontend/document/credocument.lua b/frontend/document/credocument.lua index 31e596650..c791469cd 100644 --- a/frontend/document/credocument.lua +++ b/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)