From cbe1d841ea094ec75f658594259f422383d5d425 Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Tue, 3 Nov 2026 23:30:10 +0400 Subject: [PATCH] Dispose the Navara bloom and sharpen passes when their effect is deleted EffectComposer.removePass does not dispose a pass. One withPassDisposal mixin now does it for the bloom, sharpen, styles and overlay effects. --- src/globe/navara/bloom.js | 3 +- src/globe/navara/frameDescriptor.js | 17 ++++++++++ src/globe/navara/frameDescriptor.test.mjs | 38 ++++++++++++++++++++++- src/globe/navara/overlay.js | 18 +++-------- src/globe/navara/sharpen.js | 6 ++-- src/globe/navara/stylePasses.js | 12 ++----- 6 files changed, 67 insertions(+), 27 deletions(-) diff --git a/src/globe/navara/bloom.js b/src/globe/navara/bloom.js index d7bb2b62e..44104514c 100644 --- a/src/globe/navara/bloom.js +++ b/src/globe/navara/bloom.js @@ -9,6 +9,7 @@ import { Effect as NavaraEffect, EffectDesc } from '@navaramap/three'; import { BloomEffect } from 'postprocessing'; import { createBloomPort } from './bloomSettings.js'; import { pipelinePassNames } from './internals.js'; +import { withPassDisposal } from './frameDescriptor.js'; export const BLOOM_EFFECT_KEY = 'gevBloom'; @@ -20,7 +21,7 @@ function apply(effect, settings) { } /** config.gevBloom: { settings } (bloomSettingsFromStrength). */ -class BloomEffectDesc extends EffectDesc { +class BloomEffectDesc extends withPassDisposal(EffectDesc) { static key = BLOOM_EFFECT_KEY; // Nothing anchors after it but the styles: before them, else before the // final copy (the styles anchor after it). diff --git a/src/globe/navara/frameDescriptor.js b/src/globe/navara/frameDescriptor.js index a45f976cf..771a6bfc2 100644 --- a/src/globe/navara/frameDescriptor.js +++ b/src/globe/navara/frameDescriptor.js @@ -16,3 +16,20 @@ export function withFrameUpdates(Base) { } }; } + +// Navara's EffectDesc.onDestroy takes its pass out of the composer, and +// EffectComposer.removePass does not dispose it: the pass's render targets, +// materials and shader programs would stay on the GPU for every effect +// added and deleted. A GEV effect disposes its own raw pass once removed. +export function withPassDisposal(Base) { + return class extends Base { + onDestroy() { + const pass = this.raw?.rawPass; + try { + super.onDestroy(); + } finally { + pass?.dispose(); + } + } + }; +} diff --git a/src/globe/navara/frameDescriptor.test.mjs b/src/globe/navara/frameDescriptor.test.mjs index 45142194b..32510815c 100644 --- a/src/globe/navara/frameDescriptor.test.mjs +++ b/src/globe/navara/frameDescriptor.test.mjs @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import test from 'node:test'; -import { withFrameUpdates } from './frameDescriptor.js'; +import { withFrameUpdates, withPassDisposal } from './frameDescriptor.js'; test('deleting a descriptor unsubscribes only that renderer from the warm view', () => { const frames = new Set(); @@ -41,3 +41,39 @@ test('deleting a descriptor unsubscribes only that renderer from the warm view', b.onDestroy(); assert.equal(frames.size, 0); }); + +test('a deleted effect disposes the pass the composer no longer holds', () => { + const events = []; + const pass = { dispose: () => events.push('dispose') }; + class Base { + _instance = { rawPass: pass }; + get raw() { + return this._instance; + } + onDestroy() { + events.push('remove'); + this._instance = undefined; + } + } + const effect = new (withPassDisposal(Base))(); + effect.onDestroy(); + assert.deepEqual(events, ['remove', 'dispose']); + + // A pass that was never created: nothing to dispose. + const empty = new (withPassDisposal( + class { + onDestroy() {} + }, + ))(); + assert.doesNotThrow(() => empty.onDestroy()); + + // The pass is disposed even when Navara's removal throws. + class Failing extends Base { + onDestroy() { + throw Error('removal failed'); + } + } + events.length = 0; + assert.throws(() => new (withPassDisposal(Failing))().onDestroy(), /removal/); + assert.deepEqual(events, ['dispose']); +}); diff --git a/src/globe/navara/overlay.js b/src/globe/navara/overlay.js index 3f8d7da1b..c9e182262 100644 --- a/src/globe/navara/overlay.js +++ b/src/globe/navara/overlay.js @@ -12,7 +12,7 @@ // FIRMS opts into a separate native-stage overlay before sharpening/AA. // Only that stage owns an encoded-RGBA8 blend target; W1 stays in place. -import { withFrameUpdates } from './frameDescriptor.js'; +import { withFrameUpdates, withPassDisposal } from './frameDescriptor.js'; import { EffectDesc, Pass as NavaraPass } from '@navaramap/three'; import { GevOverlayPass } from './overlayPass.js'; import { sonarUniformsFor } from './sonar.js'; @@ -22,7 +22,10 @@ export const OVERLAY_EFFECT_KEY = 'gevOverlay'; const PRE_SHARPEN_KEY = 'gevPreSharpenOverlay'; const registeredViews = new WeakMap(); -class GevOverlayEffectDesc extends withFrameUpdates(EffectDesc) { +// The pass owns its GPU target and context event handlers until disposed. +class GevOverlayEffectDesc extends withPassDisposal( + withFrameUpdates(EffectDesc), +) { static key = OVERLAY_EFFECT_KEY; // Navara takes the first existing anchor, so the latest pass comes first. static insertAfter = ['fxaa', 'smaa', 'gevSharpen', 'toneMapping']; @@ -45,17 +48,6 @@ class GevOverlayEffectDesc extends withFrameUpdates(EffectDesc) { ); } - onDestroy() { - // Navara removes the pass from its composer; our pass owns its GPU target - // and context event handlers until it is explicitly disposed. - const pass = this.raw?.rawPass; - try { - super.onDestroy(); - } finally { - pass?.dispose(); - } - } - // Each drawn frame: the scene depth the overlays test against, then each // overlay's own per-frame work (pixel footprint, origin, viewport). updateFrame(time) { diff --git a/src/globe/navara/sharpen.js b/src/globe/navara/sharpen.js index e83b810ff..4c44dfd0a 100644 --- a/src/globe/navara/sharpen.js +++ b/src/globe/navara/sharpen.js @@ -20,7 +20,7 @@ // has no public call to merge effects. A disabled pass is skipped: off costs // nothing. -import { withFrameUpdates } from './frameDescriptor.js'; +import { withFrameUpdates, withPassDisposal } from './frameDescriptor.js'; import { Effect as NavaraEffect, EffectDesc } from '@navaramap/three'; import { Effect, EffectAttribute } from 'postprocessing'; import { RGBADepthPacking, Uniform } from 'three'; @@ -87,7 +87,9 @@ class UnsharpMaskEffect extends Effect { } /** config.gevSharpen: { amount } (Cesium's `amount` uniform). */ -export class SharpenEffectDesc extends withFrameUpdates(EffectDesc) { +export class SharpenEffectDesc extends withPassDisposal( + withFrameUpdates(EffectDesc), +) { static key = SHARPEN_EFFECT_KEY; static insertAfter = ['toneMapping']; static insertBefore = ['smaa', 'fxaa', 'gevOverlay', 'final']; diff --git a/src/globe/navara/stylePasses.js b/src/globe/navara/stylePasses.js index 95379da53..ac7ab32c4 100644 --- a/src/globe/navara/stylePasses.js +++ b/src/globe/navara/stylePasses.js @@ -6,13 +6,14 @@ import { EffectDesc, Pass as NavaraPass } from '@navaramap/three'; import { pipelinePassNames } from './internals.js'; +import { withPassDisposal } from './frameDescriptor.js'; import { GevStylesPass, STYLES_EFFECT_KEY, createStylesPort, } from './stylePass.js'; -class StylesEffectDesc extends EffectDesc { +class StylesEffectDesc extends withPassDisposal(EffectDesc) { static key = STYLES_EFFECT_KEY; // Last before Navara's final copy: after bloom when it is on, else after // everything the scene and its overlays draw (nothing anchors after it). @@ -31,15 +32,6 @@ class StylesEffectDesc extends EffectDesc { // postprocessing copy: wrap it (as overlay.js does). return new NavaraPass(new GevStylesPass(this.controller), null); } - - onDestroy() { - const pass = this.raw?.rawPass; - try { - super.onDestroy(); - } finally { - pass?.dispose(); - } - } } /**