Skip to content

fix(renderer): track render target usage before cleaning them up, to fix prematurely unloading textures #2639

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Neptilo wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix(renderer): track render target usage before cleaning them up, to fix prematurely unloading textures #2639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
54849f6
fix(renderer): track and clean up render target usage to fix prematur…
Neptilo Nov 21, 2025
c6d30a4
refactor: isolate render target management in each view
Neptilo Dec 17, 2025
1407861
fix: unit tests
Neptilo Dec 17, 2025
9e7906a
fix: small but dangerous mistake (memory leak inducing)
Neptilo Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/Main/src/Core/TileMesh.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ class TileMesh extends THREE.Mesh<TileGeometry, LayeredMaterial> {
if (this.material.layersNeedUpdate) {
this.material.updateLayersUniforms(renderer);
}

// Track actual usage every time this mesh is rendered
// Use global current rendering view ID set by MainLoop
this.material.trackCurrentRenderTargetUsage();
}
}

Expand Down
42 changes: 41 additions & 1 deletion packages/Main/src/Layer/TiledGeometryLayer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import convertToTile from 'Converter/convertToTile';
import ObjectRemovalHelper from 'Process/ObjectRemovalHelper';
import { getColorLayersIdOrderedBySequence } from 'Layer/ImageryLayers';
import { CACHE_POLICIES } from 'Core/Scheduler/Cache';
import { LRUCache } from 'lru-cache';

