From 0dffbedf769b1fa9b998ee1141a4f60eb4aa64e0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 21 Jun 2025 18:01:12 +0000 Subject: [PATCH 1/3] Initial plan for issue From b3503f4916665c0b332913f1f957967d0caac283 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 21 Jun 2025 18:19:30 +0000 Subject: [PATCH 2/3] Implement lineHeight px format support Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com> --- addons/addon-webgl/src/CharAtlasUtils.ts | 11 ++- addons/addon-webgl/src/WebglRenderer.ts | 23 +++-- src/browser/renderer/LineHeight.test.ts | 78 +++++++++++++++++ src/browser/renderer/dom/DomRenderer.ts | 15 +++- .../OptionsService.lineHeight.test.ts | 86 +++++++++++++++++++ src/common/services/OptionsService.ts | 21 ++++- src/common/services/Services.ts | 2 +- typings/xterm-headless.d.ts | 6 +- typings/xterm.d.ts | 6 +- 9 files changed, 235 insertions(+), 13 deletions(-) create mode 100644 src/browser/renderer/LineHeight.test.ts create mode 100644 src/common/services/OptionsService.lineHeight.test.ts diff --git a/addons/addon-webgl/src/CharAtlasUtils.ts b/addons/addon-webgl/src/CharAtlasUtils.ts index db601383f0..0a66c1184a 100644 --- a/addons/addon-webgl/src/CharAtlasUtils.ts +++ b/addons/addon-webgl/src/CharAtlasUtils.ts @@ -10,6 +10,15 @@ import { IColorSet, ReadonlyColorSet } from 'browser/Types'; import { NULL_COLOR } from 'common/Color'; export function generateConfig(deviceCellWidth: number, deviceCellHeight: number, deviceCharWidth: number, deviceCharHeight: number, options: Required, colors: ReadonlyColorSet, devicePixelRatio: number, deviceMaxTextureSize: number): ICharAtlasConfig { + // Convert lineHeight to a numeric multiplier for the char atlas + // If it's a px value, calculate the equivalent multiplier + let lineHeightMultiplier: number = typeof options.lineHeight === 'number' ? options.lineHeight : 1; + if (typeof options.lineHeight === 'string' && options.lineHeight.endsWith('px')) { + const pxValue = parseFloat(options.lineHeight.slice(0, -2)); + // Calculate the multiplier based on font size + lineHeightMultiplier = pxValue / options.fontSize; + } + // null out some fields that don't matter const clonedColors: IColorSet = { foreground: colors.foreground, @@ -36,7 +45,7 @@ export function generateConfig(deviceCellWidth: number, deviceCellHeight: number devicePixelRatio, deviceMaxTextureSize, letterSpacing: options.letterSpacing, - lineHeight: options.lineHeight, + lineHeight: lineHeightMultiplier, deviceCellWidth: deviceCellWidth, deviceCellHeight: deviceCellHeight, deviceCharWidth: deviceCharWidth, diff --git a/addons/addon-webgl/src/WebglRenderer.ts b/addons/addon-webgl/src/WebglRenderer.ts index 576cdad510..5dd02da86f 100644 --- a/addons/addon-webgl/src/WebglRenderer.ts +++ b/addons/addon-webgl/src/WebglRenderer.ts @@ -27,6 +27,19 @@ import { addDisposableListener } from 'vs/base/browser/dom'; import { combinedDisposable, Disposable, MutableDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { createRenderDimensions } from 'browser/renderer/shared/RendererUtils'; +/** + * Calculates the line height in pixels based on the lineHeight option and character height. + */ +function calculateLineHeightInPixels(lineHeight: number | string, charHeight: number): number { + if (typeof lineHeight === 'string' && lineHeight.endsWith('px')) { + const pxValue = parseFloat(lineHeight.slice(0, -2)); + // Ensure the pixel value is at least as large as the character height + return Math.max(pxValue, charHeight); + } + // Numeric lineHeight acts as a multiplier + return Math.floor(charHeight * (lineHeight as number)); +} + export class WebglRenderer extends Disposable implements IRenderer { private _renderLayers: IRenderLayer[]; private _cursorBlinkStateManager: MutableDisposable = new MutableDisposable(); @@ -570,14 +583,14 @@ export class WebglRenderer extends Disposable implements IRenderer { // cell. this.dimensions.device.char.height = Math.ceil(this._charSizeService.height * this._devicePixelRatio); - // Calculate the device cell height, if lineHeight is _not_ 1, the resulting value will be - // floored since lineHeight can never be lower then 1, this guarentees the device cell height - // will always be larger than device char height. - this.dimensions.device.cell.height = Math.floor(this.dimensions.device.char.height * this._optionsService.rawOptions.lineHeight); + // Calculate the device cell height, supporting both numeric multipliers and 'px' values + this.dimensions.device.cell.height = calculateLineHeightInPixels(this._optionsService.rawOptions.lineHeight, this.dimensions.device.char.height); // Calculate the y offset within a cell that glyph should draw at in order for it to be centered // correctly within the cell. - this.dimensions.device.char.top = this._optionsService.rawOptions.lineHeight === 1 ? 0 : Math.round((this.dimensions.device.cell.height - this.dimensions.device.char.height) / 2); + const isDefaultLineHeight = (typeof this._optionsService.rawOptions.lineHeight === 'number' && this._optionsService.rawOptions.lineHeight === 1) || + (typeof this._optionsService.rawOptions.lineHeight === 'string' && this.dimensions.device.cell.height === this.dimensions.device.char.height); + this.dimensions.device.char.top = isDefaultLineHeight ? 0 : Math.round((this.dimensions.device.cell.height - this.dimensions.device.char.height) / 2); // Calculate the device cell width, taking the letterSpacing into account. this.dimensions.device.cell.width = this.dimensions.device.char.width + Math.round(this._optionsService.rawOptions.letterSpacing); diff --git a/src/browser/renderer/LineHeight.test.ts b/src/browser/renderer/LineHeight.test.ts new file mode 100644 index 0000000000..0dc56d20f7 --- /dev/null +++ b/src/browser/renderer/LineHeight.test.ts @@ -0,0 +1,78 @@ +/** + * Copyright (c) 2023 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { assert } from 'chai'; + +/** + * Calculates the line height in pixels based on the lineHeight option and character height. + * This is the same function used in both DomRenderer and WebglRenderer. + */ +function calculateLineHeightInPixels(lineHeight: number | string, charHeight: number): number { + if (typeof lineHeight === 'string' && lineHeight.endsWith('px')) { + const pxValue = parseFloat(lineHeight.slice(0, -2)); + // Ensure the pixel value is at least as large as the character height + return Math.max(pxValue, charHeight); + } + // Numeric lineHeight acts as a multiplier + return Math.floor(charHeight * (lineHeight as number)); +} + +describe('calculateLineHeightInPixels', () => { + describe('numeric lineHeight', () => { + it('should calculate correctly with multiplier 1', () => { + assert.equal(calculateLineHeightInPixels(1, 20), 20); + }); + + it('should calculate correctly with multiplier 1.2', () => { + assert.equal(calculateLineHeightInPixels(1.2, 20), 24); + }); + + it('should calculate correctly with multiplier 1.5', () => { + assert.equal(calculateLineHeightInPixels(1.5, 20), 30); + }); + + it('should floor the result', () => { + assert.equal(calculateLineHeightInPixels(1.3, 20), 26); // Math.floor(26) + }); + }); + + describe('string lineHeight with px format', () => { + it('should use px value directly when larger than char height', () => { + assert.equal(calculateLineHeightInPixels('25px', 20), 25); + }); + + it('should use char height when px value is smaller', () => { + assert.equal(calculateLineHeightInPixels('15px', 20), 20); + }); + + it('should handle decimal px values', () => { + assert.equal(calculateLineHeightInPixels('23.5px', 20), 23.5); + }); + + it('should handle edge case where px equals char height', () => { + assert.equal(calculateLineHeightInPixels('20px', 20), 20); + }); + + it('should handle very large px values', () => { + assert.equal(calculateLineHeightInPixels('100px', 20), 100); + }); + }); + + describe('edge cases', () => { + it('should handle zero char height with numeric multiplier', () => { + assert.equal(calculateLineHeightInPixels(1.5, 0), 0); + }); + + it('should handle zero char height with px value', () => { + assert.equal(calculateLineHeightInPixels('25px', 0), 25); + }); + + it('should handle 1px with various char heights', () => { + assert.equal(calculateLineHeightInPixels('1px', 5), 5); + assert.equal(calculateLineHeightInPixels('1px', 20), 20); + assert.equal(calculateLineHeightInPixels('1px', 0), 1); + }); + }); +}); \ No newline at end of file diff --git a/src/browser/renderer/dom/DomRenderer.ts b/src/browser/renderer/dom/DomRenderer.ts index 1c271797a7..dcbd65045e 100644 --- a/src/browser/renderer/dom/DomRenderer.ts +++ b/src/browser/renderer/dom/DomRenderer.ts @@ -24,6 +24,19 @@ const BG_CLASS_PREFIX = 'xterm-bg-'; const FOCUS_CLASS = 'xterm-focus'; const SELECTION_CLASS = 'xterm-selection'; +/** + * Calculates the line height in pixels based on the lineHeight option and character height. + */ +function calculateLineHeightInPixels(lineHeight: number | string, charHeight: number): number { + if (typeof lineHeight === 'string' && lineHeight.endsWith('px')) { + const pxValue = parseFloat(lineHeight.slice(0, -2)); + // Ensure the pixel value is at least as large as the character height + return Math.max(pxValue, charHeight); + } + // Numeric lineHeight acts as a multiplier + return Math.floor(charHeight * (lineHeight as number)); +} + let nextTerminalId = 1; /** @@ -116,7 +129,7 @@ export class DomRenderer extends Disposable implements IRenderer { this.dimensions.device.char.width = this._charSizeService.width * dpr; this.dimensions.device.char.height = Math.ceil(this._charSizeService.height * dpr); this.dimensions.device.cell.width = this.dimensions.device.char.width + Math.round(this._optionsService.rawOptions.letterSpacing); - this.dimensions.device.cell.height = Math.floor(this.dimensions.device.char.height * this._optionsService.rawOptions.lineHeight); + this.dimensions.device.cell.height = calculateLineHeightInPixels(this._optionsService.rawOptions.lineHeight, this.dimensions.device.char.height); this.dimensions.device.char.left = 0; this.dimensions.device.char.top = 0; this.dimensions.device.canvas.width = this.dimensions.device.cell.width * this._bufferService.cols; diff --git a/src/common/services/OptionsService.lineHeight.test.ts b/src/common/services/OptionsService.lineHeight.test.ts new file mode 100644 index 0000000000..253bdd394b --- /dev/null +++ b/src/common/services/OptionsService.lineHeight.test.ts @@ -0,0 +1,86 @@ +/** + * Copyright (c) 2023 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { assert } from 'chai'; +import { OptionsService } from 'common/services/OptionsService'; + +describe('OptionsService lineHeight', () => { + let optionsService: OptionsService; + + beforeEach(() => { + optionsService = new OptionsService({}); + }); + + describe('numeric lineHeight', () => { + it('should accept numeric values', () => { + optionsService.options.lineHeight = 1.5; + assert.equal(optionsService.rawOptions.lineHeight, 1.5); + }); + + it('should reject values less than 1', () => { + assert.throws(() => optionsService.options.lineHeight = 0.5); + }); + + it('should accept 1 as minimum value', () => { + optionsService.options.lineHeight = 1; + assert.equal(optionsService.rawOptions.lineHeight, 1); + }); + }); + + describe('string lineHeight with px format', () => { + it('should accept px values', () => { + optionsService.options.lineHeight = '24px'; + assert.equal(optionsService.rawOptions.lineHeight, '24px'); + }); + + it('should accept decimal px values', () => { + optionsService.options.lineHeight = '23.5px'; + assert.equal(optionsService.rawOptions.lineHeight, '23.5px'); + }); + + it('should reject px values less than 1', () => { + assert.throws(() => optionsService.options.lineHeight = '0.5px'); + }); + + it('should accept 1px as minimum value', () => { + optionsService.options.lineHeight = '1px'; + assert.equal(optionsService.rawOptions.lineHeight, '1px'); + }); + + it('should reject non-px string values', () => { + assert.throws(() => optionsService.options.lineHeight = '24' as any); + assert.throws(() => optionsService.options.lineHeight = '24em' as any); + assert.throws(() => optionsService.options.lineHeight = 'normal' as any); + }); + + it('should reject invalid px values', () => { + assert.throws(() => optionsService.options.lineHeight = 'invalidpx' as any); + assert.throws(() => optionsService.options.lineHeight = 'px' as any); + }); + }); + + describe('invalid lineHeight types', () => { + it('should reject null', () => { + assert.throws(() => optionsService.options.lineHeight = null as any); + }); + + it('should reject undefined when explicitly set', () => { + assert.throws(() => optionsService.options.lineHeight = undefined as any); + }); + + it('should reject boolean values', () => { + assert.throws(() => optionsService.options.lineHeight = true as any); + assert.throws(() => optionsService.options.lineHeight = false as any); + }); + + it('should reject arrays', () => { + assert.throws(() => optionsService.options.lineHeight = [] as any); + }); + + it('should reject objects', () => { + assert.throws(() => optionsService.options.lineHeight = {} as any); + }); + }); +}); \ No newline at end of file diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts index 6ad48b9319..5cda823311 100644 --- a/src/common/services/OptionsService.ts +++ b/src/common/services/OptionsService.ts @@ -172,12 +172,31 @@ export class OptionsService extends Disposable implements IOptionsService { case 'cursorWidth': value = Math.floor(value); // Fall through for bounds check - case 'lineHeight': case 'tabStopWidth': if (value < 1) { throw new Error(`${key} cannot be less than 1, value: ${value}`); } break; + case 'lineHeight': + if (typeof value === 'string') { + // Handle 'px' format + if (value.endsWith('px')) { + const pxValue = parseFloat(value.slice(0, -2)); + if (isNaN(pxValue) || pxValue < 1) { + throw new Error(`${key} with 'px' format must be a valid number >= 1, value: ${value}`); + } + // Keep the string value as-is for downstream processing + } else { + throw new Error(`${key} string value must end with 'px', value: ${value}`); + } + } else if (typeof value === 'number') { + if (value < 1) { + throw new Error(`${key} cannot be less than 1, value: ${value}`); + } + } else { + throw new Error(`${key} must be a number or string with 'px' suffix, value: ${value}`); + } + break; case 'minimumContrastRatio': value = Math.max(1, Math.min(21, Math.round(value * 10) / 10)); break; diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 9c3aebf73e..11e0317507 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -231,7 +231,7 @@ export interface ITerminalOptions { fontWeightBold?: FontWeight; ignoreBracketedPasteMode?: boolean; letterSpacing?: number; - lineHeight?: number; + lineHeight?: number | string; linkHandler?: ILinkHandler | null; logLevel?: LogLevel; logger?: ILogger | null; diff --git a/typings/xterm-headless.d.ts b/typings/xterm-headless.d.ts index 11d979474b..e3fd86d46a 100644 --- a/typings/xterm-headless.d.ts +++ b/typings/xterm-headless.d.ts @@ -94,9 +94,11 @@ declare module '@xterm/headless' { letterSpacing?: number; /** - * The line height used to render text. + * The line height used to render text. This can be specified as: + * - A number (e.g., 1.2) which acts as a multiplier of the font size + * - A string with 'px' suffix (e.g., '24px') which specifies an absolute pixel height */ - lineHeight?: number; + lineHeight?: number | string; /** * What log level to use, this will log for all levels below and including diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index d1c2667d09..9113dc4dbf 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -152,9 +152,11 @@ declare module '@xterm/xterm' { letterSpacing?: number; /** - * The line height used to render text. + * The line height used to render text. This can be specified as: + * - A number (e.g., 1.2) which acts as a multiplier of the font size + * - A string with 'px' suffix (e.g., '24px') which specifies an absolute pixel height */ - lineHeight?: number; + lineHeight?: number | string; /** * The handler for OSC 8 hyperlinks. Links will use the `confirm` browser From 3e60d791d571f5880ecdbd88a2593d056dbbc091 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 21 Jun 2025 18:23:00 +0000 Subject: [PATCH 3/3] Fix linting issues Co-authored-by: Tyriar <2193314+Tyriar@users.noreply.github.com> --- addons/addon-webgl/src/CharAtlasUtils.ts | 2 +- src/browser/renderer/LineHeight.test.ts | 2 +- src/common/services/OptionsService.lineHeight.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/addons/addon-webgl/src/CharAtlasUtils.ts b/addons/addon-webgl/src/CharAtlasUtils.ts index 0a66c1184a..fec9231c95 100644 --- a/addons/addon-webgl/src/CharAtlasUtils.ts +++ b/addons/addon-webgl/src/CharAtlasUtils.ts @@ -18,7 +18,7 @@ export function generateConfig(deviceCellWidth: number, deviceCellHeight: number // Calculate the multiplier based on font size lineHeightMultiplier = pxValue / options.fontSize; } - + // null out some fields that don't matter const clonedColors: IColorSet = { foreground: colors.foreground, diff --git a/src/browser/renderer/LineHeight.test.ts b/src/browser/renderer/LineHeight.test.ts index 0dc56d20f7..4950e7dff4 100644 --- a/src/browser/renderer/LineHeight.test.ts +++ b/src/browser/renderer/LineHeight.test.ts @@ -75,4 +75,4 @@ describe('calculateLineHeightInPixels', () => { assert.equal(calculateLineHeightInPixels('1px', 0), 1); }); }); -}); \ No newline at end of file +}); diff --git a/src/common/services/OptionsService.lineHeight.test.ts b/src/common/services/OptionsService.lineHeight.test.ts index 253bdd394b..d4ef7ea4fc 100644 --- a/src/common/services/OptionsService.lineHeight.test.ts +++ b/src/common/services/OptionsService.lineHeight.test.ts @@ -83,4 +83,4 @@ describe('OptionsService lineHeight', () => { assert.throws(() => optionsService.options.lineHeight = {} as any); }); }); -}); \ No newline at end of file +});