Fix advanced line wrap if decorations are present

[Remco Haszing (remcohaszing)]

Decorations may affect line wrapping. For example, they make make the text bold, or increase or decrease font size. This wasn’t originally taken into account to calculate the the break offsets.

This was discovered as part of #194609, but it’s actually unrelated. This not only affects line breaking if decorations make text bigger, but also if they make it smaller, bolder, etc.

Fixes #295163

[Remco Haszing (remcohaszing)]

This also uncovers that rerendering changes text selections. When selecting text, the selection ranges within the wrapped viewport are saved. When line wrapping changes, these ranges are restored. So for example if the user selects wrapped line 2, and then line 1 becomes longer because of new decorators, the selection is suddenly at the same wrapped position on line to. I would expect the selection to retain the same content instead.

Is this desirable behaviour or should it be changed? And should the change be part of this PR?

I tried making a screencast to clarify, but unfortunately my screencast tool is broken.

Based on this branch, you can reproduce the issue using the following code in the playground. After applying it, you have 10 seconds to select a bit of text, ideally near the beginning of a viewline.

const value = `big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big big
`

const model = monaco.editor.createModel(value, 'markdown')

const editor = monaco.editor.create(document.getElementById('container'), {
  automaticLayout: true,
  wordWrap: 'on',
  wrappingStrategy: 'advanced',
  fontFamily: 'Arial',
  model
})

const headerDecorationsCollection = editor.createDecorationsCollection()

function applyDecorations() {
  /** @type {monaco.editor.IModelDeltaDecoration[]} */
  const decorations = []
  const value = model.getValue()

  for (const match of value.matchAll(/big/g)) {
    const start = model.getPositionAt(match.index)
    const end = model.getPositionAt(match.index + match[0].length)

    const range = new monaco.Range(
      start.lineNumber,
      start.column,
      end.lineNumber,
      end.column
    )

    decorations.push({
      range,
      options: {
        lineHeight: 57,
        inlineClassName: 'big'
      }
    })
  }

  for (const match of value.matchAll(/small/g)) {
    const start = model.getPositionAt(match.index)
    const end = model.getPositionAt(match.index + match[0].length)

    const range = new monaco.Range(
      start.lineNumber,
      start.column,
      end.lineNumber,
      end.column
    )

    decorations.push({
      range,
      options: {
        lineHeight: 10,
        inlineClassName: 'small'
      }
    })
  }

  headerDecorationsCollection.set(decorations)
}

editor.onDidChangeModelContent(applyDecorations)
setTimeout(() => {
  applyDecorations()
}, 10_000)

.small {
  font-size: 9px;
}

.big {
  font-size: 55px;
}

[Henning Dieterichs (hediet)]

Can you rebase onto main (to fix merge conflicts)? And maybe squash your changes into one commit?

[Remco Haszing (remcohaszing)]

Done!

[Henning Dieterichs (hediet)]

Thanks for the PR!

I understand the problem and I think it can be fixed.

