From 40caf0aa332e53de74a8d42def09cff4af0c7765 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Tue, 27 Oct 2026 12:42:10 -0500 Subject: [PATCH] Fix(tui): preview pictures re-fit when the pane's width changes A picture's size in terminal cells was fixed the first time it was shown. LaTeX, TikZ, typst and mermaid pictures kept the size their background render finished at, and a vault image was only re-sent when the pane got narrower, never wider. So a note first previewed in a narrow pane (a split, a window still opening) kept small pictures after the pane grew: a long formula sized for 60 columns stayed a faint dotted line at 152, which is what the 0.12.0 announcement screenshot showed. The image store now keeps each picture's PNG and fits it to the width it is asked for, both ways, every time the preview redraws. A size already sent is reused, so moving between two widths sends nothing new, and at most three sizes of one picture stay in the terminal: the oldest is freed with a delete command. The kept PNGs are capped at 64 MiB; past that the least recently shown picture's bytes go, and it keeps the size it has (a vault image reloads from disk instead). It does not follow a change of the terminal's font size: the cell size is still read once, at the first picture. --- internal/tui/embeds.go | 27 +++++-- internal/tui/images.go | 136 +++++++++++++++++++++++++++++------- internal/tui/images_test.go | 132 ++++++++++++++++++++++++++++++++++ internal/tui/latex_test.go | 10 ++- 4 files changed, 272 insertions(+), 33 deletions(-) create mode 100644 internal/tui/images_test.go diff --git a/internal/tui/embeds.go b/internal/tui/embeds.go index 159f75a..252d2bb 100644 --- a/internal/tui/embeds.go +++ b/internal/tui/embeds.go @@ -392,7 +392,7 @@ func (a *App) mermaidLines(src string, srcLine, width int) []previewLine { key := contentKey("mermaid", src) if a.imageMode() == imagesKitty { if img := a.externalImage(key, width-2, "mermaid", src); img != nil && img.err == nil { - return a.pictureLines(img, srcLine, width, "mermaid") + return a.pictureLines(a.fitted(key, img, width-2), srcLine, width, "mermaid") } } d := a.diagrams() @@ -447,15 +447,18 @@ func (a *App) mathLines(body string, srcLine, width int) []previewLine { out := []previewLine{} if a.imageMode() == imagesKitty { var img *termImage + var key string if a.prefs.TerminalMath == "latex" { src := strings.TrimSpace(body) - img = a.externalImage(latexKey(string(th.Fg), currentLatexFormat(), src), width-2, "latex", src) + key = latexKey(string(th.Fg), currentLatexFormat(), src) + img = a.externalImage(key, width-2, "latex", src) out = append(out, a.latexErrorRows(img, srcLine, width)...) } else { - img = a.externalImage(contentKey("math", fmt.Sprintf("typst\x00%t\x00%s", th.Dark, inner)), width-2, "math", inner) + key = contentKey("math", fmt.Sprintf("typst\x00%t\x00%s", th.Dark, inner)) + img = a.externalImage(key, width-2, "math", inner) } if img != nil && img.err == nil { - return a.pictureLines(img, srcLine, width, "math") + return a.pictureLines(a.fitted(key, img, width-2), srcLine, width, "math") } } for _, l := range strings.Split(inner, "\n") { @@ -471,13 +474,25 @@ func (a *App) tikzLines(src string, srcLine, width int) ([]previewLine, bool) { if a.imageMode() != imagesKitty { return nil, false } - img := a.externalImage(latexKey(string(a.theme.Fg), currentLatexFormat(), latexKeySrc("tikz", src)), width-2, "tikz", src) + key := latexKey(string(a.theme.Fg), currentLatexFormat(), latexKeySrc("tikz", src)) + img := a.externalImage(key, width-2, "tikz", src) if img != nil && img.err == nil { - return a.pictureLines(img, srcLine, width, "tikz"), true + return a.pictureLines(a.fitted(key, img, width-2), srcLine, width, "tikz"), true } return a.latexErrorRows(img, srcLine, width), false } +// fitted is a rendered picture at the size that fits maxCols now. The +// background render sends it at the pane width of the moment it finished; +// without this, a picture first shown in a narrow pane (a split, a window +// still opening) stays that small after the pane grows. +func (a *App) fitted(key string, img *termImage, maxCols int) *termImage { + if f := a.images().refit(key, maxCols, 40); f != nil && f.err == nil { + return f + } + return img +} + // latexErrorRows is the first line of a failed latex render, wrapped rather // than truncated because the cause sits at the end of long paths. A render // that worked, is still running or lacks its tools has none. diff --git a/internal/tui/images.go b/internal/tui/images.go index 292b236..946079c 100644 --- a/internal/tui/images.go +++ b/internal/tui/images.go @@ -73,14 +73,37 @@ type termImage struct { } type imageStore struct { - mu sync.Mutex - byKey map[string]*termImage - nextID uint32 - cellW int - cellH int - out io.Writer + mu sync.Mutex + byKey map[string]*termImage + // pics keeps each key's PNG and the sizes it was sent at, so a picture + // re-fits when its pane changes width without being loaded again. + pics map[string]*storedPic + picBytes int + picTick int + nextID uint32 + cellW int + cellH int + out io.Writer } +// storedPic is a loaded picture and the sizes sent for it, newest first. +type storedPic struct { + png []byte + w, h int + sent []*termImage + used int +} + +// maxSentSizes is how many sizes of one picture stay in the terminal: a +// note open in two panes of different widths shows both without either +// being sent again on every redraw. An older size is freed. +const maxSentSizes = 3 + +// maxPicBytes caps the PNGs kept for re-fitting. Past it the least recently +// shown picture's bytes go; a vault image then reloads from disk when it is +// next re-fitted, and a rendered one keeps the size it has. Tests lower it. +var maxPicBytes = 64 << 20 + func newImageStore() *imageStore { s := &imageStore{byKey: map[string]*termImage{}, nextID: 1000, out: os.Stdout} s.cellW, s.cellH = cellPixelSize() @@ -94,42 +117,105 @@ func (a *App) images() *imageStore { return a.imageStore } -// imageFor returns the transmitted image for a key, loading and sending it -// on first use. maxCols bounds the width; the height follows the picture's -// aspect ratio at the terminal's cell size, capped at maxRows. +// imageFor returns the transmitted image for a key at the size that fits +// maxCols, loading the picture on first use. The width is the picture's +// own, or maxCols when that is narrower; the height follows its aspect +// ratio at the terminal's cell size, capped at maxRows. The size follows +// maxCols both ways, so a picture first shown in a narrow pane grows when +// the pane does instead of staying small. func (s *imageStore) imageFor(key string, maxCols, maxRows int, load func() ([]byte, error)) *termImage { s.mu.Lock() defer s.mu.Unlock() - if img, ok := s.byKey[key]; ok && (img.err != nil || img.cols <= maxCols) { + if img, ok := s.byKey[key]; ok && img.err != nil { return img } - data, err := load() - if err != nil { - img := &termImage{err: err} - s.byKey[key] = img - return img + if s.pics == nil { + s.pics = map[string]*storedPic{} } - pngData, w, h, err := toPNG(data, 1600) - if err != nil { - img := &termImage{err: err} - s.byKey[key] = img - return img + p, ok := s.pics[key] + if !ok { + data, err := load() + var pngData []byte + var w, h int + if err == nil { + pngData, w, h, err = toPNG(data, 1600) + } + if err != nil { + img := &termImage{err: err} + s.byKey[key] = img + return img + } + p = &storedPic{png: pngData, w: w, h: h} + s.pics[key] = p + s.picBytes += len(pngData) } - cols := max(1, min(maxCols, (w+s.cellW-1)/s.cellW)) - rows := max(1, int(float64(h)*float64(cols)*float64(s.cellW)/float64(w)/float64(s.cellH)+0.5)) + img := s.fitLocked(key, p, maxCols, maxRows) + s.trimPics() + return img +} + +// refit is imageFor for a picture already loaded under key, at the size +// that fits maxCols now, or nil when its bytes are not kept. +func (s *imageStore) refit(key string, maxCols, maxRows int) *termImage { + s.mu.Lock() + defer s.mu.Unlock() + p, ok := s.pics[key] + if !ok { + return nil + } + return s.fitLocked(key, p, maxCols, maxRows) +} + +// fitLocked returns p at the size that fits maxCols, sending that size the +// first time it is asked for. s.mu must be held. +func (s *imageStore) fitLocked(key string, p *storedPic, maxCols, maxRows int) *termImage { + s.picTick++ + p.used = s.picTick + cols := max(1, min(maxCols, (p.w+s.cellW-1)/s.cellW)) + rows := max(1, int(float64(p.h)*float64(cols)*float64(s.cellW)/float64(p.w)/float64(s.cellH)+0.5)) if maxRows > 0 && rows > maxRows { rows = maxRows - cols = max(1, int(float64(w)*float64(rows)*float64(s.cellH)/float64(h)/float64(s.cellW)+0.5)) + cols = max(1, int(float64(p.w)*float64(rows)*float64(s.cellH)/float64(p.h)/float64(s.cellW)+0.5)) + } + for i, img := range p.sent { + if img.cols == cols && img.rows == rows { + copy(p.sent[1:i+1], p.sent[:i]) + p.sent[0] = img + s.byKey[key] = img + return img + } } s.nextID++ - img := &termImage{id: s.nextID, cols: cols, rows: rows, width: w, height: h} - if err := kittyTransmit(s.out, img.id, cols, rows, pngData, insideTmux()); err != nil { + img := &termImage{id: s.nextID, cols: cols, rows: rows, width: p.w, height: p.h} + if err := kittyTransmit(s.out, img.id, cols, rows, p.png, insideTmux()); err != nil { img.err = err + s.byKey[key] = img + return img + } + p.sent = append([]*termImage{img}, p.sent...) + if len(p.sent) > maxSentSizes { + kittyDelete(s.out, p.sent[maxSentSizes].id, insideTmux()) + p.sent = p.sent[:maxSentSizes] } s.byKey[key] = img return img } +// trimPics drops the least recently shown pictures' bytes past +// maxPicBytes. Their sent sizes stay in the terminal and on screen. +func (s *imageStore) trimPics() { + for s.picBytes > maxPicBytes && len(s.pics) > 1 { + oldest, at := "", 0 + for k, p := range s.pics { + if oldest == "" || p.used < at { + oldest, at = k, p.used + } + } + s.picBytes -= len(s.pics[oldest].png) + delete(s.pics, oldest) + } +} + // toPNG decodes any supported picture and re-encodes it as PNG, scaled // down when wider than maxWidth so a huge photo does not flood the terminal. func toPNG(data []byte, maxWidth int) ([]byte, int, int, error) { diff --git a/internal/tui/images_test.go b/internal/tui/images_test.go new file mode 100644 index 0000000..7002271 --- /dev/null +++ b/internal/tui/images_test.go @@ -0,0 +1,132 @@ +package tui + +import ( + "bytes" + "image" + "strconv" + "strings" + "testing" + "time" +) + +// fullPNG is a w x h picture inked edge to edge, so cropToInk keeps its size. +func fullPNG(t *testing.T, w, h int) []byte { + t.Helper() + return inkPNG(t, w, h, image.Rect(0, 0, w, h)) +} + +func testStore() (*imageStore, *bytes.Buffer) { + out := &bytes.Buffer{} + return &imageStore{byKey: map[string]*termImage{}, nextID: 1000, cellW: 10, cellH: 20, out: out}, out +} + +// sends counts the pictures transmitted to out since the last call. +func sends(out *bytes.Buffer) int { + n := strings.Count(out.String(), "a=T,") + out.Reset() + return n +} + +func TestPictureGrowsAndShrinksWithThePane(t *testing.T) { + s, out := testStore() + data := fullPNG(t, 1552, 122) // 156 cells wide at 10 px a cell + load := func() ([]byte, error) { return data, nil } + + narrow := s.imageFor("wide", 30, 40, load) + if narrow.cols != 30 || sends(out) != 1 { + t.Fatalf("narrow pane: %d cols, want 30, sent once", narrow.cols) + } + wide := s.imageFor("wide", 120, 40, func() ([]byte, error) { t.Fatal("loaded again"); return nil, nil }) + if wide.cols != 120 || wide.id == narrow.id || sends(out) != 1 { + t.Fatalf("wider pane: %d cols (id %d, was %d), want 120 under a new id, sent once", wide.cols, wide.id, narrow.id) + } + if wide.rows < narrow.rows { + t.Fatalf("height shrank as the picture grew: %d rows from %d", wide.rows, narrow.rows) + } + again := s.imageFor("wide", 30, 40, load) + if again.id != narrow.id || sends(out) != 0 { + t.Fatalf("back to the narrow pane sent a new picture (id %d, was %d)", again.id, narrow.id) + } +} + +func TestPictureAtItsOwnSizeIsSentOnce(t *testing.T) { + s, out := testStore() + data := fullPNG(t, 120, 40) // 12 cells wide + load := func() ([]byte, error) { return data, nil } + first := s.imageFor("small", 30, 40, load) + for _, cols := range []int{60, 120, 13} { + if img := s.imageFor("small", cols, 40, load); img.id != first.id || img.cols != 12 { + t.Fatalf("at %d cols: id %d cols %d, want id %d at its own 12 cols", cols, img.id, img.cols, first.id) + } + } + if n := sends(out); n != 1 { + t.Fatalf("a picture that fits every pane was sent %d times, want 1", n) + } +} + +func TestOldestPictureSizeIsFreed(t *testing.T) { + s, out := testStore() + data := fullPNG(t, 1552, 122) + load := func() ([]byte, error) { return data, nil } + first := s.imageFor("wide", 20, 40, load) + for _, cols := range []int{40, 60, 80} { + s.imageFor("wide", cols, 40, load) + } + if !strings.Contains(out.String(), "a=d,d=I,i="+strconv.FormatUint(uint64(first.id), 10)+",") { + t.Fatalf("the fourth size did not free the first (id %d):\n%q", first.id, out.String()) + } + out.Reset() + if img := s.imageFor("wide", 20, 40, load); img.id == first.id || sends(out) != 1 { + t.Fatalf("a freed size came back under its old id %d", first.id) + } +} + +func TestKeptPictureBytesAreCapped(t *testing.T) { + prev := maxPicBytes + t.Cleanup(func() { maxPicBytes = prev }) + s, _ := testStore() + data := fullPNG(t, 400, 100) + maxPicBytes = 2*len(data) + len(data)/2 + for _, k := range []string{"a", "b", "c"} { + s.imageFor(k, 80, 40, func() ([]byte, error) { return data, nil }) + } + if _, ok := s.pics["a"]; ok || len(s.pics) != 2 { + t.Fatalf("kept %d pictures, want the 2 most recent: %v", len(s.pics), s.pics) + } + if s.refit("a", 20, 40) != nil { + t.Fatal("refit of a dropped picture did not report it") + } + if img := s.byKey["a"]; img == nil || img.err != nil { + t.Fatalf("dropping the bytes broke the picture on screen: %+v", img) + } +} + +// placeholderCols is how many placeholder cells the first picture row of a +// preview has. +func placeholderCols(lines []previewLine) int { + for _, l := range lines { + if n := strings.Count(l.text, "\U0010EEEE"); n > 0 { + return n + } + } + return 0 +} + +func TestLatexPictureRefitsWhenThePreviewWidens(t *testing.T) { + latexEnv(t) + fakeTeX(t, texFake{png: fullPNG(t, 1552, 122)}) + a, done := newMathApp(t, mathPrefs("latex", "kitty")) + body := "\\[ a_1 + a_2 + \\dots + a_{20} \\]" + a.mathLines(body, 0, 32) + if !nextRender(done, 5*time.Second) { + t.Fatal("no render finished") + } + narrow := placeholderCols(a.mathLines(body, 0, 32)) + wide := placeholderCols(a.mathLines(body, 0, 122)) + if narrow != 30 || wide != 120 { + t.Fatalf("picture is %d cells in a 32-cell preview and %d in a 122-cell one, want 30 then 120", narrow, wide) + } + if back := placeholderCols(a.mathLines(body, 0, 32)); back != 30 { + t.Fatalf("narrowed again, the picture is %d cells, want 30", back) + } +} diff --git a/internal/tui/latex_test.go b/internal/tui/latex_test.go index 8ecf087..f7783c9 100644 --- a/internal/tui/latex_test.go +++ b/internal/tui/latex_test.go @@ -101,6 +101,8 @@ type texFake struct { render func(ctx context.Context, base string) ([]byte, error) // cairo, when set, replaces a successful pdftocairo run. cairo func(ctx context.Context) ([]byte, error) + // png, when set, is the picture pdftocairo writes instead of tinyPNG. + png []byte } // fakeTeX swaps runTool for fake pdftex and pdftocairo. A format build @@ -187,17 +189,21 @@ func fakeTeX(t *testing.T, f texFake) func() []texCall { if f.cairo != nil { return f.cairo(ctx) } + pic := f.png + if pic == nil { + pic = tinyPNG(t) + } wrote := false for _, a := range args { if strings.HasSuffix(a, ".png") { - if err := os.WriteFile(inDir(dir, a), tinyPNG(t), 0o644); err != nil { + if err := os.WriteFile(inDir(dir, a), pic, 0o644); err != nil { return nil, err } wrote = true } } if last := args[len(args)-1]; !wrote && !strings.HasPrefix(last, "-") && !strings.HasSuffix(last, ".pdf") { - if err := os.WriteFile(inDir(dir, last)+".png", tinyPNG(t), 0o644); err != nil { + if err := os.WriteFile(inDir(dir, last)+".png", pic, 0o644); err != nil { return nil, err } }