From c5b970dcb9684853e1a4e470443261eb7b30a42a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adriana=20Louren=C3=A7o?= Date: Fri, 27 Mar 2026 16:21:00 +0000 Subject: [PATCH 1/6] Fix #294007: Eliminate terminal selection gap at high DPI/zoom levels Fixes a rendering inconsistency in the integrated terminal on macOS high-DPI displays where a visible gap could appear between the selection highlight and adjacent UI elements (tabs/divider). The gap was sensitive to zoom level and caused by mismatched pixel alignment between xterm rendering and container layout. Introduces an offset-column rendering approach where xterm renders one additional column while preserving base dimensions for PTY/process semantics. Aligns terminal and UI boundaries by ensuring consistent width calculations and accounting for cell width when communicating dimensions to the PTY. Refines viewport and resize synchronization and internal gap computation to ensure consistent behaviour across devicePixelRatio and zoom levels. Adds tests to verify: - separation between render columns and PTY dimensions - correct process sizing (no offset leakage) - selection gap remains within acceptable bounds This ensures the selection highlight consistently aligns with adjacent UI elements across all zoom/DPR configurations. --- .../contrib/terminal/browser/media/xterm.css | 19 ++ .../terminal/browser/terminalInstance.ts | 50 +++-- .../terminal/browser/xterm/xtermTerminal.ts | 172 +++++++++++++++++- .../test/browser/terminalInstance.test.ts | 121 ++++++++++++ .../test/browser/xterm/xtermTerminal.test.ts | 32 ++++ 5 files changed, 373 insertions(+), 21 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/media/xterm.css b/src/vs/workbench/contrib/terminal/browser/media/xterm.css index 9da68d13f61db..57f30bfd186e9 100644 --- a/src/vs/workbench/contrib/terminal/browser/media/xterm.css +++ b/src/vs/workbench/contrib/terminal/browser/media/xterm.css @@ -278,10 +278,29 @@ pointer-events: none; } +.xterm.always-show-scrollbar .xterm-scrollable-element > .xterm-scrollbar { + display: block !important; + opacity: 1 !important; + pointer-events: auto !important; +} + +.xterm.always-show-scrollbar .xterm-scrollable-element > .xterm-scrollbar > .xterm-slider { + opacity: 1 !important; +} + +.xterm.always-show-scrollbar .xterm-scrollable-element > .xterm-invisible { + opacity: 1; + pointer-events: auto; +} + .xterm .xterm-scrollable-element > .xterm-invisible.xterm-fade { transition: opacity 800ms linear; } +.xterm.always-show-scrollbar .xterm-scrollable-element > .xterm-invisible.xterm-fade { + transition: none; +} + /* Scrollable Content Inset Shadow */ .xterm .xterm-scrollable-element > .xterm-shadow { position: absolute; diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index ca7d1e3edebc3..da9749f2c258e 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -104,7 +104,7 @@ const enum Constants { DefaultCols = 80, DefaultRows = 30, - MaxCanvasWidth = 4096 + MaxCanvasWidth = 16384 } let xtermConstructor: Promise | undefined; @@ -742,6 +742,10 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { } } + private _toXtermCols(cols: number): number { + return Math.max(cols + 1, 2); + } + @debounce(50) private _fireMaximumDimensionsChanged(): void { this._onMaximumDimensionsChanged.fire(); @@ -758,7 +762,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { return undefined; } const computedStyle = dom.getWindow(this.xterm.raw.element).getComputedStyle(this.xterm.raw.element); - const horizontalPadding = parseInt(computedStyle.paddingLeft) + parseInt(computedStyle.paddingRight) + 14/*scroll bar padding*/; + const horizontalPadding = parseInt(computedStyle.paddingLeft) + parseInt(computedStyle.paddingRight) + (this.xterm?.viewportRightOffset ?? 0); const verticalPadding = parseInt(computedStyle.paddingTop) + parseInt(computedStyle.paddingBottom); TerminalInstance._lastKnownCanvasDimensions = new dom.Dimension( Math.min(Constants.MaxCanvasWidth, width - horizontalPadding), @@ -795,7 +799,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { const disableShellIntegrationReporting = (this.shellLaunchConfig.executable === undefined || this.shellType === undefined) || !shellIntegrationSupportedShellTypes.includes(this.shellType); const xterm = this._scopedInstantiationService.createInstance(XtermTerminal, this._resource, Terminal, { - cols: this._cols, + cols: this._toXtermCols(this._cols || Constants.DefaultCols), rows: this._rows, xtermColorProvider: this._scopedInstantiationService.createInstance(TerminalInstanceColorProvider, this._targetRef), capabilities: this.capabilities, @@ -806,17 +810,20 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { this._resizeDebouncer = this._register(new TerminalResizeDebouncer( () => this._isVisible, () => xterm, - async (cols, rows) => { - xterm.resize(cols, rows); - await this._updatePtyDimensions(xterm.raw); + async (xtermCols, rows) => { + const ptyCols = this.cols; + xterm.resize(xtermCols, rows); + await this._updatePtyDimensions(xterm.raw, ptyCols, rows); }, - async (cols) => { - xterm.resize(cols, xterm.raw.rows); - await this._updatePtyDimensions(xterm.raw); + async (xtermCols) => { + const ptyCols = this.cols; + xterm.resize(xtermCols, xterm.raw.rows); + await this._updatePtyDimensions(xterm.raw, ptyCols, this.rows); }, async (rows) => { - xterm.resize(xterm.raw.cols, rows); - await this._updatePtyDimensions(xterm.raw); + const ptyCols = this.cols; + xterm.resize(this._toXtermCols(ptyCols), rows); + await this._updatePtyDimensions(xterm.raw, ptyCols, rows); } )); this._register(toDisposable(() => this._resizeDebouncer = undefined)); @@ -1576,10 +1583,10 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { // Re-evaluate dimensions if the container has been set since the xterm instance was created if (this._container && this._cols === 0 && this._rows === 0) { this._initDimensions(); - this.xterm?.resize(this._cols || Constants.DefaultCols, this._rows || Constants.DefaultRows); + this.xterm?.resize(this._toXtermCols(this._cols || Constants.DefaultCols), this._rows || Constants.DefaultRows); } const originalIcon = this.shellLaunchConfig.icon; - await this._processManager.createProcess(this._shellLaunchConfig, this._cols || Constants.DefaultCols, this._rows || Constants.DefaultRows).then(result => { + await this._processManager.createProcess(this._shellLaunchConfig, this.cols || Constants.DefaultCols, this.rows || Constants.DefaultRows).then(result => { if (result) { if (hasKey(result, { message: true })) { this._onProcessExit(result); @@ -1855,7 +1862,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { // Set the new shell launch config this._shellLaunchConfig = shell; // Must be done before calling _createProcess() - await this._processManager.relaunch(this._shellLaunchConfig, this._cols || Constants.DefaultCols, this._rows || Constants.DefaultRows, reset).then(result => { + await this._processManager.relaunch(this._shellLaunchConfig, this.cols || Constants.DefaultCols, this.rows || Constants.DefaultRows, reset).then(result => { if (result) { if (hasKey(result, { message: true })) { this._onProcessExit(result); @@ -2004,8 +2011,9 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { if (isNaN(cols) || isNaN(rows)) { return; } + const xtermCols = this._toXtermCols(cols); - if (cols !== this.xterm.raw.cols || rows !== this.xterm.raw.rows) { + if (xtermCols !== this.xterm.raw.cols || rows !== this.xterm.raw.rows) { if (this._fixedRows || this._fixedCols) { await this._updateProperty(ProcessPropertyType.FixedDimensions, { cols: this._fixedCols, rows: this._fixedRows }); } @@ -2013,15 +2021,17 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { } TerminalInstance._lastKnownGridDimensions = { cols, rows }; - this._resizeDebouncer!.resize(cols, rows, immediate ?? false); + this._resizeDebouncer!.resize(xtermCols, rows, immediate ?? false); } - private async _updatePtyDimensions(rawXterm: XTermTerminal): Promise { + private async _updatePtyDimensions(rawXterm: XTermTerminal, ptyCols: number = this.cols, ptyRows: number = this.rows): Promise { const pixelWidth = rawXterm.dimensions?.css.canvas.width; const pixelHeight = rawXterm.dimensions?.css.canvas.height; - const roundedPixelWidth = pixelWidth ? Math.round(pixelWidth) : undefined; + const cellWidth = rawXterm.dimensions?.css.cell.width; + const adjustedPixelWidth = pixelWidth && cellWidth ? Math.max(0, pixelWidth - cellWidth) : pixelWidth; + const roundedPixelWidth = adjustedPixelWidth ? Math.round(adjustedPixelWidth) : undefined; const roundedPixelHeight = pixelHeight ? Math.round(pixelHeight) : undefined; - await this._processManager.setDimensions(rawXterm.cols, rawXterm.rows, undefined, roundedPixelWidth, roundedPixelHeight); + await this._processManager.setDimensions(ptyCols, ptyRows, undefined, roundedPixelWidth, roundedPixelHeight); } setShellType(shellType: TerminalShellType | undefined) { @@ -2168,7 +2178,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { const proposedCols = Math.max(this.maxCols, Math.min(this.xterm.getLongestViewportWrappedLineLength(), maxColsForTexture)); // Don't switch to fixed dimensions if the content already fits as it makes the scroll // bar look bad being off the edge - if (proposedCols > this.xterm.raw.cols) { + if (proposedCols > this.cols) { this._fixedCols = proposedCols; } } diff --git a/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts b/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts index b043728acf3dc..01868b4a7e7e6 100644 --- a/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import type { IBuffer, ITerminalOptions, ITheme, Terminal as RawXtermTerminal, LogLevel as XtermLogLevel, IMarker as IXtermMarker } from '@xterm/xterm'; +import type { IBuffer, IBufferRange, ITerminalOptions, ITheme, Terminal as RawXtermTerminal, LogLevel as XtermLogLevel, IMarker as IXtermMarker } from '@xterm/xterm'; import type { ISearchOptions, SearchAddon as SearchAddonType } from '@xterm/addon-search'; import type { Unicode11Addon as Unicode11AddonType } from '@xterm/addon-unicode11'; import type { ILigatureOptions, LigaturesAddon as LigaturesAddonType } from '@xterm/addon-ligatures'; @@ -48,6 +48,7 @@ import type { CommandDetectionCapability } from '../../../../../platform/termina import { URI } from '../../../../../base/common/uri.js'; import { isNumber } from '../../../../../base/common/types.js'; import { clamp } from '../../../../../base/common/numbers.js'; +import { onDidChangeZoomLevel } from '../../../../../base/browser/browser.js'; const enum RenderConstants { SmoothScrollDuration = 125 @@ -107,9 +108,11 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach private static _suggestedRendererType: 'dom' | undefined = undefined; private _attached?: { container: HTMLElement; options: IXtermAttachToElementOptions }; + private _selectionGapAnimationFrame: number | undefined; private _isPhysicalMouseWheel = MouseWheelClassifier.INSTANCE.isPhysicalMouseWheel(); private _lastInputEvent: string | undefined; get lastInputEvent(): string | undefined { return this._lastInputEvent; } + get viewportRightOffset(): number { return this.raw.options.scrollbar?.width ?? 0; } private _progressState: IProgressState = { state: 0, value: 0 }; get progressState(): IProgressState { return this._progressState; } get buffer() { return this.raw.buffer; } @@ -295,6 +298,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach if (this.isFocused) { this._anyFocusedTerminalHasSelection.set(this.raw.hasSelection()); } + this._scheduleSelectionGapMetrics('selectionChange'); })); this._register(this.raw.onData(e => this._lastInputEvent = e)); @@ -479,6 +483,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach if (!this._attached) { this.raw.open(container); } + this._updateViewportRightOffset(); // TODO: Move before open so the DOM renderer doesn't initialize if (options.enableGpu) { @@ -490,6 +495,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach if (!this.raw.element || !this.raw.textarea) { throw new Error('xterm elements not set after open'); } + this.raw.element.classList.add('always-show-scrollbar'); const ad = this._attachedDisposables; ad.clear(); @@ -508,15 +514,175 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach this._updateSmoothScrolling(); } }, { passive: true })); + const window = dom.getWindow(this.raw.element); + ad.add(dom.addDisposableListener(window, dom.EventType.RESIZE, () => this._triggerViewportSync('windowResize'))); + ad.add(onDidChangeZoomLevel(targetWindowId => { + if (targetWindowId === dom.getWindowId(window)) { + this._triggerViewportSync('zoomLevelChange'); + } + })); + ad.add(this.raw.onResize(() => { + this._updateViewportRightOffset(); + this._scheduleSelectionGapMetrics('xtermResize'); + })); this._refreshLigaturesAddon(); this._attached = { container, options }; + this._scheduleSelectionGapMetrics('attachToElement'); // Screen must be created at this point as xterm.open is called // eslint-disable-next-line no-restricted-syntax return this._attached?.container.querySelector('.xterm-screen')!; } + private _triggerViewportSync(reason: string): void { + this._updateViewportRightOffset(); + if (this.raw.rows > 0) { + this.raw.refresh(0, this.raw.rows - 1); + } + this._onDidRequestRefreshDimensions.fire(); + this._scheduleSelectionGapMetrics(reason); + + const targetWindow = this.raw.element ? dom.getWindow(this.raw.element) : undefined; + if (!targetWindow) { + return; + } + targetWindow.requestAnimationFrame(() => { + if (!this.raw.element) { + return; + } + this._updateViewportRightOffset(); + this.forceRefresh(); + this._onDidRequestRefreshDimensions.fire(); + this._scheduleSelectionGapMetrics(`${reason}:raf`); + }); + } + + private _updateViewportRightOffset(): void { + if (!this.raw.element) { + return; + } + this.raw.element.classList.add('always-show-scrollbar'); + const viewportElement = (this._core.viewport as { _viewportElement?: unknown } | undefined)?._viewportElement; + if (!dom.isHTMLElement(viewportElement)) { + return; + } + viewportElement.style.right = `${this.viewportRightOffset}px`; + } + + private _scheduleSelectionGapMetrics(reason: string): void { + if (!this.raw.element) { + return; + } + const window = dom.getWindow(this.raw.element); + if (this._selectionGapAnimationFrame !== undefined) { + window.cancelAnimationFrame(this._selectionGapAnimationFrame); + } + this._selectionGapAnimationFrame = window.requestAnimationFrame(() => { + this._selectionGapAnimationFrame = undefined; + try { + this._computeSelectionGapPhysical(reason); + } catch { + } + }); + } + + private _computeSelectionGapPhysical(_reason: string): number | undefined { + if (!this.raw.element || !this.raw.hasSelection()) { + return undefined; + } + + const selectionMeasurement = this._getSelectionRightEdgeCssPx(); + if (!selectionMeasurement) { + return undefined; + } + + const adjacentMeasurement = this._getAdjacentUiLeftEdgeCssPx(selectionMeasurement.rightCssPx); + if (!adjacentMeasurement) { + return undefined; + } + + const dpr = dom.getWindow(this.raw.element).devicePixelRatio; + const viewportScrollbarWidthCssPx = this._core.viewport?.scrollBarWidth ?? 0; + const domReservedRightCssPx = this._getTerminalReservedRightLaneCssPx(); + const terminalScrollbarWidthCssPx = Math.max(viewportScrollbarWidthCssPx, domReservedRightCssPx); + const uiLeftCssPxWithoutScrollbar = adjacentMeasurement.leftCssPx - terminalScrollbarWidthCssPx; + const selectionRightPhysicalPx = selectionMeasurement.rightCssPx * dpr; + const uiLeftPhysicalPx = uiLeftCssPxWithoutScrollbar * dpr; + const gapPhysicalPx = uiLeftPhysicalPx - selectionRightPhysicalPx; + return gapPhysicalPx; + } + + private _getSelectionRightEdgeCssPx(): { rightCssPx: number; source: string; selectionRange: IBufferRange } | undefined { + const selectionRange = this.raw.getSelectionPosition(); + if (!selectionRange) { + return undefined; + } + + const cellWidth = this._core._renderService?.dimensions?.css?.cell?.width; + const screenElement = this.raw.screenElement; + if (!screenElement || !cellWidth) { + return undefined; + } + + const rightmostSelectedCol = selectionRange.end.y > selectionRange.start.y + ? this.raw.cols + : clamp(selectionRange.end.x, 1, this.raw.cols); + const fallbackRightCssPx = screenElement.getBoundingClientRect().left + (rightmostSelectedCol * cellWidth); + return { rightCssPx: fallbackRightCssPx, source: 'selection-range-fallback', selectionRange }; + } + + private _getTerminalReservedRightLaneCssPx(): number { + if (!this.raw.element) { + return this.viewportRightOffset; + } + + const viewportElement = (this._core.viewport as { _viewportElement?: unknown } | undefined)?._viewportElement; + if (!dom.isHTMLElement(viewportElement)) { + return this.viewportRightOffset; + } + + const terminalRightCssPx = this.raw.element.getBoundingClientRect().right; + const viewportRightCssPx = viewportElement.getBoundingClientRect().right; + return Math.max(0, terminalRightCssPx - viewportRightCssPx); + } + + private _getAdjacentUiLeftEdgeCssPx(selectionRightCssPx: number): { leftCssPx: number; source: string } | undefined { + if (!this.raw.element) { + return undefined; + } + + const candidates: { leftCssPx: number; source: string }[] = []; + const terminalSplitViewElement = dom.findParentWithClass(this.raw.element, 'split-view-view'); + if (terminalSplitViewElement) { + const rightSibling = terminalSplitViewElement.nextElementSibling; + if (dom.isHTMLElement(rightSibling)) { + candidates.push({ + leftCssPx: rightSibling.getBoundingClientRect().left, + source: 'split-view-right-sibling' + }); + } else { + candidates.push({ + leftCssPx: terminalSplitViewElement.getBoundingClientRect().right, + source: 'terminal-split-view-right-edge' + }); + } + } + + if (candidates.length === 0) { + return undefined; + } + + const candidateOnRight = candidates + .filter(candidate => candidate.leftCssPx >= selectionRightCssPx - 0.5) + .sort((a, b) => a.leftCssPx - b.leftCssPx)[0]; + if (candidateOnRight) { + return candidateOnRight; + } + + return candidates.sort((a, b) => Math.abs(a.leftCssPx - selectionRightCssPx) - Math.abs(b.leftCssPx - selectionRightCssPx))[0]; + } + private _setFocused(isFocused: boolean) { this._onDidChangeFocus.fire(isFocused); this._anyTerminalFocusContextKey.set(isFocused); @@ -1059,6 +1225,10 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach } override dispose(): void { + if (this._selectionGapAnimationFrame !== undefined && this.raw.element) { + dom.getWindow(this.raw.element).cancelAnimationFrame(this._selectionGapAnimationFrame); + this._selectionGapAnimationFrame = undefined; + } this._anyTerminalFocusContextKey.reset(); this._anyFocusedTerminalHasSelection.reset(); this._disposeOfWebglRenderer(); diff --git a/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts b/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts index b5f566ce0de85..5435ae8a48929 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts @@ -103,6 +103,7 @@ class TestTerminalInstanceService extends Disposable implements Partial ({}), createProcess: async ( shellLaunchConfig: IShellLaunchConfig, cwd: string, @@ -118,6 +119,36 @@ class TestTerminalInstanceService extends Disposable implements Partial { + public createProcessArgs: { cols: number; rows: number } | undefined; + + async getBackend() { + return { + onPtyHostExit: Event.None, + onPtyHostUnresponsive: Event.None, + onPtyHostResponsive: Event.None, + onPtyHostRestart: Event.None, + onDidMoveWindowInstance: Event.None, + onDidRequestDetach: Event.None, + getShellEnvironment: async () => ({}), + createProcess: async ( + shellLaunchConfig: IShellLaunchConfig, + cwd: string, + cols: number, + rows: number, + unicodeVersion: '6' | '11', + env: IProcessEnvironment, + options: ITerminalProcessOptions, + shouldPersist: boolean + ) => { + this.createProcessArgs = { cols, rows }; + return this._register(new TestTerminalChildProcess(shouldPersist)); + }, + getLatency: () => Promise.resolve([]) + } as unknown as ITerminalBackend; + } +} + suite('Workbench - TerminalInstance', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); @@ -193,6 +224,96 @@ suite('Workbench - TerminalInstance', () => { // Verify that the task name is preserved strictEqual(taskTerminal.title, 'Test Task Name', 'Task terminal should preserve API-set title'); }); + + test('should render one extra xterm column while keeping PTY columns unchanged', async () => { + const instantiationService = workbenchInstantiationService({ + configurationService: () => new TestConfigurationService({ + files: {}, + terminal: { + integrated: { + fontFamily: 'monospace', + scrollback: 1000, + fastScrollSensitivity: 2, + mouseWheelScrollSensitivity: 1, + unicodeVersion: '6', + commandsToSkipShell: [], + shellIntegration: { + enabled: true + } + } + }, + }) + }, store); + instantiationService.set(ITerminalProfileResolverService, new MockTerminalProfileResolverService()); + instantiationService.stub(IViewDescriptorService, new TestViewDescriptorService()); + instantiationService.stub(IEnvironmentVariableService, store.add(instantiationService.createInstance(EnvironmentVariableService))); + instantiationService.stub(ITerminalInstanceService, store.add(new TestTerminalInstanceService())); + instantiationService.stub(ITerminalService, { setNextCommandId: async () => { } } as Partial); + + const instance = store.add(instantiationService.createInstance(TerminalInstance, terminalShellTypeContextKey, {})) as unknown as { + _xtermReadyPromise: Promise; + _cols: number; + _rows: number; + _isVisible: boolean; + _layoutSettingsChanged: boolean; + _processManager: { setDimensions: (...args: unknown[]) => Promise }; + xterm?: { raw: { cols: number } }; + }; + + await instance._xtermReadyPromise; + instance._cols = 80; + instance._rows = 24; + instance._isVisible = true; + instance._layoutSettingsChanged = false; + + let ptyDimensions: unknown[] | undefined; + instance._processManager.setDimensions = async (...args: unknown[]) => { + ptyDimensions = args; + }; + + const resize = (instance as unknown as Record)['_resize'] as (this: typeof instance, immediate?: boolean) => Promise; + await resize.call(instance, true); + + strictEqual(instance.xterm?.raw.cols, 81, 'xterm should render one extra offset column'); + strictEqual(ptyDimensions?.[0], 80, 'PTY should keep base columns'); + strictEqual(ptyDimensions?.[1], 24, 'PTY should keep base rows'); + }); + + test('should create process with base columns (without offset column)', async () => { + const instantiationService = workbenchInstantiationService({ + configurationService: () => new TestConfigurationService({ + files: {}, + terminal: { + integrated: { + fontFamily: 'monospace', + scrollback: 1000, + fastScrollSensitivity: 2, + mouseWheelScrollSensitivity: 1, + unicodeVersion: '6', + shellIntegration: { + enabled: true + } + } + }, + }) + }, store); + instantiationService.set(ITerminalProfileResolverService, new MockTerminalProfileResolverService()); + instantiationService.stub(IViewDescriptorService, new TestViewDescriptorService()); + instantiationService.stub(IEnvironmentVariableService, store.add(instantiationService.createInstance(EnvironmentVariableService))); + const recordingInstanceService = store.add(new RecordingTerminalInstanceService()); + instantiationService.stub(ITerminalInstanceService, recordingInstanceService); + instantiationService.stub(ITerminalService, { setNextCommandId: async () => { } } as Partial); + + const instance = store.add(instantiationService.createInstance(TerminalInstance, terminalShellTypeContextKey, {})) as unknown as { + _xtermReadyPromise: Promise; + }; + await instance._xtermReadyPromise; + const createProcess = (instance as unknown as Record)['_createProcess'] as (this: typeof instance) => Promise; + await createProcess.call(instance); + + strictEqual(recordingInstanceService.createProcessArgs?.cols, 80, 'process should be created with base cols'); + strictEqual(recordingInstanceService.createProcessArgs?.rows, 30, 'process should be created with default rows'); + }); }); suite('parseExitResult', () => { test('should return no message for exit code = undefined', () => { diff --git a/src/vs/workbench/contrib/terminal/test/browser/xterm/xtermTerminal.test.ts b/src/vs/workbench/contrib/terminal/test/browser/xterm/xtermTerminal.test.ts index 8db8fe5cffc78..1a8fef8972841 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/xterm/xtermTerminal.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/xterm/xtermTerminal.test.ts @@ -109,6 +109,38 @@ suite('XtermTerminal', () => { strictEqual(xterm.raw.rows, 30); }); + test('should compute selection gapPhysical <= 0 for selected text', async () => { + const paneBody = document.createElement('div'); + paneBody.className = 'pane-body integrated-terminal'; + const splitView = document.createElement('div'); + splitView.className = 'split-view-view'; + const terminalHost = document.createElement('div'); + const rightSibling = document.createElement('div'); + paneBody.appendChild(splitView); + paneBody.appendChild(rightSibling); + splitView.appendChild(terminalHost); + document.body.appendChild(paneBody); + + try { + xterm.attachToElement(terminalHost); + await write('selected text\r\n'); + xterm.raw.select(0, 0, 8); + + const xtermWithInternals = xterm as unknown as Record; + const computeSelectionGapPhysical = xtermWithInternals['_computeSelectionGapPhysical'] as (this: XtermTerminal, reason: string) => number | undefined; + const gapPhysical = computeSelectionGapPhysical.call(xterm, 'testSelectionGapAssertion'); + + strictEqual(gapPhysical !== undefined, true, 'Expected selection gapPhysical to be computed'); + if (gapPhysical === undefined) { + return; + } + strictEqual(Number.isFinite(gapPhysical), true, 'Expected parsed gapPhysical to be a finite number'); + strictEqual(gapPhysical <= 0, true, `Expected gapPhysical <= 0, got ${gapPhysical}`); + } finally { + paneBody.remove(); + } + }); + suite('getContentsAsText', () => { test('should return all buffer contents when no markers provided', async () => { await write('line 1\r\nline 2\r\nline 3\r\nline 4\r\nline 5'); From 01bbcbed770d19bc745024f7121f6d9dfcd0e6b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adriana=20Louren=C3=A7o?= Date: Fri, 27 Mar 2026 19:56:25 +0000 Subject: [PATCH 2/6] Add clarifying comments for terminal render/PTY sizing logic Explain the intent behind terminal sizing changes from the previous fix: - xterm renders one extra column for right-edge alignment - PTY/process dimensions continue to use base columns - viewport sync runs on the next frame after resize/zoom - selection-gap metric is documented for regression reasoning - tests include clearer setup and assertion intent comments --- src/vs/workbench/contrib/terminal/browser/media/xterm.css | 1 + .../contrib/terminal/browser/terminalInstance.ts | 4 ++++ .../contrib/terminal/browser/xterm/xtermTerminal.ts | 8 ++++++++ .../terminal/test/browser/terminalInstance.test.ts | 4 ++++ .../terminal/test/browser/xterm/xtermTerminal.test.ts | 4 ++++ 5 files changed, 21 insertions(+) diff --git a/src/vs/workbench/contrib/terminal/browser/media/xterm.css b/src/vs/workbench/contrib/terminal/browser/media/xterm.css index 57f30bfd186e9..d0b08ca0d2151 100644 --- a/src/vs/workbench/contrib/terminal/browser/media/xterm.css +++ b/src/vs/workbench/contrib/terminal/browser/media/xterm.css @@ -278,6 +278,7 @@ pointer-events: none; } +/* Always show the scrollbar lane so the terminal's right edge stays consistent across zoom and DPI changes. */ .xterm.always-show-scrollbar .xterm-scrollable-element > .xterm-scrollbar { display: block !important; opacity: 1 !important; diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index da9749f2c258e..ce3772f7134cb 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -743,6 +743,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { } private _toXtermCols(cols: number): number { + // xterm renders one extra column on the right to hide tiny visual gaps caused by pixel rounding. return Math.max(cols + 1, 2); } @@ -799,6 +800,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { const disableShellIntegrationReporting = (this.shellLaunchConfig.executable === undefined || this.shellType === undefined) || !shellIntegrationSupportedShellTypes.includes(this.shellType); const xterm = this._scopedInstantiationService.createInstance(XtermTerminal, this._resource, Terminal, { + // xterm gets one extra render column for visual alignment; the PTY keeps base columns. cols: this._toXtermCols(this._cols || Constants.DefaultCols), rows: this._rows, xtermColorProvider: this._scopedInstantiationService.createInstance(TerminalInstanceColorProvider, this._targetRef), @@ -2028,6 +2030,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { const pixelWidth = rawXterm.dimensions?.css.canvas.width; const pixelHeight = rawXterm.dimensions?.css.canvas.height; const cellWidth = rawXterm.dimensions?.css.cell.width; + // Subtract that extra render column before reporting pixel width to the PTY so process sizing + // stays based on the real terminal columns. const adjustedPixelWidth = pixelWidth && cellWidth ? Math.max(0, pixelWidth - cellWidth) : pixelWidth; const roundedPixelWidth = adjustedPixelWidth ? Math.round(adjustedPixelWidth) : undefined; const roundedPixelHeight = pixelHeight ? Math.round(pixelHeight) : undefined; diff --git a/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts b/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts index 01868b4a7e7e6..52162f3ea290c 100644 --- a/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts @@ -551,6 +551,8 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach if (!this.raw.element) { return; } + // Run once more on the next animation frame. This catches DOM/layout changes that are + // applied right after resize or zoom events. this._updateViewportRightOffset(); this.forceRefresh(); this._onDidRequestRefreshDimensions.fire(); @@ -567,6 +569,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach if (!dom.isHTMLElement(viewportElement)) { return; } + // Shift the viewport by the scrollbar width so text and selection end at the same visual edge. viewportElement.style.right = `${this.viewportRightOffset}px`; } @@ -578,6 +581,8 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach if (this._selectionGapAnimationFrame !== undefined) { window.cancelAnimationFrame(this._selectionGapAnimationFrame); } + // Many events can fire quickly; do one measurement per frame to avoid redundant work and to + // use up-to-date layout values. this._selectionGapAnimationFrame = window.requestAnimationFrame(() => { this._selectionGapAnimationFrame = undefined; try { @@ -587,6 +592,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach }); } + // Computes the selection-to-adjacent-UI gap in physical pixels to catch alignment regressions. private _computeSelectionGapPhysical(_reason: string): number | undefined { if (!this.raw.element || !this.raw.hasSelection()) { return undefined; @@ -628,6 +634,8 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach const rightmostSelectedCol = selectionRange.end.y > selectionRange.start.y ? this.raw.cols : clamp(selectionRange.end.x, 1, this.raw.cols); + // If no DOM range is available for the selection, estimate the right edge from selected + // columns and the measured cell width. const fallbackRightCssPx = screenElement.getBoundingClientRect().left + (rightmostSelectedCol * cellWidth); return { rightCssPx: fallbackRightCssPx, source: 'selection-range-fallback', selectionRange }; } diff --git a/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts b/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts index 5435ae8a48929..fe980a004371b 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts @@ -267,6 +267,8 @@ suite('Workbench - TerminalInstance', () => { instance._layoutSettingsChanged = false; let ptyDimensions: unknown[] | undefined; + // Record what is sent to setDimensions so this test can verify xterm render columns and + // PTY columns are intentionally different. instance._processManager.setDimensions = async (...args: unknown[]) => { ptyDimensions = args; }; @@ -308,6 +310,8 @@ suite('Workbench - TerminalInstance', () => { _xtermReadyPromise: Promise; }; await instance._xtermReadyPromise; + // Call the private startup path directly so this test can assert the initial cols/rows + // used when the process is created. const createProcess = (instance as unknown as Record)['_createProcess'] as (this: typeof instance) => Promise; await createProcess.call(instance); diff --git a/src/vs/workbench/contrib/terminal/test/browser/xterm/xtermTerminal.test.ts b/src/vs/workbench/contrib/terminal/test/browser/xterm/xtermTerminal.test.ts index 1a8fef8972841..a87e0a5432412 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/xterm/xtermTerminal.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/xterm/xtermTerminal.test.ts @@ -110,6 +110,8 @@ suite('XtermTerminal', () => { }); test('should compute selection gapPhysical <= 0 for selected text', async () => { + // Create a small split-view-like DOM so the gap calculation can find the terminal's + // neighboring pane on the right. const paneBody = document.createElement('div'); paneBody.className = 'pane-body integrated-terminal'; const splitView = document.createElement('div'); @@ -127,6 +129,8 @@ suite('XtermTerminal', () => { xterm.raw.select(0, 0, 8); const xtermWithInternals = xterm as unknown as Record; + // Call the internal gap helper directly so this test can check the regression condition + // with a focused assertion. const computeSelectionGapPhysical = xtermWithInternals['_computeSelectionGapPhysical'] as (this: XtermTerminal, reason: string) => number | undefined; const gapPhysical = computeSelectionGapPhysical.call(xterm, 'testSelectionGapAssertion'); From a4893dff36edeb5283ec3f4fcdb2ffd4abba4848 Mon Sep 17 00:00:00 2001 From: dripfyyy Date: Sun, 29 Mar 2026 19:33:04 +0100 Subject: [PATCH 3/6] Update src/vs/workbench/contrib/terminal/browser/terminalInstance.ts Conservative upper bound to avoid exceeding typical GPU/browser max texture/canvas widths. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/vs/workbench/contrib/terminal/browser/terminalInstance.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index eb4aad492a058..740eb432fcfe2 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -104,7 +104,8 @@ const enum Constants { DefaultCols = 80, DefaultRows = 30, - MaxCanvasWidth = 16384 + // Conservative upper bound to avoid exceeding typical GPU/browser max texture/canvas widths. + MaxCanvasWidth = 8192 } let xtermConstructor: Promise | undefined; From c4b5c92c162cab5efcf0955bf19e06fe0e88b309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adriana=20Louren=C3=A7o?= Date: Tue, 31 Mar 2026 17:43:06 +0100 Subject: [PATCH 4/6] terminal: stabilize scrollbar lane without forcing visibility Reserve a consistent right scrollbar lane in terminal layout while preserving normal xterm UX: auto-hide, fade, hover reveal, and interaction when visible. Remove force-visible scrollbar CSS overrides. Separate lane reservation from visibility state. Use max configured/measured width for viewport offset. --- .../contrib/terminal/browser/media/xterm.css | 21 +++---------------- .../terminal/browser/xterm/xtermTerminal.ts | 6 +++++- .../test/browser/terminalInstance.test.ts | 6 +++--- 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/media/xterm.css b/src/vs/workbench/contrib/terminal/browser/media/xterm.css index d0b08ca0d2151..20fcf1cdba314 100644 --- a/src/vs/workbench/contrib/terminal/browser/media/xterm.css +++ b/src/vs/workbench/contrib/terminal/browser/media/xterm.css @@ -278,30 +278,15 @@ pointer-events: none; } -/* Always show the scrollbar lane so the terminal's right edge stays consistent across zoom and DPI changes. */ -.xterm.always-show-scrollbar .xterm-scrollable-element > .xterm-scrollbar { - display: block !important; - opacity: 1 !important; - pointer-events: auto !important; -} - -.xterm.always-show-scrollbar .xterm-scrollable-element > .xterm-scrollbar > .xterm-slider { - opacity: 1 !important; -} - -.xterm.always-show-scrollbar .xterm-scrollable-element > .xterm-invisible { - opacity: 1; - pointer-events: auto; +/* Reserve a stable right lane for the scrollbar without forcing visibility. */ +.xterm.always-show-scrollbar .xterm-viewport { + right: 14px; } .xterm .xterm-scrollable-element > .xterm-invisible.xterm-fade { transition: opacity 800ms linear; } -.xterm.always-show-scrollbar .xterm-scrollable-element > .xterm-invisible.xterm-fade { - transition: none; -} - /* Scrollable Content Inset Shadow */ .xterm .xterm-scrollable-element > .xterm-shadow { position: absolute; diff --git a/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts b/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts index 52162f3ea290c..b37093f3383dd 100644 --- a/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts @@ -112,7 +112,11 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach private _isPhysicalMouseWheel = MouseWheelClassifier.INSTANCE.isPhysicalMouseWheel(); private _lastInputEvent: string | undefined; get lastInputEvent(): string | undefined { return this._lastInputEvent; } - get viewportRightOffset(): number { return this.raw.options.scrollbar?.width ?? 0; } + get viewportRightOffset(): number { + const configuredScrollbarWidth = this.raw.options.scrollbar?.width ?? 0; + const viewportScrollbarWidth = this._core.viewport?.scrollBarWidth ?? 0; + return Math.max(configuredScrollbarWidth, viewportScrollbarWidth); + } private _progressState: IProgressState = { state: 0, value: 0 }; get progressState(): IProgressState { return this._progressState; } get buffer() { return this.raw.buffer; } diff --git a/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts b/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts index a209b9cf207c4..2bf87bc9a1ac9 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/terminalInstance.test.ts @@ -325,8 +325,8 @@ suite('Workbench - TerminalInstance', () => { strictEqual(recordingInstanceService.createProcessArgs?.cols, 80, 'process should be created with base cols'); strictEqual(recordingInstanceService.createProcessArgs?.rows, 30, 'process should be created with default rows'); - }); - + }); + test('custom key event handler should handle commands in DEFAULT_COMMANDS_TO_SKIP_SHELL in VS Code and not xterm when sendKeybindingsToShell is disabled', async () => { const instance = await createTerminalInstance(); const keybindingService = instance['_keybindingService']; @@ -378,7 +378,7 @@ suite('Workbench - TerminalInstance', () => { } }); }); - + suite('DEFAULT_COMMANDS_TO_SKIP_SHELL', () => { test('should include zoom commands so they are not consumed by kitty keyboard protocol', () => { deepStrictEqual( From 12c652f5aff39d0a05f679ac86d6ab00cafd7cc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adriana=20Louren=C3=A7o?= Date: Tue, 31 Mar 2026 19:56:09 +0100 Subject: [PATCH 5/6] terminal: coalesce viewport-sync RAF on resize/zoom Avoid queueing redundant deferred viewport sync callbacks when resize/zoom events fire rapidly. Preserve behavior: immediate sync plus one deferred sync. Track and cancel pending viewport-sync RAF on dispose to prevent stale callbacks after teardown. --- .../terminal/browser/xterm/xtermTerminal.ts | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts b/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts index b37093f3383dd..5d5672bf97846 100644 --- a/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts @@ -109,6 +109,9 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach private static _suggestedRendererType: 'dom' | undefined = undefined; private _attached?: { container: HTMLElement; options: IXtermAttachToElementOptions }; private _selectionGapAnimationFrame: number | undefined; + private _viewportSyncAnimationFrame: number | undefined; + private _viewportSyncAnimationFrameWindow: Window | undefined; + private _viewportSyncAnimationFrameReason: string | undefined; private _isPhysicalMouseWheel = MouseWheelClassifier.INSTANCE.isPhysicalMouseWheel(); private _lastInputEvent: string | undefined; get lastInputEvent(): string | undefined { return this._lastInputEvent; } @@ -551,7 +554,16 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach if (!targetWindow) { return; } - targetWindow.requestAnimationFrame(() => { + this._viewportSyncAnimationFrameReason = reason; + this._viewportSyncAnimationFrameWindow = targetWindow; + if (this._viewportSyncAnimationFrame !== undefined) { + return; + } + this._viewportSyncAnimationFrame = targetWindow.requestAnimationFrame(() => { + this._viewportSyncAnimationFrame = undefined; + const rafReason = this._viewportSyncAnimationFrameReason ?? reason; + this._viewportSyncAnimationFrameReason = undefined; + this._viewportSyncAnimationFrameWindow = undefined; if (!this.raw.element) { return; } @@ -560,7 +572,7 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach this._updateViewportRightOffset(); this.forceRefresh(); this._onDidRequestRefreshDimensions.fire(); - this._scheduleSelectionGapMetrics(`${reason}:raf`); + this._scheduleSelectionGapMetrics(`${rafReason}:raf`); }); } @@ -1237,6 +1249,12 @@ export class XtermTerminal extends Disposable implements IXtermTerminal, IDetach } override dispose(): void { + if (this._viewportSyncAnimationFrame !== undefined && this._viewportSyncAnimationFrameWindow) { + this._viewportSyncAnimationFrameWindow.cancelAnimationFrame(this._viewportSyncAnimationFrame); + this._viewportSyncAnimationFrame = undefined; + this._viewportSyncAnimationFrameWindow = undefined; + this._viewportSyncAnimationFrameReason = undefined; + } if (this._selectionGapAnimationFrame !== undefined && this.raw.element) { dom.getWindow(this.raw.element).cancelAnimationFrame(this._selectionGapAnimationFrame); this._selectionGapAnimationFrame = undefined; From 2209b94d3cda709b7d24d3eac6d8ad8f5a9b483c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adriana=20Louren=C3=A7o?= Date: Mon, 27 Apr 2026 22:07:14 +0100 Subject: [PATCH 6/6] Fix pixel dimension calculations to preserve zero values --- .../workbench/contrib/terminal/browser/terminalInstance.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 674f22e366a26..1c0e25b6b6c8e 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -2056,9 +2056,9 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { const cellWidth = rawXterm.dimensions?.css.cell.width; // Subtract that extra render column before reporting pixel width to the PTY so process sizing // stays based on the real terminal columns. - const adjustedPixelWidth = pixelWidth && cellWidth ? Math.max(0, pixelWidth - cellWidth) : pixelWidth; - const roundedPixelWidth = adjustedPixelWidth ? Math.round(adjustedPixelWidth) : undefined; - const roundedPixelHeight = pixelHeight ? Math.round(pixelHeight) : undefined; + const adjustedPixelWidth = pixelWidth !== undefined && cellWidth ? Math.max(0, pixelWidth - cellWidth) : pixelWidth; + const roundedPixelWidth = adjustedPixelWidth !== undefined ? Math.round(adjustedPixelWidth) : undefined; + const roundedPixelHeight = pixelHeight !== undefined ? Math.round(pixelHeight) : undefined; await this._processManager.setDimensions(ptyCols, ptyRows, undefined, roundedPixelWidth, roundedPixelHeight); }