Stage cover candidates in the background instead of on the UI goroutine
Decoding, rescaling and re-encoding a cover takes around 270ms for a 2000x2000 image, and it happened on the UI goroutine the first time each candidate was looked at. Moving through a candidate list stalled the interface once per candidate. Candidates are now staged ahead of being asked for, on a background goroutine, as soon as the list is known. A render usually finds its image already waiting and does no work of its own. Only the first candidate, shown the moment the picker opens, can still be staged on demand. stageTempImage writes to a unique temporary name and renames it into place, so a background warm and a foreground render staging the same source cannot interleave their writes into one file. The test calibrates itself against the machine by timing a single staging operation and requiring Warm to return faster than that, so it fails if the work is ever moved back onto the caller's goroutine rather than passing for being fast enough on a quick machine. Closes 11d30d809904bb1161633353c52fdce4451e2ff6
diff --git a/internal/tui/kitty.go b/internal/tui/kitty.go
@@ -181,12 +181,76 @@ func stageTempImage(dir, src string) string {
// reads the file by path and never deletes it for the file medium.
sum := sha256.Sum256([]byte(src))
dst := filepath.Join(dir, fmt.Sprintf("cover-%x.png", sum[:8]))
- if err := os.WriteFile(dst, payload, 0o600); err != nil {
+
+ // Written to a unique temporary name and renamed into place, so that a
+ // background warm and a foreground render staging the same source cannot
+ // interleave their writes into one file.
+ f, err := os.CreateTemp(dir, "staging-*.png")
+ if err != nil {
+ return ""
+ }
+ if _, err := f.Write(payload); err != nil {
+ f.Close()
+ os.Remove(f.Name())
+ return ""
+ }
+ if err := f.Close(); err != nil {
+ os.Remove(f.Name())
+ return ""
+ }
+ if err := os.Rename(f.Name(), dst); err != nil {
+ os.Remove(f.Name())
return ""
}
return dst
}
+// Warm stages images ahead of them being asked for, on a background goroutine.
+//
+// Staging decodes, rescales and re-encodes, which takes around 270ms for a
+// 2000x2000 cover. Doing that on the UI goroutine stalls the whole interface
+// every time the cursor moves to a candidate that has not been looked at yet, so
+// the candidates are prepared in the background as soon as they are known and a
+// render usually finds its image already waiting.
+func (p *ImagePreviewer) Warm(paths []string) {
+ if p.mode != imageKitty || forceChars {
+ return
+ }
+ p.mu.Lock()
+ if p.staged == nil {
+ p.staged = map[string]string{}
+ }
+ dir := p.stageDir()
+ var todo []string
+ for _, path := range paths {
+ if path == "" {
+ continue
+ }
+ if staged, ok := p.staged[path]; ok && staged != "" {
+ continue
+ }
+ todo = append(todo, path)
+ }
+ p.mu.Unlock()
+
+ if dir == "" || len(todo) == 0 {
+ return
+ }
+ go func() {
+ for _, path := range todo {
+ staged := stageTempImage(dir, path)
+ if staged == "" {
+ continue
+ }
+ p.mu.Lock()
+ if _, exists := p.staged[path]; !exists {
+ p.staged[path] = staged
+ }
+ p.mu.Unlock()
+ }
+ }()
+}
+
// stageDir returns the directory staged images live in, creating it on first
// use. One directory for the whole session, rather than one per image: nothing
// deletes the file afterwards, so a directory per image leaked every cover that
diff --git a/internal/tui/preview_test.go b/internal/tui/preview_test.go
@@ -13,6 +13,7 @@ import (
"path/filepath"
"strings"
"testing"
+ "time"
"charm.land/bubbletea/v2"
)
@@ -426,3 +427,85 @@ func TestStagingReusesOneDirectoryAndCleansUp(t *testing.T) {
// Closing twice must not panic; the program can exit by more than one path.
p.Close()
}
+
+// Staging decodes, rescales and re-encodes, which is slow enough to be felt when
+// it happens on the UI goroutine as the cursor moves. Warming prepares the whole
+// candidate list in the background instead.
+func TestWarmStagesCandidatesForLater(t *testing.T) {
+ srcs := t.TempDir()
+ var paths []string
+ for i := 0; i < 3; i++ {
+ p := filepath.Join(srcs, fmt.Sprintf("cand%d.jpg", i))
+ writeJPEG(t, p, 1000)
+ paths = append(paths, p)
+ }
+
+ prev := &ImagePreviewer{mode: imageKitty}
+
+ // Calibrate against this machine: staging one of these takes this long, and
+ // a Warm that did the work itself would take at least three times as long.
+ start := time.Now()
+ stageTempImage(prev.stageDir(), paths[0])
+ one := time.Since(start)
+
+ start = time.Now()
+ prev.Warm(paths)
+ elapsed := time.Since(start)
+ if one > 0 && elapsed > one {
+ t.Errorf("Warm took %v, as long as staging a single image (%v); it is not running in the background", elapsed, one)
+ }
+
+ // It still has to finish the job on its own.
+ deadline := time.Now().Add(20 * time.Second)
+ for {
+ prev.mu.Lock()
+ done := 0
+ for _, p := range paths {
+ if prev.staged[p] != "" {
+ done++
+ }
+ }
+ prev.mu.Unlock()
+ if done == len(paths) {
+ break
+ }
+ if time.Now().After(deadline) {
+ t.Fatalf("only %d of %d candidates were staged", done, len(paths))
+ }
+ time.Sleep(5 * time.Millisecond)
+ }
+
+ // And a render then finds the image waiting rather than staging it again.
+ prev.mu.Lock()
+ warmed := prev.staged[paths[1]]
+ prev.mu.Unlock()
+ if lines := prev.Render(paths[1], 40, 20); len(lines) == 0 {
+ t.Fatal("no grid for a warmed image")
+ }
+ place, ok := prev.TakePlacement()
+ if !ok {
+ t.Fatal("no placement queued")
+ }
+ if place.tempPath != warmed {
+ t.Errorf("render staged %q instead of using the warmed %q", place.tempPath, warmed)
+ }
+}
+
+// With images drawn as characters there is nothing to stage, so warming is a
+// no-op rather than an error.
+func TestWarmDoesNothingForCharactersOrNone(t *testing.T) {
+ srcs := t.TempDir()
+ p := filepath.Join(srcs, "c.jpg")
+ writeJPEG(t, p, 64)
+
+ for _, mode := range []imageMode{imageChars, imageNone} {
+ prev := &ImagePreviewer{mode: mode}
+ prev.Warm([]string{p})
+ prev.mu.Lock()
+ n := len(prev.staged)
+ prev.mu.Unlock()
+ if n != 0 {
+ t.Errorf("mode %v staged %d images", mode, n)
+ }
+ }
+}
diff --git a/internal/tui/tui.go b/internal/tui/tui.go
@@ -272,6 +272,13 @@ func (m Module) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.screen = ScreenCoverPicker
m.scroll = 0
m.status = fmt.Sprintf("%d candidate%s", len(m.covers), plural(len(m.covers)))
+ // Prepare every candidate in the background before the user starts
+ // moving through them, so a render rarely has to stage one itself.
+ paths := make([]string, 0, len(m.covers))
+ for _, c := range m.covers {
+ paths = append(paths, c.Path)
+ }
+ m.prev.Warm(paths)
m.renderCoverPreview(m.coverPreviewWidth(), m.coverPreviewHeight())
return m, tea.Batch(m.placeCover(), replantTick())