From a4502a6d002eb2cd47e88e3ce9d79d3aa837b9c7 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 9 Feb 2026 14:30:35 -0800 Subject: [PATCH] Impelement cursor movement after image placement --- .../src/kitty/KittyGraphicsHandler.ts | 62 ++++++------ .../src/kitty/KittyGraphicsTypes.test.ts | 10 ++ .../src/kitty/KittyGraphicsTypes.ts | 6 +- addons/addon-image/test/KittyGraphics.test.ts | 97 ++++++------------- 4 files changed, 81 insertions(+), 94 deletions(-) diff --git a/addons/addon-image/src/kitty/KittyGraphicsHandler.ts b/addons/addon-image/src/kitty/KittyGraphicsHandler.ts index 0262af6a..a6d46f5b 100644 --- a/addons/addon-image/src/kitty/KittyGraphicsHandler.ts +++ b/addons/addon-image/src/kitty/KittyGraphicsHandler.ts @@ -373,7 +373,7 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler { const id = cmd.id ?? this._nextImageId - 1; const image = this._images.get(id); if (image) { - const result = this._displayImage(image, cmd.columns, cmd.rows); + const result = this._displayImage(image, cmd); if (cmd.id !== undefined) { return (result as Promise).then(r => { this._sendResponse(id, 'OK', cmd.quiet ?? 0); @@ -471,31 +471,39 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler { // Image display - private _displayImage(image: IKittyImageData, columns?: number, rows?: number): boolean | Promise { - return this._decodeAndDisplay(image, columns, rows) + private _displayImage(image: IKittyImageData, cmd: IKittyCommand): boolean | Promise { + return this._decodeAndDisplay(image, cmd) .then(() => true) .catch(() => true); } - private async _decodeAndDisplay(image: IKittyImageData, columns?: number, rows?: number): Promise { + private async _decodeAndDisplay(image: IKittyImageData, cmd: IKittyCommand): Promise { const bitmap = await this._createBitmap(image); + const cw = this._renderer.dimensions?.css.cell.width || CELL_SIZE_DEFAULT.width; + const ch = this._renderer.dimensions?.css.cell.height || CELL_SIZE_DEFAULT.height; + + // Per spec: c/r default to image's natural cell dimensions + const imgCols = cmd.columns ?? Math.ceil(bitmap.width / cw); + const imgRows = cmd.rows ?? Math.ceil(bitmap.height / ch); + let w = bitmap.width; let h = bitmap.height; - if (columns || rows) { - const cw = this._renderer.dimensions?.css.cell.width || CELL_SIZE_DEFAULT.width; - const ch = this._renderer.dimensions?.css.cell.height || CELL_SIZE_DEFAULT.height; - - if (columns) w = columns * cw; - if (rows) h = rows * ch; - - if (columns && !rows) h = Math.round(w * (bitmap.height / bitmap.width)); - else if (rows && !columns) w = Math.round(h * (bitmap.width / bitmap.height)); + // Scale bitmap to fit placement rectangle when c/r are specified + if (cmd.columns !== undefined || cmd.rows !== undefined) { + w = Math.round(imgCols * cw); + h = Math.round(imgRows * ch); } if (w * h > this._opts.pixelLimit) return; + // Save cursor position before addImage modifies it + const buffer = this._coreTerminal._core.buffer; + const savedX = buffer.x; + const savedY = buffer.y; + const savedYbase = buffer.ybase; + let storageId: number; if (w !== bitmap.width || h !== bitmap.height) { const resized = await createImageBitmap(bitmap, { resizeWidth: w, resizeHeight: h }); @@ -505,21 +513,19 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler { } this._kittyIdToStorageId.set(image.id, storageId); - // TODO: Implement cursor movement per Kitty graphics protocol spec - // Per spec: "After placing an image on the screen the cursor must be moved to the - // right by the number of cols in the image placement rectangle and down by the - // number of rows in the image placement rectangle." - // - // Default behavior (C=0 or unspecified): Move cursor by cols/rows - // With C=1: Don't move cursor at all - // - // Implementation would need: - // 1. Get placement.C value (cursor movement policy) - // 2. Calculate cols = placement.columns || Math.ceil(w / cellWidth) - // 3. Calculate rows = placement.rows || Math.ceil(h / cellHeight) - // 4. If C !== 1: Move cursor right by cols and down by rows - // this._bufferService.buffer.x += cols; - // this._bufferService.buffer.y += rows; + // Kitty cursor movement + // Per spec: cursor placed at first column after last image column, + // on the last row of the image. C=1 means don't move cursor. + if (cmd.cursorMovement === 1) { + // C=1: restore cursor to position before image was placed + const scrolled = buffer.ybase - savedYbase; + buffer.x = savedX; + buffer.y = Math.max(savedY - scrolled, 0); + } else { + // Default (C=0): advance cursor horizontally past the image + // addImage already positioned cursor on the last row via lineFeeds + buffer.x = Math.min(savedX + imgCols, this._coreTerminal.cols); + } } /** diff --git a/addons/addon-image/src/kitty/KittyGraphicsTypes.test.ts b/addons/addon-image/src/kitty/KittyGraphicsTypes.test.ts index 1bbfac67..ac79c09a 100644 --- a/addons/addon-image/src/kitty/KittyGraphicsTypes.test.ts +++ b/addons/addon-image/src/kitty/KittyGraphicsTypes.test.ts @@ -64,6 +64,16 @@ describe('KittyGraphicsTypes', () => { assert.strictEqual(cmd.compression, 'z'); }); + it('should parse cursor movement key', () => { + const cmd = parseKittyCommand('a=T,f=100,C=1'); + assert.strictEqual(cmd.cursorMovement, 1); + }); + + it('should parse cursor movement key C=0', () => { + const cmd = parseKittyCommand('a=T,f=100,C=0'); + assert.strictEqual(cmd.cursorMovement, 0); + }); + it('should parse x and y offset', () => { const cmd = parseKittyCommand('a=T,x=10,y=20'); assert.strictEqual(cmd.x, 10); diff --git a/addons/addon-image/src/kitty/KittyGraphicsTypes.ts b/addons/addon-image/src/kitty/KittyGraphicsTypes.ts index d4e1fe79..f49df028 100644 --- a/addons/addon-image/src/kitty/KittyGraphicsTypes.ts +++ b/addons/addon-image/src/kitty/KittyGraphicsTypes.ts @@ -68,7 +68,9 @@ export const enum KittyKey { // Compression type (z=zlib). This is essential for chunking larger images. COMPRESSION = 'o', // Quiet mode (1=suppress OK responses, 2=suppress error responses) - QUIET = 'q' + QUIET = 'q', + // Cursor movement policy (0=move cursor after image, 1=don't move cursor) + CURSOR_MOVEMENT = 'C' } // Pixel format constants @@ -92,6 +94,7 @@ export interface IKittyCommand { rows?: number; more?: number; quiet?: number; + cursorMovement?: number; compression?: string; payload?: string; } @@ -160,6 +163,7 @@ export function parseKittyCommand(data: string): IKittyCommand { case KittyKey.ROWS: cmd.rows = numValue; break; case KittyKey.MORE: cmd.more = numValue; break; case KittyKey.QUIET: cmd.quiet = numValue; break; + case KittyKey.CURSOR_MOVEMENT: cmd.cursorMovement = numValue; break; } } diff --git a/addons/addon-image/test/KittyGraphics.test.ts b/addons/addon-image/test/KittyGraphics.test.ts index 2ddd5eb4..063cc1ac 100644 --- a/addons/addon-image/test/KittyGraphics.test.ts +++ b/addons/addon-image/test/KittyGraphics.test.ts @@ -437,19 +437,18 @@ test.describe('Kitty Graphics Protocol', () => { test.describe('Cursor positioning', () => { // NOTE: Current tests document ACTUAL behavior (MVP - cursor doesn't move) - // Kitty spec says cursor SHOULD move by cols/rows unless C=1 is specified - // See skipped tests below for spec-compliant behavior + // Per Kitty spec: cursor placed at first column after last image column, + // on the last row of the image. C=1 means don't move cursor. - test('cursor remains at origin after transmit and display (a=T) - CURRENT MVP BEHAVIOR', async () => { - // TODO: This test documents current incomplete behavior - // Per Kitty spec, cursor should move, but MVP implementation doesn't move it + test('cursor advances past 1x1 image', async () => { const cursorBefore = await getCursor(); const seq = `\x1b_Ga=T,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`; await ctx.proxy.write(seq); await timeout(100); const cursorAfter = await getCursor(); deepStrictEqual(cursorBefore, [0, 0]); - deepStrictEqual(cursorAfter, [0, 0]); + // 1x1 pixel image occupies 1 column, cursor advances past it + deepStrictEqual(cursorAfter, [1, 0]); }); test('cursor advances with text before image', async () => { @@ -459,121 +458,89 @@ test.describe('Kitty Graphics Protocol', () => { await ctx.proxy.write(`\x1b_Ga=T,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`); await timeout(100); - // Cursor should remain at position after "Hello" - deepStrictEqual(await getCursor(), [5, 0]); + // Cursor advances 1 column past the image + deepStrictEqual(await getCursor(), [6, 0]); }); test('cursor advances with text after image', async () => { await ctx.proxy.write(`\x1b_Ga=T,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`); await timeout(100); - deepStrictEqual(await getCursor(), [0, 0]); + // Cursor at column 1 (past 1-col image) + deepStrictEqual(await getCursor(), [1, 0]); await ctx.proxy.write('World'); - deepStrictEqual(await getCursor(), [5, 0]); + deepStrictEqual(await getCursor(), [6, 0]); }); test('cursor position with multiple images on same line', async () => { await ctx.proxy.write(`\x1b_Ga=T,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`); await timeout(50); - deepStrictEqual(await getCursor(), [0, 0]); + deepStrictEqual(await getCursor(), [1, 0]); await ctx.proxy.write('###'); - deepStrictEqual(await getCursor(), [3, 0]); + deepStrictEqual(await getCursor(), [4, 0]); + // 3x1 pixel image: ceil(3/cellWidth)=1 column await ctx.proxy.write(`\x1b_Ga=T,f=100;${KITTY_RGB_3X1_BASE64}\x1b\\`); await timeout(50); - deepStrictEqual(await getCursor(), [3, 0]); + deepStrictEqual(await getCursor(), [5, 0]); }); test('cursor advances on newline after image', async () => { await ctx.proxy.write(`\x1b_Ga=T,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`); await timeout(100); - deepStrictEqual(await getCursor(), [0, 0]); + deepStrictEqual(await getCursor(), [1, 0]); await ctx.proxy.write('\n'); - deepStrictEqual(await getCursor(), [0, 1]); + deepStrictEqual(await getCursor(), [1, 1]); }); - test('cursor with placement at specific column (C key)', async () => { - // Move cursor to column 10 - await ctx.proxy.write('\x1b[11G'); // CHA - move to column 10 (1-indexed) - deepStrictEqual(await getCursor(), [10, 0]); - - // Place image with column specification - await ctx.proxy.write(`\x1b_Ga=T,f=100,C=5;${KITTY_BLACK_1X1_BASE64}\x1b\\`); - await timeout(100); - - // Cursor should still be at column 10 (Kitty protocol doesn't move cursor) - deepStrictEqual(await getCursor(), [10, 0]); - }); - - // ============================================================================ - // SPEC-COMPLIANT CURSOR MOVEMENT TESTS (Currently failing - to be implemented) - // ============================================================================ - // Per Kitty spec: "After placing an image on the screen the cursor must be - // moved to the right by the number of cols in the image placement rectangle - // and down by the number of rows in the image placement rectangle." - - test.skip('cursor should move right by cols when image placed (SPEC BEHAVIOR)', async () => { - // TODO: Implement cursor movement per Kitty spec - // When c=5 (5 columns), cursor should move right by 5 - const dim = await getDimensions(); + test('cursor should move right by cols when c specified', async () => { + // c=5: image displayed over 5 columns, r auto = ceil(1/cellHeight) = 1 await ctx.proxy.write(`\x1b_Ga=T,f=100,c=5;${KITTY_BLACK_1X1_BASE64}\x1b\\`); await timeout(100); - // Cursor should move right by 5 columns deepStrictEqual(await getCursor(), [5, 0]); }); - test.skip('cursor should move down by rows when image placed (SPEC BEHAVIOR)', async () => { - // TODO: Implement cursor movement per Kitty spec - // When r=3 (3 rows), cursor should move down by 3 - const dim = await getDimensions(); + test('cursor should move down by rows when r specified', async () => { + // r=3: image displayed over 3 rows, c auto = ceil(1/cellWidth) = 1 await ctx.proxy.write(`\x1b_Ga=T,f=100,r=3;${KITTY_BLACK_1X1_BASE64}\x1b\\`); await timeout(100); - // Cursor should move down by 3 rows - deepStrictEqual(await getCursor(), [0, 3]); + // Cursor at first column after image (col 1), on last row (row 2) + deepStrictEqual(await getCursor(), [1, 2]); }); - test.skip('cursor should move by cols AND rows when both specified (SPEC BEHAVIOR)', async () => { - // TODO: Implement cursor movement per Kitty spec + test('cursor should move by cols AND rows when both specified', async () => { await ctx.proxy.write(`\x1b_Ga=T,f=100,c=4,r=2;${KITTY_BLACK_1X1_BASE64}\x1b\\`); await timeout(100); - // Cursor should move right by 4 cols and down by 2 rows - deepStrictEqual(await getCursor(), [4, 2]); + // cursor at (4, 1): past 4 columns, on last row (row 1) + deepStrictEqual(await getCursor(), [4, 1]); }); - test.skip('cursor should NOT move when C=1 is specified (SPEC BEHAVIOR)', async () => { - // TODO: Implement cursor movement per Kitty spec - // C=1 means "no cursor movement" + test('cursor should NOT move when C=1 is specified', async () => { await ctx.proxy.write(`\x1b_Ga=T,f=100,c=5,r=3,C=1;${KITTY_BLACK_1X1_BASE64}\x1b\\`); await timeout(100); - // With C=1, cursor should stay at origin even though c=5,r=3 + // C=1: cursor stays at origin deepStrictEqual(await getCursor(), [0, 0]); }); - test.skip('cursor should calculate cols/rows from image size when not specified (SPEC BEHAVIOR)', async () => { - // TODO: Implement cursor movement per Kitty spec - // When c and r are not specified, they should be calculated from image size + test('cursor should calculate cols/rows from image size when not specified', async () => { const dim = await getDimensions(); - // 3x1 image at default cell size should occupy certain columns + // 3x1 pixel image: cols = ceil(3/cellWidth), rows = ceil(1/cellHeight) await ctx.proxy.write(`\x1b_Ga=T,f=100;${KITTY_RGB_3X1_BASE64}\x1b\\`); await timeout(100); - // Cursor should move based on image pixel size / cell size - // For 3x1 pixel image, this would be Math.ceil(3/cellWidth) cols and Math.ceil(1/cellHeight) rows const expectedCols = Math.ceil(3 / dim.cellWidth); - const expectedRows = Math.ceil(1 / dim.cellHeight); const cursor = await getCursor(); - // For a 3x1 pixel image, cursor should move at least 1 column and row - // Exact values depend on cell dimensions - strictEqual(cursor[0] >= 1, true, 'cursor should move at least 1 column'); - strictEqual(cursor[1] >= 1, true, 'cursor should move at least 1 row'); + // Cursor advances past image columns, stays on row 0 (single row image) + strictEqual(cursor[0], expectedCols, 'cursor should advance by image columns'); + strictEqual(cursor[1], 0, 'cursor should stay on row 0 for single-row image'); }); });