Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move events to main thread and remove unneeded locks #5322

Merged
merged 9 commits into from
Dec 17, 2024
26 changes: 0 additions & 26 deletions internal/driver/common/canvas.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ type SizeableCanvas interface {

// Canvas defines common canvas implementation.
type Canvas struct {
sync.RWMutex

OnFocus func(obj fyne.Focusable)
OnUnfocus func()

Expand Down Expand Up @@ -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()
}
})

Expand All @@ -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 {
Expand All @@ -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)
}
Expand All @@ -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
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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()
}
}
14 changes: 9 additions & 5 deletions internal/driver/common/window.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@ 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()
// ProcessEventQueue runs all the items in the event queue, returning once it is empty again.
func (w *Window) ProcessEventQueue() {
for {
select {
case fn := <-w.eventQueue.Out():
fn()
default:
return
}
}
}

Expand Down
59 changes: 10 additions & 49 deletions internal/driver/glfw/canvas.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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())
}

Expand All @@ -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)))
}
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 0 additions & 6 deletions internal/driver/glfw/canvas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()))
}

Expand Down
2 changes: 0 additions & 2 deletions internal/driver/glfw/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Loading
Loading