Make starting animations idempotent and improve runner performance

animation.go

@@ -1,6 +1,9 @@
 package fyne
 
-import "time"
+import (
+	"sync"
+	"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.
@@ -32,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.
 //
@@ -42,6 +61,9 @@ type Animation struct {
 	Duration    time.Duration
 	RepeatCount int
 	Tick        func(float32)
+
+	mutex sync.Mutex
+	state AnimationState
 }
 
 // NewAnimation creates a very basic animation where the callback function will be called for every
@@ -55,12 +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() {
-	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() {
-	CurrentApp().Driver().StopAnimation(a)
+	a.mutex.Lock()
+	defer a.mutex.Unlock()
+	a.state = AnimationStateStopped
+}
+
+// State returns the state of this animation.
+//
+// Since: 2.5
+func (a *Animation) State() AnimationState {
+	return a.state
 }
 
 func animationEaseIn(val float32) float32 {

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)
 }

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
-}

internal/animation/animation_test.go

@@ -47,10 +47,8 @@ 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()
+	assert.True(t, a.State() == fyne.AnimationStateStopped, "animation was not stopped")
 }
 
 func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) {
@@ -64,7 +62,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 +71,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 +84,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()
 }

internal/animation/runner.go

@@ -9,56 +9,23 @@ 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.
 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.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() {
@@ -67,20 +34,33 @@ func (r *Runner) runAnimations() {
 	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
+			// 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
+				} else if !stopped {
+					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 // nil out extra slice capacity
+			}
+			r.animations = newList
 		}
 		r.animationMutex.Lock()
 		r.runnerStarted = false

internal/driver/glfw/animation.go

@@ -3,9 +3,13 @@ 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)
 }
 
 func (d *gLDriver) StopAnimation(a *fyne.Animation) {
-	d.animation.Stop(a)
+	a.Stop()
 }

internal/driver/mobile/animation.go

@@ -3,9 +3,13 @@ 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)
 }
 
 func (d *mobileDriver) StopAnimation(a *fyne.Animation) {
-	d.animation.Stop(a)
+	a.Stop()
 }

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
 }