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