From 0f547d233c489905d69f6580e46ab7a30ce88b55 Mon Sep 17 00:00:00 2001
From: Jerome Humbert <djeedai@gmail.com>
Date: Wed, 22 May 2024 22:00:44 +0100
Subject: [PATCH] Fix effects getting stuck after a deletion

Fix some effects not updating after one or more effects have been
deleted. This was due to the indirect pass shader assuming a contiguous
`ParticleGroup` array, but that array contains gaps where effects have
been deallocated, so updating a number of rows equal to the number of
active effect means updating some unused rows and skipping some used
ones in case there's any gap.
---
 src/render/mod.rs            |  9 +++++++--
 src/render/vfx_indirect.wgsl | 11 ++++++++---
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/render/mod.rs b/src/render/mod.rs
index 3ddd3a13..8fe20469 100644
--- a/src/render/mod.rs
+++ b/src/render/mod.rs
@@ -3374,7 +3374,7 @@ impl Node for VfxSimulateNode {
             .write_buffer(render_context.command_encoder());
 
         // Compute init pass
-        let mut total_group_count = 0;
+        //let mut total_group_count = 0;
         {
             let mut compute_pass =
                 render_context
@@ -3399,7 +3399,7 @@ impl Node for VfxSimulateNode {
                     // FIXME - Currently we unconditionally count all groups because the dispatch
                     // pass always runs on all groups. We should consider if it's worth skipping
                     // e.g. dormant or finished effects at the cost of extra complexity.
-                    total_group_count += batches.group_batches.len() as u32;
+                    //total_group_count += batches.group_batches.len() as u32;
 
                     let Some(init_pipeline) =
                         pipeline_cache.get_compute_pipeline(batches.init_pipeline_id)
@@ -3517,7 +3517,12 @@ impl Node for VfxSimulateNode {
             if let Some(indirect_dispatch_pipeline) = &effects_meta.indirect_dispatch_pipeline {
                 trace!("record commands for indirect dispatch pipeline...");
 
+                // FIXME - The `vfx_indirect` shader assumes a contiguous array of ParticleGroup structures.
+                // So we need to pass the full array size, and we just update the unused groups for nothing.
+                // Otherwise we might update some unused group and miss some used ones, if there's any gap
+                // in the array.
                 const WORKGROUP_SIZE: u32 = 64;
+                let total_group_count = effects_meta.particle_group_buffer.len() as u32;
                 let workgroup_count = (total_group_count + WORKGROUP_SIZE - 1) / WORKGROUP_SIZE;
 
                 // Setup compute pass
diff --git a/src/render/vfx_indirect.wgsl b/src/render/vfx_indirect.wgsl
index 56e259cf..6aab27a9 100644
--- a/src/render/vfx_indirect.wgsl
+++ b/src/render/vfx_indirect.wgsl
@@ -21,9 +21,14 @@ fn main(@builtin(global_invocation_id) global_invocation_id: vec3<u32>) {
 
     // Cap at maximum number of groups to process
     let index = global_invocation_id.x;
-    if (index >= sim_params.num_groups) {
-        return;
-    }
+
+    // FIXME - the group_buffer array has gaps, we can't limit to the number of effects in use
+    // otherwise we'll miss some and process the unused gaps instead of some active ones.
+    // Since all writes below are idempotent, except the ping/pong one, there's no harm updating
+    // unused effect rows.
+    // if (index >= sim_params.num_groups) {
+    //     return;
+    // }
 
     // Cap at group array size, just for safety
     if (index >= arrayLength(&group_buffer)) {