diff --git a/.changeset/measure-cache-collisions.md b/.changeset/measure-cache-collisions.md new file mode 100644 index 0000000..bf4e6fe --- /dev/null +++ b/.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. diff --git a/patches/clay-measure-text-cache-key.patch b/patches/clay-measure-text-cache-key.patch new file mode 100644 index 0000000..fe37403 --- /dev/null +++ b/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 { diff --git a/specs/renderer-spec.md b/specs/renderer-spec.md index 0f4bb89..15ebe88 100644 --- a/specs/renderer-spec.md +++ b/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 diff --git a/src/clayterm.c b/src/clayterm.c index e9a8a96..4e92864 100644 --- a/src/clayterm.c +++ b/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; } diff --git a/src/clayterm.h b/src/clayterm.h index 20bee69..0aa6ead 100644 --- a/src/clayterm.h +++ b/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 diff --git a/test/width.test.ts b/test/width.test.ts index 94a2083..35b7926 100644 --- a/test/width.test.ts +++ b/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); + }); + }); });