From 217da8c1e35fd36dfc2c2536791cc47ddef2d70e Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Tue, 17 Dec 2024 16:05:55 +0000 Subject: [PATCH 01/13] Remove the RunOnMainWithContext which is no longer needed --- internal/driver/glfw/canvas.go | 2 +- internal/driver/glfw/clipboard_goxjs.go | 12 ++++-------- internal/driver/glfw/glfw_test.go | 2 +- internal/driver/glfw/loop.go | 5 ----- internal/driver/glfw/loop_test.go | 2 +- 5 files changed, 7 insertions(+), 16 deletions(-) diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index 5080f2008b..4b51655e06 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 - runOnMainWithContext(c.context.(*window), func() { + c.context.(*window).RunWithContext(func() { img = c.Painter().Capture(c) }) return img diff --git a/internal/driver/glfw/clipboard_goxjs.go b/internal/driver/glfw/clipboard_goxjs.go index e066a70dd5..1bf18d5148 100644 --- a/internal/driver/glfw/clipboard_goxjs.go +++ b/internal/driver/glfw/clipboard_goxjs.go @@ -3,8 +3,9 @@ package glfw import ( - "fyne.io/fyne/v2" glfw "github.com/fyne-io/glfw-js" + + "fyne.io/fyne/v2" ) // Declare conformity with Clipboard interface @@ -21,16 +22,11 @@ type clipboard struct { // Content returns the clipboard content func (c clipboard) Content() string { - content := "" - runOnMain(func() { - content, _ = c.window.GetClipboardString() - }) + content, _ := c.window.GetClipboardString() return content } // SetContent sets the clipboard content func (c clipboard) SetContent(content string) { - runOnMain(func() { - c.window.SetClipboardString(content) - }) + c.window.SetClipboardString(content) } diff --git a/internal/driver/glfw/glfw_test.go b/internal/driver/glfw/glfw_test.go index 4888ef590a..cf7edec2b7 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 - runOnMainWithContext(w, func() { + w.RunWithContext(func() { d.repaintWindow(w) }) diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index acf2a30526..452a7f422d 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -47,11 +47,6 @@ func runOnMain(f func()) { <-done } -// force a function f to run on the draw thread -func runOnMainWithContext(w *window, f func()) { - runOnMain(func() { w.RunWithContext(f) }) // TODO remove this completely -} - // Preallocate to avoid allocations on every drawSingleFrame. // Note that the capacity of this slice can only grow, // but its length will never be longer than the total number of diff --git a/internal/driver/glfw/loop_test.go b/internal/driver/glfw/loop_test.go index 3a998d2bb7..46701d4713 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++ { - runOnMainWithContext(w, f) + w.RunWithContext(f) } } From 44ee3560fed8676b5a156e2921709b8b4f096df8 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Tue, 17 Dec 2024 16:06:33 +0000 Subject: [PATCH 02/13] Fixup the thread semantics of window creation Fixes issue with multiple windows under the new thread model --- internal/driver/glfw/driver_desktop.go | 26 +++-- internal/driver/glfw/window.go | 126 +++++++++++------------ internal/driver/glfw/window_desktop.go | 135 ++++++++++++------------- internal/driver/glfw/window_wasm.go | 101 +++++++++--------- 4 files changed, 183 insertions(+), 205 deletions(-) diff --git a/internal/driver/glfw/driver_desktop.go b/internal/driver/glfw/driver_desktop.go index 32b6d99921..48ce4abe44 100644 --- a/internal/driver/glfw/driver_desktop.go +++ b/internal/driver/glfw/driver_desktop.go @@ -61,7 +61,9 @@ func (d *gLDriver) SetSystemTrayMenu(m *fyne.Menu) { } // it must be refreshed after init, so an earlier call would have been ineffective - d.refreshSystray(m) + runOnMain(func() { + d.refreshSystray(m) + }) }, func() { // anything required for tear-down }) @@ -135,14 +137,12 @@ func itemForMenuItem(i *fyne.MenuItem, parent *systray.MenuItem) *systray.MenuIt } func (d *gLDriver) refreshSystray(m *fyne.Menu) { - runOnMain(func() { - d.systrayMenu = m + d.systrayMenu = m - systray.ResetMenu() - d.refreshSystrayMenu(m, nil) + systray.ResetMenu() + d.refreshSystrayMenu(m, nil) - addMissingQuitForMenu(m, d) - }) + addMissingQuitForMenu(m, d) } func (d *gLDriver) refreshSystrayMenu(m *fyne.Menu, parent *systray.MenuItem) { @@ -175,13 +175,11 @@ func (d *gLDriver) SetSystemTrayIcon(resource fyne.Resource) { return } - runOnMain(func() { - if _, ok := resource.(*theme.ThemedResource); ok { - systray.SetTemplateIcon(img, img) - } else { - systray.SetIcon(img) - } - }) + if _, ok := resource.(*theme.ThemedResource); ok { + systray.SetTemplateIcon(img, img) + } else { + systray.SetIcon(img) + } } func (d *gLDriver) SystemTrayMenu() *fyne.Menu { diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index d60c7169ce..1f36b20c34 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -129,7 +129,9 @@ func (w *window) detectTextureScale() float32 { } func (w *window) Show() { - go w.doShow() + go func() { + runOnMain(w.doShow) + }() } func (w *window) doShow() { @@ -145,55 +147,51 @@ func (w *window) doShow() { return } - runOnMain(func() { - w.visible = true - view := w.view() - view.SetTitle(w.title) + w.visible = true + view := w.view() + view.SetTitle(w.title) - if !build.IsWayland && w.centered { - w.doCenterOnScreen() // lastly center if that was requested - } - view.Show() + if !build.IsWayland && w.centered { + w.doCenterOnScreen() // lastly center if that was requested + } + view.Show() - // save coordinates - if !build.IsWayland { - w.xpos, w.ypos = view.GetPos() - } + // save coordinates + if !build.IsWayland { + w.xpos, w.ypos = view.GetPos() + } - if w.fullScreen { // this does not work if called before viewport.Show() - go func() { - time.Sleep(time.Millisecond * 100) - w.SetFullScreen(true) - }() - } - }) + if w.fullScreen { // this does not work if called before viewport.Show() + go func() { + time.Sleep(time.Millisecond * 100) + w.SetFullScreen(true) + }() + } // show top canvas element if content := w.canvas.Content(); content != nil { content.Show() - runOnMainWithContext(w, func() { + w.RunWithContext(func() { w.driver.repaintWindow(w) }) } } func (w *window) Hide() { - runOnMain(func() { - if w.closing || w.viewport == nil { - return - } + if w.closing || w.viewport == nil { + return + } - w.visible = false - v := w.viewport + w.visible = false + v := w.viewport - v.Hide() + v.Hide() - // hide top canvas element - if content := w.canvas.Content(); content != nil { - content.Hide() - } - }) + // hide top canvas element + if content := w.canvas.Content(); content != nil { + content.Hide() + } } func (w *window) Close() { @@ -743,17 +741,16 @@ func (w *window) processFocused(focus bool) { w.canvas.FocusLost() w.mousePos = fyne.Position{} - go func() { // check whether another window was focused or not - time.Sleep(time.Millisecond * 100) - if curWindow != w { - return - } + // check whether another window was focused or not + time.Sleep(time.Millisecond * 100) + if curWindow != w { + return + } - curWindow = nil - if f := fyne.CurrentApp().Lifecycle().(*app.Lifecycle).OnExitedForeground(); f != nil { - f() - } - }() + curWindow = nil + if f := fyne.CurrentApp().Lifecycle().(*app.Lifecycle).OnExitedForeground(); f != nil { + f() + } } } @@ -858,17 +855,13 @@ func (w *window) RunWithContext(f func()) { w.DetachCurrentContext() } -func (w *window) RescaleContext() { - runOnMain(w.rescaleOnMain) -} - func (w *window) Context() any { return nil } func (w *window) runOnMainWhenCreated(fn func()) { if w.view() != nil { - runOnMain(fn) + fn() return } @@ -915,15 +908,14 @@ func (d *gLDriver) createWindow(title string, decorate bool) fyne.Window { if title == "" { title = defaultTitle } - runOnMain(func() { - d.initGLFW() - ret = &window{title: title, decorate: decorate, driver: d} - ret.canvas = newCanvas() - ret.canvas.context = ret - ret.SetIcon(ret.icon) - d.addWindow(ret) - }) + d.initGLFW() + + ret = &window{title: title, decorate: decorate, driver: d} + ret.canvas = newCanvas() + ret.canvas.context = ret + ret.SetIcon(ret.icon) + d.addWindow(ret) return ret } @@ -932,19 +924,17 @@ func (w *window) doShowAgain() { return } - runOnMain(func() { - // show top canvas element - if content := w.canvas.Content(); content != nil { - content.Show() - } + // show top canvas element + if content := w.canvas.Content(); content != nil { + content.Show() + } - view := w.view() - if !build.IsWayland { - view.SetPos(w.xpos, w.ypos) - } - view.Show() - w.visible = true - }) + view := w.view() + if !build.IsWayland { + view.SetPos(w.xpos, w.ypos) + } + view.Show() + w.visible = true } func (w *window) isClosing() bool { diff --git a/internal/driver/glfw/window_desktop.go b/internal/driver/glfw/window_desktop.go index 3aae611b8f..79f3e5f250 100644 --- a/internal/driver/glfw/window_desktop.go +++ b/internal/driver/glfw/window_desktop.go @@ -685,7 +685,7 @@ func (w *window) DetachCurrentContext() { glfw.DetachCurrentContext() } -func (w *window) rescaleOnMain() { +func (w *window) RescaleContext() { if w.isClosing() { return } @@ -706,89 +706,84 @@ func (w *window) rescaleOnMain() { } func (w *window) create() { - runOnMain(func() { - if !build.IsWayland { - // make the window hidden, we will set it up and then show it later - glfw.WindowHint(glfw.Visible, glfw.False) - } - if w.decorate { - glfw.WindowHint(glfw.Decorated, glfw.True) - } else { - glfw.WindowHint(glfw.Decorated, glfw.False) - } - if w.fixedSize { - glfw.WindowHint(glfw.Resizable, glfw.False) - } else { - glfw.WindowHint(glfw.Resizable, glfw.True) - } - glfw.WindowHint(glfw.AutoIconify, glfw.False) - initWindowHints() + if !build.IsWayland { + // make the window hidden, we will set it up and then show it later + glfw.WindowHint(glfw.Visible, glfw.False) + } + if w.decorate { + glfw.WindowHint(glfw.Decorated, glfw.True) + } else { + glfw.WindowHint(glfw.Decorated, glfw.False) + } + if w.fixedSize { + glfw.WindowHint(glfw.Resizable, glfw.False) + } else { + glfw.WindowHint(glfw.Resizable, glfw.True) + } + glfw.WindowHint(glfw.AutoIconify, glfw.False) + initWindowHints() - pixWidth, pixHeight := w.screenSize(w.canvas.size) - pixWidth = int(fyne.Max(float32(pixWidth), float32(w.width))) - if pixWidth == 0 { - pixWidth = 10 - } - pixHeight = int(fyne.Max(float32(pixHeight), float32(w.height))) - if pixHeight == 0 { - pixHeight = 10 - } + pixWidth, pixHeight := w.screenSize(w.canvas.size) + pixWidth = int(fyne.Max(float32(pixWidth), float32(w.width))) + if pixWidth == 0 { + pixWidth = 10 + } + pixHeight = int(fyne.Max(float32(pixHeight), float32(w.height))) + if pixHeight == 0 { + pixHeight = 10 + } - win, err := glfw.CreateWindow(pixWidth, pixHeight, w.title, nil, nil) - if err != nil { - w.driver.initFailed("window creation error", err) - return - } + win, err := glfw.CreateWindow(pixWidth, pixHeight, w.title, nil, nil) + if err != nil { + w.driver.initFailed("window creation error", err) + return + } - w.viewport = win - }) + w.viewport = win if w.view() == nil { // something went wrong above, it will have been logged return } // run the GL init on the draw thread - runOnMainWithContext(w, func() { + w.RunWithContext(func() { w.canvas.SetPainter(gl.NewPainter(w.canvas, w)) w.canvas.Painter().Init() }) - runOnMain(func() { - w.setDarkMode() - - win := w.view() - win.SetCloseCallback(w.closed) - win.SetPosCallback(w.moved) - win.SetSizeCallback(w.resized) - win.SetFramebufferSizeCallback(w.frameSized) - win.SetRefreshCallback(w.refresh) - win.SetContentScaleCallback(w.scaled) - win.SetCursorPosCallback(w.mouseMoved) - win.SetMouseButtonCallback(w.mouseClicked) - win.SetScrollCallback(w.mouseScrolled) - win.SetKeyCallback(w.keyPressed) - win.SetCharCallback(w.charInput) - win.SetFocusCallback(w.focused) - - w.canvas.detectedScale = w.detectScale() - w.canvas.scale = w.calculatedScale() - w.canvas.texScale = w.detectTextureScale() - // update window size now we have scaled detected - w.fitContent() - - for _, fn := range w.pending { - fn() - } + w.setDarkMode() + + win.SetCloseCallback(w.closed) + win.SetPosCallback(w.moved) + win.SetSizeCallback(w.resized) + win.SetFramebufferSizeCallback(w.frameSized) + win.SetRefreshCallback(w.refresh) + win.SetContentScaleCallback(w.scaled) + win.SetCursorPosCallback(w.mouseMoved) + win.SetMouseButtonCallback(w.mouseClicked) + win.SetScrollCallback(w.mouseScrolled) + win.SetKeyCallback(w.keyPressed) + win.SetCharCallback(w.charInput) + win.SetFocusCallback(w.focused) + + w.canvas.detectedScale = w.detectScale() + w.canvas.scale = w.calculatedScale() + w.canvas.texScale = w.detectTextureScale() + // update window size now we have scaled detected + w.fitContent() - if w.FixedSize() && (w.requestedWidth == 0 || w.requestedHeight == 0) { - bigEnough := w.canvas.canvasSize(w.canvas.Content().MinSize()) - w.width, w.height = scale.ToScreenCoordinate(w.canvas, bigEnough.Width), scale.ToScreenCoordinate(w.canvas, bigEnough.Height) - w.shouldWidth, w.shouldHeight = w.width, w.height - } + for _, fn := range w.pending { + fn() + } - w.requestedWidth, w.requestedHeight = w.width, w.height - // order of operation matters so we do these last items in order - w.viewport.SetSize(w.shouldWidth, w.shouldHeight) // ensure we requested latest size - }) + if w.FixedSize() && (w.requestedWidth == 0 || w.requestedHeight == 0) { + bigEnough := w.canvas.canvasSize(w.canvas.Content().MinSize()) + w.width, w.height = scale.ToScreenCoordinate(w.canvas, bigEnough.Width), scale.ToScreenCoordinate(w.canvas, bigEnough.Height) + w.shouldWidth, w.shouldHeight = w.width, w.height + } + + w.requestedWidth, w.requestedHeight = w.width, w.height + // order of operation matters so we do these last items in order + w.viewport.SetSize(w.shouldWidth, w.shouldHeight) // ensure we requested latest size } func (w *window) view() *glfw.Window { diff --git a/internal/driver/glfw/window_wasm.go b/internal/driver/glfw/window_wasm.go index c33a2cd97d..8d5e5721a0 100644 --- a/internal/driver/glfw/window_wasm.go +++ b/internal/driver/glfw/window_wasm.go @@ -483,73 +483,68 @@ func (w *window) rescaleOnMain() { } func (w *window) create() { - runOnMain(func() { - // we can't hide the window in webgl, so there might be some artifact - initWindowHints() - - pixWidth, pixHeight := w.screenSize(w.canvas.size) - pixWidth = int(fyne.Max(float32(pixWidth), float32(w.width))) - if pixWidth == 0 { - pixWidth = 10 - } - pixHeight = int(fyne.Max(float32(pixHeight), float32(w.height))) - if pixHeight == 0 { - pixHeight = 10 - } + // we can't hide the window in webgl, so there might be some artifact + initWindowHints() - win, err := glfw.CreateWindow(pixWidth, pixHeight, w.title, nil, nil) - if err != nil { - w.driver.initFailed("window creation error", err) - return - } + pixWidth, pixHeight := w.screenSize(w.canvas.size) + pixWidth = int(fyne.Max(float32(pixWidth), float32(w.width))) + if pixWidth == 0 { + pixWidth = 10 + } + pixHeight = int(fyne.Max(float32(pixHeight), float32(w.height))) + if pixHeight == 0 { + pixHeight = 10 + } - w.viewLock.Lock() - w.viewport = win - w.viewLock.Unlock() - }) + win, err := glfw.CreateWindow(pixWidth, pixHeight, w.title, nil, nil) + if err != nil { + w.driver.initFailed("window creation error", err) + return + } + + w.viewLock.Lock() + w.viewport = win + w.viewLock.Unlock() if w.view() == nil { // something went wrong above, it will have been logged return } // run the GL init on the draw thread - runOnMainWithContext(w, func() { + w.RunWithContext(func() { w.canvas.SetPainter(gl.NewPainter(w.canvas, w)) w.canvas.Painter().Init() }) - runOnMain(func() { - w.setDarkMode() - - win := w.view() - win.SetCloseCallback(w.closed) - win.SetPosCallback(w.moved) - win.SetSizeCallback(w.resized) - win.SetFramebufferSizeCallback(w.frameSized) - win.SetRefreshCallback(w.refresh) - win.SetCursorPosCallback(w.mouseMoved) - win.SetMouseButtonCallback(w.mouseClicked) - win.SetScrollCallback(w.mouseScrolled) - win.SetKeyCallback(w.keyPressed) - win.SetCharCallback(w.charInput) - win.SetFocusCallback(w.focused) - - w.canvas.detectedScale = w.detectScale() - w.canvas.scale = w.calculatedScale() - w.canvas.texScale = w.detectTextureScale() - // update window size now we have scaled detected - w.fitContent() - - for _, fn := range w.pending { - fn() - } + w.setDarkMode() + + win.SetCloseCallback(w.closed) + win.SetPosCallback(w.moved) + win.SetSizeCallback(w.resized) + win.SetFramebufferSizeCallback(w.frameSized) + win.SetRefreshCallback(w.refresh) + win.SetCursorPosCallback(w.mouseMoved) + win.SetMouseButtonCallback(w.mouseClicked) + win.SetScrollCallback(w.mouseScrolled) + win.SetKeyCallback(w.keyPressed) + win.SetCharCallback(w.charInput) + win.SetFocusCallback(w.focused) + + w.canvas.detectedScale = w.detectScale() + w.canvas.scale = w.calculatedScale() + w.canvas.texScale = w.detectTextureScale() + // update window size now we have scaled detected + w.fitContent() + + for _, fn := range w.pending { + fn() + } - w.requestedWidth, w.requestedHeight = w.width, w.height + w.requestedWidth, w.requestedHeight = w.width, w.height - width, height := win.GetSize() - w.processFrameSized(width, height) - w.processResized(width, height) - }) + width, height := win.GetSize() + w.processFrameSized(width, height) + w.processResized(width, height) } func (w *window) view() *glfw.Window { From 3261a079414708220a7b5415c488399bc8ead057 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Tue, 17 Dec 2024 16:20:37 +0000 Subject: [PATCH 03/13] Fix compile error on CI --- internal/driver/glfw/window_wasm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/driver/glfw/window_wasm.go b/internal/driver/glfw/window_wasm.go index 8d5e5721a0..4367155c12 100644 --- a/internal/driver/glfw/window_wasm.go +++ b/internal/driver/glfw/window_wasm.go @@ -463,7 +463,7 @@ func (w *window) DetachCurrentContext() { glfw.DetachCurrentContext() } -func (w *window) rescaleOnMain() { +func (w *window) RescaleContext() { if w.viewport == nil { return } From 70f9cf8cee440919c5fee361602b52f28d5d8e87 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Tue, 17 Dec 2024 17:15:25 +0000 Subject: [PATCH 04/13] Removing windowlock as well But make sure our unit tests still use correct context --- internal/driver/glfw/driver.go | 12 +----------- internal/driver/glfw/loop.go | 2 -- internal/driver/glfw/window.go | 2 -- internal/driver/glfw/window_test.go | 7 +++++-- 4 files changed, 6 insertions(+), 17 deletions(-) diff --git a/internal/driver/glfw/driver.go b/internal/driver/glfw/driver.go index 23d28dbef5..0cd5870962 100644 --- a/internal/driver/glfw/driver.go +++ b/internal/driver/glfw/driver.go @@ -7,7 +7,6 @@ import ( "image" "os" "runtime" - "sync" "github.com/fyne-io/image/ico" @@ -33,7 +32,6 @@ var curWindow *window var _ fyne.Driver = (*gLDriver)(nil) type gLDriver struct { - windowLock sync.RWMutex windows []fyne.Window done chan struct{} waitForStart chan struct{} @@ -104,8 +102,6 @@ func (d *gLDriver) Quit() { } func (d *gLDriver) addWindow(w *window) { - d.windowLock.Lock() - defer d.windowLock.Unlock() d.windows = append(d.windows, w) } @@ -113,12 +109,8 @@ func (d *gLDriver) addWindow(w *window) { // This may not do the right thing if your app has 3 or more windows open, but it was agreed this was not much // of an issue, and the added complexity to track focus was not needed at this time. func (d *gLDriver) focusPreviousWindow() { - d.windowLock.RLock() - wins := d.windows - d.windowLock.RUnlock() - var chosen *window - for _, w := range wins { + for _, w := range d.windows { win := w.(*window) if !win.visible { continue @@ -136,8 +128,6 @@ func (d *gLDriver) focusPreviousWindow() { } func (d *gLDriver) windowList() []fyne.Window { - d.windowLock.RLock() - defer d.windowLock.RUnlock() return d.windows } diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index 452a7f422d..8dc57724e4 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -167,9 +167,7 @@ func (d *gLDriver) runGL() { newWindows = append(newWindows, win) } - d.windowLock.Lock() d.windows = newWindows - d.windowLock.Unlock() if len(newWindows) == 0 { d.Quit() diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index 1f36b20c34..def454bf22 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -875,7 +875,6 @@ func (d *gLDriver) CreateWindow(title string) fyne.Window { // handling multiple windows by overlaying on the root for web var root fyne.Window - d.windowLock.RLock() hasVisible := false for _, w := range d.windows { if w.(*window).visible { @@ -884,7 +883,6 @@ func (d *gLDriver) CreateWindow(title string) fyne.Window { break } } - d.windowLock.RUnlock() if !hasVisible { return d.createWindow(title, true) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index e18d5d960d..a45544370e 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -1678,8 +1678,11 @@ func TestWindow_SetFullScreen(t *testing.T) { // } func createWindow(title string) fyne.Window { - w := d.CreateWindow(title) - w.(*window).create() + var w fyne.Window + runOnMain(func() { + w = d.CreateWindow(title) + w.(*window).create() + }) return w } From 6c2702f7f162e289b008491b719ae0bde0434bb5 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Tue, 17 Dec 2024 17:25:00 +0000 Subject: [PATCH 05/13] We should not have to wait now --- internal/driver/glfw/window.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index def454bf22..fd32af36b8 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -742,7 +742,6 @@ func (w *window) processFocused(focus bool) { w.mousePos = fyne.Position{} // check whether another window was focused or not - time.Sleep(time.Millisecond * 100) if curWindow != w { return } From d64b90ac4778703d7f16ff6cd0dcbfb05d5125af Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Tue, 17 Dec 2024 19:53:31 +0000 Subject: [PATCH 06/13] Correct test -> thread for splash windows --- internal/driver/glfw/window_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index a45544370e..8f543d14a8 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -68,8 +68,7 @@ func TestGLDriver_CreateWindow_EmptyTitle(t *testing.T) { } func TestGLDriver_CreateSplashWindow(t *testing.T) { - d := NewGLDriver() - w := d.CreateSplashWindow().(*window) + w := createSplashWindow().(*window) w.create() // Verify that the glfw driver implements desktop.Driver. @@ -1677,6 +1676,15 @@ func TestWindow_SetFullScreen(t *testing.T) { // assert.True(t, w.FullScreen()) // } +func createSplashWindow() fyne.Window { + var w fyne.Window + runOnMain(func() { + w = d.CreateSplashWindow() + w.(*window).create() + }) + return w +} + func createWindow(title string) fyne.Window { var w fyne.Window runOnMain(func() { From 415f57eae5706af685dbc2ae78823656403544e2 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Tue, 17 Dec 2024 20:02:02 +0000 Subject: [PATCH 07/13] Clarify thread handling of specific window tests --- internal/driver/glfw/window_test.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index 8f543d14a8..de364386ef 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -68,7 +68,10 @@ func TestGLDriver_CreateWindow_EmptyTitle(t *testing.T) { } func TestGLDriver_CreateSplashWindow(t *testing.T) { - w := createSplashWindow().(*window) + var w *window + runOnMain(func() { // tests launch in a different context + w = d.CreateSplashWindow().(*window) + }) w.create() // Verify that the glfw driver implements desktop.Driver. @@ -1641,7 +1644,10 @@ func TestWindow_SetContent_Twice(t *testing.T) { } func TestWindow_SetFullScreen(t *testing.T) { - w := d.CreateWindow("Full").(*window) + var w *window + runOnMain(func() { // tests launch in a different context + w = d.CreateWindow("Full").(*window) + }) w.SetFullScreen(true) w.create() @@ -1676,18 +1682,9 @@ func TestWindow_SetFullScreen(t *testing.T) { // assert.True(t, w.FullScreen()) // } -func createSplashWindow() fyne.Window { - var w fyne.Window - runOnMain(func() { - w = d.CreateSplashWindow() - w.(*window).create() - }) - return w -} - func createWindow(title string) fyne.Window { var w fyne.Window - runOnMain(func() { + runOnMain(func() { // tests launch in a different context w = d.CreateWindow(title) w.(*window).create() }) From a2fb72eefe6a3d6c2d468e0f2b40030adcad3459 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Tue, 17 Dec 2024 20:09:33 +0000 Subject: [PATCH 08/13] Adding missed usages of window helper --- internal/driver/glfw/window_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index de364386ef..09ff984691 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -55,7 +55,6 @@ func TestMain(m *testing.M) { func TestGLDriver_CreateWindow(t *testing.T) { w := createWindow("Test").(*window) - w.create() assert.Equal(t, 1, w.viewport.GetAttrib(glfw.Decorated)) assert.True(t, w.Padded()) @@ -1574,8 +1573,7 @@ func TestWindow_CloseInterception(t *testing.T) { d := NewGLDriver() t.Run("when closing window with #Close()", func(t *testing.T) { - w := d.CreateWindow("test").(*window) - w.create() + w := createWindow("test").(*window) onIntercepted := false onClosed := false w.SetCloseIntercept(func() { onIntercepted = true }) @@ -1589,8 +1587,7 @@ func TestWindow_CloseInterception(t *testing.T) { }) t.Run("when window is closed from the outside (notified by GLFW callback)", func(t *testing.T) { - w := d.CreateWindow("test").(*window) - w.create() + w := createWindow("test").(*window) onIntercepted := false w.SetCloseIntercept(func() { onIntercepted = true }) closed := make(chan bool, 1) @@ -1608,8 +1605,7 @@ func TestWindow_CloseInterception(t *testing.T) { }) t.Run("when window is closed from the outside but no interceptor is set", func(t *testing.T) { - w := d.CreateWindow("test").(*window) - w.create() + w := createWindow("test").(*window) closed := make(chan bool, 1) w.SetOnClosed(func() { closed <- true }) w.closed(w.viewport) From eb791d60279ecd0a3ee027d996a3cb357718fb51 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Tue, 17 Dec 2024 20:15:19 +0000 Subject: [PATCH 09/13] Looking for fullscreen test fix --- internal/driver/glfw/window_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index 09ff984691..94d7428a12 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -1643,11 +1643,10 @@ func TestWindow_SetFullScreen(t *testing.T) { var w *window runOnMain(func() { // tests launch in a different context w = d.CreateWindow("Full").(*window) + w.SetFullScreen(true) + w.create() }) - w.SetFullScreen(true) - w.create() - w.create() w.doShow() waitForMain() assert.Zero(t, w.width) From ea1ee739ddbdd1e23839af36dedd80ecb222cdf9 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Tue, 17 Dec 2024 20:33:45 +0000 Subject: [PATCH 10/13] Tidy away more wait code --- internal/driver/glfw/window_test.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index 94d7428a12..550f066383 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -70,8 +70,8 @@ func TestGLDriver_CreateSplashWindow(t *testing.T) { var w *window runOnMain(func() { // tests launch in a different context w = d.CreateSplashWindow().(*window) + w.create() }) - w.create() // Verify that the glfw driver implements desktop.Driver. var driver fyne.Driver = d @@ -1050,7 +1050,6 @@ func TestWindow_TappedSecondary(t *testing.T) { o := &tappableObject{Rectangle: canvas.NewRectangle(color.White)} o.SetMinSize(fyne.NewSize(100, 100)) w.SetContent(o) - waitForMain() w.mousePos = fyne.NewPos(50, 60) w.mouseClicked(w.viewport, glfw.MouseButton2, glfw.Press, 0) @@ -1070,7 +1069,6 @@ func TestWindow_TappedSecondary_OnPrimaryOnlyTarget(t *testing.T) { tapped = true }) w.SetContent(o) - waitForMain() ensureCanvasSize(t, w, fyne.NewSize(53, 44)) w.mousePos = fyne.NewPos(10, 25) @@ -1648,12 +1646,10 @@ func TestWindow_SetFullScreen(t *testing.T) { }) w.doShow() - waitForMain() assert.Zero(t, w.width) assert.Zero(t, w.height) w.SetFullScreen(false) - waitForMain() // ensure we realised size now! assert.NotZero(t, w.width) assert.NotZero(t, w.height) @@ -1932,10 +1928,6 @@ func newDoubleTappableButton() *doubleTappableButton { return but } -func waitForMain() { - runOnMain(func() {}) // this blocks until processed -} - type tabbable struct { focusable acceptTabCallCount int From d4293fdb4315e36a9e2403dff9f511adb8650395 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Fri, 20 Dec 2024 08:13:46 +0000 Subject: [PATCH 11/13] Refactor GLFW launch waiting Just use in the tests where we still have to juggle threads --- internal/driver/glfw/driver.go | 3 +-- internal/driver/glfw/glfw_test.go | 4 ---- internal/driver/glfw/loop.go | 1 - internal/driver/glfw/window.go | 8 -------- internal/driver/glfw/window_test.go | 11 ++++++++--- 5 files changed, 9 insertions(+), 18 deletions(-) diff --git a/internal/driver/glfw/driver.go b/internal/driver/glfw/driver.go index 0cd5870962..727e71b3a3 100644 --- a/internal/driver/glfw/driver.go +++ b/internal/driver/glfw/driver.go @@ -164,7 +164,6 @@ func NewGLDriver() *gLDriver { repository.Register("file", intRepo.NewFileRepository()) return &gLDriver{ - done: make(chan struct{}), - waitForStart: make(chan struct{}), + done: make(chan struct{}), } } diff --git a/internal/driver/glfw/glfw_test.go b/internal/driver/glfw/glfw_test.go index cf7edec2b7..13c4672185 100644 --- a/internal/driver/glfw/glfw_test.go +++ b/internal/driver/glfw/glfw_test.go @@ -32,10 +32,6 @@ func ensureCanvasSize(t *testing.T, w *window, size fyne.Size) { } func repaintWindow(w *window) { - // Wait for GLFW loop to be running. - // If we try to paint windows before the context is created, we will end up on the wrong thread. - <-w.driver.waitForStart - w.RunWithContext(func() { d.repaintWindow(w) }) diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index 8dc57724e4..d978b67c3b 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -86,7 +86,6 @@ func (d *gLDriver) runGL() { if !running.CompareAndSwap(false, true) { return // Run was called twice. } - close(d.waitForStart) // Signal that execution can continue. d.initGLFW() if d.trayStart != nil { diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index fd32af36b8..194e0811a0 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -129,19 +129,11 @@ func (w *window) detectTextureScale() float32 { } func (w *window) Show() { - go func() { - runOnMain(w.doShow) - }() -} - -func (w *window) doShow() { if w.view() != nil { w.doShowAgain() return } - <-w.driver.waitForStart - w.createLock.Do(w.create) if w.view() == nil { return diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index 550f066383..3d162c16a3 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -37,10 +37,13 @@ func init() { // This must be done for some of our tests to function correctly. func TestMain(m *testing.M) { d.initGLFW() + + waitForStart := make(chan struct{}) go func() { - // Wait for GLFW loop to be running. + // Wait for GLFW loop to be running (plus a moment in case od scheduling switches). // If we try to create windows before the context is created, this will fail with an exception. - <-d.waitForStart + <-waitForStart + time.Sleep(time.Millisecond * 100) initMainMenu() os.Exit(m.Run()) @@ -50,6 +53,8 @@ func TestMain(m *testing.M) { master.SetOnClosed(func() { // we do not close, keeping the driver running }) + + close(waitForStart) // Signal that execution can continue. d.Run() } @@ -1645,7 +1650,7 @@ func TestWindow_SetFullScreen(t *testing.T) { w.create() }) - w.doShow() + w.Show() assert.Zero(t, w.width) assert.Zero(t, w.height) From af0abaa5823aa0cbd2847fef21022a7c7392ae47 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Fri, 20 Dec 2024 08:38:50 +0000 Subject: [PATCH 12/13] Fix fullscreen and double tap threading --- internal/driver/glfw/loop.go | 4 ++-- internal/driver/glfw/window.go | 31 +++++++++++++------------- internal/driver/glfw/window_desktop.go | 24 +++++++++----------- 3 files changed, 28 insertions(+), 31 deletions(-) diff --git a/internal/driver/glfw/loop.go b/internal/driver/glfw/loop.go index d978b67c3b..8662a0266a 100644 --- a/internal/driver/glfw/loop.go +++ b/internal/driver/glfw/loop.go @@ -92,7 +92,7 @@ func (d *gLDriver) runGL() { d.trayStart() } if f := fyne.CurrentApp().Lifecycle().(*app.Lifecycle).OnStarted(); f != nil { - go f() // don't block main, we don't have window event queue + f() } settingsChange := make(chan fyne.Settings) @@ -181,7 +181,7 @@ func (d *gLDriver) runGL() { return } c.applyThemeOutOfTreeObjects() - go c.reloadScale() + c.reloadScale() }) } diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index 194e0811a0..971cced4a1 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -154,10 +154,7 @@ func (w *window) Show() { } if w.fullScreen { // this does not work if called before viewport.Show() - go func() { - time.Sleep(time.Millisecond * 100) - w.SetFullScreen(true) - }() + w.SetFullScreen(true) } // show top canvas element @@ -257,7 +254,7 @@ func (w *window) destroy(d *gLDriver) { if w.master { d.Quit() } else if runtime.GOOS == "darwin" { - go d.focusPreviousWindow() + d.focusPreviousWindow() } } @@ -272,7 +269,7 @@ func (w *window) processMoved(x, y int) { } w.canvas.detectedScale = w.detectScale() - go w.canvas.reloadScale() + w.canvas.reloadScale() } func (w *window) processResized(width, height int) { @@ -562,11 +559,7 @@ func (w *window) mouseClickedHandleTapDoubleTap(co fyne.CanvasObject, ev *fyne.P if doubleTap { w.mouseClickCount++ w.mouseLastClick = co - mouseCancelFunc := w.mouseCancelFunc - if mouseCancelFunc != nil { - mouseCancelFunc() - return - } + go w.waitForDoubleTap(co, ev) } else { if wid, ok := co.(fyne.Tappable); ok && co == w.mousePressed { @@ -577,12 +570,21 @@ func (w *window) mouseClickedHandleTapDoubleTap(co fyne.CanvasObject, ev *fyne.P } func (w *window) waitForDoubleTap(co fyne.CanvasObject, ev *fyne.PointEvent) { - var ctx context.Context - ctx, w.mouseCancelFunc = context.WithDeadline(context.TODO(), time.Now().Add(w.driver.DoubleTapDelay())) - defer w.mouseCancelFunc() + ctx, mouseCancelFunc := context.WithDeadline(context.TODO(), time.Now().Add(w.driver.DoubleTapDelay())) + if mouseCancelFunc != nil { + defer func() { + runOnMain(mouseCancelFunc) + }() + } <-ctx.Done() + runOnMain(func() { + w.waitForDoubleTapEnded(co, ev) + }) +} + +func (w *window) waitForDoubleTapEnded(co fyne.CanvasObject, ev *fyne.PointEvent) { if w.mouseClickCount == 2 && w.mouseLastClick == co { if wid, ok := co.(fyne.DoubleTappable); ok { wid.DoubleTapped(ev) @@ -595,7 +597,6 @@ func (w *window) waitForDoubleTap(co fyne.CanvasObject, ev *fyne.PointEvent) { w.mouseClickCount = 0 w.mousePressed = nil - w.mouseCancelFunc = nil w.mouseLastClick = nil } diff --git a/internal/driver/glfw/window_desktop.go b/internal/driver/glfw/window_desktop.go index 79f3e5f250..015a19689c 100644 --- a/internal/driver/glfw/window_desktop.go +++ b/internal/driver/glfw/window_desktop.go @@ -4,7 +4,6 @@ package glfw import ( "bytes" - "context" "image" _ "image/png" // for the icon "runtime" @@ -90,7 +89,6 @@ type window struct { mouseLastClick fyne.CanvasObject mousePressed fyne.CanvasObject mouseClickCount int - mouseCancelFunc context.CancelFunc onClosed func() onCloseIntercepted func() @@ -113,20 +111,18 @@ func (w *window) SetFullScreen(full bool) { return } - runOnMain(func() { - monitor := w.getMonitorForWindow() - mode := monitor.GetVideoMode() + monitor := w.getMonitorForWindow() + mode := monitor.GetVideoMode() - if full { - w.viewport.SetMonitor(monitor, 0, 0, mode.Width, mode.Height, mode.RefreshRate) - } else { - if w.width == 0 && w.height == 0 { // if we were fullscreen on creation... - s := w.canvas.Size().Max(w.canvas.MinSize()) - w.width, w.height = w.screenSize(s) - } - w.viewport.SetMonitor(nil, w.xpos, w.ypos, w.width, w.height, 0) + if full { + w.viewport.SetMonitor(monitor, 0, 0, mode.Width, mode.Height, mode.RefreshRate) + } else { + if w.width == 0 && w.height == 0 { // if we were fullscreen on creation... + s := w.canvas.Size().Max(w.canvas.MinSize()) + w.width, w.height = w.screenSize(s) } - }) + w.viewport.SetMonitor(nil, w.xpos, w.ypos, w.width, w.height, 0) + } } func (w *window) CenterOnScreen() { From d6b762607a588d4cfa77285b5ce332cdc1efde02 Mon Sep 17 00:00:00 2001 From: Andy Williams Date: Fri, 20 Dec 2024 08:48:35 +0000 Subject: [PATCH 13/13] Tidy trailing chan removal --- internal/driver/glfw/driver.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/driver/glfw/driver.go b/internal/driver/glfw/driver.go index 727e71b3a3..fbaeb951e0 100644 --- a/internal/driver/glfw/driver.go +++ b/internal/driver/glfw/driver.go @@ -32,9 +32,8 @@ var curWindow *window var _ fyne.Driver = (*gLDriver)(nil) type gLDriver struct { - windows []fyne.Window - done chan struct{} - waitForStart chan struct{} + windows []fyne.Window + done chan struct{} animation animation.Runner