From 639bbf5c6804ad8c56a04d3d3cd93d7e981ddd16 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Sat, 11 Jan 2025 12:34:08 +0000 Subject: [PATCH 1/4] Add a fyne.Do method for developers and update our naming/docs --- app/app.go | 2 +- app/preferences.go | 2 +- app/preferences_other.go | 2 +- cmd/fyne_demo/tutorials/advanced.go | 2 +- cmd/fyne_demo/tutorials/canvas.go | 2 +- cmd/fyne_demo/tutorials/welcome.go | 2 +- cmd/fyne_demo/tutorials/widget.go | 4 ++-- cmd/fyne_demo/tutorials/window.go | 2 +- data/binding/queue.go | 2 +- driver.go | 6 ++++-- internal/driver/glfw/driver.go | 2 +- internal/driver/mobile/canvas.go | 2 +- internal/driver/mobile/canvas_test.go | 8 ++++---- internal/driver/mobile/driver.go | 8 +++----- test/app.go | 2 +- test/driver.go | 2 +- thread.go | 10 ++++++++++ 17 files changed, 35 insertions(+), 25 deletions(-) create mode 100644 thread.go diff --git a/app/app.go b/app/app.go index 01087d4ee7..ca96339d39 100644 --- a/app/app.go +++ b/app/app.go @@ -68,7 +68,7 @@ func (a *fyneApp) NewWindow(title string) fyne.Window { } func (a *fyneApp) Run() { - go a.lifecycle.RunEventQueue(a.driver.CallFromGoroutine) + go a.lifecycle.RunEventQueue(a.driver.DoFromGoroutine) a.driver.Run() } diff --git a/app/preferences.go b/app/preferences.go index fc777811e1..be8a21e009 100644 --- a/app/preferences.go +++ b/app/preferences.go @@ -52,7 +52,7 @@ func (p *preferences) resetSavedRecently() { time.Sleep(time.Millisecond * 100) // writes are not always atomic. 10ms worked, 100 is safer. // For test reasons we need to use current app not what we were initialised with as they can differ - fyne.CurrentApp().Driver().CallFromGoroutine(func() { + fyne.Do(func() { p.prefLock.Lock() p.savedRecently = false changedDuringSaving := p.changedDuringSaving diff --git a/app/preferences_other.go b/app/preferences_other.go index a07386c66a..ff3450c6e7 100644 --- a/app/preferences_other.go +++ b/app/preferences_other.go @@ -28,6 +28,6 @@ func (p *preferences) watch() { return } - fyne.CurrentApp().Driver().CallFromGoroutine(p.load) + fyne.Do(p.load) }) } diff --git a/cmd/fyne_demo/tutorials/advanced.go b/cmd/fyne_demo/tutorials/advanced.go index c984374237..54b748cc6f 100644 --- a/cmd/fyne_demo/tutorials/advanced.go +++ b/cmd/fyne_demo/tutorials/advanced.go @@ -38,7 +38,7 @@ func advancedScreen(win fyne.Window) fyne.CanvasObject { go func(canvas fyne.Canvas) { for range time.NewTicker(time.Second).C { - fyne.CurrentApp().Driver().CallFromGoroutine(func() { + fyne.Do(func() { scale.SetText(scaleToString(canvas)) tex.SetText(textureScaleToString(canvas)) }) diff --git a/cmd/fyne_demo/tutorials/canvas.go b/cmd/fyne_demo/tutorials/canvas.go index d9c938475d..f6df7f44b5 100644 --- a/cmd/fyne_demo/tutorials/canvas.go +++ b/cmd/fyne_demo/tutorials/canvas.go @@ -22,7 +22,7 @@ func canvasScreen(_ fyne.Window) fyne.CanvasObject { gradient := canvas.NewHorizontalGradient(color.NRGBA{0x80, 0, 0, 0xff}, color.NRGBA{0, 0x80, 0, 0xff}) go func() { for range time.NewTicker(time.Second).C { - fyne.CurrentApp().Driver().CallFromGoroutine(func() { + fyne.Do(func() { gradient.Angle += 45 if gradient.Angle >= 360 { gradient.Angle -= 360 diff --git a/cmd/fyne_demo/tutorials/welcome.go b/cmd/fyne_demo/tutorials/welcome.go index abab99ecfb..3303f1462b 100644 --- a/cmd/fyne_demo/tutorials/welcome.go +++ b/cmd/fyne_demo/tutorials/welcome.go @@ -63,7 +63,7 @@ func welcomeScreen(_ fyne.Window) fyne.CanvasObject { fyne.CurrentApp().Settings().AddChangeListener(listen) go func() { for range listen { - fyne.CurrentApp().Driver().CallFromGoroutine(func() { + fyne.Do(func() { bgColor = withAlpha(theme.Color(theme.ColorNameBackground), 0xe0) bg.FillColor = bgColor bg.Refresh() diff --git a/cmd/fyne_demo/tutorials/widget.go b/cmd/fyne_demo/tutorials/widget.go index 5f72aef0c2..23db799adf 100644 --- a/cmd/fyne_demo/tutorials/widget.go +++ b/cmd/fyne_demo/tutorials/widget.go @@ -62,7 +62,7 @@ func makeActivityTab(win fyne.Window) fyne.CanvasObject { a2.Show() time.AfterFunc(10*time.Second, func() { - fyne.CurrentApp().Driver().CallFromGoroutine(func() { + fyne.Do(func() { a1.Stop() a1.Hide() a2.Stop() @@ -90,7 +90,7 @@ func makeActivityTab(win fyne.Window) fyne.CanvasObject { d.Show() time.AfterFunc(5*time.Second, func() { - fyne.CurrentApp().Driver().CallFromGoroutine(func() { + fyne.Do(func() { a3.Stop() d.Hide() }) diff --git a/cmd/fyne_demo/tutorials/window.go b/cmd/fyne_demo/tutorials/window.go index 4caf53c1d2..229f2739af 100644 --- a/cmd/fyne_demo/tutorials/window.go +++ b/cmd/fyne_demo/tutorials/window.go @@ -72,7 +72,7 @@ func windowScreen(_ fyne.Window) fyne.CanvasObject { w.Show() time.AfterFunc(3*time.Second, func() { - fyne.CurrentApp().Driver().CallFromGoroutine(w.Close) + fyne.Do(w.Close) }) })) } diff --git a/data/binding/queue.go b/data/binding/queue.go index d6902837c3..bb4d260d90 100644 --- a/data/binding/queue.go +++ b/data/binding/queue.go @@ -11,5 +11,5 @@ func queueItem(f func()) { return } - fyne.CurrentApp().Driver().CallFromGoroutine(f) + fyne.Do(f) } diff --git a/driver.go b/driver.go index e14de6e463..3d2082df86 100644 --- a/driver.go +++ b/driver.go @@ -44,8 +44,10 @@ type Driver interface { // Since: 2.5 SetDisableScreenBlanking(bool) - // CallFromGoroutine provides a way to queue a function that is running on a goroutine back to the main thread. + // DoFromGoroutine provides a way to queue a function that is running on a goroutine back to + // the central thread for Fyne updates. // This is required when background tasks want to execute code safely in the graphical context. + // // Since: 2.6 - CallFromGoroutine(func()) + DoFromGoroutine(func()) } diff --git a/internal/driver/glfw/driver.go b/internal/driver/glfw/driver.go index 605c1b8210..d20f2f3d14 100644 --- a/internal/driver/glfw/driver.go +++ b/internal/driver/glfw/driver.go @@ -56,7 +56,7 @@ func toOSIcon(icon []byte) ([]byte, error) { return buf.Bytes(), nil } -func (d *gLDriver) CallFromGoroutine(f func()) { +func (d *gLDriver) DoFromGoroutine(f func()) { runOnMain(f) } diff --git a/internal/driver/mobile/canvas.go b/internal/driver/mobile/canvas.go index 197e9c0f71..a62bc4040d 100644 --- a/internal/driver/mobile/canvas.go +++ b/internal/driver/mobile/canvas.go @@ -386,7 +386,7 @@ func (c *canvas) waitForDoubleTap(co fyne.CanvasObject, ev *fyne.PointEvent, tap defer cancel() <-ctx.Done() - fyne.CurrentApp().Driver().CallFromGoroutine(func() { + fyne.CurrentApp().Driver().DoFromGoroutine(func() { c.touchCancelLock.Lock() touchCount := c.touchTapCount touchLast := c.touchLastTapped diff --git a/internal/driver/mobile/canvas_test.go b/internal/driver/mobile/canvas_test.go index 0f7fbd040d..17fd9b802c 100644 --- a/internal/driver/mobile/canvas_test.go +++ b/internal/driver/mobile/canvas_test.go @@ -137,7 +137,7 @@ func Test_canvas_Focusable(t *testing.T) { content.Resize(fyne.NewSize(25, 25)) pos := fyne.NewPos(10, 10) - d.CallFromGoroutine(func() { + d.DoFromGoroutine(func() { c.tapDown(pos, 0) c.tapUp(pos, 0, func(wid fyne.Tappable, ev *fyne.PointEvent) { wid.Tapped(ev) @@ -149,7 +149,7 @@ func Test_canvas_Focusable(t *testing.T) { assert.Equal(t, 0, content.unfocusedTimes) }) - d.CallFromGoroutine(func() { + d.DoFromGoroutine(func() { c.tapDown(pos, 1) c.tapUp(pos, 1, func(wid fyne.Tappable, ev *fyne.PointEvent) { wid.Tapped(ev) @@ -160,7 +160,7 @@ func Test_canvas_Focusable(t *testing.T) { assert.Equal(t, 0, content.unfocusedTimes) }) - d.CallFromGoroutine(func() { + d.DoFromGoroutine(func() { c.Focus(content) assert.Equal(t, 1, content.focusedTimes) assert.Equal(t, 0, content.unfocusedTimes) @@ -530,7 +530,7 @@ func waitAndCheck(msWait time.Duration, fn func()) { defer common.DonePool.Put(waitForCheck) time.Sleep(msWait * time.Millisecond) - d.CallFromGoroutine(func() { + d.DoFromGoroutine(func() { fn() waitForCheck <- struct{}{} diff --git a/internal/driver/mobile/driver.go b/internal/driver/mobile/driver.go index 5cbc83b6c7..9f6235f846 100644 --- a/internal/driver/mobile/driver.go +++ b/internal/driver/mobile/driver.go @@ -73,7 +73,7 @@ func init() { runtime.LockOSThread() } -func (d *driver) CallFromGoroutine(fn func()) { +func (d *driver) DoFromGoroutine(fn func()) { done := common.DonePool.Get() defer common.DonePool.Put(done) @@ -433,15 +433,13 @@ func (d *driver) tapUpCanvas(w *window, x, y float32, tapID touch.Sequence) { ev.Dragged.DY *= tapMoveDecay } - d.CallFromGoroutine(func() { + d.DoFromGoroutine(func() { wid.Dragged(ev) }) time.Sleep(time.Millisecond * 16) } - d.CallFromGoroutine(func() { - wid.DragEnd() - }) + d.DoFromGoroutine(wid.DragEnd) }() }) } diff --git a/test/app.go b/test/app.go index 300d1f6545..f1d885cfd1 100644 --- a/test/app.go +++ b/test/app.go @@ -247,7 +247,7 @@ func (s *testSettings) apply() { listener <- s } - s.app.driver.CallFromGoroutine(func() { + s.app.driver.DoFromGoroutine(func() { s.app.propertyLock.Lock() painter.ClearFontCache() cache.ResetThemeCaches() diff --git a/test/driver.go b/test/driver.go index 91cf142c6e..a98beb5a6e 100644 --- a/test/driver.go +++ b/test/driver.go @@ -49,7 +49,7 @@ func NewDriverWithPainter(painter SoftwarePainter) fyne.Driver { return &driver{painter: painter} } -func (d *driver) CallFromGoroutine(f func()) { +func (d *driver) DoFromGoroutine(f func()) { f() // Tests all run on a single (but potentially different per-test) thread } diff --git a/thread.go b/thread.go new file mode 100644 index 0000000000..c791b0f328 --- /dev/null +++ b/thread.go @@ -0,0 +1,10 @@ +package fyne + +// Do is used to execute a specified function in the main Fyne runtime context. +// This is required when a background processes wishes to adjust graphical elements of a running app. +// Developers should use this only from within goroutines they have created. +// +// Since: 2.6 +func Do(fn func()) { + CurrentApp().Driver().DoFromGoroutine(fn) +} From 9231c0251d16095ce3b3b7e3bcca0c1282372df4 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Sat, 11 Jan 2025 15:50:44 +0000 Subject: [PATCH 2/4] Add some clarifying documentation --- driver.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/driver.go b/driver.go index 3d2082df86..49244f7a4c 100644 --- a/driver.go +++ b/driver.go @@ -5,7 +5,8 @@ import "time" // Driver defines an abstract concept of a Fyne render driver. // Any implementation must provide at least these methods. type Driver interface { - // CreateWindow creates a new UI Window. + // CreateWindow creates a new UI Window for a certain implementation. + // Developers should use [App.NewWindow]. CreateWindow(string) Window // AllWindows returns a slice containing all app windows. AllWindows() []Window @@ -29,8 +30,10 @@ type Driver interface { Quit() // StartAnimation registers a new animation with this driver and requests it be started. + // Developers should use the [Animation.Start] function. StartAnimation(*Animation) // StopAnimation stops an animation and unregisters from this driver. + // Developers should use the [Animation.Stop] function. StopAnimation(*Animation) // DoubleTapDelay returns the maximum duration where a second tap after a first one @@ -46,6 +49,7 @@ type Driver interface { // DoFromGoroutine provides a way to queue a function that is running on a goroutine back to // the central thread for Fyne updates. + // The driver provides the implementation normally accessed through [fyne.Do]. // This is required when background tasks want to execute code safely in the graphical context. // // Since: 2.6 From 572f6782d5c036f15f1036372aacf6b2bbb85b28 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Sat, 11 Jan 2025 15:51:52 +0000 Subject: [PATCH 3/4] Resolve some more races and simplify --- app/settings_desktop.go | 2 +- internal/driver/glfw/canvas_test.go | 14 +++++++++----- internal/driver/glfw/window_test.go | 13 ------------- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/app/settings_desktop.go b/app/settings_desktop.go index bff819b1b2..e939330301 100644 --- a/app/settings_desktop.go +++ b/app/settings_desktop.go @@ -45,7 +45,7 @@ func watchFile(path string, callback func()) *fsnotify.Watcher { watchFileAddTarget(watcher, path) } else { - callback() + fyne.Do(callback) } } diff --git a/internal/driver/glfw/canvas_test.go b/internal/driver/glfw/canvas_test.go index cb4289a47f..636cbe5e71 100644 --- a/internal/driver/glfw/canvas_test.go +++ b/internal/driver/glfw/canvas_test.go @@ -411,16 +411,20 @@ func TestGlCanvas_ResizeWithOtherOverlay(t *testing.T) { w.Canvas().Overlays().Add(over) ensureCanvasSize(t, w, fyne.NewSize(69, 36)) // TODO: address #707; overlays should always be canvas size - over.Resize(w.Canvas().Size()) + size := w.Canvas().Size() + runOnMain(func() { + over.Resize(size) + }) - size := fyne.NewSize(200, 100) + size = fyne.NewSize(200, 100) assert.NotEqual(t, size, content.Size()) - assert.NotEqual(t, size, over.Size()) w.Resize(size) ensureCanvasSize(t, w, size) - assert.Equal(t, size, content.Size(), "canvas content is resized") - assert.Equal(t, size, over.Size(), "canvas overlay is resized") + runOnMain(func() { + assert.Equal(t, size, content.Size(), "canvas content is resized") + assert.Equal(t, size, over.Size(), "canvas overlay is resized") + }) } func TestGlCanvas_ResizeWithOverlays(t *testing.T) { diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index b36e7da4bb..0a382b5361 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -7,7 +7,6 @@ import ( "net/url" "os" "runtime" - "sync/atomic" "testing" "time" @@ -1215,14 +1214,11 @@ func TestWindow_TappedAndDoubleTapped(t *testing.T) { w := createWindow("Test") waitSingleTapped := make(chan struct{}, 1) waitDoubleTapped := make(chan struct{}, 1) - tapped := atomic.Int32{} but := newDoubleTappableButton() but.OnTapped = func() { - tapped.Store(1) waitSingleTapped <- struct{}{} } but.onDoubleTap = func() { - tapped.Store(2) waitDoubleTapped <- struct{}{} } w.SetContent(but) @@ -1233,12 +1229,7 @@ func TestWindow_TappedAndDoubleTapped(t *testing.T) { w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) }) - <-waitSingleTapped - time.Sleep(500 * time.Millisecond) - - assert.Equal(t, int32(1), tapped.Load(), "Single tap should have fired") - tapped.Store(0) runOnMain(func() { w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) @@ -1246,11 +1237,7 @@ func TestWindow_TappedAndDoubleTapped(t *testing.T) { w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) }) - <-waitDoubleTapped - time.Sleep(500 * time.Millisecond) - - assert.Equal(t, int32(2), tapped.Load(), "Double tap should have fired") } func TestWindow_MouseEventContainsModifierKeys(t *testing.T) { From 82e2aa5c4eff645b3ed056bc68aa977b95d63e3e Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Sat, 11 Jan 2025 15:55:49 +0000 Subject: [PATCH 4/4] Fix typo --- thread.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/thread.go b/thread.go index c791b0f328..c96d692a91 100644 --- a/thread.go +++ b/thread.go @@ -1,7 +1,7 @@ package fyne // Do is used to execute a specified function in the main Fyne runtime context. -// This is required when a background processes wishes to adjust graphical elements of a running app. +// This is required when a background process wishes to adjust graphical elements of a running app. // Developers should use this only from within goroutines they have created. // // Since: 2.6