Fix decorations with height > 1

Fixes #4855

demo/client/components/window/testWindow.ts

@@ -62,7 +62,8 @@ export class TestWindow extends BaseWindow implements IControlWindow {
 
     // Decorations section
     this._addDt(dl, 'Decorations');
-    this._addDdWithButton(dl, 'add-decoration', 'Decoration', 'Add a decoration to the terminal', () => addDecoration(this._terminal));
+    this._addDdWithButton(dl, 'add-decoration', 'Decoration (1x1)', 'Add a 1x1 decoration to the terminal', () => addDecoration(this._terminal));
+    this._addDdWithButton(dl, 'add-decoration', 'Decoration (3x3)', 'Add a 3x3 decoration to the terminal', () => addDecoration(this._terminal, 3));
     this._addDdWithButton(dl, 'add-overview-ruler', 'Add Overview Ruler', 'Add an overview ruler to the terminal', () => addOverviewRuler(this._terminal));
     this._addDdWithButton(dl, 'decoration-stress-test', 'Stress Test', 'Toggle between adding and removing a decoration to each line', () => decorationStressTest(this._terminal));
 
@@ -790,11 +791,13 @@ function loadTestLongLines(term: Terminal, addons: AddonCollection): void {
   });
 }
 
-function addDecoration(term: Terminal): void {
+function addDecoration(term: Terminal, dim: number = 1): void {
   term.options['overviewRuler'] = { width: 14 };
   const marker = term.registerMarker(1);
   const decoration = term.registerDecoration({
     marker,
+    height: dim,
+    width: dim,
     backgroundColor: '#00FF00',
     foregroundColor: '#00FE00',
     overviewRulerOptions: { color: '#ef292980', position: 'left' }

src/common/services/DecorationService.test.ts

@@ -9,12 +9,16 @@ import { IMarker } from 'common/Types';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { Emitter } from 'vs/base/common/event';
 
-const fakeMarker: IMarker = Object.freeze(new class extends Disposable {
-  public readonly id = 1;
-  public readonly line = 1;
-  public readonly isDisposed = false;
-  public readonly onDispose = new Emitter<void>().event;
-}());
+function createFakeMarker(line: number): IMarker {
+  return Object.freeze(new class extends Disposable {
+    public readonly id = 1;
+    public readonly line = line;
+    public readonly isDisposed = false;
+    public readonly onDispose = new Emitter<void>().event;
+  }());
+}
+
+const fakeMarker: IMarker = createFakeMarker(1);
 
 describe('DecorationService', () => {
   it('should set isDisposed to true after dispose', () => {
@@ -27,4 +31,89 @@ describe('DecorationService', () => {
     decoration!.dispose();
     assert.isTrue(decoration!.isDisposed);
   });
+
+  describe('forEachDecorationAtCell', () => {
+    it('should find decoration at its marker line', () => {
+      const service = new DecorationService();
+      const decoration = service.registerDecoration({
+        marker: createFakeMarker(5),
+        width: 10
+      });
+      assert.ok(decoration);
+
+      const found: typeof decoration[] = [];
+      service.forEachDecorationAtCell(0, 5, undefined, d => found.push(d));
+      assert.strictEqual(found.length, 1);
+    });
+
+    it('should find decoration with height > 1 on subsequent lines', () => {
+      const service = new DecorationService();
+      const decoration = service.registerDecoration({
+        marker: createFakeMarker(5),
+        width: 10,
+        height: 3
+      });
+      assert.ok(decoration);
+
+      const foundAt5: typeof decoration[] = [];
+      service.forEachDecorationAtCell(0, 5, undefined, d => foundAt5.push(d));
+      assert.strictEqual(foundAt5.length, 1);
+
+      const foundAt6: typeof decoration[] = [];
+      service.forEachDecorationAtCell(0, 6, undefined, d => foundAt6.push(d));
+      assert.strictEqual(foundAt6.length, 1);
+
+      const foundAt7: typeof decoration[] = [];
+      service.forEachDecorationAtCell(0, 7, undefined, d => foundAt7.push(d));
+      assert.strictEqual(foundAt7.length, 1);
+
+      const foundAt8: typeof decoration[] = [];
+      service.forEachDecorationAtCell(0, 8, undefined, d => foundAt8.push(d));
+      assert.strictEqual(foundAt8.length, 0);
+    });
+
+    it('should not find decoration outside its x range', () => {
+      const service = new DecorationService();
+      const decoration = service.registerDecoration({
+        marker: createFakeMarker(5),
+        x: 5,
+        width: 3,
+        height: 2
+      });
+      assert.ok(decoration);
+
+      const foundAtX4: typeof decoration[] = [];
+      service.forEachDecorationAtCell(4, 5, undefined, d => foundAtX4.push(d));
+      assert.strictEqual(foundAtX4.length, 0);
+
+      const foundAtX5: typeof decoration[] = [];
+      service.forEachDecorationAtCell(5, 5, undefined, d => foundAtX5.push(d));
+      assert.strictEqual(foundAtX5.length, 1);
+
+      const foundAtX7: typeof decoration[] = [];
+      service.forEachDecorationAtCell(7, 6, undefined, d => foundAtX7.push(d));
+      assert.strictEqual(foundAtX7.length, 1);
+
+      const foundAtX8: typeof decoration[] = [];
+      service.forEachDecorationAtCell(8, 5, undefined, d => foundAtX8.push(d));
+      assert.strictEqual(foundAtX8.length, 0);
+    });
+  });
+
+  describe('getDecorationsAtCell', () => {
+    it('should find decoration with height > 1 on subsequent lines', () => {
+      const service = new DecorationService();
+      const decoration = service.registerDecoration({
+        marker: createFakeMarker(5),
+        width: 10,
+        height: 3
+      });
+      assert.ok(decoration);
+
+      assert.strictEqual([...service.getDecorationsAtCell(0, 5)].length, 1);
+      assert.strictEqual([...service.getDecorationsAtCell(0, 6)].length, 1);
+      assert.strictEqual([...service.getDecorationsAtCell(0, 7)].length, 1);
+      assert.strictEqual([...service.getDecorationsAtCell(0, 8)].length, 0);
+    });
+  });
 });

src/common/services/DecorationService.ts

@@ -14,6 +14,8 @@ import { Emitter } from 'vs/base/common/event';
 // Work variables to avoid garbage collection
 let $xmin = 0;
 let $xmax = 0;
+let $ymin = 0;
+let $ymax = 0;
 
 export class DecorationService extends Disposable implements IDecorationService {
   public serviceBrand: any;
@@ -70,7 +72,14 @@ export class DecorationService extends Disposable implements IDecorationService
   public *getDecorationsAtCell(x: number, line: number, layer?: 'bottom' | 'top'): IterableIterator<IInternalDecoration> {
     let xmin = 0;
     let xmax = 0;
-    for (const d of this._decorations.getKeyIterator(line)) {
+    let ymin = 0;
+    let ymax = 0;
+    for (const d of this._decorations.values()) {
+      ymin = d.marker.line;
+      ymax = ymin + (d.options.height ?? 1);
+      if (line < ymin || line >= ymax) {
+        continue;
+      }
       xmin = d.options.x ?? 0;
       xmax = xmin + (d.options.width ?? 1);
       if (x >= xmin && x < xmax && (!layer || (d.options.layer ?? 'bottom') === layer)) {
@@ -80,13 +89,18 @@ export class DecorationService extends Disposable implements IDecorationService
   }
 
   public forEachDecorationAtCell(x: number, line: number, layer: 'bottom' | 'top' | undefined, callback: (decoration: IInternalDecoration) => void): void {
-    this._decorations.forEachByKey(line, d => {
+    for (const d of this._decorations.values()) {
+      $ymin = d.marker.line;
+      $ymax = $ymin + (d.options.height ?? 1);
+      if (line < $ymin || line >= $ymax) {
+        continue;
+      }
       $xmin = d.options.x ?? 0;
       $xmax = $xmin + (d.options.width ?? 1);
       if (x >= $xmin && x < $xmax && (!layer || (d.options.layer ?? 'bottom') === layer)) {
         callback(d);
       }
-    });
+    }
   }
 }