From bd75180cb5cfa32db2de236e61da81a1526420b7 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Sun, 18 Feb 2024 21:50:07 +0000 Subject: [PATCH 1/6] Revert using a separate draw thread --- internal/driver/glfw/driver.go | 6 +-- internal/driver/glfw/loop.go | 76 +++++++-------------------- internal/driver/glfw/loop_desktop.go | 1 - internal/driver/glfw/loop_goxjs.go | 2 - internal/driver/glfw/window_notxdg.go | 11 +--- 5 files changed, 22 insertions(+), 74 deletions(-) diff --git a/internal/driver/glfw/driver.go b/internal/driver/glfw/driver.go index 5f9f254b58..b8d7dafa6a 100644 --- a/internal/driver/glfw/driver.go +++ b/internal/driver/glfw/driver.go @@ -8,6 +8,7 @@ import ( "os" "runtime" "sync" + "time" "github.com/fyne-io/image/ico" @@ -32,14 +33,12 @@ 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" +const doubleTapDelay = 300 * time.Millisecond type gLDriver struct { windowLock sync.RWMutex windows []fyne.Window done chan struct{} - drawDone chan struct{} waitForStart chan struct{} animation animation.Runner @@ -179,7 +178,6 @@ func NewGLDriver() *gLDriver { return &gLDriver{ done: make(chan struct{}), - drawDone: make(chan struct{}), waitForStart: make(chan struct{}), } } diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index 6c775dae8f..529e3cba7c 100644 --- a/internal/driver/glfw/loop.go +++ b/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{} @@ -56,15 +49,7 @@ 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 + 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,7 @@ func (d *gLDriver) runGL() { } } - if drawOnMainThread { - d.drawSingleFrame() - } + d.drawSingleFrame() } if windowsToRemove > 0 { oldWindows := d.windowList() @@ -200,6 +186,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 +226,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() diff --git a/internal/driver/glfw/loop_desktop.go b/internal/driver/glfw/loop_desktop.go index 8616f092f0..83a46788b7 100644 --- a/internal/driver/glfw/loop_desktop.go +++ b/internal/driver/glfw/loop_desktop.go @@ -17,7 +17,6 @@ func (d *gLDriver) initGLFW() { } initCursors() - d.startDrawThread() }) } diff --git a/internal/driver/glfw/loop_goxjs.go b/internal/driver/glfw/loop_goxjs.go index 203c1be026..37659c4b5b 100644 --- a/internal/driver/glfw/loop_goxjs.go +++ b/internal/driver/glfw/loop_goxjs.go @@ -16,8 +16,6 @@ func (d *gLDriver) initGLFW() { fyne.LogError("failed to initialise GLFW", err) return } - - d.startDrawThread() }) } diff --git a/internal/driver/glfw/window_notxdg.go b/internal/driver/glfw/window_notxdg.go index dab1c17d95..e5c52265e7 100644 --- a/internal/driver/glfw/window_notxdg.go +++ b/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) } From f823c081eb2003d6c099323502fc8233556becf5 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Mon, 26 Feb 2024 18:39:29 +0000 Subject: [PATCH 2/6] Don't run animations on another thread - tick them before each frame rendered --- internal/animation/animation_test.go | 11 +++++++++++ internal/animation/runner.go | 19 +++++++++---------- internal/driver/glfw/loop.go | 1 + internal/driver/mobile/driver.go | 1 + 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/internal/animation/animation_test.go b/internal/animation/animation_test.go index 96b5f3300a..7f5e9fcda3 100644 --- a/internal/animation/animation_test.go +++ b/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,6 +70,7 @@ func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) { Tick: func(f float32) {}, } run.Start(a) + go tick(run) // simulate a graphics draw loop run.Stop(a) // stopping animation inside tick function @@ -85,7 +93,10 @@ func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) { 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 diff --git a/internal/animation/runner.go b/internal/animation/runner.go index b45ccac069..2cf0e95b86 100644 --- a/internal/animation/runner.go +++ b/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,18 @@ 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() - } +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) { diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index 529e3cba7c..f2b2d48682 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -151,6 +151,7 @@ func (d *gLDriver) runGL() { } } + d.animation.TickAnimations() d.drawSingleFrame() } if windowsToRemove > 0 { diff --git a/internal/driver/mobile/driver.go b/internal/driver/mobile/driver.go index b987b02d32..28a69aff89 100644 --- a/internal/driver/mobile/driver.go +++ b/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) From 009fc83db04e0713c57cf2c3e5004bb3d47dd9dd Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Mon, 26 Feb 2024 19:44:54 +0000 Subject: [PATCH 3/6] Rename the runOnDraw to be more accurate --- internal/driver/glfw/canvas.go | 2 +- internal/driver/glfw/glfw_test.go | 2 +- internal/driver/glfw/loop.go | 2 +- internal/driver/glfw/loop_test.go | 2 +- internal/driver/glfw/window.go | 4 ++-- internal/driver/glfw/window_desktop.go | 2 +- internal/driver/glfw/window_wasm.go | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index 53b016f8ce..f7dbcbe193 100644 --- a/internal/driver/glfw/canvas.go +++ b/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 diff --git a/internal/driver/glfw/glfw_test.go b/internal/driver/glfw/glfw_test.go index fd4b39be00..4888ef590a 100644 --- a/internal/driver/glfw/glfw_test.go +++ b/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) }) diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index f2b2d48682..8d26dbb683 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -48,7 +48,7 @@ func runOnMain(f func()) { } // force a function f to run on the draw thread -func runOnDraw(w *window, f func()) { +func runOnMainWithContext(w *window, f func()) { runOnMain(func() { w.RunWithContext(f) }) // TODO remove this completely } diff --git a/internal/driver/glfw/loop_test.go b/internal/driver/glfw/loop_test.go index 20807a7ff6..3a998d2bb7 100644 --- a/internal/driver/glfw/loop_test.go +++ b/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) } } diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index 6639d79ba7..ba23a6cfdf 100644 --- a/internal/driver/glfw/window.go +++ b/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() diff --git a/internal/driver/glfw/window_desktop.go b/internal/driver/glfw/window_desktop.go index fc42f6da6b..0be26422a1 100644 --- a/internal/driver/glfw/window_desktop.go +++ b/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() }) diff --git a/internal/driver/glfw/window_wasm.go b/internal/driver/glfw/window_wasm.go index 0cb74c9990..25068a1495 100644 --- a/internal/driver/glfw/window_wasm.go +++ b/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() }) From d8f50390215693555242c9818fb82f0b759042e1 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Mon, 26 Feb 2024 19:45:15 +0000 Subject: [PATCH 4/6] Add missed doc --- internal/animation/runner.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/animation/runner.go b/internal/animation/runner.go index 2cf0e95b86..0acdb71776 100644 --- a/internal/animation/runner.go +++ b/internal/animation/runner.go @@ -84,6 +84,8 @@ func (r *Runner) Stop(a *fyne.Animation) { r.pendingAnimations = newList } +// 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 From 3723d6ee001f6f6e44efc340762352861cdd6d2d Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Mon, 26 Feb 2024 19:55:49 +0000 Subject: [PATCH 5/6] Check the tests dont overlap --- internal/animation/animation_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/animation/animation_test.go b/internal/animation/animation_test.go index 7f5e9fcda3..1612194bd7 100644 --- a/internal/animation/animation_test.go +++ b/internal/animation/animation_test.go @@ -73,6 +73,9 @@ func TestGLDriver_StopAnimationImmediatelyAndInsideTick(t *testing.T) { 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) @@ -86,6 +89,9 @@ 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{ From c890d2ef7665e7fd574cd55ccc3fbbbffc10a0dc Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Mon, 9 Dec 2024 21:18:16 +0000 Subject: [PATCH 6/6] Remove unneded code left from merge --- internal/driver/glfw/driver.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/driver/glfw/driver.go b/internal/driver/glfw/driver.go index b8d7dafa6a..d9c930ad91 100644 --- a/internal/driver/glfw/driver.go +++ b/internal/driver/glfw/driver.go @@ -8,7 +8,6 @@ import ( "os" "runtime" "sync" - "time" "github.com/fyne-io/image/ico" @@ -33,8 +32,6 @@ var curWindow *window // Declare conformity with Driver var _ fyne.Driver = (*gLDriver)(nil) -const doubleTapDelay = 300 * time.Millisecond - type gLDriver struct { windowLock sync.RWMutex windows []fyne.Window