Skip to content

Commit

Permalink
Merge pull request microsoft#77298 from microsoft/tyriar/77228_dimens…
Browse files Browse the repository at this point in the history
…ionss

Improve terminal grid dimensions caching
  • Loading branch information
Tyriar authored Jul 13, 2019
2 parents 167e621 + 9f69f43 commit 2443cf5
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 60 deletions.
41 changes: 20 additions & 21 deletions extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,27 +270,6 @@ suite('window namespace tests', () => {
window.createTerminal({ name: 'c', virtualProcess });
});

test('should get dimensions event when shown', (done) => {
const reg1 = window.onDidOpenTerminal(term => {
reg1.dispose();
equal(terminal, term);
term.show();
});
const virtualProcess: TerminalVirtualProcess = {
onDidWrite: new EventEmitter<string>().event,
setDimensions: dimensions => {
ok(dimensions.columns > 0);
ok(dimensions.rows > 0);
const reg2 = window.onDidCloseTerminal(() => {
reg2.dispose();
done();
});
terminal.dispose();
}
};
const terminal = window.createTerminal({ name: 'foo', virtualProcess });
});

test('should fire Terminal.onData on write', (done) => {
const reg1 = window.onDidOpenTerminal(term => {
equal(terminal, term);
Expand All @@ -313,6 +292,26 @@ suite('window namespace tests', () => {
const terminal = window.createTerminal({ name: 'foo', virtualProcess });
});

test('should fire provide dimensions on start as the terminal has been shown', (done) => {
const reg1 = window.onDidOpenTerminal(term => {
equal(terminal, term);
reg1.dispose();
});
const virtualProcess: TerminalVirtualProcess = {
onDidWrite: new EventEmitter<string>().event,
start: (dimensions) => {
ok(dimensions!.columns > 0);
ok(dimensions!.rows > 0);
const reg3 = window.onDidCloseTerminal(() => {
reg3.dispose();
done();
});
terminal.dispose();
}
};
const terminal = window.createTerminal({ name: 'foo', virtualProcess });
});

test('should respect dimension overrides', (done) => {
const reg1 = window.onDidOpenTerminal(term => {
equal(terminal, term);
Expand Down
5 changes: 4 additions & 1 deletion src/vs/vscode.proposed.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1084,8 +1084,11 @@ declare module 'vscode' {

/**
* Implement to handle when the terminal is ready to start firing events.
*
* @param initialDimensions The dimensions of the terminal, this will be undefined if the
* terminal panel has not been opened before this is called.
*/
start?(): void;
start?(initialDimensions: TerminalDimensions | undefined): void;
}

//#endregion
Expand Down
37 changes: 22 additions & 15 deletions src/vs/workbench/api/browser/mainThreadTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
*--------------------------------------------------------------------------------------------*/

import { IDisposable, DisposableStore } from 'vs/base/common/lifecycle';
import { ITerminalService, ITerminalInstance, IShellLaunchConfig, ITerminalProcessExtHostProxy, ITerminalProcessExtHostRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY, IAvailableShellsRequest, IDefaultShellAndArgsRequest } from 'vs/workbench/contrib/terminal/common/terminal';
import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceShape, MainContext, IExtHostContext, ShellLaunchConfigDto, TerminalLaunchConfig } from 'vs/workbench/api/common/extHost.protocol';
import { ITerminalService, ITerminalInstance, IShellLaunchConfig, ITerminalProcessExtHostProxy, ITerminalProcessExtHostRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY, IAvailableShellsRequest, IDefaultShellAndArgsRequest, ITerminalVirtualProcessRequest } from 'vs/workbench/contrib/terminal/common/terminal';
import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceShape, MainContext, IExtHostContext, ShellLaunchConfigDto, TerminalLaunchConfig, ITerminalDimensionsDto } from 'vs/workbench/api/common/extHost.protocol';
import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers';
import { URI } from 'vs/base/common/uri';
import { StopWatch } from 'vs/base/common/stopwatch';
Expand Down Expand Up @@ -48,7 +48,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
this._toDispose.add(_terminalService.onInstanceDimensionsChanged(instance => this._onInstanceDimensionsChanged(instance)));
this._toDispose.add(_terminalService.onInstanceMaximumDimensionsChanged(instance => this._onInstanceMaximumDimensionsChanged(instance)));
this._toDispose.add(_terminalService.onInstanceRequestExtHostProcess(request => this._onTerminalRequestExtHostProcess(request)));
this._toDispose.add(_terminalService.onInstanceRequestVirtualProcess(proxy => this._onTerminalRequestVirtualProcess(proxy)));
this._toDispose.add(_terminalService.onInstanceRequestVirtualProcess(e => this._onTerminalRequestVirtualProcess(e)));
this._toDispose.add(_terminalService.onActiveInstanceChanged(instance => this._onActiveTerminalChanged(instance ? instance.id : null)));
this._toDispose.add(_terminalService.onInstanceTitleChanged(instance => this._onTitleChanged(instance.id, instance.title)));
this._toDispose.add(_terminalService.configHelper.onWorkspacePermissionsChanged(isAllowed => this._onWorkspacePermissionsChanged(isAllowed)));
Expand Down Expand Up @@ -244,12 +244,13 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
return;
}

const ready = this._terminalProcessesReady[request.proxy.terminalId];
const proxy = request.proxy;
const ready = this._terminalProcessesReady[proxy.terminalId];
if (ready) {
ready(request.proxy);
delete this._terminalProcessesReady[request.proxy.terminalId];
ready(proxy);
delete this._terminalProcessesReady[proxy.terminalId];
} else {
this._terminalProcesses[request.proxy.terminalId] = Promise.resolve(request.proxy);
this._terminalProcesses[proxy.terminalId] = Promise.resolve(proxy);
}
const shellLaunchConfigDto: ShellLaunchConfigDto = {
name: request.shellLaunchConfig.name,
Expand All @@ -258,16 +259,17 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
cwd: request.shellLaunchConfig.cwd,
env: request.shellLaunchConfig.env
};
this._proxy.$createProcess(request.proxy.terminalId, shellLaunchConfigDto, request.activeWorkspaceRootUri, request.cols, request.rows, request.isWorkspaceShellAllowed);
request.proxy.onInput(data => this._proxy.$acceptProcessInput(request.proxy.terminalId, data));
request.proxy.onResize(dimensions => this._proxy.$acceptProcessResize(request.proxy.terminalId, dimensions.cols, dimensions.rows));
request.proxy.onShutdown(immediate => this._proxy.$acceptProcessShutdown(request.proxy.terminalId, immediate));
request.proxy.onRequestCwd(() => this._proxy.$acceptProcessRequestCwd(request.proxy.terminalId));
request.proxy.onRequestInitialCwd(() => this._proxy.$acceptProcessRequestInitialCwd(request.proxy.terminalId));
request.proxy.onRequestLatency(() => this._onRequestLatency(request.proxy.terminalId));
this._proxy.$createProcess(proxy.terminalId, shellLaunchConfigDto, request.activeWorkspaceRootUri, request.cols, request.rows, request.isWorkspaceShellAllowed);
proxy.onInput(data => this._proxy.$acceptProcessInput(proxy.terminalId, data));
proxy.onResize(dimensions => this._proxy.$acceptProcessResize(proxy.terminalId, dimensions.cols, dimensions.rows));
proxy.onShutdown(immediate => this._proxy.$acceptProcessShutdown(proxy.terminalId, immediate));
proxy.onRequestCwd(() => this._proxy.$acceptProcessRequestCwd(proxy.terminalId));
proxy.onRequestInitialCwd(() => this._proxy.$acceptProcessRequestInitialCwd(proxy.terminalId));
proxy.onRequestLatency(() => this._onRequestLatency(proxy.terminalId));
}

private _onTerminalRequestVirtualProcess(proxy: ITerminalProcessExtHostProxy): void {
private _onTerminalRequestVirtualProcess(request: ITerminalVirtualProcessRequest): void {
const proxy = request.proxy;
const ready = this._terminalProcessesReady[proxy.terminalId];
if (!ready) {
this._terminalProcesses[proxy.terminalId] = Promise.resolve(proxy);
Expand All @@ -278,6 +280,11 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape

// Note that onReisze is not being listened to here as it needs to fire when max dimensions
// change, excluding the dimension override
const initialDimensions: ITerminalDimensionsDto | undefined = request.cols && request.rows ? {
columns: request.cols,
rows: request.rows
} : undefined;
this._proxy.$startVirtualProcess(proxy.terminalId, initialDimensions);
proxy.onInput(data => this._proxy.$acceptProcessInput(proxy.terminalId, data));
proxy.onShutdown(immediate => this._proxy.$acceptProcessShutdown(proxy.terminalId, immediate));
proxy.onRequestCwd(() => this._proxy.$acceptProcessRequestCwd(proxy.terminalId));
Expand Down
6 changes: 6 additions & 0 deletions src/vs/workbench/api/common/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,11 @@ export interface IShellAndArgsDto {
args: string[] | string | undefined;
}

export interface ITerminalDimensionsDto {
columns: number;
rows: number;
}

export interface ExtHostTerminalServiceShape {
$acceptTerminalClosed(id: number): void;
$acceptTerminalOpened(id: number, name: string): void;
Expand All @@ -1146,6 +1151,7 @@ export interface ExtHostTerminalServiceShape {
$acceptTerminalDimensions(id: number, cols: number, rows: number): void;
$acceptTerminalMaximumDimensions(id: number, cols: number, rows: number): void;
$createProcess(id: number, shellLaunchConfig: ShellLaunchConfigDto, activeWorkspaceRootUri: UriComponents, cols: number, rows: number, isWorkspaceShellAllowed: boolean): void;
$startVirtualProcess(id: number, initialDimensions: ITerminalDimensionsDto | undefined): void;
$acceptProcessInput(id: number, data: string): void;
$acceptProcessResize(id: number, cols: number, rows: number): void;
$acceptProcessShutdown(id: number, immediate: boolean): void;
Expand Down
16 changes: 8 additions & 8 deletions src/vs/workbench/api/node/extHostTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { URI, UriComponents } from 'vs/base/common/uri';
import * as platform from 'vs/base/common/platform';
import * as terminalEnvironment from 'vs/workbench/contrib/terminal/common/terminalEnvironment';
import { Event, Emitter } from 'vs/base/common/event';
import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShape, IMainContext, ShellLaunchConfigDto, IShellDefinitionDto, IShellAndArgsDto } from 'vs/workbench/api/common/extHost.protocol';
import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShape, IMainContext, ShellLaunchConfigDto, IShellDefinitionDto, IShellAndArgsDto, ITerminalDimensionsDto } from 'vs/workbench/api/common/extHost.protocol';
import { ExtHostConfiguration, ExtHostConfigProvider } from 'vs/workbench/api/common/extHostConfiguration';
import { ILogService } from 'vs/platform/log/common/log';
import { EXT_HOST_CREATION_DELAY, IShellLaunchConfig, ITerminalEnvironment, ITerminalChildProcess, ITerminalDimensions } from 'vs/workbench/contrib/terminal/common/terminal';
Expand Down Expand Up @@ -329,10 +329,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
public createVirtualProcessTerminal(options: vscode.TerminalVirtualProcessOptions): vscode.Terminal {
const terminal = new ExtHostTerminal(this._proxy, options.name);
const p = new ExtHostVirtualProcess(options.virtualProcess);
terminal.createVirtualProcess().then(() => {
this._setupExtHostProcessListeners(terminal._id, p);
p.startSendingEvents();
});
terminal.createVirtualProcess().then(() => this._setupExtHostProcessListeners(terminal._id, p));
this._terminals.push(terminal);
return terminal;
}
Expand All @@ -344,7 +341,6 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
}
const p = new ExtHostVirtualProcess(virtualProcess);
this._setupExtHostProcessListeners(id, p);
p.startSendingEvents();
}

public createTerminalRenderer(name: string): vscode.TerminalRenderer {
Expand Down Expand Up @@ -585,6 +581,10 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
this._setupExtHostProcessListeners(id, new TerminalProcess(shellLaunchConfig, initialCwd, cols, rows, env, enableConpty, this._logService));
}

public $startVirtualProcess(id: number, initialDimensions: ITerminalDimensionsDto | undefined): void {
(this._terminalProcesses[id] as ExtHostVirtualProcess).startSendingEvents(initialDimensions);
}

private _setupExtHostProcessListeners(id: number, p: ITerminalChildProcess): void {
p.onProcessReady((e: { pid: number, cwd: string }) => this._proxy.$sendProcessReady(id, e.pid, e.cwd));
p.onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title));
Expand Down Expand Up @@ -779,7 +779,7 @@ class ExtHostVirtualProcess implements ITerminalChildProcess {
return Promise.resolve(0);
}

startSendingEvents(): void {
startSendingEvents(initialDimensions: ITerminalDimensionsDto | undefined): void {
// Flush all buffered events
this._queuedEvents.forEach(e => (<any>e.emitter.fire)(e.data));
this._queuedEvents = [];
Expand All @@ -798,7 +798,7 @@ class ExtHostVirtualProcess implements ITerminalChildProcess {
}

if (this._virtualProcess.start) {
this._virtualProcess.start();
this._virtualProcess.start(initialDimensions);
}
}
}
Expand Down
37 changes: 30 additions & 7 deletions src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,21 @@ export const DEFAULT_COMMANDS_TO_SKIP_SHELL: string[] = [

let xtermConstructor: Promise<typeof XTermTerminal> | undefined;

interface ICanvasDimensions {
width: number;
height: number;
}

interface IGridDimensions {
cols: number;
rows: number;
}

export class TerminalInstance implements ITerminalInstance {
private static readonly EOL_REGEX = /\r?\n/g;

private static _lastKnownDimensions: dom.Dimension | null = null;
private static _lastKnownCanvasDimensions: ICanvasDimensions | undefined;
private static _lastKnownGridDimensions: IGridDimensions | undefined;
private static _idCounter = 1;

private _processManager: ITerminalProcessManager | undefined;
Expand Down Expand Up @@ -327,16 +338,19 @@ export class TerminalInstance implements ITerminalInstance {
private _evaluateColsAndRows(width: number, height: number): number | null {
// Ignore if dimensions are undefined or 0
if (!width || !height) {
this._setLastKnownColsAndRows();
return null;
}

const dimension = this._getDimension(width, height);
if (!dimension) {
this._setLastKnownColsAndRows();
return null;
}

const font = this._configHelper.getFont(this._xterm);
if (!font.charWidth || !font.charHeight) {
this._setLastKnownColsAndRows();
return null;
}

Expand Down Expand Up @@ -368,21 +382,28 @@ export class TerminalInstance implements ITerminalInstance {
return dimension.width;
}

private _setLastKnownColsAndRows(): void {
if (TerminalInstance._lastKnownGridDimensions) {
this._cols = TerminalInstance._lastKnownGridDimensions.cols;
this._rows = TerminalInstance._lastKnownGridDimensions.rows;
}
}

@debounce(50)
private _fireMaximumDimensionsChanged(): void {
this._onMaximumDimensionsChanged.fire();
}

private _getDimension(width: number, height: number): dom.Dimension | null {
private _getDimension(width: number, height: number): ICanvasDimensions | undefined {
// The font needs to have been initialized
const font = this._configHelper.getFont(this._xterm);
if (!font || !font.charWidth || !font.charHeight) {
return null;
return undefined;
}

// The panel is minimized
if (!this._isVisible) {
return TerminalInstance._lastKnownDimensions;
return TerminalInstance._lastKnownCanvasDimensions;
} else {
// Trigger scroll event manually so that the viewport's scroll area is synced. This
// needs to happen otherwise its scrollTop value is invalid when the panel is toggled as
Expand All @@ -394,7 +415,7 @@ export class TerminalInstance implements ITerminalInstance {
}

if (!this._wrapperElement) {
return null;
return undefined;
}

const wrapperElementStyle = getComputedStyle(this._wrapperElement);
Expand All @@ -405,8 +426,8 @@ export class TerminalInstance implements ITerminalInstance {
const innerWidth = width - marginLeft - marginRight;
const innerHeight = height - bottom;

TerminalInstance._lastKnownDimensions = new dom.Dimension(innerWidth, innerHeight);
return TerminalInstance._lastKnownDimensions;
TerminalInstance._lastKnownCanvasDimensions = new dom.Dimension(innerWidth, innerHeight);
return TerminalInstance._lastKnownCanvasDimensions;
}

private async _getXtermConstructor(): Promise<typeof XTermTerminal> {
Expand Down Expand Up @@ -1295,7 +1316,9 @@ export class TerminalInstance implements ITerminalInstance {
if (cols !== this._xterm.cols || rows !== this._xterm.rows) {
this._onDimensionsChanged.fire();
}

this._xterm.resize(cols, rows);
TerminalInstance._lastKnownGridDimensions = { cols, rows };

if (this._isVisible) {
// HACK: Force the renderer to unpause by simulating an IntersectionObserver event.
Expand Down
10 changes: 8 additions & 2 deletions src/vs/workbench/contrib/terminal/common/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ export interface ITerminalService {
onInstanceDimensionsChanged: Event<ITerminalInstance>;
onInstanceMaximumDimensionsChanged: Event<ITerminalInstance>;
onInstanceRequestExtHostProcess: Event<ITerminalProcessExtHostRequest>;
onInstanceRequestVirtualProcess: Event<ITerminalProcessExtHostProxy>;
onInstanceRequestVirtualProcess: Event<ITerminalVirtualProcessRequest>;
onInstancesChanged: Event<void>;
onInstanceTitleChanged: Event<ITerminalInstance>;
onActiveInstanceChanged: Event<ITerminalInstance | undefined>;
Expand Down Expand Up @@ -297,7 +297,7 @@ export interface ITerminalService {

extHostReady(remoteAuthority: string): void;
requestExtHostProcess(proxy: ITerminalProcessExtHostProxy, shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI, cols: number, rows: number, isWorkspaceShellAllowed: boolean): void;
requestVirtualProcess(proxy: ITerminalProcessExtHostProxy): void;
requestVirtualProcess(proxy: ITerminalProcessExtHostProxy, cols: number, rows: number): void;
}

/**
Expand Down Expand Up @@ -767,6 +767,12 @@ export interface ITerminalProcessExtHostRequest {
isWorkspaceShellAllowed: boolean;
}

export interface ITerminalVirtualProcessRequest {
proxy: ITerminalProcessExtHostProxy;
cols: number;
rows: number;
}

export interface IAvailableShellsRequest {
(shells: IShellDefinition[]): void;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class TerminalProcessExtHostProxy extends Disposable implements ITerminal
// Request a process if needed, if this is a virtual process this step can be skipped as
// there is no real "process" and we know it's ready on the ext host already.
if (shellLaunchConfig.isVirtualProcess) {
this._terminalService.requestVirtualProcess(this);
this._terminalService.requestVirtualProcess(this, cols, rows);
} else {
remoteAgentService.getEnvironment().then(env => {
if (!env) {
Expand Down
Loading

0 comments on commit 2443cf5

Please sign in to comment.