Skip to content

Commit

Permalink
Fixes #33525: Split most kinds of large tokens at spaces
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexdima
alexdima committed Sep 18, 2018
1 parent 43c2879 commit 78fc0e2
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 24 deletions.
69 changes: 52 additions & 17 deletions src/vs/editor/common/viewLayout/viewLineRenderer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,9 @@ function resolveRenderLineInput(input: RenderLineInput): ResolvedRenderLineInput
}
tokens = _applyInlineDecorations(lineContent, len, tokens, input.lineDecorations);
}
if (input.isBasicASCII && !input.fontLigatures) {
tokens = splitLargeTokens(lineContent, tokens);
if (!input.containsRTL) {
// We can never split RTL text, as it ruins the rendering
tokens = splitLargeTokens(lineContent, tokens, !input.isBasicASCII || input.fontLigatures);
}

return new ResolvedRenderLineInput(
Expand Down Expand Up @@ -418,25 +419,59 @@ const enum Constants {
* It appears that having very large spans causes very slow reading of character positions.
* So here we try to avoid that.
*/
function splitLargeTokens(lineContent: string, tokens: LinePart[]): LinePart[] {
function splitLargeTokens(lineContent: string, tokens: LinePart[], onlyAtSpaces: boolean): LinePart[] {
let lastTokenEndIndex = 0;
let result: LinePart[] = [], resultLen = 0;
for (let i = 0, len = tokens.length; i < len; i++) {
const token = tokens[i];
const tokenEndIndex = token.endIndex;
let diff = (tokenEndIndex - lastTokenEndIndex);
if (diff > Constants.LongToken) {
const tokenType = token.type;
const piecesCount = Math.ceil(diff / Constants.LongToken);
for (let j = 1; j < piecesCount; j++) {
let pieceEndIndex = lastTokenEndIndex + (j * Constants.LongToken);
result[resultLen++] = new LinePart(pieceEndIndex, tokenType);

if (onlyAtSpaces) {
// Split only at spaces => we need to walk each character
for (let i = 0, len = tokens.length; i < len; i++) {
const token = tokens[i];
const tokenEndIndex = token.endIndex;
if (lastTokenEndIndex + Constants.LongToken < tokenEndIndex) {
const tokenType = token.type;

let lastSpaceOffset = -1;
let currTokenStart = lastTokenEndIndex;
for (let j = lastTokenEndIndex; j < tokenEndIndex; j++) {
if (lineContent.charCodeAt(j) === CharCode.Space) {
lastSpaceOffset = j;
}
if (lastSpaceOffset !== -1 && j - currTokenStart >= Constants.LongToken) {
// Split at `lastSpaceOffset` + 1
result[resultLen++] = new LinePart(lastSpaceOffset + 1, tokenType);
currTokenStart = lastSpaceOffset + 1;
lastSpaceOffset = -1;
}
}
if (currTokenStart !== tokenEndIndex) {
result[resultLen++] = new LinePart(tokenEndIndex, tokenType);
}
} else {
result[resultLen++] = token;
}
result[resultLen++] = new LinePart(tokenEndIndex, tokenType);
} else {
result[resultLen++] = token;

lastTokenEndIndex = tokenEndIndex;
}
} else {
// Split anywhere => we don't need to walk each character
for (let i = 0, len = tokens.length; i < len; i++) {
const token = tokens[i];
const tokenEndIndex = token.endIndex;
let diff = (tokenEndIndex - lastTokenEndIndex);
if (diff > Constants.LongToken) {
const tokenType = token.type;
const piecesCount = Math.ceil(diff / Constants.LongToken);
for (let j = 1; j < piecesCount; j++) {
let pieceEndIndex = lastTokenEndIndex + (j * Constants.LongToken);
result[resultLen++] = new LinePart(pieceEndIndex, tokenType);
}
result[resultLen++] = new LinePart(tokenEndIndex, tokenType);
} else {
result[resultLen++] = token;
}
lastTokenEndIndex = tokenEndIndex;
}
lastTokenEndIndex = tokenEndIndex;
}

return result;
Expand Down
76 changes: 69 additions & 7 deletions src/vs/editor/test/common/viewLayout/viewLineRenderer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,9 @@ suite('viewLineRenderer.renderLine', () => {
'101 chars',
_lineText.substr(0, 101),
[
'<span class="mtk1">This\u00a0is\u00a0just\u00a0a\u00a0long\u00a0line\u00a0that\u00a0contains\u00a0very\u00a0interesting\u00a0text.\u00a0This\u00a0is\u00a0just\u00a0a\u00a0long\u00a0line\u00a0that\u00a0contains\u00a0</span>',
'<span class="mtk1">This\u00a0is\u00a0just\u00a0a\u00a0long\u00a0line\u00a0that\u00a0contains\u00a0very\u00a0</span>',
'<span class="mtk1">interesting\u00a0text.\u00a0This\u00a0is\u00a0just\u00a0a\u00a0long\u00a0line\u00a0that\u00a0</span>',
'<span class="mtk1">contains\u00a0</span>',
]
);
}
Expand Down Expand Up @@ -1375,14 +1377,12 @@ suite('viewLineRenderer.renderLine 2', () => {

let expected = [
'<span>',
'<span class="mtk3">\u00a0JoyShareல்\u00a0பின்தொடர்ந்து,\u00a0விடீயோ,\u00a0ஜோக்குகள்,\u00a0அனிமேசன்,\u00a0நகைச்சுவை\u00a0படங்கள்\u00a0மற்றும்\u00a0செய்திகளை\u00a0பெறுவீர்</span>',
'<span class="mtk3">\u00a0JoyShareல்\u00a0பின்தொடர்ந்து,\u00a0விடீயோ,\u00a0ஜோக்குகள்,\u00a0</span>',
'<span class="mtk3">அனிமேசன்,\u00a0நகைச்சுவை\u00a0படங்கள்\u00a0மற்றும்\u00a0செய்திகளை\u00a0</span>',
'<span class="mtk3">பெறுவீர்</span>',
'</span>'
].join('');

let _expected = expected.split('').map(c => c.charCodeAt(0));
let _actual = actual.html.split('').map(c => c.charCodeAt(0));
assert.deepEqual(_actual, _expected);

assert.deepEqual(actual.html, expected);
});

Expand All @@ -1408,7 +1408,9 @@ suite('viewLineRenderer.renderLine 2', () => {

let expected = [
'<span>',
'<span class="mtk3">\u00a0वो\u00a0ऐसा\u00a0क्या\u00a0है\u00a0जो\u00a0हमारे\u00a0अंदर\u00a0भी\u00a0है\u00a0और\u00a0बाहर\u00a0भी\u00a0है।\u00a0जिसकी\u00a0वजह\u00a0से\u00a0हम\u00a0सब\u00a0हैं।\u00a0जिसने\u00a0इस\u00a0सृष्टि\u00a0की\u00a0रचना\u00a0की\u00a0है।</span>',
'<span class="mtk3">\u00a0वो\u00a0ऐसा\u00a0क्या\u00a0है\u00a0जो\u00a0हमारे\u00a0अंदर\u00a0भी\u00a0है\u00a0और\u00a0बाहर\u00a0भी\u00a0है।\u00a0</span>',
'<span class="mtk3">जिसकी\u00a0वजह\u00a0से\u00a0हम\u00a0सब\u00a0हैं।\u00a0जिसने\u00a0इस\u00a0सृष्टि\u00a0की\u00a0रचना\u00a0की\u00a0</span>',
'<span class="mtk3">है।</span>',
'</span>'
].join('');

Expand Down Expand Up @@ -1443,6 +1445,66 @@ suite('viewLineRenderer.renderLine 2', () => {
assert.deepEqual(actual.html, expected);
});

test('issue #33525: Long line with ligatures takes a long time to paint decorations', () => {
let actual = renderViewLine(new RenderLineInput(
false,
false,
'append data to append data to append data to append data to append data to append data to append data to append data to append data to append data to append data to append data to append data to',
false,
true,
false,
0,
createViewLineTokens([createPart(194, 3)]),
[],
4,
10,
10000,
'none',
false,
true
));

let expected = [
'<span>',
'<span class="mtk3">append\u00a0data\u00a0to\u00a0append\u00a0data\u00a0to\u00a0append\u00a0data\u00a0to\u00a0</span>',
'<span class="mtk3">append\u00a0data\u00a0to\u00a0append\u00a0data\u00a0to\u00a0append\u00a0data\u00a0to\u00a0</span>',
'<span class="mtk3">append\u00a0data\u00a0to\u00a0append\u00a0data\u00a0to\u00a0append\u00a0data\u00a0to\u00a0</span>',
'<span class="mtk3">append\u00a0data\u00a0to\u00a0append\u00a0data\u00a0to\u00a0append\u00a0data\u00a0to\u00a0</span>',
'<span class="mtk3">append\u00a0data\u00a0to</span>',
'</span>'
].join('');

assert.deepEqual(actual.html, expected);
});

test('issue #33525: Long line with ligatures takes a long time to paint decorations - not possible', () => {
let actual = renderViewLine(new RenderLineInput(
false,
false,
'appenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatato',
false,
true,
false,
0,
createViewLineTokens([createPart(194, 3)]),
[],
4,
10,
10000,
'none',
false,
true
));

let expected = [
'<span>',
'<span class="mtk3">appenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatatoappenddatato</span>',
'</span>'
].join('');

assert.deepEqual(actual.html, expected);
});

function createTestGetColumnOfLinePartOffset(lineContent: string, tabSize: number, parts: ViewLineToken[], expectedPartLengths: number[]): (partIndex: number, partLength: number, offset: number, expected: number) => void {
let renderLineOutput = renderViewLine(new RenderLineInput(
false,
Expand Down

0 comments on commit 78fc0e2

Please sign in to comment.