Skip to content

Commit

Permalink
don't allocate a new map for visible objects on every updateList
Browse files Browse the repository at this point in the history
  • Loading branch information
@dweymouth
dweymouth committed Nov 18, 2023
1 parent 38aba32 commit 1f9735d
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 44 deletions.
97 changes: 63 additions & 34 deletions widget/list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,8 @@ func (l *List) RefreshItem(id ListItemID) {
}
l.BaseWidget.Refresh()
lo := l.scroller.Content.(*fyne.Container).Layout.(*listLayout)
visible := lo.visible
if item, ok := visible[id]; ok {
lo.setupListItem(item, id, l.focused && l.currentFocus == id)
if idx := lo.searchVisible(lo.visible, id); idx >= 0 {
lo.setupListItem(lo.visible[idx].item, id, l.focused && l.currentFocus == id)
}
}

Expand Down Expand Up @@ -523,19 +522,25 @@ func (li *listItemRenderer) Refresh() {
// Declare conformity with Layout interface.
var _ fyne.Layout = (*listLayout)(nil)

type itemAndID struct {
item *listItem
id ListItemID
}

type listLayout struct {
list *List
separators []fyne.CanvasObject
children []fyne.CanvasObject

itemPool *syncPool
visible map[ListItemID]*listItem
visible []itemAndID
wasVisible []itemAndID
visibleRowHeights []float32
renderLock sync.Mutex
}

func newListLayout(list *List) fyne.Layout {
l := &listLayout{list: list, itemPool: &syncPool{}, visible: make(map[ListItemID]*listItem)}
l := &listLayout{list: list, itemPool: &syncPool{}}
list.offsetUpdated = l.offsetUpdated
return l
}
Expand Down Expand Up @@ -637,7 +642,11 @@ func (l *listLayout) updateList(newOnly bool) {
fyne.LogError("Missing UpdateCell callback required for List", nil)
}

wasVisible := l.visible
for i := 0; i < len(l.wasVisible); i++ {
l.wasVisible[i].item = nil
}
l.wasVisible = l.wasVisible[:0]
l.wasVisible = append(l.wasVisible, l.visible...)

l.list.propertyLock.Lock()
offY, minRow := l.calculateVisibleRowHeights(l.list.itemMin.Height, length)
Expand All @@ -647,7 +656,8 @@ func (l *listLayout) updateList(newOnly bool) {
return
}

visible := make(map[ListItemID]*listItem, len(l.visibleRowHeights))
oldVisibleLen := len(l.visible)
l.visible = l.visible[:0]
oldChildrenLen := len(l.children)
l.children = l.children[:0]

Expand All @@ -656,36 +666,39 @@ func (l *listLayout) updateList(newOnly bool) {
row := index + minRow
size := fyne.NewSize(width, itemHeight)

c, ok := wasVisible[row]
if !ok {
visIdx := l.searchVisible(l.wasVisible, row)
var c *listItem
if visIdx < 0 {
c = l.getItem()
if c == nil {
continue
}
c.Resize(size)
} else {
c = l.wasVisible[visIdx].item
}

c.Move(fyne.NewPos(0, y))
c.Resize(size)

y += itemHeight + separatorThickness
visible[row] = c
l.visible = append(l.visible, itemAndID{id: row, item: c})
l.children = append(l.children, c)
}
if ln := len(l.children); oldChildrenLen > ln {
// nil out slice's unreferenced items
l.children = l.children[:oldChildrenLen]
for i := ln; i < oldChildrenLen; i++ {
l.children[i] = nil
l.nilOldSliceData(l.children, len(l.children), oldChildrenLen)
// nil out l.visible slice's unreferenced items
if ln := len(l.visible); oldVisibleLen > ln {
l.visible = l.visible[:oldVisibleLen]
for i := ln; i < oldVisibleLen; i++ {
l.visible[i].item = nil
}
l.children = l.children[:ln]
l.visible = l.visible[:ln]
}

l.visible = visible

for id, old := range wasVisible {
if _, ok := l.visible[id]; !ok {
l.itemPool.Release(old)
for _, item := range l.wasVisible {
if idx := l.searchVisible(l.visible, item.id); idx < 0 {
l.itemPool.Release(item.item)
item.item = nil
}
}

Expand All @@ -696,27 +709,25 @@ func (l *listLayout) updateList(newOnly bool) {
c.Objects = c.Objects[:0]
c.Objects = append(c.Objects, l.children...)
c.Objects = append(c.Objects, l.separators...)
if ln := len(c.Objects); oldObjLen > ln {
// nil out slice's unreferenced items
c.Objects = c.Objects[:oldObjLen]
for i := ln; i < oldObjLen; i++ {
c.Objects[i] = nil
}
c.Objects = c.Objects[:ln]
}
l.nilOldSliceData(c.Objects, len(c.Objects), oldObjLen)
l.renderLock.Unlock() // user code should not be locked

if newOnly {
for row, obj := range visible {
if _, ok := wasVisible[row]; !ok {
l.setupListItem(obj, row, l.list.focused && l.list.currentFocus == row)
for _, item := range l.visible {
if idx := l.searchVisible(l.wasVisible, item.id); idx < 0 {
l.setupListItem(item.item, item.id, l.list.focused && l.list.currentFocus == item.id)
}
}
} else {
for row, obj := range visible {
l.setupListItem(obj, row, l.list.focused && l.list.currentFocus == row)
for _, item := range l.visible {
l.setupListItem(item.item, item.id, l.list.focused && l.list.currentFocus == item.id)
}
}

// we don't need wasVisible's data anymore; nil out all references
for i := 0; i < len(l.wasVisible); i++ {
l.wasVisible[i].item = nil
}
}

func (l *listLayout) updateSeparators() {
Expand All @@ -743,3 +754,21 @@ func (l *listLayout) updateSeparators() {
l.separators[i].Show()
}
}

func (l *listLayout) searchVisible(visible []itemAndID, id ListItemID) int {
for i, v := range visible {
if v.id == id {
return i
}
}
return -1
}

func (l *listLayout) nilOldSliceData(objs []fyne.CanvasObject, len, oldLen int) {
if oldLen > len {
objs = objs[:oldLen] // gain view into old data
for i := len; i < oldLen; i++ {
objs[i] = nil
}
}
}
22 changes: 12 additions & 10 deletions widget/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,22 +275,24 @@ func TestList_Select(t *testing.T) {
assert.Equal(t, float32(0), list.offsetY)
list.Select(50)
assert.Equal(t, 988, int(list.offsetY))
visible := list.scroller.Content.(*fyne.Container).Layout.(*listLayout).visible
assert.Equal(t, visible[50].background.FillColor, theme.SelectionColor())
assert.True(t, visible[50].background.Visible())
lo := list.scroller.Content.(*fyne.Container).Layout.(*listLayout)
visible50 := lo.visible[lo.searchVisible(lo.visible, 50)].item
assert.Equal(t, visible50.background.FillColor, theme.SelectionColor())
assert.True(t, visible50.background.Visible())

list.Select(5)
assert.Equal(t, 195, int(list.offsetY))
visible = list.scroller.Content.(*fyne.Container).Layout.(*listLayout).visible
assert.Equal(t, visible[5].background.FillColor, theme.SelectionColor())
assert.True(t, visible[5].background.Visible())
visible5 := lo.visible[lo.searchVisible(lo.visible, 5)].item
assert.Equal(t, visible5.background.FillColor, theme.SelectionColor())
assert.True(t, visible5.background.Visible())

list.Select(6)
assert.Equal(t, 195, int(list.offsetY))
visible = list.scroller.Content.(*fyne.Container).Layout.(*listLayout).visible
assert.False(t, visible[5].background.Visible())
assert.Equal(t, visible[6].background.FillColor, theme.SelectionColor())
assert.True(t, visible[6].background.Visible())
visible5 = lo.visible[lo.searchVisible(lo.visible, 5)].item
visible6 := lo.visible[lo.searchVisible(lo.visible, 6)].item
assert.False(t, visible5.background.Visible())
assert.Equal(t, visible6.background.FillColor, theme.SelectionColor())
assert.True(t, visible6.background.Visible())
}

func TestList_Unselect(t *testing.T) {
Expand Down

0 comments on commit 1f9735d

Please sign in to comment.