remove Tlas/Blas destroy methods

`Tlas.destroy` didn't check if the `Tlas` is used in a bind group of an active submission. The only reason we need the `destroy` methods for textures and buffers is because they allow users to eagerly release memory in browser implementations. I think we can remove the destroy methods on the acceleration structures for now as they complicate the picture without any gain. If they will be needed for Firefox we can add them back.

Author: teoxoy

player/src/lib.rs

@@ -451,18 +451,12 @@ impl GlobalPlay for wgc::global::Global {
             Action::CreateBlas { id, desc, sizes } => {
                 self.device_create_blas(device, &desc, sizes, Some(id));
             }
-            Action::FreeBlas(id) => {
-                self.blas_destroy(id).unwrap();
-            }
             Action::DestroyBlas(id) => {
                 self.blas_drop(id);
             }
             Action::CreateTlas { id, desc } => {
                 self.device_create_tlas(device, &desc, Some(id));
             }
-            Action::FreeTlas(id) => {
-                self.tlas_destroy(id).unwrap();
-            }
             Action::DestroyTlas(id) => {
                 self.tlas_drop(id);
             }

wgpu-core/src/device/life.rs

@@ -9,7 +9,6 @@ use crate::{
 };
 use smallvec::SmallVec;
 
-use crate::resource::{Blas, Tlas};
 use std::sync::Arc;
 use thiserror::Error;
 
@@ -101,26 +100,6 @@ impl ActiveSubmission {
 
         false
     }
-
-    pub fn contains_blas(&self, blas: &Blas) -> bool {
-        for encoder in &self.encoders {
-            if encoder.trackers.blas_s.contains(blas) {
-                return true;
-            }
-        }
-
-        false
-    }
-
-    pub fn contains_tlas(&self, tlas: &Tlas) -> bool {
-        for encoder in &self.encoders {
-            if encoder.trackers.tlas_s.contains(tlas) {
-                return true;
-            }
-        }
-
-        false
-    }
 }
 
 #[derive(Clone, Debug, Error)]
@@ -230,34 +209,6 @@ impl LifetimeTracker {
         })
     }
 
-    /// Returns the submission index of the most recent submission that uses the
-    /// given blas.
-    pub fn get_blas_latest_submission_index(&self, blas: &Blas) -> Option<SubmissionIndex> {
-        // We iterate in reverse order, so that we can bail out early as soon
-        // as we find a hit.
-        self.active.iter().rev().find_map(|submission| {
-            if submission.contains_blas(blas) {
-                Some(submission.index)
-            } else {
-                None
-            }
-        })
-    }
-
-    /// Returns the submission index of the most recent submission that uses the
-    /// given tlas.
-    pub fn get_tlas_latest_submission_index(&self, tlas: &Tlas) -> Option<SubmissionIndex> {
-        // We iterate in reverse order, so that we can bail out early as soon
-        // as we find a hit.
-        self.active.iter().rev().find_map(|submission| {
-            if submission.contains_tlas(tlas) {
-                Some(submission.index)
-            } else {
-                None
-            }
-        })
-    }
-
     /// Returns the submission index of the most recent submission that uses the
     /// given texture.
     pub fn get_texture_latest_submission_index(

wgpu-core/src/device/queue.rs

@@ -28,7 +28,6 @@ use crate::{
 
 use smallvec::SmallVec;
 
-use crate::resource::DestroyedAccelerationStructure;
 use crate::scratch::ScratchBuffer;
 use std::{
     iter,
@@ -252,7 +251,6 @@ pub enum TempResource {
     ScratchBuffer(ScratchBuffer),
     DestroyedBuffer(DestroyedBuffer),
     DestroyedTexture(DestroyedTexture),
-    DestroyedAccelerationStructure(DestroyedAccelerationStructure),
 }
 
 /// A series of raw [`CommandBuffer`]s that have been submitted to a

wgpu-core/src/device/ray_tracing.rs

@@ -4,10 +4,9 @@ use std::sync::Arc;
 use crate::api_log;
 #[cfg(feature = "trace")]
 use crate::device::trace;
-use crate::lock::{rank, Mutex};
+use crate::lock::rank;
 use crate::resource::{Fallible, TrackingData};
 use crate::snatch::Snatchable;
-use crate::weak_vec::WeakVec;
 use crate::{
     device::{Device, DeviceError},
     global::Global,
@@ -166,7 +165,6 @@ impl Device {
             label: desc.label.to_string(),
             max_instance_count: desc.max_instances,
             tracking_data: TrackingData::new(self.tracker_indices.tlas_s.clone()),
-            bind_groups: Mutex::new(rank::TLAS_BIND_GROUPS, WeakVec::new()),
         }))
     }
 }
@@ -247,20 +245,6 @@ impl Global {
         (id, Some(error))
     }
 
