Remove delay slot in object management (#594)

Author: cwfitzgerald

rend3-routine/shaders/src/depth.wgsl

@@ -47,26 +47,9 @@ struct VertexOutput {
 
 @vertex
 fn vs_main(@builtin(instance_index) instance_index: u32, @builtin(vertex_index) vertex_index: u32) -> VertexOutput {
-    // If the vertex index is our sentinel invalid value, return a degenerate triangle.
-    //
-    // This is used by the culling shader to discard triangles when the ordering of the
-    // triangles are important, and atomics can't be used.
-    if vertex_index == INVALID_VERTEX {
-        var vs_out: VertexOutput;
-        vs_out.position = vec4<f32>(0.0);
-        return vs_out;
-    }
     let indices = Indices(instance_index, vertex_index);
 
     let data = object_buffer[indices.object];
-    // If the object is disabled, return a degenerate triangle.
-    //
-    // This happens when the object is deleted, and we're rendering last-frame's objects.
-    if data.enabled == 0u {
-        var vs_out: VertexOutput;
-        vs_out.position = vec4<f32>(0.0);
-        return vs_out;
-    }
 
     let vs_in = get_vertices(indices);
 

rend3-routine/shaders/src/opaque.wgsl

@@ -87,26 +87,9 @@ struct VertexOutput {
 
 @vertex
 fn vs_main(@builtin(instance_index) instance_index: u32, @builtin(vertex_index) vertex_index: u32) -> VertexOutput {
-    // If the vertex index is our sentinel invalid value, return a degenerate triangle.
-    //
-    // This is used by the culling shader to discard triangles when the ordering of the
-    // triangles are important, and atomics can't be used.
-    if vertex_index == INVALID_VERTEX {
-        var vs_out: VertexOutput;
-        vs_out.position = vec4<f32>(0.0);
-        return vs_out;
-    }
     let indices = Indices(instance_index, vertex_index);
 
     let data = object_buffer[indices.object];
-    // If the object is disabled, return a degenerate triangle.
-    //
-    // This happens when the object is deleted, and we're rendering last-frame's objects.
-    if data.enabled == 0u {
-        var vs_out: VertexOutput;
-        vs_out.position = vec4<f32>(0.0);
-        return vs_out;
-    }
 
     let vs_in = get_vertices(indices);
 

rend3-routine/shaders/src/structures_object.wgsl

@@ -7,6 +7,4 @@ struct Object {
     index_count: u32,
     material_index: u32,
     vertex_attribute_start_offsets: array<u32, {{vertex_array_counts}}>,
-    // 1 if enabled, 0 if disabled
-    enabled: u32,
 }

rend3/shaders/vertex_attributes.wgsl

@@ -5,8 +5,6 @@ struct Indices {
     vertex: u32,
 }
 
-const INVALID_VERTEX: u32 = 0x00FFFFFFu;
-
 alias TriangleVertices = array<vec3f, 3>;
 alias TriangleIndices = array<u32, 3>;
 struct Triangle {

rend3/src/managers/handle_alloc.rs

@@ -18,29 +18,15 @@ where
 {
     max_allocated: AtomicUsize,
     freelist: Mutex<Vec<usize>>,
-    /// We want the render routines to be able to rely on deleted handles being valid for at
-    /// least one frame.
-    ///
-    /// To facilitate this, we first put the handle in the delay list, then at the top of
-    /// every frame, we move the handles from the delay list to the freelist.
-    ///
-    /// We do not need to do this for everything though, only for Object handles, as these
-    /// are the root handle which the renderer accesses everything.
-    delay_list: Option<Mutex<Vec<usize>>>,
     _phantom: PhantomData<T>,
 }
 
 impl<T> HandleAllocator<T>
 where
     RawResourceHandle<T>: DeletableRawResourceHandle,
 {
-    pub fn new(delay_handle_reclaimation: bool) -> Self {
-        Self {
-            max_allocated: AtomicUsize::new(0),
-            freelist: Mutex::new(Vec::new()),
-            delay_list: delay_handle_reclaimation.then(|| Mutex::new(Vec::new())),
-            _phantom: PhantomData,
-        }
+    pub fn new() -> Self {
+        Self { max_allocated: AtomicUsize::new(0), freelist: Mutex::new(Vec::new()), _phantom: PhantomData }
     }
 
     pub fn allocate(&self, renderer: &Arc<Renderer>) -> ResourceHandle<T> {
@@ -57,21 +43,6 @@ where
 
     pub fn deallocate(&self, handle: RawResourceHandle<T>) {
         let idx = handle.idx;
-        if let Some(ref delay_list) = self.delay_list {
-            delay_list.lock().push(idx);
-        } else {
-            self.freelist.lock().push(idx);
-        }
-    }
-
-    pub fn reclaim_delayed_handles(&self) -> Vec<RawResourceHandle<T>> {
-        if let Some(ref delay_list) = self.delay_list {
-            let mut locked_delay_list = delay_list.lock();
-
-            self.freelist.lock().extend_from_slice(&locked_delay_list);
-            locked_delay_list.drain(..).map(|idx| RawResourceHandle::new(idx)).collect()
-        } else {
-            Vec::new()
-        }
+        self.freelist.lock().push(idx);
     }
 }

rend3/src/managers/object.rs

@@ -31,8 +31,6 @@ pub struct ShaderObject<M: Material> {
     pub material_index: u32,
     pub vertex_attribute_start_offsets:
         <M::SupportedAttributeArrayType as MaterialArray<&'static VertexAttributeId>>::U32Array,
-    // 1 if enabled, 0 if disabled
-    pub enabled: u32,
 }
 
 impl<M: Material> Default for ShaderObject<M> {
@@ -44,7 +42,6 @@ impl<M: Material> Default for ShaderObject<M> {
             index_count: Default::default(),
             material_index: Default::default(),
             vertex_attribute_start_offsets: Zeroable::zeroed(),
-            enabled: Default::default(),
         }
     }
 }
@@ -91,7 +88,7 @@ struct ObjectArchetype {
     set_object_transform: fn(&mut WasmVecAny, &mut FreelistDerivedBuffer, usize, Mat4),
     duplicate_object: fn(&WasmVecAny, usize, ObjectChange) -> Object,
     remove: fn(&mut ObjectArchetype, usize),
-    evaluate: fn(&mut ObjectArchetype, &Device, &mut CommandEncoder, &ScatterCopy, &[RawObjectHandle]),
+    evaluate: fn(&mut ObjectArchetype, &Device, &mut CommandEncoder, &ScatterCopy),
 }
 
 /// Manages objects. That's it. ¯\\\_(ツ)\_/¯
@@ -163,15 +160,9 @@ impl ObjectManager {
         (archetype.remove)(archetype, handle.idx);
     }
 
-    pub fn evaluate(
-        &mut self,
-        device: &Device,
-        encoder: &mut CommandEncoder,
-        scatter: &ScatterCopy,
-        deferred_removals: &[RawObjectHandle],
-    ) {
+    pub fn evaluate(&mut self, device: &Device, encoder: &mut CommandEncoder, scatter: &ScatterCopy) {
         for archetype in self.archetype.values_mut() {
-            (archetype.evaluate)(archetype, device, encoder, scatter, deferred_removals);
+            (archetype.evaluate)(archetype, device, encoder, scatter);
         }
     }
 
@@ -279,7 +270,6 @@ pub(super) fn object_add_callback<M: Material>(_material: &M, args: ObjectAddCal
             first_index: (index_range.start / 4) as u32,
             index_count: ((index_range.end - index_range.start) / 4) as u32,
             vertex_attribute_start_offsets,
-            enabled: true as u32,
         },
         material_handle: args.object.material,
         mesh_kind: args.object.mesh_kind,
@@ -330,35 +320,22 @@ fn duplicate_object<M: Material>(data: &WasmVecAny, idx: usize, change: ObjectCh
 fn remove<M: Material>(archetype: &mut ObjectArchetype, idx: usize) {
     let data_vec = archetype.data_vec.downcast_slice_mut::<Option<InternalObject<M>>>().unwrap();
 
-    // We don't actually remove the object at this point,
-    // we just mark it as disabled. Next frame, this handle
-    // will be provided in `deferred_removals` in `evaluate`
-    // so we can actually delete it.
-    //
-    // We defer objects one frame so that temporal culling
-    // has valid data.
-    archetype.buffer.use_index(idx);
-    data_vec[idx].as_mut().unwrap().inner.enabled = false as u32;
+    // Only one archetype will have each handle,
+    // so if we have it, we can be sure it's ours.
+    let removed_obj = Option::take(&mut data_vec[idx]);
+
+    if removed_obj.is_some() {
+        archetype.object_count -= 1;
+    }
 }
 
 fn evaluate<M: Material>(
     archetype: &mut ObjectArchetype,
     device: &Device,
     encoder: &mut CommandEncoder,
     scatter: &ScatterCopy,
-    deferred_removals: &[RawObjectHandle],
 ) {
     let data_vec = archetype.data_vec.downcast_slice_mut::<Option<InternalObject<M>>>().unwrap();
 
-    for removal in deferred_removals {
-        // Only one archetype will have each handle,
-        // so if we have it, we can be sure it's ours.
-        let removed_obj = data_vec[removal.idx].take();
-
-        if removed_obj.is_some() {
-            archetype.object_count -= 1;
-        }
-    }
-
     archetype.buffer.apply(device, encoder, scatter, |idx| data_vec[idx].as_ref().map(|o| o.inner).unwrap_or_default())
 }

rend3/src/renderer/eval.rs

@@ -11,8 +11,6 @@ pub fn evaluate_instructions(renderer: &Renderer) -> InstructionEvaluationOutput
 
     let mut instructions = renderer.instructions.consumer.lock();
 
-    let delayed_object_handles = renderer.resource_handle_allocators.object.reclaim_delayed_handles();
-
     // 16 encoders is a reasonable default
     let mut cmd_bufs = Vec::with_capacity(16);
 
@@ -157,7 +155,7 @@ pub fn evaluate_instructions(renderer: &Renderer) -> InstructionEvaluationOutput
 
     // Do these in dependency order
     // Level 3
-    data_core.object_manager.evaluate(&renderer.device, &mut encoder, &renderer.scatter, &delayed_object_handles);
+    data_core.object_manager.evaluate(&renderer.device, &mut encoder, &renderer.scatter);
 
     // Level 2
     let d2_texture = data_core.d2_texture_manager.evaluate(&renderer.device);

rend3/src/renderer/mod.rs

@@ -82,15 +82,15 @@ struct HandleAllocators {
 impl Default for HandleAllocators {
     fn default() -> Self {
         Self {
-            mesh: HandleAllocator::new(false),
-            skeleton: HandleAllocator::new(false),
-            d2_texture: HandleAllocator::new(false),
-            d2c_texture: HandleAllocator::new(false),
-            material: HandleAllocator::new(false),
-            object: HandleAllocator::new(true),
-            directional_light: HandleAllocator::new(false),
-            point_light: HandleAllocator::new(false),
-            graph_storage: HandleAllocator::new(false),
+            mesh: HandleAllocator::new(),
+            skeleton: HandleAllocator::new(),
+            d2_texture: HandleAllocator::new(),
+            d2c_texture: HandleAllocator::new(),
+            material: HandleAllocator::new(),
+            object: HandleAllocator::new(),
+            directional_light: HandleAllocator::new(),
+            point_light: HandleAllocator::new(),
+            graph_storage: HandleAllocator::new(),
         }
     }
 }