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

Add a cache for MinSize in BaseWidget #4827

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
18 changes: 17 additions & 1 deletion internal/widget/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type Base struct {
hidden atomic.Bool
position async.Position
size async.Size
minCache async.Size
impl atomic.Pointer[fyne.Widget]
}

Expand Down Expand Up @@ -63,14 +64,21 @@ func (w *Base) Move(pos fyne.Position) {

// MinSize for the widget - it should never be resized below this value.
func (w *Base) MinSize() fyne.Size {
minCache := w.minCache.Load()
if !minCache.IsZero() {
return minCache
}

impl := w.super()

r := cache.Renderer(impl)
if r == nil {
return fyne.NewSize(0, 0)
}

return r.MinSize()
minSize := r.MinSize()
w.minCache.Store(minSize)
return minSize
}

// Visible returns whether or not this widget should be visible.
Expand Down Expand Up @@ -112,10 +120,18 @@ func (w *Base) Refresh() {
return
}

w.minCache.Store(fyne.Size{})
render := cache.Renderer(impl)
render.Refresh()
}

// ResetMinSizeCache resets the cached MinSize for this widget.
//
// Since: 2.5.0
func (w *Base) ResetMinSizeCache() {
w.minCache.Store(fyne.Size{})
}

// 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 {
Expand Down
16 changes: 8 additions & 8 deletions widget/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ type Check struct {
hovered bool

binder basicBinder

minSize fyne.Size // cached for hover/tap position calculations
}

