From 81f4c1bcf93d00ee19b7b009ec527bfe3b1ef94b Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Sat, 9 Dec 2023 15:25:22 -0800 Subject: [PATCH 1/8] use reusable slices to keep track of visible/wasVisible --- widget/gridwrap.go | 109 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 83 insertions(+), 26 deletions(-) diff --git a/widget/gridwrap.go b/widget/gridwrap.go index 8a782edd5e..b8cdab9a49 100644 --- a/widget/gridwrap.go +++ b/widget/gridwrap.go @@ -3,6 +3,7 @@ package widget import ( "fmt" "math" + "sort" "sync" "fyne.io/fyne/v2" @@ -137,8 +138,10 @@ func (l *GridWrap) RefreshItem(id GridWrapItemID) { } l.BaseWidget.Refresh() lo := l.scroller.Content.(*fyne.Container).Layout.(*gridWrapLayout) - visible := lo.visible - if item, ok := visible[id]; ok { + lo.renderLock.Lock() // ensures we are not changing visible info in render code during the search + item, ok := lo.searchVisible(lo.visible, id) + lo.renderLock.Unlock() + if ok { lo.setupGridItem(item, id, l.focused && l.currentFocus == id) } } @@ -461,17 +464,27 @@ func (gw *gridWrapItemRenderer) Refresh() { // Declare conformity with Layout interface. var _ fyne.Layout = (*gridWrapLayout)(nil) +type itemAndID struct { + item *gridWrapItem + id GridWrapItemID +} + type gridWrapLayout struct { list *GridWrap children []fyne.CanvasObject - itemPool *syncPool - visible map[GridWrapItemID]*gridWrapItem + itemPool syncPool + slicePool sync.Pool // *[]itemAndID + visible []itemAndID renderLock sync.Mutex } func newGridWrapLayout(list *GridWrap) fyne.Layout { - l := &gridWrapLayout{list: list, itemPool: &syncPool{}, visible: make(map[GridWrapItemID]*gridWrapItem)} + l := &gridWrapLayout{list: list} + l.slicePool.New = func() interface{} { + s := make([]itemAndID, 0) + return &s + } list.offsetUpdated = l.offsetUpdated return l } @@ -573,48 +586,92 @@ func (l *gridWrapLayout) updateGrid(refresh bool) { fyne.LogError("Missing UpdateCell callback required for GridWrap", nil) } - wasVisible := l.visible - l.visible = make(map[GridWrapItemID]*gridWrapItem) + // Keep pointer reference for copying slice header when returning to the pool + // https://blog.mike.norgate.xyz/unlocking-go-slice-performance-navigating-sync-pool-for-enhanced-efficiency-7cb63b0b453e + wasVisiblePtr := l.slicePool.Get().(*[]itemAndID) + wasVisible := (*wasVisiblePtr)[:0] + wasVisible = append(wasVisible, l.visible...) + + oldVisibleLen := len(l.visible) + l.visible = l.visible[:0] + var cells []fyne.CanvasObject y := offY - curItem := minItem - for row := minRow; row <= maxRow && curItem <= maxItem; row++ { + curItemID := minItem + for row := minRow; row <= maxRow && curItemID <= maxItem; row++ { x := float32(0) - for col := 0; col < colCount && curItem <= maxItem; col++ { - c, ok := wasVisible[curItem] + for col := 0; col < colCount && curItemID <= maxItem; col++ { + item, ok := l.searchVisible(wasVisible, curItemID) if !ok { - c = l.getItem() - if c == nil { + item = l.getItem() + if item == nil { continue } - c.Resize(l.list.itemMin) + item.Resize(l.list.itemMin) } - c.Move(fyne.NewPos(x, y)) + item.Move(fyne.NewPos(x, y)) if refresh { - c.Resize(l.list.itemMin) + item.Resize(l.list.itemMin) } x += l.list.itemMin.Width + theme.Padding() - l.visible[curItem] = c - cells = append(cells, c) - curItem++ + l.visible = append(l.visible, itemAndID{item: item, id: curItemID}) + cells = append(cells, item) + curItemID++ } y += l.list.itemMin.Height + theme.Padding() } + l.nilOldVisibleSliceData(l.visible, len(l.visible), oldVisibleLen) - for id, old := range wasVisible { - if _, ok := l.visible[id]; !ok { - l.itemPool.Release(old) + for _, old := range wasVisible { + if _, ok := l.searchVisible(l.visible, old.id); !ok { + l.itemPool.Release(old.item) } } l.children = cells - objects := l.children - l.list.scroller.Content.(*fyne.Container).Objects = objects + l.list.scroller.Content.(*fyne.Container).Objects = l.children + + // make a local deep copy of l.visible since rest of this function is unlocked + // and cannot safely access l.visible + visiblePtr := l.slicePool.Get().(*[]itemAndID) + visible := (*visiblePtr)[:0] + visible = append(visible, l.visible...) l.renderLock.Unlock() // user code should not be locked - for row, obj := range l.visible { - l.setupGridItem(obj, row, l.list.focused && l.list.currentFocus == row) + for _, obj := range visible { + l.setupGridItem(obj.item, obj.id, l.list.focused && l.list.currentFocus == obj.id) + } + + // nil out all references before returning slices to pool + for i := 0; i < len(wasVisible); i++ { + wasVisible[i].item = nil + } + for i := 0; i < len(visible); i++ { + visible[i].item = nil + } + *wasVisiblePtr = wasVisible // Copy the stack header over to the heap + *visiblePtr = visible + l.slicePool.Put(wasVisiblePtr) + l.slicePool.Put(visiblePtr) +} + +// invariant: visible is in ascending order of IDs +func (l *gridWrapLayout) searchVisible(visible []itemAndID, id GridWrapItemID) (*gridWrapItem, bool) { + ln := len(visible) + idx := sort.Search(ln, func(i int) bool { return visible[i].id >= id }) + if idx < ln && visible[idx].id == id { + return visible[idx].item, true + } + return nil, false +} + +func (l *gridWrapLayout) nilOldVisibleSliceData(objs []itemAndID, len, oldLen int) { + if oldLen > len { + objs = objs[:oldLen] // gain view into old data + for i := len; i < oldLen; i++ { + objs[i].item = nil + } } } From 5032db375c54ab39791732ce2ab4ba98e6f132f2 Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Sat, 9 Dec 2023 19:20:00 -0800 Subject: [PATCH 2/8] reuse allocated mem from container.Objects slice --- widget/gridwrap.go | 22 +++++++++++++++------- widget/gridwrap_test.go | 4 ++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/widget/gridwrap.go b/widget/gridwrap.go index b8cdab9a49..6b74976de7 100644 --- a/widget/gridwrap.go +++ b/widget/gridwrap.go @@ -470,8 +470,7 @@ type itemAndID struct { } type gridWrapLayout struct { - list *GridWrap - children []fyne.CanvasObject + list *GridWrap itemPool syncPool slicePool sync.Pool // *[]itemAndID @@ -595,7 +594,9 @@ func (l *gridWrapLayout) updateGrid(refresh bool) { oldVisibleLen := len(l.visible) l.visible = l.visible[:0] - var cells []fyne.CanvasObject + c := l.list.scroller.Content.(*fyne.Container) + oldObjLen := len(c.Objects) + c.Objects = c.Objects[:0] y := offY curItemID := minItem for row := minRow; row <= maxRow && curItemID <= maxItem; row++ { @@ -617,11 +618,12 @@ func (l *gridWrapLayout) updateGrid(refresh bool) { x += l.list.itemMin.Width + theme.Padding() l.visible = append(l.visible, itemAndID{item: item, id: curItemID}) - cells = append(cells, item) + c.Objects = append(c.Objects, item) curItemID++ } y += l.list.itemMin.Height + theme.Padding() } + l.nilOldSliceData(c.Objects, len(c.Objects), oldObjLen) l.nilOldVisibleSliceData(l.visible, len(l.visible), oldVisibleLen) for _, old := range wasVisible { @@ -629,9 +631,6 @@ func (l *gridWrapLayout) updateGrid(refresh bool) { l.itemPool.Release(old.item) } } - l.children = cells - - l.list.scroller.Content.(*fyne.Container).Objects = l.children // make a local deep copy of l.visible since rest of this function is unlocked // and cannot safely access l.visible @@ -667,6 +666,15 @@ func (l *gridWrapLayout) searchVisible(visible []itemAndID, id GridWrapItemID) ( return nil, false } +func (l *gridWrapLayout) nilOldSliceData(objs []fyne.CanvasObject, len, oldLen int) { + if oldLen > len { + objs = objs[:oldLen] // gain view into old data + for i := len; i < oldLen; i++ { + objs[i] = nil + } + } +} + func (l *gridWrapLayout) nilOldVisibleSliceData(objs []itemAndID, len, oldLen int) { if oldLen > len { objs = objs[:oldLen] // gain view into old data diff --git a/widget/gridwrap_test.go b/widget/gridwrap_test.go index 238389e5ed..af8fe03b2c 100644 --- a/widget/gridwrap_test.go +++ b/widget/gridwrap_test.go @@ -23,7 +23,7 @@ func TestGridWrap_Focus(t *testing.T) { assert.NotNil(t, canvas.Focused()) assert.Equal(t, 0, canvas.Focused().(*GridWrap).currentFocus) - children := list.scroller.Content.(*fyne.Container).Layout.(*gridWrapLayout).children + children := list.scroller.Content.(*fyne.Container).Objects assert.True(t, children[0].(*gridWrapItem).hovered) assert.False(t, children[1].(*gridWrapItem).hovered) assert.False(t, children[6].(*gridWrapItem).hovered) @@ -182,7 +182,7 @@ func TestGridWrap_RefreshItem(t *testing.T) { data[2] = "Replace" list.RefreshItem(2) - children := list.scroller.Content.(*fyne.Container).Layout.(*gridWrapLayout).children + children := list.scroller.Content.(*fyne.Container).Objects assert.Equal(t, children[1].(*gridWrapItem).child.(*Label).Text, "Text") assert.Equal(t, children[2].(*gridWrapItem).child.(*Label).Text, "Replace") } From 0ebdaf048b4c37662b8e158d04b538b24ff44a6c Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Sun, 10 Dec 2023 07:55:56 -0800 Subject: [PATCH 3/8] typo fix --- widget/gridwrap.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/widget/gridwrap.go b/widget/gridwrap.go index 6b74976de7..1bd25e8311 100644 --- a/widget/gridwrap.go +++ b/widget/gridwrap.go @@ -650,7 +650,7 @@ func (l *gridWrapLayout) updateGrid(refresh bool) { for i := 0; i < len(visible); i++ { visible[i].item = nil } - *wasVisiblePtr = wasVisible // Copy the stack header over to the heap + *wasVisiblePtr = wasVisible // Copy the slice header over to the heap *visiblePtr = visible l.slicePool.Put(wasVisiblePtr) l.slicePool.Put(visiblePtr) From 7344f9a3206ab25577143693830ebc3efb95eda6 Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Sun, 10 Dec 2023 10:46:03 -0800 Subject: [PATCH 4/8] WIP- optimize animation runner --- animation.go | 16 ++++++- driver.go | 9 +++- internal/animation/animation.go | 10 ---- internal/animation/animation_test.go | 18 +++---- internal/animation/runner.go | 72 ++++++++++------------------ internal/driver/glfw/animation.go | 2 +- internal/driver/mobile/animation.go | 2 +- 7 files changed, 57 insertions(+), 72 deletions(-) diff --git a/animation.go b/animation.go index a8aeba12fb..035fb3a6de 100644 --- a/animation.go +++ b/animation.go @@ -1,6 +1,8 @@ package fyne -import "time" +import ( + "time" +) // AnimationCurve represents an animation algorithm for calculating the progress through a timeline. // Custom animations can be provided by implementing the "func(float32) float32" definition. @@ -42,6 +44,8 @@ type Animation struct { Duration time.Duration RepeatCount int Tick func(float32) + + stopped bool } // NewAnimation creates a very basic animation where the callback function will be called for every @@ -55,12 +59,20 @@ func NewAnimation(d time.Duration, fn func(float32)) *Animation { // Start registers the animation with the application run-loop and starts its execution. func (a *Animation) Start() { + a.stopped = false CurrentApp().Driver().StartAnimation(a) } // Stop will end this animation and remove it from the run-loop. func (a *Animation) Stop() { - CurrentApp().Driver().StopAnimation(a) + a.stopped = true +} + +// Stopped returns true if this animation has been stopped or has completed running. +// +// Since: 2.5 +func (a *Animation) Stopped() bool { + return a.stopped } func animationEaseIn(val float32) float32 { diff --git a/driver.go b/driver.go index 8737f69148..cb1fd7347c 100644 --- a/driver.go +++ b/driver.go @@ -26,7 +26,12 @@ type Driver interface { Quit() // StartAnimation registers a new animation with this driver and requests it be started. - StartAnimation(*Animation) + // + // Deprecated: Use a.Start() instead. + StartAnimation(a *Animation) + // StopAnimation stops an animation and unregisters from this driver. - StopAnimation(*Animation) + // + // Deprecated: Use a.Stop() instead. + StopAnimation(a *Animation) } diff --git a/internal/animation/animation.go b/internal/animation/animation.go index 68eccec1d9..7ffda4ec57 100644 --- a/internal/animation/animation.go +++ b/internal/animation/animation.go @@ -1,7 +1,6 @@ package animation import ( - "sync/atomic" "time" "fyne.io/fyne/v2" @@ -14,7 +13,6 @@ type anim struct { reverse bool start time.Time total int64 - stopped uint32 // atomic, 0 == false 1 == true } func newAnim(a *fyne.Animation) *anim { @@ -23,11 +21,3 @@ func newAnim(a *fyne.Animation) *anim { animate.repeatsLeft = a.RepeatCount return animate } - -func (a *anim) setStopped() { - atomic.StoreUint32(&a.stopped, 1) -} - -func (a *anim) isStopped() bool { - return atomic.LoadUint32(&a.stopped) == 1 -} diff --git a/internal/animation/animation_test.go b/internal/animation/animation_test.go index cb5b173170..7ea11d1f45 100644 --- a/internal/animation/animation_test.go +++ b/internal/animation/animation_test.go @@ -47,10 +47,10 @@ func TestGLDriver_StopAnimation(t *testing.T) { case <-time.After(time.Second): t.Error("animation was not ticked") } - run.Stop(a) - run.animationMutex.RLock() - assert.Zero(t, len(run.animations)) - run.animationMutex.RUnlock() + a.Stop() + run.animationMutex.Lock() + assert.True(t, a.Stopped(), "animation was not stopped") + run.animationMutex.Unlock() } func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) { @@ -64,7 +64,7 @@ func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) { Tick: func(f float32) {}, } run.Start(a) - run.Stop(a) + a.Stop() // stopping animation inside tick function for i := 0; i < 10; i++ { @@ -73,7 +73,7 @@ func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) { b = &fyne.Animation{ Duration: time.Second, Tick: func(d float32) { - run.Stop(b) + b.Stop() wg.Done() }} run.Start(b) @@ -86,12 +86,12 @@ func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) { Tick: func(f float32) {}, } run.Start(c) - run.Stop(c) + c.Stop() wg.Wait() // animations stopped inside tick are really stopped in the next runner cycle time.Sleep(time.Second/60 + 100*time.Millisecond) - run.animationMutex.RLock() + run.animationMutex.Lock() assert.Zero(t, len(run.animations)) - run.animationMutex.RUnlock() + run.animationMutex.Unlock() } diff --git a/internal/animation/runner.go b/internal/animation/runner.go index e4278bfff0..6c6ebaeb4c 100644 --- a/internal/animation/runner.go +++ b/internal/animation/runner.go @@ -9,11 +9,11 @@ import ( // Runner is the main driver for animations package type Runner struct { - animationMutex sync.RWMutex - animations []*anim + animationMutex sync.Mutex pendingAnimations []*anim + runnerStarted bool - runnerStarted bool + animations []*anim // accessed only by runAnimations } // Start will register the passed application and initiate its ticking. @@ -21,66 +21,44 @@ func (r *Runner) Start(a *fyne.Animation) { r.animationMutex.Lock() defer r.animationMutex.Unlock() + r.pendingAnimations = append(r.pendingAnimations, newAnim(a)) if !r.runnerStarted { r.runnerStarted = true - r.animations = append(r.animations, newAnim(a)) r.runAnimations() - } else { - r.pendingAnimations = append(r.pendingAnimations, newAnim(a)) } } -// Stop causes an animation to stop ticking (if it was still running) and removes it from the runner. -func (r *Runner) Stop(a *fyne.Animation) { - r.animationMutex.Lock() - defer r.animationMutex.Unlock() - - newList := make([]*anim, 0, len(r.animations)) - stopped := false - for _, item := range r.animations { - if item.a != a { - newList = append(newList, item) - } else { - item.setStopped() - stopped = true - } - } - r.animations = newList - if stopped { - return - } - - newList = make([]*anim, 0, len(r.pendingAnimations)) - for _, item := range r.pendingAnimations { - if item.a != a { - newList = append(newList, item) - } else { - item.setStopped() - } - } - r.pendingAnimations = newList -} - func (r *Runner) runAnimations() { draw := time.NewTicker(time.Second / 60) go func() { for done := false; !done; { <-draw.C - r.animationMutex.Lock() - oldList := r.animations - r.animationMutex.Unlock() - newList := make([]*anim, 0, len(oldList)) - for _, a := range oldList { - if !a.isStopped() && r.tickAnimation(a) { - newList = append(newList, a) + + // tick currently running animations + newList := r.animations[:0] // references same underlying backing array + for _, a := range r.animations { + if s := a.a.Stopped(); !s && r.tickAnimation(a) { + newList = append(newList, a) // still running + } else if !s { + a.a.Stop() // mark as stopped (completed running) } } + + // bring in all pending animations r.animationMutex.Lock() - r.animations = append(newList, r.pendingAnimations...) - r.pendingAnimations = nil - done = len(r.animations) == 0 + for i, a := range r.pendingAnimations { + newList = append(newList, a) + r.pendingAnimations[i] = nil + } + r.pendingAnimations = r.pendingAnimations[:0] r.animationMutex.Unlock() + + done = len(newList) == 0 + for i := len(newList); i < len(r.animations); i++ { + r.animations[i] = nil + } + r.animations = newList } r.animationMutex.Lock() r.runnerStarted = false diff --git a/internal/driver/glfw/animation.go b/internal/driver/glfw/animation.go index ec7d72a180..7162d06447 100644 --- a/internal/driver/glfw/animation.go +++ b/internal/driver/glfw/animation.go @@ -7,5 +7,5 @@ func (d *gLDriver) StartAnimation(a *fyne.Animation) { } func (d *gLDriver) StopAnimation(a *fyne.Animation) { - d.animation.Stop(a) + a.Stop() } diff --git a/internal/driver/mobile/animation.go b/internal/driver/mobile/animation.go index a70504201d..a2a101b87e 100644 --- a/internal/driver/mobile/animation.go +++ b/internal/driver/mobile/animation.go @@ -7,5 +7,5 @@ func (d *mobileDriver) StartAnimation(a *fyne.Animation) { } func (d *mobileDriver) StopAnimation(a *fyne.Animation) { - d.animation.Stop(a) + a.Stop() } From 55eb2d81a8e46ff75eb637f0f2d6d9652799cb33 Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Sun, 10 Dec 2023 12:39:15 -0800 Subject: [PATCH 5/8] make animation.Start idempotent - no-op if started multiple times --- animation.go | 40 +++++++++++++++++++++++----- internal/animation/animation_test.go | 2 +- internal/animation/runner.go | 4 +-- internal/driver/glfw/animation.go | 4 +++ internal/driver/mobile/animation.go | 4 +++ 5 files changed, 44 insertions(+), 10 deletions(-) diff --git a/animation.go b/animation.go index 035fb3a6de..0857d3c661 100644 --- a/animation.go +++ b/animation.go @@ -1,6 +1,7 @@ package fyne import ( + "sync" "time" ) @@ -34,6 +35,22 @@ var ( AnimationLinear = animationLinear ) +// AnimationState represents the state of an animation. +// +// Since: 2.5 +type AnimationState int + +const ( + // AnimationStateNotStarted represents an animation that has been created but not yet started. + AnimationStateNotStarted AnimationState = iota + + // AnimationStateRunning represents an animation that is running. + AnimationStateRunning + + // AnimationStateStopped represents an animation that has been stopped or has finished running. + AnimationStateStopped +) + // Animation represents an animated element within a Fyne canvas. // These animations may control individual objects or entire scenes. // @@ -45,7 +62,8 @@ type Animation struct { RepeatCount int Tick func(float32) - stopped bool + mutex sync.Mutex + state AnimationState } // NewAnimation creates a very basic animation where the callback function will be called for every @@ -59,20 +77,28 @@ func NewAnimation(d time.Duration, fn func(float32)) *Animation { // Start registers the animation with the application run-loop and starts its execution. func (a *Animation) Start() { - a.stopped = false - CurrentApp().Driver().StartAnimation(a) + a.mutex.Lock() + defer a.mutex.Unlock() + if a.state == AnimationStateRunning { + return + } + a.state = AnimationStateRunning + d := CurrentApp().Driver().(interface{ StartAnimationPrivate(*Animation) }) + d.StartAnimationPrivate(a) } // Stop will end this animation and remove it from the run-loop. func (a *Animation) Stop() { - a.stopped = true + a.mutex.Lock() + defer a.mutex.Unlock() + a.state = AnimationStateStopped } -// Stopped returns true if this animation has been stopped or has completed running. +// State returns the state of this animation. // // Since: 2.5 -func (a *Animation) Stopped() bool { - return a.stopped +func (a *Animation) State() AnimationState { + return a.state } func animationEaseIn(val float32) float32 { diff --git a/internal/animation/animation_test.go b/internal/animation/animation_test.go index 7ea11d1f45..37385c18a2 100644 --- a/internal/animation/animation_test.go +++ b/internal/animation/animation_test.go @@ -49,7 +49,7 @@ func TestGLDriver_StopAnimation(t *testing.T) { } a.Stop() run.animationMutex.Lock() - assert.True(t, a.Stopped(), "animation was not stopped") + assert.True(t, a.State() == fyne.AnimationStateStopped, "animation was not stopped") run.animationMutex.Unlock() } diff --git a/internal/animation/runner.go b/internal/animation/runner.go index 6c6ebaeb4c..b4e5a0ff0d 100644 --- a/internal/animation/runner.go +++ b/internal/animation/runner.go @@ -38,9 +38,9 @@ func (r *Runner) runAnimations() { // tick currently running animations newList := r.animations[:0] // references same underlying backing array for _, a := range r.animations { - if s := a.a.Stopped(); !s && r.tickAnimation(a) { + if stopped := a.a.State() == fyne.AnimationStateStopped; !stopped && r.tickAnimation(a) { newList = append(newList, a) // still running - } else if !s { + } else if !stopped { a.a.Stop() // mark as stopped (completed running) } } diff --git a/internal/driver/glfw/animation.go b/internal/driver/glfw/animation.go index 7162d06447..80997fc103 100644 --- a/internal/driver/glfw/animation.go +++ b/internal/driver/glfw/animation.go @@ -3,6 +3,10 @@ package glfw import "fyne.io/fyne/v2" func (d *gLDriver) StartAnimation(a *fyne.Animation) { + a.Start() +} + +func (d *gLDriver) StartAnimationPrivate(a *fyne.Animation) { d.animation.Start(a) } diff --git a/internal/driver/mobile/animation.go b/internal/driver/mobile/animation.go index a2a101b87e..5b9ef2da4f 100644 --- a/internal/driver/mobile/animation.go +++ b/internal/driver/mobile/animation.go @@ -3,6 +3,10 @@ package mobile import "fyne.io/fyne/v2" func (d *mobileDriver) StartAnimation(a *fyne.Animation) { + a.Start() +} + +func (d *mobileDriver) StartAnimationPrivate(a *fyne.Animation) { d.animation.Start(a) } From b4d44573c240d6c1067390d7a23c5b43b2907785 Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Sun, 10 Dec 2023 14:03:05 -0800 Subject: [PATCH 6/8] add StartAnimationPrivate to testDriver; add some comments --- internal/animation/runner.go | 6 ++++-- test/testdriver.go | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/animation/runner.go b/internal/animation/runner.go index b4e5a0ff0d..bad7827aa2 100644 --- a/internal/animation/runner.go +++ b/internal/animation/runner.go @@ -36,7 +36,9 @@ func (r *Runner) runAnimations() { <-draw.C // tick currently running animations - newList := r.animations[:0] // references same underlying backing array + // use technique from https://github.com/golang/go/wiki/SliceTricks#filtering-without-allocating + // to filter the still-running animations for the next iteration without allocating a new slice + newList := r.animations[:0] for _, a := range r.animations { if stopped := a.a.State() == fyne.AnimationStateStopped; !stopped && r.tickAnimation(a) { newList = append(newList, a) // still running @@ -56,7 +58,7 @@ func (r *Runner) runAnimations() { done = len(newList) == 0 for i := len(newList); i < len(r.animations); i++ { - r.animations[i] = nil + r.animations[i] = nil // nil out extra slice capacity } r.animations = newList } diff --git a/test/testdriver.go b/test/testdriver.go index 146e44e1ff..273490e168 100644 --- a/test/testdriver.go +++ b/test/testdriver.go @@ -112,6 +112,11 @@ func (d *testDriver) StopAnimation(a *fyne.Animation) { // currently no animations in test app, do nothing } +func (d *testDriver) StartAnimationPrivate(a *fyne.Animation) { + /// currently no animations in test app, we just initialise it and leave + a.Tick(1.0) +} + func (d *testDriver) Quit() { // no-op } From 4175a39b812de3b9966fc5deb0822ecbabedbd5b Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Sun, 10 Dec 2023 14:23:09 -0800 Subject: [PATCH 7/8] make critical section shorter --- internal/animation/runner.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/animation/runner.go b/internal/animation/runner.go index bad7827aa2..11791fa375 100644 --- a/internal/animation/runner.go +++ b/internal/animation/runner.go @@ -19,9 +19,9 @@ type Runner struct { // Start will register the passed application and initiate its ticking. func (r *Runner) Start(a *fyne.Animation) { r.animationMutex.Lock() - defer r.animationMutex.Unlock() - r.pendingAnimations = append(r.pendingAnimations, newAnim(a)) + r.animationMutex.Unlock() + if !r.runnerStarted { r.runnerStarted = true r.runAnimations() From 92b38cdf6cf187e42a0091de319e704551d978a6 Mon Sep 17 00:00:00 2001 From: Drew Weymouth Date: Sun, 10 Dec 2023 14:26:35 -0800 Subject: [PATCH 8/8] remove now-unneeded lock in test code --- internal/animation/animation_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/animation/animation_test.go b/internal/animation/animation_test.go index 37385c18a2..be6992f884 100644 --- a/internal/animation/animation_test.go +++ b/internal/animation/animation_test.go @@ -48,9 +48,7 @@ func TestGLDriver_StopAnimation(t *testing.T) { t.Error("animation was not ticked") } a.Stop() - run.animationMutex.Lock() assert.True(t, a.State() == fyne.AnimationStateStopped, "animation was not stopped") - run.animationMutex.Unlock() } func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) {