Fix advanced line wrap if decorations are present

[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.

cc @hediet

[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;
}

[hediet]

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

[remcohaszing]

Done!

[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).

[hediet]

We really shouldn't query all decorations here.
There could be thousands of them (e.g. bracket pair colorization uses decorations, so this would list all brackets in the entire document).
getAllDecorations should never be used.

[remcohaszing]

Should we handled this here instead?

vscode/src/vs/editor/common/model/textModel.ts

Lines 2110 to 2118 in 19a7838

	public insert(node: IntervalNode): void {
	if (isNodeInjectedText(node)) {
	this._injectedTextDecorationsTree.insert(node);
	} else if (isNodeInOverviewRuler(node)) {
	this._decorationsTree1.insert(node);
	} else {
	this._decorationsTree0.insert(node);
	}
	}

We could change _injectedTextDecorationsTree to include decorations that have inlineClassNameAffectsLetterSpacing enabled. Alternatively we could add a new IntervalTree which is used to track all decorations that enable inlineClassNameAffectsLetterSpacing. In this case we don’t need to use getAllDecorations. LineInjectedText could be adjusted to also support decorations with inlineClassNameAffectsLetterSpacing. Then there’s no need anymore for lineBreaksComputer.finalize() to accept any arguments.

[hediet]

Hmm. Can we be more clever then this? This rerenders the entire view when decorations change (even if they change outside of the view).
Even worse, this re-constructs every line in the entire model.

[hediet]

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

[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