From 46a5d20513ba87b79a2fd227ce93f4434ad9dec0 Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Thu, 8 Aug 2024 04:52:24 +0200 Subject: [PATCH] Document: Do not cache panel-zoom tiles to disk and fix their caching and rendering (#12303) * Use a dedicated cache hash for partial tiles from panel-zoom * Never dump them to disk, as it confuses DocCache's crappy heuristics that rewinds the cache to skip over the hinted page to try to dump the on-screen page to disk. * Apply the zoom factor in the exact same way as any other page rect (i.e., floor coordinates, ceil dimensions), and make sure said rect is actually a Geom so it doesn't break the cache hash, which relies on Geom's custom tostring method for rects. Said scaling method *also* belongs to the Geom class anyway. * Handle such pre-scaled rects properly in renderPage, so as not to apply the zoom factor to the full page, which would attempt to create a gigantic buffer. * And now that the rect is rendered properly in an appropriately-sized buffer, use the rendered tile as-is, no need to blit it to another (potentially way too large because of the above issue) blank BB. * The zoom factor is now computed for a scale to best-fit (honoring `imageviewer_rotate_auto_for_best_fit`), ensuring the best efficiency (ImageViewer won't have to re-scale). * Cache: Reduce the maximum item size to 50% of the cache, instead of 75%. * Warn about the legacy ReaderRotation module, as it turned out to be horribly broken. The whole machinery (which is spread over *a lot* of various codepaths) is left as-is, peppered with notes & fixmes hinting at the problem. Thankfully, that's not how we actually handle rotation, so it was probably hardly ever used (which possibly explains why nobody ever noticed it breaking, and that nugget possibly dates back to the inception of the kpv -> ko refactor!). (#12309) --- .../apps/reader/modules/readerhighlight.lua | 4 +- .../apps/reader/modules/readerrotation.lua | 3 +- frontend/apps/reader/modules/readerview.lua | 1 + .../apps/reader/modules/readerzooming.lua | 1 + frontend/cache.lua | 4 +- frontend/document/document.lua | 142 +++++++++++------- 6 files changed, 100 insertions(+), 55 deletions(-) diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua index 02e5dee33..87d75bccb 100644 --- a/frontend/apps/reader/modules/readerhighlight.lua +++ b/frontend/apps/reader/modules/readerhighlight.lua @@ -1271,14 +1271,16 @@ function ReaderHighlight:onPanelZoom(arg, ges) if not hold_pos then return false end -- outside page boundary local rect = self.ui.document:getPanelFromPage(hold_pos.page, hold_pos) if not rect then return false end -- panel not found, return - local image = self.ui.document:getPagePart(hold_pos.page, rect, 0) + local image, rotate = self.ui.document:drawPagePart(hold_pos.page, rect, 0) if image then local ImageViewer = require("ui/widget/imageviewer") local imgviewer = ImageViewer:new{ image = image, + image_disposable = false, -- It's a TileCache item with_title_bar = false, fullscreen = true, + rotated = rotate, } UIManager:show(imgviewer) return true diff --git a/frontend/apps/reader/modules/readerrotation.lua b/frontend/apps/reader/modules/readerrotation.lua index 5715d24ea..b1d26d7c8 100644 --- a/frontend/apps/reader/modules/readerrotation.lua +++ b/frontend/apps/reader/modules/readerrotation.lua @@ -36,7 +36,8 @@ end ReaderRotation.onPhysicalKeyboardConnected = ReaderRotation.registerKeyEvents --- @todo Reset rotation on new document, maybe on new page? - +--- @fixme: More importantly, this breaks rendering, c.f., `Document:renderPage` +-- A modern implementation of this feature is available in Dispatcher via the `IterateRotation` Event. function ReaderRotation:onRotate(rotate_by) self.current_rotation = (self.current_rotation + rotate_by) % 360 self.ui:handleEvent(Event:new("RotationUpdate", self.current_rotation)) diff --git a/frontend/apps/reader/modules/readerview.lua b/frontend/apps/reader/modules/readerview.lua index 456272efc..220894527 100644 --- a/frontend/apps/reader/modules/readerview.lua +++ b/frontend/apps/reader/modules/readerview.lua @@ -922,6 +922,7 @@ function ReaderView:onBBoxUpdate(bbox) self.use_bbox = bbox and true or false end +--- @note: From ReaderRotation, which is broken and disabled. function ReaderView:onRotationUpdate(rotation) self.state.rotation = rotation self:recalculate() diff --git a/frontend/apps/reader/modules/readerzooming.lua b/frontend/apps/reader/modules/readerzooming.lua index 50cd5b518..cba7dbc78 100644 --- a/frontend/apps/reader/modules/readerzooming.lua +++ b/frontend/apps/reader/modules/readerzooming.lua @@ -322,6 +322,7 @@ function ReaderZooming:onRestoreDimensions(dimensions) self:setZoom() end +--- @note: From ReaderRotation, which is broken and disabled. function ReaderZooming:onRotationUpdate(rotation) self.rotation = rotation self:setZoom() diff --git a/frontend/cache.lua b/frontend/cache.lua index 2c611f9cf..f5076f819 100644 --- a/frontend/cache.lua +++ b/frontend/cache.lua @@ -136,8 +136,8 @@ function Cache:get(key) end function Cache:willAccept(size) - -- We only allow a single object to fill 75% of the cache - return size*4 < self.size*3 + -- We only allow a single object to fill 50% of the cache + return size*4 < self.size*2 end -- Blank the cache diff --git a/frontend/document/document.lua b/frontend/document/document.lua index 5bee6571f..08a2bf351 100644 --- a/frontend/document/document.lua +++ b/frontend/document/document.lua @@ -266,16 +266,22 @@ function Document:getPageNumberInFlow(page) return page end --- calculates page dimensions -function Document:getPageDimensions(pageno, zoom, rotation) - local native_dimen = self:getNativePageDimensions(pageno):copy() +-- Transform a given rect according to the specified zoom & rotation +function Document:transformRect(native_rect, zoom, rotation) + local rect = native_rect:copy() if rotation == 90 or rotation == 270 then -- switch orientation - native_dimen.w, native_dimen.h = native_dimen.h, native_dimen.w + rect.w, rect.h = rect.h, rect.w end -- Apply the zoom factor, and round to integer in a sensible manner - native_dimen:transformByScale(zoom) - return native_dimen + rect:transformByScale(zoom) + return rect +end + +-- Ditto, but we get the input rect from the full page dimensions for a given page number +function Document:getPageDimensions(pageno, zoom, rotation) + local native_rect = self:getNativePageDimensions(pageno) + return self:transformRect(native_rect, zoom, rotation) end function Document:getPageBBox(pageno) @@ -401,17 +407,37 @@ end function Document:getFullPageHash(pageno, zoom, rotation, gamma) return "renderpg|"..self.file.."|"..self.mod_time.."|"..pageno.."|" - ..zoom.."|"..rotation.."|"..gamma.."|"..self.render_mode..(self.render_color and "|color" or "|bw") + ..zoom.."|" + ..rotation.."|"..gamma.."|"..self.render_mode..(self.render_color and "|color" or "|bw") + ..(self.reflowable_font_size and "|"..self.reflowable_font_size or "") +end + +function Document:getPagePartHash(pageno, zoom, rotation, gamma, rect) + return "renderpgpart|"..self.file.."|"..self.mod_time.."|"..pageno.."|" + ..tostring(rect).."|"..zoom.."|"..tostring(rect.scaled_rect).."|" + ..rotation.."|"..gamma.."|"..self.render_mode..(self.render_color and "|color" or "|bw") ..(self.reflowable_font_size and "|"..self.reflowable_font_size or "") end function Document:renderPage(pageno, rect, zoom, rotation, gamma, hinting) - local hash_excerpt - local hash = self:getFullPageHash(pageno, zoom, rotation, gamma) - local tile = DocCache:check(hash, TileCacheItem) - if not tile then - hash_excerpt = hash.."|"..tostring(rect) - tile = DocCache:check(hash_excerpt) + -- If rect contains a nested scaled_rect object, our caller handled scaling itself (e.g., drawPagePart) + local is_prescaled = rect and rect.scaled_rect ~= nil or false + + local hash, hash_excerpt, tile + if is_prescaled then + hash = self:getPagePartHash(pageno, zoom, rotation, gamma, rect) + + tile = DocCache:check(hash, TileCacheItem) + else + hash = self:getFullPageHash(pageno, zoom, rotation, gamma) + + tile = DocCache:check(hash, TileCacheItem) + + -- In the is_prescaled branch above, we're *already* only rendering part of the page + if not tile and rect then + hash_excerpt = hash.."|"..tostring(rect) + tile = DocCache:check(hash_excerpt) + end end if tile then if self.tile_cache_validity_ts then @@ -429,29 +455,37 @@ function Document:renderPage(pageno, rect, zoom, rotation, gamma, hinting) end local page_size = self:getPageDimensions(pageno, zoom, rotation) - -- this will be the size we actually render - local size = page_size - -- we prefer to render the full page, if it fits into cache - if not DocCache:willAccept(size.w * size.h * (self.render_color and 4 or 1) + 512) then - -- whole page won't fit into cache - logger.dbg("rendering only part of the page") - --- @todo figure out how to better segment the page - if not rect then - logger.warn("aborting, since we do not have a specification for that part") - -- required part not given, so abort - if hinting then - CanvasContext:enableCPUCores(1) + + -- This will be the actual render size (i.e., the BB's dimensions) + local size + if is_prescaled then + -- Our caller already handled the scaling, honor it. + -- And we don't particulalry care whether DocCache will be able to cache it in RAM, so no need to double-check. + size = rect.scaled_rect + else + -- We prefer to render the full page, if it fits into cache... + size = page_size + if not DocCache:willAccept(size.w * size.h * (self.render_color and 4 or 1) + 512) then + -- ...and if it doesn't... + logger.dbg("Attempting to render only part of the page:", rect) + --- @todo figure out how to better segment the page + if not rect then + logger.warn("No render region was specified, we won't render the page at all!") + -- no rect specified, abort + if hinting then + CanvasContext:enableCPUCores(1) + end + return end - return + -- ...only render the requested rect + hash = hash_excerpt + size = rect end - -- only render required part - hash = hash_excerpt - size = rect end - -- prepare cache item with contained blitbuffer + -- Prepare our BB, and wrap it in a cache item for DocCache tile = TileCacheItem:new{ - persistent = true, + persistent = not is_prescaled, -- we don't want to dump page fragments to disk (unnecessary, and it would confuse DocCache's heuristics) doc_path = self.file, created_ts = os.time(), excerpt = size, @@ -460,11 +494,16 @@ function Document:renderPage(pageno, rect, zoom, rotation, gamma, hinting) } tile.size = tonumber(tile.bb.stride) * tile.bb.h + 512 -- estimation - -- create a draw context + -- We need a draw context local dc = DrawContext.new() dc:setRotate(rotation) - -- correction of rotation + -- Make the context match the rotation, + -- by pointing at the rotated origin via coordinates offsets. + -- NOTE: We rotate our *Screen* bb on rotation (SetRotationMode), not the document, + -- so we hardly ever exercize this codepath... + -- AFAICT, the only thing that will *ever* (attempt to) rotate the document is ReaderRotation's key bindings (RotationUpdate). + --- @fixme: And whaddayano, it's broken ;). The aptly named key binds in question are J/K, I shit you not. if rotation == 90 then dc:setOffset(page_size.w, 0) elseif rotation == 180 then @@ -478,7 +517,7 @@ function Document:renderPage(pageno, rect, zoom, rotation, gamma, hinting) dc:setGamma(gamma) end - -- render + -- And finally, render the page in our BB local page = self._document:openPage(pageno) page:draw(dc, tile.bb, size.x, size.y, self.render_mode) page:close() @@ -529,24 +568,25 @@ function Document:getDrawnImagesStatistics() return self._drawn_images_count, self._drawn_images_surface_ratio end -function Document:getPagePart(pageno, rect, rotation) +function Document:drawPagePart(pageno, native_rect, rotation) + -- native_rect is straight from base, so not a Geom + local rect = Geom:new(native_rect) + local canvas_size = CanvasContext:getSize() - local zoom = math.min(canvas_size.w*2 / rect.w, canvas_size.h*2 / rect.h) - -- it's really, really important to do math.floor, otherwise we get image projection - local scaled_rect = { - x = math.floor(rect.x * zoom), - y = math.floor(rect.y * zoom), - w = math.floor(rect.w * zoom), - h = math.floor(rect.h * zoom), - } - local tile = self:renderPage(pageno, scaled_rect, zoom, rotation, 1.0) - local target = Blitbuffer.new(scaled_rect.w, scaled_rect.h, self.render_color and self.color_bb_type or nil) - target:blitFrom(tile.bb, - 0, 0, - scaled_rect.x - tile.excerpt.x, - scaled_rect.y - tile.excerpt.y, - scaled_rect.w, scaled_rect.h) - return target + -- Compute a zoom in order to scale to best fit, so that ImageViewer doesn't have to rescale further. + -- Optionally, based on ImageViewer settings, we'll auto-rotate for the best resolution. + local rotate = false + if G_reader_settings:isTrue("imageviewer_rotate_auto_for_best_fit") then + rotate = (canvas_size.w > canvas_size.h) ~= (rect.w > rect.h) + end + local zoom = rotate and math.min(canvas_size.w / rect.h, canvas_size.h / rect.w) or math.min(canvas_size.w / rect.w, canvas_size.h / rect.h) + local scaled_rect = self:transformRect(rect, zoom, rotation) + -- Stuff it inside rect so renderPage knows we're handling scaling ourselves + rect.scaled_rect = scaled_rect + + -- Enable SMP via the hinting flag + local tile = self:renderPage(pageno, rect, zoom, rotation, 1.0, true) + return tile.bb, rotate end function Document:getPageText(pageno)