However, this PR has a dramatic performance impact for 99% of VS Code users (who don't use the dom-based line break algorithm, but the monospace one).
For them, whenever someone sets a decoration, all view lines would be reconstructed and the view would be flushed. This is a blocker for this PR.

I think we should handle this:

• When the default line break algorithm is used, setting decorations should only rerender affected lines (current behavior)
• Even with the advanced algorithm, only certain decorations should cause re-computing the layout. I think there is already "decorationAffectsSpacing" on a decoration to mark such decorations. Computing the layout should be incremental, i.e. if a decoration increases the width a little bit and this doesn't cause a new line break, the view should not be rerendered.

So for example if the user selects wrapped line 2, and then line 1 becomes longer because of new decorators, the selection is suddenly at the same wrapped position on line to.

This is a bug and should be fixed.
It works for injected text though, which also affects line breaks (demo).

[Henning Dieterichs (hediet)]

This looks like a significantly changed implementation.
Can you give more context on what happened here?

[Remco Haszing (remcohaszing)]

The addition of decorators means an additional <span> element is added. This required some significant changes, as the algorithm doesn’t deal with arbritrary DOM elements, but specifically one <span> with another <span> inside. It now deals with the other <span> that’s added for the decorations.

Actually I had some trouble grasping what’s going on here, so I came up with my own implementation in a small standalone project. My own implementation turned out very similar. I then applied the new formula here while trying to keep the changes to a minimum

[Remco Haszing (remcohaszing)]

I rebased onto main and added back support for line injected text. I tested the changes in the playground introduced in #249616 with the following code:

/*---------------------------------------------------------------------------------------------
 *  Copyright (c) Microsoft Corporation. All rights reserved.
 *  Licensed under the MIT License. See License.txt in the project root for license information.
 *--------------------------------------------------------------------------------------------*/

// Enable automatic dark mode for accessibility.
const dark = matchMedia('(prefers-color-scheme: dark)');
monaco.editor.setTheme(dark.matches ? 'vs-dark' : 'vs-light');
dark.addEventListener('change', () => {
	monaco.editor.setTheme(dark.matches ? 'vs-dark' : 'vs-light');
});

const content = `The world has changed

I feel it in the water.

I feel it in the earth.

I smell it in the air.

Much that once was is lost, for none now live who remember it.

It began with the forging of the Great Rings. Three were given to the Elves, immortal, wisest and fairest of all beings. Seven to the Dwarf-Lords, great miners and craftsmen of the mountain halls. And nine, nine rings were gifted to the race of Men, who above all else desire power. For within these rings was bound the strength and the will to govern each race. But they were all of them deceived, for another ring was made. Deep in the land of Mordor, in the Fires of Mount Doom, the Dark Lord Sauron forged a master ring, and into this ring he poured his cruelty, his malice and his will to dominate all life.

One ring to rule them all.

One by one, the free lands of Middle-Earth fell to the power of the Ring, but there were some who resisted. A last alliance of men and elves marched against the armies of Mordor, and on the very slopes of Mount Doom, they fought for the freedom of Middle-Earth. Victory was near, but the power of the ring could not be undone. It was in this moment, when all hope had faded, that Isildur, son of the king, took up his father’s sword.

Sauron, enemy of the free peoples of Middle-Earth, was defeated. The Ring passed to Isildur, who had this one chance to destroy evil forever, but the hearts of men are easily corrupted. And the ring of power has a will of its own. It betrayed Isildur, to his death.

And some things that should not have been forgotten were lost. History became legend. Legend became myth. And for two and a half thousand years, the ring passed out of all knowledge. Until, when chance came, it ensnared another bearer.

It came to the creature Gollum, who took it deep into the tunnels of the Misty Mountains. And there it consumed him. The ring gave to Gollum unnatural long life. For five hundred years it poisoned his mind, and in the gloom of Gollum’s cave, it waited. Darkness crept back into the forests of the world. Rumor grew of a shadow in the East, whispers of a nameless fear, and the Ring of Power perceived its time had come. It abandoned Gollum, but then something happened that the Ring did not intend. It was picked up by the most unlikely creature imaginable: a hobbit, Bilbo Baggins, of the Shire.

For the time will soon come when hobbits will shape the fortunes of all.
`;

const model = monaco.editor.createModel(content, undefined, monaco.Uri.file('example.ts'));

const editor = monaco.editor.create(document.getElementById('editor')!, {
	automaticLayout: true,
	autoIndent: 'none',
	accessibilitySupport: 'off',
	accessibilityPageSize: 1,
	autoClosingBrackets: 'never',
	autoClosingComments: 'never',
	autoClosingDelete: 'never',
	autoClosingOvertype: 'never',
	autoClosingQuotes: 'never',
	autoSurround: 'never',
	bracketPairColorization: { enabled: false },
	columnSelection: false,
	contextmenu: false,
	definitionLinkOpensInPeek: false,
	detectIndentation: false,
	disableLayerHinting: true,
	disableMonospaceOptimizations: true,
	fixedOverflowWidgets: false,
	inDiffEditor: false,
	largeFileOptimizations: true,
	linkedEditing: false,
	links: false,
	matchBrackets: 'never',
	parameterHints: { enabled: false },
	model,
	minimap: { enabled: false },
	fontFamily: 'Arial, Arimo, sans-serif, ui-sans-serif',
	fontLigatures: false,
	fontVariations: false,
	fontWeight: 'normal',
	fontSize: 14,
	lineHeight: 0,
	letterSpacing: 0,


	// Wrap lines at viewport width
	wordWrap: 'on',
	// Advanced wrapping strategy - necessary to ensure line wraps with variable width fonts
	wrappingStrategy: 'advanced',
	wordBreak: 'keepAll',
	wrappingIndent: 'none',
});

const boldCollection = editor.createDecorationsCollection();
const textCollection = editor.createDecorationsCollection();

editor.addAction({
	keybindings: [monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyB],
	id: 'bold',
	label: 'Bold',
	run(editor) {
		const selections = editor.getSelections();
		if (!selections) {
			return;
		}

		boldCollection.set(selections.map((selection) => (
			{
				range: selection,
				options: {
					inlineClassNameAffectsLetterSpacing: true,
					inlineClassName: 'cuescript bold'
				}
			}
		)));
	},
});

editor.addAction({
	keybindings: [monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyG],
	id: 'after',
	label: 'After',
	run(editor) {
		const selections = editor.getSelections();
		if (!selections) {
			return;
		}

		textCollection.set(selections.map((selection) => (
			{
				range: selection,
				options: {
					inlineClassNameAffectsLetterSpacing: true,
					inlineClassName: 'cuescript bold',
					after: {
						inlineClassName: 'foo',
						content: 'XXX'
					},
				}
			}
		)));
	},
});

// Make the model and editor available globally for fiddling in the console.
Object.assign(globalThis, {
	editor,
	model,
});

html,
body {
	font-family: system-ui;
	margin: 0;
	height: 100%;
}

main {
	display: flex;
	flex-flow: column;
	height: 100%;
}

h1 {
	background: #00a7da;
	color: #111111;
	flex: 0 1 auto;
	margin: 0;
	overflow: hidden;
	padding: 0.5rem 1rem;
	text-overflow: ellipsis;
	text-wrap: nowrap;
}

#editor {
	margin: 1rem;
	flex: 1 1 auto;
}

