debbugging led to better sphere projection for culling

crates/renderling/src/bvol.rs

@@ -12,7 +12,7 @@
 //! * https://iquilezles.org/www/articles/frustumcorrect/frustumcorrect.htm
 
 use crabslab::SlabItem;
-use glam::{Mat4, Vec3, Vec4, Vec4Swizzles};
+use glam::{Mat4, Vec2, Vec3, Vec3Swizzles, Vec4, Vec4Swizzles};
 #[cfg(target_arch = "spirv")]
 use spirv_std::num_traits::Float;
 
@@ -173,6 +173,9 @@ impl Aabb {
 #[cfg_attr(not(target_arch = "spirv"), derive(Debug))]
 #[derive(Clone, Copy, Default, PartialEq, SlabItem)]
 pub struct Frustum {
+    /// Planes constructing the sides of the frustum,
+    /// each expressed as a normal vector (xyz) and the distance (w)
+    /// from the origin along that vector.
     pub planes: [Vec4; 6],
     pub points: [Vec3; 8],
 }
@@ -336,6 +339,34 @@ impl BoundingSphere {
         let sphere = BoundingSphere::new(center, radius);
         (sphere.is_inside_frustum(camera.frustum()), sphere)
     }
+
+    /// Returns an [`Aabb`] with x and y coordinates in viewport pixels and z coordinate
+    /// in NDC depth.
+    pub fn project_onto_viewport(&self, camera: &Camera, viewport: Vec2) -> Aabb {
+        fn ndc_to_pixel(viewport: Vec2, ndc: Vec3) -> Vec2 {
+            let screen = Vec3::new((ndc.x + 1.0) * 0.5, 1.0 - (ndc.y + 1.0) * 0.5, ndc.z);
+            (screen * viewport.extend(1.0)).xy()
+        }
+
+        let viewproj = camera.view_projection();
+        let frustum = camera.frustum();
+
+        // Find the center and radius of the bounding sphere in pixel space.
+        // By pixel space, I mean where (0, 0) is the top-left of the screen
+        // and (w, h) is is the bottom-left.
+        let center_clip = viewproj * self.center.extend(1.0);
+        let front_center_ndc =
+            viewproj.project_point3(self.center + self.radius * frustum.planes[5].xyz());
+        let back_center_ndc =
+            viewproj.project_point3(self.center + self.radius * frustum.planes[0].xyz());
+        let center_ndc = center_clip.xyz() / center_clip.w;
+        let center_pixels = ndc_to_pixel(viewport, center_ndc);
+        let radius_pixels = viewport.x * (self.radius / center_clip.w);
+        Aabb::new(
+            (center_pixels - radius_pixels).extend(front_center_ndc.z),
+            (center_pixels + radius_pixels).extend(back_center_ndc.z),
+        )
+    }
 }
 
 impl BVol for BoundingSphere {

crates/renderling/src/cull.rs

@@ -3,7 +3,7 @@
 //! Frustum culling as explained in
 //! [the vulkan guide](https://vkguide.dev/docs/gpudriven/compute_culling/).
 use crabslab::{Array, Id, Slab, SlabItem};
-use glam::{UVec2, UVec3, Vec3, Vec3Swizzles};
+use glam::{UVec2, UVec3, Vec2, Vec3, Vec3Swizzles};
 #[allow(unused_imports)]
 use spirv_std::num_traits::Float;
 use spirv_std::{
@@ -12,7 +12,7 @@ use spirv_std::{
     spirv,
 };
 
-use crate::draw::DrawIndirectArgs;
+use crate::{bvol::BoundingSphere, draw::DrawIndirectArgs, stage::Renderlet};
 
 #[cfg(not(target_arch = "spirv"))]
 mod cpu;
@@ -31,15 +31,19 @@ pub fn compute_culling(
         return;
     }
 
+    crate::println!("gid: {gid}");
     // Get the draw arg
     let arg = unsafe { args.index_unchecked_mut(gid) };
     // Get the renderlet using the draw arg's renderlet id
-    let renderlet = stage_slab.read_unchecked(arg.first_instance);
+    let renderlet_id = Id::<Renderlet>::new(arg.first_instance);
+    let renderlet = stage_slab.read_unchecked(renderlet_id);
+    crate::println!("renderlet: {renderlet_id:?}");
 
     arg.vertex_count = renderlet.get_vertex_count();
     arg.instance_count = if renderlet.visible { 1 } else { 0 };
 
     if renderlet.bounds.radius == 0.0 {
+        crate::println!("renderlet bounding radius is zero, cannot cull");
         return;
     }
     let camera = stage_slab.read(renderlet.camera_id);
@@ -48,44 +52,55 @@ pub fn compute_culling(
     let (renderlet_is_inside_frustum, sphere_in_world_coords) =
         renderlet.bounds.is_inside_camera_view(&camera, model);
     if renderlet_is_inside_frustum {
+        crate::println!("renderlet is inside frustum");
+        crate::println!("znear: {}", camera.frustum().planes[0]);
+        crate::println!(" zfar: {}", camera.frustum().planes[5]);
+
         // Compute occlusion culling using the hierachical z-buffer.
         let hzb_desc = depth_pyramid_slab.read_unchecked::<DepthPyramidDescriptor>(0u32.into());
-        let viewprojection = camera.view_projection();
-
-        // Find the center and radius of the bounding sphere in screen space, where
-        // (0, 0) is the top-left of the screen and (1, 1) is is the bottom-left.
-        //
-        // z = 0 is near and z = 1 is far.
-        let center_ndc = viewprojection.project_point3(sphere_in_world_coords.center);
-        let center = Vec3::new(
-            (center_ndc.x + 1.0) * 0.5,
-            (center_ndc.y + 1.0) * -0.5,
-            center_ndc.z,
-        );
-        // Find the radius (in screen space)
-        let radius = viewprojection
-            .project_point3(Vec3::new(sphere_in_world_coords.radius, 0.0, 0.0))
-            .distance(Vec3::ZERO);
+        let viewport_size = Vec2::new(hzb_desc.size.x as f32, hzb_desc.size.y as f32);
+        let sphere_aabb = sphere_in_world_coords.project_onto_viewport(&camera, viewport_size);
+        crate::println!("sphere_aabb: {sphere_aabb:#?}");
         let size_max_element = if hzb_desc.size.x > hzb_desc.size.y {
             hzb_desc.size.x
         } else {
             hzb_desc.size.y
         } as f32;
-        let size_in_pixels = 2.0 * radius * size_max_element;
+        crate::println!("screen max dimension: {size_max_element}");
+        let size_in_pixels = sphere_aabb.max.xy() - sphere_aabb.min.xy();
+        let size_in_pixels = if size_in_pixels.x > size_in_pixels.y {
+            size_in_pixels.x
+        } else {
+            size_in_pixels.y
+        };
+        crate::println!("renderlet size in pixels: {size_in_pixels}");
         let mip_level = size_in_pixels.log2().floor() as u32;
-        let x = center.x * (hzb_desc.size.x >> mip_level) as f32;
-        let y = center.y * (hzb_desc.size.y >> mip_level) as f32;
-        let depth_id = hzb_desc.id_of_depth(
-            mip_level,
-            UVec2::new(x as u32, y as u32),
-            depth_pyramid_slab,
+        let max_mip_level = hzb_desc.mip.len() as u32 - 1;
+        let mip_level = if mip_level > max_mip_level {
+            crate::println!("mip_level maxed out at {mip_level}, setting to {max_mip_level}");
+            max_mip_level
+        } else {
+            mip_level
+        };
+        crate::println!(
+            "selected mip level: {mip_level} {}x{}",
+            viewport_size.x as u32 >> mip_level,
+            viewport_size.y as u32 >> mip_level
         );
-
+        let center = sphere_aabb.center().xy();
+        crate::println!("center: {center}");
+        let x = center.x.round() as u32 >> mip_level;
+        let y = center.y.round() as u32 >> mip_level;
+        crate::println!("mip (x, y): ({x}, {y})");
+        let depth_id = hzb_desc.id_of_depth(mip_level, UVec2::new(x, y), depth_pyramid_slab);
         let depth_in_hzb = depth_pyramid_slab.read_unchecked(depth_id);
-        let depth_of_sphere = center.z - radius;
+        crate::println!("depth_in_hzb: {depth_in_hzb}");
+        let depth_of_sphere = sphere_aabb.min.z;
+        crate::println!("depth_of_sphere: {depth_of_sphere}");
         let renderlet_is_behind_something = depth_of_sphere > depth_in_hzb;
 
         if renderlet_is_behind_something {
+            crate::println!("CULLED");
             arg.instance_count = 0;
         }
     } else {

crates/renderling/src/cull/cpu.rs

@@ -296,10 +296,7 @@ impl DepthPyramid {
         ctx: &crate::Context,
     ) -> Result<Vec<image::ImageBuffer<image::Luma<f32>, Vec<f32>>>, CullingError> {
         let size = self.size();
-        let slab_data = self
-            .slab
-            .read(ctx.get_device(), ctx.get_queue(), Self::LABEL, 0..)
-            .await?;
+        let slab_data = self.slab.read(ctx, Self::LABEL, 0..).await?;
         let mut images = vec![];
         let mut min = f32::MAX;
         let mut max = f32::MIN;
@@ -725,11 +722,14 @@ impl ComputeDepthPyramid {
 
 #[cfg(test)]
 mod test {
+    use std::collections::HashMap;
+
     use crate::{
-        bvol::BoundingSphere, cull::DepthPyramidDescriptor, math::hex_to_vec4, prelude::*,
+        bvol::BoundingSphere, cull::DepthPyramidDescriptor, draw::DrawIndirectArgs,
+        math::hex_to_vec4, prelude::*,
     };
     use crabslab::GrowableSlab;
-    use glam::{Mat4, Quat, UVec2, UVec4, Vec2, Vec3, Vec4};
+    use glam::{Mat4, Quat, UVec2, UVec3, UVec4, Vec2, Vec3, Vec4};
     use image::GenericImageView;
 
     #[test]
@@ -858,14 +858,17 @@ mod test {
             frame.present();
         };
 
+        // A hashmap to hold renderlet ids to their names.
+        let mut names = HashMap::<Id<Renderlet>, String>::default();
+
         // Add four yellow cubes in each corner
         let _ycubes = [
-            Vec2::new(-1.0, 1.0),
-            Vec2::new(1.0, 1.0),
-            Vec2::new(1.0, -1.0),
-            Vec2::new(-1.0, -1.0),
+            (Vec2::new(-1.0, 1.0), "top_left"),
+            (Vec2::new(1.0, 1.0), "top_right"),
+            (Vec2::new(1.0, -1.0), "bottom_right"),
+            (Vec2::new(-1.0, -1.0), "bottom_left"),
         ]
-        .map(|offset| {
+        .map(|(offset, suffix)| {
             let yellow = hex_to_vec4(0xFFE6A5FF);
             let transform = stage.new_value(Transform {
                 // move it back behind the purple cube
@@ -888,6 +891,7 @@ mod test {
                 ..Default::default()
             });
             stage.add_renderlet(&renderlet);
+            names.insert(renderlet.id(), format!("yellow_cube_{suffix}"));
             (renderlet, transform, vertices)
         });
 
@@ -916,6 +920,7 @@ mod test {
                 ..Default::default()
             });
             stage.add_renderlet(&renderlet);
+            names.insert(renderlet.id(), "floor".into());
             (renderlet, transform, vertices)
         };
 
@@ -945,6 +950,7 @@ mod test {
                 ..Default::default()
             });
             stage.add_renderlet(&renderlet);
+            names.insert(renderlet.id(), "green_cube".into());
             (renderlet, transform, vertices)
         };
 
@@ -974,6 +980,7 @@ mod test {
                 ..Default::default()
             });
             stage.add_renderlet(&renderlet);
+            names.insert(renderlet.id(), "purple_cube".into());
             (renderlet, transform, vertices)
         };
 
@@ -992,7 +999,6 @@ mod test {
             min = min.min(v);
         });
         let total = max.x - min.x;
-        log::info!("depth: {depth_img:#?}");
         depth_img
             .as_mut_rgb32f()
             .unwrap()
@@ -1023,5 +1029,52 @@ mod test {
             });
             img_diff::save(&format!("cull/debugging_pyramid_mip_{i}.png"), img);
         }
+
+        // The stage's slab, which contains the `Renderlet`s and their `BoundingSphere`s
+        let stage_slab =
+            futures_lite::future::block_on(stage.read(&ctx, Some("read stage"), ..)).unwrap();
+        let draw_calls = stage.draw_calls.read().unwrap();
+        let indirect_draws = draw_calls.drawing_strategy.as_indirect().unwrap();
+        // The HZB slab, which contains a `DepthPyramidDescriptor` at index 0, and all the
+        // pyramid's mips
+        let depth_pyramid_slab = futures_lite::future::block_on(
+            indirect_draws
+                .compute_culling
+                .compute_depth_pyramid
+                .depth_pyramid
+                .slab
+                .read(&ctx, Some("read hzb desc"), ..),
+        )
+        .unwrap();
+        // The indirect draw buffer
+        let mut args_slab =
+            futures_lite::future::block_on(indirect_draws.slab.read(&ctx, Some("read args"), ..))
+                .unwrap();
+        let args: &mut [DrawIndirectArgs] = bytemuck::cast_slice_mut(&mut args_slab);
+        // Number of `DrawIndirectArgs` in the `args` buffer.
+        let num_draw_calls = draw_calls.draw_count();
+
+        // Print our names so we know what we're working with
+        let mut pnames = names.iter().collect::<Vec<_>>();
+        pnames.sort();
+        for (id, name) in pnames.into_iter() {
+            log::info!("id: {id:?}, name: {name}");
+        }
+
+        for i in 0..num_draw_calls as u32 {
+            let renderlet_id = Id::<Renderlet>::new(args[i as usize].first_instance);
+            let name = names.get(&renderlet_id).unwrap();
+            if name != "green_cube" {
+                continue;
+            }
+            log::info!("");
+            log::info!("name: {name}");
+            crate::cull::compute_culling(
+                &stage_slab,
+                &depth_pyramid_slab,
+                args,
+                UVec3::new(i, 0, 0),
+            );
+        }
     }
 }

