Skip to content

Commit a27b338

Browse files
committed
Fix toggling wrapping sometimes losing track of DOM nodes
On flush, the DOM renderer expects the following render to have lines and as such defers clearing the innerHTML of the line collection in order to save a little time. Since this GPU renderer may not actually render lines however, this ends up having the possibility to lose track of rendered DOM lines and they would stick around until the next flush where it actually does clear the parent out. This was mostly reproducible when toggling wrapping in markdown files, but it was somewhat difficult I believe depending on how many lines would pass canRender or not which changes at various stages of view events being fired. Since this is a relatively risky and unnecessary change to apply when GPU acceleration is off, it will only change behavior when useGpu is true. Fixes microsoft#237527
1 parent 748e34c commit a27b338

File tree

2 files changed

+14
-3
lines changed

2 files changed

+14
-3
lines changed

src/vs/editor/browser/view/viewLayer.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,9 +277,20 @@ export class VisibleLinesCollection<T extends IVisibleLine> {
277277
return false;
278278
}
279279

280-
public onFlushed(e: viewEvents.ViewFlushedEvent): boolean {
280+
public onFlushed(e: viewEvents.ViewFlushedEvent, flushDom?: boolean): boolean {
281+
// No need to clear the dom node because a full .innerHTML will occur in
282+
// ViewLayerRenderer._render, however the the fallbakc mechanism in the
283+
// GPU renderer may cause this to be necessary as the .innerHTML call
284+
// may not happen depending on the new state, leaving stale DOM nodes
285+
// around.
286+
if (flushDom) {
287+
const start = this._linesCollection.getStartLineNumber();
288+
const end = this._linesCollection.getEndLineNumber();
289+
for (let i = start; i <= end; i++) {
290+
this._linesCollection.getLine(i).getDomNode()?.remove();
291+
}
292+
}
281293
this._linesCollection.flush();
282-
// No need to clear the dom node because a full .innerHTML will occur in ViewLayerRenderer._render
283294
return true;
284295
}
285296

src/vs/editor/browser/viewParts/viewLines/viewLines.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ export class ViewLines extends ViewPart implements IViewLines {
254254
return true;
255255
}
256256
public override onFlushed(e: viewEvents.ViewFlushedEvent): boolean {
257-
const shouldRender = this._visibleLines.onFlushed(e);
257+
const shouldRender = this._visibleLines.onFlushed(e, this._viewLineOptions.useGpu);
258258
this._maxLineWidth = 0;
259259
return shouldRender;
260260
}

0 commit comments

Comments
 (0)