Merge pull request #5322 from andydotxyz/fix/mergethreads

Move events to main thread and remove unneeded locks

internal/driver/common/canvas.go

@@ -25,8 +25,6 @@ type SizeableCanvas interface {
 
 // Canvas defines common canvas implementation.
 type Canvas struct {
-	sync.RWMutex
-
 	OnFocus   func(obj fyne.Focusable)
 	OnUnfocus func()
 
@@ -91,18 +89,13 @@ func (c *Canvas) EnsureMinSize() bool {
 	csize := c.impl.Size()
 	min := c.impl.MinSize()
 
-	c.RLock()
-	defer c.RUnlock()
-
 	var parentNeedingUpdate *RenderCacheNode
 
 	ensureMinSize := func(node *RenderCacheNode, pos fyne.Position) {
 		obj := node.obj
 		cache.SetCanvasForObject(obj, c.impl, func() {
 			if img, ok := obj.(*canvas.Image); ok {
-				c.RUnlock()
 				img.Refresh() // this may now have a different texScale
-				c.RLock()
 			}
 		})
 
@@ -111,13 +104,10 @@ func (c *Canvas) EnsureMinSize() bool {
 			parentNeedingUpdate = nil
 		}
 
-		c.RUnlock()
 		if !obj.Visible() {
-			c.RLock()
 			return
 		}
 		minSize := obj.MinSize()
-		c.RLock()
 
 		minSizeChanged := node.minSize != minSize
 		if minSizeChanged {
@@ -126,14 +116,10 @@ func (c *Canvas) EnsureMinSize() bool {
 				parentNeedingUpdate = node.parent
 			} else {
 				windowNeedsMinSizeUpdate = true
-				c.RUnlock()
 				size := obj.Size()
-				c.RLock()
 				expectedSize := minSize.Max(size)
 				if expectedSize != size && size != csize {
-					c.RUnlock()
 					obj.Resize(expectedSize)
-					c.RLock()
 				} else {
 					c.updateLayout(obj)
 				}
@@ -144,9 +130,7 @@ func (c *Canvas) EnsureMinSize() bool {
 
 	shouldResize := windowNeedsMinSizeUpdate && (csize.Width < min.Width || csize.Height < min.Height)
 	if shouldResize {
-		c.RUnlock()
 		c.impl.Resize(csize.Max(min))
-		c.RLock()
 	}
 	return windowNeedsMinSizeUpdate
 }
@@ -161,9 +145,7 @@ func (c *Canvas) Focus(obj fyne.Focusable) {
 		return
 	}
 
-	c.RLock()
 	focusMgrs := append([]*app.FocusManager{c.contentFocusMgr, c.menuFocusMgr}, c.overlays.ListFocusManagers()...)
-	c.RUnlock()
 
 	for _, mgr := range focusMgrs {
 		if mgr == nil {
@@ -305,15 +287,13 @@ func (c *Canvas) Initialize(impl SizeableCanvas, onOverlayChanged func()) {
 //
 // This function uses lock.
 func (c *Canvas) ObjectTrees() []fyne.CanvasObject {
-	c.RLock()
 	var content, menu fyne.CanvasObject
 	if c.contentTree != nil && c.contentTree.root != nil {
 		content = c.contentTree.root.obj
 	}
 	if c.menuTree != nil && c.menuTree.root != nil {
 		menu = c.menuTree.root.obj
 	}
-	c.RUnlock()
 	trees := make([]fyne.CanvasObject, 0, len(c.Overlays().List())+2)
 	trees = append(trees, content)
 	if menu != nil {
@@ -434,8 +414,6 @@ func (c *Canvas) focusManager() *app.FocusManager {
 	if focusMgr := c.overlays.TopFocusManager(); focusMgr != nil {
 		return focusMgr
 	}
-	c.RLock()
-	defer c.RUnlock()
 	if c.isMenuActive() {
 		return c.menuFocusMgr
 	}
@@ -580,14 +558,10 @@ func (c *Canvas) updateLayout(objToLayout fyne.CanvasObject) {
 		if cont.Layout != nil {
 			layout := cont.Layout
 			objects := cont.Objects
-			c.RUnlock()
 			layout.Layout(objects, cont.Size())
-			c.RLock()
 		}
 	case fyne.Widget:
 		renderer := cache.Renderer(cont)
-		c.RUnlock()
 		renderer.Layout(cont.Size())
-		c.RLock()
 	}
 }

internal/driver/common/window.go

@@ -4,45 +4,6 @@ import (
 	"fyne.io/fyne/v2/internal/async"
 )
 
-// Window defines common functionality for windows.
-type Window struct {
-	eventQueue *async.UnboundedFuncChan
-}
-
-// DestroyEventQueue destroys the event queue.
-func (w *Window) DestroyEventQueue() {
-	w.eventQueue.Close()
-}
-
-// InitEventQueue initializes the event queue.
-func (w *Window) InitEventQueue() {
-	// This channel should be closed when the window is closed.
-	w.eventQueue = async.NewUnboundedFuncChan()
-}
-
-// QueueEvent uses this method to queue up a callback that handles an event. This ensures
-// user interaction events for a given window are processed in order.
-func (w *Window) QueueEvent(fn func()) {
-	w.eventQueue.In() <- fn
-}
-
-// RunEventQueue runs the event queue. This should called inside a go routine.
-// This function blocks.
-func (w *Window) RunEventQueue() {
-	for fn := range w.eventQueue.Out() {
-		fn()
-	}
-}
-
-// WaitForEvents wait for all the events.
-func (w *Window) WaitForEvents() {
-	done := DonePool.Get()
-	defer DonePool.Put(done)
-
-	w.eventQueue.In() <- func() { done <- struct{}{} }
-	<-done
-}
-
 var DonePool = async.Pool[chan struct{}]{
 	New: func() chan struct{} {
 		return make(chan struct{})

internal/driver/glfw/canvas.go

@@ -48,18 +48,12 @@ func (c *glCanvas) Capture() image.Image {
 }
 
 func (c *glCanvas) Content() fyne.CanvasObject {
-	c.RLock()
-	retval := c.content
-	c.RUnlock()
-	return retval
+	return c.content
 }
 
 func (c *glCanvas) DismissMenu() bool {
-	c.RLock()
-	menu := c.menu
-	c.RUnlock()
-	if menu != nil && menu.(*MenuBar).IsActive() {
-		menu.(*MenuBar).Toggle()
+	if c.menu != nil && c.menu.(*MenuBar).IsActive() {
+		c.menu.(*MenuBar).Toggle()
 		return true
 	}
 	return false
@@ -70,8 +64,6 @@ func (c *glCanvas) InteractiveArea() (fyne.Position, fyne.Size) {
 }
 
 func (c *glCanvas) MinSize() fyne.Size {
-	c.RLock()
-	defer c.RUnlock()
 	return c.canvasSize(c.content.MinSize())
 }
 
@@ -96,11 +88,7 @@ func (c *glCanvas) Padded() bool {
 }
 
 func (c *glCanvas) PixelCoordinateForPosition(pos fyne.Position) (int, int) {
-	c.RLock()
-	texScale := c.texScale
-	scale := c.scale
-	c.RUnlock()
-	multiple := scale * texScale
+	multiple := c.scale * c.texScale
 	scaleInt := func(x float32) int {
 		return int(math.Round(float64(x * multiple)))
 	}
@@ -114,9 +102,7 @@ func (c *glCanvas) Resize(size fyne.Size) {
 	// This can easily be seen with fyne/cmd/hello and a scale == 1 as the text will happear blurry without the following line.
 	nearestSize := fyne.NewSize(float32(math.Ceil(float64(size.Width))), float32(math.Ceil(float64(size.Height))))
 
-	c.Lock()
 	c.size = nearestSize
-	c.Unlock()
 
 	if c.webExtraWindows != nil {
 		c.webExtraWindows.Resize(size)
@@ -131,13 +117,11 @@ func (c *glCanvas) Resize(size fyne.Size) {
 		}
 	}
 
-	c.RLock()
 	content := c.content
 	contentSize := c.contentSize(nearestSize)
 	contentPos := c.contentPos()
 	menu := c.menu
 	menuHeight := c.menuHeight()
-	c.RUnlock()
 
 	content.Resize(contentSize)
 	content.Move(contentPos)
@@ -149,20 +133,16 @@ func (c *glCanvas) Resize(size fyne.Size) {
 }
 
 func (c *glCanvas) Scale() float32 {
-	c.RLock()
-	defer c.RUnlock()
 	return c.scale
 }
 
 func (c *glCanvas) SetContent(content fyne.CanvasObject) {
 	content.Resize(content.MinSize()) // give it the space it wants then calculate the real min
 
-	c.Lock()
 	// the pass above makes some layouts wide enough to wrap, so we ask again what the true min is.
 	newSize := c.size.Max(c.canvasSize(content.MinSize()))
 
 	c.setContent(content)
-	c.Unlock()
 
 	c.Resize(newSize)
 	c.SetDirty()
@@ -185,50 +165,35 @@ func (c *glCanvas) SetOnTypedRune(typed func(rune)) {
 }
 
 func (c *glCanvas) SetPadded(padded bool) {
-	c.Lock()
-	content := c.content
 	c.padded = padded
-	pos := c.contentPos()
-	c.Unlock()
 
-	content.Move(pos)
+	c.content.Move(c.contentPos())
 }
 
 func (c *glCanvas) reloadScale() {
 	w := c.context.(*window)
-	w.viewLock.RLock()
 	windowVisible := w.visible
-	w.viewLock.RUnlock()
 	if !windowVisible {
 		return
 	}
 
-	c.Lock()
 	c.scale = w.calculatedScale()
-	c.Unlock()
 	c.SetDirty()
 
 	c.context.RescaleContext()
 }
 
 func (c *glCanvas) Size() fyne.Size {
-	c.RLock()
-	defer c.RUnlock()
 	return c.size
 }
 
 func (c *glCanvas) ToggleMenu() {
-	c.RLock()
-	menu := c.menu
-	c.RUnlock()
-	if menu != nil {
-		menu.(*MenuBar).Toggle()
+	if c.menu != nil {
+		c.menu.(*MenuBar).Toggle()
 	}
 }
 
 func (c *glCanvas) buildMenu(w *window, m *fyne.MainMenu) {
-	c.Lock()
-	defer c.Unlock()
 	c.setMenuOverlay(nil)
 	if m == nil {
 		return
@@ -331,15 +296,11 @@ func (c *glCanvas) setMenuOverlay(b fyne.CanvasObject) {
 }
 
 func (c *glCanvas) applyThemeOutOfTreeObjects() {
-	c.RLock()
-	menu := c.menu
-	padded := c.padded
-	c.RUnlock()
-	if menu != nil {
-		app.ApplyThemeTo(menu, c) // Ensure our menu gets the theme change message as it's out-of-tree
+	if c.menu != nil {
+		app.ApplyThemeTo(c.menu, c) // Ensure our menu gets the theme change message as it's out-of-tree
 	}
 
-	c.SetPadded(padded) // refresh the padding for potential theme differences
+	c.SetPadded(c.padded) // refresh the padding for potential theme differences
 }
 
 func newCanvas() *glCanvas {

internal/driver/glfw/canvas_test.go

@@ -368,16 +368,12 @@ func TestGlCanvas_PixelCoordinateAtPosition(t *testing.T) {
 	c := w.Canvas().(*glCanvas)
 
 	pos := fyne.NewPos(4, 4)
-	c.Lock()
 	c.scale = 2.5
-	c.Unlock()
 	x, y := c.PixelCoordinateForPosition(pos)
 	assert.Equal(t, int(10*c.texScale), x)
 	assert.Equal(t, int(10*c.texScale), y)
 
-	c.Lock()
 	c.texScale = 2.0
-	c.Unlock()
 	x, y = c.PixelCoordinateForPosition(pos)
 	assert.Equal(t, 20, x)
 	assert.Equal(t, 20, y)
@@ -503,9 +499,7 @@ func TestGlCanvas_Scale(t *testing.T) {
 	w := createWindow("Test").(*window)
 	c := w.Canvas().(*glCanvas)
 
-	c.Lock()
 	c.scale = 2.5
-	c.Unlock()
 	assert.Equal(t, 5, int(2*c.Scale()))
 }
 

internal/driver/glfw/clipboard.go

@@ -39,11 +39,7 @@ func (c clipboard) Content() string {
 }
 
 func (c clipboard) content() string {
-	content := ""
-	runOnMain(func() {
-		content = glfw.GetClipboardString()
-	})
-	return content
+	return glfw.GetClipboardString()
 }
 
 // SetContent sets the clipboard content
@@ -64,7 +60,5 @@ func (c clipboard) SetContent(content string) {
 }
 
 func (c clipboard) setContent(content string) {
-	runOnMain(func() {
-		glfw.SetClipboardString(content)
-	})
+	glfw.SetClipboardString(content)
 }

internal/driver/glfw/driver.go

@@ -89,7 +89,7 @@ func (d *gLDriver) Device() fyne.Device {
 func (d *gLDriver) Quit() {
 	if curWindow != nil {
 		if f := fyne.CurrentApp().Lifecycle().(*intapp.Lifecycle).OnExitedForeground(); f != nil {
-			curWindow.QueueEvent(f)
+			f()
 		}
 		curWindow = nil
 		if d.trayStop != nil {

internal/driver/glfw/driver_test.go

@@ -40,9 +40,7 @@ func Test_gLDriver_AbsolutePositionForObject(t *testing.T) {
 	// We work around w.SetMainMenu because on MacOS the main menu is a native menu.
 	c := w.canvas
 	movl := buildMenuOverlay(mm, w)
-	c.Lock()
 	c.setMenuOverlay(movl)
-	c.Unlock()
 	w.SetContent(content)
 	w.Resize(fyne.NewSize(300, 200))
 	ensureCanvasSize(t, w, fyne.NewSize(300, 200))