crates/renderling/src/draw.rs

@@ -13,10 +13,12 @@ mod cpu;
 pub use cpu::*;
 
 /// Argument buffer layout for draw_indirect commands.
+#[repr(C)]
+#[cfg_attr(not(target_arch = "spirv"), derive(bytemuck::Pod, bytemuck::Zeroable))]
 #[derive(Clone, Copy, Default, SlabItem)]
 pub struct DrawIndirectArgs {
     pub vertex_count: u32,
     pub instance_count: u32,
     pub first_vertex: u32,
-    pub first_instance: Id<Renderlet>,
+    pub first_instance: u32,
 }

crates/renderling/src/draw/cpu.rs

@@ -38,9 +38,9 @@ impl InternalRenderlet {
 ///
 /// Issues draw calls and performs culling.
 pub struct IndirectDraws {
-    slab: SlabAllocator<wgpu::Buffer>,
-    draws: Vec<Gpu<DrawIndirectArgs>>,
-    compute_culling: ComputeCulling,
+    pub(crate) slab: SlabAllocator<wgpu::Buffer>,
+    pub(crate) draws: Vec<Gpu<DrawIndirectArgs>>,
+    pub(crate) compute_culling: ComputeCulling,
 }
 
 impl IndirectDraws {
@@ -124,7 +124,7 @@ impl From<Id<Renderlet>> for DrawIndirectArgs {
             vertex_count: 0,
             instance_count: 0,
             first_vertex: 0,
-            first_instance: id,
+            first_instance: id.inner(),
         }
     }
 }
