Skip to content

Change image storage to use new ExtendedAttrs payload field #5879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PerBothner wants to merge 6 commits into
base: master
Choose a base branch
from
+72 −157
Open

Change image storage to use new ExtendedAttrs payload field #5879

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8883dc1
Change image storage to use new ExtendedAttrs payload field.
PerBothner May 13, 2026
eaa3a63
Use 'import type' in addons/addon-image/src/ImageStorage.ts
PerBothner May 21, 2026
235457c
Merge remote-tracking branch 'upstream/master' into image-ext
PerBothner May 23, 2026
7b66cd0
Add 'type' to another import in ImageStorage.ts.
PerBothner May 23, 2026
092b07d
Merge remote-tracking branch 'upstream' into image-ext
PerBothner Jun 7, 2026
fed1138
Merge branch 'master' into image-ext
PerBothner Jun 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
204 changes: 67 additions & 137 deletions addons/addon-image/src/ImageStorage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@

import { IDisposable } from '@xterm/xterm';
import { ImageRenderer } from './ImageRenderer';
import { ITerminalExt, IExtendedAttrsImage, IImageAddonOptions, IImageSpec, IBufferLineExt, BgFlags, Cell, Content, ICellSize, ExtFlags, Attributes, UnderlineStyle, ImageLayer } from './Types';
import { ITerminalExt, IImageAddonOptions, IImageSpec, ICellSize, ImageLayer } from './Types';
import { IBufferLine } from 'common/Types';
Comment thread
PerBothner marked this conversation as resolved.
Outdated
import { CellData } from 'common/buffer/CellData';
Comment thread
PerBothner marked this conversation as resolved.



// fallback default cell size
Expand All @@ -14,93 +17,12 @@ export const CELL_SIZE_DEFAULT: ICellSize = {
height: 14
};

