Merge branch 'master' into underline-dotted

src/browser/Linkifier2.test.ts

@@ -10,10 +10,6 @@ import { MockBufferService } from 'common/TestUtils.test';
 import { ILink } from 'browser/Types';
 
 class TestLinkifier2 extends Linkifier2 {
-  constructor(bufferService: IBufferService) {
-    super(bufferService);
-  }
-
   public set currentLink(link: any) {
     this._currentLink = link;
   }

src/browser/Terminal.ts

@@ -44,7 +44,7 @@ import { ThemeService } from 'browser/services/ThemeService';
 import { color, rgba } from 'common/Color';
 import { CoreTerminal } from 'common/CoreTerminal';
 import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter';
-import { toDisposable } from 'common/Lifecycle';
+import { MutableDisposable, toDisposable } from 'common/Lifecycle';
 import * as Browser from 'common/Platform';
 import { ColorRequestType, CoreMouseAction, CoreMouseButton, CoreMouseEventType, IColorEvent, ITerminalOptions, KeyboardResultType, ScrollSource, SpecialColorIndex } from 'common/Types';
 import { DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine';
@@ -118,7 +118,7 @@ export class Terminal extends CoreTerminal implements ITerminal {
   public linkifier2: ILinkifier2;
   public viewport: IViewport | undefined;
   private _compositionHelper: ICompositionHelper | undefined;
-  private _accessibilityManager: AccessibilityManager | undefined;
+  private _accessibilityManager: MutableDisposable<AccessibilityManager> = this.register(new MutableDisposable());
 
   private readonly _onCursorMove = this.register(new EventEmitter<void>());
   public readonly onCursorMove = this._onCursorMove.event;
@@ -252,12 +252,11 @@ export class Terminal extends CoreTerminal implements ITerminal {
 
   private _handleScreenReaderModeOptionChange(value: boolean): void {
     if (value) {
-      if (!this._accessibilityManager && this._renderService) {
-        this._accessibilityManager = this._instantiationService.createInstance(AccessibilityManager, this);
+      if (!this._accessibilityManager.value && this._renderService) {
+        this._accessibilityManager.value = this._instantiationService.createInstance(AccessibilityManager, this);
       }
     } else {
-      this._accessibilityManager?.dispose();
-      this._accessibilityManager = undefined;
+      this._accessibilityManager.clear();
     }
   }
 
@@ -535,7 +534,7 @@ export class Terminal extends CoreTerminal implements ITerminal {
     if (this.options.screenReaderMode) {
       // Note that this must be done *after* the renderer is created in order to
       // ensure the correct order of the dprchange event
-      this._accessibilityManager = this._instantiationService.createInstance(AccessibilityManager, this);
+      this._accessibilityManager.value = this._instantiationService.createInstance(AccessibilityManager, this);
     }
     this.register(this.optionsService.onSpecificOptionChange('screenReaderMode', e => this._handleScreenReaderModeOptionChange(e)));
 

src/browser/public/Terminal.ts

@@ -236,7 +236,7 @@ export class Terminal extends Disposable implements ITerminalApi {
     this._core.clearTextureAtlas();
   }
   public loadAddon(addon: ITerminalAddon): void {
-    return this._addonManager.loadAddon(this, addon);
+    this._addonManager.loadAddon(this, addon);
   }
   public static get strings(): ILocalizableStrings {
     return Strings;

src/browser/renderer/shared/CustomGlyphs.ts

@@ -366,11 +366,11 @@ export const powerlineDefinitions: { [index: string]: IVectorShape } = {
   // Right semi-circle solid
   '\u{E0B4}': { d: 'M0,0 L0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.FILL, rightPadding: 1 },
   // Right semi-circle line
-  '\u{E0B5}': { d: 'M0,1 C0.552,1,1,0.776,1,.5 C1,0.224,0.552,0,0,0', type: VectorType.STROKE, rightPadding: 1 },
+  '\u{E0B5}': { d: 'M.2,1 C.422,1,.8,.826,.78,.5 C.8,.174,0.422,0,.2,0', type: VectorType.STROKE, rightPadding: 1 },
   // Left semi-circle solid
   '\u{E0B6}': { d: 'M1,0 L1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.FILL, leftPadding: 1 },
   // Left semi-circle line
-  '\u{E0B7}': { d: 'M1,1 C0.448,1,0,0.776,0,.5 C0,0.224,0.448,0,1,0', type: VectorType.STROKE, leftPadding: 1 },
+  '\u{E0B7}': { d: 'M.8,1 C0.578,1,0.2,.826,.22,.5 C0.2,0.174,0.578,0,0.8,0', type: VectorType.STROKE, leftPadding: 1 },
   // Lower left triangle
   '\u{E0B8}': { d: 'M-.5,-.5 L1.5,1.5 L-.5,1.5', type: VectorType.FILL },
   // Backslash separator

src/browser/renderer/shared/TextureAtlas.ts

@@ -151,50 +151,48 @@ export class TextureAtlas implements ITextureAtlas {
     // microtask to ensure it does not interrupt textures that will be rendered in the current
     // animation frame which would result in blank rendered areas. This is actually not that
     // expensive relative to drawing the glyphs, so there is no need to wait for an idle callback.
-    if (TextureAtlas.maxAtlasPages && this._pages.length >= Math.max(4, TextureAtlas.maxAtlasPages / 2)) {
-      queueMicrotask(() => {
-        // Find the set of the largest 4 images, below the maximum size, with the highest
-        // percentages used
-        const pagesBySize = this._pages.filter(e => {
-          return e.canvas.width * 2 <= (TextureAtlas.maxTextureSize || Constants.FORCED_MAX_TEXTURE_SIZE);
-        }).sort((a, b) => {
-          if (b.canvas.width !== a.canvas.width) {
-            return b.canvas.width - a.canvas.width;
-          }
-          return b.percentageUsed - a.percentageUsed;
-        });
-        let sameSizeI = -1;
-        let size = 0;
-        for (let i = 0; i < pagesBySize.length; i++) {
-          if (pagesBySize[i].canvas.width !== size) {
-            sameSizeI = i;
-            size = pagesBySize[i].canvas.width;
-          } else if (i - sameSizeI === 3) {
-            break;
-          }
+    if (TextureAtlas.maxAtlasPages && this._pages.length >= Math.max(4, TextureAtlas.maxAtlasPages)) {
+      // Find the set of the largest 4 images, below the maximum size, with the highest
+      // percentages used
+      const pagesBySize = this._pages.filter(e => {
+        return e.canvas.width * 2 <= (TextureAtlas.maxTextureSize || Constants.FORCED_MAX_TEXTURE_SIZE);
+      }).sort((a, b) => {
+        if (b.canvas.width !== a.canvas.width) {
+          return b.canvas.width - a.canvas.width;
         }
-
-        // Gather details of the merge
-        const mergingPages = pagesBySize.slice(sameSizeI, sameSizeI + 4);
-        const sortedMergingPagesIndexes = mergingPages.map(e => e.glyphs[0].texturePage).sort((a, b) => a > b ? 1 : -1);
-        const mergedPageIndex = sortedMergingPagesIndexes[0];
-
-        // Merge into the new page
-        const mergedPage = this._mergePages(mergingPages, mergedPageIndex);
-        mergedPage.version++;
-
-        // Replace the first _merging_ page with the _merged_ page
-        this._pages[mergedPageIndex] = mergedPage;
-
-        // Delete the other 3 pages, shifting glyph texture pages as needed
-        for (let i = sortedMergingPagesIndexes.length - 1; i >= 1; i--) {
-          this._deletePage(sortedMergingPagesIndexes[i]);
-        }
-
-        // Request the model to be cleared to refresh all texture pages.
-        this._requestClearModel = true;
-        this._onAddTextureAtlasCanvas.fire(mergedPage.canvas);
+        return b.percentageUsed - a.percentageUsed;
       });
+      let sameSizeI = -1;
+      let size = 0;
+      for (let i = 0; i < pagesBySize.length; i++) {
+        if (pagesBySize[i].canvas.width !== size) {
+          sameSizeI = i;
+          size = pagesBySize[i].canvas.width;
+        } else if (i - sameSizeI === 3) {
+          break;
+        }
+      }
+
+      // Gather details of the merge
+      const mergingPages = pagesBySize.slice(sameSizeI, sameSizeI + 4);
+      const sortedMergingPagesIndexes = mergingPages.map(e => e.glyphs[0].texturePage).sort((a, b) => a > b ? 1 : -1);
+      const mergedPageIndex = this.pages.length - mergingPages.length;
+
+      // Merge into the new page
+      const mergedPage = this._mergePages(mergingPages, mergedPageIndex);
+      mergedPage.version++;
+
+      // Delete the pages, shifting glyph texture pages as needed
+      for (let i = sortedMergingPagesIndexes.length - 1; i >= 0; i--) {
+        this._deletePage(sortedMergingPagesIndexes[i]);
+      }
+
+      // Add the new merged page to the end
+      this.pages.push(mergedPage);
+
+      // Request the model to be cleared to refresh all texture pages.
+      this._requestClearModel = true;
+      this._onAddTextureAtlasCanvas.fire(mergedPage.canvas);
     }
 
     // All new atlas pages are created small as they are highly dynamic
@@ -770,13 +768,13 @@ export class TextureAtlas implements ITextureAtlas {
         }
       }
 
-      // Create a new one if too much vertical space would be wasted or there is not enough room
+      // Create a new page if too much vertical space would be wasted or there is not enough room
       // left in the page. The previous active row will become fixed in the process as it now has a
       // fixed height
       if (activeRow.y + rasterizedGlyph.size.y >= activePage.canvas.height || activeRow.height > rasterizedGlyph.size.y + Constants.ROW_PIXEL_THRESHOLD) {
         // Create the new fixed height row, creating a new page if there isn't enough room on the
         // current page
-        let wasNewPageCreated = false;
+        let wasPageAndRowFound = false;
         if (activePage.currentRow.y + activePage.currentRow.height + rasterizedGlyph.size.y >= activePage.canvas.height) {
           // Find the first page with room to create the new row on
           let candidatePage: AtlasPage | undefined;
@@ -789,15 +787,30 @@ export class TextureAtlas implements ITextureAtlas {
           if (candidatePage) {
             activePage = candidatePage;
           } else {
-            // Create a new page if there is no room
-            const newPage = this._createNewPage();
-            activePage = newPage;
-            activeRow = newPage.currentRow;
-            activeRow.height = rasterizedGlyph.size.y;
-            wasNewPageCreated = true;
+            // Before creating a new atlas page that would trigger a page merge, check if the
+            // current active row is sufficient when ignoring the ROW_PIXEL_THRESHOLD. This will
+            // improve texture utilization by using the available space before the page is merged
+            // and becomes static.
+            if (
+              TextureAtlas.maxAtlasPages &&
+              this._pages.length >= TextureAtlas.maxAtlasPages &&
+              activeRow.y + rasterizedGlyph.size.y <= activePage.canvas.height &&
+              activeRow.height >= rasterizedGlyph.size.y &&
+              activeRow.x + rasterizedGlyph.size.x <= activePage.canvas.width
+            ) {
+              // activePage and activeRow is already valid
+              wasPageAndRowFound = true;
+            } else {
+              // Create a new page if there is no room
+              const newPage = this._createNewPage();
+              activePage = newPage;
+              activeRow = newPage.currentRow;
+              activeRow.height = rasterizedGlyph.size.y;
+              wasPageAndRowFound = true;
+            }
           }
         }
-        if (!wasNewPageCreated) {
+        if (!wasPageAndRowFound) {
           // Fix the current row as the new row is being added below
           if (activePage.currentRow.height > 0) {
             activePage.fixedRows.push(activePage.currentRow);

src/browser/services/RenderService.ts

@@ -10,7 +10,7 @@ import { IRenderDebouncerWithCallback } from 'browser/Types';
 import { IRenderDimensions, IRenderer } from 'browser/renderer/shared/Types';
 import { ICharSizeService, ICoreBrowserService, IRenderService, IThemeService } from 'browser/services/Services';
 import { EventEmitter } from 'common/EventEmitter';
-import { Disposable } from 'common/Lifecycle';
+import { Disposable, MutableDisposable } from 'common/Lifecycle';
 import { DebouncedIdleTask } from 'common/TaskQueue';
 import { IBufferService, IDecorationService, IOptionsService } from 'common/services/Services';
 
@@ -23,7 +23,7 @@ interface ISelectionState {
 export class RenderService extends Disposable implements IRenderService {
   public serviceBrand: undefined;
 
-  private _renderer: IRenderer | undefined;
+  private _renderer: MutableDisposable<IRenderer> = this.register(new MutableDisposable());
   private _renderDebouncer: IRenderDebouncerWithCallback;
   private _screenDprMonitor: ScreenDprMonitor;
   private _pausedResizeTask = new DebouncedIdleTask();
@@ -49,7 +49,7 @@ export class RenderService extends Disposable implements IRenderService {
   private readonly _onRefreshRequest = this.register(new EventEmitter<{ start: number, end: number }>());
   public readonly onRefreshRequest = this._onRefreshRequest.event;
 
-  public get dimensions(): IRenderDimensions { return this._renderer!.dimensions; }
+  public get dimensions(): IRenderDimensions { return this._renderer.value!.dimensions; }
 
   constructor(
     private _rowCount: number,
@@ -63,8 +63,6 @@ export class RenderService extends Disposable implements IRenderService {
   ) {
     super();
 
-    this.register({ dispose: () => this._renderer?.dispose() });
-
     this._renderDebouncer = new RenderDebouncer(coreBrowserService.window, (start, end) => this._renderRows(start, end));
     this.register(this._renderDebouncer);
 
@@ -73,7 +71,7 @@ export class RenderService extends Disposable implements IRenderService {
     this.register(this._screenDprMonitor);
 
     this.register(bufferService.onResize(() => this._fullRefresh()));
-    this.register(bufferService.buffers.onBufferActivate(() => this._renderer?.clear()));
+    this.register(bufferService.buffers.onBufferActivate(() => this._renderer.value?.clear()));
     this.register(optionsService.onOptionChange(() => this._handleOptionsChanged()));
     this.register(this._charSizeService.onCharSizeChange(() => this.handleCharSizeChanged()));
 
@@ -148,7 +146,7 @@ export class RenderService extends Disposable implements IRenderService {
   }
 
   private _renderRows(start: number, end: number): void {
-    if (!this._renderer) {
+    if (!this._renderer.value) {
       return;
     }
 
@@ -159,11 +157,11 @@ export class RenderService extends Disposable implements IRenderService {
     end = Math.min(end, this._rowCount - 1);
 
     // Render
-    this._renderer.renderRows(start, end);
+    this._renderer.value.renderRows(start, end);
 
     // Update selection if needed
     if (this._needsSelectionRefresh) {
-      this._renderer.handleSelectionChanged(this._selectionState.start, this._selectionState.end, this._selectionState.columnSelectMode);
+      this._renderer.value.handleSelectionChanged(this._selectionState.start, this._selectionState.end, this._selectionState.columnSelectMode);
       this._needsSelectionRefresh = false;
     }
 
@@ -181,7 +179,7 @@ export class RenderService extends Disposable implements IRenderService {
   }
 
   private _handleOptionsChanged(): void {
-    if (!this._renderer) {
+    if (!this._renderer.value) {
       return;
     }
     this.refreshRows(0, this._rowCount - 1);
@@ -189,25 +187,23 @@ export class RenderService extends Disposable implements IRenderService {
   }
 
   private _fireOnCanvasResize(): void {
-    if (!this._renderer) {
+    if (!this._renderer.value) {
       return;
     }
     // Don't fire the event if the dimensions haven't changed
-    if (this._renderer.dimensions.css.canvas.width === this._canvasWidth && this._renderer.dimensions.css.canvas.height === this._canvasHeight) {
+    if (this._renderer.value.dimensions.css.canvas.width === this._canvasWidth && this._renderer.value.dimensions.css.canvas.height === this._canvasHeight) {
       return;
     }
-    this._onDimensionsChange.fire(this._renderer.dimensions);
+    this._onDimensionsChange.fire(this._renderer.value.dimensions);
   }
 
   public hasRenderer(): boolean {
-    return !!this._renderer;
+    return !!this._renderer.value;
   }
 
   public setRenderer(renderer: IRenderer): void {
-    // TODO: RenderService should be the only one to dispose the renderer
-    this._renderer?.dispose();
-    this._renderer = renderer;
-    this._renderer.onRequestRedraw(e => this.refreshRows(e.start, e.end, true));
+    this._renderer.value = renderer;
+    this._renderer.value.onRequestRedraw(e => this.refreshRows(e.start, e.end, true));
 
     // Force a refresh
     this._needsSelectionRefresh = true;
@@ -227,10 +223,10 @@ export class RenderService extends Disposable implements IRenderService {
   }
 
   public clearTextureAtlas(): void {
-    if (!this._renderer) {
+    if (!this._renderer.value) {
       return;
     }
-    this._renderer.clearTextureAtlas?.();
+    this._renderer.value.clearTextureAtlas?.();
     this._fullRefresh();
   }
 
@@ -239,50 +235,50 @@ export class RenderService extends Disposable implements IRenderService {
     // when devicePixelRatio changes
     this._charSizeService.measure();
 
-    if (!this._renderer) {
+    if (!this._renderer.value) {
       return;
     }
-    this._renderer.handleDevicePixelRatioChange();
+    this._renderer.value.handleDevicePixelRatioChange();
     this.refreshRows(0, this._rowCount - 1);
   }
 
   public handleResize(cols: number, rows: number): void {
-    if (!this._renderer) {
+    if (!this._renderer.value) {
       return;
     }
     if (this._isPaused) {
-      this._pausedResizeTask.set(() => this._renderer!.handleResize(cols, rows));
+      this._pausedResizeTask.set(() => this._renderer.value!.handleResize(cols, rows));
     } else {
-      this._renderer.handleResize(cols, rows);
+      this._renderer.value.handleResize(cols, rows);
     }
     this._fullRefresh();
   }
 
   // TODO: Is this useful when we have onResize?
   public handleCharSizeChanged(): void {
-    this._renderer?.handleCharSizeChanged();
+    this._renderer.value?.handleCharSizeChanged();
   }
 
   public handleBlur(): void {
-    this._renderer?.handleBlur();
+    this._renderer.value?.handleBlur();
   }
 
   public handleFocus(): void {
-    this._renderer?.handleFocus();
+    this._renderer.value?.handleFocus();
   }
 
   public handleSelectionChanged(start: [number, number] | undefined, end: [number, number] | undefined, columnSelectMode: boolean): void {
     this._selectionState.start = start;
     this._selectionState.end = end;
     this._selectionState.columnSelectMode = columnSelectMode;
-    this._renderer?.handleSelectionChanged(start, end, columnSelectMode);
+    this._renderer.value?.handleSelectionChanged(start, end, columnSelectMode);
   }
 
   public handleCursorMove(): void {
-    this._renderer?.handleCursorMove();
+    this._renderer.value?.handleCursorMove();
   }
 
   public clear(): void {
-    this._renderer?.clear();
+    this._renderer.value?.clear();
   }
 }

src/common/CoreTerminal.ts

@@ -21,7 +21,7 @@
  *   http://linux.die.net/man/7/urxvt
  */
 
-import { Disposable, toDisposable } from 'common/Lifecycle';
+import { Disposable, MutableDisposable, toDisposable } from 'common/Lifecycle';
 import { IInstantiationService, IOptionsService, IBufferService, ILogService, ICharsetService, ICoreService, ICoreMouseService, IUnicodeService, LogLevelEnum, ITerminalOptions, IOscLinkService } from 'common/services/Services';
 import { InstantiationService } from 'common/services/InstantiationService';
 import { LogService } from 'common/services/LogService';
@@ -57,7 +57,7 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
 
   protected _inputHandler: InputHandler;
   private _writeBuffer: WriteBuffer;
-  private _windowsWrappingHeuristics: IDisposable | undefined;
+  private _windowsWrappingHeuristics = this.register(new MutableDisposable());
 
   private readonly _onBinary = this.register(new EventEmitter<string>());
   public readonly onBinary = this._onBinary.event;
@@ -144,11 +144,6 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
     // Setup WriteBuffer
     this._writeBuffer = this.register(new WriteBuffer((data, promiseResult) => this._inputHandler.parse(data, promiseResult)));
     this.register(forwardEvent(this._writeBuffer.onWriteParsed, this._onWriteParsed));
-
-    this.register(toDisposable(() => {
-      this._windowsWrappingHeuristics?.dispose();
-      this._windowsWrappingHeuristics = undefined;
-    }));
   }
 
   public write(data: string | Uint8Array, callback?: () => void): void {
@@ -267,20 +262,19 @@ export abstract class CoreTerminal extends Disposable implements ICoreTerminal {
     if (value) {
       this._enableWindowsWrappingHeuristics();
     } else {
-      this._windowsWrappingHeuristics?.dispose();
-      this._windowsWrappingHeuristics = undefined;
+      this._windowsWrappingHeuristics.clear();
     }
   }
 
   protected _enableWindowsWrappingHeuristics(): void {
-    if (!this._windowsWrappingHeuristics) {
+    if (!this._windowsWrappingHeuristics.value) {
       const disposables: IDisposable[] = [];
       disposables.push(this.onLineFeed(updateWindowsModeWrappedState.bind(null, this._bufferService)));
       disposables.push(this.registerCsiHandler({ final: 'H' }, () => {
         updateWindowsModeWrappedState(this._bufferService);
         return false;
       }));
-      this._windowsWrappingHeuristics = toDisposable(() => {
+      this._windowsWrappingHeuristics.value = toDisposable(() => {
         for (const d of disposables) {
           d.dispose();
         }

src/common/Lifecycle.ts

@@ -13,9 +13,6 @@ export abstract class Disposable implements IDisposable {
   protected _disposables: IDisposable[] = [];
   protected _isDisposed: boolean = false;
 
-  constructor() {
-  }
-
   /**
    * Disposes the object, triggering the `dispose` method on all registered IDisposables.
    */

src/common/public/AddonManager.ts

@@ -14,9 +14,6 @@ export interface ILoadedAddon {
 export class AddonManager implements IDisposable {
   protected _addons: ILoadedAddon[] = [];
 
-  constructor() {
-  }
-
   public dispose(): void {
     for (let i = this._addons.length - 1; i >= 0; i--) {
       this._addons[i].instance.dispose();

src/headless/public/Terminal.ts

@@ -182,7 +182,7 @@ export class Terminal extends Disposable implements ITerminalApi {
   }
   public loadAddon(addon: ITerminalAddon): void {
     // TODO: This could cause issues if the addon calls renderer apis
-    return this._addonManager.loadAddon(this as any, addon);
+    this._addonManager.loadAddon(this as any, addon);
   }
 
   private _verifyIntegers(...values: number[]): void {