@@ -284,6 +284,12 @@ impl DrawCalls {
         }
     }
 
+    /// Returns the number of draw calls (direct or indirect) that will be
+    /// made during pre-rendering (compute culling) and rendering.
+    pub fn draw_count(&self) -> usize {
+        self.internal_renderlets.len()
+    }
+
     /// Perform pre-draw steps like compute culling, if available.
     ///
     /// This does not do upkeep, please call [`DrawCalls::upkeep`] before
@@ -295,7 +301,7 @@ impl DrawCalls {
         slab_buffer: &wgpu::Buffer,
         depth_texture: &Texture,
     ) -> Result<(), CullingError> {
-        let num_draw_calls = self.internal_renderlets.len();
+        let num_draw_calls = self.draw_count();
         // Only do compute culling if there are things we need to draw, otherwise
         // `wgpu` will err with something like:
         // "Buffer with 'indirect draw upkeep' label binding size is zero"
@@ -328,7 +334,7 @@ impl DrawCalls {
 
     /// Draw into the given `RenderPass`.
     pub fn draw(&self, render_pass: &mut wgpu::RenderPass) {
-        let num_draw_calls = self.internal_renderlets.len();
+        let num_draw_calls = self.draw_count();
         if num_draw_calls > 0 {
             match &self.drawing_strategy {
                 DrawingStrategy::Indirect(indirect) => {