Skip to content

Commit

Permalink
Merge pull request #5415 from andydotxyz/fix/smootherfynedotransition
Browse files Browse the repository at this point in the history
Moving window handling functions into a safer transition mode
  • Loading branch information
andydotxyz authored Jan 17, 2025
2 parents 334b6b4 + a3ff2a6 commit abef71c
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 67 deletions.
58 changes: 58 additions & 0 deletions internal/async/goroutine.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
package async

import (
"log"
"runtime"
"strings"

"fyne.io/fyne/v2"
"fyne.io/fyne/v2/internal/build"
)

// mainGoroutineID stores the main goroutine ID.
// This ID must be initialized in main.init because
// a main goroutine may not equal to 1 due to the
Expand All @@ -10,6 +19,55 @@ func init() {
mainGoroutineID = goroutineID()
}

// IsMainGoroutine returns true if it is called from the main goroutine, false otherwise.
func IsMainGoroutine() bool {
return goroutineID() == mainGoroutineID
}

// EnsureNotMain is part of our thread transition and makes sure that the passed function runs off main.
// If the context is running on a goroutine or the transition has been disabled this will blindly run.
// Otherwise, an error will be logged and the function will be called on a new goroutine.
//
// This will be removed later and should never be public
func EnsureNotMain(fn func()) {
if build.DisableThreadChecks || !IsMainGoroutine() {
fn()
return
}

log.Println("*** Error in Fyne call thread, fyne.Do called from main goroutine ***")

logStackTop(2)
go fn()
}

// EnsureMain is part of our thread transition and makes sure that the passed function runs on main.
// If the context is main or the transition has been disabled this will blindly run.
// Otherwise, an error will be logged and the function will be called on the main goroutine.
//
// This will be removed later and should never be public
func EnsureMain(fn func()) {
if build.DisableThreadChecks || IsMainGoroutine() {
fn()
return
}

log.Println("*** Error in Fyne call thread, this should have been called in fyne.Do ***")

logStackTop(1)
fyne.Do(fn)
}

func logStackTop(skip int) {
pc := make([]uintptr, 2)
count := runtime.Callers(2+skip, pc)
frames := runtime.CallersFrames(pc)
frame, more := frames.Next()
if more && count > 1 {
nextFrame, _ := frames.Next() // skip an occasional driver call to itself
if !strings.Contains(nextFrame.File, "runtime") { // don't descend into Go
frame = nextFrame
}
}
log.Printf(" From: %s:%d", frame.File, frame.Line)
}
2 changes: 2 additions & 0 deletions internal/build/build.go
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
// Package build contains information about they type of build currently running.
package build

const DisableThreadChecks = false
2 changes: 1 addition & 1 deletion internal/driver/glfw/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func toOSIcon(icon []byte) ([]byte, error) {
}

func (d *gLDriver) DoFromGoroutine(f func()) {
runOnMain(f)
async.EnsureNotMain(f)
}

func (d *gLDriver) RenderedTextSize(text string, textSize float32, style fyne.TextStyle, source fyne.Resource) (size fyne.Size, baseline float32) {
Expand Down
126 changes: 68 additions & 58 deletions internal/driver/glfw/window.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"fyne.io/fyne/v2/container"
"fyne.io/fyne/v2/driver/desktop"
"fyne.io/fyne/v2/internal/app"
"fyne.io/fyne/v2/internal/async"
"fyne.io/fyne/v2/internal/build"
"fyne.io/fyne/v2/internal/cache"
"fyne.io/fyne/v2/internal/driver"
Expand Down Expand Up @@ -129,79 +130,85 @@ func (w *window) detectTextureScale() float32 {
}

func (w *window) Show() {
if w.view() != nil {
w.doShowAgain()
return
}
async.EnsureMain(func() {
if w.view() != nil {
w.doShowAgain()
return
}

w.createLock.Do(w.create)
if w.view() == nil {
return
}
w.createLock.Do(w.create)
if w.view() == nil {
return
}

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()
w.SetFullScreen(true)
}
if w.fullScreen { // this does not work if called before viewport.Show()
w.SetFullScreen(true)
}

// 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()

w.RunWithContext(func() {
w.driver.repaintWindow(w)
})
}
w.RunWithContext(func() {
w.driver.repaintWindow(w)
})
}
})
}

