Merge pull request fyne-io#4656 from andydotxyz/fix/mergethreads

Revert using a separate draw thread

internal/animation/animation_test.go

@@ -12,6 +12,11 @@ import (
 	"fyne.io/fyne/v2"
 )
 
+func tick(run *Runner) {
+	time.Sleep(time.Millisecond * 5) // wait long enough that we are not at 0 time
+	run.TickAnimations()
+}
+
 func TestGLDriver_StartAnimation(t *testing.T) {
 	done := make(chan float32)
 	run := &Runner{}
@@ -22,6 +27,7 @@ func TestGLDriver_StartAnimation(t *testing.T) {
 		}}
 
 	run.Start(a)
+	go tick(run) // simulate a graphics draw loop
 	select {
 	case d := <-done:
 		assert.Greater(t, d, float32(0))
@@ -40,6 +46,7 @@ func TestGLDriver_StopAnimation(t *testing.T) {
 		}}
 
 	run.Start(a)
+	go tick(run) // simulate a graphics draw loop
 	select {
 	case d := <-done:
 		assert.Greater(t, d, float32(0))
@@ -63,8 +70,12 @@ func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) {
 		Tick:     func(f float32) {},
 	}
 	run.Start(a)
+	go tick(run) // simulate a graphics draw loop
 	run.Stop(a)
 
+	run = &Runner{}
+	wg = sync.WaitGroup{}
+
 	// stopping animation inside tick function
 	for i := 0; i < 10; i++ {
 		wg.Add(1)
@@ -78,14 +89,20 @@ func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) {
 		run.Start(b)
 	}
 
+	run = &Runner{}
+	wg = sync.WaitGroup{}
+
 	// Similar to first part, but in this time this animation should be added and then removed
 	// from pendingAnimation slice.
 	c := &fyne.Animation{
 		Duration: time.Second,
 		Tick:     func(f float32) {},
 	}
 	run.Start(c)
+	go tick(run) // simulate a graphics draw loop
+
 	run.Stop(c)
+	go tick(run) // simulate a graphics draw loop
 
 	wg.Wait()
 	// animations stopped inside tick are really stopped in the next runner cycle

internal/animation/runner.go

@@ -43,7 +43,6 @@ func (r *Runner) Start(a *fyne.Animation) {
 			r.animations = make([]*anim, 0, 16)
 		}
 		r.animations = append(r.animations, newAnim(a))
