From 4bb3999cbc3e727e9f1b196b5e93741510662b6c Mon Sep 17 00:00:00 2001 From: poire-z Date: Sat, 21 Apr 2018 22:03:04 +0200 Subject: [PATCH] RenderImage: factorize all image rendering and scaling code New module RenderImage (alongside existing RenderText) to provides image rendering and scaling facilities. Uses MuPDF, but tries first giflib on GIF. Allows for getting all the frames from an animated GIF. --- .../apps/reader/modules/readerhighlight.lua | 4 +- frontend/document/credocument.lua | 32 +-- frontend/ui/renderimage.lua | 200 ++++++++++++++++++ frontend/ui/widget/imagewidget.lua | 55 ++--- frontend/ui/wikipedia.lua | 21 +- .../coverbrowser.koplugin/bookinfomanager.lua | 11 +- 6 files changed, 240 insertions(+), 83 deletions(-) create mode 100644 frontend/ui/renderimage.lua diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua index 54ca84e7e..755f07b76 100644 --- a/frontend/apps/reader/modules/readerhighlight.lua +++ b/frontend/apps/reader/modules/readerhighlight.lua @@ -270,7 +270,9 @@ function ReaderHighlight:onHold(arg, ges) end -- check if we were holding on an image - local image = self.ui.document:getImageFromPosition(self.hold_pos) + -- we provide want_frames=true, so we get a list of images for + -- animated GIFs (supported by ImageViewer) + local image = self.ui.document:getImageFromPosition(self.hold_pos, true) if image then logger.dbg("hold on image") local ImageViewer = require("ui/widget/imageviewer") diff --git a/frontend/document/credocument.lua b/frontend/document/credocument.lua index 58d00bb3a..efe9fd180 100644 --- a/frontend/document/credocument.lua +++ b/frontend/document/credocument.lua @@ -4,6 +4,7 @@ local DataStorage = require("datastorage") local Document = require("document/document") local Font = require("ui/font") local Geom = require("ui/geometry") +local RenderImage = require("ui/renderimage") local Screen = require("device").screen local ffi = require("ffi") local lfs = require("libs/libkoreader-lfs") @@ -183,40 +184,25 @@ function CreDocument:getPageCount() end function CreDocument:getCoverPageImage() - -- don't need to render document in order to get cover image + -- no need to render document in order to get cover image if not self:loadDocument() then return nil -- not recognized by crengine end local data, size = self._document:getCoverPageImageData() if data and size then - local Mupdf = require("ffi/mupdf") - local ok, image = pcall(Mupdf.renderImage, data, size) - ffi.C.free(data) - if ok then - return image - end + local image = RenderImage:renderImageData(data, size) + ffi.C.free(data) -- free the userdata we got from crengine + return image end end -function CreDocument:getImageFromPosition(pos) +function CreDocument:getImageFromPosition(pos, want_frames) local data, size = self._document:getImageDataFromPosition(pos.x, pos.y) if data and size then logger.dbg("CreDocument: got image data from position", data, size) - local Mupdf = require("ffi/mupdf") - -- wrapped with pcall so we always free(data) - local ok, image = pcall(Mupdf.renderImage, data, size) - logger.dbg("Mupdf.renderImage", ok, image) - if not ok and string.find(image, "could not load image data: unknown image file format") then - -- in that case, mupdf seems to have already freed data (see mupdf/source/fitz/image.c:494), - -- as doing outselves ffi.C.free(data) would result in a crash with : - -- *** Error in `./luajit': double free or corruption (!prev): 0x0000000000e48a40 *** - logger.warn("Mupdf says 'unknown image file format', assuming mupdf has already freed image data") - else - ffi.C.free(data) -- need that explicite clean - end - if ok then - return image - end + local image = RenderImage:renderImageData(data, size, want_frames) + ffi.C.free(data) -- free the userdata we got from crengine + return image end end diff --git a/frontend/ui/renderimage.lua b/frontend/ui/renderimage.lua new file mode 100644 index 000000000..f992f7740 --- /dev/null +++ b/frontend/ui/renderimage.lua @@ -0,0 +1,200 @@ +--[[-- +Image rendering module. +]] + +local ffi = require("ffi") +local logger = require("logger") + +-- Will be loaded when needed +local Mupdf = nil +local Pic = nil + +local RenderImage = {} + +--- Renders image file as a BlitBuffer with the best renderer +-- +-- @string filename image file path +-- @bool[opt=false] want_frames whether to return a list of animated GIF frames +-- @int width requested width +-- @int height requested height +-- @treturn BlitBuffer or list of frames (each a function returning a Blitbuffer) +function RenderImage:renderImageFile(filename, want_frames, width, height) + local file = io.open(filename, "rb") + if not file then + logger.info("could not open image file:", filename) + return + end + local data = file:read("*a") + file:close() + return RenderImage:renderImageData(data, #data, want_frames, width, height) +end + + +--- Renders image data as a BlitBuffer with the best renderer +-- +-- @tparam data string or userdata (pointer) with image bytes +-- @int size size of data +-- @bool[opt=false] want_frames whether to return a list of animated GIF frames +-- @int width requested width +-- @int height requested height +-- @treturn BlitBuffer or list of frames (each a function returning a Blitbuffer) +function RenderImage:renderImageData(data, size, want_frames, width, height) + if not data or not size or size == 0 then + return + end + -- Guess if it is a GIF + local buffer = ffi.cast("unsigned char*", data) + local header = ffi.string(buffer, math.min(4, size)) + if header == "GIF8" then + logger.dbg("GIF file provided, renderImageData: using GifLib") + local image = self:renderGifImageDataWithGifLib(data, size, want_frames, width, height) + if image then + return image + end + -- fallback to rendering with MuPDF + end + logger.dbg("renderImageData: using MuPDF") + return self:renderImageDataWithMupdf(data, size, width, height) +end + +--- Renders image data as a BlitBuffer with MuPDF +-- +-- @tparam data string or userdata (pointer) with image bytes +-- @int size size of data +-- @int width requested width +-- @int height requested height +-- @treturn BlitBuffer +function RenderImage:renderImageDataWithMupdf(data, size, width, height) + if not Mupdf then Mupdf = require("ffi/mupdf") end + local ok, image = pcall(Mupdf.renderImage, data, size, width, height) + logger.dbg("Mupdf.renderImage", ok, image) + if not ok then + logger.info("failed rendering image (mupdf):", image) + return + end + return image + -- Our latest MuPDF does not seem to free() on error anymore. + -- So we can let that job to our caller who knows better. + -- XXX to remove: + -- if type(data) == 'userdata' then + -- if not ok and string.find(image, "could not load image data: unknown image file format") then + -- -- in that case, mupdf seems to have already freed data (see mupdf/source/fitz/image.c:494), + -- -- as doing outselves ffi.C.free(data) would result in a crash with : + -- -- *** Error in `./luajit': double free or corruption (!prev): 0x0000000000e48a40 *** + -- logger.warn("Mupdf says 'unknown image file format', assuming mupdf has already freed image data") + -- else + -- ffi.C.free(data) -- need that explicite clean + -- end + -- end +end + +--- Renders image data as a BlitBuffer with GifLib +-- +-- @tparam data string or userdata (pointer) with image bytes +-- @int size size of data +-- @bool[opt=false] want_frames whether to also return a list with animated GIF frames +-- @int width requested width +-- @int height requested height +-- @treturn BlitBuffer or list of frames (each a function returning a Blitbuffer) +function RenderImage:renderGifImageDataWithGifLib(data, size, want_frames, width, height) + if not data or not size or size == 0 then + return + end + if not Pic then Pic = require("ffi/pic") end + local ok, gif = pcall(Pic.openGIFDocumentFromData, data, size) + logger.dbg("Pic.openGIFDocumentFromData", ok) + if not ok then + logger.info("failed rendering image (giflib):", gif) + return + end + local nb_frames = gif:getPages() + logger.dbg("GifDocument, nb frames:", nb_frames) + if want_frames and nb_frames > 1 then + -- Returns a regular table, with functions (returning the BlitBuffer) + -- as values. Users will have to check via type() and call them. + -- (our luajit does not support __len via metatable, otherwise we + -- could have used setmetatable to avoid creating all the functions) + local frames = {} + -- As we don't cache the bb we build on the fly, let caller know it + -- will have to free them + frames.image_disposable = true + for i=1, nb_frames do + table.insert(frames, function() + local page = gif:openPage(i) + -- we do not page.close(), so image_bb is not freed + if page and page.image_bb then + return self:scaleBlitBuffer(page.image_bb, width, height) + end + end) + end + -- We can't close our GifDocument as long as we may fetch some + -- frame: we need to delay it till 'frames' is no more used. + frames.gif_close_needed = true + -- Should happen with that, but __gc seems never called... + frames = setmetatable(frames, { + __gc = function() + logger.dbg("frames.gc() called, closing GifDocument") + if frames.gif_close_needed then + gif:close() + frames.gif_close_needed = nil + end + end + }) + -- so, also set this method, so that ImageViewer can explicitely + -- call it onClose. + frames.free = function() + logger.dbg("frames.free() called, closing GifDocument") + if frames.gif_close_needed then + gif:close() + frames.gif_close_needed = nil + end + end + return frames + else + local page = gif:openPage(1) + -- we do not page.close(), so image_bb is not freed + if page and page.image_bb then + gif:close() + return self:scaleBlitBuffer(page.image_bb, width, height) + end + gif:close() + end + logger.info("failed rendering image (giflib)") +end + +--- Rescales a BlitBuffer to the requested size if needed +-- +-- @tparam bb BlitBuffer +-- @int width +-- @int height +-- @bool[opt=true] free_orig_bb free() original bb if scaled +-- @treturn BlitBuffer +function RenderImage:scaleBlitBuffer(bb, width, height, free_orig_bb) + if not width or not height then + logger.dbg("RenderImage:scaleBlitBuffer: no need") + return bb + end + -- Ensure we give integer width and height to MuPDF, to + -- avoid a black 1-pixel line at right and bottom of image + width, height = math.floor(width), math.floor(height) + if bb:getWidth() == width and bb:getHeight() == height then + logger.dbg("RenderImage:scaleBlitBuffer: no need") + return bb + end + logger.dbg("RenderImage:scaleBlitBuffer: scaling") + local scaled_bb + if G_reader_settings:isTrue("legacy_image_scaling") then + -- Uses "simple nearest neighbour scaling" + scaled_bb = bb:scale(width, height) + else + -- Better quality scaling with MuPDF + if not Mupdf then Mupdf = require("ffi/mupdf") end + scaled_bb = Mupdf.scaleBlitBuffer(bb, width, height) + end + if not free_orig_bb == false then + bb:free() + end + return scaled_bb +end + +return RenderImage diff --git a/frontend/ui/widget/imagewidget.lua b/frontend/ui/widget/imagewidget.lua index 8dbfcb497..68f0bc364 100644 --- a/frontend/ui/widget/imagewidget.lua +++ b/frontend/ui/widget/imagewidget.lua @@ -20,13 +20,13 @@ Show image from memory example: ]] -local Widget = require("ui/widget/widget") -local Screen = require("device").screen -local UIManager = require("ui/uimanager") +local Cache = require("cache") local CacheItem = require("cacheitem") -local Mupdf = require("ffi/mupdf") local Geom = require("ui/geometry") -local Cache = require("cache") +local RenderImage = require("ui/renderimage") +local Screen = require("device").screen +local UIManager = require("ui/uimanager") +local Widget = require("ui/widget/widget") local logger = require("logger") -- DPI_SCALE can't change without a restart, so let's compute it now @@ -57,7 +57,7 @@ end local ImageWidget = Widget:new{ -- Can be provided with a path to a file file = nil, - -- or an already made BlitBuffer (ie: made by Mupdf.renderImage()) + -- or an already made BlitBuffer (ie: made by RenderImage) image = nil, -- Whether BlitBuffer rendered from file should be cached @@ -132,7 +132,7 @@ function ImageWidget:_loadfile() -- 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, and unexpected results. - -- We should anyway only give self.width and self.height to Mupdf.renderImageFile(), + -- We should anyway only give self.width and self.height to renderImageFile(), -- and use them in cache hash, when self.scale_factor is nil, when we are sure -- we don't need to keep aspect ratio. local width, height @@ -155,17 +155,10 @@ function ImageWidget:_loadfile() self._bb = cache.bb self._bb_disposable = false -- don't touch or free a cached _bb else - self._bb = Mupdf.renderImageFile(self.file, width, height) + self._bb = RenderImage:renderImageFile(self.file, false, width, 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 + self._bb = RenderImage:scaleBlitBuffer(self._bb, math.floor(bb_w * DPI_SCALE), math.floor(bb_h * DPI_SCALE)) end if not self.file_do_cache then self._bb_disposable = true -- we made it, we can modify and free it @@ -248,40 +241,20 @@ function ImageWidget:_render() end -- 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") - 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 + self._bb = RenderImage:scaleBlitBuffer(self._bb, self.width, self.height, self._bb_disposable) + self._bb_disposable = true -- new bb will have to be freed 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) - 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 - -- the old one to not leak memory - if self._bb_disposable then - self._bb:free() - end - self._bb = new_bb - self._bb_disposable = true -- new object will have to be freed - bb_w, bb_h = self._bb:getWidth(), self._bb:getHeight() + self._bb = RenderImage:scaleBlitBuffer(self._bb, bb_w * self.scale_factor, bb_h * self.scale_factor, self._bb_disposable) + self._bb_disposable = true -- new bb will have to be freed end + bb_w, bb_h = self._bb:getWidth(), self._bb:getHeight() -- deal with positionning if self.width and self.height then diff --git a/frontend/ui/wikipedia.lua b/frontend/ui/wikipedia.lua index 70d78f9aa..1d92e5f71 100644 --- a/frontend/ui/wikipedia.lua +++ b/frontend/ui/wikipedia.lua @@ -1,4 +1,5 @@ local JSON = require("json") +local RenderImage = require("ui/renderimage") local Screen = require("device").screen local ffiutil = require("ffi/util") local logger = require("logger") @@ -449,26 +450,28 @@ local function image_load_bb_func(image, highres) end logger.dbg(" fetched", #data) - -- Use mupdf to render image to blitbuffer - local mupdf = require("ffi/mupdf") - local ok, bb_or_error + local bb if not highres then -- For low-res, we should ensure the image we got from wikipedia is -- the right size, so it does not overflow our reserved area -- (TextBoxWidget may have adjusted image.width and height) - ok, bb_or_error = pcall(mupdf.renderImage, data, #data, image.width, image.height) + -- We don't get animated GIF multiple frames to keep TextBoxWidget + -- simple: they will be available when viewed in highres + bb = RenderImage:renderImageData(data, #data, false, image.width, image.height) else + -- We provide want_frames=true for highres images, so ImageViewer + -- can display animated GIF -- No need for width and height for high-res - ok, bb_or_error = pcall(mupdf.renderImage, data, #data) + bb = RenderImage:renderImageData(data, #data, true) end - if not ok then - logger.warn("failed building image from", source, ":", bb_or_error) + if not bb then + logger.warn("failed building image from", source) return end if not highres then - image.bb = bb_or_error + image.bb = bb else - image.hi_bb = bb_or_error + image.hi_bb = bb end end diff --git a/plugins/coverbrowser.koplugin/bookinfomanager.lua b/plugins/coverbrowser.koplugin/bookinfomanager.lua index e770de16c..d956867b4 100644 --- a/plugins/coverbrowser.koplugin/bookinfomanager.lua +++ b/plugins/coverbrowser.koplugin/bookinfomanager.lua @@ -1,7 +1,7 @@ local Blitbuffer = require("ffi/blitbuffer") local DataStorage = require("datastorage") local DocumentRegistry = require("document/documentregistry") -local Mupdf = require("ffi/mupdf") +local RenderImage = require("ui/renderimage") local SQ3 = require("lua-ljsqlite3/init") local UIManager = require("ui/uimanager") local lfs = require("libs/libkoreader-lfs") @@ -416,14 +416,7 @@ function BookInfoManager:extractBookInfo(filepath, cover_specs) scale_factor = math.min(spec_max_cover_w / cbb_w, spec_max_cover_h / cbb_h) cbb_w = math.min(math.floor(cbb_w * scale_factor)+1, spec_max_cover_w) cbb_h = math.min(math.floor(cbb_h * scale_factor)+1, spec_max_cover_h) - local new_bb - if self.use_legacy_image_scaling then - new_bb = cover_bb:scale(cbb_w, cbb_h) - else - new_bb = Mupdf.scaleBlitBuffer(cover_bb, cbb_w, cbb_h) - end - cover_bb:free() - cover_bb = new_bb + cover_bb = RenderImage:scaleBlitBuffer(cover_bb, cbb_w, cbb_h) end dbrow.cover_w = cbb_w dbrow.cover_h = cbb_h