search

godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.04k stars 21.18k forks source link

[3.x][GLES3] CPUParticles can cause corruption of the BVH space partitioning #65841

Closed m4nu3lf closed 2 years ago

m4nu3lf commented 2 years ago

Godot version

4ff41cc4f8430b115d3b5d7f584842e44deddb3c

System information

OS: Arch Linux, CPU: Ryzen 5, GPU: RX 6600

Issue description

Changing the number of particles can cause some NaN or huge floating point numbers to be used in the particle transforms. Using the octree-based space partitioning will result in error messages but no issue. However, using the BVH-based space partitioning can result in total corruption of the BVH structure with consequent artefacts (culling will break with meshes disappearing in front of the camera).

~I believe this also can happen when the particles are instantiated and added to the scene (this would be the scenario in my main project).~ [EDIT: it seems there was a place where I was setting the particle count just after adding the particles to the scene, removing that line fixes the issue]

I did some debugging, and it seems to me that the cause is CPUParticles forcing a multimesh update before data is initialized on the visual server.

Steps to reproduce

Continuously change the number of particles in a CPUParticle node during the idle and physics updates.

Minimal reproduction project

test_cpu_particles.zip

lawnjelly commented 2 years ago

I'm not sure I understand the MRP. Is there something needed to be done to trigger the bug? There doesn't appear to be a script in the MRP.

You are correct the BVH will go horribly wrong if fed Nans etc - garbage in garbage out. So there may be some kind of order of operations issue with the CPU particles.

m4nu3lf commented 2 years ago

The main scene is not set but you can run it with godot main_scene.tscn or from the editor. I can see there is a a main_scene.tscn and a main_scene.gd files. Do you see these files?

lawnjelly commented 2 years ago

Ah sorry, mistake on my part, I had labelled the wrong MRP as yours :blush: . I must have around 1000 MRPs from issues downloaded and filed (and they are nearly all called "test" funnily enough hehe :grinning: ), and sometimes I mess up the filing lol.

It is working. :+1: Am investigating.

akien-mga commented 2 years ago

Fixed by #66115.