const subdivisionVector = new THREE.Vector3();
const boundingSphereCenter = new THREE.Vector3();
Expand All @@ -17,6 +18,9 @@ const boundingSphereCenter = new THREE.Vector3();
* as it is used internally for optimisation.
* @property {boolean} hideSkirt (default false) - Used to hide the skirt (tile borders).
* Useful when the layer opacity < 1
* @property {Map<string, THREE.WebGLArrayRenderTarget>} pendingRtDisposal
* @property {Set<string>} usedRts
* @property {LRUCache<string, THREE.WebGLArrayRenderTarget>} rtCache
*
* @extends GeometryLayer
*/
Expand Down Expand Up @@ -148,6 +152,25 @@ class TiledGeometryLayer extends GeometryLayer {
*/
this.diffuse = diffuse;

/**
* @type {Map<string, THREE.WebGLArrayRenderTarget>}
*/
this.pendingRtDisposal = new Map();
/**
* @type {Set<string>}
*/
this.usedRts = new Set();

/**
* @type {LRUCache<string, THREE.WebGLArrayRenderTarget>}
*/
this.rtCache = new LRUCache({
max: 200,
dispose: (rt, key) => {
this.pendingRtDisposal.set(key, rt);
},
});
Comment on lines +155 to +172
Copy link
Contributor

@Desplandis Desplandis Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should have a dedicated class for all caches, I could think of a class like:

class RenderTargetCache {
    private cache: LRUCache<string, THREE.WebGLArrayRenderTarget>;
    private pendingDisposal = new Map<string, THREE.WebGLArrayRenderTarget>();
    private used = new Set<string>();
    
    constructor(maxSize = 200) {
        this.rtCache = new LRUCache({
            max: maxSize,
            dispose: (rt, key) => this.pendingDisposal.set(key, rt),
        });
    }
    
    get(id: string): THREE.WebGLArrayRenderTarget | undefined { ... }
    set(id: string, rt: THREE.WebGLArrayRenderTarget): void { ... }
    cleanup(): void { ... }
    dispose(): void { ... }
}


this.level0Nodes = [];
const promises = [];

Expand Down Expand Up @@ -251,6 +274,18 @@ class TiledGeometryLayer extends GeometryLayer {
// In future, the sequence must be returned by parent geometry layer.
this.colorLayersOrder = getColorLayersIdOrderedBySequence(context.colorLayers);

// Dispose render targets that are queued for disposal and not used by this view
if (this.usedRts.size) { // important: only clean up if last loop did rendering
Copy link
Contributor

@Desplandis Desplandis Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If done correctly, we should not need this no?

I know that we don't have a method called after rendering in the layer (maybe we should add a postRender method?) but this should clean the render target of the previous render.

for (const [id, renderTarget] of this.pendingRtDisposal) {
if (this.usedRts.has(id)) { continue; }
renderTarget.dispose();
this.pendingRtDisposal.delete(id);
}

// initialize render target usage tracking for next render
this.usedRts.clear();
}

Comment on lines +277 to +288
Copy link
Contributor

@Desplandis Desplandis Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good case for refactoring into a class. A cleanup method maybe?

let commonAncestor;
for (const source of sources.values()) {
if (source.isCamera) {
Expand Down Expand Up @@ -344,7 +379,12 @@ class TiledGeometryLayer extends GeometryLayer {
}

convert(requester, extent) {
return convertToTile.convert(requester, extent, this);
return convertToTile.convert(requester, extent, this).then((tileMesh) => {
tileMesh.material.rtCache = this.rtCache;
tileMesh.material.usedRts = this.usedRts;
tileMesh.material.pendingRtDisposal = this.pendingRtDisposal;
return tileMesh;
});
}

// eslint-disable-next-line
Expand Down
229 changes: 130 additions & 99 deletions packages/Main/src/Renderer/LayeredMaterial.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,103 +78,6 @@ const defaultStructLayers: Readonly<{
},
};

const rtCache = new LRUCache<string, THREE.WebGLArrayRenderTarget>({
max: 200,
dispose: (rt) => { rt.dispose(); },
});

/**
* Updates the uniforms for layered textures,
* including populating the DataArrayTexture
* with content from individual 2D textures on the GPU.
*
* @param uniforms - The uniforms object for your material.
* @param tiles - An array of RasterTile objects, each containing textures.
* @param max - The maximum number of layers for the DataArrayTexture.
* @param type - Layer set identifier: 'c' for color, 'e' for elevation.
* @param renderer - The renderer used to render textures.
*/
function updateLayersUniforms<Type extends 'c' | 'e'>(
uniforms: { [name: string]: THREE.IUniform },
tiles: RasterTile[],
max: number,
type: Type,
renderer: THREE.WebGLRenderer) {
// Aliases for readability
const uLayers = uniforms.layers.value;
const uTextures = uniforms.textures;
const uOffsetScales = uniforms.offsetScales.value;
const uTextureCount = uniforms.textureCount;

// Flatten the 2d array: [i, j] -> layers[_layerIds[i]].textures[j]
let count = 0;
let width = 0;
let height = 0;

// Determine total count of textures and dimensions
// (assuming all textures are same size)
let textureSetId: string = type;
for (const tile of tiles) {
// FIXME: RasterElevationTile are always passed to this function alone
// so this works, but it's really not great even ignoring the dynamic
// addition of a field.
// @ts-expect-error: adding field to passed layer
tile.textureOffset = count;

for (
let i = 0;
i < tile.textures.length && count < max;
++i, ++count
) {
const texture = tile.textures[i];
if (!texture) { continue; }

textureSetId += `${texture.id}.`;
uOffsetScales[count] = tile.offsetScales[i];
uLayers[count] = tile;

const img = texture.image;
if (!img || img.width <= 0 || img.height <= 0) {
console.error('Texture image not loaded or has zero dimensions');
uTextureCount.value = 0;
return;
} else if (count == 0) {
width = img.width;
height = img.height;
} else if (width !== img.width || height !== img.height) {
console.error('Texture dimensions mismatch');
uTextureCount.value = 0;
return;
}
}
}

const cachedRT = rtCache.get(textureSetId);
if (cachedRT) {
uTextures.value = cachedRT.texture;
uTextureCount.value = count;
return;
}

// Memory management of these textures is only done by `textureArraysCache`,
// so we don't have to dispose of them manually.
const rt = makeDataArrayRenderTarget(width, height, count, tiles, max, renderer);
if (!rt) {
uTextureCount.value = 0;
return;
}

rtCache.set(textureSetId, rt);
uniforms.textures.value = rt.texture;

if (count > max) {
console.warn(
`LayeredMaterial: Not enough texture units (${max} < ${count}), `
+ 'excess textures have been discarded.',
);
}
uTextureCount.value = count;
}

export const ELEVATION_MODES = {
RGBA: 0,
Expand Down Expand Up @@ -298,6 +201,11 @@ export class LayeredMaterial extends THREE.ShaderMaterial {

public layersNeedUpdate: boolean;

// Deferred disposal data structures
public pendingRtDisposal: LRUCache<string, THREE.WebGLArrayRenderTarget> | undefined;
public usedRts: Set<string> | undefined;
public rtCache: LRUCache<string, THREE.WebGLArrayRenderTarget> | undefined;

public override defines: LayeredMaterialDefines;

constructor(options: LayeredMaterialParameters = {}, crsCount: number) {
Expand Down Expand Up @@ -481,14 +389,121 @@ export class LayeredMaterial extends THREE.ShaderMaterial {
};
}

/**
* Updates the uniforms for layered textures of a specific type,
* including populating the DataArrayTexture
* with content from individual 2D textures on the GPU.
*
* @param uniforms - The uniforms object for your material.
* @param tiles - An array of RasterTile objects, each containing textures.
* @param max - The maximum number of layers for the DataArrayTexture.
* @param type - Layer set identifier: 'c' for color, 'e' for elevation.
* @param renderer - The renderer used to render textures.
*/
private _updateLayersUniformsForType<Type extends 'c' | 'e'>(
Copy link
Contributor

@Desplandis Desplandis Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you just add the corresponding cache parameter instead of moving the function to a new method, this breaks the diff of the code?

uniforms: { [name: string]: THREE.IUniform },
tiles: RasterTile[],
max: number,
type: Type,
renderer: THREE.WebGLRenderer) {
// Aliases for readability
const uLayers = uniforms.layers.value;
const uTextures = uniforms.textures;
const uOffsetScales = uniforms.offsetScales.value;
const uTextureCount = uniforms.textureCount;

// Flatten the 2d array: [i, j] -> layers[_layerIds[i]].textures[j]
let count = 0;
let width = 0;
let height = 0;

// Determine total count of textures and dimensions
// (assuming all textures are same size)
let textureSetId: string = type;
for (const tile of tiles) {
// FIXME: RasterElevationTile are always passed to this function
// alone so this works, but it's really not great even ignoring
// the dynamic addition of a field.
// @ts-expect-error: adding field to passed layer
tile.textureOffset = count;

for (
let i = 0;
i < tile.textures.length && count < max;
++i, ++count
) {
const texture = tile.textures[i];
if (!texture) { continue; }

textureSetId += `${texture.id}.`;
uOffsetScales[count] = tile.offsetScales[i];
uLayers[count] = tile;

const img = texture.image;
if (!img || img.width <= 0 || img.height <= 0) {
console.error('Texture image not loaded or has zero dimensions');
uTextureCount.value = 0;
return;
} else if (count == 0) {
width = img.width;
height = img.height;
} else if (width !== img.width || height !== img.height) {
console.error('Texture dimensions mismatch');
uTextureCount.value = 0;
return;
}
}
}

let cachedRT = this.rtCache?.get(textureSetId);

if (!cachedRT) {
cachedRT = this.pendingRtDisposal?.get(textureSetId);
if (cachedRT) {
this.rtCache!.set(textureSetId, cachedRT);
this.pendingRtDisposal!.delete(textureSetId);
Comment on lines +463 to +464
Copy link
Contributor

@Desplandis Desplandis Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good case for factoring the two caches & marked targets, you will not have to use the ! operator!

}
}
Comment on lines +458 to +466
Copy link
Contributor

@Desplandis Desplandis Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I infer from this code:

  • it fetches a render target from the cache
  • if not present, it checks whether it was marked as disposed and in such case cache it again

I think this code would benefit to be factored within a class (within a get: string -> RenderTarget method?). Moreover, if in cache it should be marked as used.


if (cachedRT) {
uTextures.value = cachedRT.texture;
uTextureCount.value = count;
return;
}

const rt = makeDataArrayRenderTarget(width, height, count, tiles, max, renderer);
if (!rt) {
uTextureCount.value = 0;
return;
}

this.rtCache?.set(textureSetId, rt);
rt.texture.userData = { textureSetId };
Copy link
Contributor

@Desplandis Desplandis Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this is marking this render target as used for this frame. Why not adding it directly to the set of used render targets (i.e. usedRts)?

Comment on lines +480 to +481
Copy link
Contributor

@Desplandis Desplandis Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, this code would benefit to be factored within a class (within a set: string -> RenderTarget -> void method?).

uniforms.textures.value = rt.texture;

if (count > max) {
console.warn(
`LayeredMaterial: Not enough texture units (${max} < ${count}), `
+ 'excess textures have been discarded.',
);
}
uTextureCount.value = count;
}

/**
* Updates uniforms for both color and elevation layers.
* Orchestrates the processing of visible color layers and elevation tiles.
*
* @param renderer - The renderer used to render textures into arrays.
*/
public updateLayersUniforms(renderer: THREE.WebGLRenderer): void {
const colorlayers = this.colorTiles
.filter(rt => rt.visible && rt.opacity > 0);
colorlayers.sort((a, b) =>
this.colorTileIds.indexOf(a.id) - this.colorTileIds.indexOf(b.id),
);

updateLayersUniforms(
this._updateLayersUniformsForType(
this.getLayerUniforms('color'),
colorlayers,
this.defines.NUM_FS_TEXTURES,
Expand All @@ -498,7 +513,7 @@ export class LayeredMaterial extends THREE.ShaderMaterial {

if (this.elevationTileId !== undefined && this.getElevationTile()) {
if (this.elevationTile !== undefined) {
updateLayersUniforms(
this._updateLayersUniformsForType(
this.getLayerUniforms('elevation'),
[this.elevationTile],
this.defines.NUM_VS_TEXTURES,
Expand All @@ -511,6 +526,22 @@ export class LayeredMaterial extends THREE.ShaderMaterial {
this.layersNeedUpdate = false;
}

/**
* Track usage of current render targets for deferred disposal.
* Should be called every time this material is rendered.
*/
public trackCurrentRenderTargetUsage(): void {
const colorTextures = this.getUniform('colorTextures');
if (colorTextures?.userData?.textureSetId) {
this.usedRts!.add(colorTextures.userData.textureSetId);
}

const elevationTextures = this.getUniform('elevationTextures');
if (elevationTextures?.userData?.textureSetId) {
this.usedRts!.add(elevationTextures.userData.textureSetId);
}
}

public dispose(): void {
this.dispatchEvent({ type: 'dispose' });

Expand Down