eiac/xterm.js
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Fix scroll off-by-one in synthetic flood; guard repro against buffer scroll

Browse Source 
generateColoredAsciiFlood emitted a trailing \r\n after its final cell, so a
flood whose last line lands on the bottom row scrolled the screen. In the
alt-buffer repro this destroyed the pinned header row in the BUFFER, making
the 255/255 signature diff a content change, not renderer corruption. The
single-terminal merge path is actually healthy: with the scroll removed the
same 30-chunk merge-heavy workload renders the header with zero drift.

- generateColoredAsciiFlood: no linefeed after the final cell
- WebglReproExperiment: now a green guard; asserts row-1 buffer text first so
  a content bug can never masquerade as corruption again
- WebglVisual: accurate comments; erase rewritten lines so leftover prefill
  text doesn't look like corruption
- WebglSingleTermStress: replace stale EXPECTED-RED comment (test passes)
This commit is contained in:
Anthony Kim
2026-07-04 12:46:27 -07:00
parent e3399b1db8
commit a56dc02cc7
4 changed files with 50 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/addon-webgl/test/WebglReproExperiment.test.ts
+23 -6
View File
@@ -1,32 +1,43 @@
/**
* Copyright (c) 2025 The xterm.js authors. All rights reserved.
* @license MIT
* EXPERIMENT: try to reproduce vscode#322756 garbled single-terminal render.
*
* Single-terminal guard for the vscode#322756 "garbled glyph" class: a pinned
* alt-buffer header must render identically while the body churns enough
* distinct colored glyphs to force texture-atlas page merges.
*
* Note this scenario is GREEN on current master: the single-terminal mid-frame
* merge race was fixed upstream (merge-retry loop in WebglRenderer.renderRows +
* globally monotonic AtlasPage versions). The test asserts the buffer content
* first so a content bug (e.g. an accidental scroll from the churn stream) can
* never masquerade as renderer corruption.
*/
import test, { expect } from '@playwright/test';
import { ITestContext, createTestContext, openTerminal } from '../../../test/playwright/TestUtils';
import { captureRowSignatures, expectSignatureMatches, getAtlasPageCount, loadWebglStrict, setMaxAtlasPages, waitForRender, writeAndWaitForRender } from '../../../test/playwright/RendererTestUtils';
import { captureRowSignatures, expectSignatureMatches, getAtlasPageCount, loadWebglStrict, setMaxAtlasPages, writeAndWaitForRender } from '../../../test/playwright/RendererTestUtils';
import { generateColoredAsciiFlood } from '../../../test/playwright/SyntheticTui';
const HEADER = ' HEADER_REF_0123456789';
let ctx: ITestContext;
test.beforeAll(async ({ browser }) => { ctx = await createTestContext(browser); });
test.afterAll(async () => await ctx.page.close());
test.describe('repro #322756', () => {
test.describe('single-terminal merge integrity (#322756 class)', () => {
test.skip(({ browserName }) => browserName !== 'chromium');
test.describe.configure({ timeout: 60000 });
test('header survives merge race with per-chunk sync', async () => {
test('pinned header renders identically across 30 merge-heavy body redraws', async () => {
await openTerminal(ctx, { cols: 80, rows: 24 });
await loadWebglStrict(ctx);
await setMaxAtlasPages(ctx, 4);
const cols = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
// Alt-buffer TUI: pinned header on row 1, body churns below (like Copilot CLI).
await writeAndWaitForRender(ctx, '\x1b[?1049h\x1b[H HEADER_REF_0123456789');
await writeAndWaitForRender(ctx, `\x1b[?1049h\x1b[H${HEADER}`);
const ref = await captureRowSignatures(ctx, 1, cols);
let maxDiff = 0;
for (let c = 0; c < 30; c++) {
// redraw header + churn body rows 2..24 with unique glyphs, header stays put
// Redraw body rows 2..24 with unique glyphs; the header row is never touched.
let frame = '\x1b[2;1H';
frame += generateColoredAsciiFlood(23 * 78, c * 23 * 78);
await writeAndWaitForRender(ctx, frame);
@@ -37,6 +48,12 @@ test.describe('repro #322756', () => {
}
}
console.error('[repro] pages=', await getAtlasPageCount(ctx), 'headerMaxDiff=', maxDiff);
// Guard: the header must still be in the buffer. If this fails the churn
// stream scrolled the screen and any pixel diff is meaningless.
const row1Text = await ctx.page.evaluate(() => (window as any).term.buffer.active.getLine(0)?.translateToString(true));
expect(row1Text, 'churn stream must not scroll the pinned header out of the buffer').toBe(HEADER);
const after = await captureRowSignatures(ctx, 1, cols);
for (let i = 0; i < cols.length; i++) {
expectSignatureMatches(after[i], ref[i], 14, `header col ${cols[i]} garbled`);
addons/addon-webgl/test/WebglSingleTermStress.test.ts
+6 -5
View File
@@ -37,11 +37,12 @@ test.describe('WebGL single-terminal heavy render + merge', () => {
await loadWebglStrict(ctx);
});
test('heavy merge churn must not crash the GlyphRenderer (xtermjs#322756)', async () => {
// EXPECTED RED: with a small page cap, atlas merges shrink the pages array
// while GlyphRenderer.render still indexes the old page count -> throws
// "Cannot read properties of undefined (reading 'version')" and the terminal
// garbles. Capture the crash via pageerror.
test('heavy merge churn must not crash the GlyphRenderer (vscode#322756)', async () => {
// Guard: heavy merges must not throw from GlyphRenderer.render. Note the
// page cap here is lowered AFTER the renderer was built, so _atlasTextures
// keeps its original (large) length and this test cannot hit the
// page-overflow crash; that scenario needs the cap lowered BEFORE loading
// the addon and is covered separately (WebglAtlasOverflow.test.ts).
const errors: string[] = [];
const onError = (e: Error): void => { errors.push(e.message); };
ctx.page.on('pageerror', onError);
addons/addon-webgl/test/WebglVisual.test.ts
+15 -10
View File
@@ -1,20 +1,24 @@
/**
* Copyright (c) 2025 The xterm.js authors. All rights reserved.
* @license MIT
* Produces a strong BEFORE/AFTER visual of single-terminal #322756 garble:
* a plain white header that should stay readable, body churns colored text
* forcing atlas merges. Saves before.png / after.png for manual viewing.
* Produces BEFORE/AFTER screenshots of a single terminal under merge-heavy
* churn for manual inspection (before.png / after.png, mono-*.png). On current
* master the single-terminal merge path is healthy, so the pinned header lines
* are expected to remain readable in the "after" images; a corrupted header in
* these captures indicates a renderer regression. Note the mono "after" body is
* SUPPOSED to look like glyph soup — that is the mixed box-drawing/braille
* payload rendered faithfully, not corruption.
*/
import test from '@playwright/test';
import { ITestContext, createTestContext, openTerminal } from '../../../test/playwright/TestUtils';
import { loadWebglStrict, setMaxAtlasPages, waitForRender, writeAndWaitForRender } from '../../../test/playwright/RendererTestUtils';
import { loadWebglStrict, setMaxAtlasPages, writeAndWaitForRender } from '../../../test/playwright/RendererTestUtils';
import { generateColoredAsciiFlood, generateMixedGlyphBlock } from '../../../test/playwright/SyntheticTui';
test.describe('visual #322756', () => {
test.skip(({ browserName }) => browserName !== 'chromium');
test.describe.configure({ timeout: 60000 });
test('capture before/after header garble', async ({ browser }) => {
test('capture before/after pinned header under merge churn', async ({ browser }) => {
const ctx: ITestContext = await createTestContext(browser);
await openTerminal(ctx, { cols: 80, rows: 24 });
await loadWebglStrict(ctx);
@@ -30,8 +34,9 @@ test.describe('visual #322756', () => {
await ctx.page.close();
});
// Closer to vscode#322756: prefill realistic CLI text, trigger ONE merge,
// rewrite a couple lines -> some lines intact, most garble sparsely on dark bg.
// Issue-like framing: prefill realistic CLI text, force merges with a mixed
// glyph payload, then rewrite the top lines clean. The rewritten header lines
// must stay readable in mono-after.png.
test('capture mono before/after (issue-like)', async ({ browser }) => {
const ctx: ITestContext = await createTestContext(browser);
await openTerminal(ctx, { cols: 80, rows: 24 });
@@ -46,14 +51,14 @@ test.describe('visual #322756', () => {
}
await writeAndWaitForRender(ctx, pre);
await screen.screenshot({ path: 'out-esbuild-test/playwright/mono-before.png' });
// Overflow the atlas (cap=4) so pages MERGE; churn most rows so their cells
// hold stale slots, then rewrite a few lines clean (like scroll/redraw).
// Overflow the atlas (cap=4) so pages MERGE while most rows keep their cells,
// then rewrite a few lines clean (like scroll/redraw).
for (let c = 0; c < 12; c++) {
let frame = '\x1b[3;1H';
frame += generateMixedGlyphBlock(c + 1, 18, 78);
await writeAndWaitForRender(ctx, frame);
}
await writeAndWaitForRender(ctx, '\x1b[1;1H $ Shell grep installed Copilot CLI binary - 53 lines\r\n Thought for 1s');
await writeAndWaitForRender(ctx, '\x1b[1;1H\x1b[2K $ Shell grep installed Copilot CLI binary - 53 lines\r\n\x1b[2K Thought for 1s');
await screen.screenshot({ path: 'out-esbuild-test/playwright/mono-after.png' });
await ctx.page.close();
});
test/playwright/SyntheticTui.ts
+6 -1
View File
@@ -177,6 +177,11 @@ export function generateMixedGlyphBlock(seed: number = 1, lines: number = 10, co
* Emits ASCII letters/words each in a distinct 256-color foreground+background.
* Every (char,color) pair is a unique atlas glyph, so this fills/merges atlas
* pages with realistic English text (no CJK). Deterministic.
*
* No `\r\n` is emitted after the final cell: when the flood's last line lands on
* the terminal's bottom row a trailing linefeed would scroll the screen (in the
* alt buffer this destroys pinned reference rows and makes glyph-integrity
* assertions fail for buffer-content reasons rather than rendering reasons).
* @param cells Number of colored cells to emit.
* @param offset Color offset so consecutive chunks use new color combos.
*/
@@ -188,7 +193,7 @@ export function generateColoredAsciiFlood(cells: number, offset: number = 0): st
const fg = (offset + i) % 256;
const bg = (offset + i * 7) % 256;
out += `\x1b[38;5;${fg}m\x1b[48;5;${bg}m${ch}`;
if ((i + 1) % 78 === 0) { out += '\x1b[0m\r\n'; }
if ((i + 1) % 78 === 0 && i + 1 < cells) { out += '\x1b[0m\r\n'; }
}
return out + '\x1b[0m';
}