From 0aa0b9b2deacdfd4efe24e08f34eee9a5a182b5e Mon Sep 17 00:00:00 2001 From: poire-z Date: Tue, 26 Sep 2017 10:21:58 +0200 Subject: [PATCH] ImageWidget: use MuPDF for scaling images (#3260) * Cache scaled images with scale_for_dpi * Don't update original self.scale_for_dpi A same ImageWidget object can be free()'d and re-render()ed, and we would then lose scale_for_dpi on next renderings --- base | 2 +- frontend/ui/widget/imagewidget.lua | 96 +++++++++++++++++++++++------- 2 files changed, 76 insertions(+), 22 deletions(-) diff --git a/base b/base index 548077c45..3b2acc944 160000 --- a/base +++ b/base @@ -1 +1 @@ -Subproject commit 548077c4508fb4552674ef40cf29b457dac2ae23 +Subproject commit 3b2acc944f43a9e82f0d4ae92aaaa49e59949e5c diff --git a/frontend/ui/widget/imagewidget.lua b/frontend/ui/widget/imagewidget.lua index cde1ad4a1..42fbc911f 100644 --- a/frontend/ui/widget/imagewidget.lua +++ b/frontend/ui/widget/imagewidget.lua @@ -29,6 +29,14 @@ local Geom = require("ui/geometry") local Cache = require("cache") local logger = require("logger") +-- DPI_SCALE can't change without a restart, so let's compute it now +local function get_dpi_scale() + local size_scale = math.min(Screen:getWidth(), Screen:getHeight())/600 + local dpi_scale = Screen:getDPI() / 167 + return math.pow(2, math.max(0, math.log((size_scale+dpi_scale)/2)/0.69)) +end +local DPI_SCALE = get_dpi_scale() + local ImageCache = Cache:new{ max_memsize = 2*1024*1024, -- 2M of image cache current_memsize = 0, @@ -55,7 +63,7 @@ local ImageWidget = Widget:new{ -- Whether BlitBuffer rendered from file should be cached file_do_cache = true, -- Whether provided BlitBuffer can be modified by us and SHOULD be free() by us, - -- normally true unless our caller wants to reuse it's provided image + -- normally true unless our caller wants to reuse its provided image image_disposable = true, -- Width and height of container, to limit rendering to this area @@ -76,7 +84,6 @@ local ImageWidget = Widget:new{ rotation_angle = 0, -- If scale_for_dpi is true image will be rescaled according to screen dpi - -- (x2 if DPI > 332) - (formerly known as 'autoscale') scale_for_dpi = false, -- When scale_factor is not nil, native image is scaled by this factor @@ -84,9 +91,11 @@ local ImageWidget = Widget:new{ -- Special case : scale_factor == 0 : image will be scaled to best fit provided -- width and height, keeping aspect ratio (scale_factor will be updated -- from 0 to the factor used at _render() time) - -- (former 'autostrech' setting removed, use "scale_factor=0" instead) scale_factor = nil, + -- Whether to use former blitbuffer:scale() (default to using MuPDF) + use_legacy_image_scaling = G_reader_settings:isTrue("legacy_image_scaling"), + -- For initial positionning, if (possibly scaled) image overflows width/height center_x_ratio = 0.5, -- default is centered on image's center center_y_ratio = 0.5, @@ -121,25 +130,44 @@ function ImageWidget:_loadfile() if itype == "png" or itype == "jpg" or itype == "jpeg" or itype == "tiff" then local hash = "image|"..self.file.."|"..(self.width or "").."|"..(self.height or "") + -- Do the scaling for DPI here, so it can be cached and not re-done + -- each time in _render() + -- In our use cases for files (icons), we either provide width and height, + -- or just scale_for_dpi, and scale_factor should stay nil. + -- Other combinations will result in double scaling anyway, and unexpected results. + local scale_for_dpi_here = false + if self.scale_for_dpi and DPI_SCALE ~= 1 then + scale_for_dpi_here = true -- we'll do it before caching + hash = hash .. "|d" + self.already_scaled_for_dpi = true -- so we don't do it again in _render() + end local cache = ImageCache:check(hash) if cache then -- hit cache self._bb = cache.bb self._bb_disposable = false -- don't touch or free a cached _bb else + self._bb = Mupdf.renderImageFile(self.file, self.width, self.height) + if scale_for_dpi_here then + local new_bb + local bb_w, bb_h = self._bb:getWidth(), self._bb:getHeight() + if self.use_legacy_image_scaling then + new_bb = self._bb:scale(bb_w * DPI_SCALE, bb_h * DPI_SCALE) + else + new_bb = Mupdf.scaleBlitBuffer(self._bb, math.floor(bb_w * DPI_SCALE), math.floor(bb_h * DPI_SCALE)) + end + self._bb:free() + self._bb = new_bb + end if not self.file_do_cache then - self._bb = Mupdf.renderImageFile(self.file, self.width, self.height) self._bb_disposable = true -- we made it, we can modify and free it else + self._bb_disposable = false -- don't touch or free a cached _bb -- cache this image logger.dbg("cache", hash) - cache = ImageCacheItem:new{ - bb = Mupdf.renderImageFile(self.file, self.width, self.height), - } + cache = ImageCacheItem:new{ bb = self._bb } cache.size = cache.bb.pitch * cache.bb.h * cache.bb:getBpp() / 8 ImageCache:insert(hash, cache) - self._bb = cache.bb - self._bb_disposable = false -- don't touch or free a cached _bb end end else @@ -165,25 +193,39 @@ function ImageWidget:_render() -- First, rotation if self.rotation_angle ~= 0 then - if not self._bb_disposable then - -- we can't modify _bb, make a copy - self._bb = self._bb:copy() + -- Allow for easy switch to former scaling via blitbuffer methods + if self.use_legacy_image_scaling then + if not self._bb_disposable then + -- we can't modify _bb, make a copy + self._bb = self._bb:copy() + self._bb_disposable = true -- new object will have to be freed + end + self._bb:rotate(self.rotation_angle) -- rotate in-place + else + -- If we use MuPDF for scaling, we can't use bb:rotate() anymore, + -- as it only flags rotation in the blitbuffer and rotation is dealt + -- with at painting time. MuPDF does not like such a blitbuffer, and + -- we get corrupted images when using it for scaling such blitbuffers. + -- We need to make a real new blitbuffer with rotated content: + local rot_bb = self._bb:rotatedCopy(self.rotation_angle) + -- We made a new blitbuffer, we need to explicitely free + -- the old one to not leak memory + if self._bb_disposable then + self._bb:free() + end + self._bb = rot_bb self._bb_disposable = true -- new object will have to be freed end - self._bb:rotate(self.rotation_angle) -- rotate in-place end local bb_w, bb_h = self._bb:getWidth(), self._bb:getHeight() -- scale_for_dpi setting: update scale_factor (even if not set) with it - if self.scale_for_dpi then - local size_scale = math.min(Screen:getWidth(), Screen:getHeight())/600 - local dpi_scale = Screen:getDPI() / 167 - dpi_scale = math.pow(2, math.max(0, math.log((size_scale+dpi_scale)/2)/0.69)) + if self.scale_for_dpi and not self.already_scaled_for_dpi then if self.scale_factor == nil then self.scale_factor = 1 end - self.scale_factor = self.scale_factor * dpi_scale + self.scale_factor = self.scale_factor * DPI_SCALE end -- scale to best fit container : compute scale_factor for that @@ -197,18 +239,30 @@ function ImageWidget:_render() end end - -- replace blitbuffer with a resizd one if needed + -- replace blitbuffer with a resized one if needed local new_bb = nil if self.scale_factor == nil then -- no scaling, but strech to width and height, only if provided and needed if self.width and self.height and (self.width ~= bb_w or self.height ~= bb_h) then logger.dbg("ImageWidget: stretching") - new_bb = self._bb:scale(self.width, self.height) + if self.use_legacy_image_scaling then + -- Uses "simple nearest neighbour scaling" + new_bb = self._bb:scale(self.width, self.height) + else + -- Better quality scaling with MuPDF + new_bb = Mupdf.scaleBlitBuffer(self._bb, self.width, self.height) + end end elseif self.scale_factor ~= 1 then -- scale by scale_factor (not needed if scale_factor == 1) logger.dbg("ImageWidget: scaling by", self.scale_factor) - new_bb = self._bb:scale(bb_w * self.scale_factor, bb_h * self.scale_factor) + if self.use_legacy_image_scaling then + new_bb = self._bb:scale(bb_w * self.scale_factor, bb_h * self.scale_factor) + else + -- Better to ensure we give integer width and height to MuPDF, to + -- avoid a black 1-pixel line at right and bottom of image + new_bb = Mupdf.scaleBlitBuffer(self._bb, math.floor(bb_w * self.scale_factor), math.floor(bb_h * self.scale_factor)) + end end if new_bb then -- We made a new blitbuffer, we need to explicitely free