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

After despawning, new ParticleEffects don't produce particles #106

Closed
theon opened this issue Dec 21, 2022 · 4 comments · Fixed by #107 or #149
Closed

After despawning, new ParticleEffects don't produce particles #106

theon opened this issue Dec 21, 2022 · 4 comments · Fixed by #107 or #149
Labels
C - bug Something isn't working

Comments

@theon
Copy link

theon commented Dec 21, 2022

Environment

bevy: 0.9
bevy_hanabi: 0.5
OS: MacOS

Issue

After despawning ParticleEffect entities, newly spawned ParticleEffect entities don't produce particles

Reproducing

See the following code for a reproduction: theon@132ce0b

The reproduction code spawns a new ParticleEffect entity every second. Once there are MAX_EFFECTS entities it despawns the oldest entity before spawning the next.

Setting MAX_EFFECTS in that example seems to determine when the issue starts:

  • MAX_EFFECTS=1, 3rd spawned particle effect doesn't create particles
  • MAX_EFFECTS=2, 4th spawned particle effect doesn't create particles
  • MAX_EFFECTS=10, 12th spawned particle effect doesn't create particles

Example where MAX_EFFECTS=5 and the 7th spawned entity onwards doesn't produce particles:
https://user-images.githubusercontent.com/759170/209003169-085f376e-8465-41cd-91a8-1649c318d691.mp4

Note: There is no issue when commenting out the line with despawn_recursive()

@theon theon changed the title Despawning ParticleEffect prevents spawning more After despawning, new ParticleEffects don't produce particles Dec 21, 2022
@djeedai djeedai added the C - bug Something isn't working label Dec 22, 2022
djeedai added a commit that referenced this issue Dec 22, 2022
Fix bind groups not re-created correctly when an effect is despawned in
the same frame as another effect is spawned.

Add a partial workaround for the `vfx_indirect.wgsl` shader accessing
GPU buffers in order from index 0, which requires all GPU particle
buffers to be in use without any gap. The partial workaround first
destroys buffers for de-allocated effects, before re-creating new
buffers for newly-spawned ones. This is not perfect, because this
doesn't account for gaps when more buffers are de-allocated than created
in the same frame, in which case `vfx_indirect` accesses the first N
buffers, but some of them are not in use and some in use are skipped.
We're missing an indirection here.

Fixes #106
djeedai added a commit that referenced this issue Dec 22, 2022
Fix bind groups not re-created correctly when an effect is despawned in
the same frame as another effect is spawned.

Add a partial workaround for the `vfx_indirect.wgsl` shader accessing
GPU buffers in order from index 0, which requires all GPU particle
buffers to be in use without any gap. The partial workaround first
destroys buffers for de-allocated effects, before re-creating new
buffers for newly-spawned ones. This is not perfect, because this
doesn't account for gaps when more buffers are de-allocated than created
in the same frame, in which case `vfx_indirect` accesses the first N
buffers, but some of them are not in use and some in use are skipped.
We're missing an indirection here.

Fixes #106
@djeedai
Copy link
Owner

djeedai commented Dec 22, 2022

There's 2 bugs here:

  • The simple one: some bind group was not re-created, so the new effect was using the bind group of the old effect. Fixed in Fix same-frame despawn/respawn #107.

  • The hard one: the indirect compute pass assumes N effects are allocated as N consecutive GPU particle buffers, but when some effects are deleted and their GPU buffer de-allocated, there may be a gap in the numbering. This was compounded by the fact we were first allocating new effect before we deallocated old ones; this part has been inverted in Fix same-frame despawn/respawn #107 such that despawning and spawning the same number of effects per frame doesn't produce any bug. But there's no easy fix for the case where for example more effects are despawned than are spawned in the same frame (and not in LIFO order).

    For example the allocation sequence [A] -> [A B] -> [A B C] -> [A - C] (allocate 3 instances, then de-allocate the second one) will fail to simulate/render effect instance C correctly because it will attempt to process [A -] instead of [A C].

djeedai added a commit that referenced this issue Dec 22, 2022
Fix bind groups not re-created correctly when an effect is despawned in
the same frame as another effect is spawned.

