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

Cleanup of unneeded locks in canvas objects #5388

Merged
merged 17 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 11 additions & 26 deletions canvas/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,77 +8,62 @@
package canvas // import "fyne.io/fyne/v2/canvas"

import (
"sync"

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

type baseObject struct {
size async.Size // The current size of the canvas object
position async.Position // The current position of the object
Hidden bool // Is this object currently hidden

min async.Size // The minimum size this object can be
size fyne.Size // The current size of the canvas object
position fyne.Position // The current position of the object
Hidden bool // Is this object currently hidden

propertyLock sync.RWMutex
min fyne.Size // The minimum size this object can be
}

// Hide will set this object to not be visible.
func (o *baseObject) Hide() {
o.propertyLock.Lock()
defer o.propertyLock.Unlock()

o.Hidden = true
}

// MinSize returns the specified minimum size, if set, or {1, 1} otherwise.
func (o *baseObject) MinSize() fyne.Size {
min := o.min.Load()
if min.IsZero() {
if o.min.IsZero() {
return fyne.Size{Width: 1, Height: 1}
}

return min
return o.min
}

// Move the object to a new position, relative to its parent.
func (o *baseObject) Move(pos fyne.Position) {
o.position.Store(pos)
o.position = pos
}

// Position gets the current position of this canvas object, relative to its parent.
func (o *baseObject) Position() fyne.Position {
return o.position.Load()
return o.position
}

// Resize sets a new size for the canvas object.
func (o *baseObject) Resize(size fyne.Size) {
o.size.Store(size)
o.size = size
}

// SetMinSize specifies the smallest size this object should be.
func (o *baseObject) SetMinSize(size fyne.Size) {
o.min.Store(size)
o.min = size
}

// Show will set this object to be visible.
func (o *baseObject) Show() {
o.propertyLock.Lock()
defer o.propertyLock.Unlock()

o.Hidden = false
}

// Size returns the current size of this canvas object.
func (o *baseObject) Size() fyne.Size {
return o.size.Load()
return o.size
}

// Visible returns true if this object is visible, false otherwise.
func (o *baseObject) Visible() bool {
o.propertyLock.RLock()
defer o.propertyLock.RUnlock()

return !o.Hidden
}
15 changes: 8 additions & 7 deletions canvas/circle.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ type Circle struct {

// NewCircle returns a new Circle instance
func NewCircle(color color.Color) *Circle {
return &Circle{
FillColor: color,
}
return &Circle{FillColor: color}
}

// Hide will set this circle to not be visible
Expand All @@ -45,7 +43,8 @@ func (c *Circle) MinSize() fyne.Size {
func (c *Circle) Move(pos fyne.Position) {
size := c.Size()
c.Position1 = pos
c.Position2 = fyne.NewPos(c.Position1.X+size.Width, c.Position1.Y+size.Height)
c.Position2 = c.Position1.Add(size)

repaint(c)
}

Expand All @@ -66,7 +65,7 @@ func (c *Circle) Resize(size fyne.Size) {
return
}

c.Position2 = fyne.NewPos(c.Position1.X+size.Width, c.Position1.Y+size.Height)
c.Position2 = c.Position1.Add(size)

Refresh(c)
}
Expand All @@ -80,8 +79,10 @@ func (c *Circle) Show() {

// Size returns the current size of bounding box for this circle object
func (c *Circle) Size() fyne.Size {
return fyne.NewSize(float32(math.Abs(float64(c.Position2.X)-float64(c.Position1.X))),
float32(math.Abs(float64(c.Position2.Y)-float64(c.Position1.Y))))
return fyne.NewSize(
float32(math.Abs(float64(c.Position2.X)-float64(c.Position1.X))),
float32(math.Abs(float64(c.Position2.Y)-float64(c.Position1.Y))),
)
}

// Visible returns true if this circle is visible, false otherwise
Expand Down
5 changes: 0 additions & 5 deletions canvas/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"io"
"os"
"path/filepath"
"sync"

"fyne.io/fyne/v2"
"fyne.io/fyne/v2/internal/cache"
Expand Down Expand Up @@ -62,7 +61,6 @@ type Image struct {
aspect float32
icon *svg.Decoder
isSVG bool
lock sync.Mutex

// one of the following sources will provide our image data
File string // Load the image from a file
Expand Down Expand Up @@ -116,9 +114,6 @@ func (i *Image) Move(pos fyne.Position) {

// Refresh causes this image to be redrawn with its configured state.
func (i *Image) Refresh() {
i.lock.Lock()
defer i.lock.Unlock()

rc, err := i.updateReader()
if err != nil {
fyne.LogError("Failed to load image", err)
Expand Down
6 changes: 4 additions & 2 deletions canvas/line.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ type Line struct {

// Size returns the current size of bounding box for this line object
func (l *Line) Size() fyne.Size {
return fyne.NewSize(float32(math.Abs(float64(l.Position2.X)-float64(l.Position1.X))),
float32(math.Abs(float64(l.Position2.Y)-float64(l.Position1.Y))))
return fyne.NewSize(
float32(math.Abs(float64(l.Position2.X)-float64(l.Position1.X))),
float32(math.Abs(float64(l.Position2.Y)-float64(l.Position1.Y))),
)
}

// Resize sets a new bottom-right position for the line object, then it will then be refreshed.
Expand Down
9 changes: 1 addition & 8 deletions container.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package fyne

import "sync"

// Declare conformity to [CanvasObject]
var _ CanvasObject = (*Container)(nil)

Expand All @@ -12,8 +10,7 @@ type Container struct {
position Position // The current position of the Container
Hidden bool // Is this Container hidden

Layout Layout // The Layout algorithm for arranging child [CanvasObject]s
lock sync.Mutex
Layout Layout // The Layout algorithm for arranging child [CanvasObject]s
Objects []CanvasObject // The set of [CanvasObject]s this container holds
}

Expand Down Expand Up @@ -60,8 +57,6 @@ func (c *Container) Add(add CanvasObject) {
return
}

c.lock.Lock()
defer c.lock.Unlock()
c.Objects = append(c.Objects, add)
c.layout()
}
Expand Down Expand Up @@ -129,8 +124,6 @@ func (c *Container) Refresh() {
// This method is not intended to be used inside a loop, to remove all the elements.
// It is much more efficient to call [Container.RemoveAll) instead.
func (c *Container) Remove(rem CanvasObject) {
c.lock.Lock()
defer c.lock.Unlock()
if len(c.Objects) == 0 {
return
}
Expand Down
36 changes: 0 additions & 36 deletions container_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package fyne

import (
"sync"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -107,41 +106,6 @@ func TestContainer_Remove(t *testing.T) {
assert.Equal(t, float32(0), box2.Position().Y)
}

func TestContainer_Remove_Race(t *testing.T) {
var objs []CanvasObject
for i := 0; i < 100; i++ {
objs = append(objs, new(dummyObject))
}

container := NewContainerWithLayout(new(customLayout), objs...)

wg := &sync.WaitGroup{}
wg.Add(100)
for _, o := range objs {
rmo := o
go func() {
container.Remove(rmo)
wg.Done()
}()
}
wg.Wait()
assert.Empty(t, container.Objects)
}

func TestContainer_Add_Race(t *testing.T) {
container := NewContainerWithLayout(new(customLayout))
wg := &sync.WaitGroup{}
wg.Add(100)
for i := 0; i < 100; i++ {
go func() {
container.Add(new(dummyObject))
wg.Done()
}()
}
wg.Wait()
assert.Len(t, container.Objects, 100)
}

func TestContainer_RemoveAll(t *testing.T) {
box1 := new(dummyObject)
box2 := new(dummyObject)
Expand Down
2 changes: 1 addition & 1 deletion dialog/file_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ func isHidden(file fyne.URI) bool {
return name == "" || name[0] == '.'
}

func hideFile(filename string) error {
func hideFile(_ string) error {
return nil
}
4 changes: 2 additions & 2 deletions dialog/file_xdg_notflatpak.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

package dialog

func fileOpenOSOverride(d *FileDialog) bool {
func fileOpenOSOverride(_ *FileDialog) bool {
return false
}

func fileSaveOSOverride(d *FileDialog) bool {
func fileSaveOSOverride(_ *FileDialog) bool {
return false
}
52 changes: 0 additions & 52 deletions internal/async/vector.go

This file was deleted.

62 changes: 0 additions & 62 deletions internal/async/vector_test.go

This file was deleted.

Loading
Loading