From 791c020a563b2acbd99e0c6f03f12273179f73c1 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 9 Oct 2018 11:03:31 -0700 Subject: [PATCH 01/18] Allow inverse colors to be stored in glyph keys Fixes #1737 --- src/renderer/atlas/StaticCharAtlas.ts | 4 ++-- src/renderer/atlas/Types.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderer/atlas/StaticCharAtlas.ts b/src/renderer/atlas/StaticCharAtlas.ts index c0d8a814..5c56f019 100644 --- a/src/renderer/atlas/StaticCharAtlas.ts +++ b/src/renderer/atlas/StaticCharAtlas.ts @@ -41,7 +41,7 @@ export default class StaticCharAtlas extends BaseCharAtlas { const isAscii = glyph.code < 256; // A color is basic if it is one of the 4 bit ANSI colors. const isBasicColor = glyph.fg < 16; - const isDefaultColor = glyph.fg >= 256; + const isDefaultColor = glyph.fg === 256; const isDefaultBackground = glyph.bg >= 256; return isAscii && (isBasicColor || isDefaultColor) && isDefaultBackground && !glyph.italic; } @@ -60,7 +60,7 @@ export default class StaticCharAtlas extends BaseCharAtlas { let colorIndex = 0; if (glyph.fg < 256) { colorIndex = 2 + glyph.fg + (glyph.bold ? 16 : 0); - } else { + } else if (glyph.fg === 256) { // If default color and bold if (glyph.bold) { colorIndex = 1; diff --git a/src/renderer/atlas/Types.ts b/src/renderer/atlas/Types.ts index 6fb3c5d1..614a6e92 100644 --- a/src/renderer/atlas/Types.ts +++ b/src/renderer/atlas/Types.ts @@ -3,7 +3,7 @@ * @license MIT */ -export const INVERTED_DEFAULT_COLOR = -1; +export const INVERTED_DEFAULT_COLOR = 258; export const DIM_OPACITY = 0.5; export interface IGlyphIdentifier { From 1e9564788ef286ee50fc59261d80b72474d80b7a Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 9 Oct 2018 11:12:35 -0700 Subject: [PATCH 02/18] Use constants for default colors --- src/Buffer.ts | 3 ++- src/renderer/BaseRenderLayer.ts | 4 ++-- src/renderer/LinkRenderLayer.ts | 4 ++-- src/renderer/TextRenderLayer.ts | 10 +++++----- src/renderer/atlas/DynamicCharAtlas.ts | 2 +- src/renderer/atlas/StaticCharAtlas.ts | 10 +++++----- src/renderer/atlas/Types.ts | 3 ++- src/renderer/dom/DomRendererRowFactory.test.ts | 11 ++++++----- src/renderer/dom/DomRendererRowFactory.ts | 9 +++++---- 9 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index e54f752b..7b5ee99a 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -8,8 +8,9 @@ import { CharData, ITerminal, IBuffer, IBufferLine, BufferIndex, IBufferStringIt import { EventEmitter } from './common/EventEmitter'; import { IMarker } from 'xterm'; import { BufferLine, BufferLineTypedArray } from './BufferLine'; +import { DEFAULT_COLOR } from './renderer/atlas/Types'; -export const DEFAULT_ATTR = (0 << 18) | (257 << 9) | (256 << 0); +export const DEFAULT_ATTR = (0 << 18) | (DEFAULT_COLOR << 9) | (256 << 0); export const CHAR_DATA_ATTR_INDEX = 0; export const CHAR_DATA_CHAR_INDEX = 1; export const CHAR_DATA_WIDTH_INDEX = 2; diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts index 84e290e7..827356af 100644 --- a/src/renderer/BaseRenderLayer.ts +++ b/src/renderer/BaseRenderLayer.ts @@ -5,7 +5,7 @@ import { IRenderLayer, IColorSet, IRenderDimensions } from './Types'; import { CharData, ITerminal } from '../Types'; -import { DIM_OPACITY, INVERTED_DEFAULT_COLOR, IGlyphIdentifier } from './atlas/Types'; +import { DIM_OPACITY, INVERTED_DEFAULT_COLOR, IGlyphIdentifier, DEFAULT_COLOR } from './atlas/Types'; import BaseCharAtlas from './atlas/BaseCharAtlas'; import { acquireCharAtlas } from './atlas/CharAtlasCache'; import { CHAR_DATA_CHAR_INDEX } from '../Buffer'; @@ -298,7 +298,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { if (fg === INVERTED_DEFAULT_COLOR) { this._ctx.fillStyle = this._colors.background.css; - } else if (fg < 256) { + } else if (fg < DEFAULT_COLOR) { // 256 color support this._ctx.fillStyle = this._colors.ansi[fg].css; } else { diff --git a/src/renderer/LinkRenderLayer.ts b/src/renderer/LinkRenderLayer.ts index 8679939a..c0d190ac 100644 --- a/src/renderer/LinkRenderLayer.ts +++ b/src/renderer/LinkRenderLayer.ts @@ -6,7 +6,7 @@ import { ILinkHoverEvent, ITerminal, ILinkifierAccessor, LinkHoverEventTypes } from '../Types'; import { IColorSet, IRenderDimensions } from './Types'; import { BaseRenderLayer } from './BaseRenderLayer'; -import { INVERTED_DEFAULT_COLOR } from './atlas/Types'; +import { INVERTED_DEFAULT_COLOR, DEFAULT_COLOR } from './atlas/Types'; export class LinkRenderLayer extends BaseRenderLayer { private _state: ILinkHoverEvent = null; @@ -42,7 +42,7 @@ export class LinkRenderLayer extends BaseRenderLayer { private _onLinkHover(e: ILinkHoverEvent): void { if (e.fg === INVERTED_DEFAULT_COLOR) { this._ctx.fillStyle = this._colors.background.css; - } else if (e.fg < 256) { + } else if (e.fg < DEFAULT_COLOR) { // 256 color support this._ctx.fillStyle = this._colors.ansi[e.fg].css; } else { diff --git a/src/renderer/TextRenderLayer.ts b/src/renderer/TextRenderLayer.ts index 7f10e7c9..984272f6 100644 --- a/src/renderer/TextRenderLayer.ts +++ b/src/renderer/TextRenderLayer.ts @@ -6,7 +6,7 @@ import { CHAR_DATA_ATTR_INDEX, CHAR_DATA_CODE_INDEX, CHAR_DATA_CHAR_INDEX, CHAR_DATA_WIDTH_INDEX, NULL_CELL_CODE } from '../Buffer'; import { FLAGS, IColorSet, IRenderDimensions, ICharacterJoinerRegistry } from './Types'; import { CharData, ITerminal } from '../Types'; -import { INVERTED_DEFAULT_COLOR } from './atlas/Types'; +import { INVERTED_DEFAULT_COLOR, DEFAULT_COLOR } from './atlas/Types'; import { GridCache } from './GridCache'; import { BaseRenderLayer } from './BaseRenderLayer'; @@ -143,10 +143,10 @@ export class TextRenderLayer extends BaseRenderLayer { const temp = bg; bg = fg; fg = temp; - if (fg === 256) { + if (fg === DEFAULT_COLOR) { fg = INVERTED_DEFAULT_COLOR; } - if (bg === 257) { + if (bg === DEFAULT_COLOR) { bg = INVERTED_DEFAULT_COLOR; } } @@ -186,7 +186,7 @@ export class TextRenderLayer extends BaseRenderLayer { let nextFillStyle = null; // null represents default background color if (bg === INVERTED_DEFAULT_COLOR) { nextFillStyle = this._colors.foreground.css; - } else if (bg < 256) { + } else if (bg < DEFAULT_COLOR) { nextFillStyle = this._colors.ansi[bg].css; } @@ -230,7 +230,7 @@ export class TextRenderLayer extends BaseRenderLayer { this._ctx.save(); if (fg === INVERTED_DEFAULT_COLOR) { this._ctx.fillStyle = this._colors.background.css; - } else if (fg < 256) { + } else if (fg < DEFAULT_COLOR) { // 256 color support this._ctx.fillStyle = this._colors.ansi[fg].css; } else { diff --git a/src/renderer/atlas/DynamicCharAtlas.ts b/src/renderer/atlas/DynamicCharAtlas.ts index e67fe4f0..950fdd1d 100644 --- a/src/renderer/atlas/DynamicCharAtlas.ts +++ b/src/renderer/atlas/DynamicCharAtlas.ts @@ -42,7 +42,7 @@ interface IGlyphCacheValue { inBitmap: boolean; } -function getGlyphCacheKey(glyph: IGlyphIdentifier): number { +export function getGlyphCacheKey(glyph: IGlyphIdentifier): number { // Note that this only returns a valid key when code < 256 // Layout: // 0b00000000000000000000000000000001: italic (1) diff --git a/src/renderer/atlas/StaticCharAtlas.ts b/src/renderer/atlas/StaticCharAtlas.ts index 5c56f019..673a1509 100644 --- a/src/renderer/atlas/StaticCharAtlas.ts +++ b/src/renderer/atlas/StaticCharAtlas.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { DIM_OPACITY, IGlyphIdentifier } from './Types'; +import { DIM_OPACITY, IGlyphIdentifier, DEFAULT_COLOR } from './Types'; import { CHAR_ATLAS_CELL_SPACING, ICharAtlasConfig } from '../../shared/atlas/Types'; import { generateStaticCharAtlasTexture } from '../../shared/atlas/CharAtlasGenerator'; import BaseCharAtlas from './BaseCharAtlas'; @@ -41,8 +41,8 @@ export default class StaticCharAtlas extends BaseCharAtlas { const isAscii = glyph.code < 256; // A color is basic if it is one of the 4 bit ANSI colors. const isBasicColor = glyph.fg < 16; - const isDefaultColor = glyph.fg === 256; - const isDefaultBackground = glyph.bg >= 256; + const isDefaultColor = glyph.fg === DEFAULT_COLOR; + const isDefaultBackground = glyph.bg === DEFAULT_COLOR; return isAscii && (isBasicColor || isDefaultColor) && isDefaultBackground && !glyph.italic; } @@ -58,9 +58,9 @@ export default class StaticCharAtlas extends BaseCharAtlas { } let colorIndex = 0; - if (glyph.fg < 256) { + if (glyph.fg < DEFAULT_COLOR) { colorIndex = 2 + glyph.fg + (glyph.bold ? 16 : 0); - } else if (glyph.fg === 256) { + } else if (glyph.fg === DEFAULT_COLOR) { // If default color and bold if (glyph.bold) { colorIndex = 1; diff --git a/src/renderer/atlas/Types.ts b/src/renderer/atlas/Types.ts index 614a6e92..76cfd07d 100644 --- a/src/renderer/atlas/Types.ts +++ b/src/renderer/atlas/Types.ts @@ -3,7 +3,8 @@ * @license MIT */ -export const INVERTED_DEFAULT_COLOR = 258; +export const DEFAULT_COLOR = 256; +export const INVERTED_DEFAULT_COLOR = 257; export const DIM_OPACITY = 0.5; export interface IGlyphIdentifier { diff --git a/src/renderer/dom/DomRendererRowFactory.test.ts b/src/renderer/dom/DomRendererRowFactory.test.ts index 2c46d8cc..a19865cd 100644 --- a/src/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/renderer/dom/DomRendererRowFactory.test.ts @@ -10,6 +10,7 @@ import { DEFAULT_ATTR, NULL_CELL_CODE, NULL_CELL_WIDTH, NULL_CELL_CHAR } from '. import { FLAGS } from '../Types'; import { BufferLine } from '../../BufferLine'; import { IBufferLine } from '../../Types'; +import { DEFAULT_COLOR } from '../atlas/Types'; describe('DomRendererRowFactory', () => { let dom: jsdom.JSDOM; @@ -80,7 +81,7 @@ describe('DomRendererRowFactory', () => { }); it('should add classes for 256 foreground colors', () => { - const defaultAttrNoFgColor = (0 << 9) | (256 << 0); + const defaultAttrNoFgColor = (0 << 9) | (DEFAULT_COLOR << 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, undefined, 0, 5, 20); @@ -92,7 +93,7 @@ describe('DomRendererRowFactory', () => { }); it('should add classes for 256 background colors', () => { - const defaultAttrNoBgColor = (257 << 9) | (0 << 0); + const defaultAttrNoBgColor = (DEFAULT_ATTR << 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, undefined, 0, 5, 20); @@ -113,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)]); + lineData.set(0, [(FLAGS.INVERSE << 18) | (DEFAULT_ATTR << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + @@ -122,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)]); + lineData.set(0, [(FLAGS.INVERSE << 18) | (1 << 9) | (DEFAULT_COLOR << 0), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + @@ -132,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)]); + lineData.set(0, [(FLAGS.BOLD << 18) | (i << 9) | (DEFAULT_COLOR << 0), 'a', 1, 'a'.charCodeAt(0)]); 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 4bb59902..877f3fdd 100644 --- a/src/renderer/dom/DomRendererRowFactory.ts +++ b/src/renderer/dom/DomRendererRowFactory.ts @@ -6,6 +6,7 @@ import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_ATTR_INDEX, CHAR_DATA_WIDTH_INDEX } from '../../Buffer'; import { FLAGS } from '../Types'; import { IBufferLine } from '../../Types'; +import { DEFAULT_COLOR } from '../atlas/Types'; export const BOLD_CLASS = 'xterm-bold'; export const ITALIC_CLASS = 'xterm-italic'; @@ -70,10 +71,10 @@ export class DomRendererRowFactory { const temp = bg; bg = fg; fg = temp; - if (fg === 256) { + if (fg === DEFAULT_COLOR) { fg = 0; } - if (bg === 257) { + if (bg === DEFAULT_COLOR) { bg = 15; } } @@ -91,10 +92,10 @@ export class DomRendererRowFactory { } charElement.textContent = char; - if (fg !== 257) { + if (fg < DEFAULT_COLOR) { charElement.classList.add(`xterm-fg-${fg}`); } - if (bg !== 256) { + if (bg < DEFAULT_COLOR) { charElement.classList.add(`xterm-bg-${bg}`); } fragment.appendChild(charElement); From bc16a8b7e747107807d189327f87efd4eaebbee9 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 9 Oct 2018 11:35:48 -0700 Subject: [PATCH 03/18] Handle inverted default colors properly in DOM renderer This uses the actual fg for bg and bg to fg, rather than the black/white theme colors Fixes #1738 --- src/renderer/dom/DomRenderer.ts | 4 ++++ src/renderer/dom/DomRendererRowFactory.ts | 13 +++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts index 8f51c4ef..069e9a13 100644 --- a/src/renderer/dom/DomRenderer.ts +++ b/src/renderer/dom/DomRenderer.ts @@ -10,6 +10,7 @@ import { EventEmitter } from '../../common/EventEmitter'; import { ColorManager } from '../ColorManager'; import { RenderDebouncer } from '../../ui/RenderDebouncer'; import { BOLD_CLASS, ITALIC_CLASS, CURSOR_CLASS, CURSOR_STYLE_BLOCK_CLASS, CURSOR_STYLE_BAR_CLASS, CURSOR_STYLE_UNDERLINE_CLASS, DomRendererRowFactory } from './DomRendererRowFactory'; +import { INVERTED_DEFAULT_COLOR } from '../atlas/Types'; const TERMINAL_CLASS_PREFIX = 'xterm-dom-renderer-owner-'; const ROW_CONTAINER_CLASS = 'xterm-rows'; @@ -196,6 +197,9 @@ export class DomRenderer extends EventEmitter implements IRenderer { `${this._terminalSelector} .${FG_CLASS_PREFIX}${i} { color: ${c.css}; }` + `${this._terminalSelector} .${BG_CLASS_PREFIX}${i} { background-color: ${c.css}; }`; }); + styles += + `${this._terminalSelector} .${FG_CLASS_PREFIX}${INVERTED_DEFAULT_COLOR} { color: ${this.colorManager.colors.background.css}; }` + + `${this._terminalSelector} .${BG_CLASS_PREFIX}${INVERTED_DEFAULT_COLOR} { background-color: ${this.colorManager.colors.foreground.css}; }`; this._themeStyleElement.innerHTML = styles; return this.colorManager.colors; diff --git a/src/renderer/dom/DomRendererRowFactory.ts b/src/renderer/dom/DomRendererRowFactory.ts index 877f3fdd..9f8ac357 100644 --- a/src/renderer/dom/DomRendererRowFactory.ts +++ b/src/renderer/dom/DomRendererRowFactory.ts @@ -6,7 +6,7 @@ import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_ATTR_INDEX, CHAR_DATA_WIDTH_INDEX } from '../../Buffer'; import { FLAGS } from '../Types'; import { IBufferLine } from '../../Types'; -import { DEFAULT_COLOR } from '../atlas/Types'; +import { DEFAULT_COLOR, INVERTED_DEFAULT_COLOR } from '../atlas/Types'; export const BOLD_CLASS = 'xterm-bold'; export const ITALIC_CLASS = 'xterm-italic'; @@ -72,15 +72,16 @@ export class DomRendererRowFactory { bg = fg; fg = temp; if (fg === DEFAULT_COLOR) { - fg = 0; + fg = INVERTED_DEFAULT_COLOR; } if (bg === DEFAULT_COLOR) { - bg = 15; + bg = INVERTED_DEFAULT_COLOR; } } if (flags & FLAGS.BOLD) { - // Convert the FG color to the bold variant + // Convert the FG color to the bold variant. This should not happen when + // the fg is the inverse default color as there is no bold variant. if (fg < 8) { fg += 8; } @@ -92,10 +93,10 @@ export class DomRendererRowFactory { } charElement.textContent = char; - if (fg < DEFAULT_COLOR) { + if (fg !== DEFAULT_COLOR) { charElement.classList.add(`xterm-fg-${fg}`); } - if (bg < DEFAULT_COLOR) { + if (bg !== DEFAULT_COLOR) { charElement.classList.add(`xterm-bg-${bg}`); } fragment.appendChild(charElement); From 18035e239eb93a92b73911514a429cc1122ff47f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 9 Oct 2018 11:56:00 -0700 Subject: [PATCH 04/18] Fix tests --- src/renderer/dom/DomRendererRowFactory.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderer/dom/DomRendererRowFactory.test.ts b/src/renderer/dom/DomRendererRowFactory.test.ts index a19865cd..5bc849ed 100644 --- a/src/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/renderer/dom/DomRendererRowFactory.test.ts @@ -117,7 +117,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (DEFAULT_ATTR << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + + 'a' + ' ' ); }); @@ -126,7 +126,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (1 << 9) | (DEFAULT_COLOR << 0), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + + 'a' + ' ' ); }); From a5894ee2344337442aafafa34ed9328a8fbf45f1 Mon Sep 17 00:00:00 2001 From: Jeff Smith Date: Sat, 29 Sep 2018 09:39:49 -0500 Subject: [PATCH 05/18] Fix cursor movement when swapping buffers --- src/BufferSet.test.ts | 26 ++++++++++++++++++++++ src/BufferSet.ts | 4 ++++ src/InputHandler.test.ts | 48 +++++++++++++++++++++++++++++++++++++++- src/InputHandler.ts | 37 ++++++++++++++++++++++--------- 4 files changed, 103 insertions(+), 12 deletions(-) diff --git a/src/BufferSet.test.ts b/src/BufferSet.test.ts index 009ebf2e..38f2ddab 100644 --- a/src/BufferSet.test.ts +++ b/src/BufferSet.test.ts @@ -48,4 +48,30 @@ describe('BufferSet', () => { assert.equal(bufferSet.active, bufferSet.alt); }); }); + + describe('cursor handling when swapping buffers', () => { + beforeEach(() => { + bufferSet.normal.x = 0; + bufferSet.normal.y = 0; + bufferSet.alt.x = 0; + bufferSet.alt.y = 0; + }); + + it('should keep the cursor stationary when activating alt buffer', () => { + bufferSet.activateNormalBuffer(); + bufferSet.active.x = 30; + bufferSet.active.y = 10; + bufferSet.activateAltBuffer(); + assert.equal(bufferSet.active.x, 30); + assert.equal(bufferSet.active.y, 10); + }); + it('should keep the cursor stationary when activating normal buffer', () => { + bufferSet.activateAltBuffer(); + bufferSet.active.x = 30; + bufferSet.active.y = 10; + bufferSet.activateNormalBuffer(); + assert.equal(bufferSet.active.x, 30); + assert.equal(bufferSet.active.y, 10); + }); + }); }); diff --git a/src/BufferSet.ts b/src/BufferSet.ts index c91ab751..b213db0a 100644 --- a/src/BufferSet.ts +++ b/src/BufferSet.ts @@ -61,6 +61,8 @@ export class BufferSet extends EventEmitter implements IBufferSet { if (this._activeBuffer === this._normal) { return; } + this._normal.x = this._alt.x; + this._normal.y = this._alt.y; // The alt buffer should always be cleared when we switch to the normal // buffer. This frees up memory since the alt buffer should always be new // when activated. @@ -82,6 +84,8 @@ export class BufferSet extends EventEmitter implements IBufferSet { // Since the alt buffer is always cleared when the normal buffer is // activated, we want to fill it when switching to it. this._alt.fillViewportRows(); + this._alt.x = this._normal.x; + this._alt.y = this._normal.y; this._activeBuffer = this._alt; this.emit('activate', { activeBuffer: this._alt, diff --git a/src/InputHandler.test.ts b/src/InputHandler.test.ts index aaaf57c3..cd9d7d0b 100644 --- a/src/InputHandler.test.ts +++ b/src/InputHandler.test.ts @@ -6,7 +6,7 @@ import { assert, expect } from 'chai'; import { InputHandler } from './InputHandler'; import { MockInputHandlingTerminal } from './utils/TestUtils.test'; -import { NULL_CELL_CHAR, NULL_CELL_CODE, NULL_CELL_WIDTH, CHAR_DATA_CHAR_INDEX } from './Buffer'; +import { NULL_CELL_CHAR, NULL_CELL_CODE, NULL_CELL_WIDTH, CHAR_DATA_CHAR_INDEX, CHAR_DATA_ATTR_INDEX, DEFAULT_ATTR } from './Buffer'; import { Terminal } from './Terminal'; import { IBufferLine } from './Types'; @@ -506,4 +506,50 @@ describe('InputHandler', () => { inputHandler.print(String.fromCharCode(0x200B), 0, 1); }); }); + + describe('alt screen', () => { + let term: Terminal; + let handler: InputHandler; + + function lineContent(line: IBufferLine): string { + let content = ''; + for (let i = 0; i < line.length; ++i) content += line.get(i)[CHAR_DATA_CHAR_INDEX]; + return content; + } + + beforeEach(() => { + term = new Terminal(); + handler = new InputHandler(term); + }); + it('should handle DECSET/DECRST 47 (alt screen buffer)', () => { + handler.parse('\x1b[?47h\r\n\x1b[31mJUNK\x1b[?47lTEST'); + expect(lineContent(term.buffer.lines.get(0))).to.equal(Array(term.cols + 1).join(' ')); + expect(lineContent(term.buffer.lines.get(1))).to.equal(' TEST' + Array(term.cols - 7).join(' ')); + // Text color of 'TEST' should be red + expect((term.buffer.lines.get(1).get(4)[CHAR_DATA_ATTR_INDEX] >> 9) & 0x1ff).to.equal(1); + }); + it('should handle DECSET/DECRST 1047 (alt screen buffer)', () => { + handler.parse('\x1b[?1047h\r\n\x1b[31mJUNK\x1b[?1047lTEST'); + expect(lineContent(term.buffer.lines.get(0))).to.equal(Array(term.cols + 1).join(' ')); + expect(lineContent(term.buffer.lines.get(1))).to.equal(' TEST' + Array(term.cols - 7).join(' ')); + // Text color of 'TEST' should be red + expect((term.buffer.lines.get(1).get(4)[CHAR_DATA_ATTR_INDEX] >> 9) & 0x1ff).to.equal(1); + }); + it('should handle DECSET/DECRST 1048 (alt screen cursor)', () => { + handler.parse('\x1b[?1048h\r\n\x1b[31mJUNK\x1b[?1048lTEST'); + expect(lineContent(term.buffer.lines.get(0))).to.equal('TEST' + Array(term.cols - 3).join(' ')); + expect(lineContent(term.buffer.lines.get(1))).to.equal('JUNK' + Array(term.cols - 3).join(' ')); + // Text color of 'TEST' should be default + expect(term.buffer.lines.get(0).get(0)[CHAR_DATA_ATTR_INDEX]).to.equal(DEFAULT_ATTR); + // Text color of 'JUNK' should be red + expect((term.buffer.lines.get(1).get(0)[CHAR_DATA_ATTR_INDEX] >> 9) & 0x1ff).to.equal(1); + }); + it('should handle DECSET/DECRST 1049 (alt screen buffer+cursor)', () => { + handler.parse('\x1b[?1049h\r\n\x1b[31mJUNK\x1b[?1049lTEST'); + expect(lineContent(term.buffer.lines.get(0))).to.equal('TEST' + Array(term.cols - 3).join(' ')); + expect(lineContent(term.buffer.lines.get(1))).to.equal(Array(term.cols + 1).join(' ')); + // Text color of 'TEST' should be default + expect(term.buffer.lines.get(0).get(0)[CHAR_DATA_ATTR_INDEX]).to.equal(DEFAULT_ATTR); + }); + }); }); diff --git a/src/InputHandler.ts b/src/InputHandler.ts index a34590ef..3fa9b705 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -1280,7 +1280,9 @@ export class InputHandler extends Disposable implements IInputHandler { case 66: this._terminal.log('Serial port requested application keypad.'); this._terminal.applicationKeypad = true; - this._terminal.viewport.syncScrollArea(); + if (this._terminal.viewport) { + this._terminal.viewport.syncScrollArea(); + } break; case 9: // X10 Mouse // no release, no motion, no wheel, no modifiers. @@ -1329,14 +1331,19 @@ export class InputHandler extends Disposable implements IInputHandler { case 25: // show cursor this._terminal.cursorHidden = false; break; + case 1048: // alt screen cursor + this.saveCursor(params); + break; case 1049: // alt screen buffer cursor - // TODO: Not sure if we need to save/restore after switching the buffer - // this.saveCursor(params); + this.saveCursor(params); // FALL-THROUGH case 47: // alt screen buffer case 1047: // alt screen buffer this._terminal.buffers.activateAltBuffer(); - this._terminal.viewport.syncScrollArea(); + this._terminal.refresh(0, this._terminal.rows - 1); + if (this._terminal.viewport) { + this._terminal.viewport.syncScrollArea(); + } this._terminal.showCursor(); break; case 2004: // bracketed paste mode (https://cirw.in/blog/bracketed-paste) @@ -1469,7 +1476,9 @@ export class InputHandler extends Disposable implements IInputHandler { case 66: this._terminal.log('Switching back to normal keypad.'); this._terminal.applicationKeypad = false; - this._terminal.viewport.syncScrollArea(); + if (this._terminal.viewport) { + this._terminal.viewport.syncScrollArea(); + } break; case 9: // X10 Mouse case 1000: // vt200 mouse @@ -1497,18 +1506,22 @@ export class InputHandler extends Disposable implements IInputHandler { case 25: // hide cursor this._terminal.cursorHidden = true; break; + case 1048: // alt screen cursor + this.restoreCursor(params); + break; case 1049: // alt screen buffer cursor // FALL-THROUGH case 47: // normal screen buffer case 1047: // normal screen buffer - clearing it first // Ensure the selection manager has the correct buffer this._terminal.buffers.activateNormalBuffer(); - // TODO: Not sure if we need to save/restore after switching the buffer - // if (params[0] === 1049) { - // this.restoreCursor(params); - // } + if (params[0] === 1049) { + this.restoreCursor(params); + } this._terminal.refresh(0, this._terminal.rows - 1); - this._terminal.viewport.syncScrollArea(); + if (this._terminal.viewport) { + this._terminal.viewport.syncScrollArea(); + } this._terminal.showCursor(); break; case 2004: // bracketed paste mode (https://cirw.in/blog/bracketed-paste) @@ -1787,7 +1800,9 @@ export class InputHandler extends Disposable implements IInputHandler { this._terminal.originMode = false; this._terminal.wraparoundMode = true; // defaults: xterm - true, vt100 - false this._terminal.applicationKeypad = false; // ? - this._terminal.viewport.syncScrollArea(); + if (this._terminal.viewport) { + this._terminal.viewport.syncScrollArea(); + } this._terminal.applicationCursor = false; this._terminal.buffer.scrollTop = 0; this._terminal.buffer.scrollBottom = this._terminal.rows - 1; From a19d93851c191187e5f601ecb6944649800c2fb2 Mon Sep 17 00:00:00 2001 From: Jeff Smith Date: Sat, 29 Sep 2018 22:36:28 -0500 Subject: [PATCH 06/18] Each buffer needs its own saved cursor attributes --- src/Buffer.ts | 1 + src/InputHandler.test.ts | 10 ++++++++++ src/InputHandler.ts | 4 ++-- src/Terminal.ts | 1 - src/Types.ts | 2 +- src/utils/TestUtils.test.ts | 2 +- 6 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index e54f752b..5eaf880b 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -38,6 +38,7 @@ export class Buffer implements IBuffer { public tabs: any; public savedY: number; public savedX: number; + public savedCurAttr: number; public markers: Marker[] = []; private _bufferLineConstructor: IBufferLineConstructor; diff --git a/src/InputHandler.test.ts b/src/InputHandler.test.ts index cd9d7d0b..5fa3b47a 100644 --- a/src/InputHandler.test.ts +++ b/src/InputHandler.test.ts @@ -551,5 +551,15 @@ describe('InputHandler', () => { // Text color of 'TEST' should be default expect(term.buffer.lines.get(0).get(0)[CHAR_DATA_ATTR_INDEX]).to.equal(DEFAULT_ATTR); }); + it('should handle DECSET/DECRST 1049 - maintains saved cursor for alt buffer', () => { + handler.parse('\x1b[?1049h\r\n\x1b[31m\x1b[s\x1b[?1049lTEST'); + expect(lineContent(term.buffer.lines.get(0))).to.equal('TEST' + Array(term.cols - 3).join(' ')); + // Text color of 'TEST' should be default + expect(term.buffer.lines.get(0).get(0)[CHAR_DATA_ATTR_INDEX]).to.equal(DEFAULT_ATTR); + handler.parse('\x1b[?1049h\x1b[uTEST'); + expect(lineContent(term.buffer.lines.get(1))).to.equal('TEST' + Array(term.cols - 3).join(' ')); + // Text color of 'TEST' should be red + expect((term.buffer.lines.get(1).get(0)[CHAR_DATA_ATTR_INDEX] >> 9) & 0x1ff).to.equal(1); + }); }); }); diff --git a/src/InputHandler.ts b/src/InputHandler.ts index 3fa9b705..47e1875b 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -1869,7 +1869,7 @@ export class InputHandler extends Disposable implements IInputHandler { public saveCursor(params: number[]): void { this._terminal.buffer.savedX = this._terminal.buffer.x; this._terminal.buffer.savedY = this._terminal.buffer.y; - this._terminal.savedCurAttr = this._terminal.curAttr; + this._terminal.buffer.savedCurAttr = this._terminal.curAttr; } @@ -1881,7 +1881,7 @@ export class InputHandler extends Disposable implements IInputHandler { public restoreCursor(params: number[]): void { this._terminal.buffer.x = this._terminal.buffer.savedX || 0; this._terminal.buffer.y = this._terminal.buffer.savedY || 0; - this._terminal.curAttr = this._terminal.savedCurAttr || DEFAULT_ATTR; + this._terminal.curAttr = this._terminal.buffer.savedCurAttr || DEFAULT_ATTR; } diff --git a/src/Terminal.ts b/src/Terminal.ts index c9bc98ff..6675e444 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -170,7 +170,6 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II public savedCols: number; public curAttr: number; - public savedCurAttr: number; public params: (string | number)[]; public currentParam: string | number; diff --git a/src/Types.ts b/src/Types.ts index e8578426..6a6b8369 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -49,7 +49,6 @@ export interface IInputHandlingTerminal extends IEventEmitter { wraparoundMode: boolean; bracketedPasteMode: boolean; curAttr: number; - savedCurAttr: number; savedCols: number; x10Mouse: boolean; vt200Mouse: boolean; @@ -290,6 +289,7 @@ export interface IBuffer { hasScrollback: boolean; savedY: number; savedX: number; + savedCurAttr: number; isCursorInViewport: boolean; translateBufferLineToString(lineIndex: number, trimRight: boolean, startCol?: number, endCol?: number): string; getWrappedRangeForLine(y: number): { first: number, last: number }; diff --git a/src/utils/TestUtils.test.ts b/src/utils/TestUtils.test.ts index 353e615f..a5ef4b9f 100644 --- a/src/utils/TestUtils.test.ts +++ b/src/utils/TestUtils.test.ts @@ -182,7 +182,6 @@ export class MockInputHandlingTerminal implements IInputHandlingTerminal { wraparoundMode: boolean; bracketedPasteMode: boolean; curAttr: number; - savedCurAttr: number; savedCols: number; x10Mouse: boolean; vt200Mouse: boolean; @@ -304,6 +303,7 @@ export class MockBuffer implements IBuffer { scrollTop: number; savedY: number; savedX: number; + savedCurAttr: number; translateBufferLineToString(lineIndex: number, trimRight: boolean, startCol?: number, endCol?: number): string { return Buffer.prototype.translateBufferLineToString.apply(this, arguments); } From 2f900352432a3bbc989a1d31d7fdf01cd69d7d24 Mon Sep 17 00:00:00 2001 From: Jeff Smith Date: Sun, 30 Sep 2018 22:37:17 -0500 Subject: [PATCH 07/18] Clear alt buffer with erase attributes upon entering --- src/Buffer.ts | 7 +++++-- src/BufferSet.ts | 4 ++-- src/InputHandler.test.ts | 5 +++++ src/InputHandler.ts | 2 +- src/Types.ts | 2 +- 5 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/Buffer.ts b/src/Buffer.ts index 5eaf880b..6d218645 100644 --- a/src/Buffer.ts +++ b/src/Buffer.ts @@ -114,11 +114,14 @@ export class Buffer implements IBuffer { /** * Fills the buffer's viewport with blank lines. */ - public fillViewportRows(): void { + public fillViewportRows(fillAttr?: number): void { if (this.lines.length === 0) { + if (fillAttr === undefined) { + fillAttr = DEFAULT_ATTR; + } let i = this._terminal.rows; while (i--) { - this.lines.push(this.getBlankLine(DEFAULT_ATTR)); + this.lines.push(this.getBlankLine(fillAttr)); } } } diff --git a/src/BufferSet.ts b/src/BufferSet.ts index b213db0a..f84757d1 100644 --- a/src/BufferSet.ts +++ b/src/BufferSet.ts @@ -77,13 +77,13 @@ export class BufferSet extends EventEmitter implements IBufferSet { /** * Sets the alt Buffer of the BufferSet as its currently active Buffer */ - public activateAltBuffer(): void { + public activateAltBuffer(fillAttr?: number): void { if (this._activeBuffer === this._alt) { return; } // Since the alt buffer is always cleared when the normal buffer is // activated, we want to fill it when switching to it. - this._alt.fillViewportRows(); + this._alt.fillViewportRows(fillAttr); this._alt.x = this._normal.x; this._alt.y = this._normal.y; this._activeBuffer = this._alt; diff --git a/src/InputHandler.test.ts b/src/InputHandler.test.ts index 5fa3b47a..b2fea06a 100644 --- a/src/InputHandler.test.ts +++ b/src/InputHandler.test.ts @@ -561,5 +561,10 @@ describe('InputHandler', () => { // Text color of 'TEST' should be red expect((term.buffer.lines.get(1).get(0)[CHAR_DATA_ATTR_INDEX] >> 9) & 0x1ff).to.equal(1); }); + it('should handle DECSET/DECRST 1049 - clears alt buffer with erase attributes', () => { + handler.parse('\x1b[42m\x1b[?1049h'); + // Buffer should be filled with green background + expect(term.buffer.lines.get(20).get(10)[CHAR_DATA_ATTR_INDEX] & 0x1ff).to.equal(2); + }); }); }); diff --git a/src/InputHandler.ts b/src/InputHandler.ts index 47e1875b..ab75366a 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -1339,7 +1339,7 @@ export class InputHandler extends Disposable implements IInputHandler { // FALL-THROUGH case 47: // alt screen buffer case 1047: // alt screen buffer - this._terminal.buffers.activateAltBuffer(); + this._terminal.buffers.activateAltBuffer(this._terminal.eraseAttr()); this._terminal.refresh(0, this._terminal.rows - 1); if (this._terminal.viewport) { this._terminal.viewport.syncScrollArea(); diff --git a/src/Types.ts b/src/Types.ts index 6a6b8369..0f60e6f7 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -306,7 +306,7 @@ export interface IBufferSet extends IEventEmitter { active: IBuffer; activateNormalBuffer(): void; - activateAltBuffer(): void; + activateAltBuffer(fillAttr?: number): void; } export interface ISelectionManager { From 692cdfa390943c0133a8fbd1a27e6b1a9ba721a8 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 22 Nov 2018 11:32:08 -0800 Subject: [PATCH 08/18] DOM renderer: Don't output empty cells at end Fixes #1609 --- src/renderer/dom/DomRendererRowFactory.ts | 27 ++++++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/renderer/dom/DomRendererRowFactory.ts b/src/renderer/dom/DomRendererRowFactory.ts index 4bb59902..bcb0cc48 100644 --- a/src/renderer/dom/DomRendererRowFactory.ts +++ b/src/renderer/dom/DomRendererRowFactory.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_ATTR_INDEX, CHAR_DATA_WIDTH_INDEX } from '../../Buffer'; +import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_ATTR_INDEX, CHAR_DATA_WIDTH_INDEX, CHAR_DATA_CODE_INDEX, NULL_CELL_CODE } from '../../Buffer'; import { FLAGS } from '../Types'; import { IBufferLine } from '../../Types'; @@ -23,17 +23,28 @@ export class DomRendererRowFactory { public createRow(lineData: IBufferLine, isCursorRow: boolean, cursorStyle: string | undefined, cursorX: number, cellWidth: number, cols: number): DocumentFragment { const fragment = this._document.createDocumentFragment(); let colCount = 0; + let nonNullCellFound = false; - for (let x = 0; x < lineData.length; x++) { + for (let x = lineData.length - 1; x >= 0; x--) { // Don't allow any buffer to the right to be displayed if (colCount >= cols) { continue; } const charData = lineData.get(x); - const char: string = charData[CHAR_DATA_CHAR_INDEX]; - const attr: number = charData[CHAR_DATA_ATTR_INDEX]; - const width: number = charData[CHAR_DATA_WIDTH_INDEX]; + + if (!nonNullCellFound) { + const code = charData[CHAR_DATA_CODE_INDEX]; + if (code === NULL_CELL_CODE && !(isCursorRow && x === cursorX)) { + continue; + } else { + nonNullCellFound = true; + } + } + + const char = charData[CHAR_DATA_CHAR_INDEX]; + const attr = charData[CHAR_DATA_ATTR_INDEX]; + const width = charData[CHAR_DATA_WIDTH_INDEX]; // The character to the left is a wide character, drawing is owned by the char at x-1 if (width === 0) { @@ -97,7 +108,11 @@ export class DomRendererRowFactory { if (bg !== 256) { charElement.classList.add(`xterm-bg-${bg}`); } - fragment.appendChild(charElement); + if (fragment.childNodes.length === 0) { + fragment.appendChild(charElement); + } else { + fragment.insertBefore(charElement, fragment.firstChild); + } colCount += width; } return fragment; From dc077a181e3b4f34c49f22f9579b1fd9dca653ff Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 22 Nov 2018 12:03:08 -0800 Subject: [PATCH 09/18] Fix tests and the colCount feature --- .../dom/DomRendererRowFactory.test.ts | 32 ++++++----------- src/renderer/dom/DomRendererRowFactory.ts | 35 +++++++++---------- 2 files changed, 28 insertions(+), 39 deletions(-) diff --git a/src/renderer/dom/DomRendererRowFactory.test.ts b/src/renderer/dom/DomRendererRowFactory.test.ts index 2c46d8cc..03f41784 100644 --- a/src/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/renderer/dom/DomRendererRowFactory.test.ts @@ -23,11 +23,10 @@ describe('DomRendererRowFactory', () => { }); describe('createRow', () => { - it('should create an element for every character in the row', () => { + it('should not create anything for an empty row', () => { const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - ' ' + - ' ' + '' ); }); @@ -45,8 +44,7 @@ describe('DomRendererRowFactory', () => { for (const style of ['block', 'bar', 'underline']) { const fragment = rowFactory.createRow(lineData, true, style, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - ` ` + - ' ' + ` ` ); } }); @@ -65,8 +63,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [DEFAULT_ATTR | (FLAGS.BOLD << 18), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + - ' ' + 'a' ); }); @@ -74,8 +71,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [DEFAULT_ATTR | (FLAGS.ITALIC << 18), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + - ' ' + 'a' ); }); @@ -85,8 +81,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [defaultAttrNoFgColor | (i << 9), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - `a` + - ' ' + `a` ); } }); @@ -97,8 +92,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [defaultAttrNoBgColor | (i << 0), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - `a` + - ' ' + `a` ); } }); @@ -107,8 +101,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (2 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + - ' ' + 'a' ); }); @@ -116,8 +109,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (257 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + - ' ' + 'a' ); }); @@ -125,8 +117,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [(FLAGS.INVERSE << 18) | (1 << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - 'a' + - ' ' + 'a' ); }); @@ -135,8 +126,7 @@ describe('DomRendererRowFactory', () => { lineData.set(0, [(FLAGS.BOLD << 18) | (i << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]); const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20); assert.equal(getFragmentHtml(fragment), - `a` + - ' ' + `a` ); } }); diff --git a/src/renderer/dom/DomRendererRowFactory.ts b/src/renderer/dom/DomRendererRowFactory.ts index bcb0cc48..5f6b49fc 100644 --- a/src/renderer/dom/DomRendererRowFactory.ts +++ b/src/renderer/dom/DomRendererRowFactory.ts @@ -22,26 +22,29 @@ export class DomRendererRowFactory { public createRow(lineData: IBufferLine, isCursorRow: boolean, cursorStyle: string | undefined, cursorX: number, cellWidth: number, cols: number): DocumentFragment { const fragment = this._document.createDocumentFragment(); - let colCount = 0; - let nonNullCellFound = false; - for (let x = lineData.length - 1; x >= 0; x--) { + // Find the line length first, this prevents the need to output a bunch of + // empty cells at the end. This cannot easily be integrated into the main + // loop below because of the colCount feature (which can be removed after we + // properly support reflow and disallow data to go beyond the right-side of + // the viewport). + let lineLength = 0; + for (let x = 0; x < lineData.length; x++) { + const charData = lineData.get(x); + const code = charData[CHAR_DATA_CODE_INDEX]; + if (code !== NULL_CELL_CODE || (isCursorRow && x === cursorX)) { + lineLength = x + 1; + } + } + + let colCount = 0; + for (let x = 0; x < lineLength; x++) { // Don't allow any buffer to the right to be displayed if (colCount >= cols) { continue; } const charData = lineData.get(x); - - if (!nonNullCellFound) { - const code = charData[CHAR_DATA_CODE_INDEX]; - if (code === NULL_CELL_CODE && !(isCursorRow && x === cursorX)) { - continue; - } else { - nonNullCellFound = true; - } - } - const char = charData[CHAR_DATA_CHAR_INDEX]; const attr = charData[CHAR_DATA_ATTR_INDEX]; const width = charData[CHAR_DATA_WIDTH_INDEX]; @@ -108,11 +111,7 @@ export class DomRendererRowFactory { if (bg !== 256) { charElement.classList.add(`xterm-bg-${bg}`); } - if (fragment.childNodes.length === 0) { - fragment.appendChild(charElement); - } else { - fragment.insertBefore(charElement, fragment.firstChild); - } + fragment.appendChild(charElement); colCount += width; } return fragment; From 66272eb3105ff23b84e5a79b622a72e49352419c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 23 Nov 2018 05:39:50 -0800 Subject: [PATCH 10/18] Get the line length by going backwards instead --- src/renderer/dom/DomRendererRowFactory.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/renderer/dom/DomRendererRowFactory.ts b/src/renderer/dom/DomRendererRowFactory.ts index 5f6b49fc..490360bf 100644 --- a/src/renderer/dom/DomRendererRowFactory.ts +++ b/src/renderer/dom/DomRendererRowFactory.ts @@ -29,11 +29,12 @@ export class DomRendererRowFactory { // properly support reflow and disallow data to go beyond the right-side of // the viewport). let lineLength = 0; - for (let x = 0; x < lineData.length; x++) { + for (let x = lineData.length - 1; x >= 0; x--) { const charData = lineData.get(x); const code = charData[CHAR_DATA_CODE_INDEX]; if (code !== NULL_CELL_CODE || (isCursorRow && x === cursorX)) { lineLength = x + 1; + break; } } From d9513ce50df9ea1faa8e4ef7da076824df3be171 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 23 Nov 2018 06:48:37 -0800 Subject: [PATCH 11/18] Don't bother checking beyond cols --- src/renderer/dom/DomRendererRowFactory.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/renderer/dom/DomRendererRowFactory.ts b/src/renderer/dom/DomRendererRowFactory.ts index 490360bf..07303e24 100644 --- a/src/renderer/dom/DomRendererRowFactory.ts +++ b/src/renderer/dom/DomRendererRowFactory.ts @@ -29,7 +29,7 @@ export class DomRendererRowFactory { // properly support reflow and disallow data to go beyond the right-side of // the viewport). let lineLength = 0; - for (let x = lineData.length - 1; x >= 0; x--) { + for (let x = Math.min(lineData.length, cols) - 1; x >= 0; x--) { const charData = lineData.get(x); const code = charData[CHAR_DATA_CODE_INDEX]; if (code !== NULL_CELL_CODE || (isCursorRow && x === cursorX)) { @@ -38,13 +38,7 @@ export class DomRendererRowFactory { } } - let colCount = 0; for (let x = 0; x < lineLength; x++) { - // Don't allow any buffer to the right to be displayed - if (colCount >= cols) { - continue; - } - const charData = lineData.get(x); const char = charData[CHAR_DATA_CHAR_INDEX]; const attr = charData[CHAR_DATA_ATTR_INDEX]; @@ -113,7 +107,6 @@ export class DomRendererRowFactory { charElement.classList.add(`xterm-bg-${bg}`); } fragment.appendChild(charElement); - colCount += width; } return fragment; } From 478dfee6e2988fd81a61b4e85b14125b3e087e50 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 23 Nov 2018 06:51:39 -0800 Subject: [PATCH 12/18] Ensure wide chars don't overflow onto following row --- src/renderer/dom/DomRenderer.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts index fadd9b72..73c63b46 100644 --- a/src/renderer/dom/DomRenderer.ts +++ b/src/renderer/dom/DomRenderer.ts @@ -111,6 +111,8 @@ export class DomRenderer extends EventEmitter implements IRenderer { element.style.width = `${this.dimensions.canvasWidth}px`; element.style.height = `${this.dimensions.actualCellHeight}px`; element.style.lineHeight = `${this.dimensions.actualCellHeight}px`; + // Make sure rows don't overflow onto following row + element.style.overflow = 'hidden'; }); if (!this._dimensionsStyleElement) { @@ -330,7 +332,7 @@ export class DomRenderer extends EventEmitter implements IRenderer { const row = y + terminal.buffer.ydisp; const lineData = terminal.buffer.lines.get(row); const cursorStyle = terminal.options.cursorStyle; - rowElement.appendChild(this._rowFactory.createRow(lineData, row === cursorAbsoluteY, cursorStyle, cursorX, terminal.charMeasure.width, terminal.cols)); + rowElement.appendChild(this._rowFactory.createRow(lineData, row === cursorAbsoluteY, cursorStyle, cursorX, this.dimensions.actualCellWidth, terminal.cols)); } this._terminal.emit('refresh', {start, end}); From 266ea38d91c74389ba68a1630fa0d392722592ab Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 23 Nov 2018 07:35:55 -0800 Subject: [PATCH 13/18] Improve types of fill polyfill and test fallback The polyfill will now return the T instead of the TypedArray union type. Tests now target the fallback only so engines that support fill will run tests against the fallback code. --- src/core/TypedArrayUtils.test.ts | 54 ++++++++++++++++---------------- src/core/TypedArrayUtils.ts | 8 +++-- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/src/core/TypedArrayUtils.test.ts b/src/core/TypedArrayUtils.test.ts index ef86d314..1df06f4c 100644 --- a/src/core/TypedArrayUtils.test.ts +++ b/src/core/TypedArrayUtils.test.ts @@ -3,7 +3,7 @@ * @license MIT */ import { assert } from 'chai'; -import { fill } from './TypedArrayUtils'; +import { fillFallback } from './TypedArrayUtils'; type TypedArray = Uint8Array | Uint16Array | Uint32Array | Uint8ClampedArray | Int8Array | Int16Array | Int32Array @@ -42,42 +42,42 @@ describe('polyfill conformance tests', function(): void { it('should work with all typed array types', function(): void { const u81 = new Uint8Array(5); const u82 = new Uint8Array(5); - deepEquals(fill(u81, 2), u82.fill(2)); - deepEquals(fill(u81, -1), u82.fill(-1)); + deepEquals(fillFallback(u81, 2), u82.fill(2)); + deepEquals(fillFallback(u81, -1), u82.fill(-1)); const u161 = new Uint16Array(5); const u162 = new Uint16Array(5); - deepEquals(fill(u161, 2), u162.fill(2)); - deepEquals(fill(u161, 65535), u162.fill(65535)); - deepEquals(fill(u161, -1), u162.fill(-1)); + deepEquals(fillFallback(u161, 2), u162.fill(2)); + deepEquals(fillFallback(u161, 65535), u162.fill(65535)); + deepEquals(fillFallback(u161, -1), u162.fill(-1)); const u321 = new Uint32Array(5); const u322 = new Uint32Array(5); - deepEquals(fill(u321, 2), u322.fill(2)); - deepEquals(fill(u321, 65537), u322.fill(65537)); - deepEquals(fill(u321, -1), u322.fill(-1)); + deepEquals(fillFallback(u321, 2), u322.fill(2)); + deepEquals(fillFallback(u321, 65537), u322.fill(65537)); + deepEquals(fillFallback(u321, -1), u322.fill(-1)); const i81 = new Int8Array(5); const i82 = new Int8Array(5); - deepEquals(fill(i81, 2), i82.fill(2)); - deepEquals(fill(i81, -1), i82.fill(-1)); + deepEquals(fillFallback(i81, 2), i82.fill(2)); + deepEquals(fillFallback(i81, -1), i82.fill(-1)); const i161 = new Int16Array(5); const i162 = new Int16Array(5); - deepEquals(fill(i161, 2), i162.fill(2)); - deepEquals(fill(i161, 65535), i162.fill(65535)); - deepEquals(fill(i161, -1), i162.fill(-1)); + deepEquals(fillFallback(i161, 2), i162.fill(2)); + deepEquals(fillFallback(i161, 65535), i162.fill(65535)); + deepEquals(fillFallback(i161, -1), i162.fill(-1)); const i321 = new Int32Array(5); const i322 = new Int32Array(5); - deepEquals(fill(i321, 2), i322.fill(2)); - deepEquals(fill(i321, 65537), i322.fill(65537)); - deepEquals(fill(i321, -1), i322.fill(-1)); + deepEquals(fillFallback(i321, 2), i322.fill(2)); + deepEquals(fillFallback(i321, 65537), i322.fill(65537)); + deepEquals(fillFallback(i321, -1), i322.fill(-1)); const f321 = new Float32Array(5); const f322 = new Float32Array(5); - deepEquals(fill(f321, 1.2345), f322.fill(1.2345)); + deepEquals(fillFallback(f321, 1.2345), f322.fill(1.2345)); const f641 = new Float64Array(5); const f642 = new Float64Array(5); - deepEquals(fill(f641, 1.2345), f642.fill(1.2345)); + deepEquals(fillFallback(f641, 1.2345), f642.fill(1.2345)); const u8Clamped1 = new Uint8ClampedArray(5); const u8Clamped2 = new Uint8ClampedArray(5); - deepEquals(fill(u8Clamped1, 2), u8Clamped2.fill(2)); - deepEquals(fill(u8Clamped1, 257), u8Clamped2.fill(257)); + deepEquals(fillFallback(u8Clamped1, 2), u8Clamped2.fill(2)); + deepEquals(fillFallback(u8Clamped1, 257), u8Clamped2.fill(257)); }); it('should work with all typed array types - explicit looping', function(): void { const u81 = new Uint8Array(5); @@ -124,8 +124,8 @@ describe('polyfill conformance tests', function(): void { const u81 = new Uint8Array(5); const u82 = new Uint8Array(5); const u83 = new Uint8Array(5); - deepEquals(fill(u81, 2, i), u83.fill(2, i)); - deepEquals(fill(u81, -1, i), u83.fill(-1, i)); + deepEquals(fillFallback(u81, 2, i), u83.fill(2, i)); + deepEquals(fillFallback(u81, -1, i), u83.fill(-1, i)); deepEquals(loopFill(u82, 2, i), u83.fill(2, i)); deepEquals(loopFill(u82, -1, i), u83.fill(-1, i)); } @@ -135,8 +135,8 @@ describe('polyfill conformance tests', function(): void { const u81 = new Uint8Array(5); const u82 = new Uint8Array(5); const u83 = new Uint8Array(5); - deepEquals(fill(u81, 2, 0, i), u83.fill(2, 0, i)); - deepEquals(fill(u81, -1, 0, i), u83.fill(-1, 0, i)); + deepEquals(fillFallback(u81, 2, 0, i), u83.fill(2, 0, i)); + deepEquals(fillFallback(u81, -1, 0, i), u83.fill(-1, 0, i)); deepEquals(loopFill(u82, 2, 0, i), u83.fill(2, 0, i)); deepEquals(loopFill(u82, -1, 0, i), u83.fill(-1, 0, i)); } @@ -147,8 +147,8 @@ describe('polyfill conformance tests', function(): void { const u81 = new Uint8Array(5); const u82 = new Uint8Array(5); const u83 = new Uint8Array(5); - deepEquals(fill(u81, 2, i, j), u83.fill(2, i, j)); - deepEquals(fill(u81, -1, i, j), u83.fill(-1, i, j)); + deepEquals(fillFallback(u81, 2, i, j), u83.fill(2, i, j)); + deepEquals(fillFallback(u81, -1, i, j), u83.fill(-1, i, j)); deepEquals(loopFill(u82, 2, i, j), u83.fill(2, i, j)); deepEquals(loopFill(u82, -1, i, j), u83.fill(-1, i, j)); } diff --git a/src/core/TypedArrayUtils.ts b/src/core/TypedArrayUtils.ts index 56e9d7b0..2e85400c 100644 --- a/src/core/TypedArrayUtils.ts +++ b/src/core/TypedArrayUtils.ts @@ -12,11 +12,15 @@ type TypedArray = Uint8Array | Uint16Array | Uint32Array | Uint8ClampedArray | Int8Array | Int16Array | Int32Array | Float32Array | Float64Array; -export function fill(array: TypedArray, value: number, start: number = 0, end?: number | undefined): TypedArray { +export function fill(array: T, value: number, start: number = 0, end?: number | undefined): T { // all modern engines that support .fill if (array.fill) { - return array.fill(value, start, end); + return array.fill(value, start, end) as T; } + return fillFallback(array, value, start, end); +} + +export function fillFallback(array: T, value: number, start: number = 0, end?: number | undefined): T { // safari and IE 11 // since IE 11 does not support Array.prototype.fill either // we cannot use the suggested polyfill from MDN From 4f18717940e5be633228c1dabd81d0dd78f19a82 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 23 Nov 2018 07:49:35 -0800 Subject: [PATCH 14/18] Improve handling of default values --- src/core/TypedArrayUtils.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/core/TypedArrayUtils.ts b/src/core/TypedArrayUtils.ts index 2e85400c..6e1a3630 100644 --- a/src/core/TypedArrayUtils.ts +++ b/src/core/TypedArrayUtils.ts @@ -12,7 +12,7 @@ type TypedArray = Uint8Array | Uint16Array | Uint32Array | Uint8ClampedArray | Int8Array | Int16Array | Int32Array | Float32Array | Float64Array; -export function fill(array: T, value: number, start: number = 0, end?: number | undefined): T { +export function fill(array: T, value: number, start?: number, end?: number): T { // all modern engines that support .fill if (array.fill) { return array.fill(value, start, end) as T; @@ -20,7 +20,7 @@ export function fill(array: T, value: number, start: numbe return fillFallback(array, value, start, end); } -export function fillFallback(array: T, value: number, start: number = 0, end?: number | undefined): T { +export function fillFallback(array: T, value: number, start: number = 0, end: number = array.length): T { // safari and IE 11 // since IE 11 does not support Array.prototype.fill either // we cannot use the suggested polyfill from MDN @@ -29,9 +29,6 @@ export function fillFallback(array: T, value: number, star return array; } start = (array.length + start) % array.length; - if (end === undefined) { - end = array.length; - } if (end >= array.length) { end = array.length; } else { From 1b5ef5d93a047b346b20beafd68545ea6295d20e Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 23 Nov 2018 08:08:30 -0800 Subject: [PATCH 15/18] Move TypedArrayUtils to common It belongs here since the renderers will use it --- src/CharWidth.ts | 2 +- src/{core => common}/TypedArrayUtils.test.ts | 0 src/{core => common}/TypedArrayUtils.ts | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename src/{core => common}/TypedArrayUtils.test.ts (100%) rename src/{core => common}/TypedArrayUtils.ts (100%) diff --git a/src/CharWidth.ts b/src/CharWidth.ts index fd6ac55f..43cd948e 100644 --- a/src/CharWidth.ts +++ b/src/CharWidth.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { fill } from './core/TypedArrayUtils'; +import { fill } from './common/TypedArrayUtils'; export const wcwidth = (function(opts: {nul: number, control: number}): (ucs: number) => number { // extracted from https://www.cl.cam.ac.uk/%7Emgk25/ucs/wcwidth.c diff --git a/src/core/TypedArrayUtils.test.ts b/src/common/TypedArrayUtils.test.ts similarity index 100% rename from src/core/TypedArrayUtils.test.ts rename to src/common/TypedArrayUtils.test.ts diff --git a/src/core/TypedArrayUtils.ts b/src/common/TypedArrayUtils.ts similarity index 100% rename from src/core/TypedArrayUtils.ts rename to src/common/TypedArrayUtils.ts From fc6f101e160f071f8d468a3c4a3fc4443256eb37 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 23 Nov 2018 08:59:30 -0800 Subject: [PATCH 16/18] Remove unused lineHeight parameters --- src/SelectionManager.ts | 2 +- src/Terminal.ts | 4 ++-- src/Types.ts | 4 ++-- src/handlers/AltClickHandler.ts | 1 - src/ui/MouseZoneManager.ts | 2 +- src/utils/MouseHelper.test.ts | 16 ++++++++-------- src/utils/MouseHelper.ts | 6 +++--- 7 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index 86be0c48..49e21e2c 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -347,7 +347,7 @@ export class SelectionManager extends EventEmitter implements ISelectionManager * @param event The mouse event. */ private _getMouseBufferCoords(event: MouseEvent): [number, number] { - const coords = this._terminal.mouseHelper.getCoords(event, this._terminal.screenElement, this._charMeasure, this._terminal.options.lineHeight, this._terminal.cols, this._terminal.rows, true); + const coords = this._terminal.mouseHelper.getCoords(event, this._terminal.screenElement, this._charMeasure, this._terminal.cols, this._terminal.rows, true); if (!coords) { return null; } diff --git a/src/Terminal.ts b/src/Terminal.ts index 38eb3d5b..4b60cfab 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -808,7 +808,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II button = getButton(ev); // get mouse coordinates - pos = self.mouseHelper.getRawByteCoords(ev, self.screenElement, self.charMeasure, self.options.lineHeight, self.cols, self.rows); + pos = self.mouseHelper.getRawByteCoords(ev, self.screenElement, self.charMeasure, self.cols, self.rows); if (!pos) return; sendEvent(button, pos); @@ -834,7 +834,7 @@ export class Terminal extends EventEmitter implements ITerminal, IDisposable, II // ^[[M 3<^[[M@4<^[[M@5<^[[M@6<^[[M@7<^[[M#7< function sendMove(ev: MouseEvent): void { let button = pressed; - const pos = self.mouseHelper.getRawByteCoords(ev, self.screenElement, self.charMeasure, self.options.lineHeight, self.cols, self.rows); + const pos = self.mouseHelper.getRawByteCoords(ev, self.screenElement, self.charMeasure, self.cols, self.rows); if (!pos) return; // buttons marked as motions diff --git a/src/Types.ts b/src/Types.ts index e8578426..6d6187f1 100644 --- a/src/Types.ts +++ b/src/Types.ts @@ -246,8 +246,8 @@ export interface ILinkifierAccessor { } export interface IMouseHelper { - getCoords(event: { pageX: number, pageY: number }, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number, isSelection?: boolean): [number, number]; - getRawByteCoords(event: MouseEvent, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number): { x: number, y: number }; + getCoords(event: { pageX: number, pageY: number }, element: HTMLElement, charMeasure: ICharMeasure, colCount: number, rowCount: number, isSelection?: boolean): [number, number]; + getRawByteCoords(event: MouseEvent, element: HTMLElement, charMeasure: ICharMeasure, colCount: number, rowCount: number): { x: number, y: number }; } export interface ICharMeasure { diff --git a/src/handlers/AltClickHandler.ts b/src/handlers/AltClickHandler.ts index 8226fd96..9286421e 100644 --- a/src/handlers/AltClickHandler.ts +++ b/src/handlers/AltClickHandler.ts @@ -33,7 +33,6 @@ export class AltClickHandler { this._mouseEvent, this._terminal.element, this._terminal.charMeasure, - this._terminal.options.lineHeight, this._terminal.cols, this._terminal.rows, false diff --git a/src/ui/MouseZoneManager.ts b/src/ui/MouseZoneManager.ts index 491e2a05..a232f5b9 100644 --- a/src/ui/MouseZoneManager.ts +++ b/src/ui/MouseZoneManager.ts @@ -180,7 +180,7 @@ export class MouseZoneManager extends Disposable implements IMouseZoneManager { } private _findZoneEventAt(e: MouseEvent): IMouseZone { - const coords = this._terminal.mouseHelper.getCoords(e, this._terminal.screenElement, this._terminal.charMeasure, this._terminal.options.lineHeight, this._terminal.cols, this._terminal.rows); + const coords = this._terminal.mouseHelper.getCoords(e, this._terminal.screenElement, this._terminal.charMeasure, this._terminal.cols, this._terminal.rows); if (!coords) { return null; } diff --git a/src/utils/MouseHelper.test.ts b/src/utils/MouseHelper.test.ts index ac3137f4..94d63b2b 100644 --- a/src/utils/MouseHelper.test.ts +++ b/src/utils/MouseHelper.test.ts @@ -37,34 +37,34 @@ describe('MouseHelper.getCoords', () => { describe('when charMeasure is not initialized', () => { it('should return null', () => { charMeasure = new MockCharMeasure(); - assert.equal(mouseHelper.getCoords({ pageX: 0, pageY: 0 }, document.createElement('div'), charMeasure, 1, 10, 10), null); + assert.equal(mouseHelper.getCoords({ pageX: 0, pageY: 0 }, document.createElement('div'), charMeasure, 10, 10), null); }); }); describe('when pageX/pageY are not supported', () => { it('should return null', () => { - assert.equal(mouseHelper.getCoords({ pageX: undefined, pageY: undefined }, document.createElement('div'), charMeasure, 1, 10, 10), null); + assert.equal(mouseHelper.getCoords({ pageX: undefined, pageY: undefined }, document.createElement('div'), charMeasure, 10, 10), null); }); }); it('should return the cell that was clicked', () => { let coords: [number, number]; - coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH / 2, pageY: CHAR_HEIGHT / 2 }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH / 2, pageY: CHAR_HEIGHT / 2 }, document.createElement('div'), charMeasure, 10, 10); assert.deepEqual(coords, [1, 1]); - coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH, pageY: CHAR_HEIGHT }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH, pageY: CHAR_HEIGHT }, document.createElement('div'), charMeasure, 10, 10); assert.deepEqual(coords, [1, 1]); - coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH, pageY: CHAR_HEIGHT + 1 }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH, pageY: CHAR_HEIGHT + 1 }, document.createElement('div'), charMeasure, 10, 10); assert.deepEqual(coords, [1, 2]); - coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH + 1, pageY: CHAR_HEIGHT }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH + 1, pageY: CHAR_HEIGHT }, document.createElement('div'), charMeasure, 10, 10); assert.deepEqual(coords, [2, 1]); }); it('should ensure the coordinates are returned within the terminal bounds', () => { let coords: [number, number]; - coords = mouseHelper.getCoords({ pageX: -1, pageY: -1 }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ pageX: -1, pageY: -1 }, document.createElement('div'), charMeasure, 10, 10); assert.deepEqual(coords, [1, 1]); // Event are double the cols/rows - coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH * 20, pageY: CHAR_HEIGHT * 20 }, document.createElement('div'), charMeasure, 1, 10, 10); + coords = mouseHelper.getCoords({ pageX: CHAR_WIDTH * 20, pageY: CHAR_HEIGHT * 20 }, document.createElement('div'), charMeasure, 10, 10); assert.deepEqual(coords, [10, 10], 'coordinates should never come back as larger than the terminal'); }); }); diff --git a/src/utils/MouseHelper.ts b/src/utils/MouseHelper.ts index ca1bb27e..e4b3f211 100644 --- a/src/utils/MouseHelper.ts +++ b/src/utils/MouseHelper.ts @@ -52,7 +52,7 @@ export class MouseHelper { * apply an offset to the x value such that the left half of the cell will * select that cell and the right half will select the next cell. */ - public getCoords(event: {pageX: number, pageY: number}, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number, isSelection?: boolean): [number, number] { + public getCoords(event: {pageX: number, pageY: number}, element: HTMLElement, charMeasure: ICharMeasure, colCount: number, rowCount: number, isSelection?: boolean): [number, number] { // Coordinates cannot be measured if charMeasure has not been initialized if (!charMeasure.width || !charMeasure.height) { return null; @@ -85,8 +85,8 @@ export class MouseHelper { * @param colCount The number of columns in the terminal. * @param rowCount The number of rows in the terminal. */ - public getRawByteCoords(event: MouseEvent, element: HTMLElement, charMeasure: ICharMeasure, lineHeight: number, colCount: number, rowCount: number): { x: number, y: number } { - const coords = this.getCoords(event, element, charMeasure, lineHeight, colCount, rowCount); + public getRawByteCoords(event: MouseEvent, element: HTMLElement, charMeasure: ICharMeasure, colCount: number, rowCount: number): { x: number, y: number } { + const coords = this.getCoords(event, element, charMeasure, colCount, rowCount); let x = coords[0]; let y = coords[1]; From 41ba20f7499356f8ad45cb9526ed6e84c93f6267 Mon Sep 17 00:00:00 2001 From: jerch Date: Fri, 23 Nov 2018 18:40:52 +0100 Subject: [PATCH 17/18] remove not longer needed test cases Removed the `loopFill` test method as it is not needed anymore with the separation of the looping variant in `fillFallback`. --- src/common/TypedArrayUtils.test.ts | 69 ------------------------------ 1 file changed, 69 deletions(-) diff --git a/src/common/TypedArrayUtils.test.ts b/src/common/TypedArrayUtils.test.ts index 1df06f4c..69a62abc 100644 --- a/src/common/TypedArrayUtils.test.ts +++ b/src/common/TypedArrayUtils.test.ts @@ -9,26 +9,6 @@ type TypedArray = Uint8Array | Uint16Array | Uint32Array | Uint8ClampedArray | Int8Array | Int16Array | Int32Array | Float32Array | Float64Array; -// we explicitly test against the looping version in the test cases -function loopFill(array: TypedArray, value: number, start: number = 0, end?: number | undefined): TypedArray { - if (start >= array.length) { - return array; - } - start = (array.length + start) % array.length; - if (end === undefined) { - end = array.length; - } - if (end >= array.length) { - end = array.length; - } else { - end = (array.length + end) % array.length; - } - for (let i = start; i < end; ++i) { - array[i] = value; - } - return array; -} - describe('polyfill conformance tests', function(): void { function deepEquals(a: TypedArray, b: TypedArray): void { @@ -79,78 +59,29 @@ describe('polyfill conformance tests', function(): void { deepEquals(fillFallback(u8Clamped1, 2), u8Clamped2.fill(2)); deepEquals(fillFallback(u8Clamped1, 257), u8Clamped2.fill(257)); }); - it('should work with all typed array types - explicit looping', function(): void { - const u81 = new Uint8Array(5); - const u82 = new Uint8Array(5); - deepEquals(loopFill(u81, 2), u82.fill(2)); - deepEquals(loopFill(u81, -1), u82.fill(-1)); - const u161 = new Uint16Array(5); - const u162 = new Uint16Array(5); - deepEquals(loopFill(u161, 2), u162.fill(2)); - deepEquals(loopFill(u161, 65535), u162.fill(65535)); - deepEquals(loopFill(u161, -1), u162.fill(-1)); - const u321 = new Uint32Array(5); - const u322 = new Uint32Array(5); - deepEquals(loopFill(u321, 2), u322.fill(2)); - deepEquals(loopFill(u321, 65537), u322.fill(65537)); - deepEquals(loopFill(u321, -1), u322.fill(-1)); - const i81 = new Int8Array(5); - const i82 = new Int8Array(5); - deepEquals(loopFill(i81, 2), i82.fill(2)); - deepEquals(loopFill(i81, -1), i82.fill(-1)); - const i161 = new Int16Array(5); - const i162 = new Int16Array(5); - deepEquals(loopFill(i161, 2), i162.fill(2)); - deepEquals(loopFill(i161, 65535), i162.fill(65535)); - deepEquals(loopFill(i161, -1), i162.fill(-1)); - const i321 = new Int32Array(5); - const i322 = new Int32Array(5); - deepEquals(loopFill(i321, 2), i322.fill(2)); - deepEquals(loopFill(i321, 65537), i322.fill(65537)); - deepEquals(loopFill(i321, -1), i322.fill(-1)); - const f321 = new Float32Array(5); - const f322 = new Float32Array(5); - deepEquals(loopFill(f321, 1.2345), f322.fill(1.2345)); - const f641 = new Float64Array(5); - const f642 = new Float64Array(5); - deepEquals(loopFill(f641, 1.2345), f642.fill(1.2345)); - const u8Clamped1 = new Uint8ClampedArray(5); - const u8Clamped2 = new Uint8ClampedArray(5); - deepEquals(loopFill(u8Clamped1, 2), u8Clamped2.fill(2)); - deepEquals(loopFill(u8Clamped1, 257), u8Clamped2.fill(257)); - }); it('start offset', function(): void { for (let i = -2; i < 10; ++i) { const u81 = new Uint8Array(5); - const u82 = new Uint8Array(5); const u83 = new Uint8Array(5); deepEquals(fillFallback(u81, 2, i), u83.fill(2, i)); deepEquals(fillFallback(u81, -1, i), u83.fill(-1, i)); - deepEquals(loopFill(u82, 2, i), u83.fill(2, i)); - deepEquals(loopFill(u82, -1, i), u83.fill(-1, i)); } }); it('end offset', function(): void { for (let i = -2; i < 10; ++i) { const u81 = new Uint8Array(5); - const u82 = new Uint8Array(5); const u83 = new Uint8Array(5); deepEquals(fillFallback(u81, 2, 0, i), u83.fill(2, 0, i)); deepEquals(fillFallback(u81, -1, 0, i), u83.fill(-1, 0, i)); - deepEquals(loopFill(u82, 2, 0, i), u83.fill(2, 0, i)); - deepEquals(loopFill(u82, -1, 0, i), u83.fill(-1, 0, i)); } }); it('start/end offset', function(): void { for (let i = -2; i < 10; ++i) { for (let j = -2; j < 10; ++j) { const u81 = new Uint8Array(5); - const u82 = new Uint8Array(5); const u83 = new Uint8Array(5); deepEquals(fillFallback(u81, 2, i, j), u83.fill(2, i, j)); deepEquals(fillFallback(u81, -1, i, j), u83.fill(-1, i, j)); - deepEquals(loopFill(u82, 2, i, j), u83.fill(2, i, j)); - deepEquals(loopFill(u82, -1, i, j), u83.fill(-1, i, j)); } } }); From c6f1de797086949619981fa642de1ec677ff59e5 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sat, 24 Nov 2018 05:24:15 -0800 Subject: [PATCH 18/18] Introduce is256Color helper --- src/renderer/BaseRenderLayer.ts | 5 +++-- src/renderer/LinkRenderLayer.ts | 5 +++-- src/renderer/TextRenderLayer.ts | 5 +++-- src/renderer/atlas/CharAtlasUtils.ts | 5 +++++ src/renderer/atlas/StaticCharAtlas.ts | 3 ++- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/renderer/BaseRenderLayer.ts b/src/renderer/BaseRenderLayer.ts index 827356af..2afdebb5 100644 --- a/src/renderer/BaseRenderLayer.ts +++ b/src/renderer/BaseRenderLayer.ts @@ -5,10 +5,11 @@ import { IRenderLayer, IColorSet, IRenderDimensions } from './Types'; import { CharData, ITerminal } from '../Types'; -import { DIM_OPACITY, INVERTED_DEFAULT_COLOR, IGlyphIdentifier, DEFAULT_COLOR } from './atlas/Types'; +import { DIM_OPACITY, INVERTED_DEFAULT_COLOR, IGlyphIdentifier } from './atlas/Types'; import BaseCharAtlas from './atlas/BaseCharAtlas'; import { acquireCharAtlas } from './atlas/CharAtlasCache'; import { CHAR_DATA_CHAR_INDEX } from '../Buffer'; +import { is256Color } from './atlas/CharAtlasUtils'; export abstract class BaseRenderLayer implements IRenderLayer { private _canvas: HTMLCanvasElement; @@ -298,7 +299,7 @@ export abstract class BaseRenderLayer implements IRenderLayer { if (fg === INVERTED_DEFAULT_COLOR) { this._ctx.fillStyle = this._colors.background.css; - } else if (fg < DEFAULT_COLOR) { + } else if (is256Color(fg)) { // 256 color support this._ctx.fillStyle = this._colors.ansi[fg].css; } else { diff --git a/src/renderer/LinkRenderLayer.ts b/src/renderer/LinkRenderLayer.ts index c0d190ac..855830e4 100644 --- a/src/renderer/LinkRenderLayer.ts +++ b/src/renderer/LinkRenderLayer.ts @@ -6,7 +6,8 @@ import { ILinkHoverEvent, ITerminal, ILinkifierAccessor, LinkHoverEventTypes } from '../Types'; import { IColorSet, IRenderDimensions } from './Types'; import { BaseRenderLayer } from './BaseRenderLayer'; -import { INVERTED_DEFAULT_COLOR, DEFAULT_COLOR } from './atlas/Types'; +import { INVERTED_DEFAULT_COLOR } from './atlas/Types'; +import { is256Color } from './atlas/CharAtlasUtils'; export class LinkRenderLayer extends BaseRenderLayer { private _state: ILinkHoverEvent = null; @@ -42,7 +43,7 @@ export class LinkRenderLayer extends BaseRenderLayer { private _onLinkHover(e: ILinkHoverEvent): void { if (e.fg === INVERTED_DEFAULT_COLOR) { this._ctx.fillStyle = this._colors.background.css; - } else if (e.fg < DEFAULT_COLOR) { + } else if (is256Color(e.fg)) { // 256 color support this._ctx.fillStyle = this._colors.ansi[e.fg].css; } else { diff --git a/src/renderer/TextRenderLayer.ts b/src/renderer/TextRenderLayer.ts index 984272f6..7b3feed7 100644 --- a/src/renderer/TextRenderLayer.ts +++ b/src/renderer/TextRenderLayer.ts @@ -9,6 +9,7 @@ import { CharData, ITerminal } from '../Types'; import { INVERTED_DEFAULT_COLOR, DEFAULT_COLOR } from './atlas/Types'; import { GridCache } from './GridCache'; import { BaseRenderLayer } from './BaseRenderLayer'; +import { is256Color } from './atlas/CharAtlasUtils'; /** * This CharData looks like a null character, which will forc a clear and render @@ -186,7 +187,7 @@ export class TextRenderLayer extends BaseRenderLayer { let nextFillStyle = null; // null represents default background color if (bg === INVERTED_DEFAULT_COLOR) { nextFillStyle = this._colors.foreground.css; - } else if (bg < DEFAULT_COLOR) { + } else if (is256Color(bg)) { nextFillStyle = this._colors.ansi[bg].css; } @@ -230,7 +231,7 @@ export class TextRenderLayer extends BaseRenderLayer { this._ctx.save(); if (fg === INVERTED_DEFAULT_COLOR) { this._ctx.fillStyle = this._colors.background.css; - } else if (fg < DEFAULT_COLOR) { + } else if (is256Color(fg)) { // 256 color support this._ctx.fillStyle = this._colors.ansi[fg].css; } else { diff --git a/src/renderer/atlas/CharAtlasUtils.ts b/src/renderer/atlas/CharAtlasUtils.ts index 59ac07df..c504f77e 100644 --- a/src/renderer/atlas/CharAtlasUtils.ts +++ b/src/renderer/atlas/CharAtlasUtils.ts @@ -6,6 +6,7 @@ import { ITerminal } from '../../Types'; import { IColorSet } from '../Types'; import { ICharAtlasConfig } from '../../shared/atlas/Types'; +import { DEFAULT_COLOR } from './Types'; export function generateConfig(scaledCharWidth: number, scaledCharHeight: number, terminal: ITerminal, colors: IColorSet): ICharAtlasConfig { // null out some fields that don't matter @@ -51,3 +52,7 @@ export function configEquals(a: ICharAtlasConfig, b: ICharAtlasConfig): boolean a.colors.foreground === b.colors.foreground && a.colors.background === b.colors.background; } + +export function is256Color(colorCode: number): boolean { + return colorCode < DEFAULT_COLOR; +} diff --git a/src/renderer/atlas/StaticCharAtlas.ts b/src/renderer/atlas/StaticCharAtlas.ts index 673a1509..8dc8be74 100644 --- a/src/renderer/atlas/StaticCharAtlas.ts +++ b/src/renderer/atlas/StaticCharAtlas.ts @@ -7,6 +7,7 @@ import { DIM_OPACITY, IGlyphIdentifier, DEFAULT_COLOR } from './Types'; import { CHAR_ATLAS_CELL_SPACING, ICharAtlasConfig } from '../../shared/atlas/Types'; import { generateStaticCharAtlasTexture } from '../../shared/atlas/CharAtlasGenerator'; import BaseCharAtlas from './BaseCharAtlas'; +import { is256Color } from './CharAtlasUtils'; export default class StaticCharAtlas extends BaseCharAtlas { private _texture: HTMLCanvasElement | ImageBitmap; @@ -58,7 +59,7 @@ export default class StaticCharAtlas extends BaseCharAtlas { } let colorIndex = 0; - if (glyph.fg < DEFAULT_COLOR) { + if (is256Color(glyph.fg)) { colorIndex = 2 + glyph.fg + (glyph.bold ? 16 : 0); } else if (glyph.fg === DEFAULT_COLOR) { // If default color and bold