Add a partial workaround for the `vfx_indirect.wgsl` shader accessing
GPU buffers in order from index 0, which requires all GPU particle
buffers to be in use without any gap. The partial workaround first
destroys buffers for de-allocated effects, before re-creating new
buffers for newly-spawned ones. This is not perfect, because this
doesn't account for gaps when more buffers are de-allocated than created
in the same frame, in which case `vfx_indirect` accesses the first N
buffers, but some of them are not in use and some in use are skipped.
We're missing an indirection here.

Fixes #106
@djeedai
Copy link
Owner

djeedai commented Dec 22, 2022

Re-opening; this is only partially fixed as explained above.

@vultix
Copy link

vultix commented Jan 10, 2023

Just ran into this - I'd be happy to help contribute a fix once you know what solution you'd like

djeedai added a commit that referenced this issue Mar 2, 2023
Fix the invalid addressing of the indirect dispatch shader, which
assumed that all allocated particle effects were tightly packed into the
global dispatch indirect and render buffers. This is not true, due to
the fact despawned effects leave an empty entry into those two buffers,
and the buffers are indexed simultaneously by the CPU and GPU so cannot
easily be compacted into a continuous array. This change fixes the
addressing by leveraging the actual effect index stored per allocated
effect into the spawner table, which is uploaded from CPU to GPU each
frame, and therefore contains an up-to-date view of the allocations into
the indirect and render dispatch tables. Using that index to indirect
the compute shader's thread ID allows effectively addressing all the
allocated effects, even when gaps exist into the dispatch tables. This
fixes some effects not being simulated or rendered correctly when
spawned after other effects have despawned and left gaps into those
tables.

Fixes #106
djeedai added a commit that referenced this issue Mar 2, 2023
Fix the invalid addressing of the indirect dispatch shader, which
assumed that all allocated particle effects were tightly packed into the
global dispatch indirect and render buffers. This is not true, due to
the fact despawned effects leave an empty entry into those two buffers,
and the buffers are indexed simultaneously by the CPU and GPU so cannot
easily be compacted into a continuous array. This change fixes the
addressing by leveraging the actual effect index stored per allocated
effect into the spawner table, which is uploaded from CPU to GPU each
frame, and therefore contains an up-to-date view of the allocations into
the indirect and render dispatch tables. Using that index to indirect
the compute shader's thread ID allows effectively addressing all the
allocated effects, even when gaps exist into the dispatch tables. This
fixes some effects not being simulated or rendered correctly when
spawned after other effects have despawned and left gaps into those
tables.

Fixes #106
djeedai added a commit that referenced this issue Mar 2, 2023
Fix the invalid addressing of the indirect dispatch shader, which
assumed that all allocated particle effects were tightly packed into the
global dispatch indirect and render buffers. This is not true, due to
the fact despawned effects leave an empty entry into those two buffers,
and the buffers are indexed simultaneously by the CPU and GPU so cannot
easily be compacted into a continuous array. This change fixes the
addressing by leveraging the actual effect index stored per allocated
effect into the spawner table, which is uploaded from CPU to GPU each
frame, and therefore contains an up-to-date view of the allocations into
the indirect and render dispatch tables. Using that index to indirect
the compute shader's thread ID allows effectively addressing all the
allocated effects, even when gaps exist into the dispatch tables. This
fixes some effects not being simulated or rendered correctly when
spawned after other effects have despawned and left gaps into those
tables.

Fixes #106
djeedai added a commit that referenced this issue Mar 2, 2023
Fix the invalid addressing of the indirect dispatch shader, which
assumed that all allocated particle effects were tightly packed into the
global dispatch indirect and render buffers. This is not true, due to
the fact despawned effects leave an empty entry into those two buffers,
and the buffers are indexed simultaneously by the CPU and GPU so cannot
easily be compacted into a continuous array. This change fixes the
addressing by leveraging the actual effect index stored per allocated
effect into the spawner table, which is uploaded from CPU to GPU each
frame, and therefore contains an up-to-date view of the allocations into
the indirect and render dispatch tables. Using that index to indirect
the compute shader's thread ID allows effectively addressing all the
allocated effects, even when gaps exist into the dispatch tables. This
fixes some effects not being simulated or rendered correctly when
spawned after other effects have despawned and left gaps into those
tables.

Fixes #106
@djeedai
Copy link
Owner

djeedai commented Mar 2, 2023

Opportunistically closed by #149 as I believe this was the last bug, and everything should be fixed. Please reopen if there's still a case I missed, but RenderDoc shows no error in all cases I looked at. CC: @vultix @theon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - bug Something isn't working
Projects
None yet
3 participants