Skip to content
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.

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

Disposing scene background texture does not remove the geometry #26897

Closed
fromtheghost opened this issue Oct 4, 2023 · 5 comments
Closed

Disposing scene background texture does not remove the geometry #26897

fromtheghost opened this issue Oct 4, 2023 · 5 comments

Comments

@fromtheghost
Copy link

Description

When a scene background is set to a Texture, /src/renderers/webgl/WebGLBackground.js creates a mesh:

planeMesh = new Mesh(
	new PlaneGeometry( 2, 2 ),
	new ShaderMaterial( {
		name: 'BackgroundMaterial',
		uniforms: cloneUniforms( ShaderLib.background.uniforms ),
		vertexShader: ShaderLib.background.vertexShader,
		fragmentShader: ShaderLib.background.fragmentShader,
		side: FrontSide,
		depthTest: false,
		depthWrite: false,
		fog: false
	} )
);

However there doesn't seem to be any way to dispose of that PlaneGeometry. Calling scene.background.dispose() only disposes of the Texture.

Here's an example: https://codesandbox.io/s/still-voice-zwrpsn?file=/src/index.mjs

The scene has a cube and a background texture. When you execute dispose() in the gui, you are still left with 1 geometry hanging around.

Reproduction steps

  1. Create a scene and set scene.background to a Texture.
  2. Check renderer.info.memory and verify there is 1 geometry and 1 texture.
  3. Call scene.background.dispose() and see that there is still 1 geometry in memory.

Code

import * as THREE from "three";
import GUI from "lil-gui";

// Set up the scene, camera, and renderer
const scene = new THREE.Scene();
const backgroundTexture = new THREE.TextureLoader().load(
	"https://raw.githubusercontent.com/mrdoob/three.js/dev/examples/textures/water.jpg"
);
scene.background = backgroundTexture;
const camera = new THREE.PerspectiveCamera(
	75,
	window.innerWidth / window.innerHeight,
	0.1,
	1000
);
camera.position.z = 5;

const renderer = new THREE.WebGLRenderer();
renderer.setSize(window.innerWidth, window.innerHeight);
document.body.appendChild(renderer.domElement);

// Create a cube with a material that uses an envMap
const geometry = new THREE.BoxGeometry();

const material = new THREE.MeshNormalMaterial();
const cube = new THREE.Mesh(geometry, material);
scene.add(cube);

// Add a resize listener to update camera aspect ratio and renderer size
window.addEventListener("resize", function () {
	const newWidth = window.innerWidth;
	const newHeight = window.innerHeight;
	camera.aspect = newWidth / newHeight;
	camera.updateProjectionMatrix();
	renderer.setSize(newWidth, newHeight);
});

const panel = document.createElement("div");
//absolute top left position
panel.style.position = "absolute";
panel.style.top = "0px";
panel.style.left = "0px";
panel.style.backgroundColor = "black";
panel.style.padding = "5px";
panel.style.color = "white";
//add panel to document
document.body.appendChild(panel);

panel.update = () => {
	//show the renderer.info.memory stats
	panel.innerHTML = JSON.stringify(renderer.info.memory, null, 2);
};

// Render the scene
function animate() {
	requestAnimationFrame(animate);
	cube.rotation.x += 0.01;
	cube.rotation.y += 0.01;
	renderer.render(scene, camera);
	panel.update();
}
animate();

const gui = new GUI();
const guiObject = {
	dispose: () => {
		material.dispose();
		geometry.dispose();
		scene.background = null;
		backgroundTexture.dispose();
		scene.remove(cube);
	},
};

gui.add(guiObject, "dispose");

Live example

Screenshots

No response

Version

r157

Device

No response

Browser

No response

OS

No response

@gkjohnson
Copy link
Collaborator

you are still left with 1 geometry hanging around.

What problems does this cause and why do you need this 1 piece of reusable internal geometry to be disposed?

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 4, 2023

There are certain features of the engine that require to allocate internal resources (e.g. for env map conversions or backgrounds). For performance reasons, it makes sense to keep certain resources allocated. E.g. for the case when the user decides to replace a background texture with a new one.

As long as there is no memory leak we do not consider this issue as a bug.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 4, 2023

BTW: We could hide internal resources from renderer.info but then user will report mismatches when they use tools like Spector.js to compared the allocated WebGL resources and what three.js reports. So there is no way to make this perfect.

I personally prefer the current way. Meaning renderer.info reports what resources the engine allocates in total and not only what the user explicitly creates on app level.

@fromtheghost
Copy link
Author

No worries - it came up while I was in the process of trying to manage memory in a more complex environment that involved clearing and loading new scenes. I got stuck on why I couldn't get rid of everything in the renderer.info.memory object, and discovered it was the scene.background.

I thought it probably was a miniscule amount of memory but thought I'd report it in case others ran into the same issues, and might save them the several hours I spent tracking it down.

@Mugen87 Mugen87 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 4, 2023
@stevexbritton
Copy link

Hi, Even after disposing of the renderer the geometry and "BackgroundMaterial" shader still exist according to renderer.info. If I create a new renderer and a new scene are these same resources reused, cleanup or leaked?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants