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

RFC: Add caches to avoid recomputing MinSize multiple times #4681

Closed
wants to merge 11 commits into from
12 changes: 11 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()
min := r.MinSize()
w.minCache.Store(min)
return min
}

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

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

render := cache.Renderer(impl)
render.Refresh()
}
Expand Down
3 changes: 1 addition & 2 deletions widget/activity.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ func NewActivity() *Activity {

func (a *Activity) MinSize() fyne.Size {
a.ExtendBaseWidget(a)

return fyne.NewSquareSize(a.Theme().Size(theme.SizeNameInlineIcon))
return a.BaseWidget.MinSize()
}

// Start the activity indicator animation
Expand Down
15 changes: 7 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,9 @@ 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 +151,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
72 changes: 28 additions & 44 deletions widget/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,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 @@ -401,29 +400,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)

th := e.Theme()
iconSpace := th.Size(theme.SizeNameInlineIcon) + th.Size(theme.SizeNameLineSpacing)
min := e.BaseWidget.MinSize()
if e.ActionItem != nil {
min = min.Add(fyne.NewSize(iconSpace, 0))
}
if e.Validator != nil {
min = min.Add(fyne.NewSize(iconSpace, 0))
}

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 @@ -488,14 +466,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 Expand Up @@ -1679,25 +1649,39 @@ func (r *entryRenderer) MinSize() fyne.Size {
if rend := cache.Renderer(r.entry.content); rend != nil {
rend.(*entryContentRenderer).updateScrollDirections()
}
if r.scroll.Direction == widget.ScrollNone {
return r.entry.content.MinSize().Add(fyne.NewSize(0, r.entry.Theme().Size(theme.SizeNameInputBorder)*2))
}

innerPadding := r.entry.Theme().Size(theme.SizeNameInnerPadding)
textSize := r.entry.Theme().Size(theme.SizeNameText)
charMin := r.entry.placeholderProvider().charMinSize(r.entry.Password, r.entry.TextStyle, textSize)
minSize := charMin.Add(fyne.NewSquareSize(innerPadding))
th := r.entry.Theme()
minSize := fyne.Size{}

if r.scroll.Direction == widget.ScrollNone {
minSize = r.entry.content.MinSize().AddWidthHeight(0, th.Size(theme.SizeNameInputBorder)*2)
} else {
innerPadding := th.Size(theme.SizeNameInnerPadding)
textSize := th.Size(theme.SizeNameText)
charMin := r.entry.placeholderProvider().charMinSize(r.entry.Password, r.entry.TextStyle, textSize)
minSize = charMin.Add(fyne.NewSquareSize(innerPadding))

if r.entry.MultiLine {
count := r.entry.multiLineRows
if count <= 0 {
count = multiLineRows
}

if r.entry.MultiLine {
count := r.entry.multiLineRows
if count <= 0 {
count = multiLineRows
minSize.Height = charMin.Height*float32(count) + innerPadding
}

minSize.Height = charMin.Height*float32(count) + innerPadding
minSize = minSize.AddWidthHeight(innerPadding*2, innerPadding)
}

iconSpace := th.Size(theme.SizeNameInlineIcon) + th.Size(theme.SizeNameLineSpacing)
if r.entry.ActionItem != nil {
minSize.Width += iconSpace
}
if r.entry.Validator != nil {
minSize.Width += iconSpace
}

return minSize.Add(fyne.NewSize(innerPadding*2, innerPadding))
return minSize
}

func (r *entryRenderer) Objects() []fyne.CanvasObject {
Expand Down
2 changes: 1 addition & 1 deletion widget/entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ func TestEntry_MinSize(t *testing.T) {
min = entry.MinSize()
entry.ActionItem = canvas.NewCircle(color.Black)
entry.Refresh()
assert.Equal(t, min.Add(fyne.NewSize(theme.IconInlineSize()+theme.Padding(), 0)), entry.MinSize())
assert.Equal(t, min.Add(fyne.NewSize(theme.IconInlineSize()+theme.LineSpacing(), 0)), entry.MinSize())
}

func TestEntryMultiline_MinSize(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion widget/gridwrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ func (l *GridWrap) FocusLost() {
// MinSize returns the size that this widget should not shrink below.
func (l *GridWrap) MinSize() fyne.Size {
l.ExtendBaseWidget(l)

return l.BaseWidget.MinSize()
}

Expand Down
8 changes: 3 additions & 5 deletions widget/hyperlink.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,8 @@ func (hl *Hyperlink) Refresh() {

// MinSize returns the smallest size this widget can shrink to
func (hl *Hyperlink) MinSize() fyne.Size {
if len(hl.provider.Segments) == 0 {
hl.syncSegments()
}

return hl.provider.MinSize()
hl.ExtendBaseWidget(hl)
return hl.BaseWidget.MinSize()
}

// Resize sets a new size for the hyperlink.
Expand All @@ -191,6 +188,7 @@ func (hl *Hyperlink) SetText(text string) {
return // Not initialized yet.
}
hl.syncSegments()
hl.ResetMinSizeCache()
hl.provider.Refresh()
}

Expand Down
9 changes: 2 additions & 7 deletions widget/label.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package widget
import (
"fyne.io/fyne/v2"
"fyne.io/fyne/v2/data/binding"
"fyne.io/fyne/v2/internal/cache"
"fyne.io/fyne/v2/theme"
)

Expand Down Expand Up @@ -77,12 +76,8 @@ func (l *Label) CreateRenderer() fyne.WidgetRenderer {
//
// Implements: fyne.Widget
func (l *Label) MinSize() fyne.Size {
if l.provider == nil {
l.ExtendBaseWidget(l)
cache.Renderer(l.super())
}

return l.provider.MinSize()
l.ExtendBaseWidget(l)
return l.BaseWidget.MinSize()
}

// Refresh triggers a redraw of the label.
Expand Down
1 change: 0 additions & 1 deletion widget/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ func (l *List) FocusLost() {
// MinSize returns the size that this widget should not shrink below.
func (l *List) MinSize() fyne.Size {
l.ExtendBaseWidget(l)

return l.BaseWidget.MinSize()
}

Expand Down
11 changes: 3 additions & 8 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,14 @@ 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
return t.BaseWidget.MinSize()
}

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

for _, s := range t.Segments {
Expand All @@ -123,10 +117,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
6 changes: 2 additions & 4 deletions widget/separator.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ func (s *Separator) CreateRenderer() fyne.WidgetRenderer {
// Implements: fyne.Widget
func (s *Separator) MinSize() fyne.Size {
s.ExtendBaseWidget(s)
t := s.Theme().Size(theme.SizeNameSeparatorThickness)
return fyne.NewSize(t, t)
return s.BaseWidget.MinSize()
}

var _ fyne.WidgetRenderer = (*separatorRenderer)(nil)
Expand All @@ -68,8 +67,7 @@ type separatorRenderer struct {
}

func (r *separatorRenderer) MinSize() fyne.Size {
t := r.d.Theme().Size(theme.SizeNameSeparatorThickness)
return fyne.NewSize(t, t)
return fyne.NewSquareSize(r.d.Theme().Size(theme.SizeNameSeparatorThickness))
}

func (r *separatorRenderer) Refresh() {
Expand Down
Loading
Loading