From 4c72f7b02fbafd15d0cf843edece106714bcc5f7 Mon Sep 17 00:00:00 2001 From: Berke Güzel Date: Thu, 15 Oct 2026 12:38:32 +0300 Subject: Keep the snapshot unchanged when an update fails term_snapshot_update converted the viewport in place, so an allocation that failed midway left the snapshot empty and a renderer painted a blank frame. Split the update into two phases. Phase 1 makes every allocation the update needs and counts the arena words the converted rows add, then grows the arena once. Phase 2 writes the view, cells, keys, clusters, and selection, and cannot fail. A failed update now keeps the previous content, and a retry is an ordinary incremental update. invalidate() is removed. The term_snapshot_update comment and the correctness audit now say the snapshot keeps its previous content. The failure test fails each allocation in turn from the same starting state. After every failed call it compares the view, cells, row keys, selection spans, and cluster code points with the state before the call. A retry must succeed and match a fresh capture. The cases are a first update, a 32 by 16 screen of ZWJ families and regional-indicator flags (arena growth), a resize, a 300-row screen (scratch arrays), and a long scroll run. Co-Authored-By: Claude Haiku 5.5 --- src/snapshot.c | 283 +++++++++++++++++++++++++++++++-------------------------- 1 file changed, 155 insertions(+), 128 deletions(-) (limited to 'src') diff --git a/src/snapshot.c b/src/snapshot.c index 8dbb498..82a2805 100644 --- a/src/snapshot.c +++ b/src/snapshot.c @@ -46,6 +46,8 @@ void term_snapshot_free(struct term_snapshot *s) free(s); } +/* Make room for cols x rows. A failed call keeps the old contents, so the + * snapshot stays readable. */ static bool reserve(struct term_snapshot *s, int cols, int rows) { size_t cells = (size_t)cols * (size_t)rows; @@ -73,38 +75,13 @@ static bool reserve(struct term_snapshot *s, int cols, int rows) return true; } -/* The public contract makes every failed update an empty, retryable snapshot. */ -static void invalidate(struct term_snapshot *s) -{ - s->view = (struct term_view){ .palette = s->palette }; - s->source = 0; - s->arena_len = 0; - s->arena_dead = 0; - if (s->row_cap > 0) { - memset(s->keys, 0, (size_t)s->row_cap * sizeof(*s->keys)); - memset(s->row_clusters, 0, (size_t)s->row_cap * sizeof(*s->row_clusters)); - memset(s->selection, 0, (size_t)s->row_cap * sizeof(*s->selection)); - } -} - -static uint32_t arena_add(struct term_snapshot *s, const struct gstore *g, uint32_t handle, - bool *ok) +/* Append a cluster record. The caller reserved room for it. */ +static uint32_t arena_add(struct term_snapshot *s, const struct gstore *g, uint32_t handle) { uint32_t cps[GRAPHEME_MAX_LEN]; int n = term_gstore_expand(g, handle, cps, GRAPHEME_MAX_LEN); if (n <= 0) return ' '; - if (s->arena_len + (uint32_t)n + 1 > s->arena_cap) { - uint32_t cap = term_grow_cap32(s->arena_cap ? s->arena_cap * 2 : 256, - s->arena_len + (uint32_t)n + 1); - uint32_t *a = realloc(s->arena, cap * sizeof(*a)); - if (!a) { - *ok = false; - return 0; - } - s->arena = a; - s->arena_cap = cap; - } uint32_t off = s->arena_len; s->arena[off] = (uint32_t)n; memcpy(&s->arena[off + 1], cps, (size_t)n * sizeof(*cps)); @@ -112,10 +89,26 @@ static uint32_t arena_add(struct term_snapshot *s, const struct gstore *g, uint3 return TERM_TEXT_CLUSTER | off; } +/* Arena words that converting src adds. Counts what arena_add() writes. */ +static uint64_t row_words(const struct gstore *g, const struct cell *src, int cols) +{ + uint32_t cps[GRAPHEME_MAX_LEN]; + uint64_t words = 0; + for (int x = 0; x < cols; x++) { + uint32_t text = src[x].c; + if (!grapheme_is_handle(text)) + continue; + int n = term_gstore_expand(g, text, cps, GRAPHEME_MAX_LEN); + if (n > 0) + words += (uint64_t)n + 1; + } + return words; +} + /* Convert one stored row; returns the number of arena words it added. */ static uint32_t convert_row(struct term_snapshot *s, const struct gstore *g, struct term_cell *dst, - const struct cell *src, int cols, bool *ok) + const struct cell *src, int cols) { uint32_t before = s->arena_len; for (int x = 0; x < cols; x++) { @@ -124,11 +117,8 @@ static uint32_t convert_row(struct term_snapshot *s, const struct gstore *g, uint32_t text = c->c; if (TERM_EXPECT(text < 0x20, 0)) text = text ? ' ' : 0; - else if (TERM_EXPECT(grapheme_is_handle(text), 0)) { - text = arena_add(s, g, text, ok); - if (!*ok) - return 0; - } + else if (TERM_EXPECT(grapheme_is_handle(text), 0)) + text = arena_add(s, g, text); dst[x] = (struct term_cell){ .text = text, .fg = c->fg, @@ -140,43 +130,129 @@ static uint32_t convert_row(struct term_snapshot *s, const struct gstore *g, return s->arena_len - before; } +/* What one update does. Phase 1 fills it in and changes no snapshot field. */ +struct plan { + bool all; /* rebuild every row and reset the arena */ + int scrolled; /* rows the content moved up */ + const uint64_t *keys; /* key of each new row */ + const int *c0, *c1; /* selected columns of each row */ +}; + +/* Phase 2: write the plan into the snapshot. Nothing here can fail. */ +static void apply(struct term_snapshot *s, const struct term *t, const struct plan *p) +{ + int cols = t->cols, rows = t->rows; + int k = p->scrolled; + if (p->all) { + s->arena_len = 0; + s->arena_dead = 0; + } + + bool changed = p->all; + if (k > 0) { + for (int y = 0; y < k; y++) + s->arena_dead += s->row_clusters[y]; + memmove(s->cells, s->cells + (size_t)k * cols, + (size_t)(rows - k) * cols * sizeof(struct term_cell)); + memmove(s->row_clusters, s->row_clusters + k, + (size_t)(rows - k) * sizeof(uint32_t)); + memmove(s->keys, s->keys + k, + (size_t)(rows - k) * sizeof(uint64_t)); + memset(s->keys + (rows - k), 0, (size_t)k * sizeof(uint64_t)); + memset(s->row_clusters + (rows - k), 0, (size_t)k * sizeof(uint32_t)); + } + + for (int y = 0; y < rows; y++) { + uint64_t key = p->keys[y]; + if (!p->all && s->keys[y] == key) + continue; + if (!p->all) + s->arena_dead += s->row_clusters[y]; + const struct cell *src = term_screen_view_row(t, y); + s->row_clusters[y] = convert_row(s, t->gstore, s->cells + (size_t)y * cols, + src, cols); + s->keys[y] = key; + changed = true; + } + + for (int y = 0; y < rows; y++) { + struct term_span span = p->c1[y] >= p->c0[y] ? (struct term_span){ p->c0[y], p->c1[y] + 1 } + : (struct term_span){ 0 }; + if (p->all || span.begin != s->selection[y].begin || span.end != s->selection[y].end) + changed = true; + s->selection[y] = span; + } + + struct term_cursor cursor = term_cursor(t); + int view_offset = term_view_offset(t); + cursor.pos.y += view_offset; + if (cursor.pos.y >= rows) + cursor.visible = false; + const struct term_cursor *old = &s->view.cursor; + if (cursor.pos.x != old->pos.x || cursor.pos.y != old->pos.y || + cursor.style != old->style || cursor.visible != old->visible || + cursor.blink != old->blink || + s->view.scroll_offset != view_offset || + s->view.reverse_video != t->reverse_video) + changed = true; + if (p->all || s->view.palette_version != t->palette.version) { + memcpy(s->palette, t->palette.colors, sizeof(s->palette)); + s->view.palette_version = t->palette.version; + changed = true; + } + + s->source = t->serial; + s->view.size = (struct term_vector2i){ .x = cols, .y = rows }; + s->view.cursor = cursor; + s->view.scroll_offset = view_offset; + s->view.scrolled = k; + s->view.reverse_video = t->reverse_video; + if (changed || k) + s->view.version++; +} + +/* Phase 1 for one update. Allocates and checks everything that can fail, + * then hands the plan to apply(). A false return leaves the content as it was. */ static bool term_snapshot_update_inner(struct term_snapshot *s, struct term *t) { if (!s || !t) return false; int cols = t->cols, rows = t->rows; - if (!reserve(s, cols, rows)) { - invalidate(s); + bool ok = false; + bool resized = s->view.size.x != cols || s->view.size.y != rows; + bool all = resized || s->source != t->serial; + int scrolled = 0; + uint64_t need = 0; + + if (!reserve(s, cols, rows)) return false; + /* Scratch for rows above 256: new keys, then the selection spans. */ + uint64_t stack_keys[256]; + int stack[2][256]; + uint64_t *new_keys = stack_keys; + int *c0 = stack[0], *c1 = stack[1]; + void *heap = NULL; + if (rows > 256) { + heap = malloc((size_t)rows * (sizeof(*new_keys) + 2 * sizeof(*c0))); + if (!heap) + return false; + new_keys = heap; + c0 = (int *)(new_keys + rows); + c1 = c0 + rows; } + struct plan p = { .keys = new_keys, .c0 = c0, .c1 = c1 }; - bool resized = s->view.size.x != cols || s->view.size.y != rows; - bool all = resized || s->source != t->serial; /* Reclaim cluster records once most of the arena is garbage. */ if (s->arena_dead > 4096 && s->arena_dead * 2 > s->arena_len) all = true; - if (all) { - s->arena_len = 0; - s->arena_dead = 0; - } - bool changed = all; term_hist_prepare_rows(&t->hist, cols); - if (!term_hist_view_sync(&t->view, &t->hist, cols)) { - invalidate(s); - return false; - } + if (!term_hist_view_sync(&t->view, &t->hist, cols)) + goto done; term_hist_view_refine(&t->view, &t->hist, cols, rows); - uint64_t stack_keys[256]; - uint64_t *new_keys = rows <= 256 ? stack_keys : malloc((size_t)rows * sizeof(uint64_t)); - if (!new_keys) { - invalidate(s); - return false; - } for (int y = 0; y < rows; y++) new_keys[y] = term_screen_view_row_key(t, y); - int scrolled = 0; if (!all && rows > 1) { int cand_k = 0; /* Find candidate scroll offset k by checking where top rows moved. */ @@ -219,87 +295,38 @@ static bool term_snapshot_update_inner(struct term_snapshot *s, struct term *t) } } - if (scrolled > 0) { - int k = scrolled; - for (int y = 0; y < k; y++) - s->arena_dead += s->row_clusters[y]; - memmove(s->cells, s->cells + (size_t)k * cols, - (size_t)(rows - k) * cols * sizeof(struct term_cell)); - memmove(s->row_clusters, s->row_clusters + k, - (size_t)(rows - k) * sizeof(uint32_t)); - memmove(s->keys, s->keys + k, - (size_t)(rows - k) * sizeof(uint64_t)); - memset(s->keys + (rows - k), 0, (size_t)k * sizeof(uint64_t)); - memset(s->row_clusters + (rows - k), 0, (size_t)k * sizeof(uint32_t)); - } - + /* Arena words for the rows that apply() converts. Count them before + * anything changes, then grow the arena once so apply() cannot fail. */ + need = all ? 0 : s->arena_len; for (int y = 0; y < rows; y++) { - uint64_t key = new_keys[y]; - if (!all && s->keys[y] == key) + uint64_t kept = s->keys[y]; + if (scrolled) + kept = y + scrolled < rows ? s->keys[y + scrolled] : 0; + if (!all && kept == new_keys[y]) continue; - if (!all) - s->arena_dead += s->row_clusters[y]; - const struct cell *src = term_screen_view_row(t, y); - bool converted = true; - s->row_clusters[y] = convert_row(s, t->gstore, s->cells + (size_t)y * cols, - src, cols, &converted); - if (!converted) { - if (new_keys != stack_keys) - free(new_keys); - invalidate(s); - return false; - } - s->keys[y] = key; - changed = true; - } - if (new_keys != stack_keys) - free(new_keys); - - int stack[2][256]; - int *c0 = rows <= 256 ? stack[0] : malloc((size_t)rows * 2 * sizeof(int)); - if (!c0) { - invalidate(s); - return false; + need += row_words(t->gstore, term_screen_view_row(t, y), cols); } - int *c1 = rows <= 256 ? stack[1] : c0 + rows; - term_selection_view_spans(t, c0, c1); - for (int y = 0; y < rows; y++) { - struct term_span span = c1[y] >= c0[y] ? (struct term_span){ c0[y], c1[y] + 1 } - : (struct term_span){ 0 }; - if (all || span.begin != s->selection[y].begin || span.end != s->selection[y].end) - changed = true; - s->selection[y] = span; + /* Offsets must fit below TERM_TEXT_CLUSTER. */ + if (need > TERM_TEXT_CLUSTER) + goto done; + if (need > s->arena_cap) { + uint32_t cap = term_grow_cap32(s->arena_cap ? s->arena_cap * 2 : 256, (uint32_t)need); + uint32_t *a = realloc(s->arena, cap * sizeof(*a)); + if (!a) + goto done; + s->arena = a; + s->arena_cap = cap; } - if (c0 != stack[0]) - free(c0); - struct term_cursor cursor = term_cursor(t); - int view_offset = term_view_offset(t); - cursor.pos.y += view_offset; - if (cursor.pos.y >= rows) - cursor.visible = false; - const struct term_cursor *old = &s->view.cursor; - if (cursor.pos.x != old->pos.x || cursor.pos.y != old->pos.y || - cursor.style != old->style || cursor.visible != old->visible || - cursor.blink != old->blink || - s->view.scroll_offset != view_offset || - s->view.reverse_video != t->reverse_video) - changed = true; - if (all || s->view.palette_version != t->palette.version) { - memcpy(s->palette, t->palette.colors, sizeof(s->palette)); - s->view.palette_version = t->palette.version; - changed = true; - } + term_selection_view_spans(t, c0, c1); + p.all = all; + p.scrolled = scrolled; + apply(s, t, &p); + ok = true; - s->source = t->serial; - s->view.size = (struct term_vector2i){ .x = cols, .y = rows }; - s->view.cursor = cursor; - s->view.scroll_offset = view_offset; - s->view.scrolled = scrolled; - s->view.reverse_video = t->reverse_video; - if (changed || scrolled) - s->view.version++; - return true; +done: + free(heap); + return ok; } bool term_snapshot_update(struct term_snapshot *s, struct term *t) -- cgit v1.2.3