-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow all overflowing characters to extend to the next cell if followed by a space (Emojis etc) #997
Allow all overflowing characters to extend to the next cell if followed by a space (Emojis etc) #997
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,10 +16,13 @@ import { BaseRenderLayer, INVERTED_DEFAULT_COLOR } from './BaseRenderLayer'; | |
* when the character changes (a regular space ' ' character may not as it's | ||
* drawn state is a cleared cell). | ||
*/ | ||
const EMOJI_OWNED_CHAR_DATA: CharData = [null, '', 0, -1]; | ||
const OVERLAP_OWNED_CHAR_DATA: CharData = [null, '', 0, -1]; | ||
|
||
export class TextRenderLayer extends BaseRenderLayer { | ||
private _state: GridCache<CharData>; | ||
private _characterWidth: number; | ||
private _characterFont: string; | ||
private _characterOverlapCache: { [key: string]: boolean } = {}; | ||
|
||
constructor(container: HTMLElement, zIndex: number, colors: IColorSet) { | ||
super(container, 'text', zIndex, false, colors); | ||
|
@@ -28,6 +31,14 @@ export class TextRenderLayer extends BaseRenderLayer { | |
|
||
public resize(terminal: ITerminal, dim: IRenderDimensions, charSizeChanged: boolean): void { | ||
super.resize(terminal, dim, charSizeChanged); | ||
|
||
// Clear the character width cache if the font or width has changed | ||
const terminalFont = `${terminal.options.fontSize * window.devicePixelRatio}px ${terminal.options.fontFamily}`; | ||
if (this._characterWidth !== dim.scaledCharWidth || this._characterFont !== terminalFont) { | ||
this._characterWidth = dim.scaledCharWidth; | ||
this._characterFont = terminalFont; | ||
this._characterOverlapCache = {}; | ||
} | ||
// Resizing the canvas discards the contents of the canvas so clear state | ||
this._state.clear(); | ||
this._state.resize(terminal.cols, terminal.rows); | ||
|
@@ -63,12 +74,12 @@ export class TextRenderLayer extends BaseRenderLayer { | |
} | ||
|
||
// If the character is a space and the character to the left is an | ||
// emoji, skip the character and allow the emoji char to take full | ||
// control over this character's cell. | ||
// overlapping character, skip the character and allow the overlapping | ||
// char to take full control over this character's cell. | ||
if (code === 32 /*' '*/) { | ||
if (x > 0) { | ||
const previousChar: CharData = line[x - 1]; | ||
if (this._isEmoji(previousChar[CHAR_DATA_CHAR_INDEX])) { | ||
if (this._isOverlapping(previousChar[CHAR_DATA_CHAR_INDEX])) { | ||
continue; | ||
} | ||
} | ||
|
@@ -98,23 +109,23 @@ export class TextRenderLayer extends BaseRenderLayer { | |
continue; | ||
} | ||
|
||
// If the character is an emoji and the character to the right is a | ||
// If the character is an overlapping char and the character to the right is a | ||
// space, take ownership of the cell to the right. | ||
if (this._isEmoji(char)) { | ||
// If the character is an emoji, we want to force a re-render on every | ||
if (width !== 0 && this._isOverlapping(char)) { | ||
// If the character is overlapping, we want to force a re-render on every | ||
// frame. This is specifically to work around the case where two | ||
// emoji's `a` and `b` are adjacent, the cursor is moved to b and a | ||
// overlaping chars `a` and `b` are adjacent, the cursor is moved to b and a | ||
// space is added. Without this, the first half of `b` would never | ||
// get removed, and `a` would not re-render because it thinks it's | ||
// already in the correct state. | ||
this._state.cache[x][y] = EMOJI_OWNED_CHAR_DATA; | ||
this._state.cache[x][y] = OVERLAP_OWNED_CHAR_DATA; | ||
if (x < line.length && line[x + 1][CHAR_DATA_CODE_INDEX] === 32 /*' '*/) { | ||
width = 2; | ||
this._clearChar(x + 1, y); | ||
// The emoji owned char data will force a clear and render when the | ||
// emoji is no longer to the left of the character and also when the | ||
// space changes to another character. | ||
this._state.cache[x + 1][y] = EMOJI_OWNED_CHAR_DATA; | ||
// The overlapping char's char data will force a clear and render when the | ||
// overlapping char is no longer to the left of the character and also when | ||
// the space changes to another character. | ||
this._state.cache[x + 1][y] = OVERLAP_OWNED_CHAR_DATA; | ||
} | ||
} | ||
|
||
|
@@ -168,20 +179,37 @@ export class TextRenderLayer extends BaseRenderLayer { | |
} | ||
} | ||
} | ||
|
||
/** | ||
* Whether the character is an emoji. | ||
* @param char The character to search. | ||
*/ | ||
private _isEmoji(char: string): boolean { | ||
// TODO: We need a generic solution for handling characters like this | ||
// Check special ambiguous width characters | ||
if (char === '➜') { | ||
return true; | ||
} | ||
// Check emoji unicode range | ||
return char.search(/([\uD800-\uDBFF][\uDC00-\uDFFF])/g) >= 0; | ||
} | ||
|
||
/** | ||
* Whether a character is overlapping to the | ||
* next cell. | ||
*/ | ||
private _isOverlapping(char: string): boolean { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The common case:
It should be safe to assume this. |
||
// Deliver from cache if available | ||
if (this._characterOverlapCache.hasOwnProperty(char)) { | ||
return this._characterOverlapCache[char]; | ||
} | ||
|
||
// Set the correct character font for the current context | ||
let font; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of doing this use |
||
if (this._ctx.font !== this._characterFont) { | ||
font = this._ctx.font; | ||
this._ctx.font = this._characterFont; | ||
} | ||
|
||
// Measure the width of the character, but Math.floor it | ||
// because that is what the renderer does when it calculates | ||
// the character dimensions we are comparing against | ||
const overlaps = Math.floor(this._ctx.measureText(char).width) > this._characterWidth; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function seems a lot faster than I was expecting 😄 |
||
|
||
// Restore the original font to the context | ||
if (font) this._ctx.font = font; | ||
|
||
// Cache and return | ||
return this._characterOverlapCache[char] = overlaps; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's make this 2 lines, I always have to have a second look as doing this behaves very differently in different languages. |
||
|
||
} | ||
|
||
/** | ||
* Clear the charcater at the cell specified. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also pass in char
code
, maybe theCharData
object would be easier since we need 2 of its props?