From ad08a2e15b59619ffa24e0efbc60f739f025a181 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 6 Jan 2025 10:15:16 -0800 Subject: [PATCH 1/4] Render selection under ligatures for webgl Part of #5231 --- addons/addon-webgl/src/WebglRenderer.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/addons/addon-webgl/src/WebglRenderer.ts b/addons/addon-webgl/src/WebglRenderer.ts index 6c270d12..d96f0817 100644 --- a/addons/addon-webgl/src/WebglRenderer.ts +++ b/addons/addon-webgl/src/WebglRenderer.ts @@ -507,8 +507,15 @@ export class WebglRenderer extends Disposable implements IRenderer { j = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; this._glyphRenderer.value!.updateCell(x, y, NULL_CELL_CODE, 0, 0, 0, NULL_CELL_CHAR, 0, 0); this._model.cells[j] = NULL_CELL_CODE; - // Don't re-resolve the cell color since multi-colored ligature backgrounds are not - // supported + // HACK: Generally we don't support multi-colored ligature backgrounds, however it's + // important here that we re-resolve the cell color since selections are regular + // background colors. + // + // This can result in bad aliasing since currently ligature glyphs drawn using a single + // texture. This is most noticable when the background colors across the ligature differ + // drastically. This could be improved in the future by sourcing from different glyphs + // for each cell when the foreground or background differ. + this._cellColorResolver.resolve(cell, x, row, this.dimensions.device.cell.width); this._model.cells[j + RENDER_MODEL_BG_OFFSET] = this._cellColorResolver.result.bg; this._model.cells[j + RENDER_MODEL_FG_OFFSET] = this._cellColorResolver.result.fg; this._model.cells[j + RENDER_MODEL_EXT_OFFSET] = this._cellColorResolver.result.ext; From 0521378e70eda5f775464613695632d1703bd3e4 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 6 Jan 2025 10:50:13 -0800 Subject: [PATCH 2/4] Fix ligature selection in DOM renderer Part of #5231 --- .../renderer/dom/DomRendererRowFactory.ts | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index d71edeb9..194565f1 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -91,6 +91,7 @@ export class DomRendererRowFactory { let oldSpacing = 0; let oldIsInSelection: boolean = false; let spacing = 0; + let skipJoinedCheckUntilX = 0; const classes: string[] = []; const hasHover = linkStart !== -1 && linkEnd !== -1; @@ -106,29 +107,41 @@ export class DomRendererRowFactory { // If true, indicates that the current character(s) to draw were joined. let isJoined = false; + + // Indicates whether this cell is part of a joined range that should be ignored as it cannot + // be rendered entirely, like the selection state differs across the range. + let isValidJoinRange = (x >= skipJoinedCheckUntilX); + let lastCharX = x; // Process any joined character ranges as needed. Because of how the // ranges are produced, we know that they are valid for the characters // and attributes of our input. let cell = this._workCell; - if (joinedRanges.length > 0 && x === joinedRanges[0][0]) { - isJoined = true; + if (joinedRanges.length > 0 && x === joinedRanges[0][0] && isValidJoinRange) { const range = joinedRanges.shift()!; + // If the ligature's selection state is not consistent, don't join it. This helps the + // selection render correctly regardless whether they should be joined. + if (this._isCellInSelection(range[0], row) !== this._isCellInSelection(range[1], row)) { + isValidJoinRange = false; + skipJoinedCheckUntilX = range[1]; + } else { + isJoined = true; - // We already know the exact start and end column of the joined range, - // so we get the string and width representing it directly - cell = new JoinedCellData( - this._workCell, - lineData.translateToString(true, range[0], range[1]), - range[1] - range[0] - ); + // We already know the exact start and end column of the joined range, + // so we get the string and width representing it directly + cell = new JoinedCellData( + this._workCell, + lineData.translateToString(true, range[0], range[1]), + range[1] - range[0] + ); - // Skip over the cells occupied by this range in the loop - lastCharX = range[1] - 1; + // Skip over the cells occupied by this range in the loop + lastCharX = range[1] - 1; - // Recalculate width - width = cell.getWidth(); + // Recalculate width + width = cell.getWidth(); + } } const isInSelection = this._isCellInSelection(x, row); @@ -178,6 +191,7 @@ export class DomRendererRowFactory { && !isCursorCell && !isJoined && !isDecorated + && isValidJoinRange ) { // no span alterations, thus only account chars skipping all code below if (cell.isInvisible()) { @@ -435,7 +449,7 @@ export class DomRendererRowFactory { } // exclude conditions for cell merging - never merge these - if (!isCursorCell && !isJoined && !isDecorated) { + if (!isCursorCell && !isJoined && !isDecorated && isValidJoinRange) { cellAmount++; } else { charElement.textContent = text; From 5bfb130f03bc91d48d20355f754b76289b22c53f Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 6 Jan 2025 10:52:04 -0800 Subject: [PATCH 3/4] Apply same approach for rendering joined selection in webgl --- addons/addon-webgl/src/WebglRenderer.ts | 36 ++++++++++++------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/addons/addon-webgl/src/WebglRenderer.ts b/addons/addon-webgl/src/WebglRenderer.ts index d96f0817..bd703832 100644 --- a/addons/addon-webgl/src/WebglRenderer.ts +++ b/addons/addon-webgl/src/WebglRenderer.ts @@ -422,19 +422,24 @@ export class WebglRenderer extends Disposable implements IRenderer { // ranges are produced, we know that they are valid for the characters // and attributes of our input. if (joinedRanges.length > 0 && x === joinedRanges[0][0]) { - isJoined = true; range = joinedRanges.shift()!; - // We already know the exact start and end column of the joined range, - // so we get the string and width representing it directly. - cell = new JoinedCellData( - cell, - line!.translateToString(true, range[0], range[1]), - range[1] - range[0] - ); + // If the ligature's selection state is not consistent, don't join it. This helps the + // selection render correctly regardless whether they should be joined. + if (this._model.selection.isCellSelected(this._terminal, range[0], row) === this._model.selection.isCellSelected(this._terminal, range[1], row)) { + isJoined = true; - // Skip over the cells occupied by this range in the loop - lastCharX = range[1] - 1; + // We already know the exact start and end column of the joined range, + // so we get the string and width representing it directly. + cell = new JoinedCellData( + cell, + line!.translateToString(true, range[0], range[1]), + range[1] - range[0] + ); + + // Skip over the cells occupied by this range in the loop + lastCharX = range[1] - 1; + } } chars = cell.getChars(); @@ -507,15 +512,8 @@ export class WebglRenderer extends Disposable implements IRenderer { j = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL; this._glyphRenderer.value!.updateCell(x, y, NULL_CELL_CODE, 0, 0, 0, NULL_CELL_CHAR, 0, 0); this._model.cells[j] = NULL_CELL_CODE; - // HACK: Generally we don't support multi-colored ligature backgrounds, however it's - // important here that we re-resolve the cell color since selections are regular - // background colors. - // - // This can result in bad aliasing since currently ligature glyphs drawn using a single - // texture. This is most noticable when the background colors across the ligature differ - // drastically. This could be improved in the future by sourcing from different glyphs - // for each cell when the foreground or background differ. - this._cellColorResolver.resolve(cell, x, row, this.dimensions.device.cell.width); + // Don't re-resolve the cell color since multi-colored ligature backgrounds are not + // supported this._model.cells[j + RENDER_MODEL_BG_OFFSET] = this._cellColorResolver.result.bg; this._model.cells[j + RENDER_MODEL_FG_OFFSET] = this._cellColorResolver.result.fg; this._model.cells[j + RENDER_MODEL_EXT_OFFSET] = this._cellColorResolver.result.ext; From c7a1e0aea343d443a89395e1d981ccce8f56be27 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 6 Jan 2025 11:03:10 -0800 Subject: [PATCH 4/4] Handle selections in the middle of ligatures too --- addons/addon-webgl/src/WebglRenderer.ts | 18 ++++++++++++++++-- .../renderer/dom/DomRendererRowFactory.ts | 8 ++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/addons/addon-webgl/src/WebglRenderer.ts b/addons/addon-webgl/src/WebglRenderer.ts index bd703832..aa9587aa 100644 --- a/addons/addon-webgl/src/WebglRenderer.ts +++ b/addons/addon-webgl/src/WebglRenderer.ts @@ -377,6 +377,8 @@ export class WebglRenderer extends Disposable implements IRenderer { let line: IBufferLine; let joinedRanges: [number, number][]; let isJoined: boolean; + let skipJoinedCheckUntilX: number = 0; + let isValidJoinRange: boolean = true; let lastCharX: number; let range: [number, number]; let chars: string; @@ -405,6 +407,7 @@ export class WebglRenderer extends Disposable implements IRenderer { row = y + terminal.buffer.ydisp; line = terminal.buffer.lines.get(row)!; this._model.lineLengths[y] = 0; + skipJoinedCheckUntilX = 0; joinedRanges = this._characterJoinerService.getJoinedCharacters(row); for (x = 0; x < terminal.cols; x++) { lastBg = this._cellColorResolver.result.bg; @@ -416,17 +419,28 @@ export class WebglRenderer extends Disposable implements IRenderer { // If true, indicates that the current character(s) to draw were joined. isJoined = false; + + // Indicates whether this cell is part of a joined range that should be ignored as it cannot + // be rendered entirely, like the selection state differs across the range. + isValidJoinRange = (x >= skipJoinedCheckUntilX); + lastCharX = x; // Process any joined character ranges as needed. Because of how the // ranges are produced, we know that they are valid for the characters // and attributes of our input. - if (joinedRanges.length > 0 && x === joinedRanges[0][0]) { + if (joinedRanges.length > 0 && x === joinedRanges[0][0] && isValidJoinRange) { range = joinedRanges.shift()!; // If the ligature's selection state is not consistent, don't join it. This helps the // selection render correctly regardless whether they should be joined. - if (this._model.selection.isCellSelected(this._terminal, range[0], row) === this._model.selection.isCellSelected(this._terminal, range[1], row)) { + const firstSelectionState = this._model.selection.isCellSelected(this._terminal, range[0], row); + for (i = range[0] + 1; i < range[1]; i++) { + isValidJoinRange &&= (firstSelectionState === this._model.selection.isCellSelected(this._terminal, i, row)); + } + if (!isValidJoinRange) { + skipJoinedCheckUntilX = range[1]; + } else { isJoined = true; // We already know the exact start and end column of the joined range, diff --git a/src/browser/renderer/dom/DomRendererRowFactory.ts b/src/browser/renderer/dom/DomRendererRowFactory.ts index 194565f1..d5750112 100644 --- a/src/browser/renderer/dom/DomRendererRowFactory.ts +++ b/src/browser/renderer/dom/DomRendererRowFactory.ts @@ -84,6 +84,7 @@ export class DomRendererRowFactory { let charElement: HTMLSpanElement | undefined; let cellAmount = 0; let text = ''; + let i = 0; let oldBg = 0; let oldFg = 0; let oldExt = 0; @@ -122,8 +123,11 @@ export class DomRendererRowFactory { const range = joinedRanges.shift()!; // If the ligature's selection state is not consistent, don't join it. This helps the // selection render correctly regardless whether they should be joined. - if (this._isCellInSelection(range[0], row) !== this._isCellInSelection(range[1], row)) { - isValidJoinRange = false; + const firstSelectionState = this._isCellInSelection(range[0], row); + for (i = range[0] + 1; i < range[1]; i++) { + isValidJoinRange &&= (firstSelectionState === this._isCellInSelection(i, row)); + } + if (!isValidJoinRange) { skipJoinedCheckUntilX = range[1]; } else { isJoined = true;