func (w *window) Hide() {
if w.closing || w.viewport == nil {
return
}
async.EnsureMain(func() {
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() {
if w.isClosing() {
return
}
async.EnsureMain(func() {
if w.isClosing() {
return
}

// trigger callbacks - early so window still exists
if fn := w.onClosed; fn != nil {
w.onClosed = nil // avoid possibility of calling twice
fn()
}
// trigger callbacks - early so window still exists
if fn := w.onClosed; fn != nil {
w.onClosed = nil // avoid possibility of calling twice
fn()
}

w.closing = true
w.viewport.SetShouldClose(true)
w.closing = true
w.viewport.SetShouldClose(true)

cache.RangeTexturesFor(w.canvas, w.canvas.Painter().Free)
w.canvas.WalkTrees(nil, func(node *common.RenderCacheNode, _ fyne.Position) {
if wid, ok := node.Obj().(fyne.Widget); ok {
cache.DestroyRenderer(wid)
}
cache.RangeTexturesFor(w.canvas, w.canvas.Painter().Free)
w.canvas.WalkTrees(nil, func(node *common.RenderCacheNode, _ fyne.Position) {
if wid, ok := node.Obj().(fyne.Widget); ok {
cache.DestroyRenderer(wid)
}
})
})
}

Expand Down Expand Up @@ -232,7 +239,7 @@ func (w *window) SetContent(content fyne.CanvasObject) {
if content != nil {
content.Show()
}
w.RescaleContext()
async.EnsureMain(w.RescaleContext)
}

func (w *window) Canvas() fyne.Canvas {
Expand Down Expand Up @@ -860,16 +867,19 @@ func (w *window) Context() any {

func (w *window) runOnMainWhenCreated(fn func()) {
if w.view() != nil {
fn()
async.EnsureMain(fn)
return
}

w.pending = append(w.pending, fn)
}

func (d *gLDriver) CreateWindow(title string) fyne.Window {
func (d *gLDriver) CreateWindow(title string) (win fyne.Window) {
if runtime.GOOS != "js" {
return d.createWindow(title, true)
async.EnsureMain(func() {
win = d.createWindow(title, true)
})
return win
}

// handling multiple windows by overlaying on the root for web
Expand Down
16 changes: 9 additions & 7 deletions internal/driver/mobile/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,17 @@ func init() {
}

func (d *driver) DoFromGoroutine(fn func()) {
done := common.DonePool.Get()
defer common.DonePool.Put(done)
async.EnsureNotMain(func() {
done := common.DonePool.Get()
defer common.DonePool.Put(done)

d.queuedFuncs.In() <- func() {
fn()
done <- struct{}{}
}
d.queuedFuncs.In() <- func() {
fn()
done <- struct{}{}
}

<-done
<-done
})
}

func (d *driver) CreateWindow(title string) fyne.Window {
Expand Down
4 changes: 3 additions & 1 deletion test/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"time"

"fyne.io/fyne/v2"
"fyne.io/fyne/v2/internal/async"
intdriver "fyne.io/fyne/v2/internal/driver"
"fyne.io/fyne/v2/internal/painter"
"fyne.io/fyne/v2/internal/painter/software"
Expand Down Expand Up @@ -50,7 +51,8 @@ func NewDriverWithPainter(painter SoftwarePainter) fyne.Driver {
}

func (d *driver) DoFromGoroutine(f func()) {
f() // Tests all run on a single (but potentially different per-test) thread
// Tests all run on a single (but potentially different per-test) thread
async.EnsureNotMain(f)
}

func (d *driver) AbsolutePositionForObject(co fyne.CanvasObject) fyne.Position {
Expand Down

0 comments on commit abef71c

Please sign in to comment.