Make start animations idempotent and improve runner performance

animation.go

@@ -1,6 +1,9 @@
 package fyne
 
-import "time"
+import (
+	"sync/atomic"
+	"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,8 @@ type Animation struct {
 	Duration    time.Duration
 	RepeatCount int
 	Tick        func(float32)
+
+	state atomic.Int64
 }
 
 // NewAnimation creates a very basic animation where the callback function will be called for every
@@ -55,12 +76,25 @@ 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)
+	old := a.state.Swap(int64(AnimationStateRunning))
+	if old == int64(AnimationStateRunning) {
+		return
+	}
+
+	d := CurrentApp().Driver().(interface{ StartAnimationInternal(*Animation) })
+	d.StartAnimationInternal(a)
 }
 
 // Stop will end this animation and remove it from the run-loop.
 func (a *Animation) Stop() {
-	CurrentApp().Driver().StopAnimation(a)
+	a.state.Store(int64(AnimationStateStopped))
+}
+
+// State returns the state of this animation.
+//
+// Since: 2.5
+func (a *Animation) State() AnimationState {
+	return AnimationState(a.state.Load())
 }
 
 func animationEaseIn(val float32) float32 {

driver.go

@@ -28,9 +28,14 @@ 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)
 
 	// DoubleTapDelay returns the maximum duration where a second tap after a first one
 	// will be considered a DoubleTap instead of two distinct Tap events.

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     atomic.Bool
 }
 
 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() {
-	a.stopped.Store(true)
-}
-
-func (a *anim) isStopped() bool {
-	return a.stopped.Load()
-}

internal/animation/animation_test.go

@@ -46,10 +46,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) {
@@ -63,7 +61,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++ {
@@ -72,7 +70,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)
@@ -85,12 +83,10 @@ 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()
 	assert.Zero(t, len(run.animations))
-	run.animationMutex.RUnlock()
 }

internal/animation/runner.go

@@ -2,91 +2,67 @@ package animation
 
 import (
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"fyne.io/fyne/v2"
 )
 
 // Runner is the main driver for animations package
 type Runner struct {
-	animationMutex    sync.RWMutex
-	animations        []*anim
-	pendingAnimations []*anim
+	pendingAnimationsMutex sync.Mutex
+	pendingAnimations      []*anim
+	runnerStarted          atomic.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.pendingAnimationsMutex.Lock()
+	r.pendingAnimations = append(r.pendingAnimations, newAnim(a))
+	r.pendingAnimationsMutex.Unlock()
 
-	if !r.runnerStarted {
-		r.runnerStarted = true
-		r.animations = append(r.animations, newAnim(a))
-		r.runAnimations()
-	} else {
-		r.pendingAnimations = append(r.pendingAnimations, newAnim(a))
+	if r.runnerStarted.CompareAndSwap(false, true) {
+		go r.runAnimations()
 	}
 }
 
-// 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()
+func (r *Runner) runAnimations() {
+	draw := time.NewTicker(time.Second / 60)
 
-	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
+	for done := false; !done; {
+		<-draw.C
+
+		// 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)
+			}
 		}
-	}
-	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()
+		// bring in all pending animations
+		r.pendingAnimationsMutex.Lock()
+		for i, a := range r.pendingAnimations {
+			newList = append(newList, a)
+			r.pendingAnimations[i] = nil
 		}
-	}
-	r.pendingAnimations = newList
-}
+		r.pendingAnimations = r.pendingAnimations[:0]
+		r.pendingAnimationsMutex.Unlock()
 
-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)
-				}
-			}
-			r.animationMutex.Lock()
-			r.animations = append(newList, r.pendingAnimations...)
-			r.pendingAnimations = nil
-			done = len(r.animations) == 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.animationMutex.Lock()
-		r.runnerStarted = false
-		r.animationMutex.Unlock()
-		draw.Stop()
-	}()
+		r.animations = newList
+	}
+	r.runnerStarted.Store(false)
+	draw.Stop()
 }
 
 // tickAnimation will process a frame of animation and return true if this should continue animating

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) StartAnimationInternal(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) StartAnimationInternal(a *fyne.Animation) {
 	d.animation.Start(a)
 }
 
 func (d *mobileDriver) StopAnimation(a *fyne.Animation) {
-	d.animation.Stop(a)
+	a.Stop()
 }

test/testdriver.go

@@ -113,6 +113,11 @@ func (d *testDriver) StopAnimation(a *fyne.Animation) {
 	// currently no animations in test app, do nothing
 }
 
+func (d *testDriver) StartAnimationInternal(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
 }