Fix shared-atlas garble: track page changes per renderer, not consume-once
TextureAtlas.beginFrame() returned a consume-once boolean, so after a page merge only the FIRST renderer to draw rebuilt its vertex model. Terminals sharing the atlas via CharAtlasCache kept stale texture-page indexes/coords and drew garbled glyphs on their next partial render (vscode#322756). Replace the flag with a monotonic TextureAtlas.pagesVersion, bumped on page merges and overflow-page creation. Each GlyphRenderer tracks its own last-seen value in beginFrame(), so every terminal sharing the atlas rebuilds its model independently. setAtlas() resets the tracker to force a rebuild when the atlas instance changes. WebglSharedAtlasGarble now passes (was diff ~130/255 vs tolerance 14); full webgl addon suite 74/74 green.
This commit is contained in:
@@ -100,6 +100,7 @@ export class GlyphRenderer extends Disposable {
|
||||
private readonly _attributesBuffer: WebGLBuffer;
|
||||
|
||||
private _atlas: ITextureAtlas | undefined;
|
||||
private _lastAtlasPagesVersion: number = -1;
|
||||
private _activeBuffer: number = 0;
|
||||
private readonly _vertices: IVertices = {
|
||||
count: 0,
|
||||
@@ -213,8 +214,21 @@ export class GlyphRenderer extends Disposable {
|
||||
this.handleResize();
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns whether the full model must be rebuilt before rendering this frame.
|
||||
* This is tracked per renderer (not on the shared atlas) so that every
|
||||
* terminal sharing the atlas rebuilds its model after a page merge, not just
|
||||
* the first one to render (vscode#322756).
|
||||
*/
|
||||
public beginFrame(): boolean {
|
||||
return this._atlas ? this._atlas.beginFrame() : true;
|
||||
if (!this._atlas) {
|
||||
return true;
|
||||
}
|
||||
if (this._atlas.pagesVersion !== this._lastAtlasPagesVersion) {
|
||||
this._lastAtlasPagesVersion = this._atlas.pagesVersion;
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
public updateCell(x: number, y: number, code: number, bg: number, fg: number, ext: number, chars: string, width: number, lastBg: number): void {
|
||||
@@ -375,6 +389,9 @@ export class GlyphRenderer extends Disposable {
|
||||
|
||||
public setAtlas(atlas: ITextureAtlas): void {
|
||||
this._atlas = atlas;
|
||||
// Force a model rebuild on the next frame as glyph coordinates from a
|
||||
// previous atlas are meaningless in the new one.
|
||||
this._lastAtlasPagesVersion = -1;
|
||||
this.invalidateAtlasTextures();
|
||||
}
|
||||
|
||||
|
||||
@@ -132,12 +132,16 @@ export class TextureAtlas implements ITextureAtlas {
|
||||
}
|
||||
}
|
||||
|
||||
private _requestClearModel = false;
|
||||
public beginFrame(): boolean {
|
||||
const result = this._requestClearModel;
|
||||
this._requestClearModel = false;
|
||||
return result;
|
||||
}
|
||||
/**
|
||||
* Incremented whenever the structure of the pages array changes in a way that
|
||||
* invalidates texture page indexes/coordinates cached outside the atlas (page
|
||||
* merges, overflow page creation). Renderers compare this against their own
|
||||
* last-seen value to decide when to rebuild their models; the atlas is shared
|
||||
* across terminals, so a consume-once flag would only repair the first
|
||||
* renderer to draw (see the shared-atlas garble in vscode#322756).
|
||||
*/
|
||||
private _pagesVersion = 0;
|
||||
public get pagesVersion(): number { return this._pagesVersion; }
|
||||
|
||||
public clearTexture(): void {
|
||||
if (this._pages[0].currentRow.x === 0 && this._pages[0].currentRow.y === 0) {
|
||||
@@ -206,8 +210,8 @@ export class TextureAtlas implements ITextureAtlas {
|
||||
// Add the new merged page to the end
|
||||
this.pages.push(mergedPage);
|
||||
|
||||
// Request the model to be cleared to refresh all texture pages.
|
||||
this._requestClearModel = true;
|
||||
// Invalidate models so all texture pages are refreshed.
|
||||
this._pagesVersion++;
|
||||
this._onAddTextureAtlasCanvas.fire(mergedPage.canvas);
|
||||
}
|
||||
|
||||
@@ -818,8 +822,8 @@ export class TextureAtlas implements ITextureAtlas {
|
||||
this._overflowSizePage = new AtlasPage(this._document, this._config.deviceMaxTextureSize);
|
||||
this.pages.push(this._overflowSizePage);
|
||||
|
||||
// Request the model to be cleared to refresh all texture pages.
|
||||
this._requestClearModel = true;
|
||||
// Invalidate models so all texture pages are refreshed.
|
||||
this._pagesVersion++;
|
||||
this._onAddTextureAtlasCanvas.fire(this._overflowSizePage.canvas);
|
||||
}
|
||||
activePage = this._overflowSizePage;
|
||||
|
||||
@@ -68,10 +68,13 @@ export interface ITextureAtlas extends IDisposable {
|
||||
warmUp(): void;
|
||||
|
||||
/**
|
||||
* Call when a frame is being drawn, this will return true if the atlas was cleared to make room
|
||||
* for a new set of glyphs.
|
||||
* Incremented whenever the pages array changes structurally (page merge,
|
||||
* overflow page creation), invalidating any texture page indexes/coordinates
|
||||
* cached by renderers. Tracked as a version rather than a consumable flag
|
||||
* because the atlas may be shared by multiple renderers, each of which must
|
||||
* notice the change independently.
|
||||
*/
|
||||
beginFrame(): boolean;
|
||||
readonly pagesVersion: number;
|
||||
|
||||
/**
|
||||
* Clear all glyphs from the texture atlas.
|
||||
|
||||
Reference in New Issue
Block a user