-		r.runAnimations()
 	} else {
 		if r.pendingAnimations == nil {
 			// initialize with excess capacity to avoid re-allocations
@@ -85,18 +84,20 @@ func (r *Runner) Stop(a *fyne.Animation) {
 	r.pendingAnimations = newList
 }
 
-func (r *Runner) runAnimations() {
-	draw := time.NewTicker(time.Second / 60)
-	go func() {
-		for done := false; !done; {
-			<-draw.C
-			done = r.runOneFrame()
-		}
+// TickAnimations progresses all running animations by one tick.
+// This will be called from the driver to update objects immediately before next paint.
+func (r *Runner) TickAnimations() {
+	if !r.runnerStarted {
+		return
+	}
+
+	done := r.runOneFrame()
+
+	if done {
 		r.animationMutex.Lock()
 		r.runnerStarted = false
 		r.animationMutex.Unlock()
-		draw.Stop()
-	}()
+	}
 }
 
 func (r *Runner) runOneFrame() (done bool) {

internal/driver/glfw/canvas.go

@@ -41,7 +41,7 @@ type glCanvas struct {
 
 func (c *glCanvas) Capture() image.Image {
 	var img image.Image
-	runOnDraw(c.context.(*window), func() {
+	runOnMainWithContext(c.context.(*window), func() {
 		img = c.Painter().Capture(c)
 	})
 	return img

internal/driver/glfw/driver.go

@@ -32,14 +32,10 @@ var curWindow *window
 // Declare conformity with Driver
 var _ fyne.Driver = (*gLDriver)(nil)
 
-// A workaround on Apple M1/M2, just use 1 thread until fixed upstream.
-const drawOnMainThread bool = runtime.GOOS == "darwin" && runtime.GOARCH == "arm64"
-
 type gLDriver struct {
 	windowLock   sync.RWMutex
 	windows      []fyne.Window
 	done         chan struct{}
-	drawDone     chan struct{}
 	waitForStart chan struct{}
 
 	animation animation.Runner
@@ -179,7 +175,6 @@ func NewGLDriver() *gLDriver {
 
 	return &gLDriver{
 		done:         make(chan struct{}),
-		drawDone:     make(chan struct{}),
 		waitForStart: make(chan struct{}),
 	}
 }

internal/driver/glfw/glfw_test.go

@@ -36,7 +36,7 @@ func repaintWindow(w *window) {
 	// If we try to paint windows before the context is created, we will end up on the wrong thread.
 	<-w.driver.waitForStart
 
-	runOnDraw(w, func() {
+	runOnMainWithContext(w, func() {
 		d.repaintWindow(w)
 	})
 

internal/driver/glfw/loop.go

@@ -19,15 +19,8 @@ type funcData struct {
 	done chan struct{} // Zero allocation signalling channel
 }
 
-type drawData struct {
-	f    func()
-	win  *window
-	done chan struct{} // Zero allocation signalling channel
-}
-
 // channel for queuing functions on the main thread
 var funcQueue = make(chan funcData)
-var drawFuncQueue = make(chan drawData)
 var running atomic.Bool
 var initOnce = &sync.Once{}
 
@@ -55,16 +48,8 @@ func runOnMain(f func()) {
 }
 
 // force a function f to run on the draw thread
-func runOnDraw(w *window, f func()) {
-	if drawOnMainThread {
-		runOnMain(func() { w.RunWithContext(f) })
-		return
-	}
-	done := common.DonePool.Get()
-	defer common.DonePool.Put(done)
-
-	drawFuncQueue <- drawData{f: f, win: w, done: done}
-	<-done
+func runOnMainWithContext(w *window, f func()) {
+	runOnMain(func() { w.RunWithContext(f) }) // TODO remove this completely
 }
 
 // Preallocate to avoid allocations on every drawSingleFrame.
@@ -117,12 +102,15 @@ func (d *gLDriver) runGL() {
 	if f := fyne.CurrentApp().Lifecycle().(*app.Lifecycle).OnStarted(); f != nil {
 		go f() // don't block main, we don't have window event queue
 	}
+
+	settingsChange := make(chan fyne.Settings)
+	fyne.CurrentApp().Settings().AddChangeListener(settingsChange)
+
 	eventTick := time.NewTicker(time.Second / 60)
 	for {
 		select {
 		case <-d.done:
 			eventTick.Stop()
-			d.drawDone <- struct{}{} // wait for draw thread to stop
 			d.Terminate()
 			l := fyne.CurrentApp().Lifecycle().(*app.Lifecycle)
 			if f := l.OnStopped(); f != nil {
@@ -163,9 +151,8 @@ func (d *gLDriver) runGL() {
 					}
 				}
 
-				if drawOnMainThread {
-					d.drawSingleFrame()
-				}
+				d.animation.TickAnimations()
+				d.drawSingleFrame()
 			}
 			if windowsToRemove > 0 {
 				oldWindows := d.windowList()
@@ -200,6 +187,18 @@ func (d *gLDriver) runGL() {
 					d.Quit()
 				}
 			}
+		case set := <-settingsChange:
+			painter.ClearFontCache()
+			cache.ResetThemeCaches()
+			app.ApplySettingsWithCallback(set, fyne.CurrentApp(), func(w fyne.Window) {
+				c, ok := w.Canvas().(*glCanvas)
+				if !ok {
+					return
+				}
+				c.applyThemeOutOfTreeObjects()
+				go c.reloadScale()
+			})
+
 		}
 	}
 }
@@ -228,44 +227,6 @@ func (d *gLDriver) repaintWindow(w *window) {
 	})
 }
 
-func (d *gLDriver) startDrawThread() {
-	settingsChange := make(chan fyne.Settings)
-	fyne.CurrentApp().Settings().AddChangeListener(settingsChange)
-	var drawCh <-chan time.Time
-	if drawOnMainThread {
-		drawCh = make(chan time.Time) // don't tick when on M1
-	} else {
-		drawCh = time.NewTicker(time.Second / 60).C
-	}
-
-	go func() {
-		runtime.LockOSThread()
-
-		for {
-			select {
-			case <-d.drawDone:
-				return
-			case f := <-drawFuncQueue:
-				f.win.RunWithContext(f.f)
-				f.done <- struct{}{}
-			case set := <-settingsChange:
-				painter.ClearFontCache()
-				cache.ResetThemeCaches()
-				app.ApplySettingsWithCallback(set, fyne.CurrentApp(), func(w fyne.Window) {
-					c, ok := w.Canvas().(*glCanvas)
-					if !ok {
-						return
-					}
-					c.applyThemeOutOfTreeObjects()
-					go c.reloadScale()
-				})
-			case <-drawCh:
-				d.drawSingleFrame()
-			}
-		}
-	}()
-}
-
 // refreshWindow requests that the specified window be redrawn
 func refreshWindow(w *window) {
 	w.canvas.SetDirty()

internal/driver/glfw/loop_desktop.go

@@ -17,7 +17,6 @@ func (d *gLDriver) initGLFW() {
 		}
 
 		initCursors()
-		d.startDrawThread()
 	})
 }
 

internal/driver/glfw/loop_goxjs.go

@@ -16,8 +16,6 @@ func (d *gLDriver) initGLFW() {
 			fyne.LogError("failed to initialise GLFW", err)
 			return
 		}
-
-		d.startDrawThread()
 	})
 }
 

internal/driver/glfw/loop_test.go

@@ -26,6 +26,6 @@ func BenchmarkRunOnDraw(b *testing.B) {
 	b.ReportAllocs()
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		runOnDraw(w, f)
+		runOnMainWithContext(w, f)
 	}
 }

internal/driver/glfw/window.go

@@ -176,7 +176,7 @@ func (w *window) doShow() {
 	if content := w.canvas.Content(); content != nil {
 		content.Show()
 
-		runOnDraw(w, func() {
+		runOnMainWithContext(w, func() {
 			w.driver.repaintWindow(w)
 		})
 	}
@@ -214,7 +214,7 @@ func (w *window) Close() {
 	}
 
 	// set w.closing flag inside draw thread to ensure we can free textures
-	runOnDraw(w, func() {
+	runOnMainWithContext(w, func() {
 		w.viewLock.Lock()
 		w.closing = true
 		w.viewLock.Unlock()

internal/driver/glfw/window_desktop.go

@@ -759,7 +759,7 @@ func (w *window) create() {
 	}
 
 	// run the GL init on the draw thread
-	runOnDraw(w, func() {
+	runOnMainWithContext(w, func() {
 		w.canvas.SetPainter(gl.NewPainter(w.canvas, w))
 		w.canvas.Painter().Init()
 	})

internal/driver/glfw/window_notxdg.go

@@ -11,13 +11,6 @@ func (w *window) platformResize(canvasSize fyne.Size) {
 		return
 	}
 
-	if drawOnMainThread {
-		w.canvas.Resize(canvasSize)
-		d.repaintWindow(w)
-	} else {
-		runOnDraw(w, func() {
-			w.canvas.Resize(canvasSize)
-			d.repaintWindow(w)
-		})
-	}
+	w.canvas.Resize(canvasSize)
+	d.repaintWindow(w)
 }

internal/driver/glfw/window_wasm.go

@@ -516,7 +516,7 @@ func (w *window) create() {
 	}
 
 	// run the GL init on the draw thread
-	runOnDraw(w, func() {
+	runOnMainWithContext(w, func() {
 		w.canvas.SetPainter(gl.NewPainter(w.canvas, w))
 		w.canvas.Painter().Init()
 	})

internal/driver/mobile/driver.go

@@ -284,6 +284,7 @@ func (d *driver) handlePaint(e paint.Event, w *window) {
 		c.Painter().Init() // we cannot init until the context is set above
 	}
 
+	d.animation.TickAnimations()
 	canvasNeedRefresh := c.FreeDirtyTextures() > 0 || c.CheckDirtyAndClear()
 	if canvasNeedRefresh {
 		newSize := fyne.NewSize(float32(d.currentSize.WidthPx)/c.scale, float32(d.currentSize.HeightPx)/c.scale)