diff --git a/CHANGELOG.md b/CHANGELOG.md index a84e35db..87aa8e46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -475,15 +475,15 @@ because it turns other people's test suites red. ### Fixed -- **The window uses far less memory, and rebuilding a screen no longer adds - to it.** Every - quiet line on a screen - a subtitle, a caption, the count of bytes beside a - size, the line a folded section keeps, the message under a field - parsed - its own copy of the fonts, and parsed another each time the screen was - rebuilt and the line said something new. After visiting the four tabs the - window held about 290 MB and it now holds about 120 MB, and rebuilding a - screen no longer adds to it. Nothing on the screen looks different. The open - list of formats still does this when you type into its filter, and is next. +- **The window uses far less memory, and rebuilding a screen or opening a + list no longer adds to it.** Every quiet line on a screen - a subtitle, a + caption, the count of bytes beside a size, the line a folded section keeps, + the message under a field - parsed its own copy of the fonts, and parsed + another each time the screen was rebuilt and the line said something new. + The open list of formats did the same every time it opened, and more with + every letter typed into its filter - ten openings kept about 160 MB. After + visiting the four tabs the window held about 290 MB and it now holds about + 120 MB. Nothing on the screen looks different. - **The `Several batches` screen no longer slows down the longer it is used.** Every batch added, removed or copied, every format chosen and every diff --git a/internal/guard/formatlist_test.go b/internal/guard/formatlist_test.go index d2ffcb05..620b5bf7 100644 --- a/internal/guard/formatlist_test.go +++ b/internal/guard/formatlist_test.go @@ -235,6 +235,110 @@ func TestTheArrowsInTheFormatListStepOverTheHeadings(t *testing.T) { } } +// TestTheRowTheKeyboardIsOnIsAlwaysInSight walks the keyboard through a list +// taller than the room it was given and asks, after every key, whether the +// row it stands on is drawn whole inside that room - and, on the first value +// of a kind, whether the heading over it is too, since arrowing up to the top +// of a kind is meant to show what kind it was (OpenList.moveTo). +// +// Written 2026-09-23 before the rows left widget.List, because the scrolling +// had no guard of its own: no stored screen scrolls the list, so a list that +// kept the keyboard on a row out of sight would have passed every picture. +// Read off the canvas, row against list, so it asks what a person sees and +// not what the list says it did. +func TestTheRowTheKeyboardIsOnIsAlwaysInSight(t *testing.T) { + _, _, list, _ := openFormatList(t) + entries := list.Rows() + room := list.Size().Height - list.HeadHeight() + if room >= float32(len(entries))*parts.ListRowHeight() { + t.Fatalf("the list has %.0f px for %d rows of %.0f, so it never scrolls and nothing here is asked", + room, len(entries), parts.ListRowHeight()) + } + drv := fyne.CurrentApp().Driver() + inSight := func(label string) bool { + row := list.RowShowing(label) + if row == nil { + return false + } + top := drv.AbsolutePositionForObject(list).Y + list.HeadHeight() + at := drv.AbsolutePositionForObject(row).Y + return at >= top-0.5 && at+row.Size().Height <= top+room+0.5 + } + check := func(key fyne.KeyName) { + t.Helper() + list.TypedKey(&fyne.KeyEvent{Name: key}) + at := list.Active() + if at < 0 || at >= len(entries) { + t.Fatalf("after %s the keyboard stands on no row", key) + } + if !inSight(entries[at].Label) { + t.Fatalf("after %s the keyboard is on %q, which is not drawn whole inside the list", key, entries[at].Label) + } + if at > 0 && !entries[at-1].Choosable && !inSight(entries[at-1].Label) { + t.Fatalf("after %s the keyboard is on %q, the first of its kind, and the heading %q over it is out of sight", + key, entries[at].Label, entries[at-1].Label) + } + } + + check(fyne.KeyEnd) + check(fyne.KeyHome) + values := 0 + for _, e := range entries { + if e.Choosable { + values++ + } + } + for i := 1; i < values; i++ { + check(fyne.KeyDown) + } + if last := entries[len(entries)-1].Label; activeLabel(list) != last { + t.Fatalf("Down %d times from the first value ended on %q, not on the last one, %s", values-1, activeLabel(list), last) + } + for i := 1; i < values; i++ { + check(fyne.KeyUp) + } +} + +// TestEmptyingTheFilterOfAListWithNothingChosenDrawsEveryRowAgain types a +// filter that keeps nothing into a list whose box holds no value yet, empties +// it, and asks what is drawn. +// +// An emptied filter puts the keyboard back on the value in the box - and with +// no value in the box that found nothing, and nothing redrew the rows either: +// the list went on drawing the one row saying nothing matched while it held +// every format again. Found on 2026-09-23 reading the four states of the rows +// leaving widget.List, and red on widget.List as well. A box holding no value +// is real: a chooser starts that way (the catalogue has one of the formats). +func TestEmptyingTheFilterOfAListWithNothingChosenDrawsEveryRowAgain(t *testing.T) { + app := test.NewApp() + app.Settings().SetTheme(parts.Theme()) + t.Cleanup(func() { test.NewApp() }) + + list := parts.NewOpenList(format.IDs(), "", func(string, bool) {}, func(bool) {}) + list.KindOf = parts.KindOfFile + list.GroupUnder(parts.KindHeading) + list.WithFilter() + w := test.NewWindow(list) + t.Cleanup(w.Close) + w.Resize(fyne.NewSize(300, 600)) + + typeInto(list.Filter(), "zz") + if rows := list.DrawnRows(); len(rows) != 1 || !rows[0].Heading() { + t.Fatalf("zz was typed and %d row(s) are drawn, not the one saying nothing matches - the state this asks about was not reached", len(rows)) + } + list.Filter().SetText("") + first := "" + for _, row := range list.Rows() { + if row.Choosable { + first = row.Label + break + } + } + if list.RowShowing(first) == nil { + t.Errorf("the filter was emptied and the list draws %d row(s), none of them %s, its first value", len(list.DrawnRows()), first) + } +} + // TestTypingAtTheShutFormatMenuOpensItsFilter types a whole name at the menu // with its list shut. One letter at a time used to walk the values starting // with each letter in turn, so "jxl" ended on log. Now the first letter opens diff --git a/internal/guard/testdata/screens/catalogue.png b/internal/guard/testdata/screens/catalogue.png index 93a3beff..4ddc0deb 100644 Binary files a/internal/guard/testdata/screens/catalogue.png and b/internal/guard/testdata/screens/catalogue.png differ diff --git a/internal/guard/testdata/screens/catalogue.xml b/internal/guard/testdata/screens/catalogue.xml index 38fc6834..048e0afa 100644 --- a/internal/guard/testdata/screens/catalogue.xml +++ b/internal/guard/testdata/screens/catalogue.xml @@ -877,41 +877,20 @@ - + - - - - - - - png - - - - - - - jpg - - - - - - avif - - - - - - - - - - - - - + + + png + + + + + jpg + + + + avif @@ -933,40 +912,19 @@ - + - - - - - - - png - - - - - - jpg - - - - - - avif - - - - - - - - - - - - - + + + png + + + + jpg + + + + avif @@ -988,68 +946,66 @@ - - - - - - - - - avif - - - - - - bmp - - - - - - csv - - - - - - docx - - - - - - gif - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + avif + + + + bmp + + + + csv + + + + docx + + + + gif + + + + html + + + + ico + + + + jpg + + + + + json + + + + jxl + + + + log + + + + md + + + + + + + + @@ -1069,134 +1025,68 @@ - + - - - - - - - - avif - - - - - - - bmp - - - - - - - csv - - - - - - - - docx - - - - - - - gif - - - - - - - html - - - - - - - ico - - - - - - - jpg - - - - - - - json - - - - - - - jxl - - - - - - - log - - - - - - - md - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + avif + + + + + bmp + + + + + csv + + + + + + docx + + + + + gif + + + + + html + + + + + ico + + + + + jpg + + + + + json + + + + + jxl + + + + + log + + + + + md @@ -1218,32 +1108,16 @@ - + - - - - - - - Write a label inside each generated file, including the ones that are far too small to hold it - - - - - - - png - - - - - - - - - - + + + Write a label inside each generated file, including the ones that are far too small to hold it + + + + + png @@ -1266,130 +1140,168 @@ - - - - - - - - - Archives · 2 - - - - - - - targz - - - - - - - zip - - - - - - Documents · 4 - - - - - - - docx - - - - - - - pdf - - - - - - - pptx - - - - - - - xlsx - - - - - - Pictures · 10 - - - - - - - avif - - - - - - - bmp - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + Archives · 2 + + + + + targz + + + + + zip + + + + Documents · 4 + + + + + docx + + + + + pdf + + + + + pptx + + + + + xlsx + + + + Pictures · 10 + + + + + avif + + + + + bmp + + + + + gif + + + + + ico + + + + + jpg + + + + + jxl + + + + + + png + + + + + svg + + + + + tiff + + + + + webp + + + + Sound · 1 + + + + + wav + + + + Text and data · 9 + + + + + csv + + + + + html + + + + + json + + + + + log + + + + + md + + + + + toml + + + + + txt + + + + + xml + + + + + yaml + + + + + + + + @@ -1427,144 +1339,109 @@ - - - - - - - - - - - p - ptx - - - - - - Pictures · 10 - - - - - - - avif - - - - - - - bm - p - - - - - - - - gif - - - - - - - ico - - - - - - - j - p - g - - - - - - - jxl - - - - - - - - - p - ng - - - - - - - svg - - - - - - - tiff - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + Archives · 1 + + + + + zi + p + + + + + Documents · 2 + + + + + + p + df + + + + + + p + ptx + + + + Pictures · 10 + + + + + avif + + + + + bm + p + + + + + + gif + + + + + ico + + + + + j + p + g + + + + + jxl + + + + + + + p + ng + + + + + svg + + + + + tiff + + + + + web + p + + + + + + + + + @@ -1599,19 +1476,11 @@ - + - - - - - - - Nothing matches - - - - + + + Nothing matches diff --git a/internal/guard/testdata/screens/generate-menu-hovered.xml b/internal/guard/testdata/screens/generate-menu-hovered.xml index d4d70396..6e557f0f 100644 --- a/internal/guard/testdata/screens/generate-menu-hovered.xml +++ b/internal/guard/testdata/screens/generate-menu-hovered.xml @@ -489,239 +489,168 @@ - - - - - - - - - Archives · 2 - - - - - - - targz - - - - - - - zip - - - - - - Documents · 4 - - - - - - - docx - - - - - - - pdf - - - - - - - pptx - - - - - - - xlsx - - - - - - Pictures · 10 - - - - - - - - avif - - - - - - - bmp - - - - - - - gif - - - - - - - ico - - - - - - - jpg - - - - - - - jxl - - - - - - - png - - - - - - - svg - - - - - - - tiff - - - - - - - webp - - - - - - Sound · 1 - - - - - - - wav - - - - - - Text and data · 9 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + Archives · 2 + + + + + targz + + + + + zip + + + + Documents · 4 + + + + + docx + + + + + pdf + + + + + pptx + + + + + xlsx + + + + Pictures · 10 + + + + + + avif + + + + + bmp + + + + + gif + + + + + ico + + + + + jpg + + + + + jxl + + + + + png + + + + + svg + + + + + tiff + + + + + webp + + + + Sound · 1 + + + + + wav + + + + Text and data · 9 + + + + + csv + + + + + html + + + + + json + + + + + log + + + + + md + + + + + toml + + + + + txt + + + + + xml + + + + + yaml + + + + + + + + diff --git a/internal/guard/testdata/screens/generate-menu-keyed.xml b/internal/guard/testdata/screens/generate-menu-keyed.xml index ce19722f..aaed0638 100644 --- a/internal/guard/testdata/screens/generate-menu-keyed.xml +++ b/internal/guard/testdata/screens/generate-menu-keyed.xml @@ -489,239 +489,168 @@ - - - - - - - - - Archives · 2 - - - - - - - targz - - - - - - - zip - - - - - - Documents · 4 - - - - - - - docx - - - - - - - pdf - - - - - - - pptx - - - - - - - xlsx - - - - - - Pictures · 10 - - - - - - - - avif - - - - - - - bmp - - - - - - - gif - - - - - - - ico - - - - - - - jpg - - - - - - - jxl - - - - - - - png - - - - - - - svg - - - - - - - tiff - - - - - - - webp - - - - - - Sound · 1 - - - - - - - wav - - - - - - Text and data · 9 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + Archives · 2 + + + + + targz + + + + + zip + + + + Documents · 4 + + + + + docx + + + + + pdf + + + + + pptx + + + + + xlsx + + + + Pictures · 10 + + + + + + avif + + + + + bmp + + + + + gif + + + + + ico + + + + + jpg + + + + + jxl + + + + + png + + + + + svg + + + + + tiff + + + + + webp + + + + Sound · 1 + + + + + wav + + + + Text and data · 9 + + + + + csv + + + + + html + + + + + json + + + + + log + + + + + md + + + + + toml + + + + + txt + + + + + xml + + + + + yaml + + + + + + + + diff --git a/internal/guard/testdata/screens/generate-menu.xml b/internal/guard/testdata/screens/generate-menu.xml index 4ba61bdf..d91504e9 100644 --- a/internal/guard/testdata/screens/generate-menu.xml +++ b/internal/guard/testdata/screens/generate-menu.xml @@ -489,239 +489,168 @@ - - - - - - - - - Archives · 2 - - - - - - - targz - - - - - - - zip - - - - - - Documents · 4 - - - - - - - docx - - - - - - - pdf - - - - - - - pptx - - - - - - - xlsx - - - - - - Pictures · 10 - - - - - - - - avif - - - - - - - bmp - - - - - - - gif - - - - - - - ico - - - - - - - jpg - - - - - - - jxl - - - - - - - png - - - - - - - svg - - - - - - - tiff - - - - - - - webp - - - - - - Sound · 1 - - - - - - - wav - - - - - - Text and data · 9 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + Archives · 2 + + + + + targz + + + + + zip + + + + Documents · 4 + + + + + docx + + + + + pdf + + + + + pptx + + + + + xlsx + + + + Pictures · 10 + + + + + + avif + + + + + bmp + + + + + gif + + + + + ico + + + + + jpg + + + + + jxl + + + + + png + + + + + svg + + + + + tiff + + + + + webp + + + + Sound · 1 + + + + + wav + + + + Text and data · 9 + + + + + csv + + + + + html + + + + + json + + + + + log + + + + + md + + + + + toml + + + + + txt + + + + + xml + + + + + yaml + + + + + + + + diff --git a/internal/guard/testdata/screens/preset-menu-setting.xml b/internal/guard/testdata/screens/preset-menu-setting.xml index 4c6533bb..3dc1f4c5 100644 --- a/internal/guard/testdata/screens/preset-menu-setting.xml +++ b/internal/guard/testdata/screens/preset-menu-setting.xml @@ -452,239 +452,168 @@ - - - - - - - - - Archives · 2 - - - - - - - targz - - - - - - - zip - - - - - - Documents · 4 - - - - - - - docx - - - - - - - - pdf - - - - - - - pptx - - - - - - - xlsx - - - - - - Pictures · 10 - - - - - - - avif - - - - - - - bmp - - - - - - - gif - - - - - - - ico - - - - - - - jpg - - - - - - - jxl - - - - - - - png - - - - - - - svg - - - - - - - tiff - - - - - - - webp - - - - - - Sound · 1 - - - - - - - wav - - - - - - Text and data · 9 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + Archives · 2 + + + + + targz + + + + + zip + + + + Documents · 4 + + + + + docx + + + + + + pdf + + + + + pptx + + + + + xlsx + + + + Pictures · 10 + + + + + avif + + + + + bmp + + + + + gif + + + + + ico + + + + + jpg + + + + + jxl + + + + + png + + + + + svg + + + + + tiff + + + + + webp + + + + Sound · 1 + + + + + wav + + + + Text and data · 9 + + + + + csv + + + + + html + + + + + json + + + + + log + + + + + md + + + + + toml + + + + + txt + + + + + xml + + + + + yaml + + + + + + + + diff --git a/internal/guard/testdata/screens/preset-menu.xml b/internal/guard/testdata/screens/preset-menu.xml index e658e2bf..af811acc 100644 --- a/internal/guard/testdata/screens/preset-menu.xml +++ b/internal/guard/testdata/screens/preset-menu.xml @@ -395,59 +395,28 @@ - + - - - - - - - - empty-and-minimal - - - - - - size-boundaries - - - - - - tabular-import - - - - - - text-encoding - - - - - - upload-validation - - - - - - - - - - - - - - - - - - - + + + + empty-and-minimal + + + + size-boundaries + + + + tabular-import + + + + text-encoding + + + + upload-validation diff --git a/internal/guard/themescope_test.go b/internal/guard/themescope_test.go index a48c840a..184255e2 100644 --- a/internal/guard/themescope_test.go +++ b/internal/guard/themescope_test.go @@ -24,10 +24,12 @@ import ( // area, caption and count of bytes was under one. Without them: 2 sets, 27 MB, // and nothing added by the rebuilds. // -// The open list of formats is not on this screen - it is a popup - and it -// still draws its rows under one (parts/openlist.go, rowTheme). That is the -// next thing the same document names, and it is named here so that nobody -// reads this guard as covering it. +// The open list of formats is asked too, since its rows left one on +// 2026-09-23 (section 4e): every opening was a new override and the letters +// its filter makes bold were new strings, so ten openings kept 158 MB. It is +// a popup rather than part of a screen, and it keeps its rows inside its +// renderer, where the walk of the screens does not go - so it is opened, typed +// into, and walked through what it draws. func TestNoScreenStandsInAThemeOverride(t *testing.T) { content, _ := laidOutWindow(t) @@ -51,9 +53,21 @@ func TestNoScreenStandsInAThemeOverride(t *testing.T) { cat := test.NewWindow(catalogue.Screen()) t.Cleanup(cat.Close) - for name, root := range map[string]fyne.CanvasObject{"the window": content, "the catalogue": cat.Content()} { + _, _, list, filter := openFormatList(t) + typeInto(filter, "p") + rows := 0 + walkDrawn(list, func(o fyne.CanvasObject) { + if _, is := o.(*parts.ListRow); is { + rows++ + } + }) + if rows == 0 { + t.Fatal("the open list of formats draws no row, so its rows were not looked at") + } + + for name, root := range map[string]fyne.CanvasObject{"the window": content, "the catalogue": cat.Content(), "the open list of formats": list} { overrides := 0 - walk(root, func(o fyne.CanvasObject) { + walkDrawn(root, func(o fyne.CanvasObject) { if _, is := o.(*container.ThemeOverride); is { overrides++ } @@ -64,3 +78,24 @@ func TestNoScreenStandsInAThemeOverride(t *testing.T) { } } } + +// walkDrawn visits everything a tree draws: into a container's objects and into +// every widget's renderer, which is where a widget keeps what it built - the +// rows of an open list among them. walk stops at a widget it has no case for, +// and that is the half this guard needs. +func walkDrawn(o fyne.CanvasObject, visit func(fyne.CanvasObject)) { + if o == nil { + return + } + visit(o) + switch v := o.(type) { + case *fyne.Container: + for _, child := range v.Objects { + walkDrawn(child, visit) + } + case fyne.Widget: + for _, child := range test.WidgetRenderer(v).Objects() { + walkDrawn(child, visit) + } + } +} diff --git a/internal/gui/catalogue/lists.go b/internal/gui/catalogue/lists.go index 0f9b6d87..a0df3000 100644 --- a/internal/gui/catalogue/lists.go +++ b/internal/gui/catalogue/lists.go @@ -20,12 +20,12 @@ import ( // list about a short window. // // Nor has it a width of its own. On a form the list is as wide as the box it -// drops from, and the toolkit's list measures its width off an empty template -// row, so a list stood here bare was a 42 px strip with the first letter of -// each value on it - seen on the render of 2026-09-15, and accepted with the -// rest of the catalogue before anybody read it at that height. Each state -// is drawn as wide as the box a menu of the same values would be, which is -// the rule the form follows. +// drops from, and on its own it is as wide as a row with no words, so a list +// stood here bare was a 42 px strip with the first letter of each value on +// it - seen on the render of 2026-09-15, and accepted with the rest of the +// catalogue before anybody read it at that height. Each state is drawn as +// wide as the box a menu of the same values would be, which is the rule the +// form follows. func openList() Entry { few := []string{"png", "jpg", "avif"} many := []string{"avif", "bmp", "csv", "docx", "gif", "html", "ico", "jpg", "json", "jxl", "log", "md"} diff --git a/internal/gui/parts/listcontents.go b/internal/gui/parts/listcontents.go index d874e5d7..423f1b7f 100644 --- a/internal/gui/parts/listcontents.go +++ b/internal/gui/parts/listcontents.go @@ -1,12 +1,8 @@ package parts -import ( - "fyne.io/fyne/v2/widget" -) - // listContents is what an open list holds and what it has drawn: the values, // how they are grouped and narrowed, the rows that arrangement comes to, and -// the rows the toolkit has actually built for it. +// the rows on the screen showing it. // // Its own type rather than more of OpenList, since the list of 2026-09-23 // took headings and a filter. OpenList stood at 24 methods, the fourth type in @@ -33,25 +29,21 @@ type listContents struct { // the moment a list has a heading. entries []listEntry - // rows is the row showing each position, recorded as the list fills them. - // - // A registry rather than a walk, because a walk cannot get in: widget.List - // keeps the rows it built inside its renderer, so a tree walk stops at the - // list and reports an open list with nothing in it. Measured on 2026-08-18 - // while trying to photograph a row under the pointer. - // - // Every entry is current whatever the list has scrolled past, because a - // recycled row is refilled before it is shown and fill is what writes here. - rows map[widget.ListItemID]*ListRow + // view is the rows on the screen. Asked rather than walked, because a + // walk cannot get in: the rows are inside the list's renderer, so a tree + // walk stops at the list and reports an open list with nothing in it. + // Measured on 2026-08-18 while trying to photograph a row under the + // pointer, when the rows were widget.List's. + view *rowView } // rearrange works out the rows again after the filter or the grouping -// changed, and forgets the rows it recorded: a row built for the old -// arrangement can still hold a label the list no longer draws, and -// RowShowing would report it. +// changed, and forgets the rows on the screen until they are filled again: a +// row filled for the old arrangement can still hold a label the list no +// longer draws, and RowShowing would report it. func (c *listContents) rearrange() { c.entries = arrange(c.options, c.headingOf, c.typed) - c.rows = map[widget.ListItemID]*ListRow{} + c.view.shown = 0 } // Rows is what this list is showing, for a guard to read, in the order it is @@ -80,16 +72,18 @@ func (c *listContents) Rows() []Choice { // copy. A rule with two homes is a rule no test can pin down. func (c *listContents) isChosen(value string) bool { return value == c.chosen } -// DrawnRows is every row the list has actually built, for a guard that has to -// ask what is on the screen rather than what the list holds. +// DrawnRows is every row in sight, for a guard that has to ask what is on the +// screen rather than what the list holds. // // Rows above answers from the options, which is the right half for "what is in // this list" and the wrong half for "what does a row draw". A picture that // never reaches a row would pass the first and fail this one. func (c *listContents) DrawnRows() []*ListRow { - out := make([]*ListRow, 0, len(c.rows)) - for _, row := range c.rows { - out = append(out, row) + var out []*ListRow + for at, row := range c.view.built[:c.view.shown] { + if c.view.inSight(at) { + out = append(out, row) + } } return out } @@ -97,7 +91,7 @@ func (c *listContents) DrawnRows() []*ListRow { // RowShowing is the row currently drawing one value, or nil if that value is // scrolled out of sight. For a guard that needs to press or hover a real row. func (c *listContents) RowShowing(label string) *ListRow { - for _, row := range c.rows { + for _, row := range c.DrawnRows() { if row.Label() == label { return row } diff --git a/internal/gui/parts/listrow.go b/internal/gui/parts/listrow.go index ee31ca58..15ef90ca 100644 --- a/internal/gui/parts/listrow.go +++ b/internal/gui/parts/listrow.go @@ -17,11 +17,12 @@ import ( // and draw its own surface for the three states a row has - plain, under the // pointer, and holding the keyboard. // -// It is built once per visible row and refilled as the list scrolls, which is -// what makes the list cost the same at thirteen values and at a hundred -// thousand. Measured in tools/probes/fynelist on 2026-08-18: widget.List is -// flat at 0.1 MB across 1000, 10 000 and 100 000 rows, where a box holding -// every row costs 449 MB at the largest. +// It is built once per position of the list and refilled when what the list +// holds changes. Once per VISIBLE row until 2026-09-23, inside widget.List, +// which is what made a list cost the same at thirteen values and at a hundred +// thousand - measured in tools/probes/fynelist on 2026-08-18, flat at 0.1 MB +// where a box holding every row costs 449 MB at the largest. Why every +// position now, and when that stops being right, is in rowView. type ListRow struct { widget.BaseWidget diff --git a/internal/gui/parts/openlist.go b/internal/gui/parts/openlist.go index 934fedb4..5cdad041 100644 --- a/internal/gui/parts/openlist.go +++ b/internal/gui/parts/openlist.go @@ -1,7 +1,6 @@ package parts import ( - "image/color" "math" "strings" @@ -61,8 +60,6 @@ type OpenList struct { take func(value string, byKeyboard bool) close func(byKeyboard bool) - list *widget.List - // KindOf says what picture goes in front of one value, or nil for a list // whose values are not things of different kinds. Set from outside, because // only the screen putting values in knows what they are. @@ -77,13 +74,8 @@ type OpenList struct { // on, and close when they left without settling on one. func NewOpenList(options []string, chosen string, take func(string, bool), close func(bool)) *OpenList { l := &OpenList{active: -1, take: take, close: close, - listContents: listContents{options: options, chosen: chosen, rows: map[widget.ListItemID]*ListRow{}}} + listContents: listContents{options: options, chosen: chosen, view: newRowView()}} l.entries = arrange(options, nil, "") - l.list = widget.NewList( - func() int { return len(l.entries) }, - func() fyne.CanvasObject { return newListRow() }, - l.fill, - ) l.ExtendBaseWidget(l) return l } @@ -125,12 +117,7 @@ func (l *OpenList) Filter() *FilterBox { return l.filter } func (l *OpenList) narrowTo(typed string) { l.typed = typed l.rearrange() - // ScrollToOffset rather than ScrollToTop: the toolkit's ScrollToTop reaches - // for the list's scroller without asking whether it exists yet, and it does - // not until the list is first drawn - fyne v2.8.1 widget/list.go, line 358 - // against 366. A filter set before that (the catalogue does) took the - // process down. - l.list.ScrollToOffset(0) + l.view.toTop() if strings.TrimSpace(typed) == "" { l.active = -1 l.StartOn(l.chosen) @@ -141,18 +128,15 @@ func (l *OpenList) narrowTo(typed string) { return } l.active = -1 - l.list.Refresh() + l.view.show(len(l.entries), l.fill) } // fill puts one row of the arrangement into one row of the list. The row it is -// given is recycled, so every field is set every time - a row left holding the -// last value it had is the classic defect of a list that only builds what it -// can see. -func (l *OpenList) fill(id widget.ListItemID, row fyne.CanvasObject) { - r, ok := row.(*ListRow) - if !ok || id < 0 || id >= len(l.entries) { - return - } +// given is recycled - the same row shows whatever stands at its position in +// the arrangement of the moment - so every field is set every time: a row left +// holding the last value it had is the classic defect of a list that reuses +// its rows. It is drawn by whoever asked for the fill. +func (l *OpenList) fill(id int, r *ListRow) { entry := l.entries[id] r.label = entry.text r.heading = entry.kind != entryValue @@ -171,8 +155,6 @@ func (l *OpenList) fill(id widget.ListItemID, row fyne.CanvasObject) { r.active = l.shown && id == l.active r.onTap = func() { l.take(value, false) } } - l.rows[id] = r - r.Refresh() } // Choice is one row of an open list, for a guard to read. Choosable is false @@ -217,7 +199,7 @@ func (l *OpenList) MinSize() fyne.Size { if height < head+listRowHeight() { height = head + listRowHeight() } - return fyne.NewSize(l.list.MinSize().Width, height) + return fyne.NewSize(l.view.scroll.MinSize().Width, height) } // HeadHeight is the room the filter box takes at the top of the list, with @@ -252,10 +234,12 @@ func ListCeiling(canvasHeight float32) float32 { } func (l *OpenList) CreateRenderer() fyne.WidgetRenderer { + l.view.drawn = true + l.view.show(len(l.entries), l.fill) // The surface is drawn here rather than left to the popup, so that the // colour a guard measures for "an open list is told from the form behind // it" is the colour actually on the screen. - rows := container.NewThemeOverride(l.list, rowTheme{}) + rows := l.view.scroll if l.filter == nil { return widget.NewSimpleRenderer(container.NewStack(floatingSurface(), rows)) } @@ -263,52 +247,19 @@ func (l *OpenList) CreateRenderer() fyne.WidgetRenderer { return widget.NewSimpleRenderer(container.NewStack(floatingSurface(), container.NewBorder(head, nil, nil, nil, rows))) } -// rowTheme is our theme with the room between rows taken out. -// -// It exists because a sentence written in theme.go on 2026-08-12 was wrong, -// and the way it was wrong is the expensive kind. That note said the theme is -// asked for a size by NAME and not by widget, so "it is the only knob there -// is" - and the first half is true while the conclusion is not. A theme can be -// replaced for a SUBTREE with container.NewThemeOverride, which nobody had -// looked for, so a list was tightened by moving the padding of the entire form. -// -// Measured: widget.List spaces its rows by theme.SizeNamePadding, called -// separatorThickness in list.go - the same 6 px the form is built from. With -// the override the rows sit against each other and the row height is the whole -// of the pitch. -type rowTheme struct{} - -func (rowTheme) Color(n fyne.ThemeColorName, v fyne.ThemeVariant) color.Color { - return Theme().Color(n, v) -} -func (rowTheme) Font(s fyne.TextStyle) fyne.Resource { return Theme().Font(s) } -func (rowTheme) Icon(n fyne.ThemeIconName) fyne.Resource { return Theme().Icon(n) } -func (rowTheme) Size(n fyne.ThemeSizeName) float32 { - switch n { - case theme.SizeNamePadding: - // Rows sit against each other. What separates them is the surface a - // row draws when the pointer or the keyboard is on it, which is a - // thing somebody can see, rather than a gap, which is not. - return 0 - case theme.SizeNameSeparatorThickness: - // And no rule between them either. widget.List draws a hairline - // between rows, which the room between them used to hide - with the - // room gone it came out as a line every 28 px and the list read as a - // ruled table rather than as a menu. Seen on the render, which is the - // only place it could have been seen: the tree says a separator is - // present either way. - return 0 - } - return Theme().Size(n) -} - // ListRowHeight is one row, for a guard asking how many rows fit in a height. func ListRowHeight() float32 { return listRowHeight() } // listRowHeight is one row, worked out rather than typed: whatever is taller // out of the text and the mark, plus our own room above and below. +// +// The text measured rather than a label built and asked, since 2026-09-23: +// every row asks this whenever it is sized, and a label built each time was +// about 2 MB an opening that stayed for the minute the toolkit keeps a +// renderer (docs/GUI-MEMORY-2026-09-23.md section 2.5). The same number - a +// label's height less its inner padding is its text's. func listRowHeight() float32 { - text := widget.NewLabel("Ag").MinSize().Height - 2*Theme().Size(theme.SizeNameInnerPadding) + text := fyne.MeasureText("Ag", Theme().Size(theme.SizeNameText), fyne.TextStyle{}).Height icon := Theme().Size(theme.SizeNameInlineIcon) tall := text if icon > tall { @@ -410,10 +361,10 @@ func (l *OpenList) moveTo(at int) { l.active = at l.shown = true if at > 0 && l.entries[at-1].kind == entryHeading { - l.list.ScrollTo(at - 1) + l.view.bringIntoView(at - 1) } - l.list.ScrollTo(at) - l.list.Refresh() + l.view.bringIntoView(at) + l.view.show(len(l.entries), l.fill) } // Active is the row the keyboard is on, or -1, for a guard. @@ -427,10 +378,13 @@ func (l *OpenList) StartOn(value string) { if e.kind == entryValue && e.text == value { l.moveTo(i) l.shown = false - l.list.Refresh() - return + break } } + // Drawn whether or not the value was found. A box holding no value finds + // nothing, and an emptied filter lands here with the rows of what it had + // narrowed to still drawn. + l.view.show(len(l.entries), l.fill) } // Showing says whether the keyboard position is drawn, for a guard. diff --git a/internal/gui/parts/rowview.go b/internal/gui/parts/rowview.go new file mode 100644 index 00000000..3c6d6b3a --- /dev/null +++ b/internal/gui/parts/rowview.go @@ -0,0 +1,126 @@ +package parts + +import ( + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/container" +) + +// rowView is the rows of an open list on the screen: one ListRow for every +// position of the arrangement, against each other in a scroll. +// +// Ours since 2026-09-23. It was widget.List until then, under a theme +// override that took out the room the toolkit puts between rows - and Fyne +// 2.8.1 gives an override a new scope at construction, at CreateRenderer and +// at every Refresh, and parses the fonts again for every scope a new string is +// drawn in. Every opening of a list was a new override, and the letters a +// filter makes bold are new strings: ten openings of the format list with a +// new letter each kept 158 MB that nothing gave back, measured in the real +// window (docs/GUI-MEMORY-2026-09-23.md section 4e). Rows we lay out ourselves +// are spaced the way we say, and no override is needed. +// +// Every position gets a row, where widget.List built only the rows in sight. +// The longest list in the window is the formats under their headings, about +// thirty rows, and building them is not what an opening costs (section 4f of +// the same document). A menu of hundreds of values is where that stops being +// true, and the answer then is to build only the rows in sight. +type rowView struct { + scroll *container.Scroll + stack *fyne.Container + // built is every row made so far, kept for the next arrangement, and + // shown how many of them the current one uses. + built []*ListRow + shown int + // drawn says the list has a renderer. Rows are built only from then on, + // as widget.List built them. + drawn bool +} + +func newRowView() *rowView { + v := &rowView{stack: container.New(rowStack{})} + v.scroll = container.NewVScroll(v.stack) + return v +} + +// show puts a row on the screen for each of n positions, filled by fill, and +// draws them once. +func (v *rowView) show(n int, fill func(at int, row *ListRow)) { + if !v.drawn { + return + } + for len(v.built) < n { + v.built = append(v.built, newListRow()) + } + objects := make([]fyne.CanvasObject, n) + for at, row := range v.built[:n] { + fill(at, row) + objects[at] = row + } + v.stack.Objects = objects + v.shown = n + // The scroll first takes the new height of the rows, then refreshes them. + v.scroll.Refresh() +} + +// bringIntoView scrolls the least that shows the row at this position whole: +// up to it when it is above what is in sight, and until it is the last row in +// sight when it is below. widget.List's scrollTo, with no room between rows. +// Takes effect at the next show. +// +// Nothing happens before the list has a height, and that is a difference +// from widget.List. A list filtered before it is laid out - the catalogue +// does that - came up from widget.List 124 px down, with the row the keyboard +// was on above what was in sight (the stored catalogue tree until 2026-09-23). +// Here it comes up at its top, which is where typing puts a list on the +// screen. +func (v *rowView) bringIntoView(at int) { + if v.scroll.Size().Height <= 0 { + return + } + row := listRowHeight() + top := float32(at) * row + switch { + case top < v.scroll.Offset.Y: + v.scroll.Offset.Y = top + case top+row > v.scroll.Offset.Y+v.scroll.Size().Height: + v.scroll.Offset.Y = top + row - v.scroll.Size().Height + } +} + +// toTop scrolls back to the first row. Takes effect at the next show. +func (v *rowView) toTop() { v.scroll.Offset.Y = 0 } + +// inSight says whether any of the row at this position is inside the room the +// list has, which is what widget.List built a row for. +func (v *rowView) inSight(at int) bool { + row := listRowHeight() + top := float32(at) * row + return top+row > v.scroll.Offset.Y && top < v.scroll.Offset.Y+v.scroll.Size().Height +} + +// rowStack puts the rows of an open list against each other: each one row +// tall and as wide as the list, in the order given. +// +// Against each other is the point. What separates two rows is the surface a +// row draws under the pointer or the keyboard, which is a thing somebody can +// see, rather than a gap, which is not - and no rule between them either: +// widget.List drew a hairline in its gap, and with the gap taken out it came +// out as a line every 28 px, so the list read as a ruled table rather than as +// a menu. Measured before any of this existed: thirteen formats made a list +// 476 px tall, 78 px of which was the gap between rows (OpenList). +type rowStack struct{} + +// MinSize is every row, as wide as a row with no words - which is what +// widget.List measured its width off, an empty template row. The list is +// drawn at the width of the box it drops from, and the box is sized for the +// longest word (RowWidthFor). +func (rowStack) MinSize(objects []fyne.CanvasObject) fyne.Size { + return fyne.NewSize(RowWidthFor(0, false), float32(len(objects))*listRowHeight()) +} + +func (rowStack) Layout(objects []fyne.CanvasObject, size fyne.Size) { + row := listRowHeight() + for at, o := range objects { + o.Move(fyne.NewPos(0, float32(at)*row)) + o.Resize(fyne.NewSize(size.Width, row)) + } +}