/**
* Extend extended attribute to also hold image tile information.
*
* Object definition is copied from base repo to fully mimick its behavior.
* Image data is added as additional public properties `imageId` and `tileId`.
*/
class ExtendedAttrsImage implements IExtendedAttrsImage {
private _ext: number = 0;
public get ext(): number {
if (this._urlId) {
return (
(this._ext & ~ExtFlags.UNDERLINE_STYLE) |
(this.underlineStyle << 26)
);
}
return this._ext;
}
public set ext(value: number) { this._ext = value; }

public get underlineStyle(): UnderlineStyle {
// Always return the URL style if it has one
if (this._urlId) {
return UnderlineStyle.DASHED;
}
return (this._ext & ExtFlags.UNDERLINE_STYLE) >> 26;
}
public set underlineStyle(value: UnderlineStyle) {
this._ext &= ~ExtFlags.UNDERLINE_STYLE;
this._ext |= (value << 26) & ExtFlags.UNDERLINE_STYLE;
}

public get underlineColor(): number {
return this._ext & (Attributes.CM_MASK | Attributes.RGB_MASK);
}
public set underlineColor(value: number) {
this._ext &= ~(Attributes.CM_MASK | Attributes.RGB_MASK);
this._ext |= value & (Attributes.CM_MASK | Attributes.RGB_MASK);
}

public get underlineVariantOffset(): number {
const val = (this._ext & ExtFlags.VARIANT_OFFSET) >> 29;
if (val < 0) {
return val ^ 0xFFFFFFF8;
}
return val;
}
public set underlineVariantOffset(value: number) {
this._ext &= ~ExtFlags.VARIANT_OFFSET;
this._ext |= (value << 29) & ExtFlags.VARIANT_OFFSET;
}

private _urlId: number = 0;
public get urlId(): number {
return this._urlId;
}
public set urlId(value: number) {
this._urlId = value;
}

class ImageTileInfo {
constructor(
ext: number = 0,
urlId: number = 0,
public imageId = -1,
public tileId = -1
) {
this._ext = ext;
this._urlId = urlId;
}

public clone(): IExtendedAttrsImage {
/**
* Technically we dont need a clone variant of ExtendedAttrsImage,
* as we never clone a cell holding image data.
* Note: Clone is only meant to be used by the InputHandler for
* sticky attributes, which is never the case for image data.
* We still provide a proper clone method to reflect the full ext attr
* state in case there are future use cases for clone.
*/
return new ExtendedAttrsImage(this._ext, this._urlId, this.imageId, this.tileId);
}

public isEmpty(): boolean {
return this.underlineStyle === UnderlineStyle.NONE && this._urlId === 0 && this.imageId === -1;
public tileId = -1) {
}
}
const EMPTY_ATTRS = new ExtendedAttrsImage();


/**
* ImageStorage - extension of CoreTerminal:
Expand All @@ -122,6 +44,7 @@ export class ImageStorage implements IDisposable {
private _needsFullClear = false;
// hard limit of stored pixels (fallback limit of 10 MB)
private _pixelLimit: number = 2500000;
private _workCell: CellData = new CellData();

private _viewportMetrics: { cols: number, rows: number };
public onImageAdded: (() => void) | undefined;
Expand Down Expand Up @@ -271,10 +194,10 @@ export class ImageStorage implements IDisposable {

this._terminal._core._inputHandler._dirtyRowTracker.markDirty(buffer.y);
for (let row = 0; row < rows; ++row) {
const line = buffer.lines.get(buffer.y + buffer.ybase);
const line = buffer.lines.get(buffer.y + buffer.ybase)!;
for (let col = 0; col < cols; ++col) {
if (offset + col >= termCols) break;
this._writeToCell(line as IBufferLineExt, offset + col, imageId, row * cols + col);
this._writeToCell(line, offset + col, imageId, row * cols + col);
tileCount++;
}
if (scrolling) {
Expand Down Expand Up @@ -417,22 +340,18 @@ export class ImageStorage implements IDisposable {
const placeholderCalls: { col: number, row: number, count: number }[] = [];

// walk all cells in viewport and collect tiles found
// Note: We check _extendedAttrs directly (not just HAS_EXTENDED flag)
// Note: We check extended directly (not just HAS_EXTENDED flag)
// because text writes clear the BG flag but leave image tile data intact.
// This lets top-layer images survive text overwrites (kitty C=1 behavior).
for (let row = start; row <= end; ++row) {
const line = buffer.lines.get(row + buffer.ydisp) as IBufferLineExt;
const line = buffer.lines.get(row + buffer.ydisp);
if (!line) return;
const workCell = this._workCell;
for (let col = 0; col < cols; ++col) {
let e: IExtendedAttrsImage;
if (line.getBg(col) & BgFlags.HAS_EXTENDED) {
e = line._extendedAttrs[col] ?? EMPTY_ATTRS;
} else {
const maybeImg = line._extendedAttrs[col] as IExtendedAttrsImage | undefined;
if (!maybeImg || maybeImg.imageId === undefined || maybeImg.imageId === -1) {
continue;
}
e = maybeImg;
line.loadCell(col, workCell);

@Tyriar Tyriar May 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit of a code smell that we touch private API here at all. If the image addon only needs to attach extended data, could we make a real API to allow attaching data using an arbitrary key?

interface IBufferCell {
  setCustomData(key: string, value: object);
  getCustomData(key: string): object | undefined
}

Even adding this public API to xterm.d.ts and keeping the internals access as-is would be preferable to bringing in loadCell/CellData imo.

@jerch does this look like the shape you would expect?

@PerBothner PerBothner May 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setCustomData/getCustomData functionality is certainly nice and general - but it could be expensive. Conceptually we're adding a separate Map for each cell - or at least one for every cell with HAS_EXTENDED. Presumably we'd want keep ExtendedAttrs (or its replacement) immutable.

We can replace the loadCell with the public getCell method - but ImageStore._writeToCell need the ability to modify a cell, currently done with setCell. What is the public API equivalent? From what I understand, getCell returns a copy of the state of a cell. Calling setCustomData presumably only updates the copy - we need to write back to the BufferLine.

@PerBothner PerBothner May 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment below for a (slightly) more detailed suggestion. I've focused on the internal API; we can discuss the public API once the concepts are clear.

let e = workCell.hasExtendedAttrs() && workCell.extended.payload;
if (! (e instanceof ImageTileInfo)) {
continue;
}
const imageId = e.imageId;
if (imageId === undefined || imageId === -1) {
Expand All @@ -451,8 +370,9 @@ export class ImageStorage implements IDisposable {
* Also check _extendedAttrs directly for cells where text cleared HAS_EXTENDED.
*/
while (++col < cols) {
const nextE = line._extendedAttrs[col] as IExtendedAttrsImage | undefined;
if (!nextE || nextE.imageId !== imageId || nextE.tileId !== startTile + count) {
line.loadCell(col, workCell);
const nextE = workCell.hasExtendedAttrs() && workCell.extended.payload;
if (! (nextE instanceof ImageTileInfo) || !nextE || nextE.imageId !== imageId || nextE.tileId !== startTile + count) {
break;
}
e = nextE;
Expand Down Expand Up @@ -503,10 +423,15 @@ export class ImageStorage implements IDisposable {
const buffer = this._terminal._core.buffer;
const rows = buffer.lines.length;
const oldCol = this._viewportMetrics.cols - 1;
const workCell = this._workCell;
for (let row = 0; row < rows; ++row) {
const line = buffer.lines.get(row) as IBufferLineExt;
if (line.getBg(oldCol) & BgFlags.HAS_EXTENDED) {
const e: IExtendedAttrsImage = line._extendedAttrs[oldCol] ?? EMPTY_ATTRS;
const line = buffer.lines.get(row) !;
line.loadCell(oldCol, workCell);
if (workCell.hasExtendedAttrs()) {
const e = workCell.extended.payload;
if (! (e instanceof ImageTileInfo)) {
continue;
}
const imageId = e.imageId;
if (imageId === undefined || imageId === -1) {
continue;
Expand All @@ -523,7 +448,7 @@ export class ImageStorage implements IDisposable {
// expand only if right side is empty (nothing got wrapped from below)
let hasData = false;
for (let rightCol = oldCol + 1; rightCol > metrics.cols; ++rightCol) {
if (line._data[rightCol * Cell.SIZE + Cell.CONTENT] & Content.HAS_CONTENT_MASK) {
if (line.hasContent(rightCol)) {
hasData = true;
break;
}
Expand All @@ -535,7 +460,7 @@ export class ImageStorage implements IDisposable {
const end = Math.min(metrics.cols, tilesPerRow - (e.tileId % tilesPerRow) + oldCol);
let lastTile = e.tileId;
for (let expandCol = oldCol + 1; expandCol < end; ++expandCol) {
this._writeToCell(line as IBufferLineExt, expandCol, imageId, ++lastTile);
this._writeToCell(line, expandCol, imageId, ++lastTile);
imgSpec.tileCount++;
}
}
Expand All @@ -549,10 +474,10 @@ export class ImageStorage implements IDisposable {
*/
public getImageAtBufferCell(x: number, y: number): HTMLCanvasElement | undefined {
const buffer = this._terminal._core.buffer;
const line = buffer.lines.get(y) as IBufferLineExt;
if (line && line.getBg(x) & BgFlags.HAS_EXTENDED) {
const e: IExtendedAttrsImage = line._extendedAttrs[x] ?? EMPTY_ATTRS;
if (e.imageId && e.imageId !== -1) {
const line = buffer.lines.get(y);
if (line && line.loadCell(x, this._workCell).hasExtendedAttrs()) {
const e = this._workCell.extended.payload;
if (e instanceof ImageTileInfo && e.imageId && e.imageId !== -1) {
const orig = this._images.get(e.imageId)?.orig;
if (window.ImageBitmap && orig instanceof ImageBitmap) {
const canvas = ImageRenderer.createCanvas(window.document, orig.width, orig.height);
Expand All @@ -569,10 +494,10 @@ export class ImageStorage implements IDisposable {
*/
public extractTileAtBufferCell(x: number, y: number): HTMLCanvasElement | undefined {
const buffer = this._terminal._core.buffer;
const line = buffer.lines.get(y) as IBufferLineExt;
if (line && line.getBg(x) & BgFlags.HAS_EXTENDED) {
const e: IExtendedAttrsImage = line._extendedAttrs[x] ?? EMPTY_ATTRS;
if (e.imageId && e.imageId !== -1 && e.tileId !== -1) {
const line = buffer.lines.get(y);
if (line && line.loadCell(x, this._workCell).hasExtendedAttrs()) {
const e = this._workCell.extended.payload;
if (e instanceof ImageTileInfo && e.imageId && e.imageId !== -1 && e.tileId !== -1) {
const spec = this._images.get(e.imageId);
if (spec) {
return this._renderer.extractTile(spec, e.tileId);
Expand Down Expand Up @@ -600,31 +525,34 @@ export class ImageStorage implements IDisposable {
return used - current;
}

private _writeToCell(line: IBufferLineExt, x: number, imageId: number, tileId: number): void {
if (line._data[x * Cell.SIZE + Cell.BG] & BgFlags.HAS_EXTENDED) {
const old = line._extendedAttrs[x];
if (old) {
if (old.imageId !== undefined) {
// found an old ExtendedAttrsImage, since we know that
// they are always isolated instances (single cell usage),
// we can re-use it and just update their id entries
const oldSpec = this._images.get(old.imageId);
if (oldSpec) {
// early eviction for in-viewport overwrites
oldSpec.tileCount--;
}
old.imageId = imageId;
old.tileId = tileId;
return;
private _writeToCell(line: IBufferLine, x: number, imageId: number, tileId: number): void {
const workCell = this._workCell;
line.loadCell(x, workCell);
if (this._workCell.hasExtendedAttrs()) {
const old = workCell.extended.payload;
if (old instanceof ImageTileInfo) {
// found an old ExtendedAttrsImage, since we know that
// they are always isolated instances (single cell usage),
// we can re-use it and just update their id entries
const oldSpec = this._images.get(old.imageId);
if (oldSpec) {
// early eviction for in-viewport overwrites
oldSpec.tileCount--;
}
// found a plain ExtendedAttrs instance, clone it to new entry
line._extendedAttrs[x] = new ExtendedAttrsImage(old.ext, old.urlId, imageId, tileId);
old.imageId = imageId;
old.tileId = tileId;
return;
}
// found a plain ExtendedAttrs instance
workCell.extended.payload = new ImageTileInfo(imageId, tileId);
return;
}
// fall-through: always create new ExtendedAttrsImage entry
line._data[x * Cell.SIZE + Cell.BG] |= BgFlags.HAS_EXTENDED;
line._extendedAttrs[x] = new ExtendedAttrsImage(0, 0, imageId, tileId);
const extattr = workCell.extended.clone();
extattr.payload = new ImageTileInfo(imageId, tileId);
workCell.extended = extattr;
workCell.updateExtended();
line.setCell(x, workCell);
}

private _evictOnAlternate(): void {
Expand All @@ -637,15 +565,17 @@ export class ImageStorage implements IDisposable {
// re-count tiles on whole buffer
const buffer = this._terminal._core.buffer;
for (let y = 0; y < this._terminal.rows; ++y) {
const line = buffer.lines.get(y) as IBufferLineExt;
const line = buffer.lines.get(y);
if (!line) {
continue;
}
const workCell = this._workCell;
for (let x = 0; x < this._terminal.cols; ++x) {
if (line._data[x * Cell.SIZE + Cell.BG] & BgFlags.HAS_EXTENDED) {
const imgId = line._extendedAttrs[x]?.imageId;
if (imgId) {
const spec = this._images.get(imgId);
line.loadCell(x, workCell);
if (workCell.hasExtendedAttrs()) {
const payload = workCell.extended.payload;
if (payload instanceof ImageTileInfo) {
const spec = this._images.get(payload.imageId);
if (spec) {
spec.tileCount++;
}
Expand Down
19 changes: 3 additions & 16 deletions addons/addon-image/src/Types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { IDisposable, IMarker, Terminal } from '@xterm/xterm';
import { Attributes, BgFlags, Content, ExtFlags, UnderlineStyle } from 'common/buffer/Constants';
import type { AttributeData } from 'common/buffer/AttributeData';
import type { IParams, IDcsHandler, IOscHandler, IApcHandler, IEscapeSequenceParser } from 'common/parser/Types';
import type { IBufferLine, IExtendedAttrs, IInputHandler } from 'common/Types';
import type { IInputHandler } from 'common/Types';
import type { ITerminal, ReadonlyColorSet } from 'browser/Types';
import type { IRenderDimensions } from 'browser/renderer/shared/Types';
import type { ICoreBrowserService, IRenderService, IThemeService } from 'browser/services/Services';
Expand Down Expand Up @@ -47,26 +47,14 @@ export interface IResetHandler {
reset(): void;
}

/* eslint-disable */
/**
* Stub into private interfaces.
* This should be kept in line with common libs.
* Any change made here should be replayed in the accessors test case to
* have a somewhat reliable testing against code changes in the core repo.
*/

// overloaded IExtendedAttrs to hold image refs
export interface IExtendedAttrsImage extends IExtendedAttrs {
imageId: number;
tileId: number;
clone(): IExtendedAttrsImage;
}

/* eslint-disable */
export interface IBufferLineExt extends IBufferLine {
_extendedAttrs: {[index: number]: IExtendedAttrsImage | undefined};
_data: Uint32Array;
}

interface IInputHandlerExt extends IInputHandler {
_parser: IEscapeSequenceParser;
_curAttrData: AttributeData;
Expand All @@ -77,6 +65,7 @@ interface IInputHandlerExt extends IInputHandler {
};
onRequestReset(handler: () => void): IDisposable;
}
/* eslint-enable */

export interface ICoreTerminalExt extends ITerminal {
_themeService: IThemeService | undefined;
Expand All @@ -88,8 +77,6 @@ export interface ICoreTerminalExt extends ITerminal {
export interface ITerminalExt extends Terminal {
_core: ICoreTerminalExt;
}
/* eslint-enable */


/**
* Some storage definitions.
Expand Down
1 change: 1 addition & 0 deletions src/common/Types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export interface IExtendedAttrs {
underlineColor: number;
underlineVariantOffset: number;
urlId: number;
payload: Object | undefined;
clone(): IExtendedAttrs;
isEmpty(): boolean;
}
Expand Down
3 changes: 2 additions & 1 deletion src/common/buffer/AttributeData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export class AttributeData implements IAttributeData {
*/
export class ExtendedAttrs implements IExtendedAttrs {
private _ext: number = 0;
public payload: Object | undefined;
public get ext(): number {
if (this._urlId) {
return (
Expand Down Expand Up @@ -206,6 +207,6 @@ export class ExtendedAttrs implements IExtendedAttrs {
* that needs to be persistant in the buffer.
*/
public isEmpty(): boolean {
return this.underlineStyle === UnderlineStyle.NONE && this._urlId === 0;
return this.underlineStyle === UnderlineStyle.NONE && this._urlId === 0 && this.payload === undefined;
}
}
Loading