Change image storage to use new ExtendedAttrs payload field

addons/addon-image/src/ImageStorage.ts

@@ -5,106 +5,24 @@
 
 import { IDisposable } from '@xterm/xterm';
 import { ImageRenderer } from './ImageRenderer';
-import {
-  ITerminalExt, IExtendedAttrsImage, IImageAddonOptions, IImageSpec,
-  IBufferLineExt, BgFlags, Cell, Content, ICellSize, ExtFlags, Attributes,
-  UnderlineStyle, IAddImageOpts
+import type {
+  ITerminalExt, IImageAddonOptions, IImageSpec, ICellSize, IAddImageOpts
 } from './Types';
-
+import type { IBufferLine } from 'common/buffer/Types';
+import { CellData } from 'common/buffer/CellData';
 
 // fallback default cell size
 export const CELL_SIZE_DEFAULT: ICellSize = {
   width: 7,
   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:
@@ -126,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;
@@ -277,10 +196,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 (opts.scrolling) {
@@ -428,11 +347,11 @@ export class ImageStorage implements IDisposable {
 
     // walk all cells in viewport and collect tiles found
     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;
       for (let col = 0; col < cols; ++col) {
-        if (line.getBg(col) & BgFlags.HAS_EXTENDED) {
-          let e: IExtendedAttrsImage = line._extendedAttrs[col] ?? EMPTY_ATTRS;
+        const e = line.getExtended(col)?.payload;
+        if (e instanceof ImageTileInfo) {
           const imageId = e.imageId;
           if (imageId === undefined || imageId === -1) {
             continue;
@@ -448,13 +367,11 @@ export class ImageStorage implements IDisposable {
              * - cell has same image id
              * - cell has consecutive tile id
              */
-            while (
-              ++col < cols
-              && (line.getBg(col) & BgFlags.HAS_EXTENDED)
-              && (e = line._extendedAttrs[col] ?? EMPTY_ATTRS)
-              && (e.imageId === imageId)
-              && (e.tileId === startTile + count)
-            ) {
+            while (++col < cols) {
+              const nextE = line.getExtended(col)?.payload;
+              if (!(nextE instanceof ImageTileInfo) || nextE.imageId !== imageId || nextE.tileId !== startTile + count) {
+                break;
+              }
               count++;
             }
             col--;
@@ -504,9 +421,9 @@ export class ImageStorage implements IDisposable {
     const rows = buffer.lines.length;
     const oldCol = this._viewportMetrics.cols - 1;
     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)!;
+      const e = line.getExtended(oldCol)?.payload;
+      if (e instanceof ImageTileInfo) {
         const imageId = e.imageId;
         if (imageId === undefined || imageId === -1) {
           continue;
@@ -523,7 +440,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;
           }
@@ -535,7 +452,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++;
         }
       }
@@ -549,10 +466,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) {
+      const e = line.getExtended(x)?.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);
@@ -569,10 +486,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) {
+      const e = line.getExtended(x)?.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);
@@ -600,31 +517,31 @@ 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;
       }
     }
-    // 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);
+    // fall-through: always create new ExtendedAttrs entry
+    const extattr = workCell.extended.clone();
+    extattr.payload = new ImageTileInfo(imageId, tileId);
+    workCell.extended = extattr;
+    workCell.updateExtended();
+    line.setCell(x, workCell);
   }
 
   private _evictOnAlternate(): void {
@@ -637,18 +554,16 @@ 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;
       }
       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);
-            if (spec) {
-              spec.tileCount++;
-            }
+        const payload = line.getExtended(x)?.payload;
+        if (payload instanceof ImageTileInfo) {
+          const spec = this._images.get(payload.imageId);
+          if (spec) {
+            spec.tileCount++;
           }
         }
       }

addons/addon-image/src/Types.ts

@@ -10,7 +10,6 @@ import { Attributes, BgFlags, Content, ExtFlags, UnderlineStyle } from 'common/b
 import type { AttributeData } from 'common/buffer/AttributeData';
 import type { IParams, IDcsHandler, IOscHandler, IApcHandler, IEscapeSequenceParser } from 'common/parser/Types';
 import type { IInputHandler } from 'common/Types';
-import type { IBufferLine, IExtendedAttrs } from 'common/buffer/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';
@@ -48,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;
@@ -78,6 +65,7 @@ interface IInputHandlerExt extends IInputHandler {
   };
   onRequestReset(handler: () => void): IDisposable;
 }
+/* eslint-enable */
 
 export interface ICoreTerminalExt extends ITerminal {
   _themeService: IThemeService | undefined;
@@ -89,8 +77,6 @@ export interface ICoreTerminalExt extends ITerminal {
 export interface ITerminalExt extends Terminal {
   _core: ICoreTerminalExt;
 }
-/* eslint-enable */
-
 
 /**
  * Some storage definitions.

src/common/buffer/AttributeData.ts

@@ -139,6 +139,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 (
@@ -207,6 +208,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;
   }
 }