-    pub fn blas_destroy(&self, blas_id: BlasId) -> Result<(), resource::DestroyError> {
-        profiling::scope!("Blas::destroy");
-        api_log!("Blas::destroy {blas_id:?}");
-
-        let blas = self.hub.blas_s.get(blas_id).get()?;
-
-        #[cfg(feature = "trace")]
-        if let Some(trace) = blas.device.trace.lock().as_mut() {
-            trace.add(trace::Action::FreeBlas(blas_id));
-        }
-
-        blas.destroy()
-    }
-
     pub fn blas_drop(&self, blas_id: BlasId) {
         profiling::scope!("Blas::drop");
         api_log!("Blas::drop {blas_id:?}");
@@ -275,20 +259,6 @@ impl Global {
         }
     }
 
-    pub fn tlas_destroy(&self, tlas_id: TlasId) -> Result<(), resource::DestroyError> {
-        profiling::scope!("Tlas::destroy");
-        api_log!("Tlas::destroy {tlas_id:?}");
-
-        let tlas = self.hub.tlas_s.get(tlas_id).get()?;
-
-        #[cfg(feature = "trace")]
-        if let Some(trace) = tlas.device.trace.lock().as_mut() {
-            trace.add(trace::Action::FreeTlas(tlas_id));
-        }
-
-        tlas.destroy()
-    }
-
     pub fn tlas_drop(&self, tlas_id: TlasId) {
         profiling::scope!("Tlas::drop");
         api_log!("Tlas::drop {tlas_id:?}");

wgpu-core/src/device/trace.rs

@@ -132,13 +132,11 @@ pub enum Action<'a> {
         desc: crate::resource::BlasDescriptor<'a>,
         sizes: wgt::BlasGeometrySizeDescriptors,
     },
-    FreeBlas(id::BlasId),
     DestroyBlas(id::BlasId),
     CreateTlas {
         id: id::TlasId,
         desc: crate::resource::TlasDescriptor<'a>,
     },
-    FreeTlas(id::TlasId),
     DestroyTlas(id::TlasId),
 }
 

wgpu-core/src/lock/rank.rs

@@ -148,7 +148,6 @@ define_lock_ranks! {
     rank BLAS_BUILT_INDEX "Blas::built_index" followed by { }
     rank TLAS_BUILT_INDEX "Tlas::built_index" followed by { }
     rank TLAS_DEPENDENCIES "Tlas::dependencies" followed by { }
-    rank TLAS_BIND_GROUPS "Tlas::bind_groups" followed by { }
 
     #[cfg(test)]
     rank PAWN "pawn" followed by { ROOK, BISHOP }

wgpu-core/src/resource.rs

@@ -1898,42 +1898,6 @@ impl AccelerationStructure for Blas {
     }
 }
 
-impl Blas {
-    pub(crate) fn destroy(self: &Arc<Self>) -> Result<(), DestroyError> {
-        let device = &self.device;
-
-        let temp = {
-            let mut snatch_guard = device.snatchable_lock.write();
-
-            let raw = match self.raw.snatch(&mut snatch_guard) {
-                Some(raw) => raw,
-                None => {
-                    return Err(DestroyError::AlreadyDestroyed);
-                }
-            };
-
-            drop(snatch_guard);
-
-            queue::TempResource::DestroyedAccelerationStructure(DestroyedAccelerationStructure {
-                raw: ManuallyDrop::new(raw),
-                device: Arc::clone(&self.device),
-                label: self.label().to_owned(),
-                bind_groups: WeakVec::new(),
-            })
-        };
-
-        if let Some(queue) = device.get_queue() {
-            let mut life_lock = queue.lock_life();
-            let last_submit_index = life_lock.get_blas_latest_submission_index(self);
-            if let Some(last_submit_index) = last_submit_index {
-                life_lock.schedule_resource_destruction(temp, last_submit_index);
-            }
-        }
-
-        Ok(())
-    }
-}
-
 crate::impl_resource_type!(Blas);
 crate::impl_labeled!(Blas);
 crate::impl_parent_device!(Blas);
