Fix text measurement cache collisions

.changeset/measure-cache-collisions.md

@@ -0,0 +1,5 @@
+---
+"@bomb.sh/tty": patch
+---
+
+Fixes text that could be measured and drawn as a different string. Two texts whose measurement-cache hashes collide now each keep their own width and wrapping, within one frame and across renders.

patches/clay-measure-text-cache-key.patch

@@ -0,0 +1,67 @@
+diff --git a/clay.h b/clay.h
+index 6452cd6..1a29854 100644
+--- a/clay.h
++++ b/clay.h
+@@ -1299,6 +1299,11 @@ typedef struct {
+     int32_t measuredWordsStartIndex;
+     float minWidth;
+     bool containsNewlines;
++    // Cache key: the hash alone can collide, so lookups also compare these
++    Clay_String cacheText;
++    uint16_t cacheFontId;
++    uint16_t cacheFontSize;
++    uint16_t cacheLetterSpacing;
+     // Hash map data
+     uint32_t id;
+     int32_t nextIndex;
+@@ -1638,6 +1643,12 @@ Clay__MeasuredWord *Clay__AddMeasuredWord(Clay__MeasuredWord word, Clay__Measure
+     }
+ }
+
++bool Clay__MemCmp(const char *s1, const char *s2, int32_t length);
++
++// Returns storage holding a copy of the key that stays readable for as long as
++// the published cache entry, given the owner from errorHandler.userData.
++const char *clayterm_copy_text_key(void *userData, const char *chars, int32_t length);
++
+ Clay__MeasureTextCacheItem *Clay__MeasureTextCached(Clay_String *text, Clay_TextElementConfig *config) {
+     Clay_Context* context = Clay_GetCurrentContext();
+     #ifndef CLAY_WASM
+@@ -1658,7 +1669,12 @@ Clay__MeasureTextCacheItem *Clay__MeasureTextCached(Clay_String *text, Clay_Text
+     int32_t elementIndex = context->measureTextHashMap.internalArray[hashBucket];
+     while (elementIndex != 0) {
+         Clay__MeasureTextCacheItem *hashEntry = Clay__MeasureTextCacheItemArray_Get(&context->measureTextHashMapInternal, elementIndex);
+-        if (hashEntry->id == id) {
++        if (hashEntry->id == id &&
++            hashEntry->cacheText.length == text->length &&
++            hashEntry->cacheFontId == config->fontId &&
++            hashEntry->cacheFontSize == config->fontSize &&
++            hashEntry->cacheLetterSpacing == config->letterSpacing &&
++            Clay__MemCmp(hashEntry->cacheText.chars, text->chars, text->length)) {
+             hashEntry->generation = context->generation;
+             return hashEntry;
+         }
+@@ -1689,7 +1705,12 @@ Clay__MeasureTextCacheItem *Clay__MeasureTextCached(Clay_String *text, Clay_Text
+     }
+
+     int32_t newItemIndex = 0;
+-    Clay__MeasureTextCacheItem newCacheItem = { .measuredWordsStartIndex = -1, .id = id, .generation = context->generation };
++    Clay__MeasureTextCacheItem newCacheItem = {
++        .measuredWordsStartIndex = -1, .id = id, .generation = context->generation,
++        .cacheText = { .length = text->length, .chars = NULL },
++        .cacheFontId = config->fontId, .cacheFontSize = config->fontSize,
++        .cacheLetterSpacing = config->letterSpacing
++    };
+     Clay__MeasureTextCacheItem *measured = NULL;
+     if (context->measureTextHashMapInternalFreeList.length > 0) {
+         newItemIndex = Clay__int32_tArray_GetValue(&context->measureTextHashMapInternalFreeList, context->measureTextHashMapInternalFreeList.length - 1);
+@@ -1771,6 +1792,9 @@ Clay__MeasureTextCacheItem *Clay__MeasureTextCached(Clay_String *text, Clay_Text
+     measured->unwrappedDimensions.width = measuredWidth;
+     measured->unwrappedDimensions.height = measuredHeight;
+
++    // Only an entry linked below can be found again, so only it keeps a key.
++    measured->cacheText.chars = clayterm_copy_text_key(context->errorHandler.userData, text->chars, text->length);
++
+     if (elementIndexPrevious != 0) {
+         Clay__MeasureTextCacheItemArray_Get(&context->measureTextHashMapInternal, elementIndexPrevious)->nextIndex = newItemIndex;
+     } else {

specs/renderer-spec.md

@@ -1256,6 +1256,15 @@ counted as separate cells in layout. Measurement and rendering MUST agree: if
 the measurer ignores a combining mark for width purposes, the renderer MUST
 still attach and emit it.
 
+**Measurement cache.** Clay caches each text's measurement under a hash of its
+content and text config. A local Clay patch reuses a cached measurement only
+when the text bytes and config also match, so strings whose hashes collide are
+each measured as themselves. A frame's text bytes do not outlive its render, so
+each renderer instance copies the keys of cached measurements into a fixed
+region of its own state, clearing Clay's cache before any layout whose text
+might not fit there. A frame too large for that region keys its measurements on
+its own bytes, and the cache is cleared once that frame is presented.
+
 **Cell representation.** Each cell in the buffer stores a grapheme cluster — a
 base Unicode codepoint plus up to 8 combining-mark codepoints — together with a
 foreground color (packed ARGB with attribute flags in the high byte) and a

src/clayterm.c

@@ -96,10 +96,21 @@ struct Clayterm {
       caret_y;   /* resolved cell (column, row); -1 sentinel until placed */
   int has_caret; /* 1 when the current frame will emit a cursor */
   int had_caret_last_frame; /* 1 when the previous frame emitted a cursor */
+  /* Text-measurement cache keys (patches/clay-measure-text-cache-key.patch).
+   * Clay matches a cached measurement by its text, so each published entry
+   * keeps a copy here: the frame's wire bytes are overwritten by the next
+   * render. Copies are appended and reclaimed together by resetting Clay's
+   * cache before a layout that might not fit. When this frame's keys are
+   * borrowed from its wire bytes instead, the cache is reset once the frame
+   * has been presented so no borrowed key outlives reduce(). */
+  char *text_keys;
+  int text_keys_used;
+  int text_keys_borrowed;
 };
 
 /* Memory layout inside the arena provided by the host:
  *   [Clayterm struct] [front cells] [back cells] [output buffer]
+ *   [Clay arena] [text keys]
  */
 /* 128 bytes per cell: ~84 bytes worst-case for CUP + SGR sequences, plus up
  * to 4 (base SMP char) + 8×4 (combining marks) = 36 bytes of cluster text. */
@@ -667,8 +678,9 @@ int clayterm_size(int w, int h) {
   int clay_bytes = (int)Clay_MinMemorySize();
   return align8((int)sizeof(struct Clayterm)) + align8(cell_bytes) /* front */
          + align8(cell_bytes)                                      /* back */
-         + align8(out_bytes)    /* output buffer */
-         + align64(clay_bytes); /* Clay arena */
+         + align8(out_bytes)                /* output buffer */
+         + align64(clay_bytes)              /* Clay arena */
+         + align8(CLAYTERM_TEXT_KEY_BYTES); /* text keys */
 }
 
 static void clay_error(Clay_ErrorData err) {
@@ -746,6 +758,7 @@ struct Clayterm *init(void *mem, int w, int h) {
       .lastbg = 0xffffffff,
       .lastx = -1,
       .lasty = -1,
+      .text_keys = clay_mem + clay_bytes,
   };
 
   // initialize back buffer with spaces and default fg/bg
@@ -790,6 +803,17 @@ void reduce(struct Clayterm *ct, uint32_t *buf, int len, int mode, int row,
   ct->caret_y = -1;
   ct->has_caret = 0;
 
+  /* A layout copies at most one key per distinct text, and no text's key is
+   * longer than the words encoding it, so len bounds this layout's copies.
+   * Reset only here, before Clay_BeginLayout, never while a layout holds
+   * measured-word references. */
+  ct->text_keys_borrowed = len > CLAYTERM_TEXT_KEY_BYTES / 4;
+  if (ct->text_keys_borrowed ||
+      len > (CLAYTERM_TEXT_KEY_BYTES - ct->text_keys_used) / 4) {
+    Clay_ResetMeasureTextCache();
+    ct->text_keys_used = 0;
+  }
+
   Clay_BeginLayout();
 
   while (i < len) {
@@ -1110,9 +1134,31 @@ void reduce(struct Clayterm *ct, uint32_t *buf, int len, int mode, int row,
     buf_str(&ct->out, "\x1b\\");
   }
 
+  /* Borrowed keys point into this call's wire bytes; none may outlive it. */
+  if (ct->text_keys_borrowed) {
+    Clay_ResetMeasureTextCache();
+    ct->text_keys_used = 0;
+  }
+
   ct_active_context = NULL;
 }
 
+const char *clayterm_copy_text_key(void *userData, const char *chars,
+                                   int32_t length) {
+  struct Clayterm *ct = (struct Clayterm *)userData;
+  /* A key that does not fit is borrowed; reduce() resets the cache after
+   * presenting the frame that borrowed it. */
+  if (ct->text_keys_borrowed || length < 0 ||
+      length > CLAYTERM_TEXT_KEY_BYTES - ct->text_keys_used) {
+    ct->text_keys_borrowed = 1;
+    return chars;
+  }
+  char *key = ct->text_keys + ct->text_keys_used;
+  memcpy(key, chars, (size_t)length);
+  ct->text_keys_used += length;
+  return key;
+}
+
 char *output(struct Clayterm *ct) { return ct->out.data; }
 
 int length(struct Clayterm *ct) { return ct->out.length; }

src/clayterm.h

@@ -26,6 +26,13 @@ struct Clayterm;
 #define CLAYTERM_PROP_TRANSITION 0x40
 #define CLAYTERM_PROP_CURSOR 0x80
 
+/* Bytes of retained text-measurement cache keys per instance. Mirrors the
+ * host's fixed transfer budget in term-native.ts (1MB of payload plus 120
+ * bytes per Clay element at the 8192-element default), so any frame within
+ * that budget has its keys copied. Larger frames borrow theirs from the
+ * frame's own wire bytes for the duration of reduce(). */
+#define CLAYTERM_TEXT_KEY_BYTES (1024 * 1024 + 8192 * 120)
+
 /* WASM exports */
 int clayterm_size(int w, int h);
 struct Clayterm *init(void *mem, int w, int h);
@@ -45,4 +52,9 @@ int pointer_over_count(void);
 int pointer_over_id_string_length(int index);
 int pointer_over_id_string_ptr(int index);
 
+/* Clay hook (patches/clay-measure-text-cache-key.patch): userData is the
+ * owning Clayterm from Clay's error handler. */
+const char *clayterm_copy_text_key(void *userData, const char *chars,
+                                   int32_t length);
+
 #endif

test/width.test.ts

@@ -1,10 +1,41 @@
 import { describe, expect, it } from "./suite.ts";
 import { createTerm } from "../term.ts";
-import { close, grow, open, pack, text } from "../ops.ts";
+import { close, grow, type Op, open, pack, text } from "../ops.ts";
 import { createTermNative } from "../term-native.ts";
 import { print } from "./print.ts";
 
 const decode = (bytes: Uint8Array) => new TextDecoder().decode(bytes);
+const trim = (s: string) => s.split("\n").map((l) => l.trimEnd()).join("\n");
+
+// Two distinct strings whose measurement-cache hashes collide under the
+// default text config. Each must still measure and draw as itself.
+const SHORT = "synthetic sample synthetic sample synthetic sample 022aa2";
+const LONG =
+  "synthetic sample synthetic sample synthetic sample synthetic sample synthetic sample 01fda4";
+
+// Clay_ErrorType value, mirrored by ERROR_TYPES in term.ts
+const TEXT_MEASUREMENT_CAPACITY_EXCEEDED = 3;
+
+function boxes(contents: string[]): Op[] {
+  return [
+    open("root", {
+      layout: { width: grow(), height: grow(), direction: "ttb" },
+    }),
+    ...contents.flatMap((content, i) => [
+      open(`box-${i}`),
+      text(content),
+      close(),
+    ]),
+    close(),
+  ];
+}
+
+// The visible picture of boxes(contents) on a w x h grid.
+function picture(contents: string[], w: number, h: number): string {
+  let lines = contents.slice(0, h).map((c) => c.slice(0, w));
+  while (lines.length < h) lines.push("");
+  return lines.join("\n");
+}
 
 function indexOfSeq(haystack: Uint8Array, needle: number[]): number {
   outer: for (let i = 0; i + needle.length <= haystack.length; i++) {
@@ -97,4 +128,130 @@ describe("width", () => {
     expect(indexOfSeq(out, [0xed, 0xa0, 0x80])).toBe(-1);
     expect(print(decode(out), 10, 3)).toContain("a�b");
   });
+
+  describe("measurement cache", () => {
+    it("measures colliding strings as themselves in one frame", async () => {
+      let term = await createTerm({ width: 100, height: 3 });
+      let result = term.render(boxes([SHORT, LONG]));
+
+      expect(SHORT.length).toBe(57);
+      expect(LONG.length).toBe(91);
+      expect(result.info.get("box-0")?.bounds.width).toBe(57);
+      expect(result.info.get("box-1")?.bounds.width).toBe(91);
+      expect(trim(print(decode(result.output), 100, 3))).toEqual(
+        [SHORT, LONG, ""].join("\n"),
+      );
+    });
+
+    it("measures colliding strings as themselves across renders", async () => {
+      let term = await createTerm({ width: 100, height: 3 });
+      let screen = "";
+
+      for (let content of [SHORT, LONG, SHORT]) {
+        let result = term.render(boxes([content]));
+        screen += decode(result.output);
+        expect(result.info.get("box-0")?.bounds.width).toBe(content.length);
+        expect(trim(print(screen, 100, 3))).toEqual(
+          picture([content], 100, 3),
+        );
+      }
+    });
+
+    it("reuses text across renders that exhaust retained key storage", async () => {
+      let term = await createTerm({ width: 186, height: 40 });
+      let pass = (n: number) =>
+        Array.from(
+          { length: 390 },
+          (_, i) => `${n}-${String(i).padStart(4, "0")}${"x".repeat(4994)}`,
+        );
+
+      // each pass retains ~2MB of distinct text; the second cannot fit
+      // beside the first, and the third reuses the second
+      let screen = "";
+      for (let contents of [pass(0), pass(1), pass(1), pass(0)]) {
+        let result = term.render(boxes(contents));
+        screen += decode(result.output);
+        expect(result.errors).toEqual([]);
+        expect(result.info.get("box-0")?.bounds.width).toBe(5000);
+        expect(result.info.get("box-389")?.bounds.width).toBe(5000);
+        expect(trim(print(screen, 186, 40))).toEqual(
+          picture(contents, 186, 40),
+        );
+      }
+
+      let result = term.render(boxes([SHORT, LONG]));
+      expect(result.info.get("box-0")?.bounds.width).toBe(57);
+      expect(result.info.get("box-1")?.bounds.width).toBe(91);
+    });
+
+    it("lays out a frame larger than retained key storage after a resize", async () => {
+      let term = await createTerm({ width: 400, height: 100 });
+      term.update([{ type: "resize", width: 186, height: 40 }]);
+      let contents = Array.from(
+        { length: 430 },
+        (_, i) => `${i}${"x".repeat(5000)}`,
+      );
+
+      let screen = "";
+      for (let round = 0; round < 2; round++) {
+        let result = term.render(boxes(contents));
+        screen += decode(result.output);
+        expect(result.errors).toEqual([]);
+        expect(result.info.get("box-429")?.bounds.width).toBe(5003);
+        expect(trim(print(screen, 186, 40))).toEqual(
+          picture(contents, 186, 40),
+        );
+      }
+
+      let result = term.render(boxes([SHORT, LONG]));
+      screen += decode(result.output);
+      expect(result.info.get("box-0")?.bounds.width).toBe(57);
+      expect(result.info.get("box-1")?.bounds.width).toBe(91);
+      expect(trim(print(screen, 186, 40))).toEqual(
+        picture([SHORT, LONG], 186, 40),
+      );
+    });
+
+    it("leaves the packed frame intact when measurement capacity runs out", async () => {
+      let native = await createTermNative(186, 40, {
+        syncOutput: false,
+        pointerShape: false,
+      });
+      let ops = [
+        open("root", { layout: { direction: "ttb" } }),
+        open("words"),
+        text("a ".repeat(20000)),
+        close(),
+        open("long"),
+        text("z".repeat(1100000)),
+        close(),
+        close(),
+      ];
+      let len = pack(
+        ops,
+        native.memory.buffer,
+        native.opsBuf,
+        native.memory.buffer.byteLength,
+      );
+      // pack() returns a length in 32-bit words
+      let before = new Uint8Array(native.memory.buffer, native.opsBuf, len * 4)
+        .slice();
+
+      native.reduce(native.statePtr, native.opsBuf, len, 0, 1, 0, 0, 0, 0);
+
+      let after = new Uint8Array(native.memory.buffer, native.opsBuf, len * 4);
+      expect(after).toEqual(before);
+      let count = native.errorCount(native.statePtr);
+      for (let i = 0; i < count; i++) {
+        expect(native.errorType(native.statePtr, i)).toBe(
+          TEXT_MEASUREMENT_CAPACITY_EXCEEDED,
+        );
+      }
+
+      let replacement = await createTerm({ width: 100, height: 3 });
+      let result = replacement.render(boxes([SHORT, LONG]));
+      expect(result.info.get("box-0")?.bounds.width).toBe(57);
+      expect(result.info.get("box-1")?.bounds.width).toBe(91);
+    });
+  });
 });