From 4ffdaffe87604cbe51420a5762d9620c385449d0 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Thu, 9 Jan 2025 22:56:50 +0100 Subject: [PATCH 01/16] canvas: Remove locks that are not needed with new threading model --- canvas/base.go | 37 +++++++++++-------------------------- canvas/circle.go | 15 ++++++++------- canvas/image.go | 5 ----- canvas/line.go | 6 ++++-- 4 files changed, 23 insertions(+), 40 deletions(-) diff --git a/canvas/base.go b/canvas/base.go index 0be24dbcd0..5b4a1ee0e1 100644 --- a/canvas/base.go +++ b/canvas/base.go @@ -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 } diff --git a/canvas/circle.go b/canvas/circle.go index 51d3ce4f2c..35092d84b8 100644 --- a/canvas/circle.go +++ b/canvas/circle.go @@ -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 @@ -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) } @@ -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) } @@ -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 diff --git a/canvas/image.go b/canvas/image.go index c8c18a41f1..df0c2eacca 100644 --- a/canvas/image.go +++ b/canvas/image.go @@ -9,7 +9,6 @@ import ( "io" "os" "path/filepath" - "sync" "fyne.io/fyne/v2" "fyne.io/fyne/v2/internal/cache" @@ -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 @@ -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) diff --git a/canvas/line.go b/canvas/line.go index 9fc1821fb8..8673c60bc3 100644 --- a/canvas/line.go +++ b/canvas/line.go @@ -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. From c8722ee427a414a519c21d8119f820e8d6982a36 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Thu, 9 Jan 2025 23:06:00 +0100 Subject: [PATCH 02/16] internal/widget: Remove unneeded locks from within base widget --- internal/widget/base.go | 41 ++++++++++++++++--------------------- internal/widget/scroller.go | 4 ++-- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/internal/widget/base.go b/internal/widget/base.go index 36d2c9aaef..cb624586c1 100644 --- a/internal/widget/base.go +++ b/internal/widget/base.go @@ -1,20 +1,17 @@ package widget import ( - "sync/atomic" - "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" - "fyne.io/fyne/v2/internal/async" "fyne.io/fyne/v2/internal/cache" ) // Base provides a helper that handles basic widget behaviours. type Base struct { - hidden atomic.Bool - position async.Position - size async.Size - impl atomic.Pointer[fyne.Widget] + hidden bool + position fyne.Position + size fyne.Size + impl fyne.Widget } // ExtendBaseWidget is used by an extending widget to make use of BaseWidget functionality. @@ -24,12 +21,12 @@ func (w *Base) ExtendBaseWidget(wid fyne.Widget) { return } - w.impl.Store(&wid) + w.impl = wid } // Size gets the current size of this widget. func (w *Base) Size() fyne.Size { - return w.size.Load() + return w.size } // Resize sets a new size for a widget. @@ -39,7 +36,7 @@ func (w *Base) Resize(size fyne.Size) { return } - w.size.Store(size) + w.size = size impl := w.super() if impl == nil { @@ -50,13 +47,13 @@ func (w *Base) Resize(size fyne.Size) { // Position gets the current position of this widget, relative to its parent. func (w *Base) Position() fyne.Position { - return w.position.Load() + return w.position } // Move the widget to a new position, relative to its parent. // Note this should not be used if the widget is being managed by a Layout within a Container. func (w *Base) Move(pos fyne.Position) { - w.position.Store(pos) + w.position = pos Repaint(w.super()) } @@ -76,15 +73,17 @@ func (w *Base) MinSize() fyne.Size { // Visible returns whether or not this widget should be visible. // Note that this may not mean it is currently visible if a parent has been hidden. func (w *Base) Visible() bool { - return !w.hidden.Load() + return !w.hidden } // Show this widget so it becomes visible func (w *Base) Show() { - if !w.hidden.CompareAndSwap(true, false) { + if !w.hidden { return // Visible already } + w.hidden = false + impl := w.super() if impl == nil { return @@ -94,10 +93,12 @@ func (w *Base) Show() { // Hide this widget so it is no longer visible func (w *Base) Hide() { - if !w.hidden.CompareAndSwap(false, true) { + if w.hidden { return // Hidden already } + w.hidden = true + impl := w.super() if impl == nil { return @@ -112,19 +113,13 @@ func (w *Base) Refresh() { return } - render := cache.Renderer(impl) - render.Refresh() + cache.Renderer(impl).Refresh() } // super will return the actual object that this represents. // If extended then this is the extending widget, otherwise it is nil. func (w *Base) super() fyne.Widget { - impl := w.impl.Load() - if impl == nil { - return nil - } - - return *impl + return w.impl } // Repaint instructs the containing canvas to redraw, even if nothing changed. diff --git a/internal/widget/scroller.go b/internal/widget/scroller.go index 5d5d3f3367..76c6321192 100644 --- a/internal/widget/scroller.go +++ b/internal/widget/scroller.go @@ -281,7 +281,7 @@ type scrollContainerRenderer struct { } func (r *scrollContainerRenderer) layoutBars(size fyne.Size) { - scrollerSize := r.scroll.size.Load() + scrollerSize := r.scroll.Size() if r.scroll.Direction == ScrollVerticalOnly || r.scroll.Direction == ScrollBoth { r.vertArea.Resize(fyne.NewSize(r.vertArea.MinSize().Width, size.Height)) r.vertArea.Move(fyne.NewPos(scrollerSize.Width-r.vertArea.Size().Width, 0)) @@ -364,7 +364,7 @@ func (r *scrollContainerRenderer) updatePosition() { if r.scroll.Content == nil { return } - scrollSize := r.scroll.size.Load() + scrollSize := r.scroll.Size() contentSize := r.scroll.Content.Size() r.scroll.Content.Move(fyne.NewPos(-r.scroll.Offset.X, -r.scroll.Offset.Y)) From 768b06c09b970c998d50a8c2e84bc27a10dc3890 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Thu, 9 Jan 2025 23:11:31 +0100 Subject: [PATCH 03/16] Remove container locks and racy tests --- container.go | 9 +-------- container_test.go | 36 ------------------------------------ 2 files changed, 1 insertion(+), 44 deletions(-) diff --git a/container.go b/container.go index a9357591cf..2b9752123a 100644 --- a/container.go +++ b/container.go @@ -1,7 +1,5 @@ package fyne -import "sync" - // Declare conformity to [CanvasObject] var _ CanvasObject = (*Container)(nil) @@ -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 } @@ -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() } @@ -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 } diff --git a/container_test.go b/container_test.go index 58f7ae56b5..1e0fb0a43d 100644 --- a/container_test.go +++ b/container_test.go @@ -1,7 +1,6 @@ package fyne import ( - "sync" "testing" "github.com/stretchr/testify/assert" @@ -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) From 8c9110b40ae0e423eedaa576701a50b031c00aac Mon Sep 17 00:00:00 2001 From: Jacalz Date: Thu, 9 Jan 2025 23:13:18 +0100 Subject: [PATCH 04/16] Make my editor a bit less unhappy --- dialog/file_unix.go | 2 +- dialog/file_xdg_notflatpak.go | 4 ++-- internal/driver/mobile/file_desktop.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dialog/file_unix.go b/dialog/file_unix.go index ce3743b378..67fbf2f688 100644 --- a/dialog/file_unix.go +++ b/dialog/file_unix.go @@ -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 } diff --git a/dialog/file_xdg_notflatpak.go b/dialog/file_xdg_notflatpak.go index 4494e18b7f..78528dbc47 100644 --- a/dialog/file_xdg_notflatpak.go +++ b/dialog/file_xdg_notflatpak.go @@ -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 } diff --git a/internal/driver/mobile/file_desktop.go b/internal/driver/mobile/file_desktop.go index aa2538b9eb..aac22de345 100644 --- a/internal/driver/mobile/file_desktop.go +++ b/internal/driver/mobile/file_desktop.go @@ -10,7 +10,7 @@ import ( "fyne.io/fyne/v2/storage/repository" ) -func deleteURI(u fyne.URI) error { +func deleteURI(_ fyne.URI) error { // no-op as we use the internal FileRepository return nil } @@ -30,6 +30,6 @@ func nativeFileSave(*fileSave, bool) (io.WriteCloser, error) { return nil, nil } -func registerRepository(d *driver) { +func registerRepository(_ *driver) { repository.Register("file", intRepo.NewFileRepository()) } From eca93bab874a34c34ecb54a461395818d17945a8 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Thu, 9 Jan 2025 23:17:23 +0100 Subject: [PATCH 05/16] widget: Remove unnecessary function --- widget/accordion.go | 2 +- widget/button.go | 2 +- widget/entry.go | 6 +++--- widget/fileicon.go | 4 ++-- widget/hyperlink.go | 4 ++-- widget/menu.go | 2 +- widget/select.go | 2 +- widget/widget.go | 4 ---- 8 files changed, 11 insertions(+), 15 deletions(-) diff --git a/widget/accordion.go b/widget/accordion.go index 9d617838cf..4b362e0903 100644 --- a/widget/accordion.go +++ b/widget/accordion.go @@ -207,7 +207,7 @@ func (r *accordionRenderer) Refresh() { } func (r *accordionRenderer) updateObjects() { - th := r.container.themeWithLock() + th := r.container.Theme() is := len(r.container.Items) hs := len(r.headers) ds := len(r.dividers) diff --git a/widget/button.go b/widget/button.go index eea6fca21d..931f3a6b48 100644 --- a/widget/button.go +++ b/widget/button.go @@ -305,7 +305,7 @@ func (r *buttonRenderer) Refresh() { // applyTheme updates this button to match the current theme // must be called with the button propertyLock RLocked func (r *buttonRenderer) applyTheme() { - th := r.button.themeWithLock() + th := r.button.Theme() v := fyne.CurrentApp().Settings().ThemeVariant() fgColorName, bgColorName, bgBlendName := r.buttonColorNames() if bg := r.background; bg != nil { diff --git a/widget/entry.go b/widget/entry.go index e42e30952c..2f54a07bce 100644 --- a/widget/entry.go +++ b/widget/entry.go @@ -919,7 +919,7 @@ func (e *Entry) copyToClipboard(clipboard fyne.Clipboard) { } func (e *Entry) cursorColAt(text []rune, pos fyne.Position) int { - th := e.themeWithLock() + th := e.Theme() textSize := th.Size(theme.SizeNameText) innerPad := th.Size(theme.SizeNameInnerPadding) @@ -1063,7 +1063,7 @@ func (e *Entry) placeholderProvider() *RichText { } e.placeholder.Scroll = widget.ScrollNone - e.placeholder.inset = fyne.NewSize(0, e.themeWithLock().Size(theme.SizeNameInputBorder)) + e.placeholder.inset = fyne.NewSize(0, e.Theme().Size(theme.SizeNameInputBorder)) style := RichTextStyleInline style.ColorName = theme.ColorNamePlaceHolder @@ -1367,7 +1367,7 @@ func (e *Entry) textProvider() *RichText { } e.text.Scroll = widget.ScrollNone - e.text.inset = fyne.NewSize(0, e.themeWithLock().Size(theme.SizeNameInputBorder)) + e.text.inset = fyne.NewSize(0, e.Theme().Size(theme.SizeNameInputBorder)) e.text.Segments = []RichTextSegment{&TextSegment{Style: RichTextStyleInline, Text: e.Text}} return &e.text } diff --git a/widget/fileicon.go b/widget/fileicon.go index 9cde75d9a6..15524dd2d2 100644 --- a/widget/fileicon.go +++ b/widget/fileicon.go @@ -46,7 +46,7 @@ func (i *FileIcon) SetURI(uri fyne.URI) { // must be called with i.propertyLock RLocked func (i *FileIcon) setURI(uri fyne.URI) { if uri == nil { - i.resource = i.themeWithLock().Icon(theme.IconNameFile) + i.resource = i.Theme().Icon(theme.IconNameFile) return } @@ -101,7 +101,7 @@ func (i *FileIcon) lookupIcon(uri fyne.URI) fyne.Resource { return theme.FolderIcon() } - th := i.themeWithLock() + th := i.Theme() mainMimeType, _ := mime.Split(uri.MimeType()) switch mainMimeType { case "application": diff --git a/widget/hyperlink.go b/widget/hyperlink.go index 9d5a3e031f..04c3083a26 100644 --- a/widget/hyperlink.go +++ b/widget/hyperlink.go @@ -122,14 +122,14 @@ func (hl *Hyperlink) MouseOut() { } func (hl *Hyperlink) focusWidth() float32 { - th := hl.themeWithLock() + th := hl.Theme() innerPad := th.Size(theme.SizeNameInnerPadding) return fyne.Min(hl.size.Load().Width, hl.textSize.Width+innerPad+th.Size(theme.SizeNamePadding)*2) - innerPad } func (hl *Hyperlink) focusXPos() float32 { - innerPad := hl.themeWithLock().Size(theme.SizeNameInnerPadding) + innerPad := hl.Theme().Size(theme.SizeNameInnerPadding) switch hl.Alignment { case fyne.TextAlignLeading: diff --git a/widget/menu.go b/widget/menu.go index f48d743df9..38a2a249f9 100644 --- a/widget/menu.go +++ b/widget/menu.go @@ -299,7 +299,7 @@ func (r *menuRenderer) layoutActiveChild() { cp.X = c.Size().Width - absPos.X - childSize.Width } } - requiredHeight := childSize.Height - r.m.themeWithLock().Size(theme.SizeNamePadding) + requiredHeight := childSize.Height - r.m.Theme().Size(theme.SizeNamePadding) availableHeight := c.Size().Height - absPos.Y missingHeight := requiredHeight - availableHeight if missingHeight > 0 { diff --git a/widget/select.go b/widget/select.go index b5f04473d4..8fd45f6e2b 100644 --- a/widget/select.go +++ b/widget/select.go @@ -415,7 +415,7 @@ func (s *selectRenderer) Refresh() { s.updateLabel() s.updateIcon(th) s.background.FillColor = s.bgColor(th, v) - s.background.CornerRadius = s.combo.themeWithLock().Size(theme.SizeNameInputRadius) + s.background.CornerRadius = s.combo.Theme().Size(theme.SizeNameInputRadius) s.Layout(s.combo.Size()) if s.combo.popUp != nil { diff --git a/widget/widget.go b/widget/widget.go index b899406e78..62b56f8a90 100644 --- a/widget/widget.go +++ b/widget/widget.go @@ -131,10 +131,6 @@ func (w *BaseWidget) Refresh() { // // Since: 2.5 func (w *BaseWidget) Theme() fyne.Theme { - return w.themeWithLock() -} - -func (w *BaseWidget) themeWithLock() fyne.Theme { cached := w.themeCache if cached == nil { cached = cache.WidgetTheme(w.super()) From 09446f7479e81bc13959ca6b9abd199673c7c9ec Mon Sep 17 00:00:00 2001 From: Jacalz Date: Thu, 9 Jan 2025 23:18:45 +0100 Subject: [PATCH 06/16] widget: Make BaseWidget theme lookup code more readable --- widget/widget.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/widget/widget.go b/widget/widget.go index 62b56f8a90..0fabbb3038 100644 --- a/widget/widget.go +++ b/widget/widget.go @@ -132,16 +132,17 @@ func (w *BaseWidget) Refresh() { // Since: 2.5 func (w *BaseWidget) Theme() fyne.Theme { cached := w.themeCache - if cached == nil { - cached = cache.WidgetTheme(w.super()) - // don't cache the default as it may change - if cached == nil { - return theme.Current() - } + if cached != nil { + return cached + } - w.themeCache = cached + cached = cache.WidgetTheme(w.super()) + // don't cache the default as it may change + if cached == nil { + return theme.Current() } + w.themeCache = cached return cached } From 1aeb76dc7c823737e8198c449fa170cb6863949b Mon Sep 17 00:00:00 2001 From: Jacalz Date: Sat, 11 Jan 2025 13:32:03 +0100 Subject: [PATCH 07/16] widget: Clean up locks within the BaseWidget --- widget/check.go | 6 ++--- widget/check_group.go | 4 ++-- widget/entry.go | 22 +++++++----------- widget/entry_password.go | 2 +- widget/entry_validation.go | 2 +- widget/hyperlink.go | 6 ++--- widget/progressbarinfinite.go | 2 +- widget/radio_group.go | 4 ++-- widget/richtext.go | 8 +++---- widget/select.go | 21 +++++++++-------- widget/slider.go | 2 +- widget/table.go | 8 +++---- widget/tree.go | 8 +++---- widget/widget.go | 44 +++++++++++++++-------------------- 14 files changed, 64 insertions(+), 75 deletions(-) diff --git a/widget/check.go b/widget/check.go index de22f44d53..3df6629776 100644 --- a/widget/check.go +++ b/widget/check.go @@ -322,7 +322,7 @@ func (c *checkRenderer) Layout(size fyne.Size) { func (c *checkRenderer) applyTheme(th fyne.Theme, v fyne.ThemeVariant) { c.label.Color = th.Color(theme.ColorNameForeground, v) c.label.TextSize = th.Size(theme.SizeNameText) - if c.check.disabled.Load() { + if c.check.Disabled() { c.label.Color = th.Color(theme.ColorNameDisabled, v) } } @@ -359,7 +359,7 @@ func (c *checkRenderer) updateResource(th fyne.Theme) { res.ColorName = theme.ColorNamePrimary bgRes.ColorName = theme.ColorNameBackground } - if c.check.disabled.Load() { + if c.check.Disabled() { if c.check.Checked { res = theme.NewThemedResource(theme.CheckButtonCheckedIcon()) } @@ -374,7 +374,7 @@ func (c *checkRenderer) updateResource(th fyne.Theme) { // must be called while holding c.check.propertyLock for reading func (c *checkRenderer) updateFocusIndicator(th fyne.Theme, v fyne.ThemeVariant) { - if c.check.disabled.Load() { + if c.check.Disabled() { c.focusIndicator.FillColor = color.Transparent } else if c.check.focused { c.focusIndicator.FillColor = th.Color(theme.ColorNameFocus, v) diff --git a/widget/check_group.go b/widget/check_group.go index 5c9c5764db..7c035848a3 100644 --- a/widget/check_group.go +++ b/widget/check_group.go @@ -164,7 +164,7 @@ func (r *CheckGroup) update() { item.Text = r.Options[i] item.Checked = contains - item.DisableableWidget.disabled.Store(r.disabled.Load()) + item.DisableableWidget.disabled = r.Disabled() item.Refresh() } } @@ -257,7 +257,7 @@ func (r *checkGroupRenderer) updateItems() { } item.Text = r.checks.Options[i] item.Checked = contains - item.disabled.Store(r.checks.disabled.Load()) + item.disabled = r.checks.Disabled() item.Refresh() } } diff --git a/widget/entry.go b/widget/entry.go index 2f54a07bce..4057466ff3 100644 --- a/widget/entry.go +++ b/widget/entry.go @@ -229,7 +229,7 @@ func (e *Entry) Disable() { // // Implements: fyne.Disableable func (e *Entry) Disabled() bool { - return e.DisableableWidget.disabled.Load() + return e.DisableableWidget.Disabled() } // DoubleTapped is called when this entry has been double tapped so we should select text below the pointer @@ -298,13 +298,7 @@ func (e *Entry) Enable() { // ExtendBaseWidget is used by an extending widget to make use of BaseWidget functionality. func (e *Entry) ExtendBaseWidget(wid fyne.Widget) { - impl := e.super() - if impl != nil { - return - } - - e.impl.Store(&wid) - + e.BaseWidget.ExtendBaseWidget(wid) e.registerShortcut() } @@ -1328,7 +1322,7 @@ func (e *Entry) textPosFromRowCol(row, col int) int { func (e *Entry) syncSegments() { colName := theme.ColorNameForeground wrap := e.textWrap() - disabled := e.disabled.Load() + disabled := e.Disabled() if disabled { colName = theme.ColorNameDisabled } @@ -1696,7 +1690,7 @@ func (r *entryRenderer) Objects() []fyne.CanvasObject { func (r *entryRenderer) Refresh() { content := r.entry.content - focusedAppearance := r.entry.focused && !r.entry.disabled.Load() + focusedAppearance := r.entry.focused && !r.entry.Disabled() scroll := r.entry.Scroll wrapping := r.entry.Wrapping @@ -1711,7 +1705,7 @@ func (r *entryRenderer) Refresh() { inputBorder := th.Size(theme.SizeNameInputBorder) // correct our scroll wrappers if the wrap mode changed - entrySize := r.entry.size.Load().Subtract(fyne.NewSize(r.trailingInset(), inputBorder*2)) + entrySize := r.entry.Size().Subtract(fyne.NewSize(r.trailingInset(), inputBorder*2)) if wrapping == fyne.TextWrapOff && scroll == widget.ScrollNone && r.scroll.Content != nil { r.scroll.Hide() r.scroll.Content = nil @@ -1774,7 +1768,7 @@ func (r *entryRenderer) ensureValidationSetup() { if r.entry.validationStatus == nil { r.entry.validationStatus = newValidationStatus(r.entry) r.objects = append(r.objects, r.entry.validationStatus) - r.Layout(r.entry.size.Load()) + r.Layout(r.entry.Size()) r.entry.validate() r.Refresh() @@ -1803,7 +1797,7 @@ func (e *entryContent) CreateRenderer() fyne.WidgetRenderer { r := &entryContentRenderer{e.entry.cursorAnim.cursor, []fyne.CanvasObject{}, objects, provider, placeholder, e} r.updateScrollDirections() - r.Layout(e.size.Load()) + r.Layout(e.Size()) return r } @@ -1870,7 +1864,7 @@ func (r *entryContentRenderer) Refresh() { provider := r.content.entry.textProvider() placeholder := r.content.entry.placeholderProvider() focused := r.content.entry.focused - focusedAppearance := focused && !r.content.entry.disabled.Load() + focusedAppearance := focused && !r.content.entry.Disabled() selections := r.selection r.updateScrollDirections() diff --git a/widget/entry_password.go b/widget/entry_password.go index 9bbcf03a24..7b7ceefcda 100644 --- a/widget/entry_password.go +++ b/widget/entry_password.go @@ -78,7 +78,7 @@ func (r *passwordRevealerRenderer) Refresh() { r.icon.Resource = th.Icon(theme.IconNameVisibilityOff) } - if r.entry.disabled.Load() { + if r.entry.Disabled() { r.icon.Resource = theme.NewDisabledResource(r.icon.Resource) } r.icon.Refresh() diff --git a/widget/entry_validation.go b/widget/entry_validation.go index 5095cd25cd..09a16904a6 100644 --- a/widget/entry_validation.go +++ b/widget/entry_validation.go @@ -116,7 +116,7 @@ func (r *validationStatusRenderer) MinSize() fyne.Size { func (r *validationStatusRenderer) Refresh() { th := r.entry.Theme() - if r.entry.disabled.Load() { + if r.entry.Disabled() { r.icon.Hide() return } diff --git a/widget/hyperlink.go b/widget/hyperlink.go index 04c3083a26..7ff915de35 100644 --- a/widget/hyperlink.go +++ b/widget/hyperlink.go @@ -125,7 +125,7 @@ func (hl *Hyperlink) focusWidth() float32 { th := hl.Theme() innerPad := th.Size(theme.SizeNameInnerPadding) - return fyne.Min(hl.size.Load().Width, hl.textSize.Width+innerPad+th.Size(theme.SizeNamePadding)*2) - innerPad + return fyne.Min(hl.Size().Width, hl.textSize.Width+innerPad+th.Size(theme.SizeNamePadding)*2) - innerPad } func (hl *Hyperlink) focusXPos() float32 { @@ -135,9 +135,9 @@ func (hl *Hyperlink) focusXPos() float32 { case fyne.TextAlignLeading: return innerPad / 2 case fyne.TextAlignCenter: - return (hl.size.Load().Width - hl.focusWidth()) / 2 + return (hl.Size().Width - hl.focusWidth()) / 2 case fyne.TextAlignTrailing: - return (hl.size.Load().Width - hl.focusWidth()) - innerPad/2 + return (hl.Size().Width - hl.focusWidth()) - innerPad/2 default: return 0 // unreached } diff --git a/widget/progressbarinfinite.go b/widget/progressbarinfinite.go index da2a3b565a..0b250cbf51 100644 --- a/widget/progressbarinfinite.go +++ b/widget/progressbarinfinite.go @@ -35,7 +35,7 @@ func (p *infProgressRenderer) MinSize() fyne.Size { } func (p *infProgressRenderer) updateBar(done float32) { - size := p.progress.size.Load() + size := p.progress.Size() progressWidth := size.Width spanWidth := progressWidth + (progressWidth * (maxProgressBarInfiniteWidthRatio / 2)) maxBarWidth := progressWidth * maxProgressBarInfiniteWidthRatio diff --git a/widget/radio_group.go b/widget/radio_group.go index 3cf9bcb11f..702c8b7939 100644 --- a/widget/radio_group.go +++ b/widget/radio_group.go @@ -236,8 +236,8 @@ func (r *radioGroupRenderer) updateItems(refresh bool) { item.Selected = sel changed = true } - if d := r.radio.disabled.Load(); d != item.disabled.Load() { - item.disabled.Store(d) + if d := r.radio.Disabled(); d != item.Disabled() { + item.disabled = d changed = true } diff --git a/widget/richtext.go b/widget/richtext.go index 4d44203215..1baf9d9f7d 100644 --- a/widget/richtext.go +++ b/widget/richtext.go @@ -118,11 +118,11 @@ func (t *RichText) Refresh() { // // Implements: fyne.Widget func (t *RichText) Resize(size fyne.Size) { - if size == t.size.Load() { + if size == t.Size() { return } - t.size.Store(size) + t.size = size skipResize := !t.minCache.IsZero() && size.Width >= t.minCache.Width && size.Height >= t.minCache.Height && t.Wrapping == fyne.TextWrapOff && t.Truncation == fyne.TextTruncateOff @@ -389,14 +389,14 @@ func (t *RichText) rows() int { func (t *RichText) updateRowBounds() { th := t.Theme() innerPadding := th.Size(theme.SizeNameInnerPadding) - fitSize := t.size.Load() + fitSize := t.Size() if t.scr != nil { fitSize = t.scr.Content.MinSize() } fitSize.Height -= (innerPadding + t.inset.Height) * 2 var bounds []rowBoundary - maxWidth := t.size.Load().Width - 2*innerPadding + 2*t.inset.Width + maxWidth := t.Size().Width - 2*innerPadding + 2*t.inset.Width wrapWidth := maxWidth var currentBound *rowBoundary diff --git a/widget/select.go b/widget/select.go index 8fd45f6e2b..a7f982918f 100644 --- a/widget/select.go +++ b/widget/select.go @@ -96,7 +96,7 @@ func (s *Select) CreateRenderer() fyne.WidgetRenderer { txtProv.inset = fyne.NewSquareSize(th.Size(theme.SizeNamePadding)) txtProv.ExtendBaseWidget(txtProv) txtProv.Truncation = fyne.TextTruncateEllipsis - if s.disabled.Load() { + if s.Disabled() { txtProv.Segments[0].(*TextSegment).Style.ColorName = theme.ColorNameDisabled } @@ -421,7 +421,7 @@ func (s *selectRenderer) Refresh() { if s.combo.popUp != nil { s.combo.popUp.alignment = s.combo.Alignment s.combo.popUp.Move(s.combo.popUpPos()) - s.combo.popUp.Resize(fyne.NewSize(s.combo.size.Load().Width, s.combo.popUp.MinSize().Height)) + s.combo.popUp.Resize(fyne.NewSize(s.combo.Size().Width, s.combo.popUp.MinSize().Height)) s.combo.popUp.Refresh() } s.background.Refresh() @@ -429,7 +429,7 @@ func (s *selectRenderer) Refresh() { } func (s *selectRenderer) bgColor(th fyne.Theme, v fyne.ThemeVariant) color.Color { - if s.combo.disabled.Load() { + if s.combo.Disabled() { return th.Color(theme.ColorNameDisabledButton, v) } if s.combo.focused { @@ -443,7 +443,7 @@ func (s *selectRenderer) bgColor(th fyne.Theme, v fyne.ThemeVariant) color.Color func (s *selectRenderer) updateIcon(th fyne.Theme) { icon := th.Icon(theme.IconNameArrowDropDown) - if s.combo.disabled.Load() { + if s.combo.Disabled() { s.icon.Resource = theme.NewDisabledResource(icon) } else { s.icon.Resource = icon @@ -456,16 +456,17 @@ func (s *selectRenderer) updateLabel() { s.combo.PlaceHolder = defaultPlaceHolder } - s.label.Segments[0].(*TextSegment).Style.Alignment = s.combo.Alignment - if s.combo.disabled.Load() { - s.label.Segments[0].(*TextSegment).Style.ColorName = theme.ColorNameDisabled + segment := s.label.Segments[0].(*TextSegment) + segment.Style.Alignment = s.combo.Alignment + if s.combo.Disabled() { + segment.Style.ColorName = theme.ColorNameDisabled } else { - s.label.Segments[0].(*TextSegment).Style.ColorName = theme.ColorNameForeground + segment.Style.ColorName = theme.ColorNameForeground } if s.combo.Selected == "" { - s.label.Segments[0].(*TextSegment).Text = s.combo.PlaceHolder + segment.Text = s.combo.PlaceHolder } else { - s.label.Segments[0].(*TextSegment).Text = s.combo.Selected + segment.Text = s.combo.Selected } s.label.Refresh() } diff --git a/widget/slider.go b/widget/slider.go index 781a5c08a2..35f90c7068 100644 --- a/widget/slider.go +++ b/widget/slider.go @@ -230,7 +230,7 @@ func (s *Slider) getRatio(e *fyne.PointEvent) float64 { x := e.Position.X y := e.Position.Y - size := s.size.Load() + size := s.Size() switch s.Orientation { case Vertical: diff --git a/widget/table.go b/widget/table.go index 18c6af40ad..1ebbb3f721 100644 --- a/widget/table.go +++ b/widget/table.go @@ -861,7 +861,7 @@ func (t *Table) visibleColumnWidths(colWidth float32, cols int) (visible map[int padding := t.Theme().Size(theme.SizeNamePadding) stick := t.StickyColumnCount - size := t.size.Load() + size := t.Size() if len(t.columnWidths) == 0 { paddedWidth := colWidth + padding @@ -961,7 +961,7 @@ func (t *Table) visibleRowHeights(rowHeight float32, rows int) (visible map[int] padding := t.Theme().Size(theme.SizeNamePadding) stick := t.StickyRowCount - size := t.size.Load() + size := t.Size() if len(t.rowHeights) == 0 { paddedHeight := rowHeight + padding @@ -1448,7 +1448,7 @@ func (r *tableCellsRenderer) moveIndicators() { r.cells.t.dividerLayer.Content.Refresh() } - size := r.cells.t.size.Load() + size := r.cells.t.Size() divs := 0 i := 0 @@ -1552,7 +1552,7 @@ func (r *tableCellsRenderer) moveMarker(marker fyne.CanvasObject, row, col int, } y2 := y1 + heights[row] - size := r.cells.t.size.Load() + size := r.cells.t.Size() if x2 < 0 || x1 > size.Width || y2 < 0 || y1 > size.Height { marker.Hide() } else { diff --git a/widget/tree.go b/widget/tree.go index de48c91339..569130f89b 100644 --- a/widget/tree.go +++ b/widget/tree.go @@ -229,11 +229,11 @@ func (t *Tree) OpenBranch(uid TreeNodeID) { // Resize sets a new size for a widget. func (t *Tree) Resize(size fyne.Size) { - if size == t.size.Load() { + if size == t.Size() { return } - t.size.Store(size) + t.size = size t.Refresh() // trigger a redraw } @@ -584,11 +584,11 @@ func (c *treeContent) CreateRenderer() fyne.WidgetRenderer { } func (c *treeContent) Resize(size fyne.Size) { - if size == c.size.Load() { + if size == c.Size() { return } - c.size.Store(size) + c.size = size c.Refresh() // trigger a redraw } diff --git a/widget/widget.go b/widget/widget.go index 0fabbb3038..fb38511750 100644 --- a/widget/widget.go +++ b/widget/widget.go @@ -2,11 +2,8 @@ package widget // import "fyne.io/fyne/v2/widget" import ( - "sync/atomic" - "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" - "fyne.io/fyne/v2/internal/async" "fyne.io/fyne/v2/internal/cache" internalWidget "fyne.io/fyne/v2/internal/widget" "fyne.io/fyne/v2/theme" @@ -14,27 +11,26 @@ import ( // BaseWidget provides a helper that handles basic widget behaviours. type BaseWidget struct { - size async.Size - position async.Position + size fyne.Size + position fyne.Position Hidden bool - impl atomic.Pointer[fyne.Widget] + impl fyne.Widget themeCache fyne.Theme } // ExtendBaseWidget is used by an extending widget to make use of BaseWidget functionality. func (w *BaseWidget) ExtendBaseWidget(wid fyne.Widget) { - impl := w.super() - if impl != nil { + if w.super() != nil { return } - w.impl.Store(&wid) + w.impl = wid } // Size gets the current size of this widget. func (w *BaseWidget) Size() fyne.Size { - return w.size.Load() + return w.size } // Resize sets a new size for a widget. @@ -44,7 +40,7 @@ func (w *BaseWidget) Resize(size fyne.Size) { return } - w.size.Store(size) + w.size = size impl := w.super() if impl == nil { @@ -55,13 +51,13 @@ func (w *BaseWidget) Resize(size fyne.Size) { // Position gets the current position of this widget, relative to its parent. func (w *BaseWidget) Position() fyne.Position { - return w.position.Load() + return w.position } // Move the widget to a new position, relative to its parent. // Note this should not be used if the widget is being managed by a Layout within a Container. func (w *BaseWidget) Move(pos fyne.Position) { - w.position.Store(pos) + w.position = pos internalWidget.Repaint(w.super()) } @@ -122,8 +118,7 @@ func (w *BaseWidget) Refresh() { w.themeCache = nil - render := cache.Renderer(impl) - render.Refresh() + cache.Renderer(impl).Refresh() } // Theme returns a cached Theme instance for this widget (or its extending widget). @@ -149,12 +144,7 @@ func (w *BaseWidget) Theme() fyne.Theme { // super will return the actual object that this represents. // If extended then this is the extending widget, otherwise it is nil. func (w *BaseWidget) super() fyne.Widget { - impl := w.impl.Load() - if impl == nil { - return nil - } - - return *impl + return w.impl } // DisableableWidget describes an extension to BaseWidget which can be disabled. @@ -162,15 +152,17 @@ func (w *BaseWidget) super() fyne.Widget { type DisableableWidget struct { BaseWidget - disabled atomic.Bool + disabled bool } // Enable this widget, updating any style or features appropriately. func (w *DisableableWidget) Enable() { - if !w.disabled.CompareAndSwap(true, false) { + if !w.Disabled() { return // Enabled already } + w.disabled = false + impl := w.super() if impl == nil { return @@ -180,10 +172,12 @@ func (w *DisableableWidget) Enable() { // Disable this widget so that it cannot be interacted with, updating any style appropriately. func (w *DisableableWidget) Disable() { - if !w.disabled.CompareAndSwap(false, true) { + if w.Disabled() { return // Disabled already } + w.disabled = true + impl := w.super() if impl == nil { return @@ -193,7 +187,7 @@ func (w *DisableableWidget) Disable() { // Disabled returns true if this widget is currently disabled or false if it can currently be interacted with. func (w *DisableableWidget) Disabled() bool { - return w.disabled.Load() + return w.disabled } // NewSimpleRenderer creates a new SimpleRenderer to render a widget using a From 39cc4655be2bb4ebb928f6c10030f07d64bf02c9 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Sat, 11 Jan 2025 13:39:03 +0100 Subject: [PATCH 08/16] Remove some unneeded locks within gridwrap, list and richtext --- widget/gridwrap.go | 12 +++--------- widget/list.go | 7 ------- widget/richtext.go | 6 ------ 3 files changed, 3 insertions(+), 22 deletions(-) diff --git a/widget/gridwrap.go b/widget/gridwrap.go index e7999040c5..41d512cf73 100644 --- a/widget/gridwrap.go +++ b/widget/gridwrap.go @@ -4,7 +4,6 @@ import ( "fmt" "math" "sort" - "sync" "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" @@ -143,9 +142,7 @@ func (l *GridWrap) RefreshItem(id GridWrapItemID) { } l.BaseWidget.Refresh() lo := l.scroller.Content.(*fyne.Container).Layout.(*gridWrapLayout) - lo.renderLock.Lock() // ensures we are not changing visible info in render code during the search item, ok := lo.searchVisible(lo.visible, id) - lo.renderLock.Unlock() if ok { lo.setupGridItem(item, id, l.focused && l.currentFocus == id) } @@ -510,10 +507,9 @@ type gridItemAndID struct { type gridWrapLayout struct { list *GridWrap - itemPool async.Pool[fyne.CanvasObject] - slicePool async.Pool[*[]gridItemAndID] - visible []gridItemAndID - renderLock sync.Mutex + itemPool async.Pool[fyne.CanvasObject] + slicePool async.Pool[*[]gridItemAndID] + visible []gridItemAndID } func newGridWrapLayout(list *GridWrap) fyne.Layout { @@ -608,7 +604,6 @@ func (l *gridWrapLayout) updateGrid(refresh bool) { // code here is a mashup of listLayout.updateList and gridWrapLayout.Layout padding := l.list.Theme().Size(theme.SizeNamePadding) - l.renderLock.Lock() length := 0 if f := l.list.Length; f != nil { length = f() @@ -679,7 +674,6 @@ func (l *gridWrapLayout) updateGrid(refresh bool) { visiblePtr := l.slicePool.Get() visible := (*visiblePtr)[:0] visible = append(visible, l.visible...) - l.renderLock.Unlock() // user code should not be locked for _, obj := range visible { l.setupGridItem(obj.item, obj.id, l.list.focused && l.list.currentFocus == obj.id) diff --git a/widget/list.go b/widget/list.go index 872e8b4022..25a0e75bd6 100644 --- a/widget/list.go +++ b/widget/list.go @@ -4,7 +4,6 @@ import ( "fmt" "math" "sort" - "sync" "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" @@ -132,9 +131,7 @@ func (l *List) RefreshItem(id ListItemID) { } l.BaseWidget.Refresh() lo := l.scroller.Content.(*fyne.Container).Layout.(*listLayout) - lo.renderLock.RLock() // ensures we are not changing visible info in render code during the search item, ok := lo.searchVisible(lo.visible, id) - lo.renderLock.RUnlock() if ok { lo.setupListItem(item, id, l.focused && l.currentFocus == id) } @@ -611,7 +608,6 @@ type listLayout struct { visible []listItemAndID slicePool async.Pool[*[]listItemAndID] visibleRowHeights []float32 - renderLock sync.RWMutex } func newListLayout(list *List) fyne.Layout { @@ -688,7 +684,6 @@ func (l *listLayout) setupListItem(li *listItem, id ListItemID, focus bool) { func (l *listLayout) updateList(newOnly bool) { th := l.list.Theme() separatorThickness := th.Size(theme.SizeNamePadding) - l.renderLock.Lock() width := l.list.Size().Width length := 0 if f := l.list.Length; f != nil { @@ -706,7 +701,6 @@ func (l *listLayout) updateList(newOnly bool) { offY, minRow := l.calculateVisibleRowHeights(l.list.itemMin.Height, length, th) if len(l.visibleRowHeights) == 0 && length > 0 { // we can't show anything until we have some dimensions - l.renderLock.Unlock() // user code should not be locked return } @@ -759,7 +753,6 @@ func (l *listLayout) updateList(newOnly bool) { visiblePtr := l.slicePool.Get() visible := (*visiblePtr)[:0] visible = append(visible, l.visible...) - l.renderLock.Unlock() // user code should not be locked if newOnly { for _, vis := range visible { diff --git a/widget/richtext.go b/widget/richtext.go index 1baf9d9f7d..c3b4b74af8 100644 --- a/widget/richtext.go +++ b/widget/richtext.go @@ -4,7 +4,6 @@ import ( "image/color" "math" "strings" - "sync" "unicode" "github.com/go-text/typesetting/di" @@ -44,7 +43,6 @@ type RichText struct { prop *canvas.Rectangle // used to apply text minsize to the scroller `scr`, if present - TODO improve #2464 visualCache map[RichTextSegment][]fyne.CanvasObject - cacheLock sync.Mutex minCache fyne.Size } @@ -205,8 +203,6 @@ func (t *RichText) deleteFromTo(lowBound int, highBound int) []rune { // cachedSegmentVisual returns a cached segment visual representation. // The offset value is > 0 if the segment had been split and so we need multiple objects. func (t *RichText) cachedSegmentVisual(seg RichTextSegment, offset int) fyne.CanvasObject { - t.cacheLock.Lock() - defer t.cacheLock.Unlock() if t.visualCache == nil { t.visualCache = make(map[RichTextSegment][]fyne.CanvasObject) } @@ -225,8 +221,6 @@ func (t *RichText) cachedSegmentVisual(seg RichTextSegment, offset int) fyne.Can } func (t *RichText) cleanVisualCache() { - t.cacheLock.Lock() - defer t.cacheLock.Unlock() if len(t.visualCache) <= len(t.Segments) { return } From 784c7bd7a88ce57d7492a5e1e7e9d093069b8a30 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Sat, 11 Jan 2025 13:40:25 +0100 Subject: [PATCH 09/16] Remove async API no longer needed --- internal/async/vector.go | 52 ----------------------------- internal/async/vector_test.go | 62 ----------------------------------- 2 files changed, 114 deletions(-) delete mode 100644 internal/async/vector.go delete mode 100644 internal/async/vector_test.go diff --git a/internal/async/vector.go b/internal/async/vector.go deleted file mode 100644 index 939b53171e..0000000000 --- a/internal/async/vector.go +++ /dev/null @@ -1,52 +0,0 @@ -package async - -import ( - "math" - "sync/atomic" - - "fyne.io/fyne/v2" -) - -// Position is an atomic version of fyne.Position. -// Loads and stores are guaranteed to happen using a single atomic operation. -type Position struct { - pos atomic.Uint64 -} - -// Load performs an atomic load on the fyne.Position value. -func (p *Position) Load() fyne.Position { - return fyne.NewPos(twoFloat32FromUint64(p.pos.Load())) -} - -// Store performs an atomic store on the fyne.Position value. -func (p *Position) Store(pos fyne.Position) { - p.pos.Store(uint64fromTwoFloat32(pos.X, pos.Y)) -} - -// Size is an atomic version of fyne.Size. -// Loads and stores are guaranteed to happen using a single atomic operation. -type Size struct { - size atomic.Uint64 -} - -// Load performs an atomic load on the fyne.Size value. -func (s *Size) Load() fyne.Size { - return fyne.NewSize(twoFloat32FromUint64(s.size.Load())) -} - -// Store performs an atomic store on the fyne.Size value. -func (s *Size) Store(size fyne.Size) { - s.size.Store(uint64fromTwoFloat32(size.Width, size.Height)) -} - -func uint64fromTwoFloat32(a, b float32) uint64 { - x := uint64(math.Float32bits(a)) - y := uint64(math.Float32bits(b)) - return (y << 32) | x -} - -func twoFloat32FromUint64(combined uint64) (float32, float32) { - x := uint32(combined) - y := uint32(combined >> 32) - return math.Float32frombits(x), math.Float32frombits(y) -} diff --git a/internal/async/vector_test.go b/internal/async/vector_test.go deleted file mode 100644 index 183bf64ec6..0000000000 --- a/internal/async/vector_test.go +++ /dev/null @@ -1,62 +0,0 @@ -package async_test - -import ( - "testing" - - "fyne.io/fyne/v2" - "fyne.io/fyne/v2/internal/async" - "github.com/stretchr/testify/assert" -) - -var globalSize fyne.Size - -func BenchmarkAtomicLoadAndStore(b *testing.B) { - local := fyne.Size{} - size := async.Size{} - - // Each loop iteration replicates how .Resize() in BaseWidget checks size changes. - for i := 0; i < b.N; i++ { - new := fyne.Size{Width: float32(i), Height: float32(i)} - if new == size.Load() { - continue - } - - size.Store(new) - } - - globalSize = local -} - -func TestSize(t *testing.T) { - size := async.Size{} - assert.Equal(t, fyne.Size{}, size.Load()) - - square := fyne.NewSquareSize(100) - size.Store(square) - assert.Equal(t, square, size.Load()) - - uneven := fyne.NewSize(125, 600) - size.Store(uneven) - assert.Equal(t, uneven, size.Load()) - - floats := fyne.NewSize(-22.565, 133.333) - size.Store(floats) - assert.Equal(t, floats, size.Load()) -} - -func TestPosition(t *testing.T) { - pos := async.Position{} - assert.Equal(t, fyne.Position{}, pos.Load()) - - even := fyne.NewSquareOffsetPos(100) - pos.Store(even) - assert.Equal(t, even, pos.Load()) - - uneven := fyne.NewPos(125, 600) - pos.Store(uneven) - assert.Equal(t, uneven, pos.Load()) - - floats := fyne.NewPos(-22.565, 133.333) - pos.Store(floats) - assert.Equal(t, floats, pos.Load()) -} From b926697b75cb47375ef5c2038d0ccd95fb122eb2 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Sat, 11 Jan 2025 22:11:35 +0100 Subject: [PATCH 10/16] Fix race button tap test --- widget/button_test.go | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/widget/button_test.go b/widget/button_test.go index 3ff075a97c..342e923a85 100644 --- a/widget/button_test.go +++ b/widget/button_test.go @@ -85,30 +85,18 @@ func TestButton_Disable(t *testing.T) { } func TestButton_Enable(t *testing.T) { - tapped := make(chan bool) + tapped := false button := widget.NewButton("Test", func() { - tapped <- true + tapped = true }) button.Disable() - go test.Tap(button) - func() { - select { - case <-tapped: - assert.Fail(t, "Button should have been disabled") - case <-time.After(1 * time.Second): - } - }() + test.Tap(button) + assert.False(t, tapped, "Button should be disabled") button.Enable() - go test.Tap(button) - func() { - select { - case <-tapped: - case <-time.After(1 * time.Second): - assert.Fail(t, "Button should have been re-enabled") - } - }() + test.Tap(button) + assert.True(t, tapped, "Button should have been re-enabled") } func TestButton_Disabled(t *testing.T) { From 8ed292a391a48f73b02eaaec201bf29a3fc6ec4f Mon Sep 17 00:00:00 2001 From: Jacalz Date: Mon, 13 Jan 2025 19:00:31 +0100 Subject: [PATCH 11/16] Got all races fixed, I think --- internal/driver/glfw/canvas_test.go | 35 +++++++----- internal/driver/glfw/window_test.go | 86 +++++++++++++++++++---------- 2 files changed, 77 insertions(+), 44 deletions(-) diff --git a/internal/driver/glfw/canvas_test.go b/internal/driver/glfw/canvas_test.go index 636cbe5e71..eb5aadccfb 100644 --- a/internal/driver/glfw/canvas_test.go +++ b/internal/driver/glfw/canvas_test.go @@ -107,23 +107,28 @@ func TestGlCanvas_ChildMinSizeChangesInDifferentScrollAffectAncestorsUpToScroll( w.Resize(oldCanvasSize) repaintWindow(w) - oldLeftColSize := leftCol.Size() - oldLeftScrollSize := leftColScroll.Size() - oldRightColSize := rightCol.Size() - oldRightScrollSize := rightColScroll.Size() - leftObj2.SetMinSize(fyne.NewSize(50, 100)) - rightObj2.SetMinSize(fyne.NewSize(50, 200)) + var oldLeftColSize, oldLeftScrollSize, oldRightColSize, oldRightScrollSize fyne.Size + runOnMain(func() { + oldLeftColSize = leftCol.Size() + oldLeftScrollSize = leftColScroll.Size() + oldRightColSize = rightCol.Size() + oldRightScrollSize = rightColScroll.Size() + leftObj2.SetMinSize(fyne.NewSize(50, 100)) + rightObj2.SetMinSize(fyne.NewSize(50, 200)) + }) c.Refresh(leftObj2) c.Refresh(rightObj2) repaintWindow(w) - assert.Equal(t, oldCanvasSize, c.Size()) - assert.Equal(t, oldLeftScrollSize, leftColScroll.Size()) - assert.Equal(t, oldRightScrollSize, rightColScroll.Size()) - expectedLeftColSize := oldLeftColSize.Add(fyne.NewSize(0, 50)) - assert.Equal(t, expectedLeftColSize, leftCol.Size()) - expectedRightColSize := oldRightColSize.Add(fyne.NewSize(0, 150)) - assert.Equal(t, expectedRightColSize, rightCol.Size()) + runOnMain(func() { + assert.Equal(t, oldCanvasSize, c.Size()) + assert.Equal(t, oldLeftScrollSize, leftColScroll.Size()) + assert.Equal(t, oldRightScrollSize, rightColScroll.Size()) + expectedLeftColSize := oldLeftColSize.Add(fyne.NewSize(0, 50)) + assert.Equal(t, expectedLeftColSize, leftCol.Size()) + expectedRightColSize := oldRightColSize.Add(fyne.NewSize(0, 150)) + assert.Equal(t, expectedRightColSize, rightCol.Size()) + }) } func TestGlCanvas_Content(t *testing.T) { @@ -417,7 +422,9 @@ func TestGlCanvas_ResizeWithOtherOverlay(t *testing.T) { }) size = fyne.NewSize(200, 100) - assert.NotEqual(t, size, content.Size()) + runOnMain(func() { + assert.NotEqual(t, size, content.Size()) + }) w.Resize(size) ensureCanvasSize(t, w, size) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index 0a382b5361..5e4618eb1c 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -98,7 +98,9 @@ func TestWindow_MinSize_Fixed(t *testing.T) { assertCanvasSize(t, w, minSizePlusPadding) w = createWindow("Test") - r.SetMinSize(fyne.NewSize(100, 100)) + runOnMain(func() { + r.SetMinSize(fyne.NewSize(100, 100)) + }) w.SetFixedSize(true) w.SetContent(r) assertCanvasSize(t, w, minSizePlusPadding) @@ -272,8 +274,10 @@ func TestWindow_HandleHoverable(t *testing.T) { w.Resize(fyne.NewSize(30, 20)) repaintWindow(w) - require.Equal(t, fyne.NewPos(0, 0), h1.Position()) - require.Equal(t, fyne.NewPos(14, 0), h2.Position()) + runOnMain(func() { + require.Equal(t, fyne.NewPos(0, 0), h1.Position()) + require.Equal(t, fyne.NewPos(14, 0), h2.Position()) + }) runOnMain(func() { w.mouseMoved(w.viewport, 9, 9) @@ -1054,7 +1058,9 @@ func TestWindow_DragEndWithoutTappedEvent(t *testing.T) { w.SetContent(do) repaintWindow(w) - require.Equal(t, fyne.NewPos(4, 4), do.Position()) + runOnMain(func() { + require.Equal(t, fyne.NewPos(4, 4), do.Position()) + }) runOnMain(func() { w.mouseMoved(w.viewport, 11, 11) @@ -1447,16 +1453,24 @@ func TestWindow_SetPadded(t *testing.T) { ensureCanvasSize(t, w, oldCanvasSize) repaintWindow(w) - contentSize := content.Size() - expectedCanvasSize := contentSize. - Add(fyne.NewSize(2*tt.expectedPad, 2*tt.expectedPad)). - Add(fyne.NewSize(0, tt.expectedMenuHeight)) + + var contentSize, expectedCanvasSize fyne.Size + runOnMain(func() { + contentSize = content.Size() + expectedCanvasSize = contentSize. + Add(fyne.NewSize(2*tt.expectedPad, 2*tt.expectedPad)). + Add(fyne.NewSize(0, tt.expectedMenuHeight)) + }) w.SetPadded(tt.padding) repaintWindow(w) - assert.Equal(t, contentSize, content.Size()) - assert.Equal(t, fyne.NewPos(tt.expectedPad, tt.expectedPad+tt.expectedMenuHeight), content.Position()) - assert.Equal(t, expectedCanvasSize, w.Canvas().Size()) + + canvas := w.Canvas() + runOnMain(func() { + assert.Equal(t, contentSize, content.Size()) + assert.Equal(t, fyne.NewPos(tt.expectedPad, tt.expectedPad+tt.expectedMenuHeight), content.Position()) + assert.Equal(t, expectedCanvasSize, canvas.Size()) + }) }) } } @@ -1564,33 +1578,45 @@ func TestWindow_ManualFocus(t *testing.T) { w.SetContent(content) repaintWindow(w) - w.mouseMoved(w.viewport, 9, 9) - w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) - w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) - assert.Equal(t, 1, content.focusedTimes) - assert.Equal(t, 0, content.unfocusedTimes) + runOnMain(func() { + w.mouseMoved(w.viewport, 9, 9) + w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) + w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) + assert.Equal(t, 1, content.focusedTimes) + assert.Equal(t, 0, content.unfocusedTimes) - w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) - w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) - assert.Equal(t, 1, content.focusedTimes) - assert.Equal(t, 0, content.unfocusedTimes) + w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) + w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) + assert.Equal(t, 1, content.focusedTimes) + assert.Equal(t, 0, content.unfocusedTimes) + }) w.Canvas().Focus(content) - assert.Equal(t, 1, content.focusedTimes) - assert.Equal(t, 0, content.unfocusedTimes) + + runOnMain(func() { + assert.Equal(t, 1, content.focusedTimes) + assert.Equal(t, 0, content.unfocusedTimes) + }) w.Canvas().Unfocus() - assert.Equal(t, 1, content.focusedTimes) - assert.Equal(t, 1, content.unfocusedTimes) - content.Disable() + runOnMain(func() { + assert.Equal(t, 1, content.focusedTimes) + assert.Equal(t, 1, content.unfocusedTimes) + + content.Disable() + }) + w.Canvas().Focus(content) - assert.Equal(t, 1, content.focusedTimes) - assert.Equal(t, 1, content.unfocusedTimes) - w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) - assert.Equal(t, 1, content.focusedTimes) - assert.Equal(t, 1, content.unfocusedTimes) + runOnMain(func() { + assert.Equal(t, 1, content.focusedTimes) + assert.Equal(t, 1, content.unfocusedTimes) + + w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) + assert.Equal(t, 1, content.focusedTimes) + assert.Equal(t, 1, content.unfocusedTimes) + }) } func TestWindow_ClipboardCopy_DisabledEntry(t *testing.T) { From e547b5f30057e9774e79310c2f0d07e99d28a825 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Mon, 13 Jan 2025 19:02:29 +0100 Subject: [PATCH 12/16] One more glfw driver race fix --- internal/driver/glfw/window_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index 5e4618eb1c..efe6894f11 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -1412,9 +1412,11 @@ func TestWindow_Padded(t *testing.T) { content := canvas.NewRectangle(color.White) w.SetContent(content) - width, _ := w.minSizeOnScreen() - assert.Equal(t, int(theme.Padding()*2+content.MinSize().Width), width) - assert.Equal(t, theme.Padding(), content.Position().X) + runOnMain(func() { + width, _ := w.minSizeOnScreen() + assert.Equal(t, int(theme.Padding()*2+content.MinSize().Width), width) + assert.Equal(t, theme.Padding(), content.Position().X) + }) } func TestWindow_SetPadded(t *testing.T) { From c9c42a78eb675e6eb4138eb0bf49811143c6ffba Mon Sep 17 00:00:00 2001 From: Jacalz Date: Mon, 13 Jan 2025 19:41:46 +0100 Subject: [PATCH 13/16] CI seems to get more races than me --- internal/driver/glfw/canvas_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/driver/glfw/canvas_test.go b/internal/driver/glfw/canvas_test.go index eb5aadccfb..0688e3d0bd 100644 --- a/internal/driver/glfw/canvas_test.go +++ b/internal/driver/glfw/canvas_test.go @@ -398,11 +398,16 @@ func TestGlCanvas_Resize(t *testing.T) { ensureCanvasSize(t, w, fyne.NewSize(69, 36)) size := fyne.NewSize(200, 100) - assert.NotEqual(t, size, content.Size()) + runOnMain(func() { + assert.NotEqual(t, size, content.Size()) + }) w.Resize(size) ensureCanvasSize(t, w, size) - assert.Equal(t, size, content.Size()) + + runOnMain(func() { + assert.Equal(t, size, content.Size()) + }) } // TODO: this can be removed when #707 is addressed From b6f7a23a2257070f79540b4f9f0cd49ebf7c5ef5 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Mon, 13 Jan 2025 19:43:29 +0100 Subject: [PATCH 14/16] Another race that CI reported --- internal/driver/glfw/window_test.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index efe6894f11..c859e6cb26 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -1121,15 +1121,17 @@ func TestWindow_TappedSecondary(t *testing.T) { o.SetMinSize(fyne.NewSize(100, 100)) w.SetContent(o) - w.mousePos = fyne.NewPos(50, 60) - w.mouseClicked(w.viewport, glfw.MouseButton2, glfw.Press, 0) - w.mouseClicked(w.viewport, glfw.MouseButton2, glfw.Release, 0) + runOnMain(func() { + w.mousePos = fyne.NewPos(50, 60) + w.mouseClicked(w.viewport, glfw.MouseButton2, glfw.Press, 0) + w.mouseClicked(w.viewport, glfw.MouseButton2, glfw.Release, 0) - assert.Nil(t, o.popTapEvent(), "no primary tap") - if e, _ := o.popSecondaryTapEvent().(*fyne.PointEvent); assert.NotNil(t, e, "tapped secondary") { - assert.Equal(t, fyne.NewPos(50, 60), e.AbsolutePosition) - assert.Equal(t, fyne.NewPos(46, 56), e.Position) - } + assert.Nil(t, o.popTapEvent(), "no primary tap") + if e, _ := o.popSecondaryTapEvent().(*fyne.PointEvent); assert.NotNil(t, e, "tapped secondary") { + assert.Equal(t, fyne.NewPos(50, 60), e.AbsolutePosition) + assert.Equal(t, fyne.NewPos(46, 56), e.Position) + } + }) } func TestWindow_TappedSecondary_OnPrimaryOnlyTarget(t *testing.T) { From b33ecc6f70735806e81fae6ca0134e876e0bc190 Mon Sep 17 00:00:00 2001 From: Jacalz Date: Mon, 13 Jan 2025 19:47:17 +0100 Subject: [PATCH 15/16] More small race fixes in glfw driver --- internal/driver/glfw/window_test.go | 32 +++++++++++++++-------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index c859e6cb26..a5547a1eb1 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -1143,18 +1143,18 @@ func TestWindow_TappedSecondary_OnPrimaryOnlyTarget(t *testing.T) { w.SetContent(o) ensureCanvasSize(t, w, fyne.NewSize(53, 44)) - w.mousePos = fyne.NewPos(10, 25) - w.mouseClicked(w.viewport, glfw.MouseButton2, glfw.Press, 0) - w.mouseClicked(w.viewport, glfw.MouseButton2, glfw.Release, 0) + runOnMain(func() { + w.mousePos = fyne.NewPos(10, 25) + w.mouseClicked(w.viewport, glfw.MouseButton2, glfw.Press, 0) + w.mouseClicked(w.viewport, glfw.MouseButton2, glfw.Release, 0) - assert.False(t, tapped) + assert.False(t, tapped) - runOnMain(func() { w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) - }) - assert.True(t, tapped) + assert.True(t, tapped) + }) } func TestWindow_TappedIgnoresScrollerClip(t *testing.T) { @@ -1177,17 +1177,19 @@ func TestWindow_TappedIgnoresScrollerClip(t *testing.T) { refreshWindow(w.window) // ensure any async resize is done ensureCanvasSize(t, w, fyne.NewSize(108, 212)) - w.mousePos = fyne.NewPos(10, 80) - w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) - w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) + runOnMain(func() { + w.mousePos = fyne.NewPos(10, 80) + w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) + w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) - assert.False(t, tapped, "Tapped button that was clipped") + assert.False(t, tapped, "Tapped button that was clipped") - w.mousePos = fyne.NewPos(10, 120) - w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) - w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) + w.mousePos = fyne.NewPos(10, 120) + w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) + w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0) - assert.True(t, tapped, "Tapped button that was clipped") + assert.True(t, tapped, "Tapped button that was clipped") + }) } func TestWindow_TappedIgnoredWhenMovedOffOfTappable(t *testing.T) { From 4773721e7aa50721999ee36b71dce5642233364c Mon Sep 17 00:00:00 2001 From: Jacalz Date: Mon, 13 Jan 2025 20:03:19 +0100 Subject: [PATCH 16/16] One more to make CI races happy? --- internal/driver/glfw/window_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/driver/glfw/window_test.go b/internal/driver/glfw/window_test.go index a5547a1eb1..5a6973eb35 100644 --- a/internal/driver/glfw/window_test.go +++ b/internal/driver/glfw/window_test.go @@ -1232,9 +1232,10 @@ func TestWindow_TappedAndDoubleTapped(t *testing.T) { waitDoubleTapped <- struct{}{} } w.SetContent(but) - but.Resize(fyne.NewSquareSize(50)) runOnMain(func() { + but.Resize(fyne.NewSquareSize(50)) + w.mouseMoved(w.viewport, 15, 25) w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Press, 0) w.mouseClicked(w.viewport, glfw.MouseButton1, glfw.Release, 0)