From 374a8b70eb786d28d78b61da34d5b69b1c474647 Mon Sep 17 00:00:00 2001 From: Hindol Adhya Date: Thu, 6 Sep 2018 11:37:24 +0530 Subject: [PATCH 1/2] Add support for block and underline style cursor in DOM rendering mode --- src/Types.ts | 8 +++++ src/renderer/dom/DomRenderer.ts | 18 ++++++---- .../dom/DomRendererRowFactory.test.ts | 36 ++++++++++--------- src/renderer/dom/DomRendererRowFactory.ts | 21 +++++++++-- 4 files changed, 57 insertions(+), 26 deletions(-) diff --git a/src/Types.ts b/src/Types.ts index 21c6c57105..cbb068e2f3 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -522,3 +522,11 @@ export interface IBufferLine { deleteCells(pos: number, n: number, fill: CharData): void; replaceCells(start: number, end: number, fill: CharData): void; } + +/** + * Interface for cursor options in the cursor line of the terminal buffer. + */ +export interface ICursorOptions { + isCursorRow: boolean; + cursorStyle?: string; +} diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts index 97eabc32d8..084edd7bc1 100644 --- a/src/renderer/dom/DomRenderer.ts +++ b/src/renderer/dom/DomRenderer.ts @@ -9,7 +9,7 @@ import { ITheme } from 'xterm'; import { EventEmitter } from '../../EventEmitter'; import { ColorManager } from '../ColorManager'; import { RenderDebouncer } from '../../ui/RenderDebouncer'; -import { BOLD_CLASS, ITALIC_CLASS, CURSOR_CLASS, DomRendererRowFactory } from './DomRendererRowFactory'; +import { BOLD_CLASS, ITALIC_CLASS, CURSOR_CLASS, CURSOR_STYLE_BLOCK_CLASS, CURSOR_STYLE_BAR_CLASS, CURSOR_STYLE_UNDERLINE_CLASS, DomRendererRowFactory } from './DomRendererRowFactory'; const TERMINAL_CLASS_PREFIX = 'xterm-dom-renderer-owner-'; const ROW_CONTAINER_CLASS = 'xterm-rows'; @@ -160,13 +160,19 @@ export class DomRenderer extends EventEmitter implements IRenderer { `}`; // Cursor styles += - `${this._terminalSelector} .${ROW_CONTAINER_CLASS}.${FOCUS_CLASS} .${CURSOR_CLASS} {` + + `${this._terminalSelector} .${ROW_CONTAINER_CLASS}:not(.${FOCUS_CLASS}) .${CURSOR_CLASS} {` + + ` outline: 1px solid ${this.colorManager.colors.cursor.css};` + + ` outline-offset: -1px;` + + `}` + + `${this._terminalSelector} .${ROW_CONTAINER_CLASS}.${FOCUS_CLASS} .${CURSOR_CLASS}.${CURSOR_STYLE_BLOCK_CLASS} {` + ` background-color: ${this.colorManager.colors.cursor.css};` + ` color: ${this.colorManager.colors.cursorAccent.css};` + `}` + - `${this._terminalSelector} .${ROW_CONTAINER_CLASS}:not(.${FOCUS_CLASS}) .${CURSOR_CLASS} {` + - ` outline: 1px solid #fff;` + - ` outline-offset: -1px;` + + `${this._terminalSelector} .${ROW_CONTAINER_CLASS}.${FOCUS_CLASS} .${CURSOR_CLASS}.${CURSOR_STYLE_BAR_CLASS} {` + + ` box-shadow: 1px 0 0 ${this.colorManager.colors.cursor.css} inset;` + + `}` + + `${this._terminalSelector} .${ROW_CONTAINER_CLASS}.${FOCUS_CLASS} .${CURSOR_CLASS}.${CURSOR_STYLE_UNDERLINE_CLASS} {` + + ` box-shadow: 0 -1px 0 ${this.colorManager.colors.cursor.css} inset;` + `}`; // Selection styles += @@ -319,7 +325,7 @@ export class DomRenderer extends EventEmitter implements IRenderer { const row = y + terminal.buffer.ydisp; const lineData = terminal.buffer.lines.get(row); - rowElement.appendChild(this._rowFactory.createRow(lineData, row === cursorAbsoluteY, cursorX, terminal.charMeasure.width, terminal.cols)); + rowElement.appendChild(this._rowFactory.createRow(lineData, { isCursorRow: row === cursorAbsoluteY, cursorStyle: terminal.getOption('cursorStyle') }, cursorX, terminal.charMeasure.width, terminal.cols)); } this._terminal.emit('refresh', {start, end}); diff --git a/src/renderer/dom/DomRendererRowFactory.test.ts b/src/renderer/dom/DomRendererRowFactory.test.ts index 17b1b938b3..eaf95d5934 100644 --- a/src/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/renderer/dom/DomRendererRowFactory.test.ts @@ -24,7 +24,7 @@ describe('DomRendererRowFactory', () => { describe('createRow', () => { it('should create an element for every character in the row', () => { - const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); assert.equal(getFragmentHtml(fragment), ' ' + ' ' @@ -35,24 +35,26 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [DEFAULT_ATTR, '語', 2, '語'.charCodeAt(0)]); // There should be no element for the following "empty" cell lineData.set(1, [DEFAULT_ATTR, '', 0, undefined]); - const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); assert.equal(getFragmentHtml(fragment), '語' ); }); - it('should add class for cursor', () => { - const fragment = rowFactory.createRow(lineData, true, 0, 5, 20); - assert.equal(getFragmentHtml(fragment), - ' ' + - ' ' - ); + it('should add class for cursor and cursor style', () => { + for (const style of ['block', 'bar', 'underline']) { + const fragment = rowFactory.createRow(lineData, { isCursorRow: true, cursorStyle: style }, 0, 5, 20); + assert.equal(getFragmentHtml(fragment), + ` ` + + ' ' + ); + } }); it('should not render cells that go beyond the terminal\'s columns', () => { lineData.set(0, [DEFAULT_ATTR, 'a', 1, 'a'.charCodeAt(0)]); lineData.set(1, [DEFAULT_ATTR, 'b', 1, 'b'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, false, 0, 5, 1); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 1); assert.equal(getFragmentHtml(fragment), 'a' ); @@ -61,7 +63,7 @@ describe('DomRendererRowFactory', () => { describe('attributes', () => { it('should add class for bold', () => { lineData.set(0, [DEFAULT_ATTR | (FLAGS.BOLD << 18), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -70,7 +72,7 @@ describe('DomRendererRowFactory', () => { it('should add class for italic', () => { lineData.set(0, [DEFAULT_ATTR | (FLAGS.ITALIC << 18), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -81,7 +83,7 @@ describe('DomRendererRowFactory', () => { const defaultAttrNoFgColor = (0 << 9) | (256 << 0); for (let i = 0; i < 256; i++) { lineData.set(0, [defaultAttrNoFgColor | (i << 9), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' @@ -93,7 +95,7 @@ describe('DomRendererRowFactory', () => { const defaultAttrNoBgColor = (257 << 9) | (0 << 0); for (let i = 0; i < 256; i++) { lineData.set(0, [defaultAttrNoBgColor | (i << 0), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' @@ -103,7 +105,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert colors', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (2 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -112,7 +114,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert default fg color', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (257 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -121,7 +123,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert default bg color', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (1 << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -131,7 +133,7 @@ describe('DomRendererRowFactory', () => { it('should turn bold fg text bright', () => { for (let i = 0; i < 8; i++) { lineData.set(0, [(FLAGS.BOLD << 18) | (i << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' diff --git a/src/renderer/dom/DomRendererRowFactory.ts b/src/renderer/dom/DomRendererRowFactory.ts index 351055ef7c..692e09e857 100644 --- a/src/renderer/dom/DomRendererRowFactory.ts +++ b/src/renderer/dom/DomRendererRowFactory.ts @@ -5,11 +5,14 @@ import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_ATTR_INDEX, CHAR_DATA_WIDTH_INDEX } from '../../Buffer'; import { FLAGS } from '../Types'; -import { IBufferLine } from '../../Types'; +import { IBufferLine, ICursorOptions } from '../../Types'; export const BOLD_CLASS = 'xterm-bold'; export const ITALIC_CLASS = 'xterm-italic'; export const CURSOR_CLASS = 'xterm-cursor'; +export const CURSOR_STYLE_BLOCK_CLASS = 'xterm-cursor-block'; +export const CURSOR_STYLE_BAR_CLASS = 'xterm-cursor-bar'; +export const CURSOR_STYLE_UNDERLINE_CLASS = 'xterm-cursor-underline'; export class DomRendererRowFactory { constructor( @@ -17,7 +20,7 @@ export class DomRendererRowFactory { ) { } - public createRow(lineData: IBufferLine, isCursorRow: boolean, cursorX: number, cellWidth: number, cols: number): DocumentFragment { + public createRow(lineData: IBufferLine, cursorOptions: ICursorOptions, cursorX: number, cellWidth: number, cols: number): DocumentFragment { const fragment = this._document.createDocumentFragment(); let colCount = 0; @@ -46,8 +49,20 @@ export class DomRendererRowFactory { let bg = attr & 0x1ff; let fg = (attr >> 9) & 0x1ff; - if (isCursorRow && x === cursorX) { + if (cursorOptions.isCursorRow && x === cursorX) { charElement.classList.add(CURSOR_CLASS); + + switch (cursorOptions.cursorStyle) { + case 'block': + charElement.classList.add(CURSOR_STYLE_BLOCK_CLASS); + break; + case 'bar': + charElement.classList.add(CURSOR_STYLE_BAR_CLASS); + break; + case 'underline': + charElement.classList.add(CURSOR_STYLE_UNDERLINE_CLASS); + break; + } } // If inverse flag is on, the foreground should become the background. From 2716413e665c4f7b4c30ecd6b3338a6cb220df67 Mon Sep 17 00:00:00 2001 From: Hindol Adhya Date: Fri, 7 Sep 2018 12:50:34 +0530 Subject: [PATCH 2/2] Updates as per code review comments Getting rid of the ICursorOptions interface to improve performance --- src/Types.ts | 8 ------- src/renderer/dom/DomRenderer.ts | 3 ++- .../dom/DomRendererRowFactory.test.ts | 24 +++++++++---------- src/renderer/dom/DomRendererRowFactory.ts | 14 +++++------ 4 files changed, 21 insertions(+), 28 deletions(-) diff --git a/src/Types.ts b/src/Types.ts index cbb068e2f3..21c6c57105 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -522,11 +522,3 @@ export interface IBufferLine { deleteCells(pos: number, n: number, fill: CharData): void; replaceCells(start: number, end: number, fill: CharData): void; } - -/** - * Interface for cursor options in the cursor line of the terminal buffer. - */ -export interface ICursorOptions { - isCursorRow: boolean; - cursorStyle?: string; -} diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts index 084edd7bc1..b3f69ffcd5 100644 --- a/src/renderer/dom/DomRenderer.ts +++ b/src/renderer/dom/DomRenderer.ts @@ -325,7 +325,8 @@ export class DomRenderer extends EventEmitter implements IRenderer { const row = y + terminal.buffer.ydisp; const lineData = terminal.buffer.lines.get(row); - rowElement.appendChild(this._rowFactory.createRow(lineData, { isCursorRow: row === cursorAbsoluteY, cursorStyle: terminal.getOption('cursorStyle') }, cursorX, terminal.charMeasure.width, terminal.cols)); + const cursorStyle = terminal.options.cursorStyle; + rowElement.appendChild(this._rowFactory.createRow(lineData, row === cursorAbsoluteY, cursorStyle, cursorX, terminal.charMeasure.width, terminal.cols)); } this._terminal.emit('refresh', {start, end}); diff --git a/src/renderer/dom/DomRendererRowFactory.test.ts b/src/renderer/dom/DomRendererRowFactory.test.ts index eaf95d5934..4e795b6cbd 100644 --- a/src/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/renderer/dom/DomRendererRowFactory.test.ts @@ -24,7 +24,7 @@ describe('DomRendererRowFactory', () => { describe('createRow', () => { it('should create an element for every character in the row', () => { - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), ' ' + ' ' @@ -35,7 +35,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [DEFAULT_ATTR, '語', 2, '語'.charCodeAt(0)]); // There should be no element for the following "empty" cell lineData.set(1, [DEFAULT_ATTR, '', 0, undefined]); - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), '語' ); @@ -43,7 +43,7 @@ describe('DomRendererRowFactory', () => { it('should add class for cursor and cursor style', () => { for (const style of ['block', 'bar', 'underline']) { - const fragment = rowFactory.createRow(lineData, { isCursorRow: true, cursorStyle: style }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, true, style, 0, 5, 20); assert.equal(getFragmentHtml(fragment), ` ` + ' ' @@ -54,7 +54,7 @@ describe('DomRendererRowFactory', () => { it('should not render cells that go beyond the terminal\'s columns', () => { lineData.set(0, [DEFAULT_ATTR, 'a', 1, 'a'.charCodeAt(0)]); lineData.set(1, [DEFAULT_ATTR, 'b', 1, 'b'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 1); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 1); assert.equal(getFragmentHtml(fragment), 'a' ); @@ -63,7 +63,7 @@ describe('DomRendererRowFactory', () => { describe('attributes', () => { it('should add class for bold', () => { lineData.set(0, [DEFAULT_ATTR | (FLAGS.BOLD << 18), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -72,7 +72,7 @@ describe('DomRendererRowFactory', () => { it('should add class for italic', () => { lineData.set(0, [DEFAULT_ATTR | (FLAGS.ITALIC << 18), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -83,7 +83,7 @@ describe('DomRendererRowFactory', () => { const defaultAttrNoFgColor = (0 << 9) | (256 << 0); for (let i = 0; i < 256; i++) { lineData.set(0, [defaultAttrNoFgColor | (i << 9), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' @@ -95,7 +95,7 @@ describe('DomRendererRowFactory', () => { const defaultAttrNoBgColor = (257 << 9) | (0 << 0); for (let i = 0; i < 256; i++) { lineData.set(0, [defaultAttrNoBgColor | (i << 0), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' @@ -105,7 +105,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert colors', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (2 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -114,7 +114,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert default fg color', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (257 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -123,7 +123,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert default bg color', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (1 << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -133,7 +133,7 @@ describe('DomRendererRowFactory', () => { it('should turn bold fg text bright', () => { for (let i = 0; i < 8; i++) { lineData.set(0, [(FLAGS.BOLD << 18) | (i << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]); - const fragment = rowFactory.createRow(lineData, { isCursorRow: false }, 0, 5, 20); + const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' diff --git a/src/renderer/dom/DomRendererRowFactory.ts b/src/renderer/dom/DomRendererRowFactory.ts index 692e09e857..4bb5990223 100644 --- a/src/renderer/dom/DomRendererRowFactory.ts +++ b/src/renderer/dom/DomRendererRowFactory.ts @@ -5,7 +5,7 @@ import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_ATTR_INDEX, CHAR_DATA_WIDTH_INDEX } from '../../Buffer'; import { FLAGS } from '../Types'; -import { IBufferLine, ICursorOptions } from '../../Types'; +import { IBufferLine } from '../../Types'; export const BOLD_CLASS = 'xterm-bold'; export const ITALIC_CLASS = 'xterm-italic'; @@ -20,7 +20,7 @@ export class DomRendererRowFactory { ) { } - public createRow(lineData: IBufferLine, cursorOptions: ICursorOptions, cursorX: number, cellWidth: number, cols: number): DocumentFragment { + public createRow(lineData: IBufferLine, isCursorRow: boolean, cursorStyle: string | undefined, cursorX: number, cellWidth: number, cols: number): DocumentFragment { const fragment = this._document.createDocumentFragment(); let colCount = 0; @@ -49,19 +49,19 @@ export class DomRendererRowFactory { let bg = attr & 0x1ff; let fg = (attr >> 9) & 0x1ff; - if (cursorOptions.isCursorRow && x === cursorX) { + if (isCursorRow && x === cursorX) { charElement.classList.add(CURSOR_CLASS); - switch (cursorOptions.cursorStyle) { - case 'block': - charElement.classList.add(CURSOR_STYLE_BLOCK_CLASS); - break; + switch (cursorStyle) { case 'bar': charElement.classList.add(CURSOR_STYLE_BAR_CLASS); break; case 'underline': charElement.classList.add(CURSOR_STYLE_UNDERLINE_CLASS); break; + default: + charElement.classList.add(CURSOR_STYLE_BLOCK_CLASS); + break; } }