Arcanization of wgpu core resources (#3626)

Arcanization of wgpu_core resources --------- Co-authored-by: Elabajaba <Elabajaba@users.noreply.github.com> Co-authored-by: Niklas Korz <niklas@niklaskorz.de> Co-authored-by: grovesNL <josh@joshgroves.com> Co-authored-by: Jim Blandy <jimb@red-bean.com> Co-authored-by: Mauro Gentile <Mauro.Gentile@ubisoft.com> Co-authored-by: Sludge <96552222+SludgePhD@users.noreply.github.com>
gfx-rs · Nov 20, 2023 · 6e21f7a · 6e21f7a
1 parent a827c18
commit 6e21f7a
Show file tree

Hide file tree

Showing 87 changed files with 7,311 additions and 6,998 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -42,6 +42,25 @@ Bottom level categories:
 
 ### Changes
 
+- Arcanization of wgpu core resources: 
+Removed Token and LifeTime related management
+Removed RefCount and MultiRefCount in favour of using only Arc internal reference count
+Removing mut from resources and added instead internal members locks on demand or atomics operations
+Resources now implement Drop and destroy stuff when last Arc resources is released
+Resources hold an Arc in order to be able to implement Drop
+Resources have an utility to retrieve the id of the resource itself
+Remove all guards and just retrive the Arc needed on-demand to unlock registry of resources asap
+Verify correct resources release when unused or not needed
+Check Web and Metal compliation (thanks to @niklaskorz)
+Fix tests on all platforms
+Test a multithreaded scenario
+Storage is now holding only user-land resources, but Arc is keeping refcount for resources
+When user unregister a resource, it's not dropped if still in use due to refcount inside wgpu
+IdentityManager is now unique and free is called on resource drop instead of storage unregister
+Identity changes due to Arcanization and Registry being just the user reference
+Added MemLeaks test and fixing mem leaks
+By @gents83 in [#3626](https://github.com/gfx-rs/wgpu/pull/3626) and tnx also to @jimblandy, @nical, @Wumpf, @Elabajaba & @cwfitzgerald
+
 #### General
 
 - Log vulkan validation layer messages during instance creation and destruction: By @exrook in [#4586](https://github.com/gfx-rs/wgpu/pull/4586)
@@ -1122,7 +1141,7 @@ both `raw_window_handle::HasRawWindowHandle` and `raw_window_handle::HasRawDispl
 
 #### Vulkan
 
-- Fix `astc_hdr` formats support by @jinleili in [#2971]](https://github.com/gfx-rs/wgpu/pull/2971)
+- Fix `astc_hdr` formats support by @jinleili in [#2971]](https://github.com/gfx-rs/wgpu/pull/2971)
 - Update to Naga b209d911 (2022-9-1) to avoid generating SPIR-V that
   violates Vulkan valid usage rules `VUID-StandaloneSpirv-Flat-06202`
   and `VUID-StandaloneSpirv-Flat-04744`. By @jimblandy in
@@ -2331,4 +2350,4 @@ DeviceDescriptor {
 - concept of the storage hub
 - basic recording of passes and command buffers
 - submission-based lifetime tracking and command buffer recycling
-- automatic resource transitions
+- automatic resource transitions
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/deno_webgpu/buffer.rs b/deno_webgpu/buffer.rs
@@ -106,7 +106,7 @@ pub async fn op_webgpu_buffer_get_map_async(
                     2 => wgpu_core::device::HostMap::Write,
                     _ => unreachable!(),
                 },
-                callback: wgpu_core::resource::BufferMapCallback::from_rust(callback),
+                callback: Some(wgpu_core::resource::BufferMapCallback::from_rust(callback)),
             }
         ))
         .err();

diff --git a/deno_webgpu/error.rs b/deno_webgpu/error.rs
@@ -104,9 +104,7 @@ impl From<DeviceError> for WebGpuError {
         match err {
             DeviceError::Lost => WebGpuError::Lost,
             DeviceError::OutOfMemory => WebGpuError::OutOfMemory,
-            DeviceError::ResourceCreationFailed
-            | DeviceError::Invalid
-            | DeviceError::WrongDevice => WebGpuError::Validation(fmt_err(&err)),
+            _ => WebGpuError::Validation(fmt_err(&err)),
         }
     }
 }

diff --git a/deno_webgpu/lib.rs b/deno_webgpu/lib.rs
@@ -661,10 +661,11 @@ pub async fn op_webgpu_request_device(
         limits: required_limits.unwrap_or_default(),
     };
 
-    let (device, maybe_err) = gfx_select!(adapter => instance.adapter_request_device(
+    let (device, _queue, maybe_err) = gfx_select!(adapter => instance.adapter_request_device(
       adapter,
       &descriptor,
       std::env::var("DENO_WEBGPU_TRACE").ok().as_ref().map(std::path::Path::new),
+      (),
       ()
     ));
     if let Some(err) = maybe_err {

diff --git a/player/src/bin/play.rs b/player/src/bin/play.rs
@@ -54,7 +54,7 @@ fn main() {
         IdentityPassThroughFactory,
         wgt::InstanceDescriptor::default(),
     );
-    let mut command_buffer_id_manager = wgc::identity::IdentityManager::default();
+    let mut command_buffer_id_manager = wgc::identity::IdentityManager::new();
 
     #[cfg(feature = "winit")]
     let surface = unsafe {
@@ -88,10 +88,11 @@ fn main() {
             let info = gfx_select!(adapter => global.adapter_get_info(adapter)).unwrap();
             log::info!("Picked '{}'", info.name);
             let id = wgc::id::TypedId::zip(1, 0, backend);
-            let (_, error) = gfx_select!(adapter => global.adapter_request_device(
+            let (_, _, error) = gfx_select!(adapter => global.adapter_request_device(
                 adapter,
                 &desc,
                 None,
+                id,
                 id
             ));
             if let Some(e) = error {

diff --git a/player/src/lib.rs b/player/src/lib.rs
@@ -10,37 +10,22 @@
 
 use wgc::device::trace;
 
-use std::{borrow::Cow, fmt::Debug, fs, marker::PhantomData, path::Path};
+use std::{borrow::Cow, fs, path::Path};
 
-#[derive(Debug)]
-pub struct IdentityPassThrough<I>(PhantomData<I>);
+pub struct IdentityPassThroughFactory;
 
-impl<I: Clone + Debug + wgc::id::TypedId> wgc::identity::IdentityHandler<I>
-    for IdentityPassThrough<I>
-{
+impl<I: wgc::id::TypedId> wgc::identity::IdentityHandlerFactory<I> for IdentityPassThroughFactory {
     type Input = I;
-    fn process(&self, id: I, backend: wgt::Backend) -> I {
-        let (index, epoch, _backend) = id.unzip();
-        I::zip(index, epoch, backend)
-    }
-    fn free(&self, _id: I) {}
-}
 
-pub struct IdentityPassThroughFactory;
-
-impl<I: Clone + Debug + wgc::id::TypedId> wgc::identity::IdentityHandlerFactory<I>
-    for IdentityPassThroughFactory
-{
-    type Filter = IdentityPassThrough<I>;
-    fn spawn(&self) -> Self::Filter {
-        IdentityPassThrough(PhantomData)
+    fn input_to_id(id_in: Self::Input) -> I {
+        id_in
     }
-}
-impl wgc::identity::GlobalIdentityHandlerFactory for IdentityPassThroughFactory {
-    fn ids_are_generated_in_wgpu() -> bool {
+
+    fn autogenerate_ids() -> bool {
         false
     }
 }
+impl wgc::identity::GlobalIdentityHandlerFactory for IdentityPassThroughFactory {}
 
 pub trait GlobalPlay {
     fn encode_commands<A: wgc::hal_api::HalApi>(
@@ -53,7 +38,7 @@ pub trait GlobalPlay {
         device: wgc::id::DeviceId,
         action: trace::Action,
         dir: &Path,
-        comb_manager: &mut wgc::identity::IdentityManager,
+        comb_manager: &mut wgc::identity::IdentityManager<wgc::id::CommandBufferId>,
     );
 }
 
@@ -168,10 +153,10 @@ impl GlobalPlay for wgc::global::Global<IdentityPassThroughFactory> {
         device: wgc::id::DeviceId,
         action: trace::Action,
         dir: &Path,
-        comb_manager: &mut wgc::identity::IdentityManager,
+        comb_manager: &mut wgc::identity::IdentityManager<wgc::id::CommandBufferId>,
     ) {
         use wgc::device::trace::Action;
-        log::info!("action {:?}", action);
+        log::debug!("action {:?}", action);
         //TODO: find a way to force ID perishing without excessive `maintain()` calls.
         match action {
             Action::Init { .. } => {
@@ -269,7 +254,7 @@ impl GlobalPlay for wgc::global::Global<IdentityPassThroughFactory> {
                 self.bind_group_drop::<A>(id);
             }
             Action::CreateShaderModule { id, desc, data } => {
-                log::info!("Creating shader from {}", data);
+                log::debug!("Creating shader from {}", data);
                 let code = fs::read_to_string(dir.join(&data)).unwrap();
                 let source = if data.ends_with(".wgsl") {
                     wgc::pipeline::ShaderModuleSource::Wgsl(Cow::Owned(code.clone()))
@@ -390,7 +375,7 @@ impl GlobalPlay for wgc::global::Global<IdentityPassThroughFactory> {
                 let (encoder, error) = self.device_create_command_encoder::<A>(
                     device,
                     &wgt::CommandEncoderDescriptor { label: None },
-                    comb_manager.alloc(device.backend()),
+                    comb_manager.process(device.backend()),
                 );
                 if let Some(e) = error {
                     panic!("{e}");

diff --git a/player/tests/data/bind-group.ron b/player/tests/data/bind-group.ron
@@ -2,30 +2,6 @@
     features: 0x0,
     expectations: [], //not crash!
     actions: [
-        CreatePipelineLayout(Id(0, 1, Empty), (
-            label: Some("empty"),
-            bind_group_layouts: [],
-            push_constant_ranges: [],
-        )),
-        CreateShaderModule(
-            id: Id(0, 1, Empty),
-            desc: (
-                label: None,
-                flags: (bits: 3),
-            ),
-            data: "empty.wgsl",
-        ),
-        CreateComputePipeline(
-            id: Id(0, 1, Empty),
-            desc: (
-                label: None,
-                layout: Some(Id(0, 1, Empty)),
-                stage: (
-                    module: Id(0, 1, Empty),
-                    entry_point: "main",
-                ),
-            ),
-        ),
         CreateBuffer(Id(0, 1, Empty), (
             label: None,
             size: 16,
@@ -58,16 +34,42 @@
                 )
             ],
         )),
+        CreatePipelineLayout(Id(0, 1, Empty), (
+            label: Some("empty"),
+            bind_group_layouts: [
+                Id(0, 1, Empty),
+            ],
+            push_constant_ranges: [],
+        )),
+        CreateShaderModule(
+            id: Id(0, 1, Empty),
+            desc: (
+                label: None,
+                flags: (bits: 3),
+            ),
+            data: "empty.wgsl",
+        ),
+        CreateComputePipeline(
+            id: Id(0, 1, Empty),
+            desc: (
+                label: None,
+                layout: Some(Id(0, 1, Empty)),
+                stage: (
+                    module: Id(0, 1, Empty),
+                    entry_point: "main",
+                ),
+            ),
+        ),
         Submit(1, [
             RunComputePass(
                 base: (
                     commands: [
-                        SetPipeline(Id(0, 1, Empty)),
                         SetBindGroup(
                             index: 0,
                             num_dynamic_offsets: 0,
                             bind_group_id: Id(0, 1, Empty),
                         ),
+                        SetPipeline(Id(0, 1, Empty)),
                     ],
                     dynamic_offsets: [],
                     string_data: [],

diff --git a/player/tests/data/zero-init-texture-binding.ron b/player/tests/data/zero-init-texture-binding.ron
@@ -56,7 +56,7 @@
             format: "rgba8unorm",
             usage: 9, // STORAGE + COPY_SRC
             view_formats: [],
-        )),
+        )),        
         CreateTextureView(
             id: Id(1, 1, Empty),
             parent_id: Id(1, 1, Empty),