From 5ec29d853a754f7d73afeaf9e37dd53664d3e206 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Thu, 18 Mar 2021 23:30:31 -0500 Subject: [PATCH 01/17] fix memory leak in widget renderers, textures and canvases caches --- internal/cache/base.go | 135 +++++++++++++++++++++++++++++ internal/cache/canvases.go | 36 ++++++++ internal/cache/texture_common.go | 39 +++++++++ internal/cache/texture_desktop.go | 18 ++++ internal/cache/texture_gomobile.go | 20 +++++ internal/cache/widget.go | 34 ++++++-- internal/driver/glfw/canvas.go | 7 +- internal/driver/glfw/driver.go | 9 +- internal/painter/gl/draw.go | 2 +- internal/painter/gl/gl_common.go | 13 ++- internal/painter/gl/gl_core.go | 5 +- internal/painter/gl/gl_es.go | 5 +- internal/painter/gl/gl_gomobile.go | 5 +- 13 files changed, 297 insertions(+), 31 deletions(-) create mode 100644 internal/cache/base.go create mode 100644 internal/cache/canvases.go create mode 100644 internal/cache/texture_common.go create mode 100644 internal/cache/texture_desktop.go create mode 100644 internal/cache/texture_gomobile.go diff --git a/internal/cache/base.go b/internal/cache/base.go new file mode 100644 index 0000000000..06d08f96b1 --- /dev/null +++ b/internal/cache/base.go @@ -0,0 +1,135 @@ +package cache + +import ( + "sync" + "time" + + "fyne.io/fyne/v2" +) + +var cleanCh = make(chan struct{}, 1) +var cleanTaskOnce sync.Once + +func init() { + + cleanTaskOnce.Do(func() { + + go func() { + expired := make([]fyne.CanvasObject, 0, 50) + + cleanTask := func(excludeCanvases bool) { + now := time.Now() + // -- Renderers clean task + expired = expired[:0] + renderersLock.RLock() + for wid, rinfo := range renderers { + if rinfo.isExpired(now) { + rinfo.renderer.Destroy() + expired = append(expired, wid) + } + } + renderersLock.RUnlock() + if len(expired) > 0 { + renderersLock.Lock() + for i, exp := range expired { + delete(renderers, exp.(fyne.Widget)) + expired[i] = nil + } + renderersLock.Unlock() + } + + // -- Textures clean task + expired = expired[:0] + texturesLock.RLock() + for obj, tinfo := range textures { + if tinfo.isExpired(now) { + expired = append(expired, obj) + } + } + texturesLock.RUnlock() + if len(expired) > 0 { + texturesLock.Lock() + for i, exp := range expired { + if free := textures[exp].freeFn; free != nil { + go free(exp) + } + delete(textures, exp) + expired[i] = nil + } + texturesLock.Unlock() + } + + if excludeCanvases { + return + } + + // -- Canvases clean task + expired = expired[:0] + canvasesLock.RLock() + for obj, cinfo := range canvases { + if cinfo.isExpired(now) { + expired = append(expired, obj) + } + } + canvasesLock.RUnlock() + if len(expired) > 0 { + canvasesLock.Lock() + for i, exp := range expired { + delete(canvases, exp) + expired[i] = nil + } + canvasesLock.Unlock() + } + } + + t := time.NewTicker(time.Minute) + for { + select { + case <-cleanCh: + cleanTask(false) + // do not trigger another clean task so fast + time.Sleep(10 * time.Second) + t.Reset(time.Minute) + case <-t.C: + // canvases cache can't be invalidated using the ticker + // because if we do it, there wouldn't be a way to recover them later + cleanTask(true) + } + } + }() + }) + +} + +// Clean triggers clean cache task. +func Clean() { + select { + case cleanCh <- struct{}{}: + default: + return + } +} + +// =============================================================== +// Privates +// =============================================================== + +type expiringCache struct { + expireLock sync.RWMutex + expires time.Time +} + +// isExpired check if the cache data is expired. +func (c *expiringCache) isExpired(now time.Time) bool { + c.expireLock.RLock() + defer c.expireLock.RUnlock() + return c.expires.Before(now) +} + +// setAlive updates expiration time. +func (c *expiringCache) setAlive() { + t := time.Now().Add(1 * time.Minute) + c.expireLock.Lock() + c.expires = t + c.expireLock.Unlock() +} diff --git a/internal/cache/canvases.go b/internal/cache/canvases.go new file mode 100644 index 0000000000..2cc651b659 --- /dev/null +++ b/internal/cache/canvases.go @@ -0,0 +1,36 @@ +package cache + +import ( + "sync" + + "fyne.io/fyne/v2" +) + +var canvasesLock sync.RWMutex +var canvases = make(map[fyne.CanvasObject]*canvasInfo, 1024) + +// GetCanvasForObject returns the canvas for the specified object. +func GetCanvasForObject(obj fyne.CanvasObject) fyne.Canvas { + canvasesLock.RLock() + cinfo, ok := canvases[obj] + canvasesLock.RUnlock() + if cinfo == nil || !ok { + return nil + } + cinfo.setAlive() + return cinfo.canvas +} + +// SetCanvasForObject sets the canvas for the specified object. +func SetCanvasForObject(obj fyne.CanvasObject, canvas fyne.Canvas) { + cinfo := &canvasInfo{canvas: canvas} + cinfo.setAlive() + canvasesLock.Lock() + canvases[obj] = cinfo + canvasesLock.Unlock() +} + +type canvasInfo struct { + expiringCache + canvas fyne.Canvas +} diff --git a/internal/cache/texture_common.go b/internal/cache/texture_common.go new file mode 100644 index 0000000000..84201d4843 --- /dev/null +++ b/internal/cache/texture_common.go @@ -0,0 +1,39 @@ +package cache + +import ( + "sync" + + "fyne.io/fyne/v2" +) + +var texturesLock sync.RWMutex +var textures = make(map[fyne.CanvasObject]*textureInfo, 1024) + +// DestroyTexture destroys cached texture. +func DestroyTexture(obj fyne.CanvasObject) { + texturesLock.Lock() + delete(textures, obj) + texturesLock.Unlock() +} + +// GetTexture gets cached texture. +func GetTexture(obj fyne.CanvasObject) (TextureType, bool) { + texturesLock.RLock() + texInfo, ok := textures[obj] + texturesLock.RUnlock() + if texInfo == nil || !ok { + return noTexture, false + } + texInfo.setAlive() + return texInfo.texture, true +} + +// SetTexture sets cached texture. +func SetTexture(obj fyne.CanvasObject, texture TextureType, freeFn func(obj fyne.CanvasObject)) { + texInfo := &textureInfo{texture: texture} + texInfo.freeFn = freeFn + texInfo.setAlive() + texturesLock.Lock() + textures[obj] = texInfo + texturesLock.Unlock() +} diff --git a/internal/cache/texture_desktop.go b/internal/cache/texture_desktop.go new file mode 100644 index 0000000000..fbd85d17b7 --- /dev/null +++ b/internal/cache/texture_desktop.go @@ -0,0 +1,18 @@ +// +build !android,!ios,!mobile + +package cache + +import ( + "fyne.io/fyne/v2" +) + +// TextureType represents an uploaded GL texture +type TextureType = uint32 + +var noTexture = TextureType(0) + +type textureInfo struct { + expiringCache + texture TextureType + freeFn func(obj fyne.CanvasObject) +} diff --git a/internal/cache/texture_gomobile.go b/internal/cache/texture_gomobile.go new file mode 100644 index 0000000000..231bfb74ca --- /dev/null +++ b/internal/cache/texture_gomobile.go @@ -0,0 +1,20 @@ +// +build android ios mobile + +package cache + +import ( + "fyne.io/fyne/v2" + + "github.com/fyne-io/mobile/gl" +) + +// TextureType represents an uploaded GL texture +type TextureType = gl.Texture + +var noTexture = gl.Texture{0} + +type textureInfo struct { + expiringCache + texture TextureType + freeFn func(obj fyne.CanvasObject) +} diff --git a/internal/cache/widget.go b/internal/cache/widget.go index e705df680e..ed21c3cab3 100644 --- a/internal/cache/widget.go +++ b/internal/cache/widget.go @@ -6,7 +6,8 @@ import ( "fyne.io/fyne/v2" ) -var renderers sync.Map +var renderersLock sync.RWMutex +var renderers = map[fyne.Widget]*rendererInfo{} type isBaseWidget interface { ExtendBaseWidget(fyne.Widget) @@ -24,16 +25,24 @@ func Renderer(wid fyne.Widget) fyne.WidgetRenderer { wid = wd.super() } } - renderer, ok := renderers.Load(wid) + + renderersLock.RLock() + rinfo, ok := renderers[wid] + renderersLock.RUnlock() if !ok { - renderer = wid.CreateRenderer() - renderers.Store(wid, renderer) + rinfo = &rendererInfo{renderer: wid.CreateRenderer()} + renderersLock.Lock() + renderers[wid] = rinfo + renderersLock.Unlock() } - if renderer == nil { + if rinfo == nil { return nil } - return renderer.(fyne.WidgetRenderer) + + rinfo.setAlive() + + return rinfo.renderer } // DestroyRenderer frees a render implementation for a widget. @@ -41,12 +50,21 @@ func Renderer(wid fyne.Widget) fyne.WidgetRenderer { func DestroyRenderer(wid fyne.Widget) { Renderer(wid).Destroy() - renderers.Delete(wid) + renderersLock.Lock() + delete(renderers, wid) + renderersLock.Unlock() } // IsRendered returns true of the widget currently has a renderer. // One will be created the first time a widget is shown but may be removed after it is hidden. func IsRendered(wid fyne.Widget) bool { - _, found := renderers.Load(wid) + renderersLock.RLock() + _, found := renderers[wid] + renderersLock.RUnlock() return found } + +type rendererInfo struct { + expiringCache + renderer fyne.WidgetRenderer +} diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index ae6c305fe2..a616c04518 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -325,9 +325,7 @@ func (c *glCanvas) ensureMinSize() bool { windowNeedsMinSizeUpdate := false ensureMinSize := func(node *renderCacheNode) { obj := node.obj - canvasMutex.Lock() - canvases[obj] = c - canvasMutex.Unlock() + cache.SetCanvasForObject(obj, c) if !obj.Visible() { return @@ -471,6 +469,9 @@ func (c *glCanvas) setDirty(dirty bool) { defer c.dirtyMutex.Unlock() c.dirty = dirty + if dirty { + cache.Clean() + } } func (c *glCanvas) setMenuOverlay(b fyne.CanvasObject) { diff --git a/internal/driver/glfw/driver.go b/internal/driver/glfw/driver.go index 7eef21173b..cc524df4dd 100644 --- a/internal/driver/glfw/driver.go +++ b/internal/driver/glfw/driver.go @@ -11,6 +11,7 @@ import ( "fyne.io/fyne/v2" "fyne.io/fyne/v2/internal/animation" + "fyne.io/fyne/v2/internal/cache" "fyne.io/fyne/v2/internal/driver" "fyne.io/fyne/v2/internal/painter" intRepo "fyne.io/fyne/v2/internal/repository" @@ -20,9 +21,7 @@ import ( const mainGoroutineID = 1 var ( - canvasMutex sync.RWMutex - canvases = make(map[fyne.CanvasObject]fyne.Canvas) - isWayland = false + isWayland = false ) // Declare conformity with Driver @@ -43,9 +42,7 @@ func (d *gLDriver) RenderedTextSize(text string, size float32, style fyne.TextSt } func (d *gLDriver) CanvasForObject(obj fyne.CanvasObject) fyne.Canvas { - canvasMutex.RLock() - defer canvasMutex.RUnlock() - return canvases[obj] + return cache.GetCanvasForObject(obj) } func (d *gLDriver) AbsolutePositionForObject(co fyne.CanvasObject) fyne.Position { diff --git a/internal/painter/gl/draw.go b/internal/painter/gl/draw.go index d3fea7841a..782e0f4400 100644 --- a/internal/painter/gl/draw.go +++ b/internal/painter/gl/draw.go @@ -11,7 +11,7 @@ import ( func (p *glPainter) drawTextureWithDetails(o fyne.CanvasObject, creator func(canvasObject fyne.CanvasObject) Texture, pos fyne.Position, size, frame fyne.Size, fill canvas.ImageFill, alpha float32, pad float32) { - texture := getTexture(o, creator) + texture := p.getTexture(o, creator) if texture == NoTexture { return } diff --git a/internal/painter/gl/gl_common.go b/internal/painter/gl/gl_common.go index 03a16b197b..f659f73278 100644 --- a/internal/painter/gl/gl_common.go +++ b/internal/painter/gl/gl_common.go @@ -11,11 +11,10 @@ import ( "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" + "fyne.io/fyne/v2/internal/cache" "fyne.io/fyne/v2/internal/painter" ) -var textures = make(map[fyne.CanvasObject]Texture, 1024) - func logGLError(err uint32) { if err == 0 { return @@ -28,14 +27,14 @@ func logGLError(err uint32) { } } -func getTexture(object fyne.CanvasObject, creator func(canvasObject fyne.CanvasObject) Texture) Texture { - texture, ok := textures[object] +func (p *glPainter) getTexture(object fyne.CanvasObject, creator func(canvasObject fyne.CanvasObject) Texture) Texture { + texture, ok := cache.GetTexture(object) if !ok { - texture = creator(object) - textures[object] = texture + texture = cache.TextureType(creator(object)) + cache.SetTexture(object, texture, p.freeTexture) } - return texture + return Texture(texture) } func (p *glPainter) newGlCircleTexture(obj fyne.CanvasObject) Texture { diff --git a/internal/painter/gl/gl_core.go b/internal/painter/gl/gl_core.go index 34196f8dc8..2a4c9677c9 100644 --- a/internal/painter/gl/gl_core.go +++ b/internal/painter/gl/gl_core.go @@ -12,6 +12,7 @@ import ( "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" + "fyne.io/fyne/v2/internal/cache" "fyne.io/fyne/v2/theme" ) @@ -85,7 +86,7 @@ func (p *glPainter) SetOutputSize(width, height int) { } func (p *glPainter) freeTexture(obj fyne.CanvasObject) { - texture, ok := textures[obj] + texture, ok := cache.GetTexture(obj) if !ok { return } @@ -93,7 +94,7 @@ func (p *glPainter) freeTexture(obj fyne.CanvasObject) { tex := uint32(texture) gl.DeleteTextures(1, &tex) logError() - delete(textures, obj) + cache.DestroyTexture(obj) } func glInit() { diff --git a/internal/painter/gl/gl_es.go b/internal/painter/gl/gl_es.go index 5ff64a4f3b..ab6fbd5b1a 100644 --- a/internal/painter/gl/gl_es.go +++ b/internal/painter/gl/gl_es.go @@ -14,6 +14,7 @@ import ( "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" + "fyne.io/fyne/v2/internal/cache" "fyne.io/fyne/v2/theme" ) @@ -87,7 +88,7 @@ func (p *glPainter) SetOutputSize(width, height int) { } func (p *glPainter) freeTexture(obj fyne.CanvasObject) { - texture, ok := textures[obj] + texture, ok := cache.GetTexture(obj) if !ok { return } @@ -95,7 +96,7 @@ func (p *glPainter) freeTexture(obj fyne.CanvasObject) { tex := uint32(texture) gl.DeleteTextures(1, &tex) logError() - delete(textures, obj) + cache.DestroyTexture(obj) } func glInit() { diff --git a/internal/painter/gl/gl_gomobile.go b/internal/painter/gl/gl_gomobile.go index 52df739b90..0e42bfc245 100644 --- a/internal/painter/gl/gl_gomobile.go +++ b/internal/painter/gl/gl_gomobile.go @@ -13,6 +13,7 @@ import ( "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" + "fyne.io/fyne/v2/internal/cache" "fyne.io/fyne/v2/theme" ) @@ -97,14 +98,14 @@ func (p *glPainter) SetOutputSize(width, height int) { } func (p *glPainter) freeTexture(obj fyne.CanvasObject) { - texture, ok := textures[obj] + texture, ok := cache.GetTexture(obj) if !ok { return } p.glctx().DeleteTexture(gl.Texture(texture)) p.logError() - delete(textures, obj) + cache.DestroyTexture(obj) } func (p *glPainter) compileShader(source string, shaderType gl.Enum) (gl.Shader, error) { From 1b8916c48ac97d8846ffd3dad8ae88557edbe2b0 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Fri, 19 Mar 2021 16:38:21 -0500 Subject: [PATCH 02/17] fix the way textures are destroyed from cache --- internal/cache/base.go | 20 +------------------- internal/cache/texture_common.go | 24 ++++++++++++++++++++---- internal/cache/texture_desktop.go | 5 ----- internal/cache/texture_gomobile.go | 7 +------ internal/driver/glfw/loop.go | 4 ++++ internal/painter/gl/gl_common.go | 2 +- internal/painter/gl/gl_core.go | 2 +- internal/painter/gl/gl_es.go | 2 +- internal/painter/gl/gl_gomobile.go | 2 +- 9 files changed, 30 insertions(+), 38 deletions(-) diff --git a/internal/cache/base.go b/internal/cache/base.go index 06d08f96b1..082e89b0be 100644 --- a/internal/cache/base.go +++ b/internal/cache/base.go @@ -39,25 +39,7 @@ func init() { } // -- Textures clean task - expired = expired[:0] - texturesLock.RLock() - for obj, tinfo := range textures { - if tinfo.isExpired(now) { - expired = append(expired, obj) - } - } - texturesLock.RUnlock() - if len(expired) > 0 { - texturesLock.Lock() - for i, exp := range expired { - if free := textures[exp].freeFn; free != nil { - go free(exp) - } - delete(textures, exp) - expired[i] = nil - } - texturesLock.Unlock() - } + // TODO find a way to clean textures here if excludeCanvases { return diff --git a/internal/cache/texture_common.go b/internal/cache/texture_common.go index 84201d4843..2ba0a6cdcb 100644 --- a/internal/cache/texture_common.go +++ b/internal/cache/texture_common.go @@ -2,6 +2,7 @@ package cache import ( "sync" + "time" "fyne.io/fyne/v2" ) @@ -9,13 +10,29 @@ import ( var texturesLock sync.RWMutex var textures = make(map[fyne.CanvasObject]*textureInfo, 1024) -// DestroyTexture destroys cached texture. -func DestroyTexture(obj fyne.CanvasObject) { +// DeleteTexture deletes the texture from the cache map. +func DeleteTexture(obj fyne.CanvasObject) { texturesLock.Lock() delete(textures, obj) texturesLock.Unlock() } +// RangeExpiredTextures range over the expired textures. +func RangeExpiredTextures(f func(fyne.CanvasObject)) { + now := time.Now() + expired := make([]fyne.CanvasObject, 0, 10) + texturesLock.RLock() + for obj, tinfo := range textures { + if tinfo.isExpired(now) { + expired = append(expired, obj) + } + } + texturesLock.RUnlock() + for _, obj := range expired { + f(obj) + } +} + // GetTexture gets cached texture. func GetTexture(obj fyne.CanvasObject) (TextureType, bool) { texturesLock.RLock() @@ -29,9 +46,8 @@ func GetTexture(obj fyne.CanvasObject) (TextureType, bool) { } // SetTexture sets cached texture. -func SetTexture(obj fyne.CanvasObject, texture TextureType, freeFn func(obj fyne.CanvasObject)) { +func SetTexture(obj fyne.CanvasObject, texture TextureType) { texInfo := &textureInfo{texture: texture} - texInfo.freeFn = freeFn texInfo.setAlive() texturesLock.Lock() textures[obj] = texInfo diff --git a/internal/cache/texture_desktop.go b/internal/cache/texture_desktop.go index fbd85d17b7..11618878d4 100644 --- a/internal/cache/texture_desktop.go +++ b/internal/cache/texture_desktop.go @@ -2,10 +2,6 @@ package cache -import ( - "fyne.io/fyne/v2" -) - // TextureType represents an uploaded GL texture type TextureType = uint32 @@ -14,5 +10,4 @@ var noTexture = TextureType(0) type textureInfo struct { expiringCache texture TextureType - freeFn func(obj fyne.CanvasObject) } diff --git a/internal/cache/texture_gomobile.go b/internal/cache/texture_gomobile.go index 231bfb74ca..9f822741fd 100644 --- a/internal/cache/texture_gomobile.go +++ b/internal/cache/texture_gomobile.go @@ -2,11 +2,7 @@ package cache -import ( - "fyne.io/fyne/v2" - - "github.com/fyne-io/mobile/gl" -) +import "github.com/fyne-io/mobile/gl" // TextureType represents an uploaded GL texture type TextureType = gl.Texture @@ -16,5 +12,4 @@ var noTexture = gl.Texture{0} type textureInfo struct { expiringCache texture TextureType - freeFn func(obj fyne.CanvasObject) } diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index 7ef2682ded..f445619de4 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -8,6 +8,7 @@ import ( "fyne.io/fyne/v2" "fyne.io/fyne/v2/internal" + "fyne.io/fyne/v2/internal/cache" "fyne.io/fyne/v2/internal/driver" "fyne.io/fyne/v2/internal/painter" @@ -238,6 +239,9 @@ func freeDirtyTextures(canvas *glCanvas) { return false } driver.WalkCompleteObjectTree(object, freeWalked, nil) + cache.RangeExpiredTextures(func(co fyne.CanvasObject) { + canvas.painter.Free(co) + }) default: return } diff --git a/internal/painter/gl/gl_common.go b/internal/painter/gl/gl_common.go index f659f73278..6cd7289f76 100644 --- a/internal/painter/gl/gl_common.go +++ b/internal/painter/gl/gl_common.go @@ -32,7 +32,7 @@ func (p *glPainter) getTexture(object fyne.CanvasObject, creator func(canvasObje if !ok { texture = cache.TextureType(creator(object)) - cache.SetTexture(object, texture, p.freeTexture) + cache.SetTexture(object, texture) } return Texture(texture) } diff --git a/internal/painter/gl/gl_core.go b/internal/painter/gl/gl_core.go index 2a4c9677c9..7014894ec3 100644 --- a/internal/painter/gl/gl_core.go +++ b/internal/painter/gl/gl_core.go @@ -94,7 +94,7 @@ func (p *glPainter) freeTexture(obj fyne.CanvasObject) { tex := uint32(texture) gl.DeleteTextures(1, &tex) logError() - cache.DestroyTexture(obj) + cache.DeleteTexture(obj) } func glInit() { diff --git a/internal/painter/gl/gl_es.go b/internal/painter/gl/gl_es.go index ab6fbd5b1a..1b6b89b4d4 100644 --- a/internal/painter/gl/gl_es.go +++ b/internal/painter/gl/gl_es.go @@ -96,7 +96,7 @@ func (p *glPainter) freeTexture(obj fyne.CanvasObject) { tex := uint32(texture) gl.DeleteTextures(1, &tex) logError() - cache.DestroyTexture(obj) + cache.DeleteTexture(obj) } func glInit() { diff --git a/internal/painter/gl/gl_gomobile.go b/internal/painter/gl/gl_gomobile.go index 0e42bfc245..fd0df47384 100644 --- a/internal/painter/gl/gl_gomobile.go +++ b/internal/painter/gl/gl_gomobile.go @@ -105,7 +105,7 @@ func (p *glPainter) freeTexture(obj fyne.CanvasObject) { p.glctx().DeleteTexture(gl.Texture(texture)) p.logError() - cache.DestroyTexture(obj) + cache.DeleteTexture(obj) } func (p *glPainter) compileShader(source string, shaderType gl.Enum) (gl.Shader, error) { From c1c52b6ff6d4aafbae3e8822012f283dd9b07f56 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Sat, 20 Mar 2021 20:44:13 -0500 Subject: [PATCH 03/17] remove texture cache mutex --- internal/cache/base.go | 8 ++++--- internal/cache/texture_common.go | 37 +++++++++++--------------------- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/internal/cache/base.go b/internal/cache/base.go index 082e89b0be..77f69e2a27 100644 --- a/internal/cache/base.go +++ b/internal/cache/base.go @@ -7,6 +7,8 @@ import ( "fyne.io/fyne/v2" ) +const cacheDuration = 1 * time.Minute + var cleanCh = make(chan struct{}, 1) var cleanTaskOnce sync.Once @@ -64,14 +66,14 @@ func init() { } } - t := time.NewTicker(time.Minute) + t := time.NewTicker(cacheDuration) for { select { case <-cleanCh: cleanTask(false) // do not trigger another clean task so fast time.Sleep(10 * time.Second) - t.Reset(time.Minute) + t.Reset(cacheDuration) case <-t.C: // canvases cache can't be invalidated using the ticker // because if we do it, there wouldn't be a way to recover them later @@ -110,7 +112,7 @@ func (c *expiringCache) isExpired(now time.Time) bool { // setAlive updates expiration time. func (c *expiringCache) setAlive() { - t := time.Now().Add(1 * time.Minute) + t := time.Now().Add(cacheDuration) c.expireLock.Lock() c.expires = t c.expireLock.Unlock() diff --git a/internal/cache/texture_common.go b/internal/cache/texture_common.go index 2ba0a6cdcb..b9030f43b7 100644 --- a/internal/cache/texture_common.go +++ b/internal/cache/texture_common.go @@ -1,43 +1,24 @@ package cache import ( - "sync" "time" "fyne.io/fyne/v2" ) -var texturesLock sync.RWMutex +// NOTE: Texture cache functions should be called always in +// the same go routine. + var textures = make(map[fyne.CanvasObject]*textureInfo, 1024) // DeleteTexture deletes the texture from the cache map. func DeleteTexture(obj fyne.CanvasObject) { - texturesLock.Lock() delete(textures, obj) - texturesLock.Unlock() -} - -// RangeExpiredTextures range over the expired textures. -func RangeExpiredTextures(f func(fyne.CanvasObject)) { - now := time.Now() - expired := make([]fyne.CanvasObject, 0, 10) - texturesLock.RLock() - for obj, tinfo := range textures { - if tinfo.isExpired(now) { - expired = append(expired, obj) - } - } - texturesLock.RUnlock() - for _, obj := range expired { - f(obj) - } } // GetTexture gets cached texture. func GetTexture(obj fyne.CanvasObject) (TextureType, bool) { - texturesLock.RLock() texInfo, ok := textures[obj] - texturesLock.RUnlock() if texInfo == nil || !ok { return noTexture, false } @@ -45,11 +26,19 @@ func GetTexture(obj fyne.CanvasObject) (TextureType, bool) { return texInfo.texture, true } +// RangeExpiredTextures range over the expired textures. +func RangeExpiredTextures(f func(fyne.CanvasObject)) { + now := time.Now() + for obj, tinfo := range textures { + if tinfo.isExpired(now) { + f(obj) + } + } +} + // SetTexture sets cached texture. func SetTexture(obj fyne.CanvasObject, texture TextureType) { texInfo := &textureInfo{texture: texture} texInfo.setAlive() - texturesLock.Lock() textures[obj] = texInfo - texturesLock.Unlock() } From 32b0d4533bac6297a445f021071aa5b0e95fb951 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Sun, 21 Mar 2021 13:49:53 -0500 Subject: [PATCH 04/17] use draw events to run the clean task instead of creating a new go routine, added cache.NotifyCanvasRefresh, fix the way textures are freed, fix memory leak on window.Close --- internal/cache/base.go | 202 ++++++++++++++++++----------- internal/cache/texture_common.go | 44 ++++++- internal/cache/texture_desktop.go | 2 +- internal/cache/texture_gomobile.go | 2 +- internal/driver/glfw/canvas.go | 2 +- internal/driver/glfw/loop.go | 7 +- internal/driver/glfw/window.go | 16 ++- internal/painter/gl/gl_common.go | 2 +- 8 files changed, 185 insertions(+), 92 deletions(-) diff --git a/internal/cache/base.go b/internal/cache/base.go index 77f69e2a27..28fb355dc3 100644 --- a/internal/cache/base.go +++ b/internal/cache/base.go @@ -7,95 +7,139 @@ import ( "fyne.io/fyne/v2" ) -const cacheDuration = 1 * time.Minute - -var cleanCh = make(chan struct{}, 1) -var cleanTaskOnce sync.Once - -func init() { - - cleanTaskOnce.Do(func() { - - go func() { - expired := make([]fyne.CanvasObject, 0, 50) - - cleanTask := func(excludeCanvases bool) { - now := time.Now() - // -- Renderers clean task - expired = expired[:0] - renderersLock.RLock() - for wid, rinfo := range renderers { - if rinfo.isExpired(now) { - rinfo.renderer.Destroy() - expired = append(expired, wid) - } - } - renderersLock.RUnlock() - if len(expired) > 0 { - renderersLock.Lock() - for i, exp := range expired { - delete(renderers, exp.(fyne.Widget)) - expired[i] = nil - } - renderersLock.Unlock() - } - - // -- Textures clean task - // TODO find a way to clean textures here - - if excludeCanvases { - return - } - - // -- Canvases clean task - expired = expired[:0] - canvasesLock.RLock() - for obj, cinfo := range canvases { - if cinfo.isExpired(now) { - expired = append(expired, obj) - } - } - canvasesLock.RUnlock() - if len(expired) > 0 { - canvasesLock.Lock() - for i, exp := range expired { - delete(canvases, exp) - expired[i] = nil - } - canvasesLock.Unlock() - } - } - - t := time.NewTicker(cacheDuration) - for { - select { - case <-cleanCh: - cleanTask(false) - // do not trigger another clean task so fast - time.Sleep(10 * time.Second) - t.Reset(cacheDuration) - case <-t.C: - // canvases cache can't be invalidated using the ticker - // because if we do it, there wouldn't be a way to recover them later - cleanTask(true) - } - } - }() - }) +const ( + cacheDuration = 1 * time.Minute + cleanTaskInterval = cacheDuration / 2 +) + +var canvasRefreshCh = make(chan struct{}, 1) +var expiredObjects = make([]fyne.CanvasObject, 0, 50) +var lastClean time.Time +// CleanTask run cache clean task, it should be called on paint events. +func CleanTask() { + now := time.Now() + // do not run clean task so fast + if now.Sub(lastClean) < 10*time.Second { + return + } + canvasRefresh := false + select { + case <-canvasRefreshCh: + canvasRefresh = true + default: + if now.Sub(lastClean) < cleanTaskInterval { + return + } + } + destroyExpiredRenderers(now) + // canvases cache should be invalidated only on canvas refresh, otherwise there wouldn't + // be a way to recover them later + if canvasRefresh { + destroyExpiredCanvases(now) + } + lastClean = time.Now() } -// Clean triggers clean cache task. -func Clean() { +// ForceCleanFor forces a complete remove of all the objects that belong to the specified +// canvas. Usually used to free all objects from a closing windows. +func ForceCleanFor(canvas fyne.Canvas) { + deletingObjs := make([]fyne.CanvasObject, 0, 50) + + // find all objects that belong to the specified canvas + canvasesLock.RLock() + for obj, cinfo := range canvases { + if cinfo.canvas == canvas { + deletingObjs = append(deletingObjs, obj) + } + } + canvasesLock.RUnlock() + if len(deletingObjs) == 0 { + return + } + + // remove them from canvas cache + canvasesLock.Lock() + for _, dobj := range deletingObjs { + delete(canvases, dobj) + } + canvasesLock.Unlock() + + // destroy their renderers and delete them from the renderer + // cache if they are widgets + renderersLock.Lock() + for _, dobj := range deletingObjs { + wid, ok := dobj.(fyne.Widget) + if !ok { + continue + } + winfo, ok := renderers[wid] + if !ok { + continue + } + winfo.renderer.Destroy() + delete(renderers, wid) + } + renderersLock.Unlock() +} + +// NotifyCanvasRefresh notifies to the caches that a canvas refresh was triggered. +func NotifyCanvasRefresh() { select { - case cleanCh <- struct{}{}: + case canvasRefreshCh <- struct{}{}: default: return } } // =============================================================== -// Privates +// Private functions +// =============================================================== + +// destroyExpiredCanvases deletes objects from the canvases cache. +func destroyExpiredCanvases(now time.Time) { + expiredObjects = expiredObjects[:0] + canvasesLock.RLock() + for obj, cinfo := range canvases { + if cinfo.isExpired(now) { + expiredObjects = append(expiredObjects, obj) + } + } + canvasesLock.RUnlock() + if len(expiredObjects) > 0 { + canvasesLock.Lock() + for i, exp := range expiredObjects { + delete(canvases, exp) + expiredObjects[i] = nil + } + canvasesLock.Unlock() + } +} + +// destroyExpiredRenderers deletes the renderer from the cache and calls +// renderer.Destroy() +func destroyExpiredRenderers(now time.Time) { + expiredObjects = expiredObjects[:0] + renderersLock.RLock() + for wid, rinfo := range renderers { + if rinfo.isExpired(now) { + rinfo.renderer.Destroy() + expiredObjects = append(expiredObjects, wid) + } + } + renderersLock.RUnlock() + if len(expiredObjects) > 0 { + renderersLock.Lock() + for i, exp := range expiredObjects { + delete(renderers, exp.(fyne.Widget)) + expiredObjects[i] = nil + } + renderersLock.Unlock() + } +} + +// =============================================================== +// Private types // =============================================================== type expiringCache struct { diff --git a/internal/cache/texture_common.go b/internal/cache/texture_common.go index b9030f43b7..241db4e90e 100644 --- a/internal/cache/texture_common.go +++ b/internal/cache/texture_common.go @@ -26,19 +26,55 @@ func GetTexture(obj fyne.CanvasObject) (TextureType, bool) { return texInfo.texture, true } -// RangeExpiredTextures range over the expired textures. -func RangeExpiredTextures(f func(fyne.CanvasObject)) { +// RangeExpiredTexturesFor range over the expired textures for the specified canvas. +// +// Note: If this is used to free textures, then it should be called inside a current +// gl context to ensure textures are deleted from gl. +func RangeExpiredTexturesFor(canvas fyne.Canvas, f func(fyne.CanvasObject)) { now := time.Now() for obj, tinfo := range textures { - if tinfo.isExpired(now) { + if tinfo.isExpired(now) && tinfo.canvas == canvas { + f(obj) + } + } +} + +// RangeTexturesFor range over the textures for the specified canvas. +// +// Note: If this is used to free textures, then it should be called inside a current +// gl context to ensure textures are deleted from gl. +func RangeTexturesFor(canvas fyne.Canvas, f func(fyne.CanvasObject)) { + for obj, tinfo := range textures { + if tinfo.canvas == canvas { f(obj) } } } // SetTexture sets cached texture. -func SetTexture(obj fyne.CanvasObject, texture TextureType) { +func SetTexture(obj fyne.CanvasObject, texture TextureType, canvas fyne.Canvas) { texInfo := &textureInfo{texture: texture} + texInfo.canvas = canvas texInfo.setAlive() textures[obj] = texInfo } + +// =============================================================== +// Privates +// =============================================================== + +// textureCacheBase defines base texture cache object. +type textureCacheBase struct { + expires time.Time + canvas fyne.Canvas +} + +// isExpired check if the cache data is expired. +func (c *textureCacheBase) isExpired(now time.Time) bool { + return c.expires.Before(now) +} + +// setAlive updates expiration time. +func (c *textureCacheBase) setAlive() { + c.expires = time.Now().Add(cacheDuration) +} diff --git a/internal/cache/texture_desktop.go b/internal/cache/texture_desktop.go index 11618878d4..cb03a923e0 100644 --- a/internal/cache/texture_desktop.go +++ b/internal/cache/texture_desktop.go @@ -8,6 +8,6 @@ type TextureType = uint32 var noTexture = TextureType(0) type textureInfo struct { - expiringCache + textureCacheBase texture TextureType } diff --git a/internal/cache/texture_gomobile.go b/internal/cache/texture_gomobile.go index 9f822741fd..33bdcab16a 100644 --- a/internal/cache/texture_gomobile.go +++ b/internal/cache/texture_gomobile.go @@ -10,6 +10,6 @@ type TextureType = gl.Texture var noTexture = gl.Texture{0} type textureInfo struct { - expiringCache + textureCacheBase texture TextureType } diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index a616c04518..c3e36cb4be 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -470,7 +470,7 @@ func (c *glCanvas) setDirty(dirty bool) { c.dirty = dirty if dirty { - cache.Clean() + cache.NotifyCanvasRefresh() } } diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index f445619de4..5068e78f63 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -215,6 +215,7 @@ func (d *gLDriver) startDrawThread() { d.repaintWindow(w) } + cache.CleanTask() } } }() @@ -239,10 +240,10 @@ func freeDirtyTextures(canvas *glCanvas) { return false } driver.WalkCompleteObjectTree(object, freeWalked, nil) - cache.RangeExpiredTextures(func(co fyne.CanvasObject) { - canvas.painter.Free(co) - }) default: + cache.RangeExpiredTexturesFor(canvas, func(obj fyne.CanvasObject) { + canvas.painter.Free(obj) + }) return } } diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index 2e1c4418f3..891bc1d252 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -429,9 +429,16 @@ func (w *window) Close() { return } - w.closing = true - w.viewport.SetShouldClose(true) + // set w.closing flag inside draw thread to ensure we can free textures + runOnDraw(w, func() { + w.closing = true + w.viewport.SetShouldClose(true) + cache.RangeTexturesFor(w.canvas, func(obj fyne.CanvasObject) { + w.canvas.painter.Free(obj) + }) + }) + // destroy current renderers w.canvas.walkTrees(nil, func(node *renderCacheNode) { switch co := node.obj.(type) { case fyne.Widget: @@ -439,6 +446,11 @@ func (w *window) Close() { } }) + // remove all canvas objects from the cache + w.queueEvent(func() { + cache.ForceCleanFor(w.canvas) + }) + // trigger callbacks if w.onClosed != nil { w.queueEvent(w.onClosed) diff --git a/internal/painter/gl/gl_common.go b/internal/painter/gl/gl_common.go index 6cd7289f76..ee07a1ef30 100644 --- a/internal/painter/gl/gl_common.go +++ b/internal/painter/gl/gl_common.go @@ -32,7 +32,7 @@ func (p *glPainter) getTexture(object fyne.CanvasObject, creator func(canvasObje if !ok { texture = cache.TextureType(creator(object)) - cache.SetTexture(object, texture) + cache.SetTexture(object, texture, p.canvas) } return Texture(texture) } From 801d2710db75f34943d3babdd6c9b704d009ad47 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Thu, 25 Mar 2021 12:13:01 -0500 Subject: [PATCH 05/17] move svg_cache to cache package, improve mobile driver by listening to theme changes and remove 10ms delay on every paint event --- cmd/fyne_settings/settings/appearance.go | 6 +- internal/cache/base.go | 45 ++++- internal/cache/svg.go | 47 +++++ .../svg_cache_test.go => cache/svg_test.go} | 41 ++-- internal/cache/texture_common.go | 14 +- internal/driver/glfw/loop.go | 1 + internal/driver/gomobile/driver.go | 181 ++++++++++-------- internal/painter/gl/painter.go | 7 - internal/painter/image.go | 23 ++- internal/painter/svg_cache.go | 114 ----------- test/testapp.go | 6 +- 11 files changed, 239 insertions(+), 246 deletions(-) create mode 100644 internal/cache/svg.go rename internal/{painter/svg_cache_test.go => cache/svg_test.go} (58%) delete mode 100644 internal/painter/svg_cache.go diff --git a/cmd/fyne_settings/settings/appearance.go b/cmd/fyne_settings/settings/appearance.go index 54d4a798fb..ac39b8a739 100644 --- a/cmd/fyne_settings/settings/appearance.go +++ b/cmd/fyne_settings/settings/appearance.go @@ -14,7 +14,7 @@ import ( "fyne.io/fyne/v2/app" "fyne.io/fyne/v2/canvas" "fyne.io/fyne/v2/container" - "fyne.io/fyne/v2/internal/painter" + "fyne.io/fyne/v2/internal/cache" "fyne.io/fyne/v2/layout" "fyne.io/fyne/v2/theme" "fyne.io/fyne/v2/tools/playground" @@ -118,7 +118,7 @@ func (s *Settings) createPreview() image.Image { th = theme.DarkTheme() } - painter.SvgCacheReset() // reset icon cache + cache.ResetSvg() // reset icon cache fyne.CurrentApp().Settings().(overrideTheme).OverrideTheme(th, s.fyneSettings.PrimaryColor) empty := widget.NewLabel("") @@ -136,7 +136,7 @@ func (s *Settings) createPreview() image.Image { time.Sleep(canvas.DurationShort) img := c.Capture() - painter.SvgCacheReset() // ensure we re-create the correct cached assets + cache.ResetSvg() // ensure we re-create the correct cached assets fyne.CurrentApp().Settings().(overrideTheme).OverrideTheme(oldTheme, oldColor) return img } diff --git a/internal/cache/base.go b/internal/cache/base.go index 28fb355dc3..d3569abae9 100644 --- a/internal/cache/base.go +++ b/internal/cache/base.go @@ -1,13 +1,14 @@ package cache import ( + "os" "sync" "time" "fyne.io/fyne/v2" ) -const ( +var ( cacheDuration = 1 * time.Minute cleanTaskInterval = cacheDuration / 2 ) @@ -16,6 +17,13 @@ var canvasRefreshCh = make(chan struct{}, 1) var expiredObjects = make([]fyne.CanvasObject, 0, 50) var lastClean time.Time +func init() { + if t, err := time.ParseDuration(os.Getenv("FYNE_CACHE")); err == nil { + cacheDuration = t + cleanTaskInterval = cacheDuration / 2 + } +} + // CleanTask run cache clean task, it should be called on paint events. func CleanTask() { now := time.Now() @@ -33,6 +41,7 @@ func CleanTask() { } } destroyExpiredRenderers(now) + destroyExpiredSvgs(now) // canvases cache should be invalidated only on canvas refresh, otherwise there wouldn't // be a way to recover them later if canvasRefresh { @@ -138,6 +147,25 @@ func destroyExpiredRenderers(now time.Time) { } } +// destroyExpiredSvgs destroys expired svgs cache data. +func destroyExpiredSvgs(now time.Time) { + expiredSvgs := make([]string, 0, 20) + svgLock.RLock() + for s, sinfo := range svgs { + if sinfo.isExpired(now) { + expiredSvgs = append(expiredSvgs, s) + } + } + svgLock.RUnlock() + if len(expiredSvgs) > 0 { + svgLock.Lock() + for _, exp := range expiredSvgs { + delete(svgs, exp) + } + svgLock.Unlock() + } +} + // =============================================================== // Private types // =============================================================== @@ -161,3 +189,18 @@ func (c *expiringCache) setAlive() { c.expires = t c.expireLock.Unlock() } + +type expiringCacheNoLock struct { + expires time.Time +} + +// isExpired check if the cache data is expired. +func (c *expiringCacheNoLock) isExpired(now time.Time) bool { + return c.expires.Before(now) +} + +// setAlive updates expiration time. +func (c *expiringCacheNoLock) setAlive() { + t := time.Now().Add(cacheDuration) + c.expires = t +} diff --git a/internal/cache/svg.go b/internal/cache/svg.go new file mode 100644 index 0000000000..d479154a2b --- /dev/null +++ b/internal/cache/svg.go @@ -0,0 +1,47 @@ +package cache + +import ( + "image" + "sync" +) + +var svgLock sync.RWMutex +var svgs = make(map[string]*svgInfo) + +// GetSvg gets svg image from cache if it exists. +func GetSvg(name string, w int, h int) *image.NRGBA { + svgLock.RLock() + sinfo, ok := svgs[name] + svgLock.RUnlock() + if !ok || sinfo == nil || sinfo.w != w || sinfo.h != h { + return nil + } + sinfo.setAlive() + return sinfo.pix +} + +// ResetSvg clears all the svg cache map +func ResetSvg() { + svgLock.Lock() + svgs = make(map[string]*svgInfo) + svgLock.Unlock() +} + +// SetSvg sets a svg into the cache map. +func SetSvg(name string, pix *image.NRGBA, w int, h int) { + sinfo := &svgInfo{ + pix: pix, + w: w, + h: h, + } + sinfo.setAlive() + svgLock.Lock() + svgs[name] = sinfo + svgLock.Unlock() +} + +type svgInfo struct { + expiringCacheNoLock + pix *image.NRGBA + w, h int +} diff --git a/internal/painter/svg_cache_test.go b/internal/cache/svg_test.go similarity index 58% rename from internal/painter/svg_cache_test.go rename to internal/cache/svg_test.go index 72c34763d7..cc5d9da7bb 100644 --- a/internal/painter/svg_cache_test.go +++ b/internal/cache/svg_test.go @@ -1,58 +1,61 @@ -package painter +package cache import ( "image" "testing" - "github.com/stretchr/testify/assert" - "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" + "github.com/stretchr/testify/assert" ) func TestSvgCacheGet(t *testing.T) { - SvgCacheReset() + ResetSvg() img := addToCache("empty.svg", "", 25, 25) - assert.Equal(t, 1, len(rasters)) + assert.Equal(t, 1, len(svgs)) - newImg := svgCacheGet("empty.svg", 25, 25) + newImg := GetSvg("empty.svg", 25, 25) assert.Equal(t, img, newImg) - miss := svgCacheGet("missing.svg", 25, 25) + miss := GetSvg("missing.svg", 25, 25) assert.Nil(t, miss) - miss = svgCacheGet("empty.svg", 30, 30) + miss = GetSvg("empty.svg", 30, 30) assert.Nil(t, miss) } func TestSvgCacheGet_File(t *testing.T) { - SvgCacheReset() + ResetSvg() img := addFileToCache("testdata/stroke.svg", 25, 25) - assert.Equal(t, 1, len(rasters)) + assert.Equal(t, 1, len(svgs)) - newImg := svgCacheGet("testdata/stroke.svg", 25, 25) + newImg := GetSvg("testdata/stroke.svg", 25, 25) assert.Equal(t, img, newImg) - miss := svgCacheGet("missing.svg", 25, 25) + miss := GetSvg("missing.svg", 25, 25) assert.Nil(t, miss) - miss = svgCacheGet("testdata/stroke.svg", 30, 30) + miss = GetSvg("testdata/stroke.svg", 30, 30) assert.Nil(t, miss) } func TestSvgCacheReset(t *testing.T) { - SvgCacheReset() + ResetSvg() _ = addToCache("empty.svg", "", 25, 25) - assert.Equal(t, 1, len(rasters)) + assert.Equal(t, 1, len(svgs)) - SvgCacheReset() - assert.Equal(t, 0, len(rasters)) + ResetSvg() + assert.Equal(t, 0, len(svgs)) } func addFileToCache(path string, w, h int) image.Image { img := canvas.NewImageFromFile(path) - return PaintImage(img, nil, w, h) + tex := image.NewNRGBA(image.Rect(0, 0, w, h)) + SetSvg(img.File, tex, w, h) + return tex } func addToCache(name, content string, w, h int) image.Image { img := canvas.NewImageFromResource(fyne.NewStaticResource(name, []byte(content))) - return PaintImage(img, nil, w, h) + tex := image.NewNRGBA(image.Rect(0, 0, w, h)) + SetSvg(img.Resource.Name(), tex, w, h) + return tex } diff --git a/internal/cache/texture_common.go b/internal/cache/texture_common.go index 241db4e90e..32ca0582f1 100644 --- a/internal/cache/texture_common.go +++ b/internal/cache/texture_common.go @@ -65,16 +65,6 @@ func SetTexture(obj fyne.CanvasObject, texture TextureType, canvas fyne.Canvas) // textureCacheBase defines base texture cache object. type textureCacheBase struct { - expires time.Time - canvas fyne.Canvas -} - -// isExpired check if the cache data is expired. -func (c *textureCacheBase) isExpired(now time.Time) bool { - return c.expires.Before(now) -} - -// setAlive updates expiration time. -func (c *textureCacheBase) setAlive() { - c.expires = time.Now().Add(cacheDuration) + expiringCacheNoLock + canvas fyne.Canvas } diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index 5068e78f63..93da73ed42 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -198,6 +198,7 @@ func (d *gLDriver) startDrawThread() { } case <-settingsChange: painter.ClearFontCache() + cache.ResetSvg() for _, win := range d.windowList() { go win.Canvas().(*glCanvas).reloadScale() } diff --git a/internal/driver/gomobile/driver.go b/internal/driver/gomobile/driver.go index 4a428f54e7..71038b3370 100644 --- a/internal/driver/gomobile/driver.go +++ b/internal/driver/gomobile/driver.go @@ -17,6 +17,7 @@ import ( "fyne.io/fyne/v2/canvas" "fyne.io/fyne/v2/internal" "fyne.io/fyne/v2/internal/animation" + "fyne.io/fyne/v2/internal/cache" "fyne.io/fyne/v2/internal/driver" "fyne.io/fyne/v2/internal/painter" pgl "fyne.io/fyne/v2/internal/painter/gl" @@ -113,100 +114,110 @@ func (d *mobileDriver) Quit() { func (d *mobileDriver) Run() { app.Main(func(a app.App) { d.app = a - var currentSize size.Event - for e := range a.Events() { - current := d.currentWindow() - if current == nil { - continue - } - canvas := current.Canvas().(*mobileCanvas) - - switch e := a.Filter(e).(type) { - case lifecycle.Event: - switch e.Crosses(lifecycle.StageVisible) { - case lifecycle.CrossOn: - d.glctx, _ = e.DrawContext.(gl.Context) - d.onStart() - - // this is a fix for some android phone to prevent the app from being drawn as a blank screen after being pushed in the background - canvas.Content().Refresh() + settingsChange := make(chan fyne.Settings) + fyne.CurrentApp().Settings().AddChangeListener(settingsChange) + draw := time.NewTicker(time.Second / 60) - a.Send(paint.Event{}) - case lifecycle.CrossOff: - d.onStop() - d.glctx = nil + for { + select { + case <-draw.C: + a.Send(paint.Event{}) + case <-settingsChange: + painter.ClearFontCache() + cache.ResetSvg() + a.Send(paint.Event{}) + case e := <-a.Events(): + current := d.currentWindow() + if current == nil { + continue } - switch e.Crosses(lifecycle.StageFocused) { - case lifecycle.CrossOff: // will enter background - if runtime.GOOS == "darwin" { - if d.glctx == nil { - continue + canvas := current.Canvas().(*mobileCanvas) + + switch e := a.Filter(e).(type) { + case lifecycle.Event: + switch e.Crosses(lifecycle.StageVisible) { + case lifecycle.CrossOn: + d.glctx, _ = e.DrawContext.(gl.Context) + d.onStart() + + // this is a fix for some android phone to prevent the app from being drawn as a blank screen after being pushed in the background + canvas.Content().Refresh() + + a.Send(paint.Event{}) + case lifecycle.CrossOff: + d.onStop() + d.glctx = nil + } + switch e.Crosses(lifecycle.StageFocused) { + case lifecycle.CrossOff: // will enter background + if runtime.GOOS == "darwin" { + if d.glctx == nil { + continue + } + + size := fyne.NewSize(float32(currentSize.WidthPx)/canvas.scale, float32(currentSize.HeightPx)/canvas.scale) + d.paintWindow(current, size) + a.Publish() } - - size := fyne.NewSize(float32(currentSize.WidthPx)/canvas.scale, float32(currentSize.HeightPx)/canvas.scale) - d.paintWindow(current, size) - a.Publish() } - } - case size.Event: - if e.WidthPx <= 0 { - continue - } - currentSize = e - currentOrientation = e.Orientation - currentDPI = e.PixelsPerPt * 72 - - dev := d.device - dev.safeTop = e.InsetTopPx - dev.safeLeft = e.InsetLeftPx - dev.safeHeight = e.HeightPx - e.InsetTopPx - e.InsetBottomPx - dev.safeWidth = e.WidthPx - e.InsetLeftPx - e.InsetRightPx - canvas.scale = fyne.CurrentDevice().SystemScaleForWindow(nil) - canvas.painter.SetFrameBufferScale(1.0) - - // make sure that we paint on the next frame - canvas.Content().Refresh() - case paint.Event: - if d.glctx == nil || e.External { - continue - } - if !canvas.inited { - canvas.inited = true - canvas.painter.Init() // we cannot init until the context is set above - } - - if d.freeDirtyTextures(canvas) { - newSize := fyne.NewSize(float32(currentSize.WidthPx)/canvas.scale, float32(currentSize.HeightPx)/canvas.scale) + case size.Event: + if e.WidthPx <= 0 { + continue + } + currentSize = e + currentOrientation = e.Orientation + currentDPI = e.PixelsPerPt * 72 + + dev := d.device + dev.safeTop = e.InsetTopPx + dev.safeLeft = e.InsetLeftPx + dev.safeHeight = e.HeightPx - e.InsetTopPx - e.InsetBottomPx + dev.safeWidth = e.WidthPx - e.InsetLeftPx - e.InsetRightPx + canvas.scale = fyne.CurrentDevice().SystemScaleForWindow(nil) + canvas.painter.SetFrameBufferScale(1.0) + + // make sure that we paint on the next frame + canvas.Content().Refresh() + case paint.Event: + if d.glctx == nil || e.External { + continue + } + if !canvas.inited { + canvas.inited = true + canvas.painter.Init() // we cannot init until the context is set above + } - if canvas.minSizeChanged() { - canvas.ensureMinSize() + if d.freeDirtyTextures(canvas) { + newSize := fyne.NewSize(float32(currentSize.WidthPx)/canvas.scale, float32(currentSize.HeightPx)/canvas.scale) - canvas.sizeContent(newSize) // force resize of content - } else { // if screen changed - current.Resize(newSize) - } + if canvas.minSizeChanged() { + canvas.ensureMinSize() - d.paintWindow(current, newSize) - a.Publish() - } + canvas.sizeContent(newSize) // force resize of content + } else { // if screen changed + current.Resize(newSize) + } - time.Sleep(time.Millisecond * 10) - a.Send(paint.Event{}) - case touch.Event: - switch e.Type { - case touch.TypeBegin: - d.tapDownCanvas(canvas, e.X, e.Y, e.Sequence) - case touch.TypeMove: - d.tapMoveCanvas(canvas, e.X, e.Y, e.Sequence) - case touch.TypeEnd: - d.tapUpCanvas(canvas, e.X, e.Y, e.Sequence) - } - case key.Event: - if e.Direction == key.DirPress { - d.typeDownCanvas(canvas, e.Rune, e.Code) - } else if e.Direction == key.DirRelease { - d.typeUpCanvas(canvas, e.Rune, e.Code) + d.paintWindow(current, newSize) + a.Publish() + } + cache.CleanTask() + case touch.Event: + switch e.Type { + case touch.TypeBegin: + d.tapDownCanvas(canvas, e.X, e.Y, e.Sequence) + case touch.TypeMove: + d.tapMoveCanvas(canvas, e.X, e.Y, e.Sequence) + case touch.TypeEnd: + d.tapUpCanvas(canvas, e.X, e.Y, e.Sequence) + } + case key.Event: + if e.Direction == key.DirPress { + d.typeDownCanvas(canvas, e.Rune, e.Code) + } else if e.Direction == key.DirRelease { + d.typeUpCanvas(canvas, e.Rune, e.Code) + } } } } diff --git a/internal/painter/gl/painter.go b/internal/painter/gl/painter.go index bc17a1c965..2ad7519aaf 100644 --- a/internal/painter/gl/painter.go +++ b/internal/painter/gl/painter.go @@ -4,11 +4,9 @@ package gl import ( "image" "math" - "sync" "fyne.io/fyne/v2" "fyne.io/fyne/v2/internal/driver" - "fyne.io/fyne/v2/internal/painter" ) // Painter defines the functionality of our OpenGL based renderer @@ -83,8 +81,6 @@ func (p *glPainter) textureScale(v float32) float32 { return float32(math.Round(float64(v * p.pixScale))) } -var startCacheMonitor = &sync.Once{} - // NewPainter creates a new GL based renderer for the provided canvas. // If it is a master painter it will also initialise OpenGL func NewPainter(c fyne.Canvas, ctx driver.WithContext) Painter { @@ -92,9 +88,6 @@ func NewPainter(c fyne.Canvas, ctx driver.WithContext) Painter { p.SetFrameBufferScale(1.0) glInit() - startCacheMonitor.Do(func() { - go painter.SvgCacheMonitorTheme() - }) return p } diff --git a/internal/painter/image.go b/internal/painter/image.go index bfbc62cd6d..347d443d51 100644 --- a/internal/painter/image.go +++ b/internal/painter/image.go @@ -14,12 +14,31 @@ import ( "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" "fyne.io/fyne/v2/internal" + "fyne.io/fyne/v2/internal/cache" "github.com/srwiley/oksvg" "github.com/srwiley/rasterx" "golang.org/x/image/draw" ) +var aspects = make(map[interface{}]float32, 16) + +// GetAspect looks up an aspect ratio of an image +func GetAspect(img *canvas.Image) float32 { + aspect := float32(0.0) + if img.Resource != nil { + aspect = aspects[img.Resource.Name()] + } else if img.File != "" { + aspect = aspects[img.File] + } + + if aspect == 0 { + aspect = aspects[img] + } + + return aspect +} + // PaintImage renders a given fyne Image to a Go standard image func PaintImage(img *canvas.Image, c fyne.Canvas, width, height int) image.Image { if width <= 0 || height <= 0 { @@ -50,7 +69,7 @@ func PaintImage(img *canvas.Image, c fyne.Canvas, width, height int) image.Image } if isSVG { - tex := svgCacheGet(name, width, height) + tex := cache.GetSvg(name, width, height) if tex == nil { // Not in cache, so load the item and add to cache @@ -91,7 +110,7 @@ func PaintImage(img *canvas.Image, c fyne.Canvas, width, height int) image.Image return nil } - svgCachePut(name, tex, width, height) + cache.SetSvg(name, tex, width, height) } return tex diff --git a/internal/painter/svg_cache.go b/internal/painter/svg_cache.go deleted file mode 100644 index 510c387535..0000000000 --- a/internal/painter/svg_cache.go +++ /dev/null @@ -1,114 +0,0 @@ -package painter - -import ( - "image" - "os" - "sync" - "time" - - "fyne.io/fyne/v2" - "fyne.io/fyne/v2/canvas" -) - -type rasterInfo struct { - pix *image.NRGBA - w, h int - expires time.Time -} - -var cacheDuration = time.Minute * 5 -var rasters = make(map[string]*rasterInfo) -var aspects = make(map[interface{}]float32, 16) -var rasterMutex sync.RWMutex -var janitorOnce sync.Once - -func init() { - if t, err := time.ParseDuration(os.Getenv("FYNE_CACHE")); err == nil { - cacheDuration = t - } - - svgCacheJanitor() -} - -// GetAspect looks up an aspect ratio of an image -func GetAspect(img *canvas.Image) float32 { - aspect := float32(0.0) - if img.Resource != nil { - aspect = aspects[img.Resource.Name()] - } else if img.File != "" { - aspect = aspects[img.File] - } - - if aspect == 0 { - aspect = aspects[img] - } - - return aspect -} - -func svgCacheJanitor() { - delay := cacheDuration / 2 - if delay < time.Second { - delay = time.Second - } - - go janitorOnce.Do(func() { - for { - time.Sleep(delay) - now := time.Now() - rasterMutex.Lock() - for k, v := range rasters { - if v.expires.Before(now) { - delete(rasters, k) - } - } - rasterMutex.Unlock() - } - }) -} - -func svgCacheGet(name string, w int, h int) *image.NRGBA { - rasterMutex.RLock() - defer rasterMutex.RUnlock() - v, ok := rasters[name] - if !ok || v == nil || v.w != w || v.h != h { - return nil - } - v.expires = time.Now().Add(cacheDuration) - return v.pix -} - -func svgCachePut(name string, pix *image.NRGBA, w int, h int) { - rasterMutex.Lock() - defer rasterMutex.Unlock() - defer func() { - recover() - }() - - rasters[name] = &rasterInfo{ - pix: pix, - w: w, - h: h, - expires: time.Now().Add(cacheDuration), - } -} - -// SvgCacheReset clears the SVG cache. -func SvgCacheReset() { - rasterMutex.Lock() - rasters = make(map[string]*rasterInfo) - rasterMutex.Unlock() -} - -// SvgCacheMonitorTheme hooks up the SVG cache to listen for theme changes and resets the cache -func SvgCacheMonitorTheme() { - listener := make(chan fyne.Settings) - fyne.CurrentApp().Settings().AddChangeListener(listener) - go func() { - for { - <-listener - SvgCacheReset() - } - }() - -} diff --git a/test/testapp.go b/test/testapp.go index 729436b916..35121508ef 100644 --- a/test/testapp.go +++ b/test/testapp.go @@ -8,7 +8,7 @@ import ( "fyne.io/fyne/v2" "fyne.io/fyne/v2/internal" "fyne.io/fyne/v2/internal/app" - "fyne.io/fyne/v2/internal/painter" + "fyne.io/fyne/v2/internal/cache" "fyne.io/fyne/v2/theme" ) @@ -94,7 +94,7 @@ func NewApp() fyne.App { settings := &testSettings{scale: 1.0, theme: Theme()} prefs := internal.NewInMemoryPreferences() test := &testApp{settings: settings, prefs: prefs, storage: &testStorage{}, driver: NewDriver().(*testDriver)} - painter.SvgCacheReset() + cache.ResetSvg() fyne.SetCurrentApp(test) listener := make(chan fyne.Settings) @@ -102,7 +102,7 @@ func NewApp() fyne.App { go func() { for { <-listener - painter.SvgCacheReset() + cache.ResetSvg() app.ApplySettings(test.Settings(), test) test.propertyLock.Lock() From 71578553f697ab35ad6b0da529fec3fffbace043 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Thu, 25 Mar 2021 13:40:21 -0500 Subject: [PATCH 06/17] revert mobile driver changes --- internal/driver/gomobile/driver.go | 181 ++++++++++++++--------------- 1 file changed, 85 insertions(+), 96 deletions(-) diff --git a/internal/driver/gomobile/driver.go b/internal/driver/gomobile/driver.go index 71038b3370..4a428f54e7 100644 --- a/internal/driver/gomobile/driver.go +++ b/internal/driver/gomobile/driver.go @@ -17,7 +17,6 @@ import ( "fyne.io/fyne/v2/canvas" "fyne.io/fyne/v2/internal" "fyne.io/fyne/v2/internal/animation" - "fyne.io/fyne/v2/internal/cache" "fyne.io/fyne/v2/internal/driver" "fyne.io/fyne/v2/internal/painter" pgl "fyne.io/fyne/v2/internal/painter/gl" @@ -114,110 +113,100 @@ func (d *mobileDriver) Quit() { func (d *mobileDriver) Run() { app.Main(func(a app.App) { d.app = a - var currentSize size.Event - settingsChange := make(chan fyne.Settings) - fyne.CurrentApp().Settings().AddChangeListener(settingsChange) - draw := time.NewTicker(time.Second / 60) - for { - select { - case <-draw.C: - a.Send(paint.Event{}) - case <-settingsChange: - painter.ClearFontCache() - cache.ResetSvg() - a.Send(paint.Event{}) - case e := <-a.Events(): - current := d.currentWindow() - if current == nil { - continue - } - canvas := current.Canvas().(*mobileCanvas) - - switch e := a.Filter(e).(type) { - case lifecycle.Event: - switch e.Crosses(lifecycle.StageVisible) { - case lifecycle.CrossOn: - d.glctx, _ = e.DrawContext.(gl.Context) - d.onStart() - - // this is a fix for some android phone to prevent the app from being drawn as a blank screen after being pushed in the background - canvas.Content().Refresh() - - a.Send(paint.Event{}) - case lifecycle.CrossOff: - d.onStop() - d.glctx = nil - } - switch e.Crosses(lifecycle.StageFocused) { - case lifecycle.CrossOff: // will enter background - if runtime.GOOS == "darwin" { - if d.glctx == nil { - continue - } - - size := fyne.NewSize(float32(currentSize.WidthPx)/canvas.scale, float32(currentSize.HeightPx)/canvas.scale) - d.paintWindow(current, size) - a.Publish() - } - } - case size.Event: - if e.WidthPx <= 0 { - continue - } - currentSize = e - currentOrientation = e.Orientation - currentDPI = e.PixelsPerPt * 72 - - dev := d.device - dev.safeTop = e.InsetTopPx - dev.safeLeft = e.InsetLeftPx - dev.safeHeight = e.HeightPx - e.InsetTopPx - e.InsetBottomPx - dev.safeWidth = e.WidthPx - e.InsetLeftPx - e.InsetRightPx - canvas.scale = fyne.CurrentDevice().SystemScaleForWindow(nil) - canvas.painter.SetFrameBufferScale(1.0) - - // make sure that we paint on the next frame - canvas.Content().Refresh() - case paint.Event: - if d.glctx == nil || e.External { - continue - } - if !canvas.inited { - canvas.inited = true - canvas.painter.Init() // we cannot init until the context is set above - } + var currentSize size.Event + for e := range a.Events() { + current := d.currentWindow() + if current == nil { + continue + } + canvas := current.Canvas().(*mobileCanvas) - if d.freeDirtyTextures(canvas) { - newSize := fyne.NewSize(float32(currentSize.WidthPx)/canvas.scale, float32(currentSize.HeightPx)/canvas.scale) + switch e := a.Filter(e).(type) { + case lifecycle.Event: + switch e.Crosses(lifecycle.StageVisible) { + case lifecycle.CrossOn: + d.glctx, _ = e.DrawContext.(gl.Context) + d.onStart() - if canvas.minSizeChanged() { - canvas.ensureMinSize() + // this is a fix for some android phone to prevent the app from being drawn as a blank screen after being pushed in the background + canvas.Content().Refresh() - canvas.sizeContent(newSize) // force resize of content - } else { // if screen changed - current.Resize(newSize) + a.Send(paint.Event{}) + case lifecycle.CrossOff: + d.onStop() + d.glctx = nil + } + switch e.Crosses(lifecycle.StageFocused) { + case lifecycle.CrossOff: // will enter background + if runtime.GOOS == "darwin" { + if d.glctx == nil { + continue } - d.paintWindow(current, newSize) + size := fyne.NewSize(float32(currentSize.WidthPx)/canvas.scale, float32(currentSize.HeightPx)/canvas.scale) + d.paintWindow(current, size) a.Publish() } - cache.CleanTask() - case touch.Event: - switch e.Type { - case touch.TypeBegin: - d.tapDownCanvas(canvas, e.X, e.Y, e.Sequence) - case touch.TypeMove: - d.tapMoveCanvas(canvas, e.X, e.Y, e.Sequence) - case touch.TypeEnd: - d.tapUpCanvas(canvas, e.X, e.Y, e.Sequence) - } - case key.Event: - if e.Direction == key.DirPress { - d.typeDownCanvas(canvas, e.Rune, e.Code) - } else if e.Direction == key.DirRelease { - d.typeUpCanvas(canvas, e.Rune, e.Code) + } + case size.Event: + if e.WidthPx <= 0 { + continue + } + currentSize = e + currentOrientation = e.Orientation + currentDPI = e.PixelsPerPt * 72 + + dev := d.device + dev.safeTop = e.InsetTopPx + dev.safeLeft = e.InsetLeftPx + dev.safeHeight = e.HeightPx - e.InsetTopPx - e.InsetBottomPx + dev.safeWidth = e.WidthPx - e.InsetLeftPx - e.InsetRightPx + canvas.scale = fyne.CurrentDevice().SystemScaleForWindow(nil) + canvas.painter.SetFrameBufferScale(1.0) + + // make sure that we paint on the next frame + canvas.Content().Refresh() + case paint.Event: + if d.glctx == nil || e.External { + continue + } + if !canvas.inited { + canvas.inited = true + canvas.painter.Init() // we cannot init until the context is set above + } + + if d.freeDirtyTextures(canvas) { + newSize := fyne.NewSize(float32(currentSize.WidthPx)/canvas.scale, float32(currentSize.HeightPx)/canvas.scale) + + if canvas.minSizeChanged() { + canvas.ensureMinSize() + + canvas.sizeContent(newSize) // force resize of content + } else { // if screen changed + current.Resize(newSize) } + + d.paintWindow(current, newSize) + a.Publish() + } + + time.Sleep(time.Millisecond * 10) + a.Send(paint.Event{}) + case touch.Event: + switch e.Type { + case touch.TypeBegin: + d.tapDownCanvas(canvas, e.X, e.Y, e.Sequence) + case touch.TypeMove: + d.tapMoveCanvas(canvas, e.X, e.Y, e.Sequence) + case touch.TypeEnd: + d.tapUpCanvas(canvas, e.X, e.Y, e.Sequence) + } + case key.Event: + if e.Direction == key.DirPress { + d.typeDownCanvas(canvas, e.Rune, e.Code) + } else if e.Direction == key.DirRelease { + d.typeUpCanvas(canvas, e.Rune, e.Code) } } } From 29ae75faae3fd8d66b62d818c7c15002b5c1c005 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Fri, 9 Apr 2021 15:10:25 -0500 Subject: [PATCH 07/17] remove repeated code --- internal/driver/glfw/loop.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index c80bccfdea..3730180fce 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -200,9 +200,6 @@ func (d *gLDriver) startDrawThread() { case set := <-settingsChange: painter.ClearFontCache() cache.ResetSvg() - for _, win := range d.windowList() { - go win.Canvas().(*glCanvas).reloadScale() - } app.ApplySettingsWithCallback(set, fyne.CurrentApp(), func(w fyne.Window) { c, ok := w.Canvas().(*glCanvas) if !ok { From 36842d9a21bf6099dc8bf2b8ad97b84f2583e687 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Fri, 9 Apr 2021 17:19:04 -0500 Subject: [PATCH 08/17] apply clearing cache tasks to mobile driver --- internal/driver/gomobile/canvas.go | 2 ++ internal/driver/gomobile/driver.go | 4 ++++ internal/driver/gomobile/window.go | 6 ++++++ 3 files changed, 12 insertions(+) diff --git a/internal/driver/gomobile/canvas.go b/internal/driver/gomobile/canvas.go index e1596a3738..c7d22815b8 100644 --- a/internal/driver/gomobile/canvas.go +++ b/internal/driver/gomobile/canvas.go @@ -158,6 +158,7 @@ func (c *mobileCanvas) Refresh(obj fyne.CanvasObject) { default: // queue is full, ignore } + cache.NotifyCanvasRefresh() } func (c *mobileCanvas) RemoveShortcut(shortcut fyne.Shortcut) { @@ -274,6 +275,7 @@ func (c *mobileCanvas) minSizeChanged() bool { } minSize := obj.MinSize() + // TODO decision about minSizeCache if minSize != c.minSizeCache[obj] { minSizeChange = true diff --git a/internal/driver/gomobile/driver.go b/internal/driver/gomobile/driver.go index d59b4ba679..aa5e2b3458 100644 --- a/internal/driver/gomobile/driver.go +++ b/internal/driver/gomobile/driver.go @@ -218,6 +218,7 @@ func (d *mobileDriver) Run() { d.paintWindow(current, newSize) a.Publish() } + cache.CleanTask() case touch.Event: switch e.Type { case touch.TypeBegin: @@ -456,6 +457,9 @@ func (d *mobileDriver) freeDirtyTextures(canvas *mobileCanvas) bool { } driver.WalkCompleteObjectTree(object, freeWalked, nil) default: + cache.RangeExpiredTexturesFor(canvas, func(obj fyne.CanvasObject) { + canvas.painter.Free(obj) + }) return freed } } diff --git a/internal/driver/gomobile/window.go b/internal/driver/gomobile/window.go index cfb6f65537..4978e017f0 100644 --- a/internal/driver/gomobile/window.go +++ b/internal/driver/gomobile/window.go @@ -155,6 +155,9 @@ func (w *window) Close() { d.windows = append(d.windows[:pos], d.windows[pos+1:]...) } + // TODO free textures synchronously with the + // draw thread + w.canvas.walkTree(nil, func(obj, _ fyne.CanvasObject) { switch co := obj.(type) { case fyne.Widget: @@ -162,6 +165,9 @@ func (w *window) Close() { } }) + // TODO clean canvases cache and out of scope + // renderers + if w.onClosed != nil { w.onClosed() } From 32239334ecead4efb89b5574c15967bd6b3926b3 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Thu, 13 May 2021 12:51:04 -0500 Subject: [PATCH 09/17] added cache logic to common driver and canvas, free resources when gomobile window is closed --- internal/driver/common/canvas.go | 10 +++++++--- internal/driver/common/driver.go | 12 ++---------- internal/driver/gomobile/window.go | 13 +++++++++---- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/internal/driver/common/canvas.go b/internal/driver/common/canvas.go index 374d1b227b..41063ed37d 100644 --- a/internal/driver/common/canvas.go +++ b/internal/driver/common/canvas.go @@ -61,9 +61,7 @@ func (c *Canvas) EnsureMinSize() bool { ensureMinSize := func(node *RenderCacheNode) { obj := node.obj - canvasMutex.Lock() - canvases[obj] = c.impl - canvasMutex.Unlock() + cache.SetCanvasForObject(obj, c.impl) if !obj.Visible() { return @@ -197,6 +195,9 @@ func (c *Canvas) FreeDirtyTextures() bool { } driver.WalkCompleteObjectTree(object, freeWalked, nil) default: + cache.RangeExpiredTexturesFor(c.impl, func(obj fyne.CanvasObject) { + c.painter.Free(obj) + }) return freed } } @@ -291,6 +292,9 @@ func (c *Canvas) SetDirty(dirty bool) { c.dirtyMutex.Lock() c.dirty = dirty c.dirtyMutex.Unlock() + if dirty { + cache.NotifyCanvasRefresh() + } } // SetMenuTreeAndFocusMgr sets menu tree and focus manager. diff --git a/internal/driver/common/driver.go b/internal/driver/common/driver.go index e0cb565500..bcca9b8cde 100644 --- a/internal/driver/common/driver.go +++ b/internal/driver/common/driver.go @@ -1,19 +1,11 @@ package common import ( - "sync" - "fyne.io/fyne/v2" -) - -var ( - canvasMutex sync.RWMutex - canvases = make(map[fyne.CanvasObject]fyne.Canvas) + "fyne.io/fyne/v2/internal/cache" ) // CanvasForObject returns the canvas for the specified object. func CanvasForObject(obj fyne.CanvasObject) fyne.Canvas { - canvasMutex.RLock() - defer canvasMutex.RUnlock() - return canvases[obj] + return cache.GetCanvasForObject(obj) } diff --git a/internal/driver/gomobile/window.go b/internal/driver/gomobile/window.go index 76b592d276..75a91ea517 100644 --- a/internal/driver/gomobile/window.go +++ b/internal/driver/gomobile/window.go @@ -157,17 +157,22 @@ func (w *window) Close() { d.windows = append(d.windows[:pos], d.windows[pos+1:]...) } - // TODO free textures synchronously with the - // draw thread + // free textures + cache.RangeTexturesFor(w.canvas, func(obj fyne.CanvasObject) { + w.canvas.Painter().Free(obj) + }) + // destroy current renderers w.canvas.WalkTrees(nil, func(node *common.RenderCacheNode) { if wid, ok := node.Obj().(fyne.Widget); ok { cache.DestroyRenderer(wid) } }) - // TODO clean canvases cache and out of scope - // renderers + // remove all canvas objects from the cache + w.QueueEvent(func() { + cache.ForceCleanFor(w.canvas) + }) // Call this in a go routine, because this function could be called // inside a button which callback would be queued in this event queue From 07a7d301d92de528860af747006bb4141b758475 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Thu, 13 May 2021 14:21:22 -0500 Subject: [PATCH 10/17] group variables --- internal/cache/base.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/cache/base.go b/internal/cache/base.go index d3569abae9..16b758e610 100644 --- a/internal/cache/base.go +++ b/internal/cache/base.go @@ -11,11 +11,11 @@ import ( var ( cacheDuration = 1 * time.Minute cleanTaskInterval = cacheDuration / 2 -) -var canvasRefreshCh = make(chan struct{}, 1) -var expiredObjects = make([]fyne.CanvasObject, 0, 50) -var lastClean time.Time + canvasRefreshCh = make(chan struct{}, 1) + expiredObjects = make([]fyne.CanvasObject, 0, 50) + lastClean time.Time +) func init() { if t, err := time.ParseDuration(os.Getenv("FYNE_CACHE")); err == nil { From 2a3abfb48f29e4b243f8cc4a7a48cf18d63dd3eb Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Sat, 29 May 2021 17:20:00 -0500 Subject: [PATCH 11/17] destroy widget renderers only when canvas refresh and improve DestroyRenderer function --- internal/cache/base.go | 7 ++++--- internal/cache/widget.go | 11 +++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/internal/cache/base.go b/internal/cache/base.go index 16b758e610..0eaef5b059 100644 --- a/internal/cache/base.go +++ b/internal/cache/base.go @@ -40,11 +40,12 @@ func CleanTask() { return } } - destroyExpiredRenderers(now) destroyExpiredSvgs(now) - // canvases cache should be invalidated only on canvas refresh, otherwise there wouldn't - // be a way to recover them later if canvasRefresh { + // Destroy renderers on canvas refresh to avoid flickering screen. + destroyExpiredRenderers(now) + // canvases cache should be invalidated only on canvas refresh, otherwise there wouldn't + // be a way to recover them later destroyExpiredCanvases(now) } lastClean = time.Now() diff --git a/internal/cache/widget.go b/internal/cache/widget.go index ed21c3cab3..0815ff6bcf 100644 --- a/internal/cache/widget.go +++ b/internal/cache/widget.go @@ -48,8 +48,15 @@ func Renderer(wid fyne.Widget) fyne.WidgetRenderer { // DestroyRenderer frees a render implementation for a widget. // This is typically for internal use only. func DestroyRenderer(wid fyne.Widget) { - Renderer(wid).Destroy() - + renderersLock.RLock() + rinfo, ok := renderers[wid] + renderersLock.RUnlock() + if !ok { + return + } + if rinfo != nil { + rinfo.renderer.Destroy() + } renderersLock.Lock() delete(renderers, wid) renderersLock.Unlock() From d0041e58f5bb944bf9599eb1ea3e234aabb90ff7 Mon Sep 17 00:00:00 2001 From: Pablo Fuentes Date: Thu, 3 Jun 2021 01:55:17 -0500 Subject: [PATCH 12/17] Update comment in internal/cache/texture_common.go Co-authored-by: Jacob --- internal/cache/texture_common.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/cache/texture_common.go b/internal/cache/texture_common.go index 32ca0582f1..c52e85aa1f 100644 --- a/internal/cache/texture_common.go +++ b/internal/cache/texture_common.go @@ -6,8 +6,8 @@ import ( "fyne.io/fyne/v2" ) -// NOTE: Texture cache functions should be called always in -// the same go routine. +// NOTE: Texture cache functions should always be called in +// the same goroutine. var textures = make(map[fyne.CanvasObject]*textureInfo, 1024) From 6fc2bfc5c3b853e8aa199ee50db085993179aeac Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Fri, 4 Jun 2021 12:09:49 -0500 Subject: [PATCH 13/17] rename CleanTask to Clean and ForceCleanFor to CleanCanvas as suggested in the PR review --- internal/cache/base.go | 10 +++++----- internal/driver/glfw/loop.go | 2 +- internal/driver/glfw/window.go | 2 +- internal/driver/gomobile/driver.go | 2 +- internal/driver/gomobile/window.go | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/cache/base.go b/internal/cache/base.go index 0eaef5b059..040ac9ceb8 100644 --- a/internal/cache/base.go +++ b/internal/cache/base.go @@ -24,10 +24,10 @@ func init() { } } -// CleanTask run cache clean task, it should be called on paint events. -func CleanTask() { +// Clean run cache clean task, it should be called on paint events. +func Clean() { now := time.Now() - // do not run clean task so fast + // do not run clean task too fast if now.Sub(lastClean) < 10*time.Second { return } @@ -51,9 +51,9 @@ func CleanTask() { lastClean = time.Now() } -// ForceCleanFor forces a complete remove of all the objects that belong to the specified +// CleanCanvas performs a complete remove of all the objects that belong to the specified // canvas. Usually used to free all objects from a closing windows. -func ForceCleanFor(canvas fyne.Canvas) { +func CleanCanvas(canvas fyne.Canvas) { deletingObjs := make([]fyne.CanvasObject, 0, 50) // find all objects that belong to the specified canvas diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index aebeaf832f..0a7527e22f 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -222,7 +222,7 @@ func (d *gLDriver) startDrawThread() { d.repaintWindow(w) } - cache.CleanTask() + cache.Clean() } } }() diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index 2754879c4b..9c6eb46fb7 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -450,7 +450,7 @@ func (w *window) Close() { // remove all canvas objects from the cache w.QueueEvent(func() { - cache.ForceCleanFor(w.canvas) + cache.CleanCanvas(w.canvas) }) // trigger callbacks diff --git a/internal/driver/gomobile/driver.go b/internal/driver/gomobile/driver.go index 2a8b927a23..7d9d2bf353 100644 --- a/internal/driver/gomobile/driver.go +++ b/internal/driver/gomobile/driver.go @@ -262,7 +262,7 @@ func (d *mobileDriver) handlePaint(e paint.Event, w fyne.Window) { d.paintWindow(w, newSize) d.app.Publish() } - cache.CleanTask() + cache.Clean() } func (d *mobileDriver) onStart() { diff --git a/internal/driver/gomobile/window.go b/internal/driver/gomobile/window.go index 75a91ea517..1ecb1c139a 100644 --- a/internal/driver/gomobile/window.go +++ b/internal/driver/gomobile/window.go @@ -171,7 +171,7 @@ func (w *window) Close() { // remove all canvas objects from the cache w.QueueEvent(func() { - cache.ForceCleanFor(w.canvas) + cache.CleanCanvas(w.canvas) }) // Call this in a go routine, because this function could be called From ad7f9743507deec02014da5f07c44ccf7b144eab Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Fri, 4 Jun 2021 13:54:37 -0500 Subject: [PATCH 14/17] remove unnecessary comments --- internal/cache/base.go | 12 ------------ internal/cache/texture_common.go | 4 ---- internal/driver/glfw/window.go | 2 -- internal/driver/gomobile/window.go | 3 --- 4 files changed, 21 deletions(-) diff --git a/internal/cache/base.go b/internal/cache/base.go index 040ac9ceb8..966ca05422 100644 --- a/internal/cache/base.go +++ b/internal/cache/base.go @@ -56,7 +56,6 @@ func Clean() { func CleanCanvas(canvas fyne.Canvas) { deletingObjs := make([]fyne.CanvasObject, 0, 50) - // find all objects that belong to the specified canvas canvasesLock.RLock() for obj, cinfo := range canvases { if cinfo.canvas == canvas { @@ -68,15 +67,12 @@ func CleanCanvas(canvas fyne.Canvas) { return } - // remove them from canvas cache canvasesLock.Lock() for _, dobj := range deletingObjs { delete(canvases, dobj) } canvasesLock.Unlock() - // destroy their renderers and delete them from the renderer - // cache if they are widgets renderersLock.Lock() for _, dobj := range deletingObjs { wid, ok := dobj.(fyne.Widget) @@ -102,10 +98,6 @@ func NotifyCanvasRefresh() { } } -// =============================================================== -// Private functions -// =============================================================== - // destroyExpiredCanvases deletes objects from the canvases cache. func destroyExpiredCanvases(now time.Time) { expiredObjects = expiredObjects[:0] @@ -167,10 +159,6 @@ func destroyExpiredSvgs(now time.Time) { } } -// =============================================================== -// Private types -// =============================================================== - type expiringCache struct { expireLock sync.RWMutex expires time.Time diff --git a/internal/cache/texture_common.go b/internal/cache/texture_common.go index c52e85aa1f..e54c6a1404 100644 --- a/internal/cache/texture_common.go +++ b/internal/cache/texture_common.go @@ -59,10 +59,6 @@ func SetTexture(obj fyne.CanvasObject, texture TextureType, canvas fyne.Canvas) textures[obj] = texInfo } -// =============================================================== -// Privates -// =============================================================== - // textureCacheBase defines base texture cache object. type textureCacheBase struct { expiringCacheNoLock diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index 9c6eb46fb7..e576cce13f 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -441,14 +441,12 @@ func (w *window) Close() { }) }) - // destroy current renderers w.canvas.WalkTrees(nil, func(node *common.RenderCacheNode) { if wid, ok := node.Obj().(fyne.Widget); ok { cache.DestroyRenderer(wid) } }) - // remove all canvas objects from the cache w.QueueEvent(func() { cache.CleanCanvas(w.canvas) }) diff --git a/internal/driver/gomobile/window.go b/internal/driver/gomobile/window.go index 1ecb1c139a..0f717139c5 100644 --- a/internal/driver/gomobile/window.go +++ b/internal/driver/gomobile/window.go @@ -157,19 +157,16 @@ func (w *window) Close() { d.windows = append(d.windows[:pos], d.windows[pos+1:]...) } - // free textures cache.RangeTexturesFor(w.canvas, func(obj fyne.CanvasObject) { w.canvas.Painter().Free(obj) }) - // destroy current renderers w.canvas.WalkTrees(nil, func(node *common.RenderCacheNode) { if wid, ok := node.Obj().(fyne.Widget); ok { cache.DestroyRenderer(wid) } }) - // remove all canvas objects from the cache w.QueueEvent(func() { cache.CleanCanvas(w.canvas) }) From 342712ae6e4b8df86211d439a00b05b129023793 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Fri, 4 Jun 2021 15:23:58 -0500 Subject: [PATCH 15/17] remove NotifyCanvasRefresh logic --- internal/cache/base.go | 36 ++++++++++++------------------ internal/driver/common/canvas.go | 3 --- internal/driver/glfw/loop.go | 5 +++-- internal/driver/gomobile/driver.go | 5 +++-- 4 files changed, 20 insertions(+), 29 deletions(-) diff --git a/internal/cache/base.go b/internal/cache/base.go index 966ca05422..bbf6f46917 100644 --- a/internal/cache/base.go +++ b/internal/cache/base.go @@ -12,9 +12,9 @@ var ( cacheDuration = 1 * time.Minute cleanTaskInterval = cacheDuration / 2 - canvasRefreshCh = make(chan struct{}, 1) - expiredObjects = make([]fyne.CanvasObject, 0, 50) - lastClean time.Time + expiredObjects = make([]fyne.CanvasObject, 0, 50) + lastClean time.Time + skippedCleanWithCanvasRefresh = false ) func init() { @@ -25,23 +25,24 @@ func init() { } // Clean run cache clean task, it should be called on paint events. -func Clean() { +func Clean(canvasRefreshed bool) { now := time.Now() // do not run clean task too fast if now.Sub(lastClean) < 10*time.Second { + if canvasRefreshed { + skippedCleanWithCanvasRefresh = true + } return } - canvasRefresh := false - select { - case <-canvasRefreshCh: - canvasRefresh = true - default: - if now.Sub(lastClean) < cleanTaskInterval { - return - } + if skippedCleanWithCanvasRefresh { + skippedCleanWithCanvasRefresh = false + canvasRefreshed = true + } + if !canvasRefreshed && now.Sub(lastClean) < cleanTaskInterval { + return } destroyExpiredSvgs(now) - if canvasRefresh { + if canvasRefreshed { // Destroy renderers on canvas refresh to avoid flickering screen. destroyExpiredRenderers(now) // canvases cache should be invalidated only on canvas refresh, otherwise there wouldn't @@ -89,15 +90,6 @@ func CleanCanvas(canvas fyne.Canvas) { renderersLock.Unlock() } -// NotifyCanvasRefresh notifies to the caches that a canvas refresh was triggered. -func NotifyCanvasRefresh() { - select { - case canvasRefreshCh <- struct{}{}: - default: - return - } -} - // destroyExpiredCanvases deletes objects from the canvases cache. func destroyExpiredCanvases(now time.Time) { expiredObjects = expiredObjects[:0] diff --git a/internal/driver/common/canvas.go b/internal/driver/common/canvas.go index 41063ed37d..9b4a18795f 100644 --- a/internal/driver/common/canvas.go +++ b/internal/driver/common/canvas.go @@ -292,9 +292,6 @@ func (c *Canvas) SetDirty(dirty bool) { c.dirtyMutex.Lock() c.dirty = dirty c.dirtyMutex.Unlock() - if dirty { - cache.NotifyCanvasRefresh() - } } // SetMenuTreeAndFocusMgr sets menu tree and focus manager. diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index 0a7527e22f..dbbb9185a1 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -209,6 +209,7 @@ func (d *gLDriver) startDrawThread() { go c.reloadScale() }) case <-draw.C: + canvasRefreshed := false for _, win := range d.windowList() { w := win.(*window) w.viewLock.RLock() @@ -219,10 +220,10 @@ func (d *gLDriver) startDrawThread() { if closing || !canvas.IsDirty() || !visible { continue } - + canvasRefreshed = true d.repaintWindow(w) } - cache.Clean() + cache.Clean(canvasRefreshed) } } }() diff --git a/internal/driver/gomobile/driver.go b/internal/driver/gomobile/driver.go index 7d9d2bf353..74f6e34386 100644 --- a/internal/driver/gomobile/driver.go +++ b/internal/driver/gomobile/driver.go @@ -250,7 +250,8 @@ func (d *mobileDriver) handlePaint(e paint.Event, w fyne.Window) { c.Painter().Init() // we cannot init until the context is set above } - if c.FreeDirtyTextures() || c.IsDirty() { + canvasNeedRefresh := c.FreeDirtyTextures() || c.IsDirty() + if canvasNeedRefresh { newSize := fyne.NewSize(float32(d.currentSize.WidthPx)/c.scale, float32(d.currentSize.HeightPx)/c.scale) if c.EnsureMinSize() { @@ -262,7 +263,7 @@ func (d *mobileDriver) handlePaint(e paint.Event, w fyne.Window) { d.paintWindow(w, newSize) d.app.Publish() } - cache.Clean() + cache.Clean(canvasNeedRefresh) } func (d *mobileDriver) onStart() { From 569a354fc8e6f8d4579f90839c105d541a337b33 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Wed, 9 Jun 2021 01:29:21 -0500 Subject: [PATCH 16/17] call cache.CleanCanvas in window.destroy() instead of in window.Close() for glfw driver as suggested in PR review --- internal/driver/glfw/window.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index e576cce13f..7062a30ba5 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -447,10 +447,6 @@ func (w *window) Close() { } }) - w.QueueEvent(func() { - cache.CleanCanvas(w.canvas) - }) - // trigger callbacks if w.onClosed != nil { w.QueueEvent(w.onClosed) @@ -509,6 +505,7 @@ func (w *window) closed(viewport *glfw.Window) { // destroy this window and, if it's the last window quit the app func (w *window) destroy(d *gLDriver) { w.DestroyEventQueue() + cache.CleanCanvas(w.canvas) if w.master { d.Quit() From 2c65c07d1dc94afe1af491abfa36fd59475ee096 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Tue, 15 Jun 2021 04:59:38 -0500 Subject: [PATCH 17/17] added tests --- internal/cache/base.go | 11 +- internal/cache/base_test.go | 273 +++++++++++++++++++++++++++++++ internal/cache/texture_common.go | 4 +- 3 files changed, 281 insertions(+), 7 deletions(-) create mode 100644 internal/cache/base_test.go diff --git a/internal/cache/base.go b/internal/cache/base.go index bbf6f46917..ff0982df38 100644 --- a/internal/cache/base.go +++ b/internal/cache/base.go @@ -15,6 +15,9 @@ var ( expiredObjects = make([]fyne.CanvasObject, 0, 50) lastClean time.Time skippedCleanWithCanvasRefresh = false + + // testing purpose only + timeNow func() time.Time = time.Now ) func init() { @@ -26,7 +29,7 @@ func init() { // Clean run cache clean task, it should be called on paint events. func Clean(canvasRefreshed bool) { - now := time.Now() + now := timeNow() // do not run clean task too fast if now.Sub(lastClean) < 10*time.Second { if canvasRefreshed { @@ -49,7 +52,7 @@ func Clean(canvasRefreshed bool) { // be a way to recover them later destroyExpiredCanvases(now) } - lastClean = time.Now() + lastClean = timeNow() } // CleanCanvas performs a complete remove of all the objects that belong to the specified @@ -165,7 +168,7 @@ func (c *expiringCache) isExpired(now time.Time) bool { // setAlive updates expiration time. func (c *expiringCache) setAlive() { - t := time.Now().Add(cacheDuration) + t := timeNow().Add(cacheDuration) c.expireLock.Lock() c.expires = t c.expireLock.Unlock() @@ -182,6 +185,6 @@ func (c *expiringCacheNoLock) isExpired(now time.Time) bool { // setAlive updates expiration time. func (c *expiringCacheNoLock) setAlive() { - t := time.Now().Add(cacheDuration) + t := timeNow().Add(cacheDuration) c.expires = t } diff --git a/internal/cache/base_test.go b/internal/cache/base_test.go new file mode 100644 index 0000000000..e65a178596 --- /dev/null +++ b/internal/cache/base_test.go @@ -0,0 +1,273 @@ +package cache + +import ( + "fmt" + "os" + "testing" + "time" + + "fyne.io/fyne/v2" + "github.com/stretchr/testify/assert" +) + +func TestMain(m *testing.M) { + ret := m.Run() + testClearAll() + os.Exit(ret) +} + +func TestCacheClean(t *testing.T) { + destroyedRenderersCnt := 0 + testClearAll() + tm := &timeMock{} + + for k := 0; k < 2; k++ { + tm.setTime(10, 10+k*10) + for i := 0; i < 20; i++ { + SetSvg(fmt.Sprintf("%d%d", k, i), nil, i, i+1) + Renderer(&dummyWidget{onDestroy: func() { + destroyedRenderersCnt++ + }}) + SetCanvasForObject(&dummyWidget{}, &dummyCanvas{}) + } + } + + t.Run("no_expired_objects", func(t *testing.T) { + lastClean = tm.createTime(10, 20) + Clean(false) + assert.Len(t, svgs, 40) + assert.Len(t, renderers, 40) + assert.Len(t, canvases, 40) + assert.Zero(t, destroyedRenderersCnt) + assert.Equal(t, tm.now, lastClean) + + tm.setTime(10, 30) + Clean(true) + assert.Len(t, svgs, 40) + assert.Len(t, renderers, 40) + assert.Len(t, canvases, 40) + assert.Zero(t, destroyedRenderersCnt) + assert.Equal(t, tm.now, lastClean) + }) + + t.Run("do_not_clean_too_fast", func(t *testing.T) { + lastClean = tm.createTime(10, 30) + // when no canvas refresh and has been transcurred less than + // cleanTaskInterval duration, no clean task should occur. + tm.setTime(10, 42) + Clean(false) + assert.Less(t, lastClean.UnixNano(), tm.now.UnixNano()) + + Clean(true) + assert.Equal(t, tm.now, lastClean) + + // when canvas refresh the clean task is only executed if it has been + // transcurred more than 10 seconds since the lastClean. + tm.setTime(10, 45) + Clean(true) + assert.Less(t, lastClean.UnixNano(), tm.now.UnixNano()) + + tm.setTime(10, 53) + Clean(true) + assert.Equal(t, tm.now, lastClean) + + assert.Len(t, svgs, 40) + assert.Len(t, renderers, 40) + assert.Len(t, canvases, 40) + assert.Zero(t, destroyedRenderersCnt) + }) + + t.Run("clean_no_canvas_refresh", func(t *testing.T) { + lastClean = tm.createTime(10, 11) + tm.setTime(11, 12) + Clean(false) + assert.Len(t, svgs, 20) + assert.Len(t, renderers, 40) + assert.Len(t, canvases, 40) + assert.Zero(t, destroyedRenderersCnt) + + tm.setTime(11, 42) + Clean(false) + assert.Len(t, svgs, 0) + assert.Len(t, renderers, 40) + assert.Len(t, canvases, 40) + assert.Zero(t, destroyedRenderersCnt) + }) + + t.Run("clean_canvas_refresh", func(t *testing.T) { + lastClean = tm.createTime(10, 11) + tm.setTime(11, 11) + Clean(true) + assert.Len(t, svgs, 0) + assert.Len(t, renderers, 20) + assert.Len(t, canvases, 20) + assert.Equal(t, 20, destroyedRenderersCnt) + + tm.setTime(11, 22) + Clean(true) + assert.Len(t, svgs, 0) + assert.Len(t, renderers, 0) + assert.Len(t, canvases, 0) + assert.Equal(t, 40, destroyedRenderersCnt) + }) + + t.Run("skipped_clean_with_canvas_refresh", func(t *testing.T) { + testClearAll() + lastClean = tm.createTime(13, 10) + tm.setTime(13, 10) + assert.False(t, skippedCleanWithCanvasRefresh) + Clean(true) + assert.Equal(t, tm.now, lastClean) + + Renderer(&dummyWidget{}) + + tm.setTime(13, 15) + Clean(true) + assert.True(t, skippedCleanWithCanvasRefresh) + assert.Less(t, lastClean.UnixNano(), tm.now.UnixNano()) + assert.Len(t, renderers, 1) + + tm.setTime(14, 21) + Clean(false) + assert.False(t, skippedCleanWithCanvasRefresh) + assert.Equal(t, tm.now, lastClean) + assert.Len(t, renderers, 0) + }) +} + +func TestCleanCanvas(t *testing.T) { + destroyedRenderersCnt := 0 + testClearAll() + + dcanvas1 := &dummyCanvas{} + dcanvas2 := &dummyCanvas{} + + for i := 0; i < 20; i++ { + dwidget := &dummyWidget{onDestroy: func() { + destroyedRenderersCnt++ + }} + Renderer(dwidget) + SetCanvasForObject(dwidget, dcanvas1) + } + + for i := 0; i < 22; i++ { + dwidget := &dummyWidget{onDestroy: func() { + destroyedRenderersCnt++ + }} + Renderer(dwidget) + SetCanvasForObject(dwidget, dcanvas2) + } + + assert.Len(t, renderers, 42) + assert.Len(t, canvases, 42) + + CleanCanvas(dcanvas1) + assert.Len(t, renderers, 22) + assert.Len(t, canvases, 22) + assert.Equal(t, 20, destroyedRenderersCnt) + for _, cinfo := range canvases { + assert.Equal(t, dcanvas2, cinfo.canvas) + } + + CleanCanvas(dcanvas2) + assert.Len(t, renderers, 0) + assert.Len(t, canvases, 0) + assert.Equal(t, 42, destroyedRenderersCnt) +} + +func Test_expiringCache(t *testing.T) { + tm := &timeMock{} + tm.setTime(10, 10) + + c := &expiringCache{} + assert.True(t, c.isExpired(tm.now)) + + c.setAlive() + + tm.setTime(10, 20) + assert.False(t, c.isExpired(tm.now)) + + tm.setTime(10, 11) + tm.now = tm.now.Add(cacheDuration) + assert.True(t, c.isExpired(tm.now)) +} + +func Test_expiringCacheNoLock(t *testing.T) { + tm := &timeMock{} + tm.setTime(10, 10) + + c := &expiringCacheNoLock{} + assert.True(t, c.isExpired(tm.now)) + + c.setAlive() + + tm.setTime(10, 20) + assert.False(t, c.isExpired(tm.now)) + + tm.setTime(10, 11) + tm.now = tm.now.Add(cacheDuration) + assert.True(t, c.isExpired(tm.now)) +} + +type dummyCanvas struct { + fyne.Canvas +} + +type dummyWidget struct { + fyne.Widget + onDestroy func() +} + +func (w *dummyWidget) CreateRenderer() fyne.WidgetRenderer { + return &dummyWidgetRenderer{widget: w} +} + +type dummyWidgetRenderer struct { + widget *dummyWidget + objects []fyne.CanvasObject +} + +func (r *dummyWidgetRenderer) Destroy() { + if r.widget.onDestroy != nil { + r.widget.onDestroy() + } +} + +func (r *dummyWidgetRenderer) Layout(size fyne.Size) { +} + +func (r *dummyWidgetRenderer) MinSize() fyne.Size { + return fyne.NewSize(0, 0) +} + +func (r *dummyWidgetRenderer) Objects() []fyne.CanvasObject { + return r.objects +} + +func (r *dummyWidgetRenderer) Refresh() { +} + +type timeMock struct { + now time.Time +} + +func (t *timeMock) createTime(min, sec int) time.Time { + return time.Date(2021, time.June, 15, 2, min, sec, 0, time.UTC) +} + +func (t *timeMock) setTime(min, sec int) { + t.now = time.Date(2021, time.June, 15, 2, min, sec, 0, time.UTC) + timeNow = func() time.Time { + return t.now + } +} + +func testClearAll() { + expiredObjects = make([]fyne.CanvasObject, 0, 50) + skippedCleanWithCanvasRefresh = false + canvases = make(map[fyne.CanvasObject]*canvasInfo, 1024) + svgs = make(map[string]*svgInfo) + textures = make(map[fyne.CanvasObject]*textureInfo, 1024) + renderers = map[fyne.Widget]*rendererInfo{} + timeNow = time.Now +} diff --git a/internal/cache/texture_common.go b/internal/cache/texture_common.go index e54c6a1404..dfbd583e27 100644 --- a/internal/cache/texture_common.go +++ b/internal/cache/texture_common.go @@ -1,8 +1,6 @@ package cache import ( - "time" - "fyne.io/fyne/v2" ) @@ -31,7 +29,7 @@ func GetTexture(obj fyne.CanvasObject) (TextureType, bool) { // Note: If this is used to free textures, then it should be called inside a current // gl context to ensure textures are deleted from gl. func RangeExpiredTexturesFor(canvas fyne.Canvas, f func(fyne.CanvasObject)) { - now := time.Now() + now := timeNow() for obj, tinfo := range textures { if tinfo.isExpired(now) && tinfo.canvas == canvas { f(obj)