From 0577fde59eb11db1f370b5ea735198c8be2572dd Mon Sep 17 00:00:00 2001 From: poire-z Date: Sun, 15 Jan 2017 21:47:22 +0100 Subject: [PATCH] Better tracking, management and freeing of BlitBuffers This should fix the most obvious memory leaks (noticable when using images as screensaver). ImageWidget: added a few more options needed by ImageViewer, and removed 'overflow' option as logic was wrong. --- frontend/ui/screensaver.lua | 1 + .../ui/widget/container/alphacontainer.lua | 18 +++++ frontend/ui/widget/imagewidget.lua | 69 ++++++++++++++----- 3 files changed, 69 insertions(+), 19 deletions(-) diff --git a/frontend/ui/screensaver.lua b/frontend/ui/screensaver.lua index be14a445f..1d449afb8 100644 --- a/frontend/ui/screensaver.lua +++ b/frontend/ui/screensaver.lua @@ -30,6 +30,7 @@ local function createWidgetFromFile(file) return createWidgetFromImage( ImageWidget:new{ file = file, + file_do_cache = false, height = Screen:getHeight(), width = Screen:getWidth(), autostretch = true, diff --git a/frontend/ui/widget/container/alphacontainer.lua b/frontend/ui/widget/container/alphacontainer.lua index 71359624c..52ececcda 100644 --- a/frontend/ui/widget/container/alphacontainer.lua +++ b/frontend/ui/widget/container/alphacontainer.lua @@ -29,6 +29,9 @@ function AlphaContainer:paintTo(bb, x, y) or private_bb:getWidth() ~= contentSize.w or private_bb:getHeight() ~= contentSize.h then + if private_bb then + private_bb:free() -- free the one we're going to replace + end -- create private blitbuffer for our child widget to paint to private_bb = BlitBuffer.new(contentSize.w, contentSize.h, bb:getType()) self.private_bb = private_bb @@ -38,6 +41,9 @@ function AlphaContainer:paintTo(bb, x, y) or self.background_bb:getWidth() ~= contentSize.w or self.background_bb:getHeight() ~= contentSize.h then + if self.background_bb then + self.background_bb:free() -- free the one we're going to replace + end self.background_bb = BlitBuffer.new(contentSize.w, contentSize.h, bb:getType()) end self.background_bb:blitFrom(bb, 0, 0, x, y) @@ -51,4 +57,16 @@ function AlphaContainer:paintTo(bb, x, y) bb:addblitFrom(private_bb, x, y, nil, nil, nil, nil, self.alpha) end +function AlphaContainer:onCloseWidget() + if self.private_bb then + self.private_bb:free() + self.private_bb = nil + end + if self.background_bb then + self.background_bb:free() + self.background_bb = nil + end +end + + return AlphaContainer diff --git a/frontend/ui/widget/imagewidget.lua b/frontend/ui/widget/imagewidget.lua index 5bcb09838..9b149a333 100644 --- a/frontend/ui/widget/imagewidget.lua +++ b/frontend/ui/widget/imagewidget.lua @@ -46,8 +46,17 @@ function ImageCacheItem:onFree() 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()) image = nil, + + -- 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 + image_disposable = true, + invert = nil, dim = nil, hide = nil, @@ -62,15 +71,19 @@ local ImageWidget = Widget:new{ -- widget size. i.e. either fit the width or fit the height according to the -- original image size. autostretch = false, - -- when overflow is set to true, image will be stretched to fit the widget - -- size vertically and horizontally, without impact original aspect ratio. - -- But overflow part will be ignored. - overflow = false, - _bb = nil + -- when pre_rotate is not 0, native image is rotated by this angle + -- before applying the other autostretch/autoscale settings + pre_rotate = 0, + -- former 'overflow' setting removed, as logic was wrong + + _bb = nil, + _bb_disposable = true -- whether we should free() our _bb } function ImageWidget:_loadimage() self._bb = self.image + -- don't touch or free if caller doesn't want that + self._bb_disposable = self.image_disposable end function ImageWidget:_loadfile() @@ -82,9 +95,11 @@ function ImageWidget:_loadfile() if cache then -- hit cache self._bb = cache.bb + self._bb_disposable = false -- don't touch or free a cached _bb else - if self.height and self.height > 200 then -- don't cache big images + 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 -- cache this image logger.dbg("cache", hash) @@ -94,6 +109,7 @@ function ImageWidget:_loadfile() 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 @@ -112,6 +128,14 @@ function ImageWidget:_render() else error("cannot render image") end + if self.pre_rotate ~= 0 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.pre_rotate) -- rotate in-place + end local native_w, native_h = self._bb:getWidth(), self._bb:getHeight() local w, h = self.width, self.height if self.autoscale then @@ -126,22 +150,20 @@ function ImageWidget:_render() h = self.height w = self.width * ratio else - h = self.height * ratio - w = self.width - end - elseif self.overflow then - local ratio = native_w / self.width / native_h * self.height - if ratio < 1 then h = self.height / ratio w = self.width - else - h = self.height - w = self.width / ratio end end end if (w and w ~= native_w) or (h and h ~= native_h) then - self._bb = self._bb:scale(w or native_w, h or native_h) + -- We're making a new blitbuffer, we need to explicitely free + -- the old one to not leak memory + local new_bb = self._bb:scale(w or native_w, h or native_h) + if self._bb_disposable then + self._bb:free() + end + self._bb = new_bb + self._bb_disposable = true -- new object will have to be freed end end @@ -177,11 +199,20 @@ function ImageWidget:paintTo(bb, x, y) end end +-- This will normally be called by our WidgetContainer:free() +-- But it SHOULD explicitely be called if we are getting replaced +-- (ie: in some other widget's update()), to not leak memory with +-- BlitBuffer zombies function ImageWidget:free() - if self.image then - self.image:free() - self.image = nil + if self._bb and self._bb_disposable and self._bb.free then + self._bb:free() + self._bb = nil end end +function ImageWidget:onCloseWidget() + -- free when UIManager:close() was called + self:free() +end + return ImageWidget