.cuescript {
	&.bold {
		color: red;
		font-weight: bold;
	}
}

[Aiday Marlen Kyzy (aiday-mar)]

Hi Remco Haszing (@remcohaszing) thank you for the PR! I didn't know you made this PR to take into account line break data, I just found out. Unfortunately I am currently already working on adopting variable font sizes and as part of the PR I have modified the code that calculates the line breaks to correctly take into account variable font-sizes. The PR is here: #248410. The approach we have taken is to use the DomLineBreaksComputer class when we detect there are variable font sizes on a line and inside of this class to render the line exactly as it is in the editor.

[Remco Haszing (remcohaszing)]

The approach we have taken is to use the DomLineBreaksComputer class when we detect there are variable font sizes on a line and inside of this class to render the line exactly as it is in the editor.

I like that approach better than my own.

This PR does one more thing of which it’s not clear to me whether #248410 does it too. For advanced text wrapping, this PR takes decorations into account that specify the option inlineClassNameAffectsLetterSpacing. This is used to support bold and italic text, or even different fonts.

[Aiday Marlen Kyzy (aiday-mar)]

Hi thanks for your comment. The other PR applies all the inline decorations as they are applied in the editor so I believe it should also apply the decorations you mentioned. I will double check that.

[Remco Haszing (remcohaszing)]

The other PR applies all the inline decorations as they are applied in the editor so I believe it should also apply the decorations you mentioned.

I’m not seeing that.

I also see 1 known issue in #253007.

The line breaks are not correctly computed when the word wrap is toggled

This PR solves that, at least for advanced line wrapping.

Demo:

[Remco Haszing (remcohaszing)]

Friendly ping Henning Dieterichs (@hediet) Aiday Marlen Kyzy (@aiday-mar) :)

I’m not sure what the on-hold label means. This change is working really well, but having to keep rebasing and resolving merge conflicts, adds a risk of introducing bugs.

[Aiday Marlen Kyzy (aiday-mar)]

Hi Remco Haszing (@remcohaszing) I am currently working on a PR to solve this exact problem. I think we will be going with the other approach, so I am not sure we will be pursuing forward with this current PR.