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

Fix several race conditions in internal/driver/glfw package #2048

Merged
merged 8 commits into from
Apr 8, 2021
49 changes: 34 additions & 15 deletions internal/driver/glfw/canvas.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,11 @@ func (c *glCanvas) Content() fyne.CanvasObject {
}

func (c *glCanvas) DismissMenu() bool {
if c.menu != nil && c.menu.(*MenuBar).IsActive() {
c.menu.(*MenuBar).Toggle()
c.RLock()
menu := c.menu
c.RUnlock()
if menu != nil && menu.(*MenuBar).IsActive() {
menu.(*MenuBar).Toggle()
return true
}
return false
Expand Down Expand Up @@ -154,7 +157,9 @@ func (c *glCanvas) Padded() bool {
}

func (c *glCanvas) PixelCoordinateForPosition(pos fyne.Position) (int, int) {
c.RLock()
texScale := c.texScale
c.RUnlock()
multiple := c.Scale() * texScale
scaleInt := func(x float32) int {
return int(math.Round(float64(x * multiple)))
Expand Down Expand Up @@ -243,7 +248,11 @@ func (c *glCanvas) SetPadded(padded bool) {
}

func (c *glCanvas) reloadScale() {
if !c.context.(*window).visible {
w := c.context.(*window)
w.viewLock.RLock()
windowVisible := w.visible
w.viewLock.RUnlock()
if !windowVisible {
return
}

Expand All @@ -262,8 +271,11 @@ func (c *glCanvas) Size() fyne.Size {
}

func (c *glCanvas) ToggleMenu() {
if c.menu != nil {
c.menu.(*MenuBar).Toggle()
c.RLock()
menu := c.menu
c.RUnlock()
if menu != nil {
menu.(*MenuBar).Toggle()
}
}

Expand Down Expand Up @@ -407,19 +419,23 @@ func (c *glCanvas) menuHeight() float32 {
}

func (c *glCanvas) objectTrees() []fyne.CanvasObject {
c.RLock()
content := c.content
menu := c.menu
c.RUnlock()
trees := make([]fyne.CanvasObject, 0, len(c.Overlays().List())+2)
trees = append(trees, c.content)
if c.menu != nil {
trees = append(trees, c.menu)
trees = append(trees, content)
if menu != nil {
trees = append(trees, menu)
}
trees = append(trees, c.Overlays().List()...)
return trees
}

func (c *glCanvas) overlayChanged() {
c.Lock()
defer c.Unlock()
c.dirtyMutex.Lock()
c.dirty = true
c.dirtyMutex.Unlock()
}

func (c *glCanvas) paint(size fyne.Size) {
Expand Down Expand Up @@ -468,9 +484,8 @@ func (c *glCanvas) setContent(content fyne.CanvasObject) {

func (c *glCanvas) setDirty(dirty bool) {
c.dirtyMutex.Lock()
defer c.dirtyMutex.Unlock()

c.dirty = dirty
c.dirtyMutex.Unlock()
}

func (c *glCanvas) setMenuOverlay(b fyne.CanvasObject) {
Expand All @@ -493,11 +508,15 @@ func (c *glCanvas) setupThemeListener() {
go func() {
for {
<-listener
if c.menu != nil {
app.ApplyThemeTo(c.menu, c) // Ensure our menu gets the theme change message as it's out-of-tree
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
}

c.SetPadded(c.padded) // refresh the padding for potential theme differences
c.SetPadded(padded) // refresh the padding for potential theme differences
}
}()
}
Expand Down
6 changes: 6 additions & 0 deletions internal/driver/glfw/canvas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,12 +366,16 @@ 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 @@ -487,7 +491,9 @@ 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: 2 additions & 0 deletions internal/driver/glfw/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ 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().(*glCanvas)
movl := buildMenuOverlay(mm, c)
c.Lock()
c.setMenuOverlay(movl)
c.Unlock()
w.SetContent(content)
w.Resize(fyne.NewSize(200, 199))

Expand Down
Loading