Skip to content

Commit c6c6758

Browse files
TyriarTyriar
authoredNov 23, 2018
Merge pull request #1792 from Tyriar/1609_dom_no_null_cells
DOM renderer: Don't output empty cells at end of a line
·
5.5.03.9.0
2 parents a693eb8 + f363872 commit c6c6758

File tree

3 files changed

+32
-32
lines changed

3 files changed

+32
-32
lines changed
 

‎src/renderer/dom/DomRenderer.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ export class DomRenderer extends EventEmitter implements IRenderer {
111111
element.style.width = `${this.dimensions.canvasWidth}px`;
112112
element.style.height = `${this.dimensions.actualCellHeight}px`;
113113
element.style.lineHeight = `${this.dimensions.actualCellHeight}px`;
114+
// Make sure rows don't overflow onto following row
115+
element.style.overflow = 'hidden';
114116
});
115117

116118
if (!this._dimensionsStyleElement) {
@@ -330,7 +332,7 @@ export class DomRenderer extends EventEmitter implements IRenderer {
330332
const row = y + terminal.buffer.ydisp;
331333
const lineData = terminal.buffer.lines.get(row);
332334
const cursorStyle = terminal.options.cursorStyle;
333-
rowElement.appendChild(this._rowFactory.createRow(lineData, row === cursorAbsoluteY, cursorStyle, cursorX, terminal.charMeasure.width, terminal.cols));
335+
rowElement.appendChild(this._rowFactory.createRow(lineData, row === cursorAbsoluteY, cursorStyle, cursorX, this.dimensions.actualCellWidth, terminal.cols));
334336
}
335337

336338
this._terminal.emit('refresh', {start, end});

‎src/renderer/dom/DomRendererRowFactory.test.ts

+11-21
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,10 @@ describe('DomRendererRowFactory', () => {
2323
});
2424

2525
describe('createRow', () => {
26-
it('should create an element for every character in the row', () => {
26+
it('should not create anything for an empty row', () => {
2727
const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20);
2828
assert.equal(getFragmentHtml(fragment),
29-
'<span> </span>' +
30-
'<span> </span>'
29+
''
3130
);
3231
});
3332

@@ -45,8 +44,7 @@ describe('DomRendererRowFactory', () => {
4544
for (const style of ['block', 'bar', 'underline']) {
4645
const fragment = rowFactory.createRow(lineData, true, style, 0, 5, 20);
4746
assert.equal(getFragmentHtml(fragment),
48-
`<span class="xterm-cursor xterm-cursor-${style}"> </span>` +
49-
'<span> </span>'
47+
`<span class="xterm-cursor xterm-cursor-${style}"> </span>`
5048
);
5149
}
5250
});
@@ -65,17 +63,15 @@ describe('DomRendererRowFactory', () => {
6563
lineData.set(0, [DEFAULT_ATTR | (FLAGS.BOLD << 18), 'a', 1, 'a'.charCodeAt(0)]);
6664
const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20);
6765
assert.equal(getFragmentHtml(fragment),
68-
'<span class="xterm-bold">a</span>' +
69-
'<span> </span>'
66+
'<span class="xterm-bold">a</span>'
7067
);
7168
});
7269

7370
it('should add class for italic', () => {
7471
lineData.set(0, [DEFAULT_ATTR | (FLAGS.ITALIC << 18), 'a', 1, 'a'.charCodeAt(0)]);
7572
const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20);
7673
assert.equal(getFragmentHtml(fragment),
77-
'<span class="xterm-italic">a</span>' +
78-
'<span> </span>'
74+
'<span class="xterm-italic">a</span>'
7975
);
8076
});
8177

@@ -85,8 +81,7 @@ describe('DomRendererRowFactory', () => {
8581
lineData.set(0, [defaultAttrNoFgColor | (i << 9), 'a', 1, 'a'.charCodeAt(0)]);
8682
const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20);
8783
assert.equal(getFragmentHtml(fragment),
88-
`<span class="xterm-fg-${i}">a</span>` +
89-
'<span> </span>'
84+
`<span class="xterm-fg-${i}">a</span>`
9085
);
9186
}
9287
});
@@ -97,8 +92,7 @@ describe('DomRendererRowFactory', () => {
9792
lineData.set(0, [defaultAttrNoBgColor | (i << 0), 'a', 1, 'a'.charCodeAt(0)]);
9893
const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20);
9994
assert.equal(getFragmentHtml(fragment),
100-
`<span class="xterm-bg-${i}">a</span>` +
101-
'<span> </span>'
95+
`<span class="xterm-bg-${i}">a</span>`
10296
);
10397
}
10498
});
@@ -107,26 +101,23 @@ describe('DomRendererRowFactory', () => {
107101
lineData.set(0, [(FLAGS.INVERSE << 18) | (2 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]);
108102
const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20);
109103
assert.equal(getFragmentHtml(fragment),
110-
'<span class="xterm-fg-1 xterm-bg-2">a</span>' +
111-
'<span> </span>'
104+
'<span class="xterm-fg-1 xterm-bg-2">a</span>'
112105
);
113106
});
114107

