webgl: stop the link render layer from acquiring a texture atlas - #6156
Open
andylizf wants to merge 1 commit into
Open
webgl: stop the link render layer from acquiring a texture atlas#6156andylizf wants to merge 1 commit into
andylizf wants to merge 1 commit into
Conversation
BaseRenderLayer acquired a texture atlas on every resize, colour change and transparency change, passing a hard-coded 2048 as the device max texture size. Nothing in the layer ever read that atlas: LinkRenderLayer, its only subclass, draws underlines with fillRect and never touches _charAtlas. Since xtermjs#5929 made deviceMaxTextureSize part of atlas cache equality, that call stopped being a harmless cache hit. On any GPU whose MAX_TEXTURE_SIZE is not 2048, the layer's config never equals the renderer's, so acquireTextureAtlas released the terminal from its real atlas -- disposing it outright when the terminal was the sole owner -- and built a 2048 one, after which the renderer's own refresh disposed that and rebuilt the real one. WebglRenderer.handleResize calls the layer's resize before its own atlas refresh, so every terminal resize paid this twice over, and embedders that resize on tab switch (VS Code does) paid it on every tab switch. Remove the acquisition. The two integration tests pin the renderer to the same atlas instance across resizes and across an unchanged theme reassignment; both fail on the previous code with the atlas replaced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BaseRenderLayerin the WebGL addon acquires a texture atlas on every resize, colour change and transparency change, passing a hard-coded2048as the device max texture size. Nothing in the layer reads that atlas:LinkRenderLayer, its only subclass, draws underlines withfillRectand never touches_charAtlas.Since #5929 added
deviceMaxTextureSizetoconfigEquals, that acquisition stopped being a cache hit. On any GPU whoseMAX_TEXTURE_SIZEis not 2048, the layer's config never equals the renderer's, soacquireTextureAtlas:atlas.dispose()when the terminal is the sole owner;deviceMaxTextureSize: 2048and warms it up;WebglRenderer._refreshCharAtlaswith the real size, which disposes the 2048 atlas and rebuilds the real one.WebglRenderer.handleResizecallslayer.resize()before its own_refreshCharAtlas(), so every terminal resize pays this in full. Embedders that resize on tab switch (VS Code does, viaTerminalInstance._resize→xterm.resize) pay it on every tab switch.Each cycle discards two atlases' worth of 512×512 page canvases and a warm-up of 93 glyphs. On a VS Code session with ~30 CJK-heavy terminals, the GPU process's IOSurface count climbs with tab switches and heavy output and only comes back on a full atlas evict; this PR removes one of the sources feeding that.
Changes
BaseRenderLayer.ts: remove_charAtlas,_refreshCharAtlasand its three call sites (constructor colour listener,_setTransparency,resize), plus the now-unused imports. −24 lines, no behaviour change for the link layer, which never used the atlas.test/WebglLinkLayerAtlas.test.ts: two Playwright tests that tag the renderer's atlas and wrap itsdispose, then assert the same instance survives (a) four resizes and (b) an unchanged theme reassignment, withdisposenever called. Both fail on master with the atlas replaced.Testing
npm run test-unit: 2403 passingnpm run lint: cleannpm run test-integration --suite=addon-webgl(Chromium): 64 passed, 9 skipped (Firefox/WebKit projects)Why this is low risk
The removed code produced a value nothing consumed.
acquireTextureAtlas/removeTerminalFromCachepairing stays withWebglRenderer, which already owns both sides.LinkRenderLayerkeeps its own canvas and itsonChangeColors→reset()path.Related: #5929 (made the mismatch observable), #6074 (atlas growth from per-cell background colours, a separate source), microsoft/vscode#329118 (maintainer trace attributing renderer load to rapid atlas invalidation).