@@ -1954,7 +1918,6 @@ pub struct Tlas {
     /// The `label` from the descriptor used to create the resource.
     pub(crate) label: String,
     pub(crate) tracking_data: TrackingData,
-    pub(crate) bind_groups: Mutex<WeakVec<BindGroup>>,
 }
 
 impl Drop for Tlas {
@@ -1987,71 +1950,3 @@ crate::impl_labeled!(Tlas);
 crate::impl_parent_device!(Tlas);
 crate::impl_storage_item!(Tlas);
 crate::impl_trackable!(Tlas);
-
-impl Tlas {
-    pub(crate) fn destroy(self: &Arc<Self>) -> Result<(), DestroyError> {
-        let device = &self.device;
-
-        let temp = {
-            let mut snatch_guard = device.snatchable_lock.write();
-
-            let raw = match self.raw.snatch(&mut snatch_guard) {
-                Some(raw) => raw,
-                None => {
-                    return Err(DestroyError::AlreadyDestroyed);
-                }
-            };
-
-            drop(snatch_guard);
-
-            queue::TempResource::DestroyedAccelerationStructure(DestroyedAccelerationStructure {
-                raw: ManuallyDrop::new(raw),
-                device: Arc::clone(&self.device),
-                label: self.label().to_owned(),
-                bind_groups: mem::take(&mut self.bind_groups.lock()),
-            })
-        };
-
-        if let Some(queue) = device.get_queue() {
-            let mut life_lock = queue.lock_life();
-            let last_submit_index = life_lock.get_tlas_latest_submission_index(self);
-            if let Some(last_submit_index) = last_submit_index {
-                life_lock.schedule_resource_destruction(temp, last_submit_index);
-            }
-        }
-
-        Ok(())
-    }
-}
-
-#[derive(Debug)]
-pub struct DestroyedAccelerationStructure {
-    raw: ManuallyDrop<Box<dyn hal::DynAccelerationStructure>>,
-    device: Arc<Device>,
-    label: String,
-    // only filled if the acceleration structure is a TLAS
-    bind_groups: WeakVec<BindGroup>,
-}
-
-impl DestroyedAccelerationStructure {
-    pub fn label(&self) -> &dyn Debug {
-        &self.label
-    }
-}
-
-impl Drop for DestroyedAccelerationStructure {
-    fn drop(&mut self) {
-        let mut deferred = self.device.deferred_destroy.lock();
-        deferred.push(DeferredDestroy::BindGroups(mem::take(
-            &mut self.bind_groups,
-        )));
-        drop(deferred);
-
-        resource_log!("Destroy raw Buffer (destroyed) {:?}", self.label());
-        // SAFETY: We are in the Drop impl and we don't use self.raw anymore after this point.
-        let raw = unsafe { ManuallyDrop::take(&mut self.raw) };
-        unsafe {
-            hal::DynDevice::destroy_acceleration_structure(self.device.raw(), raw);
-        }
-    }
-}

wgpu-core/src/track/ray_tracing.rs

@@ -46,12 +46,6 @@ impl<T: AccelerationStructure> AccelerationStructureTracker<T> {
         }
     }
 
-    /// Returns true if the given buffer is tracked.
-    pub fn contains(&self, acceleration_structure: &T) -> bool {
-        self.metadata
-            .contains(acceleration_structure.tracker_index().as_usize())
-    }
-
     /// Inserts a single resource into the resource tracker.
     pub fn set_single(&mut self, resource: Arc<T>) {
         let index: usize = resource.tracker_index().as_usize();

wgpu/src/api/blas.rs

@@ -1,4 +1,4 @@
-use crate::context::{Context, DynContext};
+use crate::context::Context;
 use crate::{Buffer, Data, Label, C};
 use std::sync::Arc;
 use std::thread;
@@ -35,8 +35,7 @@ static_assertions::assert_impl_all!(CreateBlasDescriptor<'_>: Send, Sync);
 ///
 /// Each one contains:
 /// - A reference to a BLAS, this ***must*** be interacted with using [TlasInstance::new] or [TlasInstance::set_blas], a
-///   TlasInstance that references a BLAS keeps that BLAS from being dropped, but if the BLAS is explicitly destroyed (e.g.
-///   using [Blas::destroy]) the TlasInstance becomes invalid
+///   TlasInstance that references a BLAS keeps that BLAS from being dropped
 /// - A user accessible transformation matrix
 /// - A user accessible mask
 /// - A user accessible custom index
@@ -171,10 +170,6 @@ impl Blas {
     pub fn handle(&self) -> Option<u64> {
         self.handle
     }
-    /// Destroy the associated native resources as soon as possible.
-    pub fn destroy(&self) {
-        DynContext::blas_destroy(&*self.shared.context, self.shared.data.as_ref());
-    }
 }
 
 impl Drop for BlasShared {

wgpu/src/api/tlas.rs

@@ -1,5 +1,5 @@
 use crate::api::blas::{ContextTlasInstance, DynContextTlasInstance, TlasInstance};
-use crate::context::{Context, DynContext};
+use crate::context::Context;
 use crate::{BindingResource, Buffer, Data, Label, C};
 use std::ops::{Index, IndexMut, Range};
 use std::sync::Arc;
@@ -27,13 +27,6 @@ pub struct Tlas {
 }
 static_assertions::assert_impl_all!(Tlas: WasmNotSendSync);
 
-impl Tlas {
-    /// Destroy the associated native resources as soon as possible.
-    pub fn destroy(&self) {
-        DynContext::tlas_destroy(&*self.context, self.data.as_ref());
-    }
-}
-
 impl Drop for Tlas {
     fn drop(&mut self) {
         if !thread::panicking() {

wgpu/src/backend/webgpu.rs

@@ -3487,18 +3487,10 @@ impl crate::context::Context for ContextWebGpu {
         unimplemented!("Raytracing not implemented for web");
     }
 
-    fn blas_destroy(&self, _blas_data: &Self::BlasData) {
-        unimplemented!("Raytracing not implemented for web");
-    }
-
     fn blas_drop(&self, _blas_data: &Self::BlasData) {
         unimplemented!("Raytracing not implemented for web");
     }
 
-    fn tlas_destroy(&self, _tlas_data: &Self::TlasData) {
-        unimplemented!("Raytracing not implemented for web");
-    }
-
     fn tlas_drop(&self, _tlas_data: &Self::TlasData) {
         unimplemented!("Raytracing not implemented for web");
     }