From b853aa492914bbaef91a73eeae0c904b621bfd9e Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Wed, 14 Mar 2018 10:05:29 -0700 Subject: [PATCH 1/9] Separate foreground & background rendering passes I'm primarily interested in doing this because it'll allow me to optimize the background rendering in later diffs, but this may also make it possible to fix some minor rendering issues. Right now, if we draw a single-width character that overflows its cell's bounds, and the character to the right of it has a background, that background may cover up the first character's foreground. By drawing the background in a separate pass, we can avoid those cases. There's still some issues with how dirty regions are computed that makes rendering stuff like that flaky, but this at least gets us closer to "correct" rendering. Other terminal emulators (e.g. alacritty) render the foreground and background in separate passes: https://github.com/jwilm/alacritty/blob/1b7ffea/src/renderer/mod.rs#L766 --- src/renderer/TextRenderLayer.ts | 155 +++++++++++++++----------------- 1 file changed, 72 insertions(+), 83 deletions(-) diff --git a/src/renderer/TextRenderLayer.ts b/src/renderer/TextRenderLayer.ts index 2487a294..8b061511 100644 --- a/src/renderer/TextRenderLayer.ts +++ b/src/renderer/TextRenderLayer.ts @@ -48,21 +48,24 @@ export class TextRenderLayer extends BaseRenderLayer { this.clearAll(); } - public onGridChanged(terminal: ITerminal, startRow: number, endRow: number): void { - // Resize has not been called yet - if (this._state.cache.length === 0) { - return; - } - + private _forEachCell( + terminal: ITerminal, + startRow: number, + endRow: number, + callback: ( + code: number, + char: string, + width: number, + x: number, + y: number, + fg: number, + bg: number, + flags: number + ) => void + ): void { for (let y = startRow; y <= endRow; y++) { const row = y + terminal.buffer.ydisp; const line = terminal.buffer.lines.get(row); - - this.clearCells(0, y, terminal.cols, 1); - // for (let x = 0; x < terminal.cols; x++) { - // this._state.cache[x][y] = null; - // } - for (let x = 0; x < terminal.cols; x++) { const charData = line[x]; const code: number = charData[CHAR_DATA_CODE_INDEX]; @@ -73,51 +76,12 @@ export class TextRenderLayer extends BaseRenderLayer { // The character to the left is a wide character, drawing is owned by // the char at x-1 if (width === 0) { - // this._state.cache[x][y] = null; - continue; - } - - // If the character is a space and the character to the left is an - // overlapping character, skip the character and allow the overlapping - // char to take full control over this character's cell. - if (code === 32 /*' '*/) { - if (x > 0) { - const previousChar: CharData = line[x - 1]; - if (this._isOverlapping(previousChar)) { - continue; - } - } - } - - // Skip rendering if the character is identical - // const state = this._state.cache[x][y]; - // if (state && state[CHAR_DATA_CHAR_INDEX] === char && state[CHAR_DATA_ATTR_INDEX] === attr) { - // // Skip render, contents are identical - // this._state.cache[x][y] = charData; - // continue; - // } - - // Clear the old character was not a space with the default background - // const wasInverted = !!(state && state[CHAR_DATA_ATTR_INDEX] && state[CHAR_DATA_ATTR_INDEX] >> 18 & FLAGS.INVERSE); - // if (state && !(state[CHAR_DATA_CODE_INDEX] === 32 /*' '*/ && (state[CHAR_DATA_ATTR_INDEX] & 0x1ff) >= 256 && !wasInverted)) { - // this._clearChar(x, y); - // } - // this._state.cache[x][y] = charData; - - const flags = attr >> 18; - let bg = attr & 0x1ff; - - // Skip rendering if the character is invisible - const isDefaultBackground = bg >= 256; - const isInvisible = flags & FLAGS.INVISIBLE; - const isInverted = flags & FLAGS.INVERSE; - if (!code || (code === 32 /*' '*/ && isDefaultBackground && !isInverted) || isInvisible) { continue; } // If the character is an overlapping char and the character to the right is a // space, take ownership of the cell to the right. - if (width !== 0 && this._isOverlapping(charData)) { + if (this._isOverlapping(charData)) { // If the character is overlapping, we want to force a re-render on every // frame. This is specifically to work around the case where two // overlaping chars `a` and `b` are adjacent, the cursor is moved to b and a @@ -135,10 +99,12 @@ export class TextRenderLayer extends BaseRenderLayer { } } + const flags = attr >> 18; + let bg = attr & 0x1ff; let fg = (attr >> 9) & 0x1ff; // If inverse flag is on, the foreground should become the background. - if (isInverted) { + if (flags & FLAGS.INVERSE) { const temp = bg; bg = fg; fg = temp; @@ -150,47 +116,70 @@ export class TextRenderLayer extends BaseRenderLayer { } } - // Clear the cell next to this character if it's wide - if (width === 2) { - // this.clearCells(x + 1, y, 1, 1); - } - - // Draw background - if (bg < 256) { - this._ctx.save(); - this._ctx.fillStyle = (bg === INVERTED_DEFAULT_COLOR ? this._colors.foreground.css : this._colors.ansi[bg].css); - this.fillCells(x, y, width, 1); - this._ctx.restore(); - } - - this._ctx.save(); if (flags & FLAGS.BOLD) { - this._ctx.font = this._getFont(terminal, true); // Convert the FG color to the bold variant if (fg < 8) { fg += 8; } } - if (flags & FLAGS.UNDERLINE) { - if (fg === INVERTED_DEFAULT_COLOR) { - this._ctx.fillStyle = this._colors.background.css; - } else if (fg < 256) { - // 256 color support - this._ctx.fillStyle = this._colors.ansi[fg].css; - } else { - this._ctx.fillStyle = this._colors.foreground.css; - } - this.fillBottomLineAtCells(x, y); - } - - this.drawChar(terminal, char, code, width, x, y, fg, bg, !!(flags & FLAGS.BOLD), !!(flags & FLAGS.DIM)); - - this._ctx.restore(); + callback(code, char, width, x, y, fg, bg, flags); } } } + private _drawBackground(terminal: ITerminal, startRow: number, endRow: number): void { + this._forEachCell(terminal, startRow, endRow, (code, char, width, x, y, fg, bg, flags) => { + // libvte and xterm both draw the background (but not foreground) of invisible characters, + // so we should too. + const isDefaultBackground = bg >= 256; + if (!isDefaultBackground) { + this._ctx.save(); + this._ctx.fillStyle = (bg === INVERTED_DEFAULT_COLOR ? this._colors.foreground.css : this._colors.ansi[bg].css); + this.fillCells(x, y, width, 1); + this._ctx.restore(); + } + }); + } + + private _drawForeground(terminal: ITerminal, startRow: number, endRow: number): void { + this._forEachCell(terminal, startRow, endRow, (code, char, width, x, y, fg, bg, flags) => { + if (flags & FLAGS.INVISIBLE) { + return; + } + if (flags & FLAGS.UNDERLINE) { + this._ctx.save(); + if (fg === INVERTED_DEFAULT_COLOR) { + this._ctx.fillStyle = this._colors.background.css; + } else if (fg < 256) { + // 256 color support + this._ctx.fillStyle = this._colors.ansi[fg].css; + } else { + this._ctx.fillStyle = this._colors.foreground.css; + } + this.fillBottomLineAtCells(x, y); + this._ctx.restore(); + } + this.drawChar( + terminal, char, code, + width, x, y, + fg, bg, + !!(flags & FLAGS.BOLD), !!(flags & FLAGS.DIM) + ); + }); + } + + public onGridChanged(terminal: ITerminal, startRow: number, endRow: number): void { + // Resize has not been called yet + if (this._state.cache.length === 0) { + return; + } + + this.clearCells(0, startRow, terminal.cols, endRow - startRow + 1); // endRow is inclusive + this._drawBackground(terminal, startRow, endRow); + this._drawForeground(terminal, startRow, endRow); + } + public onOptionsChanged(terminal: ITerminal): void { this.setTransparency(terminal, terminal.options.allowTransparency); } From 0fe59df2e91e766f5bcec2e7e2479cb32a32bd3b Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Wed, 18 Apr 2018 20:22:55 -0700 Subject: [PATCH 2/9] Rename startRow/endRow in TextRenderLayer Renaming these to firstRow/lastRow makes the inclusivity of the range clearer. Addresses this comment: https://github.com/xtermjs/xterm.js/pull/1393#discussion_r182471582 --- src/renderer/TextRenderLayer.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/renderer/TextRenderLayer.ts b/src/renderer/TextRenderLayer.ts index 8b061511..58d0d790 100644 --- a/src/renderer/TextRenderLayer.ts +++ b/src/renderer/TextRenderLayer.ts @@ -50,8 +50,8 @@ export class TextRenderLayer extends BaseRenderLayer { private _forEachCell( terminal: ITerminal, - startRow: number, - endRow: number, + firstRow: number, + lastRow: number, callback: ( code: number, char: string, @@ -63,7 +63,7 @@ export class TextRenderLayer extends BaseRenderLayer { flags: number ) => void ): void { - for (let y = startRow; y <= endRow; y++) { + for (let y = firstRow; y <= lastRow; y++) { const row = y + terminal.buffer.ydisp; const line = terminal.buffer.lines.get(row); for (let x = 0; x < terminal.cols; x++) { @@ -128,8 +128,8 @@ export class TextRenderLayer extends BaseRenderLayer { } } - private _drawBackground(terminal: ITerminal, startRow: number, endRow: number): void { - this._forEachCell(terminal, startRow, endRow, (code, char, width, x, y, fg, bg, flags) => { + private _drawBackground(terminal: ITerminal, firstRow: number, lastRow: number): void { + this._forEachCell(terminal, firstRow, lastRow, (code, char, width, x, y, fg, bg, flags) => { // libvte and xterm both draw the background (but not foreground) of invisible characters, // so we should too. const isDefaultBackground = bg >= 256; @@ -142,8 +142,8 @@ export class TextRenderLayer extends BaseRenderLayer { }); } - private _drawForeground(terminal: ITerminal, startRow: number, endRow: number): void { - this._forEachCell(terminal, startRow, endRow, (code, char, width, x, y, fg, bg, flags) => { + private _drawForeground(terminal: ITerminal, firstRow: number, lastRow: number): void { + this._forEachCell(terminal, firstRow, lastRow, (code, char, width, x, y, fg, bg, flags) => { if (flags & FLAGS.INVISIBLE) { return; } @@ -169,15 +169,15 @@ export class TextRenderLayer extends BaseRenderLayer { }); } - public onGridChanged(terminal: ITerminal, startRow: number, endRow: number): void { + public onGridChanged(terminal: ITerminal, firstRow: number, lastRow: number): void { // Resize has not been called yet if (this._state.cache.length === 0) { return; } - this.clearCells(0, startRow, terminal.cols, endRow - startRow + 1); // endRow is inclusive - this._drawBackground(terminal, startRow, endRow); - this._drawForeground(terminal, startRow, endRow); + this.clearCells(0, firstRow, terminal.cols, lastRow - firstRow + 1); + this._drawBackground(terminal, firstRow, lastRow); + this._drawForeground(terminal, firstRow, lastRow); } public onOptionsChanged(terminal: ITerminal): void { From 9e863d037742346e6dbe201ab60a10e819c5fd31 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 23 Apr 2018 11:52:42 -0700 Subject: [PATCH 3/9] Add no else return tslint rule --- package.json | 1 + src/CompositionHelper.ts | 7 +++---- src/handlers/AltClickHandler.ts | 15 ++++++--------- tslint.json | 9 ++++++++- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/package.json b/package.json index 49ccc387..d4a06f82 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ "npm-run-all": "^4.1.2", "sorcery": "^0.10.0", "tslint": "^5.9.1", + "tslint-consistent-codestyle": "^1.13.0", "typescript": "~2.7.1", "vinyl-buffer": "^1.0.0", "vinyl-source-stream": "^1.1.0", diff --git a/src/CompositionHelper.ts b/src/CompositionHelper.ts index 389cb782..b721b7f1 100644 --- a/src/CompositionHelper.ts +++ b/src/CompositionHelper.ts @@ -92,11 +92,10 @@ export class CompositionHelper { } else if (ev.keyCode === 16 || ev.keyCode === 17 || ev.keyCode === 18) { // Continue composing if the keyCode is a modifier key return false; - } else { - // Finish composition immediately. This is mainly here for the case where enter is - // pressed and the handler needs to be triggered before the command is executed. - this._finalizeComposition(false); } + // Finish composition immediately. This is mainly here for the case where enter is + // pressed and the handler needs to be triggered before the command is executed. + this._finalizeComposition(false); } if (ev.keyCode === 229) { diff --git a/src/handlers/AltClickHandler.ts b/src/handlers/AltClickHandler.ts index f77637ea..221a993f 100644 --- a/src/handlers/AltClickHandler.ts +++ b/src/handlers/AltClickHandler.ts @@ -73,12 +73,11 @@ export class AltClickHandler { private _resetStartingRow(): string { if (this._moveToRequestedRow().length === 0) { return ''; - } else { - return repeat(this._bufferLine( - this._startCol, this._startRow, this._startCol, - this._startRow - this._wrappedRowsForRow(this._startRow), false - ).length, this._sequence(Direction.Left)); } + return repeat(this._bufferLine( + this._startCol, this._startRow, this._startCol, + this._startRow - this._wrappedRowsForRow(this._startRow), false + ).length, this._sequence(Direction.Left)); } /** @@ -180,9 +179,8 @@ export class AltClickHandler { (this._startCol >= this._endCol && startRow < this._endRow)) { // down/left or same y/left return Direction.Right; - } else { - return Direction.Left; } + return Direction.Left; } /** @@ -191,9 +189,8 @@ export class AltClickHandler { private _verticalDirection(): Direction { if (this._startRow > this._endRow) { return Direction.Up; - } else { - return Direction.Down; } + return Direction.Down; } /** diff --git a/tslint.json b/tslint.json index d42fda71..5cd44ecf 100644 --- a/tslint.json +++ b/tslint.json @@ -1,4 +1,7 @@ { + "rulesDirectory": [ + "tslint-consistent-codestyle" + ], "rules": { "array-type": [ true, @@ -86,6 +89,10 @@ "check-type", "check-type-operator", "check-preblock" - ] + ], + + "no-else-after-return": { + "options": "allow-else-if" + } } } From 9942dc384dd0435a36a66d52076788d2fe88fee0 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 23 Apr 2018 17:17:21 -0700 Subject: [PATCH 4/9] Enforce upper case for public const/enum members Fixes #1406 --- package.json | 1 + src/AccessibilityManager.ts | 20 ++++++++++---------- src/handlers/AltClickHandler.ts | 22 +++++++++++----------- tslint.json | 6 ++++++ 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/package.json b/package.json index 49ccc387..d4a06f82 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ "npm-run-all": "^4.1.2", "sorcery": "^0.10.0", "tslint": "^5.9.1", + "tslint-consistent-codestyle": "^1.13.0", "typescript": "~2.7.1", "vinyl-buffer": "^1.0.0", "vinyl-source-stream": "^1.1.0", diff --git a/src/AccessibilityManager.ts b/src/AccessibilityManager.ts index 5a1f4770..4d1c689e 100644 --- a/src/AccessibilityManager.ts +++ b/src/AccessibilityManager.ts @@ -13,8 +13,8 @@ import { IDisposable } from 'xterm'; const MAX_ROWS_TO_READ = 20; enum BoundaryPosition { - Top, - Bottom + TOP, + BOTTOM } export class AccessibilityManager implements IDisposable { @@ -54,8 +54,8 @@ export class AccessibilityManager implements IDisposable { this._rowContainer.appendChild(this._rowElements[i]); } - this._topBoundaryFocusListener = e => this._onBoundaryFocus(e, BoundaryPosition.Top); - this._bottomBoundaryFocusListener = e => this._onBoundaryFocus(e, BoundaryPosition.Bottom); + this._topBoundaryFocusListener = e => this._onBoundaryFocus(e, BoundaryPosition.TOP); + this._bottomBoundaryFocusListener = e => this._onBoundaryFocus(e, BoundaryPosition.BOTTOM); this._rowElements[0].addEventListener('focus', this._topBoundaryFocusListener); this._rowElements[this._rowElements.length - 1].addEventListener('focus', this._bottomBoundaryFocusListener); @@ -101,11 +101,11 @@ export class AccessibilityManager implements IDisposable { private _onBoundaryFocus(e: FocusEvent, position: BoundaryPosition): void { const boundaryElement = e.target; - const beforeBoundaryElement = this._rowElements[position === BoundaryPosition.Top ? 1 : this._rowElements.length - 2]; + const beforeBoundaryElement = this._rowElements[position === BoundaryPosition.TOP ? 1 : this._rowElements.length - 2]; // Don't scroll if the buffer top has reached the end in that direction const posInSet = boundaryElement.getAttribute('aria-posinset'); - const lastRowPos = position === BoundaryPosition.Top ? '1' : `${this._terminal.buffer.lines.length}`; + const lastRowPos = position === BoundaryPosition.TOP ? '1' : `${this._terminal.buffer.lines.length}`; if (posInSet === lastRowPos) { return; } @@ -119,7 +119,7 @@ export class AccessibilityManager implements IDisposable { // Remove old boundary element from array let topBoundaryElement: HTMLElement; let bottomBoundaryElement: HTMLElement; - if (position === BoundaryPosition.Top) { + if (position === BoundaryPosition.TOP) { topBoundaryElement = boundaryElement; bottomBoundaryElement = this._rowElements.pop()!; this._rowContainer.removeChild(bottomBoundaryElement); @@ -134,7 +134,7 @@ export class AccessibilityManager implements IDisposable { bottomBoundaryElement.removeEventListener('focus', this._bottomBoundaryFocusListener); // Add new element to array/DOM - if (position === BoundaryPosition.Top) { + if (position === BoundaryPosition.TOP) { const newElement = this._createAccessibilityTreeNode(); this._rowElements.unshift(newElement); this._rowContainer.insertAdjacentElement('afterbegin', newElement); @@ -149,10 +149,10 @@ export class AccessibilityManager implements IDisposable { this._rowElements[this._rowElements.length - 1].addEventListener('focus', this._bottomBoundaryFocusListener); // Scroll up - this._terminal.scrollLines(position === BoundaryPosition.Top ? -1 : 1); + this._terminal.scrollLines(position === BoundaryPosition.TOP ? -1 : 1); // Focus new boundary before element - this._rowElements[position === BoundaryPosition.Top ? 1 : this._rowElements.length - 2].focus(); + this._rowElements[position === BoundaryPosition.TOP ? 1 : this._rowElements.length - 2].focus(); // Prevent the standard behavior e.preventDefault(); diff --git a/src/handlers/AltClickHandler.ts b/src/handlers/AltClickHandler.ts index f77637ea..33808973 100644 --- a/src/handlers/AltClickHandler.ts +++ b/src/handlers/AltClickHandler.ts @@ -7,10 +7,10 @@ import { ITerminal, ICircularList, LineData } from '../Types'; import { C0 } from '../EscapeSequences'; enum Direction { - Up = 'A', - Down = 'B', - Right = 'C', - Left = 'D' + UP = 'A', + DOWN = 'B', + RIGHT = 'C', + LEFT = 'D' } export class AltClickHandler { @@ -77,7 +77,7 @@ export class AltClickHandler { return repeat(this._bufferLine( this._startCol, this._startRow, this._startCol, this._startRow - this._wrappedRowsForRow(this._startRow), false - ).length, this._sequence(Direction.Left)); + ).length, this._sequence(Direction.LEFT)); } } @@ -110,7 +110,7 @@ export class AltClickHandler { return repeat(this._bufferLine( this._startCol, startRow, this._endCol, endRow, - direction === Direction.Right + direction === Direction.RIGHT ).length, this._sequence(direction)); } @@ -133,7 +133,7 @@ export class AltClickHandler { let endRow = this._endRow - this._wrappedRowsForRow(this._endRow); for (let i = 0; i < Math.abs(startRow - endRow); i++) { - let direction = this._verticalDirection() === Direction.Up ? -1 : 1; + let direction = this._verticalDirection() === Direction.UP ? -1 : 1; if ((this._lines.get(startRow + (direction * i))).isWrapped) { wrappedRows++; @@ -179,9 +179,9 @@ export class AltClickHandler { startRow <= this._endRow) || // down/right or same y/right (this._startCol >= this._endCol && startRow < this._endRow)) { // down/left or same y/left - return Direction.Right; + return Direction.RIGHT; } else { - return Direction.Left; + return Direction.LEFT; } } @@ -190,9 +190,9 @@ export class AltClickHandler { */ private _verticalDirection(): Direction { if (this._startRow > this._endRow) { - return Direction.Up; + return Direction.UP; } else { - return Direction.Down; + return Direction.DOWN; } } diff --git a/tslint.json b/tslint.json index d42fda71..2ead68ed 100644 --- a/tslint.json +++ b/tslint.json @@ -1,4 +1,5 @@ { + "rulesDirectory": ["tslint-consistent-codestyle"], "rules": { "array-type": [ true, @@ -86,6 +87,11 @@ "check-type", "check-type-operator", "check-preblock" + ], + + "naming-convention": [ + true, + {"type": "property", "modifiers": ["public", "static", "const"], "format": "UPPER_CASE"} ] } } From fcde7be62ae6beae4f56eaa6fc7248484085bc0c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 23 Apr 2018 17:35:36 -0700 Subject: [PATCH 5/9] Enforce const enums Fixes #1408 --- src/AccessibilityManager.ts | 2 +- src/Parser.ts | 2 +- src/SelectionManager.ts | 2 +- src/Types.ts | 2 +- src/handlers/AltClickHandler.ts | 2 +- src/renderer/Types.ts | 2 +- tslint.json | 3 +++ 7 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/AccessibilityManager.ts b/src/AccessibilityManager.ts index 4d1c689e..a7e205e3 100644 --- a/src/AccessibilityManager.ts +++ b/src/AccessibilityManager.ts @@ -12,7 +12,7 @@ import { IDisposable } from 'xterm'; const MAX_ROWS_TO_READ = 20; -enum BoundaryPosition { +const enum BoundaryPosition { TOP, BOTTOM } diff --git a/src/Parser.ts b/src/Parser.ts index 21e5a612..372d8443 100644 --- a/src/Parser.ts +++ b/src/Parser.ts @@ -150,7 +150,7 @@ csiStateHandler['s'] = (handler, params) => handler.saveCursor(params); csiStateHandler['u'] = (handler, params) => handler.restoreCursor(params); csiStateHandler[C0.CAN] = (handler, params, prefix, postfix, parser) => parser.setState(ParserState.NORMAL); -export enum ParserState { +export const enum ParserState { NORMAL = 0, ESCAPED = 1, CSI_PARAM = 2, diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index a50203bd..93da887a 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -54,7 +54,7 @@ interface IWordPosition { /** * A selection mode, this drives how the selection behaves on mouse move. */ -enum SelectionMode { +const enum SelectionMode { NORMAL, WORD, LINE diff --git a/src/Types.ts b/src/Types.ts index 2442b027..15498ce1 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -17,7 +17,7 @@ export type LineData = CharData[]; export type LinkMatcherHandler = (event: MouseEvent, uri: string) => void; export type LinkMatcherValidationCallback = (uri: string, callback: (isValid: boolean) => void) => void; -export enum LinkHoverEventTypes { +export const enum LinkHoverEventTypes { HOVER = 'linkhover', TOOLTIP = 'linktooltip', LEAVE = 'linkleave' diff --git a/src/handlers/AltClickHandler.ts b/src/handlers/AltClickHandler.ts index 33808973..ca31f02c 100644 --- a/src/handlers/AltClickHandler.ts +++ b/src/handlers/AltClickHandler.ts @@ -6,7 +6,7 @@ import { ITerminal, ICircularList, LineData } from '../Types'; import { C0 } from '../EscapeSequences'; -enum Direction { +const enum Direction { UP = 'A', DOWN = 'B', RIGHT = 'C', diff --git a/src/renderer/Types.ts b/src/renderer/Types.ts index 8c464bec..bd5b13eb 100644 --- a/src/renderer/Types.ts +++ b/src/renderer/Types.ts @@ -10,7 +10,7 @@ import { IColorSet } from '../shared/Types'; /** * Flags used to render terminal text properly. */ -export enum FLAGS { +export const enum FLAGS { BOLD = 1, UNDERLINE = 2, BLINK = 4, diff --git a/tslint.json b/tslint.json index 2ead68ed..dd259d63 100644 --- a/tslint.json +++ b/tslint.json @@ -92,6 +92,9 @@ "naming-convention": [ true, {"type": "property", "modifiers": ["public", "static", "const"], "format": "UPPER_CASE"} + ], + "prefer-const-enum": [ + true ] } } From a5a03c40f21ab4c473313453e09f9c353ff184c5 Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Tue, 24 Apr 2018 23:15:22 -0700 Subject: [PATCH 6/9] Batch background draws together to reduce draws Adjacent cells on the same row sharing the same background color will be drawn using the same `fillCells` call. This should make drawing an application with a solid or mostly solid background color (e.g. vim) faster. I set vim to draw the background color for a file, and started scrolling through it. Before, _drawBackground was taking around 6ms per frame. After this commit, it was taking around 0.5ms per frame. Based on prior experience, I'd expect these results to be more drastic for larger terminal windows. The demo's window is pretty small. --- src/renderer/BaseRenderLayer.ts | 2 +- src/renderer/TextRenderLayer.ts | 52 +++++++++++++++++++++++++++++---- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts index fbffd547..4200e112 100644 --- a/src/renderer/BaseRenderLayer.ts +++ b/src/renderer/BaseRenderLayer.ts @@ -26,7 +26,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { private _container: HTMLElement, id: string, zIndex: number, - private _alpha: boolean, + protected _alpha: boolean, protected _colors: IColorSet ) { this._canvas = document.createElement('canvas'); diff --git a/src/renderer/TextRenderLayer.ts b/src/renderer/TextRenderLayer.ts index 58d0d790..6c9a72c5 100644 --- a/src/renderer/TextRenderLayer.ts +++ b/src/renderer/TextRenderLayer.ts @@ -128,18 +128,58 @@ export class TextRenderLayer extends BaseRenderLayer { } } + /** + * Draws the background for a specified range of columns. Tries to batch adjacent cells of the + * same color together to reduce draw calls. + */ private _drawBackground(terminal: ITerminal, firstRow: number, lastRow: number): void { + const ctx = this._ctx; + const cols = terminal.cols; + let startX: number = 0; + let startY: number = 0; + let prevFillStyle: string | null = null; + + ctx.save(); + this._forEachCell(terminal, firstRow, lastRow, (code, char, width, x, y, fg, bg, flags) => { // libvte and xterm both draw the background (but not foreground) of invisible characters, // so we should too. - const isDefaultBackground = bg >= 256; - if (!isDefaultBackground) { - this._ctx.save(); - this._ctx.fillStyle = (bg === INVERTED_DEFAULT_COLOR ? this._colors.foreground.css : this._colors.ansi[bg].css); - this.fillCells(x, y, width, 1); - this._ctx.restore(); + let nextFillStyle = null; // null represents default background color + if (bg === INVERTED_DEFAULT_COLOR) { + nextFillStyle = this._colors.foreground.css; + } else if (bg < 256) { + nextFillStyle = this._colors.ansi[bg].css; } + + if (prevFillStyle === null) { + // This is either the first iteration, or the default background was set. Either way, we + // don't need to draw anything. + startX = x; + startY = y; + } if (y !== startY) { + // our row changed, draw the previous row + ctx.fillStyle = prevFillStyle; + this.fillCells(startX, startY, cols - startX, 1); + startX = x; + startY = y; + } else if (prevFillStyle !== nextFillStyle) { + // our color changed, draw the previous characters in this row + ctx.fillStyle = prevFillStyle; + this.fillCells(startX, startY, x - startX, 1); + startX = x; + startY = y; + } + + prevFillStyle = nextFillStyle; }); + + // flush the last color we encountered + if (prevFillStyle !== null) { + ctx.fillStyle = prevFillStyle; + this.fillCells(startX, startY, cols - startX, 1); + } + + ctx.restore(); } private _drawForeground(terminal: ITerminal, firstRow: number, lastRow: number): void { From 994895af6de250d89d05365d7ef7213269c6f7c1 Mon Sep 17 00:00:00 2001 From: pro-src <34285059+pro-src@users.noreply.github.com> Date: Thu, 26 Apr 2018 11:53:20 -0500 Subject: [PATCH 7/9] Update README.md Replace the deprecated octal escapes with Unicode escapes. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Deprecated_octal --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 48a46fd9..50e954da 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ To start using xterm.js on your browser, add the `xterm.js` and `xterm.css` to t From 882d5a86d390f220545c42b302a2a2bb8318da2c Mon Sep 17 00:00:00 2001 From: pro-src <34285059+pro-src@users.noreply.github.com> Date: Fri, 27 Apr 2018 02:19:48 -0500 Subject: [PATCH 8/9] Update README.md - prefer hex escapes over octal --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 50e954da..07baec5a 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ To start using xterm.js on your browser, add the `xterm.js` and `xterm.css` to t From 513847fd806b55719fb08a522d11f2ff5aab2216 Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Fri, 27 Apr 2018 20:47:55 -0700 Subject: [PATCH 9/9] Make BaseRenderLayer's _alpha private again This was accidentally left over from some earlier changes I was playing with. --- src/renderer/BaseRenderLayer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts index 4200e112..fbffd547 100644 --- a/src/renderer/BaseRenderLayer.ts +++ b/src/renderer/BaseRenderLayer.ts @@ -26,7 +26,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { private _container: HTMLElement, id: string, zIndex: number, - protected _alpha: boolean, + private _alpha: boolean, protected _colors: IColorSet ) { this._canvas = document.createElement('canvas');