From cbe2b7bd15a26970f1153f73ad6e420fe600bd9b Mon Sep 17 00:00:00 2001 From: AzeleaLee Date: Mon, 17 Aug 2026 07:33:38 +0800 Subject: [PATCH 1/2] fix: dispose renderer-owned geometry in SparkRenderer --- src/SparkRenderer.ts | 3 ++ test/SparkRenderer.test.ts | 65 +++++++++++++++++++++++ test/glsl-loader.mjs | 104 +++++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 test/SparkRenderer.test.ts create mode 100644 test/glsl-loader.mjs diff --git a/src/SparkRenderer.ts b/src/SparkRenderer.ts index 25434513..762ecdd0 100644 --- a/src/SparkRenderer.ts +++ b/src/SparkRenderer.ts @@ -326,6 +326,7 @@ export class SparkRenderer extends THREE.Mesh { readonly renderer: THREE.WebGLRenderer; readonly material: THREE.ShaderMaterial; readonly uniforms: ReturnType; + private readonly ownedGeometry: SplatGeometry; autoUpdate: boolean; preUpdate: boolean; @@ -487,6 +488,7 @@ export class SparkRenderer extends THREE.Mesh { }); super(geometry, material); + this.ownedGeometry = geometry; this.material = material; this.uniforms = uniforms; // Disable frustum culling because we want to always draw them all @@ -674,6 +676,7 @@ export class SparkRenderer extends THREE.Mesh { dispose() { // @ts-ignore Object3D has a dispose method in Three.js >= r186 super.dispose?.(); + this.ownedGeometry.dispose(); if (this.target) { this.target.dispose(); diff --git a/test/SparkRenderer.test.ts b/test/SparkRenderer.test.ts new file mode 100644 index 00000000..f05a4226 --- /dev/null +++ b/test/SparkRenderer.test.ts @@ -0,0 +1,65 @@ +import assert from "node:assert/strict"; +import { register } from "node:module"; +import test from "node:test"; +import * as THREE from "three"; + +register(new URL("./glsl-loader.mjs", import.meta.url)); + +const { SparkRenderer } = await import("../src/SparkRenderer.js"); + +function makeRendererStub(): THREE.WebGLRenderer { + return { + getContext() { + return { + getExtension() { + return null; + }, + }; + }, + } as THREE.WebGLRenderer; +} + +test("SparkRenderer.dispose releases its internal geometry", () => { + const spark = new SparkRenderer({ renderer: makeRendererStub() }); + const geometry = spark.geometry; + let disposeEvents = 0; + + geometry.addEventListener("dispose", () => { + disposeEvents += 1; + }); + + spark.dispose(); + + assert.equal(disposeEvents, 1); +}); + +test("SparkRenderer.dispose does not dispose geometry assigned from outside", () => { + const spark = new SparkRenderer({ renderer: makeRendererStub() }); + const internalGeometry = spark.geometry; + const externalGeometry = new THREE.BufferGeometry(); + let internalDisposeEvents = 0; + let externalDisposeEvents = 0; + + internalGeometry.addEventListener("dispose", () => { + internalDisposeEvents += 1; + }); + externalGeometry.addEventListener("dispose", () => { + externalDisposeEvents += 1; + }); + + spark.geometry = externalGeometry; + spark.dispose(); + + assert.equal(internalDisposeEvents, 1); + assert.equal(externalDisposeEvents, 0); +}); + +test("SparkRenderer.dispose is safe to call twice", () => { + const spark = new SparkRenderer({ renderer: makeRendererStub() }); + + spark.dispose(); + + assert.doesNotThrow(() => { + spark.dispose(); + }); +}); diff --git a/test/glsl-loader.mjs b/test/glsl-loader.mjs new file mode 100644 index 00000000..06e2240b --- /dev/null +++ b/test/glsl-loader.mjs @@ -0,0 +1,104 @@ +import { readFile } from "node:fs/promises"; + +export async function resolve(specifier, context, nextResolve) { + if (specifier === "spark-rs") { + return { + shortCircuit: true, + url: "spark-rs:module", + }; + } + + if (specifier === "spark-rs/spark_rs_bg.wasm?arraybuffer&base64") { + return { + shortCircuit: true, + url: "spark-rs:wasm", + }; + } + + if (specifier.endsWith("?worker&inline")) { + return { + shortCircuit: true, + url: "spark:worker", + }; + } + + if (specifier.endsWith(".glsl")) { + return { + shortCircuit: true, + url: new URL(specifier, context.parentURL).href, + }; + } + + return nextResolve(specifier, context); +} + +export async function load(url, context, nextLoad) { + if (url === "spark-rs:module") { + return { + format: "module", + shortCircuit: true, + source: ` + const unimplemented = () => { + throw new Error("spark-rs test shim should not be executed"); + }; + export default async function init_wasm() {} + export const sort_splats = unimplemented; + export const sort32_splats = unimplemented; + export const decode_to_gsplatarray = unimplemented; + export const decode_to_csplatarray = unimplemented; + export const decode_to_packedsplats = unimplemented; + export const new_lod_tree = unimplemented; + export const new_shared_lod_tree = unimplemented; + export const init_lod_tree = unimplemented; + export const dispose_lod_tree = unimplemented; + export const traverse_lod_trees = unimplemented; + export const dynamic_traverse_lod_trees = unimplemented; + export const tiny_lod_packedsplats = unimplemented; + export const bhatt_lod_packedsplats = unimplemented; + export const update_lod_trees = unimplemented; + export const decode_to_extsplats = unimplemented; + export const tiny_lod_extsplats = unimplemented; + export const bhatt_lod_extsplats = unimplemented; + export const get_lod_tree_level = unimplemented; + export const get_raycast_buffer = unimplemented; + export const get_raycast_buffer2 = unimplemented; + export const raycast_ext_buffers = unimplemented; + export const raycast_packed_buffer = unimplemented; + export const decode_rad_header = unimplemented; + `, + }; + } + + if (url === "spark-rs:wasm") { + return { + format: "module", + shortCircuit: true, + source: "export default new Uint8Array([0,97,115,109,1,0,0,0]).buffer;", + }; + } + + if (url === "spark:worker") { + return { + format: "module", + shortCircuit: true, + source: ` + export default class BundledWorker { + postMessage() {} + terminate() {} + } + `, + }; + } + + if (url.endsWith(".glsl")) { + const source = await readFile(new URL(url), "utf8"); + + return { + format: "module", + shortCircuit: true, + source: `export default ${JSON.stringify(source)};`, + }; + } + + return nextLoad(url, context); +} From 6030f83e7c77745632d11b2fdfff880dc230f2ad Mon Sep 17 00:00:00 2001 From: Oscar Lorentzon Date: Wed, 9 Sep 2026 15:17:49 -0700 Subject: [PATCH 2/2] Simplify to geometry and material dispose, remove tests --- src/SparkRenderer.ts | 6 +-- test/SparkRenderer.test.ts | 65 ----------------------- test/glsl-loader.mjs | 104 ------------------------------------- 3 files changed, 3 insertions(+), 172 deletions(-) delete mode 100644 test/SparkRenderer.test.ts delete mode 100644 test/glsl-loader.mjs diff --git a/src/SparkRenderer.ts b/src/SparkRenderer.ts index 762ecdd0..eea1d7b3 100644 --- a/src/SparkRenderer.ts +++ b/src/SparkRenderer.ts @@ -326,7 +326,6 @@ export class SparkRenderer extends THREE.Mesh { readonly renderer: THREE.WebGLRenderer; readonly material: THREE.ShaderMaterial; readonly uniforms: ReturnType; - private readonly ownedGeometry: SplatGeometry; autoUpdate: boolean; preUpdate: boolean; @@ -488,7 +487,6 @@ export class SparkRenderer extends THREE.Mesh { }); super(geometry, material); - this.ownedGeometry = geometry; this.material = material; this.uniforms = uniforms; // Disable frustum culling because we want to always draw them all @@ -676,7 +674,9 @@ export class SparkRenderer extends THREE.Mesh { dispose() { // @ts-ignore Object3D has a dispose method in Three.js >= r186 super.dispose?.(); - this.ownedGeometry.dispose(); + + this.geometry.dispose(); + this.material.dispose(); if (this.target) { this.target.dispose(); diff --git a/test/SparkRenderer.test.ts b/test/SparkRenderer.test.ts deleted file mode 100644 index f05a4226..00000000 --- a/test/SparkRenderer.test.ts +++ /dev/null @@ -1,65 +0,0 @@ -import assert from "node:assert/strict"; -import { register } from "node:module"; -import test from "node:test"; -import * as THREE from "three"; - -register(new URL("./glsl-loader.mjs", import.meta.url)); - -const { SparkRenderer } = await import("../src/SparkRenderer.js"); - -function makeRendererStub(): THREE.WebGLRenderer { - return { - getContext() { - return { - getExtension() { - return null; - }, - }; - }, - } as THREE.WebGLRenderer; -} - -test("SparkRenderer.dispose releases its internal geometry", () => { - const spark = new SparkRenderer({ renderer: makeRendererStub() }); - const geometry = spark.geometry; - let disposeEvents = 0; - - geometry.addEventListener("dispose", () => { - disposeEvents += 1; - }); - - spark.dispose(); - - assert.equal(disposeEvents, 1); -}); - -test("SparkRenderer.dispose does not dispose geometry assigned from outside", () => { - const spark = new SparkRenderer({ renderer: makeRendererStub() }); - const internalGeometry = spark.geometry; - const externalGeometry = new THREE.BufferGeometry(); - let internalDisposeEvents = 0; - let externalDisposeEvents = 0; - - internalGeometry.addEventListener("dispose", () => { - internalDisposeEvents += 1; - }); - externalGeometry.addEventListener("dispose", () => { - externalDisposeEvents += 1; - }); - - spark.geometry = externalGeometry; - spark.dispose(); - - assert.equal(internalDisposeEvents, 1); - assert.equal(externalDisposeEvents, 0); -}); - -test("SparkRenderer.dispose is safe to call twice", () => { - const spark = new SparkRenderer({ renderer: makeRendererStub() }); - - spark.dispose(); - - assert.doesNotThrow(() => { - spark.dispose(); - }); -}); diff --git a/test/glsl-loader.mjs b/test/glsl-loader.mjs deleted file mode 100644 index 06e2240b..00000000 --- a/test/glsl-loader.mjs +++ /dev/null @@ -1,104 +0,0 @@ -import { readFile } from "node:fs/promises"; - -export async function resolve(specifier, context, nextResolve) { - if (specifier === "spark-rs") { - return { - shortCircuit: true, - url: "spark-rs:module", - }; - } - - if (specifier === "spark-rs/spark_rs_bg.wasm?arraybuffer&base64") { - return { - shortCircuit: true, - url: "spark-rs:wasm", - }; - } - - if (specifier.endsWith("?worker&inline")) { - return { - shortCircuit: true, - url: "spark:worker", - }; - } - - if (specifier.endsWith(".glsl")) { - return { - shortCircuit: true, - url: new URL(specifier, context.parentURL).href, - }; - } - - return nextResolve(specifier, context); -} - -export async function load(url, context, nextLoad) { - if (url === "spark-rs:module") { - return { - format: "module", - shortCircuit: true, - source: ` - const unimplemented = () => { - throw new Error("spark-rs test shim should not be executed"); - }; - export default async function init_wasm() {} - export const sort_splats = unimplemented; - export const sort32_splats = unimplemented; - export const decode_to_gsplatarray = unimplemented; - export const decode_to_csplatarray = unimplemented; - export const decode_to_packedsplats = unimplemented; - export const new_lod_tree = unimplemented; - export const new_shared_lod_tree = unimplemented; - export const init_lod_tree = unimplemented; - export const dispose_lod_tree = unimplemented; - export const traverse_lod_trees = unimplemented; - export const dynamic_traverse_lod_trees = unimplemented; - export const tiny_lod_packedsplats = unimplemented; - export const bhatt_lod_packedsplats = unimplemented; - export const update_lod_trees = unimplemented; - export const decode_to_extsplats = unimplemented; - export const tiny_lod_extsplats = unimplemented; - export const bhatt_lod_extsplats = unimplemented; - export const get_lod_tree_level = unimplemented; - export const get_raycast_buffer = unimplemented; - export const get_raycast_buffer2 = unimplemented; - export const raycast_ext_buffers = unimplemented; - export const raycast_packed_buffer = unimplemented; - export const decode_rad_header = unimplemented; - `, - }; - } - - if (url === "spark-rs:wasm") { - return { - format: "module", - shortCircuit: true, - source: "export default new Uint8Array([0,97,115,109,1,0,0,0]).buffer;", - }; - } - - if (url === "spark:worker") { - return { - format: "module", - shortCircuit: true, - source: ` - export default class BundledWorker { - postMessage() {} - terminate() {} - } - `, - }; - } - - if (url.endsWith(".glsl")) { - const source = await readFile(new URL(url), "utf8"); - - return { - format: "module", - shortCircuit: true, - source: `export default ${JSON.stringify(source)};`, - }; - } - - return nextLoad(url, context); -}