Merge pull request #3702 from meganrogge/overviewrulerFix

src/browser/Decorations/OverviewRulerRenderer.ts

@@ -38,6 +38,11 @@ export class OverviewRulerRenderer extends Disposable {
   }
   private _animationFrame: number | undefined;
 
+  private _shouldUpdateDimensions: boolean | undefined = true;
+  private _shouldUpdateAnchor: boolean | undefined = true;
+
+  private _containerHeight: number | undefined;
+
   constructor(
     private readonly _viewportElement: HTMLElement,
     private readonly _screenElement: HTMLElement,
@@ -57,22 +62,53 @@ export class OverviewRulerRenderer extends Disposable {
     } else {
       this._ctx = ctx;
     }
-    this._queueRefresh(true);
+    this._registerDecorationListeners();
+    this._registerBufferChangeListeners();
+    this._registerDimensionChangeListeners();
+  }
+
+  /**
+   * On decoration add or remove, redraw
+   */
+  private _registerDecorationListeners(): void {
+    this.register(this._decorationService.onDecorationRegistered(() => this._queueRefresh(undefined, true)));
+    this.register(this._decorationService.onDecorationRemoved(decoration => this._removeDecoration(decoration)));
+  }
+
+  /**
+   * On buffer change, redraw
+   * and hide the canvas if the alt buffer is active
+   */
+  private _registerBufferChangeListeners(): void {
+    this.register(this._renderService.onRenderedBufferChange(() => this._queueRefresh()));
     this.register(this._bufferService.buffers.onBufferActivate(() => {
       this._canvas!.style.display = this._bufferService.buffer === this._bufferService.buffers.alt ? 'none' : 'block';
     }));
-    this.register(this._renderService.onRenderedBufferChange(() => this._queueRefresh()));
+  }
-    this.register(this._renderService.onDimensionsChange(() => this._queueRefresh(true, true)));
+  /**
-    this.register(addDisposableDomListener(window, 'resize', () => this._queueRefresh(true)));
+   * On dimension change, update canvas dimensions
-    this.register(this._decorationService.onDecorationRegistered(() => this._queueRefresh(undefined, true)));
+   * and then redraw
-    this.register(this._decorationService.onDecorationRemoved(decoration => this._removeDecoration(decoration)));
+   */
-    this.register(this._optionsService.onOptionChange(o => {
+  private _registerDimensionChangeListeners(): void {
-      if (o === 'overviewRulerWidth') {
+    // container height changed
-        this._refreshDrawConstants();
+    this.register(this._renderService.onRender((): void => {
-        this._queueRefresh();
+      if (!this._containerHeight || this._containerHeight !== this._screenElement.clientHeight) {
+        this._queueRefresh(true);
+        this._containerHeight = this._screenElement.clientHeight;
       }
     }));
-    this._refreshDrawConstants();
+    // overview ruler width changed
+    this.register(this._optionsService.onOptionChange(o => {
+      if (o === 'overviewRulerWidth') {
+        this._queueRefresh(true);
+      }
+    }));
+    // device pixel ratio changed
+    this.register(addDisposableDomListener(window, 'resize', () => {
+      this._queueRefresh(true);
+    }));
+    // set the canvas dimensions
+    this._queueRefresh(true);
   }
 
   public override dispose(): void {
@@ -104,8 +140,8 @@ export class OverviewRulerRenderer extends Disposable {
     drawX.right = drawWidth.left + drawWidth.center;
   }
 
-  private _refreshStyle(decoration: IInternalDecoration, updateAnchor?: boolean): void {
+  private _refreshStyle(decoration: IInternalDecoration): void {
-    if (updateAnchor) {
+    if (this._shouldUpdateAnchor) {
       if (decoration.options.anchor === 'right') {
         this._canvas.style.right = decoration.options.x ? `${decoration.options.x * this._renderService.dimensions.actualCellWidth}px` : '';
       } else {
@@ -122,7 +158,7 @@ export class OverviewRulerRenderer extends Disposable {
       /* x */ drawX[decoration.options.overviewRulerOptions.position!],
       /* y */ Math.round(
         (this._canvas.height - 1) * // -1 to ensure at least 2px are allowed for decoration on last line
-        (decoration.options.marker.line / this._bufferService.buffers.active.lines.length) - drawHeight[decoration.options.overviewRulerOptions.position!] / 2
+      (decoration.options.marker.line / this._bufferService.buffers.active.lines.length) - drawHeight[decoration.options.overviewRulerOptions.position!] / 2
       ),
       /* w */ drawWidth[decoration.options.overviewRulerOptions.position!],
       /* h */ drawHeight[decoration.options.overviewRulerOptions.position!]
@@ -131,44 +167,48 @@ export class OverviewRulerRenderer extends Disposable {
 
   private _refreshCanvasDimensions(): void {
     this._canvas.style.width = `${this._width}px`;
-    this._canvas.style.height = `${this._screenElement.clientHeight}px`;
     this._canvas.width = Math.round(this._width * window.devicePixelRatio);
+    this._canvas.style.height = `${this._screenElement.clientHeight}px`;
     this._canvas.height = Math.round(this._screenElement.clientHeight * window.devicePixelRatio);
     this._refreshDrawConstants();
   }
 
-  private _refreshDecorations(updateCanvasDimensions?: boolean, updateAnchor?: boolean): void {
+  private _refreshDecorations(): void {
-    if (updateCanvasDimensions) {
+    if (this._shouldUpdateDimensions) {
       this._refreshCanvasDimensions();
     }
     this._ctx.clearRect(0, 0, this._canvas.width, this._canvas.height);
     for (const decoration of this._decorationService.decorations) {
       if (decoration.options.overviewRulerOptions && decoration.options.overviewRulerOptions.position !== 'full') {
-        this._renderDecoration(decoration, updateAnchor);
+        this._renderDecoration(decoration);
       }
     }
     for (const decoration of this._decorationService.decorations) {
       if (decoration.options.overviewRulerOptions && decoration.options.overviewRulerOptions.position === 'full') {
-        this._renderDecoration(decoration, updateAnchor);
+        this._renderDecoration(decoration);
       }
     }
+    this._shouldUpdateDimensions = false;
+    this._shouldUpdateAnchor = false;
   }
 
-  private _renderDecoration(decoration: IInternalDecoration, updateAnchor?: boolean): void {
+  private _renderDecoration(decoration: IInternalDecoration): void {
     const element = this._decorationElements.get(decoration);
     if (!element) {
       this._decorationElements.set(decoration, this._canvas);
       decoration.onDispose(() => this._queueRefresh());
     }
-    this._refreshStyle(decoration, updateAnchor);
+    this._refreshStyle(decoration);
   }
 
   private _queueRefresh(updateCanvasDimensions?: boolean, updateAnchor?: boolean): void {
+    this._shouldUpdateDimensions = updateCanvasDimensions || this._shouldUpdateDimensions;
+    this._shouldUpdateAnchor = updateAnchor || this._shouldUpdateAnchor;
     if (this._animationFrame !== undefined) {
       return;
     }
     this._animationFrame = window.requestAnimationFrame(() => {
-      this._refreshDecorations(updateCanvasDimensions, updateAnchor);
+      this._refreshDecorations();
       this._animationFrame = undefined;
     });
   }

src/browser/Terminal.ts

@@ -613,7 +613,8 @@ export class Terminal extends CoreTerminal implements ITerminal {
     this.optionsService.onOptionChange(() => {
       if (!this._overviewRulerRenderer && this.options.overviewRulerWidth && this._viewportElement && this.screenElement) {
         this._overviewRulerRenderer = this._instantiationService.createInstance(OverviewRulerRenderer, this._viewportElement, this.screenElement);
-      }});
+      }
+    });
     // Measure the character size
     this._charSizeService.measure();
 

src/browser/TestUtils.test.ts

@@ -371,6 +371,7 @@ export class MockRenderService implements IRenderService {
   public serviceBrand: undefined;
   public onDimensionsChange: IEvent<IRenderDimensions> = new EventEmitter<IRenderDimensions>().event;
   public onRenderedBufferChange: IEvent<{ start: number, end: number }, void> = new EventEmitter<{ start: number, end: number }>().event;
+  public onRender: IEvent<{ start: number, end: number }, void> = new EventEmitter<{ start: number, end: number }>().event;
   public onRefreshRequest: IEvent<{ start: number, end: number}, void> = new EventEmitter<{ start: number, end: number }>().event;
   public dimensions: IRenderDimensions = {
     scaledCharWidth: 0,

src/browser/services/RenderService.ts

@@ -39,8 +39,10 @@ export class RenderService extends Disposable implements IRenderService {
 
   private _onDimensionsChange = new EventEmitter<IRenderDimensions>();
   public get onDimensionsChange(): IEvent<IRenderDimensions> { return this._onDimensionsChange.event; }
+  private _onRenderedBufferChange = new EventEmitter<{ start: number, end: number }>();
+  public get onRenderedBufferChange(): IEvent<{ start: number, end: number }> { return this._onRenderedBufferChange.event; }
   private _onRender = new EventEmitter<{ start: number, end: number }>();
-  public get onRenderedBufferChange(): IEvent<{ start: number, end: number }> { return this._onRender.event; }
+  public get onRender(): IEvent<{ start: number, end: number }> { return this._onRender.event; }
   private _onRefreshRequest = new EventEmitter<{ start: number, end: number }>();
   public get onRefreshRequest(): IEvent<{ start: number, end: number }> { return this._onRefreshRequest.event; }
 
@@ -122,8 +124,9 @@ export class RenderService extends Disposable implements IRenderService {
 
     // Fire render event only if it was not a redraw
     if (!this._isNextRenderRedrawOnly) {
-      this._onRender.fire({ start, end });
+      this._onRenderedBufferChange.fire({ start, end });
     }
+    this._onRender.fire({ start, end });
     this._isNextRenderRedrawOnly = true;
   }
 

src/browser/services/Services.ts

@@ -50,6 +50,10 @@ export interface IRenderService extends IDisposable {
    * or selections are rendered.
    */
   onRenderedBufferChange: IEvent<{ start: number, end: number }>;
+  /**
+   * Fires on render
+   */
+  onRender: IEvent<{ start: number, end: number }>;
   onRefreshRequest: IEvent<{ start: number, end: number }>;
 
   dimensions: IRenderDimensions;