From 007410995a5218c3bde2196065deb11f41ffabc5 Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Wed, 8 Nov 2017 21:04:04 +0000 Subject: [PATCH 1/5] Fix#1117 --- fixtures/typings-test/typings-test.ts | 8 +++++--- src/Interfaces.ts | 3 ++- src/Terminal.ts | 10 ++++++++-- src/renderer/BaseRenderLayer.ts | 22 +++++++++++++++------- src/renderer/CharAtlas.ts | 18 ++++++++++++------ src/renderer/TextRenderLayer.ts | 2 +- typings/xterm.d.ts | 6 ++++++ 7 files changed, 49 insertions(+), 20 deletions(-) diff --git a/fixtures/typings-test/typings-test.ts b/fixtures/typings-test/typings-test.ts index 93fd501f..b8425205 100644 --- a/fixtures/typings-test/typings-test.ts +++ b/fixtures/typings-test/typings-test.ts @@ -143,8 +143,9 @@ namespace methods_core { const r18: (data: string) => void = t.getOption('handler'); const r19: string = t.getOption('bellSound'); const r20: string = t.getOption('bellStyle'); - const r21: boolean = t.getOption('enableBold'); - const r22: number = t.getOption('letterSpacing'); + const r21: number = t.getOption('letterSpacing'); + const r22: number = t.getOption('fontWeight'); + const r23: number = t.getOption('fontWeightBold'); } { const t: Terminal = new Terminal(); @@ -157,7 +158,8 @@ namespace methods_core { t.setOption('cursorBlink', true); t.setOption('debug', true); t.setOption('disableStdin', true); - t.setOption('enableBold', true); + t.setOption('fontWeight', 'normal'); + t.setOption('fontWeightBold', 'bold'); t.setOption('popOnBell', true); t.setOption('screenKeys', true); t.setOption('useFlowControl', true); diff --git a/src/Interfaces.ts b/src/Interfaces.ts index 354e2c4e..77e94978 100644 --- a/src/Interfaces.ts +++ b/src/Interfaces.ts @@ -137,9 +137,10 @@ export interface ITerminalOptions { cursorStyle?: string; debug?: boolean; disableStdin?: boolean; - enableBold?: boolean; fontSize?: number; fontFamily?: string; + fontWeight?: string; + fontWeightBold?: string; geometry?: [number, number]; handler?: (data: string) => void; letterSpacing?: number; diff --git a/src/Terminal.ts b/src/Terminal.ts index 867c522d..1f08ce8a 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -77,9 +77,10 @@ const DEFAULT_OPTIONS: ITerminalOptions = { cursorStyle: 'block', bellSound: BellSound, bellStyle: 'none', - enableBold: true, fontFamily: 'courier-new, courier, monospace', fontSize: 15, + fontWeight: 'normal', + fontWeightBold: 'bold', lineHeight: 1.0, letterSpacing: 0, scrollback: 1000, @@ -416,7 +417,6 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT this.renderer.clear(); this.charMeasure.measure(this.options); break; - case 'enableBold': case 'letterSpacing': case 'lineHeight': // When the font changes the size of the cells may change which requires a renderer clear @@ -424,6 +424,12 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT this.renderer.onResize(this.cols, this.rows, false); this.refresh(0, this.rows - 1); // this.charMeasure.measure(this.options); + case 'fontWeight': + case 'fontWeightBold': + // When the font weight changes the size of the cells may change which requires a renderer clear + this.renderer.clear(); + this.renderer.onResize(this.cols, this.rows, true); + this.refresh(0, this.rows - 1); case 'scrollback': this.buffers.resize(this.cols, this.rows); this.viewport.syncScrollArea(); diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts index b4becbce..a5f4370c 100644 --- a/src/renderer/BaseRenderLayer.ts +++ b/src/renderer/BaseRenderLayer.ts @@ -201,7 +201,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { * @param color The color of the character. */ protected fillCharTrueColor(terminal: ITerminal, charData: CharData, x: number, y: number): void { - this._ctx.font = `${terminal.options.fontSize * window.devicePixelRatio}px ${terminal.options.fontFamily}`; + this._ctx.font = this._getFont(terminal, false); this._ctx.textBaseline = 'top'; this._clipRow(terminal, y); this._ctx.fillText( @@ -230,7 +230,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { colorIndex = fg + 2; } else { // If default color and bold - if (bold && terminal.options.enableBold) { + if (bold) { colorIndex = 1; } } @@ -252,7 +252,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { } // Draw the non-bold version of the same color if bold is not enabled - if (bold && !terminal.options.enableBold) { + if (bold) { // Ignore default color as it's not touched above if (colorIndex > 1) { colorIndex -= 8; @@ -289,10 +289,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { */ private _drawUncachedChar(terminal: ITerminal, char: string, width: number, fg: number, x: number, y: number, bold: boolean, dim: boolean): void { this._ctx.save(); - this._ctx.font = `${terminal.options.fontSize * window.devicePixelRatio}px ${terminal.options.fontFamily}`; - if (bold && terminal.options.enableBold) { - this._ctx.font = `bold ${this._ctx.font}`; - } + this._ctx.font = this._getFont(terminal, bold); this._ctx.textBaseline = 'top'; if (fg === INVERTED_DEFAULT_COLOR) { @@ -332,5 +329,16 @@ export abstract class BaseRenderLayer implements IRenderLayer { this._scaledCellHeight); this._ctx.clip(); } + + /** + * Gets the current font. + * @param terminal The terminal. + * @param isBold The font weight that should be used uses terminal option as fallback. + */ + private _getFont(terminal: ITerminal, isBold: boolean): string { + const fontWeight = isBold ? terminal.options.fontWeightBold : terminal.options.fontWeight; + + return `${fontWeight} ${terminal.options.fontSize * window.devicePixelRatio}px ${terminal.options.fontFamily}`; + } } diff --git a/src/renderer/CharAtlas.ts b/src/renderer/CharAtlas.ts index 9ac1e161..50b9b9bd 100644 --- a/src/renderer/CharAtlas.ts +++ b/src/renderer/CharAtlas.ts @@ -12,6 +12,8 @@ export const CHAR_ATLAS_CELL_SPACING = 1; interface ICharAtlasConfig { fontSize: number; fontFamily: string; + fontWeight: string; + fontWeightBold: string; scaledCharWidth: number; scaledCharHeight: number; colors: IColorSet; @@ -64,7 +66,7 @@ export function acquireCharAtlas(terminal: ITerminal, colors: IColorSet, scaledC } const newEntry: ICharAtlasCacheEntry = { - bitmap: generator.generate(scaledCharWidth, scaledCharHeight, terminal.options.fontSize, terminal.options.fontFamily, colors.background, colors.foreground, colors.ansi), + bitmap: generator.generate(scaledCharWidth, scaledCharHeight, terminal.options.fontSize, terminal.options.fontFamily, terminal.options.fontWeight, terminal.options.fontWeightBold, colors.background, colors.foreground, colors.ansi), config: newConfig, ownedBy: [terminal] }; @@ -86,6 +88,8 @@ function generateConfig(scaledCharWidth: number, scaledCharHeight: number, termi scaledCharHeight, fontFamily: terminal.options.fontFamily, fontSize: terminal.options.fontSize, + fontWeight: terminal.options.fontWeight, + fontWeightBold: terminal.options.fontWeightBold, colors: clonedColors }; } @@ -98,6 +102,8 @@ function configEquals(a: ICharAtlasConfig, b: ICharAtlasConfig): boolean { } return a.fontFamily === b.fontFamily && a.fontSize === b.fontSize && + a.fontWeight === b.fontWeight && + a.fontWeightBold === b.fontWeightBold && a.scaledCharWidth === b.scaledCharWidth && a.scaledCharHeight === b.scaledCharHeight && a.colors.foreground === b.colors.foreground && @@ -126,7 +132,7 @@ class CharAtlasGenerator { this._ctx.scale(window.devicePixelRatio, window.devicePixelRatio); } - public generate(scaledCharWidth: number, scaledCharHeight: number, fontSize: number, fontFamily: string, background: string, foreground: string, ansiColors: string[]): HTMLCanvasElement | Promise { + public generate(scaledCharWidth: number, scaledCharHeight: number, fontSize: number, fontFamily: string, fontWeight: string, fontWeightBold: string, background: string, foreground: string, ansiColors: string[]): HTMLCanvasElement | Promise { const cellWidth = scaledCharWidth + CHAR_ATLAS_CELL_SPACING; const cellHeight = scaledCharHeight + CHAR_ATLAS_CELL_SPACING; this._canvas.width = 255 * cellWidth; @@ -137,7 +143,7 @@ class CharAtlasGenerator { this._ctx.save(); this._ctx.fillStyle = foreground; - this._ctx.font = `${fontSize * window.devicePixelRatio}px ${fontFamily}`; + this._ctx.font = `${fontWeight} ${fontSize * window.devicePixelRatio}px ${fontFamily}`; this._ctx.textBaseline = 'top'; // Default color @@ -151,7 +157,7 @@ class CharAtlasGenerator { } // Default color bold this._ctx.save(); - this._ctx.font = `bold ${this._ctx.font}`; + this._ctx.font = `${fontWeightBold} ${fontSize * window.devicePixelRatio}px ${fontFamily}`; for (let i = 0; i < 256; i++) { this._ctx.save(); this._ctx.beginPath(); @@ -163,11 +169,11 @@ class CharAtlasGenerator { this._ctx.restore(); // Colors 0-15 - this._ctx.font = `${fontSize * window.devicePixelRatio}px ${fontFamily}`; + this._ctx.font = `${fontWeight} ${fontSize * window.devicePixelRatio}px ${fontFamily}`; for (let colorIndex = 0; colorIndex < 16; colorIndex++) { // colors 8-15 are bold if (colorIndex === 8) { - this._ctx.font = `bold ${this._ctx.font}`; + this._ctx.font = `${fontWeightBold} ${fontSize * window.devicePixelRatio}px ${fontFamily}`; } const y = (colorIndex + 2) * cellHeight; // Draw ascii characters diff --git a/src/renderer/TextRenderLayer.ts b/src/renderer/TextRenderLayer.ts index 2ecb6ec6..5a13242f 100644 --- a/src/renderer/TextRenderLayer.ts +++ b/src/renderer/TextRenderLayer.ts @@ -33,7 +33,7 @@ export class TextRenderLayer extends BaseRenderLayer { super.resize(terminal, dim, charSizeChanged); // Clear the character width cache if the font or width has changed - const terminalFont = `${terminal.options.fontSize * window.devicePixelRatio}px ${terminal.options.fontFamily}`; + const terminalFont = `${terminal.options.fontWeight} ${terminal.options.fontSize * window.devicePixelRatio}px ${terminal.options.fontFamily}`; if (this._characterWidth !== dim.scaledCharWidth || this._characterFont !== terminalFont) { this._characterWidth = dim.scaledCharWidth; this._characterFont = terminalFont; diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index b0f5ac4d..aa7d8f3e 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -442,6 +442,12 @@ declare module 'xterm' { * @param value The option value. */ setOption(key: 'fontFamily' | 'termName' | 'bellSound', value: string): void; + /** + * Sets an option on the terminal. + * @param key The option key. + * @param value The option value. + */ + setOption(key: 'fontWeight' | 'fontWeightBold', value: null | 'normal' | 'bold' | 'bolder' | 'lighter' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900'): void; /** * Sets an option on the terminal. * @param key The option key. From f12eb5367bd2bf566cffd18c9407f8c936d7f7cb Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Wed, 8 Nov 2017 23:40:13 +0000 Subject: [PATCH 2/5] Adress feedback --- src/Terminal.ts | 23 +++++++++++++++-------- typings/xterm.d.ts | 2 +- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 1f08ce8a..9b3e485c 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -367,6 +367,16 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT value = 'block'; } break; + case 'fontWeight': + if (!value) { + value = 'normal'; + } + break; + case 'fontWeightBold': + if (!value) { + value = 'bold'; + } + break; case 'lineHeight': if (value < 1) { console.warn(`${key} cannot be less than 1, value: ${value}`); @@ -419,17 +429,14 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT break; case 'letterSpacing': case 'lineHeight': + case 'fontWeight': + case 'fontWeightBold': + const didCharSizeChange = (key === 'fontWeight' || key === 'fontWeightBold'); + // When the font changes the size of the cells may change which requires a renderer clear this.renderer.clear(); - this.renderer.onResize(this.cols, this.rows, false); + this.renderer.onResize(this.cols, this.rows, didCharSizeChange); this.refresh(0, this.rows - 1); - // this.charMeasure.measure(this.options); - case 'fontWeight': - case 'fontWeightBold': - // When the font weight changes the size of the cells may change which requires a renderer clear - this.renderer.clear(); - this.renderer.onResize(this.cols, this.rows, true); - this.refresh(0, this.rows - 1); case 'scrollback': this.buffers.resize(this.cols, this.rows); this.viewport.syncScrollArea(); diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index aa7d8f3e..ced58fcf 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -404,7 +404,7 @@ declare module 'xterm' { * Retrieves an option's value from the terminal. * @param key The option key. */ - getOption(key: 'bellSound' | 'bellStyle' | 'cursorStyle' | 'fontFamily' | 'termName'): string; + getOption(key: 'bellSound' | 'bellStyle' | 'cursorStyle' | 'fontFamily' | 'fontWeight' | 'fontWeightBold'| 'termName'): string; /** * Retrieves an option's value from the terminal. * @param key The option key. From 51de968fdecdebd541c1480bc0622bf5ce8c2606 Mon Sep 17 00:00:00 2001 From: Bruno Ribeito Date: Sun, 12 Nov 2017 23:05:27 +0000 Subject: [PATCH 3/5] Fix non-white colors not being drawn as bold --- src/renderer/BaseRenderLayer.ts | 11 ++--------- src/renderer/CharAtlas.ts | 12 ++++++++---- src/renderer/TextRenderLayer.ts | 4 ++-- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts index a5f4370c..977cbf52 100644 --- a/src/renderer/BaseRenderLayer.ts +++ b/src/renderer/BaseRenderLayer.ts @@ -251,13 +251,6 @@ export abstract class BaseRenderLayer implements IRenderLayer { this._ctx.globalAlpha = DIM_OPACITY; } - // Draw the non-bold version of the same color if bold is not enabled - if (bold) { - // Ignore default color as it's not touched above - if (colorIndex > 1) { - colorIndex -= 8; - } - } this._ctx.drawImage(this._charAtlas, code * charAtlasCellWidth, @@ -333,9 +326,9 @@ export abstract class BaseRenderLayer implements IRenderLayer { /** * Gets the current font. * @param terminal The terminal. - * @param isBold The font weight that should be used uses terminal option as fallback. + * @param isBold If we should use the bold fontWeight. */ - private _getFont(terminal: ITerminal, isBold: boolean): string { + protected _getFont(terminal: ITerminal, isBold: boolean): string { const fontWeight = isBold ? terminal.options.fontWeightBold : terminal.options.fontWeight; return `${fontWeight} ${terminal.options.fontSize * window.devicePixelRatio}px ${terminal.options.fontFamily}`; diff --git a/src/renderer/CharAtlas.ts b/src/renderer/CharAtlas.ts index 50b9b9bd..1458ba76 100644 --- a/src/renderer/CharAtlas.ts +++ b/src/renderer/CharAtlas.ts @@ -143,7 +143,7 @@ class CharAtlasGenerator { this._ctx.save(); this._ctx.fillStyle = foreground; - this._ctx.font = `${fontWeight} ${fontSize * window.devicePixelRatio}px ${fontFamily}`; + this._ctx.font = this._getFont(fontWeight, fontSize, fontFamily); this._ctx.textBaseline = 'top'; // Default color @@ -157,7 +157,7 @@ class CharAtlasGenerator { } // Default color bold this._ctx.save(); - this._ctx.font = `${fontWeightBold} ${fontSize * window.devicePixelRatio}px ${fontFamily}`; + this._ctx.font = this._getFont(fontWeightBold, fontSize, fontFamily); for (let i = 0; i < 256; i++) { this._ctx.save(); this._ctx.beginPath(); @@ -169,11 +169,11 @@ class CharAtlasGenerator { this._ctx.restore(); // Colors 0-15 - this._ctx.font = `${fontWeight} ${fontSize * window.devicePixelRatio}px ${fontFamily}`; + this._ctx.font = this._getFont(fontWeight, fontSize, fontFamily); for (let colorIndex = 0; colorIndex < 16; colorIndex++) { // colors 8-15 are bold if (colorIndex === 8) { - this._ctx.font = `${fontWeightBold} ${fontSize * window.devicePixelRatio}px ${fontFamily}`; + this._ctx.font = this._getFont(fontWeightBold, fontSize, fontFamily); } const y = (colorIndex + 2) * cellHeight; // Draw ascii characters @@ -225,4 +225,8 @@ class CharAtlasGenerator { } } } + + private _getFont(fontWeight: string, fontSize: number, fontFamily: string): string { + return `${fontWeight} ${fontSize * window.devicePixelRatio}px ${fontFamily}`; + } } diff --git a/src/renderer/TextRenderLayer.ts b/src/renderer/TextRenderLayer.ts index 5a13242f..41454adb 100644 --- a/src/renderer/TextRenderLayer.ts +++ b/src/renderer/TextRenderLayer.ts @@ -33,7 +33,7 @@ export class TextRenderLayer extends BaseRenderLayer { super.resize(terminal, dim, charSizeChanged); // Clear the character width cache if the font or width has changed - const terminalFont = `${terminal.options.fontWeight} ${terminal.options.fontSize * window.devicePixelRatio}px ${terminal.options.fontFamily}`; + const terminalFont = this._getFont(terminal, false); if (this._characterWidth !== dim.scaledCharWidth || this._characterFont !== terminalFont) { this._characterWidth = dim.scaledCharWidth; this._characterFont = terminalFont; @@ -166,7 +166,7 @@ export class TextRenderLayer extends BaseRenderLayer { this._ctx.save(); if (flags & FLAGS.BOLD) { - this._ctx.font = `bold ${this._ctx.font}`; + this._ctx.font = this._getFont(terminal, true); // Convert the FG color to the bold variant if (fg < 8) { fg += 8; From f3ecbea3e885541e8ef17c4992e3c9e6c8073183 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 23 Jan 2018 07:55:18 -0800 Subject: [PATCH 4/5] Fix tests --- fixtures/typings-test/typings-test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fixtures/typings-test/typings-test.ts b/fixtures/typings-test/typings-test.ts index 2aa2264e..4718110d 100644 --- a/fixtures/typings-test/typings-test.ts +++ b/fixtures/typings-test/typings-test.ts @@ -145,8 +145,8 @@ namespace methods_core { const r21: boolean = t.getOption('enableBold'); const r22: number = t.getOption('letterSpacing'); const r23: boolean = t.getOption('macOptionIsMeta'); - const r24: number = t.getOption('fontWeight'); - const r25: number = t.getOption('fontWeightBold'); + const r24: string = t.getOption('fontWeight'); + const r25: string = t.getOption('fontWeightBold'); } { const t: Terminal = new Terminal(); @@ -160,6 +160,8 @@ namespace methods_core { t.setOption('debug', true); t.setOption('disableStdin', true); t.setOption('fontWeight', 'normal'); + t.setOption('fontWeight', 'bold'); + t.setOption('fontWeightBold', 'normal'); t.setOption('fontWeightBold', 'bold'); t.setOption('popOnBell', true); t.setOption('screenKeys', true); From 01c95168f205199b767607008688dad1e64b85d9 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 23 Jan 2018 08:02:48 -0800 Subject: [PATCH 5/5] Make font weight types in ITerminalOption more strict --- src/Interfaces.ts | 6 +++--- src/Types.ts | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Interfaces.ts b/src/Interfaces.ts index 424f7692..6d1c36d3 100644 --- a/src/Interfaces.ts +++ b/src/Interfaces.ts @@ -4,7 +4,7 @@ */ import { ICharset, ILinkMatcherOptions } from './Interfaces'; -import { LinkMatcherHandler, LinkMatcherValidationCallback, LineData } from './Types'; +import { LinkMatcherHandler, LinkMatcherValidationCallback, LineData, FontWeight } from './Types'; import { IColorSet, IRenderer } from './renderer/Interfaces'; import { IMouseZoneManager } from './input/Interfaces'; @@ -139,8 +139,8 @@ export interface ITerminalOptions { disableStdin?: boolean; fontSize?: number; fontFamily?: string; - fontWeight?: string; - fontWeightBold?: string; + fontWeight?: FontWeight; + fontWeightBold?: FontWeight; handler?: (data: string) => void; letterSpacing?: number; lineHeight?: number; diff --git a/src/Types.ts b/src/Types.ts index 3263282e..336ac5cc 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -16,3 +16,5 @@ export enum LinkHoverEventTypes { TOOLTIP = 'linktooltip', LEAVE = 'linkleave' } + +export type FontWeight = 'normal' | 'bold' | 'bolder' | 'lighter' | '100' | '200' | '300' | '400' | '500' | '600' | '700' | '800' | '900';