Fix particle jitter when scene tree is paused

[etodd]

Fixes #85213, fixes #50824, fixes #65390.

Attached is a test project which pauses and unpauses every second, and includes:

• A GPUParticles2D with gravity, tangential acceleration, and turbulence.
• A GPUParticles3D with gravity, tangential acceleration, and turbulence.
• A GPUParticles3D with gravity and particle trails.

This PR is based on #89991, but in my testing, that PR changed the behavior of turbulence when the fixed FPS is set to anything other than 30.

particle-pause-jitter-fix.zip

[clayjohn]

I tested and can confirm that this fixing the issue with pausing, but this does not fix #50824 as that issue includes low speed scale. Using a low speed scale in the MRP still reliably reproduces the issue.

I suspect that much of this issue comes from the particles interpolation which happens in the copy shader. The frame_remainder is passed in here:

godot/servers/rendering/renderer_rd/storage_rd/particles_storage.cpp

Line 1243 in 568589c

copy_push_constant.frame_remainder = particles->interpolate ? particles->frame_remainder : 0.0;

And used here:

godot/servers/rendering/renderer_rd/shaders/particles_copy.glsl

Line 199 in 568589c

txform[3].xyz += particles.data[particle].velocity * params.frame_remainder;

You should check the values of frame remainder, as it must be fluctuating when it shouldn't. I agree that #89991 probably isn't the right approach. But its interesting to see that suppressing velocity was enough. That indicates to me that the core of the issue is in the interpolation (which is just frame-remainder * velocity)

Also note, that the same fixes will need to be implemented for the compatibility renderer too: https://github.com/godotengine/godot/blob/master/drivers/gles3/storage/particles_storage.cpp

[etodd]

Thanks for the review. 🙏 You're right, frame_remainder is the issue. I think it's supposed to advance according to the engine time scale * the particle time scale.

I simplified the logic a bit when fixed_fps > 0 and the time scale is zero. The old logic called _particles_process(particles, 0.0) every frame. Now it stops calling it (unless particles->clear is true). I think this is right?

[etodd]

I ran the attached test scene with the following settings:

• Forward plus renderer
• Forward plus renderer with Engine.time_scale = 0.5
• Compatibility renderer
• Compatibility renderer with Engine.time_scale = 0.5

Any other scenarios I should test this with?

[clayjohn]

Yes, you should test with a low speed_scale like in #50824

[etodd]

Thanks for the suggestion! I did some more testing with low engine time scale and low particle system speed scale. Turns out I had the engine time scale multiplied in there when it wasn't needed, and I also needed to remove the particle system speed scaling from inside _particles_process, since it's now applied outside it.

The only other scenario where _particles_process is called is during pre-processing. I believe the way it is in this PR is correct - the particle system speed scale is not applied here. If the particle lifetime is 1 second and I set the pre-process time to 1 second, I expect it to pre-process 1 complete particle lifetime, regardless of the speed scale.

I also went ahead and rebased the PR on top of the master branch.

[QbieShay]

Please double check its current behaviour. If currently preprocess takes speed_scale into account, it should continue to do so.

[etodd]

Currently preprocess does take speed_scale into account, but I believe this is a bug. For example, if you spawn a particle system with preprocessing enabled while the SceneTree is paused, it will do the preprocessing, but it won't have any effect because the speed scale is 0.

Here's a sample project demonstrating that. It pauses the SceneTree, waits two seconds, then spawns a particle system with some preprocessing (which will not actually occur on master branch, you won't see anything). With this PR, you should see the particle system spawn with correct preprocessing.

particle-pause-jitter-fix.zip

That said, I understand if it's desirable to maintain the current behavior for other reasons. Happy to defer if that's the decision.

[etodd]

Any other thoughts on this? One slightly annoying thing is that at low speed scales, the interpolation becomes very obvious. It would be great if the fixed FPS automatically adjusted with the time scale so it actually simulates at the desired FPS regardless of speed scale. But maybe that would be best addressed in a separate PR.

[Calinou]

It would be great if the fixed FPS automatically adjusted with the time scale so it actually simulates at the desired FPS regardless of speed scale. But maybe that would be best addressed in a separate PR.

Changing fixed FPS can affect particle behavior in an undesired manner, especially when collision is involved. Also, changing fixed FPS at runtime can introduce visible jitter, and the FPS must be an integer value (while the time scale is a float). Therefore, automatic adjustment based on time scale should be an opt-in property discussed in a separate proposal.

Note that the particle interpolation is currently more obvious than it could be due to some properties not being interpolated: #51318
Once this is done, it'll look a lot better.

[QbieShay]

@etodd I am working on a PR to request particle process that will work even if the scene tree is paused. I think preprocess should stay as it is right now, and we'll be able to tackle the usecase you mentioned once my PR is in!

[QbieShay]

Seek PR: #92089

[etodd]

@QbieShay I think your changes fixed the preprocessing bug. 🎉 Which means this PR should now have no effect other than fixing the jitter. I rebased the PR and it still works on my test projects.

[etodd]

Re-tested the first test project with Engine.time_scale at 1.0 and 0.1, and on forward+ and compatibility renderer.

Also checked the preprocess behavior on the second test project is the same on 4.4-beta1 and this branch.

[clayjohn]

Discussed in the rendering meeting today. This looks fine to me

[etodd]

Rebased and re-tested!

[akien-mga]

Thanks!

[akien-mga]

Cherry-picked for 4.4.1.

[BaconEggsRL]

Nice! Just ran into this issue on 4.4 and tested with 4.4.1, and it's fixed
Love that :D