// NewCheck creates a new check widget with the set label and change handler
Expand Down Expand Up @@ -119,8 +117,9 @@ func (c *Check) MouseMoved(me *desktop.MouseEvent) {

// only hovered if cached minSize has not been initialized (test code)
// or the pointer is within the "active" area of the widget (its minSize)
c.hovered = c.minSize.IsZero() ||
(me.Position.X <= c.minSize.Width && me.Position.Y <= c.minSize.Height)
minSize := c.MinSize()
c.hovered = minSize.IsZero() ||
(me.Position.X <= minSize.Width && me.Position.Y <= minSize.Height)

if oldHovered != c.hovered {
c.Refresh()
Expand All @@ -132,8 +131,10 @@ func (c *Check) Tapped(pe *fyne.PointEvent) {
if c.Disabled() {
return
}
if !c.minSize.IsZero() &&
(pe.Position.X > c.minSize.Width || pe.Position.Y > c.minSize.Height) {

minSize := c.MinSize()
if !minSize.IsZero() &&
(pe.Position.X > minSize.Width || pe.Position.Y > minSize.Height) {
// tapped outside the active area of the widget
return
}
Expand All @@ -151,8 +152,7 @@ func (c *Check) Tapped(pe *fyne.PointEvent) {
// MinSize returns the size that this widget should not shrink below
func (c *Check) MinSize() fyne.Size {
c.ExtendBaseWidget(c)
c.minSize = c.BaseWidget.MinSize()
return c.minSize
return c.BaseWidget.MinSize()
}

// CreateRenderer is a private method to Fyne which links this widget to its renderer
Expand Down
23 changes: 1 addition & 22 deletions widget/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ type Entry struct {
ActionItem fyne.CanvasObject `json:"-"`
binder basicBinder
conversionError error
minCache fyne.Size
multiLineRows int // override global default number of visible lines

// undoStack stores the data necessary for undo/redo functionality
Expand Down Expand Up @@ -402,20 +401,8 @@ func (e *Entry) KeyUp(key *fyne.KeyEvent) {
//
// Implements: fyne.Widget
func (e *Entry) MinSize() fyne.Size {
e.propertyLock.RLock()
cached := e.minCache
e.propertyLock.RUnlock()
if !cached.IsZero() {
return cached
}

e.ExtendBaseWidget(e)
min := e.BaseWidget.MinSize()

e.propertyLock.Lock()
e.minCache = min
e.propertyLock.Unlock()
return min
return e.BaseWidget.MinSize()
}

// MouseDown called on mouse click, this triggers a mouse click which can move the cursor,
Expand Down Expand Up @@ -480,14 +467,6 @@ func (e *Entry) Redo() {
e.Refresh()
}

func (e *Entry) Refresh() {
e.propertyLock.Lock()
e.minCache = fyne.Size{}
e.propertyLock.Unlock()

e.BaseWidget.Refresh()
}

// SelectedText returns the text currently selected in this Entry.
// If there is no selection it will return the empty string.
func (e *Entry) SelectedText() string {
Expand Down
13 changes: 6 additions & 7 deletions widget/richtext.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ type RichText struct {

visualCache map[RichTextSegment][]fyne.CanvasObject
cacheLock sync.Mutex
minCache fyne.Size
}

// NewRichText returns a new RichText widget that renders the given text and segments.
Expand Down Expand Up @@ -89,19 +88,18 @@ func (t *RichText) CreateRenderer() fyne.WidgetRenderer {
// MinSize calculates the minimum size of a rich text widget.
// This is based on the contained text with a standard amount of padding added.
func (t *RichText) MinSize() fyne.Size {
// we don't return the minCache here, as any internal segments could have caused it to change...
t.ExtendBaseWidget(t)

min := t.BaseWidget.MinSize()
t.minCache = min
return min
// We don't return the cached value as any internal segments could have caused it to change...
Copy link
Member Author

@Jacalz Jacalz May 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should likely fix this as it kind of goes against the idea of refreshing a widget when it changes state. Sounds like a bug but it isn't new behaviour to this PR so I'll let it slide for now.

t.ResetMinSizeCache()
return t.BaseWidget.MinSize()
}

// Refresh triggers a redraw of the rich text.
//
// Implements: fyne.Widget
func (t *RichText) Refresh() {
t.minCache = fyne.Size{}
t.ResetMinSizeCache()
t.updateRowBounds()

for _, s := range t.Segments {
Expand All @@ -123,10 +121,11 @@ func (t *RichText) Resize(size fyne.Size) {
}

t.size.Store(size)
minSize := t.MinSize()

t.propertyLock.RLock()
segments := t.Segments
skipResize := !t.minCache.IsZero() && size.Width >= t.minCache.Width && size.Height >= t.minCache.Height && t.Wrapping == fyne.TextWrapOff && t.Truncation == fyne.TextTruncateOff
skipResize := !minSize.IsZero() && size.Width >= minSize.Width && size.Height >= minSize.Height && t.Wrapping == fyne.TextWrapOff && t.Truncation == fyne.TextTruncateOff
t.propertyLock.RUnlock()

if skipResize {
Expand Down
2 changes: 1 addition & 1 deletion widget/select_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (e *SelectEntry) Disable() {
// Implements: fyne.Widget
func (e *SelectEntry) MinSize() fyne.Size {
e.ExtendBaseWidget(e)
return e.Entry.MinSize()
return e.BaseWidget.MinSize()
}

// Move changes the relative position of the select entry.
Expand Down
20 changes: 19 additions & 1 deletion widget/widget.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
// BaseWidget provides a helper that handles basic widget behaviours.
type BaseWidget struct {
size async.Size
minCache async.Size
position async.Position
Hidden bool

Expand Down Expand Up @@ -69,14 +70,21 @@ func (w *BaseWidget) Move(pos fyne.Position) {

// MinSize for the widget - it should never be resized below this value.
func (w *BaseWidget) MinSize() fyne.Size {
minCache := w.minCache.Load()
if !minCache.IsZero() {
return minCache
}

impl := w.super()

r := cache.Renderer(impl)
if r == nil {
return fyne.Size{}
}

return r.MinSize()
minSize := r.MinSize()
w.minCache.Store(minSize)
return minSize
}

// Visible returns whether or not this widget should be visible.
Expand Down Expand Up @@ -123,6 +131,7 @@ func (w *BaseWidget) Refresh() {
return
}

w.minCache.Store(fyne.Size{})
w.propertyLock.Lock()
w.themeCache = nil
w.propertyLock.Unlock()
Expand Down Expand Up @@ -156,6 +165,13 @@ func (w *BaseWidget) themeWithLock() fyne.Theme {
return cached
}

// ResetMinSizeCache resets the cached MinSize for this widget.
//
// Since: 2.5.0
func (w *BaseWidget) ResetMinSizeCache() {
w.minCache.Store(fyne.Size{})
}

// SetFieldsAndRefresh helps to make changes to a widget that should be followed by a refresh.
// This method is a guaranteed thread-safe way of directly manipulating widget fields.
// Widgets extending BaseWidget should use this in their setter functions.
Expand All @@ -170,6 +186,8 @@ func (w *BaseWidget) SetFieldsAndRefresh(f func()) {
if impl == nil {
return
}

w.minCache.Store(fyne.Size{})
impl.Refresh()
}

Expand Down
34 changes: 31 additions & 3 deletions widget/widget_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,21 +79,49 @@ func TestSimpleRenderer(t *testing.T) {
test.AssertImageMatches(t, "simple_renderer.png", window.Canvas().Capture())
}

func TestMinSizeCache(t *testing.T) {
label := NewLabel("a")

wid := newTestWidget(label)
assert.Equal(t, 0, wid.minSizeCalls)

minSize := wid.MinSize()
assert.NotEqual(t, 0, minSize)
assert.Equal(t, 1, wid.minSizeCalls)

wid.MinSize()
assert.Equal(t, 1, wid.minSizeCalls)
}

type testWidget struct {
BaseWidget
obj fyne.CanvasObject
obj fyne.CanvasObject
minSizeCalls int
}

func newTestWidget(o fyne.CanvasObject) fyne.Widget {
func newTestWidget(o fyne.CanvasObject) *testWidget {
t := &testWidget{obj: o}
t.ExtendBaseWidget(t)
return t
}

func (t *testWidget) CreateRenderer() fyne.WidgetRenderer {
return NewSimpleRenderer(t.obj)
return &testRenderer{
WidgetRenderer: NewSimpleRenderer(t.obj),
widget: t,
}
}

func waitForBinding() {
time.Sleep(time.Millisecond * 100) // data resolves on background thread
}

type testRenderer struct {
widget *testWidget
fyne.WidgetRenderer
}

func (r *testRenderer) MinSize() fyne.Size {
r.widget.minSizeCalls++
return r.WidgetRenderer.MinSize()
}
Loading