godotengine / godot

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

GPUParticles2D triggering "at end" sub-emitter twice #97904

Open ShotgunCrocodile opened 2 weeks ago

ShotgunCrocodile commented 2 weeks ago

Tested versions

System information

Godot v4.3.stable - macOS 14.4.1 - Vulkan (Forward+) - integrated Apple M1 - Apple M1 (8 Threads)

Issue description

GPUParticles2D "at end" sub-emitter emits multiple times. This can be illustrated simply by setting "amount at end" to 1 and it will emit 2 particles. Set it to 2, you get two particles and so on. In addition they seem to be split across the location of the particle in its last two frames of life. This is very noticeable for any fast moving particle, once it dies its locations in the last two frames are decently far apart and will show two discrete bursts of particles using the "at end" emitter.

This does appear to be a regression since in 4.2 I see the correct number of particles in my test project provided below.

Steps to reproduce

Minimal reproduction project (MRP)

particlebugreport.zip

Calinou commented 2 weeks ago

@ShotgunCrocodile Can you try following the same steps on 4.3 dev/beta/RCs to determine when the regression started? You can download them here.

AtlaStar commented 2 weeks ago

Currently bisecting, but in testing things out a bit I have found that the incorrect behavior happens on creation of the particle node, and on load, and that changing the lifetime value will restore things from improper state. Changing it then returning it to the previous value will also work, meaning that the refresh of the lifetime value is likely what is fixing things.

ShotgunCrocodile commented 2 weeks ago

@ShotgunCrocodile Can you try following the same steps on 4.3 dev/beta/RCs to determine when the regression started? You can download them here.

I am out to dinner, I can do this when I get back if still needed.

AtlaStar commented 2 weeks ago

PR #85189 is the start for the change in behavior @Calinou, @ShotgunCrocodile.

Two things to note; there is another issue involving this PR as lifetime values in the parent greater than 1 simply do not work using the test project as presented. This does not change even after updating the lifetime to refresh any internal state in particle storage. The second thing is that since the reported issue resolves itself when updating the lifetime value in editor, it is likely that that half of the equation was not caused by that PR, but rather exposed by it.

ShotgunCrocodile commented 2 weeks ago

I messed around in the editor a bit and I replicated the time parameter fixing the behavior. However in my case it reverted to the previous behavior after just building the project a few times with no other changes. The fractional delta checkbox also seemed to fix the behavior for awhile, but again it reverted after running the project a few times.

I tried all of these fixes in my actual project where I first discovered the issue and none of them modified the behavior.

AtlaStar commented 2 weeks ago

So, looks like the bug may actually be related to what was mentioned in the old comment that the above mentioned PR overwrote; sometimes the system won't emit particles if the parent would become inactive...but it also appears that sometimes it emits particles twice before becoming inactive. Specifically what you are observing is it emitting particles twice, up to the max amount of particles the child system can emit.

Basically there are a couple of ways to work around your issue; first is to convert the process material to a shader and at the end where it has the emit_particles call, wrap it in an if (ACTIVE) so it only activates once. Problem is that sometimes this prevents particles from drawing, because it is heavily dependent on the Fixed FPS and the timeline as well...the other is what I said before, or even potentially turning the Fixed FPS up.

The core problem appears to be how compute passes are batched; for example, using RenderDoc I was able to see that the particle compute shader had dispatched 3 times in a single frame, all within the same compute pass...so it appears that the issue might lie with how they are being dispatched to begin with, and possibly even a lack of synchronization...which is a bit beyond the scope of my knowledge unfortunately.

AtlaStar commented 2 weeks ago

Actually, found a work around that seems pretty stable;

if ((CUSTOM.y / CUSTOM.w * LIFETIME) > (LIFETIME - DELTA)) {
    emit_count = sub_emitter_amount_at_end;
}
ShotgunCrocodile commented 2 weeks ago

@AtlaStar Thanks for the workaround ideas, they are much appreciated.

In my case I combined both which worked for the non-toy version of my issue. With the FPS on the child set to 1000 and the parent to 20 and with the shader code modifications you suggested in the parent I was consistently getting the double emission rather than a mixture of usually 2 emissions with the occasional 1. Maybe I missed a step or we have some other setting that differs, but in any case that made it safe to use the if (ACTIVE) prior suggestion since it turned 100% double emission into 100% single emission. Which seems stable so far, though the behavior has changed on me without touching the particle effect scene in the past. Plus the risk of emitting 0 times is fine, its easy to miss 1 particle not emitting subparticles among hundreds, whereas if all of them are doubling it is very noticeably wrong.

AtlaStar commented 2 weeks ago

Yeah, I think the deeper issues lies ultimately in the way the particle shader steps get queued up, so it doesn't surprise me that the behavior you are having differs from mine...I also could have just forgotten that I was doing that check.

Either way, glad that it is working as a work around for now. I also think that I saw a TODO in the particles storage involving batching all particle compute passes into a singular one which imo is really the only proper fix for this, but it looks like it was added 2 years ago when the particles storage classes were made.

Spoke too soon; things are definitely deterministic, it was my testing method that was flawed. The testing performed has shown that it is likely a precision error though.