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 1 commit into
base: master
Choose a base branch
from
+73 −154
Open

Change image storage to use new ExtendedAttrs payload field #5879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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';
import { CellData } from 'common/buffer/CellData';



// 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);
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