115108
it('should correctly invert default fg color', () => {
116109
lineData.set(0, [(FLAGS.INVERSE << 18) | (257 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]);
117110
const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20);
118111
assert.equal(getFragmentHtml(fragment),
119-
'<span class="xterm-fg-1 xterm-bg-15">a</span>' +
120-
'<span> </span>'
112+
'<span class="xterm-fg-1 xterm-bg-15">a</span>'
121113
);
122114
});
123115

124116
it('should correctly invert default bg color', () => {
125117
lineData.set(0, [(FLAGS.INVERSE << 18) | (1 << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]);
126118
const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20);
127119
assert.equal(getFragmentHtml(fragment),
128-
'<span class="xterm-fg-0 xterm-bg-1">a</span>' +
129-
'<span> </span>'
120+
'<span class="xterm-fg-0 xterm-bg-1">a</span>'
130121
);
131122
});
132123

@@ -135,8 +126,7 @@ describe('DomRendererRowFactory', () => {
135126
lineData.set(0, [(FLAGS.BOLD << 18) | (i << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]);
136127
const fragment = rowFactory.createRow(lineData, false, undefined, 0, 5, 20);
137128
assert.equal(getFragmentHtml(fragment),
138-
`<span class="xterm-bold xterm-fg-${i + 8}">a</span>` +
139-
'<span> </span>'
129+
`<span class="xterm-bold xterm-fg-${i + 8}">a</span>`
140130
);
141131
}
142132
});

‎src/renderer/dom/DomRendererRowFactory.ts

+18-10
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* @license MIT
44
*/
55

6-
import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_ATTR_INDEX, CHAR_DATA_WIDTH_INDEX } from '../../Buffer';
6+
import { CHAR_DATA_CHAR_INDEX, CHAR_DATA_ATTR_INDEX, CHAR_DATA_WIDTH_INDEX, CHAR_DATA_CODE_INDEX, NULL_CELL_CODE } from '../../Buffer';
77
import { FLAGS } from '../Types';
88
import { IBufferLine } from '../../Types';
99

@@ -22,18 +22,27 @@ export class DomRendererRowFactory {
2222

2323
public createRow(lineData: IBufferLine, isCursorRow: boolean, cursorStyle: string | undefined, cursorX: number, cellWidth: number, cols: number): DocumentFragment {
2424
const fragment = this._document.createDocumentFragment();
25-
let colCount = 0;
2625

27-
for (let x = 0; x < lineData.length; x++) {
28-
// Don't allow any buffer to the right to be displayed
29-
if (colCount >= cols) {
30-
continue;
26+
// Find the line length first, this prevents the need to output a bunch of
27+
// empty cells at the end. This cannot easily be integrated into the main
28+
// loop below because of the colCount feature (which can be removed after we
29+
// properly support reflow and disallow data to go beyond the right-side of
30+
// the viewport).
31+
let lineLength = 0;
32+
for (let x = Math.min(lineData.length, cols) - 1; x >= 0; x--) {
33+
const charData = lineData.get(x);
34+
const code = charData[CHAR_DATA_CODE_INDEX];
35+
if (code !== NULL_CELL_CODE || (isCursorRow && x === cursorX)) {
36+
lineLength = x + 1;
37+
break;
3138
}
39+
}
3240

41+
for (let x = 0; x < lineLength; x++) {
3342
const charData = lineData.get(x);
34-
const char: string = charData[CHAR_DATA_CHAR_INDEX];
35-
const attr: number = charData[CHAR_DATA_ATTR_INDEX];
36-
const width: number = charData[CHAR_DATA_WIDTH_INDEX];
43+
const char = charData[CHAR_DATA_CHAR_INDEX];
44+
const attr = charData[CHAR_DATA_ATTR_INDEX];
45+
const width = charData[CHAR_DATA_WIDTH_INDEX];
3746

3847
// The character to the left is a wide character, drawing is owned by the char at x-1
3948
if (width === 0) {
@@ -98,7 +107,6 @@ export class DomRendererRowFactory {
98107
charElement.classList.add(`xterm-bg-${bg}`);
99108
}
100109
fragment.appendChild(charElement);
101-
colCount += width;
102110
}
103111
return fragment;
104112
}

0 commit comments

Comments
 (0)
Please sign in to comment.