Split render orientation and size

Split the calculation of the particle orientation (generally, from
`OrientModifier`) and its size (_e.g._ from `SizeOverLifetimeModifier`)
in such a way they become independent, and we can mix and match
orientations with screen space size or simulation space size.

Fix screen space size to avoid a perspective divide, which was
effectively breaking the depth-independent property of the size.

Fixes #269

Author: djeedai

CHANGELOG.md

@@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Moved most shared WGSL code into an import module `vfx_common.wgsl`. This requires using `naga_oil` for import resolution, which in turns means `naga` and `naga_oil` are now dependencies of `bevy_hanabi` itself.
 
+### Fixed
+
+- Fixed a bug where particle attributes used in the context of a function would emit invalid code. (#275)
+- Fixed a bug where the screen-space size of particles was not correctly computed, leading to small variations in size. The new code correctly sizes particles based on a screen pixel size. This may increase the actual size of particles, if a larger size had been previously used to compensate the error introduced by this bug. (#269)
+- Fixed a bug where screen-space size ignored the particle's local orientation. (#269)
+
 ## [0.9.0] 2023-12-26
 
 ### Added

src/lib.rs

@@ -922,7 +922,6 @@ impl EffectShaderSource {
             vertex_code,
             fragment_code,
             render_extra,
-            render_sim_space_transform_code,
             alpha_cutoff_code,
             particle_texture,
             layout_flags,
@@ -954,15 +953,6 @@ impl EffectShaderSource {
                 String::new()
             };
 
-            let render_sim_space_transform_code = match asset.simulation_space.eval(&render_context)
-            {
-                Ok(s) => s,
-                Err(err) => {
-                    error!("Failed to compile effect's simulation space: {:?}", err);
-                    return Err(ShaderGenerateError::Expr(err));
-                }
-            };
-
             let mut layout_flags = LayoutFlags::NONE;
             if asset.simulation_space == SimulationSpace::Local {
                 layout_flags |= LayoutFlags::LOCAL_SPACE_SIMULATION;
@@ -990,7 +980,6 @@ impl EffectShaderSource {
                 render_context.vertex_code,
                 render_context.fragment_code,
                 render_context.render_extra,
-                render_sim_space_transform_code,
                 alpha_cutoff_code,
                 render_context.particle_texture,
                 layout_flags,
@@ -1070,10 +1059,6 @@ impl EffectShaderSource {
             .replace("{{VERTEX_MODIFIERS}}", &vertex_code)
             .replace("{{FRAGMENT_MODIFIERS}}", &fragment_code)
             .replace("{{RENDER_EXTRA}}", &render_extra)
-            .replace(
-                "{{SIMULATION_SPACE_TRANSFORM_PARTICLE}}",
-                &render_sim_space_transform_code,
-            )
             .replace("{{ALPHA_CUTOFF}}", &alpha_cutoff_code)
             .replace("{{FLIPBOOK_SCALE}}", &flipbook_scale_code)
             .replace("{{FLIPBOOK_ROW_COUNT}}", &flipbook_row_count_code)
@@ -1208,7 +1193,7 @@ impl CompiledParticleEffect {
         let render_shader = shader_cache.get_or_insert(&asset.name, &shader_source.render, shaders);
 
         trace!(
-            "tick_spawners: init_shader={:?} update_shader={:?} render_shader={:?} has_image={} layout_flags={:?}",
+            "CompiledParticleEffect::update(): init_shader={:?} update_shader={:?} render_shader={:?} has_image={} layout_flags={:?}",
             init_shader,
             update_shader,
             render_shader,

src/render/effect_cache.rs

@@ -307,7 +307,9 @@ impl EffectBuffer {
                 count: None,
             },
         ];
-        if layout_flags.contains(LayoutFlags::LOCAL_SPACE_SIMULATION) {
+        if layout_flags.contains(LayoutFlags::LOCAL_SPACE_SIMULATION)
+            || layout_flags.contains(LayoutFlags::SCREEN_SPACE_SIZE)
+        {
             entries.push(BindGroupLayoutEntry {
                 binding: 3,
                 visibility: ShaderStages::VERTEX,
@@ -319,7 +321,11 @@ impl EffectBuffer {
                 count: None,
             });
         }
-        trace!("Creating render layout with {} entries", entries.len());
+        trace!(
+            "Creating render layout with {} entries (flags: {:?})",
+            entries.len(),
+            layout_flags
+        );
         let particles_buffer_layout_with_dispatch =
             render_device.create_bind_group_layout(&BindGroupLayoutDescriptor {
                 entries: &entries,

src/render/mod.rs

@@ -48,7 +48,7 @@ use crate::{
     render::batch::{BatchInput, BatchState, Batcher, EffectBatch},
     spawn::EffectSpawner,
     CompiledParticleEffect, EffectProperties, EffectShader, HanabiPlugin, ParticleLayout,
-    PropertyLayout, RemovedEffectsEvent, SimulationCondition, SimulationSpace,
+    PropertyLayout, RemovedEffectsEvent, SimulationCondition,
 };
 
 mod aligned_buffer_vec;
@@ -1041,7 +1041,7 @@ impl SpecializedRenderPipeline for ParticlesRenderPipeline {
                 count: None,
             },
         ];
-        if key.local_space_simulation {
+        if key.local_space_simulation || key.screen_space_size {
             entries.push(BindGroupLayoutEntry {
                 binding: 3,
                 visibility: ShaderStages::VERTEX,
@@ -1088,11 +1088,13 @@ impl SpecializedRenderPipeline for ParticlesRenderPipeline {
         // Key: PARTICLE_SCREEN_SPACE_SIZE
         if key.screen_space_size {
             shader_defs.push("PARTICLE_SCREEN_SPACE_SIZE".into());
+            shader_defs.push("RENDER_NEEDS_SPAWNER".into());
         }
 
         // Key: LOCAL_SPACE_SIMULATION
         if key.local_space_simulation {
             shader_defs.push("LOCAL_SPACE_SIMULATION".into());
+            shader_defs.push("RENDER_NEEDS_SPAWNER".into());
         }
 
         // Key: USE_ALPHA_MASK
@@ -1362,22 +1364,13 @@ pub(crate) fn extract_effects(
             );
             let property_layout = asset.property_layout();
 
-            let mut layout_flags = LayoutFlags::NONE;
-            if asset.simulation_space == SimulationSpace::Local {
-                layout_flags |= LayoutFlags::LOCAL_SPACE_SIMULATION;
-            }
-            if let crate::AlphaMode::Mask(_) = &asset.alpha_mode {
-                layout_flags |= LayoutFlags::USE_ALPHA_MASK;
-            }
-            // TODO - should we init the other flags here? (they're currently not used)
-
-            trace!("Found new effect: entity {:?} | capacity {} | particle_layout {:?} | property_layout {:?}", entity, asset.capacity(), particle_layout, property_layout);
+            trace!("Found new effect: entity {:?} | capacity {} | particle_layout {:?} | property_layout {:?} | layout_flags {:?}", entity, asset.capacity(), particle_layout, property_layout, effect.layout_flags);
             AddedEffect {
                 entity,
                 capacity: asset.capacity(),
                 particle_layout,
                 property_layout,
-                layout_flags,
+                layout_flags: effect.layout_flags,
                 handle,
             }
         })
@@ -2545,7 +2538,7 @@ pub(crate) fn queue_effects(
                         }),
                     },
                 ];
-                if buffer.layout_flags().contains(LayoutFlags::LOCAL_SPACE_SIMULATION) {
+                if buffer.layout_flags().contains(LayoutFlags::LOCAL_SPACE_SIMULATION) || buffer.layout_flags().contains(LayoutFlags::SCREEN_SPACE_SIZE) {
                     entries.push(BindGroupEntry {
                         binding: 3,
                         resource: BindingResource::Buffer(BufferBinding {
@@ -2555,7 +2548,7 @@ pub(crate) fn queue_effects(
                         }),
                     });
                 }
-                trace!("Creating render bind group with {} entries", entries.len());
+                trace!("Creating render bind group with {} entries (layour flags: {:?})", entries.len(), buffer.layout_flags());
                 let render = render_device.create_bind_group(
                     &format!("hanabi:bind_group_render_vfx{buffer_index}_particles")[..],
                      buffer.particle_layout_bind_group_with_dispatch(),
@@ -2786,6 +2779,9 @@ fn draw<'w>(
     let dyn_uniform_indices = if effect_batch
         .layout_flags
         .contains(LayoutFlags::LOCAL_SPACE_SIMULATION)
+        || effect_batch
+            .layout_flags
+            .contains(LayoutFlags::SCREEN_SPACE_SIZE)
     {
         &dyn_uniform_indices
     } else {

src/render/vfx_render.wgsl

@@ -26,7 +26,7 @@ struct VertexOutput {
 @group(1) @binding(0) var<storage, read> particle_buffer : ParticleBuffer;
 @group(1) @binding(1) var<storage, read> indirect_buffer : IndirectBuffer;
 @group(1) @binding(2) var<storage, read> dispatch_indirect : DispatchIndirect;
-#ifdef LOCAL_SPACE_SIMULATION
+#ifdef RENDER_NEEDS_SPAWNER
 @group(1) @binding(3) var<storage, read> spawner : Spawner; // NOTE - same group as update
 #endif
 #ifdef PARTICLE_TEXTURE
@@ -70,6 +70,41 @@ fn get_camera_rotation_effect_space() -> mat3x3<f32> {
 #endif
 }
 
+/// Unpack a compressed transform stored in transposed row-major form.
+fn unpack_compressed_transform(compressed_transform: mat3x4<f32>) -> mat4x4<f32> {
+    return transpose(
+        mat4x4(
+            compressed_transform[0],
+            compressed_transform[1],
+            compressed_transform[2],
+            vec4<f32>(0.0, 0.0, 0.0, 1.0)
+        )
+    );
+}
+
+/// Transform a simulation space position into a world space position.
+///
+/// The simulation space depends on the effect's SimulationSpace value, and is either
+/// the effect space (SimulationSpace::Local) or the world space (SimulationSpace::Global).
+fn transform_position_simulation_to_world(sim_position: vec3<f32>) -> vec4<f32> {
+#ifdef LOCAL_SPACE_SIMULATION
+    let transform = unpack_compressed_transform(spawner.transform);
+    return transform * vec4<f32>(sim_position, 1.0);
+#else
+    return vec4<f32>(sim_position, 1.0);
+#endif
+}
+
+/// Transform a simulation space position into a clip space position.
+///
+/// The simulation space depends on the effect's SimulationSpace value, and is either
+/// the effect space (SimulationSpace::Local) or the world space (SimulationSpace::Global).
+/// The clip space is the final [-1:1]^3 space output from the vertex shader, before
+/// perspective divide and viewport transform are applied.
+fn transform_position_simulation_to_clip(sim_position: vec3<f32>) -> vec4<f32> {
+    return view.view_proj * transform_position_simulation_to_world(sim_position);
+}
+
 {{RENDER_EXTRA}}
 
 @vertex
@@ -100,31 +135,24 @@ fn vertex(
 
 {{VERTEX_MODIFIERS}}
 
-#ifdef LOCAL_SPACE_SIMULATION
-    let transform = transpose(
-        mat4x4(
-            spawner.transform[0],
-            spawner.transform[1],
-            spawner.transform[2],
-            vec4<f32>(0.0, 0.0, 0.0, 1.0)
-        )
-    );
+#ifdef PARTICLE_SCREEN_SPACE_SIZE
+    // Get perspective divide factor from clip space position. This is the "average" factor for the entire
+    // particle, taken at its position (mesh origin), and applied uniformly for all vertices.
+    let w_cs = transform_position_simulation_to_clip(particle.position).w;
+    // Scale size by w_cs to negate the perspective divide which will happen later after the vertex shader.
+    // The 2.0 factor is because clip space is in [-1:1] so we need to divide by the half screen size only.
+    let screen_size_pixels = view.viewport.zw;
+    let projection_scale = vec2<f32>(view.projection[0][0], view.projection[1][1])
+    size = (size * w_cs * 2.0) / min(screen_size_pixels.x * projection_scale.x, screen_size_pixels.y * projection_scale.y);
 #endif
 
-#ifdef PARTICLE_SCREEN_SPACE_SIZE
-    let half_screen = view.viewport.zw / 2.;
-    let vpos = vertex_position * vec3<f32>(size.x / half_screen.x, size.y / half_screen.y, 1.0);
-    let local_position = particle.position;
-    let world_position = {{SIMULATION_SPACE_TRANSFORM_PARTICLE}};
-    out.position = view.view_proj * world_position + vec4<f32>(vpos, 0.0);
-#else
+    // Expand particle mesh vertex based on particle position ("origin"), and local
+    // orientation and size of the particle mesh (currently: only quad).
     let vpos = vertex_position * vec3<f32>(size.x, size.y, 1.0);
-    let local_position = particle.position
+    let sim_position = particle.position
         + axis_x * vpos.x
         + axis_y * vpos.y;
-    let world_position = {{SIMULATION_SPACE_TRANSFORM_PARTICLE}};
-    out.position = view.view_proj * world_position;
-#endif
+    out.position = transform_position_simulation_to_clip(sim_position);
 
     out.color = color;