godotengine / godotengine/godot

`GPUParticles3D` renders with a freed transforms uniform set on their first emitting frame

Open
#122,005 5 comments 1 reaction 0 assignees View on GitHub
bug confirmed topic:3d topic:particles topic:rendering
Dominant language
C++
Stars
117k
Forks
26.8k
PR merge metrics
PR metrics pending

Description

### Tested versions

- Reproducible in: v4.7.1.stable, and current `master` (eda2a48)
- Likely affects 4.x broadly but I didn't test other versions
- Tested on Forward+ but also affects Mobile

### System information

Godot v4.7.1.stable - Windows 11 (build 26100) - Multi-window, 4 monitors - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3090 (NVIDIA; 32.0.15.9597) - Intel(R) Core(TM) i9-10980XE CPU @ 3.00GHz (36 threads) - 63.70 GiB memory - WASAPI (48000 Hz, Stereo/mono)

### Issue description

`RenderForwardClustered::_geometry_instance_update()` caches the particles instance-buffer
uniform set, and only then calls `update_particles()`:

```cpp
ginstance->transforms_uniform_set = particles_storage->particles_get_instance_buffer_uniform_set(
ginstance->data->base, scene_shader.default_shader_rd, TRANSFORMS_UNIFORM_SET);

if (particles_storage->particles_get_frame_counter(ginstance->data->base) == 0) {
// Particles haven't been cleared or updated, update once now to ensure they are ready to render.
particles_storage->update_particles();
}
```

The problem is `update_particles()` calls `_particles_update_buffers()`, which can free the particle instance buffer, and with it the uniform set that was just cached. Two branches do so:

1. `enable_motion_vectors`: when motion vectors first become required, the instance buffer is
reallocated at double size and the old one is freed. I've used this pathway to trigger the bug in the MRP, although realistically this is unlikely to happen in an actual project.
2. `userdata_count != particles->userdata_count` : `_particles_free_data()` frees every buffer.
This happens when the process material's shader data was not valid at the moment the particle
buffer was first allocated (so `0` was recorded) and reports a non-zero count later. This is what I encountered in my personal project, probably because of scene instantiation on a worker thread.

Either way `ginstance->transforms_uniform_set` is left holding a freed RID.

`ParticlesStorage` clears its own cached copy correctly and emits
`DEPENDENCY_CHANGED_PARTICLES_INSTANCES`, but that notification calls `_mark_dirty()` on the
geometry instance that is currently being updated. When the update completes it clears the
dirty state so the notification is discarded. The dead RID then persists for the rest of the session, causing this message to spam:

```
ERROR: RenderingDevice::draw_list_bind_uniform_set: Parameter "uniform_set" is null.
at: (servers/rendering/rendering_device.cpp)
```

Because the bind is skipped, the particle system probably draws with whatever
transform set the previously drawn object left bound in set 2, i.e. incorrect per-instance
transforms, but for me the main observable behavior was just a wall of "uniform_set is null" errors appearing seemingly out of nowhere, which is not very descriptive; that's why I'm reporting what I think is the underlying issue.

To actually end up in this scenario, multiple things have to happen at once (same frame) which is why the attached MRP is a little contrived, but in my personal project I was running into these errors regularly.

`RenderForwardMobile::_geometry_instance_update()` has the same ordering.

### Steps to reproduce

Launch the attached project

### Minimal reproduction project (MRP)

[particles-mrp.zip](https://github.com/user-attachments/files/30618628/particles-mrp.zip)

Contributor guide

Open the contributing guide

Research direction

Start at RenderForwardClustered::_geometry_instance_update() and RenderForwardMobile::_geometry_instance_update(), then trace particles_storage->update_particles() and _particles_update_buffers() for both buffer-freeing branches. Use the attached particles-mrp.zip to reproduce the null uniform-set errors; done means the first emitting frame no longer retains a freed transforms uniform set and the errors stop.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
computer-graphics, game-dev
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.