From 6d5528e51c41cee68d93202c6188e6948024176d Mon Sep 17 00:00:00 2001 From: Nimi Wariboko Jr Date: Mon, 21 Dec 2020 13:59:21 -0800 Subject: [PATCH 01/15] [gl] add support for wayland display servers --- src/backend/gl/src/window/egl.rs | 330 +++++++++++++++++++++++-------- 1 file changed, 252 insertions(+), 78 deletions(-) diff --git a/src/backend/gl/src/window/egl.rs b/src/backend/gl/src/window/egl.rs index 2efb5d94b09..fba57eaac4b 100644 --- a/src/backend/gl/src/window/egl.rs +++ b/src/backend/gl/src/window/egl.rs @@ -3,7 +3,7 @@ use crate::{conv, native, GlContainer, PhysicalDevice, Starc}; use glow::HasContext; use hal::{image, window as w}; -use std::{os::raw, ptr}; +use std::{os::raw, ptr, sync::Mutex}; #[derive(Debug)] pub struct Swapchain { @@ -18,67 +18,57 @@ pub struct Swapchain { #[derive(Debug)] pub struct Instance { egl: Starc, - display: egl::Display, + wsi_library: Option, + inner: Mutex, +} + +#[derive(Debug)] +pub struct Inner { + egl: Starc, version: (i32, i32), + supports_native_window: bool, + display: egl::Display, config: egl::Config, context: egl::Context, - supports_native_window: bool, - wsi_library: Option, + wl_display: Option<*mut raw::c_void>, } unsafe impl Send for Instance {} unsafe impl Sync for Instance {} +const EGL_PLATFORM_WAYLAND_KHR: u32 = 0x31D8; +const EGL_PLATFORM_X11_KHR: u32 = 0x31D5; + type XOpenDisplayFun = unsafe extern "system" fn(display_name: *const raw::c_char) -> *mut raw::c_void; -impl hal::Instance for Instance { - fn create(_: &str, _: u32) -> Result { - let egl = match unsafe { egl::DynamicInstance::load() } { - Ok(egl) => Starc::new(egl), - Err(e) => { - log::warn!("Unable to open libEGL.so: {:?}", e); - return Err(hal::UnsupportedBackend); - } - }; - - let client_extensions = egl - .query_string(None, egl::EXTENSIONS) - .map_err(|_| hal::UnsupportedBackend)? - .to_string_lossy(); - log::info!("Client extensions: {:?}", client_extensions); - let client_ext_list = client_extensions.split_whitespace().collect::>(); - - let mut wsi_library = None; - let x11_display = if client_ext_list.contains(&"EGL_EXT_platform_x11") { - log::info!("Loading X11 library to get the current display"); - if let Ok(library) = libloading::Library::new("libX11.so") { - let func: libloading::Symbol = - unsafe { library.get(b"XOpenDisplay").unwrap() }; - let result = unsafe { func(ptr::null()) }; - wsi_library = Some(library); - ptr::NonNull::new(result) - } else { - None - } - } else { - None - }; - let display = if let Some(x11_display) = x11_display { - log::info!("Using X11 platform"); - const EGL_PLATFORM_X11_KHR: u32 = 0x31D5; - let display_attributes = [egl::ATTRIB_NONE]; - egl.get_platform_display( - EGL_PLATFORM_X11_KHR, - x11_display.as_ptr(), - &display_attributes, - ) - .unwrap() - } else { - log::info!("Using default platform"); - egl.get_display(egl::DEFAULT_DISPLAY).unwrap() - }; +type WlDisplayConnectFun = + unsafe extern "system" fn(display_name: *const raw::c_char) -> *mut raw::c_void; +type WlDisplayDisconnectFun = unsafe extern "system" fn(display: *const raw::c_void); + +type WlEglWindowCreateFun = unsafe extern "system" fn( + surface: *const raw::c_void, + width: raw::c_int, + height: raw::c_int, +) -> *mut raw::c_void; + +type WlEglWindowResizeFun = unsafe extern "system" fn( + window: *const raw::c_void, + width: raw::c_int, + height: raw::c_int, + dx: raw::c_int, + dy: raw::c_int, +); + +type WlEglWindowDestroyFun = unsafe extern "system" fn(window: *const raw::c_void); + +impl Inner { + fn create( + egl: Starc, + display: egl::Display, + wsi_library: Option<&libloading::Library>, + ) -> Result { let version = egl .initialize(display) .map_err(|_| hal::UnsupportedBackend)?; @@ -177,20 +167,132 @@ impl hal::Instance for Instance { } }; - Ok(Instance { + Ok(Self { egl, display, version, + supports_native_window, config, context, - supports_native_window, + wl_display: None, + }) + } +} + +impl Drop for Inner { + fn drop(&mut self) { + if let Err(e) = self.egl.destroy_context(self.display, self.context) { + log::warn!("Error in destroy_context: {:?}", e); + } + if let Err(e) = self.egl.terminate(self.display) { + log::warn!("Error in terminate: {:?}", e); + } + } +} + +impl hal::Instance for Instance { + fn create(_: &str, _: u32) -> Result { + let egl = match unsafe { egl::DynamicInstance::load() } { + Ok(egl) => Starc::new(egl), + Err(e) => { + log::warn!("Unable to open libEGL.so: {:?}", e); + return Err(hal::UnsupportedBackend); + } + }; + + let client_extensions = egl + .query_string(None, egl::EXTENSIONS) + .map_err(|_| hal::UnsupportedBackend)? + .to_string_lossy(); + log::info!("Client extensions: {:?}", client_extensions); + let client_ext_list = client_extensions.split_whitespace().collect::>(); + + let mut wsi_library = None; + + let wayland_display = if client_ext_list.contains(&"EGL_EXT_platform_wayland") { + log::info!("Loading Wayland library to get the current display"); + if let Ok(library) = libloading::Library::new("libwayland-client.so") { + /* We try to connect and disconnect here to simply ensure there + * is an active wayland display available. + */ + let wl_display_connect: libloading::Symbol = + unsafe { library.get(b"wl_display_connect").unwrap() }; + let wl_display_disconnect: libloading::Symbol = + unsafe { library.get(b"wl_display_disconnect").unwrap() }; + if let Some(display) = ptr::NonNull::new(unsafe { wl_display_connect(ptr::null()) }) + { + unsafe { wl_display_disconnect(display.as_ptr()) } + if let Ok(library) = libloading::Library::new("libwayland-egl.so") { + Some(((), library)) + } else { + None + } + } else { + None + } + } else { + None + } + } else { + None + }; + + let x11_display = if client_ext_list.contains(&"EGL_EXT_platform_x11") { + log::info!("Loading X11 library to get the current display"); + if let Ok(library) = libloading::Library::new("libX11.so") { + let func: libloading::Symbol = + unsafe { library.get(b"XOpenDisplay").unwrap() }; + let result = unsafe { func(ptr::null()) }; + ptr::NonNull::new(result).map(|ptr| (ptr, library)) + } else { + None + } + } else { + None + }; + + let display = match (wayland_display, x11_display) { + (Some((_, library)), _) => { + log::info!("Using Wayland platform"); + let display_attributes = [egl::ATTRIB_NONE]; + wsi_library = Some(library); + egl.get_platform_display( + EGL_PLATFORM_WAYLAND_KHR, + egl::DEFAULT_DISPLAY, + &display_attributes, + ) + .unwrap() + } + (_, Some((x11_display, library))) => { + log::info!("Using X11 platform"); + let display_attributes = [egl::ATTRIB_NONE]; + wsi_library = Some(library); + egl.get_platform_display( + EGL_PLATFORM_X11_KHR, + x11_display.as_ptr(), + &display_attributes, + ) + .unwrap() + } + _ => { + log::info!("Using default platform"); + egl.get_display(egl::DEFAULT_DISPLAY).unwrap() + } + }; + + let inner = Inner::create(egl.clone(), display, wsi_library.as_ref())?; + + Ok(Instance { + egl, + inner: Mutex::new(inner), wsi_library, }) } fn enumerate_adapters(&self) -> Vec> { + let inner = self.inner.lock().unwrap(); self.egl - .make_current(self.display, None, None, Some(self.context)) + .make_current(inner.display, None, None, Some(inner.context)) .unwrap(); let context = unsafe { glow::Context::from_loader_function(|name| { @@ -206,6 +308,8 @@ impl hal::Instance for Instance { has_handle: &impl raw_window_handle::HasRawWindowHandle, ) -> Result { use raw_window_handle::RawWindowHandle as Rwh; + + let mut inner = self.inner.lock().unwrap(); let mut native_window = match has_handle.raw_window_handle() { #[cfg(not(target_os = "android"))] Rwh::Xlib(handle) => handle.window, @@ -213,28 +317,83 @@ impl hal::Instance for Instance { Rwh::Xcb(handle) => handle.window as _, #[cfg(target_os = "android")] Rwh::Android(handle) => handle.a_native_window, + #[cfg(not(target_os = "android"))] + Rwh::Wayland(handle) => { + /* Wayland displays are not sharable between surfaces so if the + * surface we receive from this handle is from a different + * display, we must re-initialize the context. + * + * See gfx-rs/gfx#3545 + */ + if inner + .wl_display + .map(|ptr| ptr != handle.display) + .unwrap_or(true) + { + use std::ops::DerefMut; + let display_attributes = [egl::ATTRIB_NONE]; + let display = self + .egl + .get_platform_display( + EGL_PLATFORM_WAYLAND_KHR, + handle.display, + &display_attributes, + ) + .unwrap(); + + let new_inner = + Inner::create(self.egl.clone(), display, self.wsi_library.as_ref()) + .map_err(|_| w::InitError::UnsupportedWindowHandle)?; + + let old_inner = std::mem::replace(inner.deref_mut(), new_inner); + inner.wl_display = Some(handle.display); + drop(old_inner); + } + + let window = { + let wl_egl_window_create: libloading::Symbol = self + .wsi_library + .as_ref() + .expect("unsupported window") + .get(b"wl_egl_window_create") + .unwrap(); + let result = wl_egl_window_create(handle.surface, 640, 480); + ptr::NonNull::new(result) + }; + window.expect("unsupported window").as_ptr() as _ + } other => panic!("Unsupported window: {:?}", other), }; - let attributes = [ - egl::RENDER_BUFFER as usize, - egl::SINGLE_BUFFER as usize, - // Always enable sRGB - egl::GL_COLORSPACE as usize, - egl::GL_COLORSPACE_SRGB as usize, - egl::ATTRIB_NONE, - ]; + let mut attributes = vec![egl::RENDER_BUFFER as usize, egl::SINGLE_BUFFER as usize]; + if inner.version >= (1, 5) { + // Always enable sRGB in EGL 1.5 + attributes.push(egl::GL_COLORSPACE as usize); + attributes.push(egl::GL_COLORSPACE_SRGB as usize); + } + attributes.push(egl::ATTRIB_NONE); + + let native_window = match has_handle.raw_window_handle() { + #[cfg(not(target_os = "android"))] + Rwh::Wayland(_) => native_window as *mut raw::c_void, + _ => &mut native_window as *mut _ as *mut _, + }; match self.egl.create_platform_window_surface( - self.display, - self.config, - &mut native_window as *mut _ as *mut _, + inner.display, + inner.config, + native_window, &attributes, ) { Ok(raw) => Ok(Surface { egl: self.egl.clone(), raw, - display: self.display, - context: self.context, - presentable: self.supports_native_window, + display: inner.display, + context: inner.context, + presentable: inner.supports_native_window, + wl_window: match has_handle.raw_window_handle() { + #[cfg(not(target_os = "android"))] + Rwh::Wayland(_) => Some(native_window), + _ => None, + }, swapchain: None, }), Err(e) => { @@ -245,17 +404,18 @@ impl hal::Instance for Instance { } unsafe fn destroy_surface(&self, surface: Surface) { - self.egl.destroy_surface(self.display, surface.raw).unwrap(); - } -} - -impl Drop for Instance { - fn drop(&mut self) { - if let Err(e) = self.egl.destroy_context(self.display, self.context) { - log::warn!("Error in destroy_context: {:?}", e); - } - if let Err(e) = self.egl.terminate(self.display) { - log::warn!("Error in terminate: {:?}", e); + let inner = self.inner.lock().unwrap(); + self.egl + .destroy_surface(inner.display, surface.raw) + .unwrap(); + if let Some(wl_window) = surface.wl_window { + let wl_egl_window_destroy: libloading::Symbol = self + .wsi_library + .as_ref() + .expect("unsupported window") + .get(b"wl_egl_window_destroy") + .unwrap(); + wl_egl_window_destroy(wl_window) } } } @@ -267,6 +427,7 @@ pub struct Surface { display: egl::Display, context: egl::Context, presentable: bool, + wl_window: Option<*mut raw::c_void>, pub(crate) swapchain: Option, } @@ -283,6 +444,19 @@ impl w::PresentationSurface for Surface { ) -> Result<(), w::SwapchainError> { self.unconfigure_swapchain(device); + if let Some(window) = self.wl_window { + let library = libloading::Library::new("libwayland-egl.so").unwrap(); + let wl_egl_window_resize: libloading::Symbol = + library.get(b"wl_egl_window_resize").unwrap(); + wl_egl_window_resize( + window, + config.extent.width as i32, + config.extent.height as i32, + 0, + 0, + ); + } + let desc = conv::describe_format(config.format).unwrap(); let gl = &device.share.context; From 8b55a715b167e2054083bcf3bfb17a45898208b6 Mon Sep 17 00:00:00 2001 From: Nimi Wariboko Jr Date: Tue, 22 Dec 2020 12:14:28 -0800 Subject: [PATCH 02/15] [gl] Cleanup Instance fields; Refactor nested statements --- src/backend/gl/src/window/egl.rs | 133 ++++++++++++++----------------- 1 file changed, 62 insertions(+), 71 deletions(-) diff --git a/src/backend/gl/src/window/egl.rs b/src/backend/gl/src/window/egl.rs index fba57eaac4b..a80f2f41c3e 100644 --- a/src/backend/gl/src/window/egl.rs +++ b/src/backend/gl/src/window/egl.rs @@ -17,7 +17,6 @@ pub struct Swapchain { #[derive(Debug)] pub struct Instance { - egl: Starc, wsi_library: Option, inner: Mutex, } @@ -63,6 +62,31 @@ type WlEglWindowResizeFun = unsafe extern "system" fn( type WlEglWindowDestroyFun = unsafe extern "system" fn(window: *const raw::c_void); +fn open_x_display() -> Option<(ptr::NonNull, libloading::Library)> { + log::info!("Loading X11 library to get the current display"); + let library = libloading::Library::new("libX11.so").ok()?; + let func: libloading::Symbol = + unsafe { library.get(b"XOpenDisplay").unwrap() }; + let result = unsafe { func(ptr::null()) }; + ptr::NonNull::new(result).map(|ptr| (ptr, library)) +} + +fn test_wayland_display() -> Option { + /* We try to connect and disconnect here to simply ensure there + * is an active wayland display available. + */ + log::info!("Loading Wayland library to get the current display"); + let client_library = libloading::Library::new("libwayland-client.so").ok()?; + let wl_display_connect: libloading::Symbol = + unsafe { client_library.get(b"wl_display_connect").unwrap() }; + let wl_display_disconnect: libloading::Symbol = + unsafe { client_library.get(b"wl_display_disconnect").unwrap() }; + let display = ptr::NonNull::new(unsafe { wl_display_connect(ptr::null()) })?; + unsafe { wl_display_disconnect(display.as_ptr()) }; + let library = libloading::Library::new("libwayland-egl.so").ok()?; + Some(library) +} + impl Inner { fn create( egl: Starc, @@ -210,80 +234,45 @@ impl hal::Instance for Instance { let mut wsi_library = None; let wayland_display = if client_ext_list.contains(&"EGL_EXT_platform_wayland") { - log::info!("Loading Wayland library to get the current display"); - if let Ok(library) = libloading::Library::new("libwayland-client.so") { - /* We try to connect and disconnect here to simply ensure there - * is an active wayland display available. - */ - let wl_display_connect: libloading::Symbol = - unsafe { library.get(b"wl_display_connect").unwrap() }; - let wl_display_disconnect: libloading::Symbol = - unsafe { library.get(b"wl_display_disconnect").unwrap() }; - if let Some(display) = ptr::NonNull::new(unsafe { wl_display_connect(ptr::null()) }) - { - unsafe { wl_display_disconnect(display.as_ptr()) } - if let Ok(library) = libloading::Library::new("libwayland-egl.so") { - Some(((), library)) - } else { - None - } - } else { - None - } - } else { - None - } + test_wayland_display() } else { None }; let x11_display = if client_ext_list.contains(&"EGL_EXT_platform_x11") { - log::info!("Loading X11 library to get the current display"); - if let Ok(library) = libloading::Library::new("libX11.so") { - let func: libloading::Symbol = - unsafe { library.get(b"XOpenDisplay").unwrap() }; - let result = unsafe { func(ptr::null()) }; - ptr::NonNull::new(result).map(|ptr| (ptr, library)) - } else { - None - } + open_x_display() } else { None }; - let display = match (wayland_display, x11_display) { - (Some((_, library)), _) => { - log::info!("Using Wayland platform"); - let display_attributes = [egl::ATTRIB_NONE]; - wsi_library = Some(library); - egl.get_platform_display( - EGL_PLATFORM_WAYLAND_KHR, - egl::DEFAULT_DISPLAY, - &display_attributes, - ) - .unwrap() - } - (_, Some((x11_display, library))) => { - log::info!("Using X11 platform"); - let display_attributes = [egl::ATTRIB_NONE]; - wsi_library = Some(library); - egl.get_platform_display( - EGL_PLATFORM_X11_KHR, - x11_display.as_ptr(), - &display_attributes, - ) - .unwrap() - } - _ => { - log::info!("Using default platform"); - egl.get_display(egl::DEFAULT_DISPLAY).unwrap() - } + let display = if let Some(library) = wayland_display { + log::info!("Using Wayland platform"); + let display_attributes = [egl::ATTRIB_NONE]; + wsi_library = Some(library); + egl.get_platform_display( + EGL_PLATFORM_WAYLAND_KHR, + egl::DEFAULT_DISPLAY, + &display_attributes, + ) + .unwrap() + } else if let Some((x11_display, library)) = x11_display { + log::info!("Using X11 platform"); + let display_attributes = [egl::ATTRIB_NONE]; + wsi_library = Some(library); + egl.get_platform_display( + EGL_PLATFORM_X11_KHR, + x11_display.as_ptr(), + &display_attributes, + ) + .unwrap() + } else { + log::info!("Using default platform"); + egl.get_display(egl::DEFAULT_DISPLAY).unwrap() }; let inner = Inner::create(egl.clone(), display, wsi_library.as_ref())?; Ok(Instance { - egl, inner: Mutex::new(inner), wsi_library, }) @@ -291,12 +280,13 @@ impl hal::Instance for Instance { fn enumerate_adapters(&self) -> Vec> { let inner = self.inner.lock().unwrap(); - self.egl + inner + .egl .make_current(inner.display, None, None, Some(inner.context)) .unwrap(); let context = unsafe { glow::Context::from_loader_function(|name| { - self.egl.get_proc_address(name).unwrap() as *const _ + inner.egl.get_proc_address(name).unwrap() as *const _ }) }; // Create physical device @@ -332,7 +322,7 @@ impl hal::Instance for Instance { { use std::ops::DerefMut; let display_attributes = [egl::ATTRIB_NONE]; - let display = self + let display = inner .egl .get_platform_display( EGL_PLATFORM_WAYLAND_KHR, @@ -342,7 +332,7 @@ impl hal::Instance for Instance { .unwrap(); let new_inner = - Inner::create(self.egl.clone(), display, self.wsi_library.as_ref()) + Inner::create(inner.egl.clone(), display, self.wsi_library.as_ref()) .map_err(|_| w::InitError::UnsupportedWindowHandle)?; let old_inner = std::mem::replace(inner.deref_mut(), new_inner); @@ -372,26 +362,26 @@ impl hal::Instance for Instance { } attributes.push(egl::ATTRIB_NONE); - let native_window = match has_handle.raw_window_handle() { + let native_window_ptr = match has_handle.raw_window_handle() { #[cfg(not(target_os = "android"))] Rwh::Wayland(_) => native_window as *mut raw::c_void, _ => &mut native_window as *mut _ as *mut _, }; - match self.egl.create_platform_window_surface( + match inner.egl.create_platform_window_surface( inner.display, inner.config, - native_window, + native_window_ptr, &attributes, ) { Ok(raw) => Ok(Surface { - egl: self.egl.clone(), + egl: inner.egl.clone(), raw, display: inner.display, context: inner.context, presentable: inner.supports_native_window, wl_window: match has_handle.raw_window_handle() { #[cfg(not(target_os = "android"))] - Rwh::Wayland(_) => Some(native_window), + Rwh::Wayland(_) => Some(native_window_ptr), _ => None, }, swapchain: None, @@ -405,7 +395,8 @@ impl hal::Instance for Instance { unsafe fn destroy_surface(&self, surface: Surface) { let inner = self.inner.lock().unwrap(); - self.egl + inner + .egl .destroy_surface(inner.display, surface.raw) .unwrap(); if let Some(wl_window) = surface.wl_window { From 9f7be918b0889f2276a20a1f1a4ff0dd795f363e Mon Sep 17 00:00:00 2001 From: Nimi Wariboko Jr Date: Tue, 22 Dec 2020 16:40:14 -0800 Subject: [PATCH 03/15] [gl] prefer to use parking_lot::Mutex rather than stdlib --- src/backend/gl/src/window/egl.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/backend/gl/src/window/egl.rs b/src/backend/gl/src/window/egl.rs index a80f2f41c3e..a17087b31b7 100644 --- a/src/backend/gl/src/window/egl.rs +++ b/src/backend/gl/src/window/egl.rs @@ -3,7 +3,8 @@ use crate::{conv, native, GlContainer, PhysicalDevice, Starc}; use glow::HasContext; use hal::{image, window as w}; -use std::{os::raw, ptr, sync::Mutex}; +use parking_lot::Mutex; +use std::{os::raw, ptr}; #[derive(Debug)] pub struct Swapchain { @@ -279,7 +280,7 @@ impl hal::Instance for Instance { } fn enumerate_adapters(&self) -> Vec> { - let inner = self.inner.lock().unwrap(); + let inner = self.inner.lock(); inner .egl .make_current(inner.display, None, None, Some(inner.context)) @@ -299,7 +300,7 @@ impl hal::Instance for Instance { ) -> Result { use raw_window_handle::RawWindowHandle as Rwh; - let mut inner = self.inner.lock().unwrap(); + let mut inner = self.inner.lock(); let mut native_window = match has_handle.raw_window_handle() { #[cfg(not(target_os = "android"))] Rwh::Xlib(handle) => handle.window, @@ -394,7 +395,7 @@ impl hal::Instance for Instance { } unsafe fn destroy_surface(&self, surface: Surface) { - let inner = self.inner.lock().unwrap(); + let inner = self.inner.lock(); inner .egl .destroy_surface(inner.display, surface.raw) From 921613fc8c67647dfa66f1e1b612d4e7b406ef36 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Thu, 24 Dec 2020 10:54:46 -0500 Subject: [PATCH 04/15] Make fence resets and presentation semaphores to be externally synchronized. Also, simplify the signature of descriptor set copies. --- examples/mesh-shading/main.rs | 4 ++-- examples/quad/main.rs | 4 ++-- src/backend/dx11/src/device.rs | 8 +++----- src/backend/dx11/src/lib.rs | 6 ++++-- src/backend/dx12/src/device.rs | 8 +++----- src/backend/dx12/src/lib.rs | 2 +- src/backend/empty/src/lib.rs | 7 +++---- src/backend/gl/src/device.rs | 7 ++----- src/backend/gl/src/native.rs | 1 + src/backend/gl/src/queue.rs | 2 +- src/backend/metal/src/command.rs | 8 ++++++-- src/backend/metal/src/device.rs | 5 ++--- src/backend/metal/src/native.rs | 2 ++ src/backend/vulkan/src/device.rs | 10 ++++------ src/backend/vulkan/src/lib.rs | 2 +- src/backend/webgpu/src/command.rs | 2 +- src/backend/webgpu/src/device.rs | 12 +++++++----- src/hal/src/device.rs | 18 +++++++++++------- src/hal/src/queue/mod.rs | 2 +- src/hal/src/window.rs | 4 ++-- 20 files changed, 59 insertions(+), 55 deletions(-) diff --git a/examples/mesh-shading/main.rs b/examples/mesh-shading/main.rs index 19edd0d1bd6..bebba30bbb9 100644 --- a/examples/mesh-shading/main.rs +++ b/examples/mesh-shading/main.rs @@ -583,7 +583,7 @@ where // updated with a CPU->GPU data copy are not in use by the GPU, so we can perform those updates. // In this case there are none to be done, however. unsafe { - let fence = &self.submission_complete_fences[frame_idx]; + let fence = &mut self.submission_complete_fences[frame_idx]; self.device .wait_for_fence(fence, !0) .expect("Failed to wait for fence"); @@ -637,7 +637,7 @@ where let result = self.queue_group.queues[0].present( &mut self.surface, surface_image, - Some(&self.submission_complete_semaphores[frame_idx]), + Some(&mut self.submission_complete_semaphores[frame_idx]), ); self.device.destroy_framebuffer(framebuffer); diff --git a/examples/quad/main.rs b/examples/quad/main.rs index d93d5fc28c6..efad6f36273 100644 --- a/examples/quad/main.rs +++ b/examples/quad/main.rs @@ -808,7 +808,7 @@ where // updated with a CPU->GPU data copy are not in use by the GPU, so we can perform those updates. // In this case there are none to be done, however. unsafe { - let fence = &self.submission_complete_fences[frame_idx]; + let fence = &mut self.submission_complete_fences[frame_idx]; self.device .wait_for_fence(fence, !0) .expect("Failed to wait for fence"); @@ -866,7 +866,7 @@ where let result = self.queue_group.queues[0].present( &mut self.surface, surface_image, - Some(&self.submission_complete_semaphores[frame_idx]), + Some(&mut self.submission_complete_semaphores[frame_idx]), ); self.device.destroy_framebuffer(framebuffer); diff --git a/src/backend/dx11/src/device.rs b/src/backend/dx11/src/device.rs index 1d19b9a82da..aed47da5181 100644 --- a/src/backend/dx11/src/device.rs +++ b/src/backend/dx11/src/device.rs @@ -2113,11 +2113,9 @@ impl device::Device for Device { unsafe fn copy_descriptor_sets<'a, I>(&self, copy_iter: I) where - I: IntoIterator, - I::Item: Borrow>, + I: IntoIterator>, { - for copy in copy_iter { - let _copy = copy.borrow(); + for _copy in copy_iter { //TODO /* for offset in 0 .. copy.count { @@ -2224,7 +2222,7 @@ impl device::Device for Device { })) } - unsafe fn reset_fence(&self, fence: &Fence) -> Result<(), device::OutOfMemory> { + unsafe fn reset_fence(&self, fence: &mut Fence) -> Result<(), device::OutOfMemory> { *fence.mutex.lock() = false; Ok(()) } diff --git a/src/backend/dx11/src/lib.rs b/src/backend/dx11/src/lib.rs index d68c1e116cb..22bc98d8c7b 100644 --- a/src/backend/dx11/src/lib.rs +++ b/src/backend/dx11/src/lib.rs @@ -971,7 +971,9 @@ impl window::PresentationSurface for Surface { // We must also delete the image data. // // This should not panic as all images must be deleted before - let mut present_image = Arc::try_unwrap(present.image).expect("Not all acquired images were deleted before the swapchain was reconfigured."); + let mut present_image = Arc::try_unwrap(present.image).expect( + "Not all acquired images were deleted before the swapchain was reconfigured.", + ); present_image.internal.release_resources(); let result = present.swapchain.ResizeBuffers( @@ -1167,7 +1169,7 @@ impl queue::CommandQueue for CommandQueue { &mut self, surface: &mut Surface, _image: SwapchainImage, - _wait_semaphore: Option<&Semaphore>, + _wait_semaphore: Option<&mut Semaphore>, ) -> Result, window::PresentError> { let mut presentation = surface.presentation.as_mut().unwrap(); let (interval, flags) = match presentation.mode { diff --git a/src/backend/dx12/src/device.rs b/src/backend/dx12/src/device.rs index d9f704993ff..cf15856575f 100644 --- a/src/backend/dx12/src/device.rs +++ b/src/backend/dx12/src/device.rs @@ -3187,13 +3187,11 @@ impl d::Device for Device { unsafe fn copy_descriptor_sets<'a, I>(&self, copy_iter: I) where - I: IntoIterator, - I::Item: Borrow>, + I: IntoIterator>, { let mut accum = descriptors_cpu::MultiCopyAccumulator::default(); - for copy_wrap in copy_iter { - let copy = copy_wrap.borrow(); + for copy in copy_iter { let src_info = ©.src_set.binding_infos[copy.src_binding as usize]; let dst_info = ©.dst_set.binding_infos[copy.dst_binding as usize]; @@ -3353,7 +3351,7 @@ impl d::Device for Device { }) } - unsafe fn reset_fence(&self, fence: &r::Fence) -> Result<(), d::OutOfMemory> { + unsafe fn reset_fence(&self, fence: &mut r::Fence) -> Result<(), d::OutOfMemory> { assert_eq!(winerror::S_OK, fence.raw.signal(0)); Ok(()) } diff --git a/src/backend/dx12/src/lib.rs b/src/backend/dx12/src/lib.rs index 907977263ff..ec82c62f672 100644 --- a/src/backend/dx12/src/lib.rs +++ b/src/backend/dx12/src/lib.rs @@ -511,7 +511,7 @@ impl q::CommandQueue for CommandQueue { &mut self, surface: &mut window::Surface, image: window::SwapchainImage, - _wait_semaphore: Option<&resource::Semaphore>, + _wait_semaphore: Option<&mut resource::Semaphore>, ) -> Result, hal::window::PresentError> { surface.present(image).map(|()| None) } diff --git a/src/backend/empty/src/lib.rs b/src/backend/empty/src/lib.rs index dcaeb9a7a3b..3fd2114c52a 100644 --- a/src/backend/empty/src/lib.rs +++ b/src/backend/empty/src/lib.rs @@ -179,7 +179,7 @@ impl queue::CommandQueue for CommandQueue { &mut self, _surface: &mut Surface, _image: SwapchainImage, - _wait_semaphore: Option<&()>, + _wait_semaphore: Option<&mut ()>, ) -> Result, window::PresentError> { Ok(None) } @@ -420,8 +420,7 @@ impl device::Device for Device { unsafe fn copy_descriptor_sets<'a, I>(&self, _: I) where - I: IntoIterator, - I::Item: Borrow>, + I: IntoIterator>, { unimplemented!("{}", NOT_SUPPORTED_MESSAGE) } @@ -584,7 +583,7 @@ impl device::Device for Device { unimplemented!("{}", NOT_SUPPORTED_MESSAGE) } - unsafe fn reset_fence(&self, _: &()) -> Result<(), device::OutOfMemory> { + unsafe fn reset_fence(&self, _: &mut ()) -> Result<(), device::OutOfMemory> { Ok(()) } diff --git a/src/backend/gl/src/device.rs b/src/backend/gl/src/device.rs index b468ada9be5..088828ca280 100644 --- a/src/backend/gl/src/device.rs +++ b/src/backend/gl/src/device.rs @@ -1767,12 +1767,9 @@ impl d::Device for Device { unsafe fn copy_descriptor_sets<'a, I>(&self, copies: I) where - I: IntoIterator, - I::Item: Borrow>, + I: IntoIterator>, { for copy in copies { - let copy = copy.borrow(); - let src_set = ©.src_set; let dst_set = ©.dst_set; if std::ptr::eq(src_set, dst_set) { @@ -1808,7 +1805,7 @@ impl d::Device for Device { Ok(n::Fence(cell)) } - unsafe fn reset_fence(&self, fence: &n::Fence) -> Result<(), d::OutOfMemory> { + unsafe fn reset_fence(&self, fence: &mut n::Fence) -> Result<(), d::OutOfMemory> { fence.0.replace(n::FenceInner::Idle { signaled: false }); Ok(()) } diff --git a/src/backend/gl/src/native.rs b/src/backend/gl/src/native.rs index bf9d51f9dce..2e21ad5106c 100644 --- a/src/backend/gl/src/native.rs +++ b/src/backend/gl/src/native.rs @@ -65,6 +65,7 @@ pub(crate) enum FenceInner { Pending(Option<::Fence>), } +//TODO: reconsider the use of `Cell` #[derive(Debug)] pub struct Fence(pub(crate) Cell); unsafe impl Send for Fence {} diff --git a/src/backend/gl/src/queue.rs b/src/backend/gl/src/queue.rs index 48b6e87c6ae..74b73eab5fe 100644 --- a/src/backend/gl/src/queue.rs +++ b/src/backend/gl/src/queue.rs @@ -1115,7 +1115,7 @@ impl hal::queue::CommandQueue for CommandQueue { &mut self, surface: &mut Surface, image: native::SwapchainImage, - _wait_semaphore: Option<&native::Semaphore>, + _wait_semaphore: Option<&mut native::Semaphore>, ) -> Result, hal::window::PresentError> { surface.present(image, &self.share.context) } diff --git a/src/backend/metal/src/command.rs b/src/backend/metal/src/command.rs index 198de893a7a..82e1127f1ba 100644 --- a/src/backend/metal/src/command.rs +++ b/src/backend/metal/src/command.rs @@ -2411,9 +2411,13 @@ impl hal::queue::CommandQueue for CommandQueue { &mut self, _surface: &mut window::Surface, image: window::SwapchainImage, - wait_semaphore: Option<&native::Semaphore>, + wait_semaphore: Option<&mut native::Semaphore>, ) -> Result, PresentError> { - self.wait(wait_semaphore); + if let Some(semaphore) = wait_semaphore { + if let Some(ref system) = semaphore.system { + system.wait(!0); + } + } let queue = self.shared.queue.lock(); let drawable = image.into_drawable(); diff --git a/src/backend/metal/src/device.rs b/src/backend/metal/src/device.rs index 3d6a0ccb3b2..17a11a20bb0 100644 --- a/src/backend/metal/src/device.rs +++ b/src/backend/metal/src/device.rs @@ -2370,8 +2370,7 @@ impl hal::device::Device for Device { unsafe fn copy_descriptor_sets<'a, I>(&self, copies: I) where - I: IntoIterator, - I::Item: Borrow>, + I: IntoIterator>, { for _copy in copies { unimplemented!() @@ -2963,7 +2962,7 @@ impl hal::device::Device for Device { Ok(n::Fence(mutex)) } - unsafe fn reset_fence(&self, fence: &n::Fence) -> Result<(), d::OutOfMemory> { + unsafe fn reset_fence(&self, fence: &mut n::Fence) -> Result<(), d::OutOfMemory> { debug!("Resetting fence ptr {:?}", fence.0.raw() as *const _); *fence.0.lock() = n::FenceInner::Idle { signaled: false }; Ok(()) diff --git a/src/backend/metal/src/native.rs b/src/backend/metal/src/native.rs index c25e8cabe63..9188282b4b5 100644 --- a/src/backend/metal/src/native.rs +++ b/src/backend/metal/src/native.rs @@ -998,6 +998,8 @@ pub enum FenceInner { PendingSubmission(metal::CommandBuffer), } +//TODO: reconsider the `Mutex` +// it's only used in `submit()` #[derive(Debug)] pub struct Fence(pub(crate) Mutex); diff --git a/src/backend/vulkan/src/device.rs b/src/backend/vulkan/src/device.rs index 381f8c9e001..5d44636bcac 100644 --- a/src/backend/vulkan/src/device.rs +++ b/src/backend/vulkan/src/device.rs @@ -15,7 +15,7 @@ use hal::{ }; use std::{ - borrow::Borrow, + borrow::{Borrow, BorrowMut}, ffi::{CStr, CString}, mem, ops::Range, @@ -1751,12 +1751,10 @@ impl d::Device for Device { unsafe fn copy_descriptor_sets<'a, I>(&self, copies: I) where - I: IntoIterator, - I::Item: Borrow>, + I: IntoIterator>, I::IntoIter: ExactSizeIterator, { - let copies = copies.into_iter().map(|copy| { - let c = copy.borrow(); + let copies = copies.into_iter().map(|c| { vk::CopyDescriptorSet::builder() .src_set(c.src_set.raw) .src_binding(c.src_binding as u32) @@ -1865,7 +1863,7 @@ impl d::Device for Device { unsafe fn reset_fences(&self, fences: I) -> Result<(), d::OutOfMemory> where I: IntoIterator, - I::Item: Borrow, + I::Item: BorrowMut, I::IntoIter: ExactSizeIterator, { let fences = fences.into_iter().map(|fence| fence.borrow().0); diff --git a/src/backend/vulkan/src/lib.rs b/src/backend/vulkan/src/lib.rs index cb9dab82405..c3d59ebaa00 100644 --- a/src/backend/vulkan/src/lib.rs +++ b/src/backend/vulkan/src/lib.rs @@ -1467,7 +1467,7 @@ impl queue::CommandQueue for CommandQueue { &mut self, surface: &mut window::Surface, image: window::SurfaceImage, - wait_semaphore: Option<&native::Semaphore>, + wait_semaphore: Option<&mut native::Semaphore>, ) -> Result, PresentError> { let ssc = surface.swapchain.as_ref().unwrap(); let wait_semaphore = if let Some(wait_semaphore) = wait_semaphore { diff --git a/src/backend/webgpu/src/command.rs b/src/backend/webgpu/src/command.rs index 9f298775504..c3a709420fd 100644 --- a/src/backend/webgpu/src/command.rs +++ b/src/backend/webgpu/src/command.rs @@ -52,7 +52,7 @@ impl hal::queue::CommandQueue for CommandQueue { &mut self, _surface: &mut ::Surface, _image: <::Surface as PresentationSurface>::SwapchainImage, - _wait_semaphore: Option<&::Semaphore>, + _wait_semaphore: Option<&mut ::Semaphore>, ) -> Result, PresentError> { todo!() } diff --git a/src/backend/webgpu/src/device.rs b/src/backend/webgpu/src/device.rs index 26ec1856261..045a578866e 100644 --- a/src/backend/webgpu/src/device.rs +++ b/src/backend/webgpu/src/device.rs @@ -1,4 +1,7 @@ -use std::{borrow::Borrow, ops::Range}; +use std::{ + borrow::{Borrow, BorrowMut}, + ops::Range, +}; use hal::{ buffer, @@ -350,8 +353,7 @@ impl hal::device::Device for Device { unsafe fn copy_descriptor_sets<'a, I>(&self, _copy_iter: I) where - I: IntoIterator, - I::Item: Borrow>, + I: IntoIterator>, { todo!() } @@ -401,7 +403,7 @@ impl hal::device::Device for Device { unsafe fn reset_fence( &self, - _fence: &::Fence, + _fence: &mut ::Fence, ) -> Result<(), OutOfMemory> { todo!() } @@ -409,7 +411,7 @@ impl hal::device::Device for Device { unsafe fn reset_fences(&self, _fences: I) -> Result<(), OutOfMemory> where I: IntoIterator, - I::Item: Borrow<::Fence>, + I::Item: BorrowMut<::Fence>, { todo!() } diff --git a/src/hal/src/device.rs b/src/hal/src/device.rs index 8e8b6a39496..23ff3a69047 100644 --- a/src/hal/src/device.rs +++ b/src/hal/src/device.rs @@ -23,7 +23,12 @@ use crate::{ Backend, MemoryTypeId, }; -use std::{any::Any, borrow::Borrow, fmt, iter, ops::Range}; +use std::{ + any::Any, + borrow::{Borrow, BorrowMut}, + fmt, iter, + ops::Range, +}; /// Error occurred caused device to be lost. #[derive(Clone, Debug, PartialEq, thiserror::Error)] @@ -559,8 +564,7 @@ pub trait Device: fmt::Debug + Any + Send + Sync { /// Structure specifying a copy descriptor set operation unsafe fn copy_descriptor_sets<'a, I>(&self, copy_iter: I) where - I: IntoIterator, - I::Item: Borrow>, + I: IntoIterator>, I::IntoIter: ExactSizeIterator; /// Map a memory object into application address space @@ -613,7 +617,7 @@ pub trait Device: fmt::Debug + Any + Send + Sync { fn create_fence(&self, signaled: bool) -> Result; /// Resets a given fence to its original, unsignaled state. - unsafe fn reset_fence(&self, fence: &B::Fence) -> Result<(), OutOfMemory> { + unsafe fn reset_fence(&self, fence: &mut B::Fence) -> Result<(), OutOfMemory> { self.reset_fences(iter::once(fence)) } @@ -621,11 +625,11 @@ pub trait Device: fmt::Debug + Any + Send + Sync { unsafe fn reset_fences(&self, fences: I) -> Result<(), OutOfMemory> where I: IntoIterator, - I::Item: Borrow, + I::Item: BorrowMut, I::IntoIter: ExactSizeIterator, { - for fence in fences { - self.reset_fence(fence.borrow())?; + for mut fence in fences { + self.reset_fence(fence.borrow_mut())?; } Ok(()) } diff --git a/src/hal/src/queue/mod.rs b/src/hal/src/queue/mod.rs index f913664b5f3..9bf4d4e0d26 100644 --- a/src/hal/src/queue/mod.rs +++ b/src/hal/src/queue/mod.rs @@ -131,7 +131,7 @@ pub trait CommandQueue: fmt::Debug + Any + Send + Sync { &mut self, surface: &mut B::Surface, image: >::SwapchainImage, - wait_semaphore: Option<&B::Semaphore>, + wait_semaphore: Option<&mut B::Semaphore>, ) -> Result, PresentError>; /// Wait for the queue to be idle. diff --git a/src/hal/src/window.rs b/src/hal/src/window.rs index e4bd7ef33ea..58a2a316403 100644 --- a/src/hal/src/window.rs +++ b/src/hal/src/window.rs @@ -37,12 +37,12 @@ //! # let device: empty::Device = return; //! # let mut present_queue: empty::CommandQueue = return; //! # unsafe { -//! let render_semaphore = device.create_semaphore().unwrap(); +//! let mut render_semaphore = device.create_semaphore().unwrap(); //! //! let (frame, suboptimal) = surface.acquire_image(!0).unwrap(); //! // render the scene.. //! // `render_semaphore` will be signalled once rendering has been finished -//! present_queue.present(&mut surface, frame, Some(&render_semaphore)); +//! present_queue.present(&mut surface, frame, Some(&mut render_semaphore)); //! # }} //! ``` //! From a17bc42bf7981e26db57b7c88948da02a8618842 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Thu, 24 Dec 2020 16:24:16 -0500 Subject: [PATCH 05/15] Refactor descriptor set copies and writes to work on &mut sets. We have to make it work with a single set at a time now, which works well with the relaxation we had on the types of descriptors in each write. It's also fine with wgpu. --- examples/colour-uniform/main.rs | 56 ++--- examples/compute/main.rs | 13 +- examples/mesh-shading/main.rs | 8 +- examples/quad/main.rs | 8 +- src/backend/dx11/src/device.rs | 301 +++++++++++----------- src/backend/dx11/src/lib.rs | 2 + src/backend/dx12/src/command.rs | 5 +- src/backend/dx12/src/device.rs | 414 +++++++++++++++---------------- src/backend/dx12/src/resource.rs | 45 ++-- src/backend/empty/src/lib.rs | 12 +- src/backend/gl/src/command.rs | 5 +- src/backend/gl/src/device.rs | 176 ++++++------- src/backend/gl/src/native.rs | 6 +- src/backend/metal/src/device.rs | 294 +++++++++++----------- src/backend/metal/src/native.rs | 2 + src/backend/vulkan/src/device.rs | 207 ++++++++-------- src/hal/src/device.rs | 17 +- src/hal/src/pso/descriptor.rs | 6 +- src/warden/src/gpu.rs | 15 +- 19 files changed, 745 insertions(+), 847 deletions(-) diff --git a/examples/colour-uniform/main.rs b/examples/colour-uniform/main.rs index 3e5e19801ee..39e948aee41 100644 --- a/examples/colour-uniform/main.rs +++ b/examples/colour-uniform/main.rs @@ -863,14 +863,14 @@ impl Uniform { let buffer = Some(buffer); desc.write_to_state( - vec![DescSetWrite { + DescSetWrite { binding, array_offset: 0, descriptors: Some(pso::Descriptor::Buffer( buffer.as_ref().unwrap().get_buffer(), buffer::SubRange::WHOLE, )), - }], + }, &mut device.borrow_mut().device, ); @@ -950,23 +950,20 @@ struct DescSetWrite { impl DescSet { unsafe fn write_to_state<'a, 'b: 'a, W>( &'b mut self, - write: Vec>, + d: DescSetWrite, device: &mut B::Device, ) where W: IntoIterator, + W::IntoIter: ExactSizeIterator, W::Item: std::borrow::Borrow>, { - let set = self.set.as_ref().unwrap(); - let write: Vec<_> = write - .into_iter() - .map(|d| pso::DescriptorSetWrite { - binding: d.binding, - array_offset: d.array_offset, - descriptors: d.descriptors, - set, - }) - .collect(); - device.write_descriptor_sets(write); + let set = self.set.as_mut().unwrap(); + device.write_descriptor_set(pso::DescriptorSetWrite { + binding: d.binding, + array_offset: d.array_offset, + descriptors: d.descriptors, + set, + }); } fn get_layout(&self) -> &B::DescriptorSetLayout { @@ -1049,21 +1046,22 @@ impl ImageState { .expect("Can't create sampler"); desc.write_to_state( - vec![ - DescSetWrite { - binding: 0, - array_offset: 0, - descriptors: Some(pso::Descriptor::Image( - &image_view, - i::Layout::ShaderReadOnlyOptimal, - )), - }, - DescSetWrite { - binding: 1, - array_offset: 0, - descriptors: Some(pso::Descriptor::Sampler(&sampler)), - }, - ], + DescSetWrite { + binding: 0, + array_offset: 0, + descriptors: Some(pso::Descriptor::Image( + &image_view, + i::Layout::ShaderReadOnlyOptimal, + )), + }, + device, + ); + desc.write_to_state( + DescSetWrite { + binding: 1, + array_offset: 0, + descriptors: Some(pso::Descriptor::Sampler(&sampler)), + }, device, ); diff --git a/examples/compute/main.rs b/examples/compute/main.rs index 6a0820a5db3..f708e4b8bc6 100644 --- a/examples/compute/main.rs +++ b/examples/compute/main.rs @@ -165,19 +165,18 @@ fn main() { ) }; - let desc_set; - - unsafe { - desc_set = desc_pool.allocate_set(&set_layout).unwrap(); - device.write_descriptor_sets(Some(pso::DescriptorSetWrite { - set: &desc_set, + let desc_set = unsafe { + let mut desc_set = desc_pool.allocate_set(&set_layout).unwrap(); + device.write_descriptor_set(pso::DescriptorSetWrite { + set: &mut desc_set, binding: 0, array_offset: 0, descriptors: Some(pso::Descriptor::Buffer( &device_buffer, buffer::SubRange::WHOLE, )), - })); + }); + desc_set }; let mut command_pool = diff --git a/examples/mesh-shading/main.rs b/examples/mesh-shading/main.rs index bebba30bbb9..4cbce701bfb 100644 --- a/examples/mesh-shading/main.rs +++ b/examples/mesh-shading/main.rs @@ -267,7 +267,7 @@ where } .expect("Can't create descriptor pool"), ); - let desc_set = unsafe { desc_pool.allocate_set(&set_layout) }.unwrap(); + let mut desc_set = unsafe { desc_pool.allocate_set(&set_layout) }.unwrap(); // Buffer allocations println!("Memory types: {:?}", memory_types); @@ -327,15 +327,15 @@ where }; unsafe { - device.write_descriptor_sets(vec![pso::DescriptorSetWrite { - set: &desc_set, + device.write_descriptor_set(pso::DescriptorSetWrite { + set: &mut desc_set, binding: 0, array_offset: 0, descriptors: Some(pso::Descriptor::Buffer( &*positions_buffer, buffer::SubRange::WHOLE, )), - }]); + }); } let caps = surface.capabilities(&adapter.physical_device); diff --git a/examples/quad/main.rs b/examples/quad/main.rs index efad6f36273..ecaa1849e7b 100644 --- a/examples/quad/main.rs +++ b/examples/quad/main.rs @@ -289,7 +289,7 @@ where } .expect("Can't create descriptor pool"), ); - let desc_set = unsafe { desc_pool.allocate_set(&set_layout) }.unwrap(); + let mut desc_set = unsafe { desc_pool.allocate_set(&set_layout) }.unwrap(); // Buffer allocations println!("Memory types: {:?}", memory_types); @@ -438,15 +438,15 @@ where ); unsafe { - device.write_descriptor_sets(iter::once(pso::DescriptorSetWrite { - set: &desc_set, + device.write_descriptor_set(pso::DescriptorSetWrite { + set: &mut desc_set, binding: 0, array_offset: 0, descriptors: vec![ pso::Descriptor::Image(&*image_srv, i::Layout::ShaderReadOnlyOptimal), pso::Descriptor::Sampler(&*sampler), ], - })); + }); } // copy buffer to texture diff --git a/src/backend/dx11/src/device.rs b/src/backend/dx11/src/device.rs index aed47da5181..675d4756426 100644 --- a/src/backend/dx11/src/device.rs +++ b/src/backend/dx11/src/device.rs @@ -1976,180 +1976,165 @@ impl device::Device for Device { }) } - unsafe fn write_descriptor_sets<'a, I, J>(&self, write_iter: I) + unsafe fn write_descriptor_set<'a, I>(&self, op: pso::DescriptorSetWrite<'a, Backend, I>) where - I: IntoIterator>, - J: IntoIterator, - J::Item: Borrow>, + I: IntoIterator, + I::Item: Borrow>, { - for write in write_iter { - // Get baseline mapping - let mut mapping = write - .set - .layout - .pool_mapping - .map_register(|mapping| mapping.offset); - - // Iterate over layout bindings until the first binding is found. - let binding_start = write - .set - .layout - .bindings - .iter() - .position(|binding| binding.binding == write.binding) - .unwrap(); - - // If we've skipped layout bindings, we need to add them to get the correct binding offset - for binding in &write.set.layout.bindings[..binding_start] { - let content = DescriptorContent::from(binding.ty); - mapping.add_content_many(content, binding.stage_flags, binding.count as _); - } + // Get baseline mapping + let mut mapping = op + .set + .layout + .pool_mapping + .map_register(|mapping| mapping.offset); + + // Iterate over layout bindings until the first binding is found. + let binding_start = op + .set + .layout + .bindings + .iter() + .position(|binding| binding.binding == op.binding) + .unwrap(); - // We start at the given binding index and array index - let mut binding_index = binding_start; - let mut array_index = write.array_offset; + // If we've skipped layout bindings, we need to add them to get the correct binding offset + for binding in &op.set.layout.bindings[..binding_start] { + let content = DescriptorContent::from(binding.ty); + mapping.add_content_many(content, binding.stage_flags, binding.count as _); + } - // If we're skipping array indices in the current binding, we need to add them to get the correct binding offset - if array_index > 0 { - let binding: &pso::DescriptorSetLayoutBinding = - &write.set.layout.bindings[binding_index]; - let content = DescriptorContent::from(binding.ty); - mapping.add_content_many(content, binding.stage_flags, array_index as _); - } + // We start at the given binding index and array index + let mut binding_index = binding_start; + let mut array_index = op.array_offset; - // Iterate over the descriptors, figuring out the corresponding binding, and adding - // it to the set of bindings. - // - // When we hit the end of an array of descriptors and there are still descriptors left - // over, we will spill into writing the next binding. - for descriptor in write.descriptors { - let binding: &pso::DescriptorSetLayoutBinding = - &write.set.layout.bindings[binding_index]; - - let handles = match *descriptor.borrow() { - pso::Descriptor::Buffer(buffer, ref _sub) => RegisterData { - c: match buffer.internal.disjoint_cb { - Some(dj_buf) => dj_buf as *mut _, - None => buffer.internal.raw as *mut _, - }, - t: buffer.internal.srv.map_or(ptr::null_mut(), |p| p as *mut _), - u: buffer.internal.uav.map_or(ptr::null_mut(), |p| p as *mut _), - s: ptr::null_mut(), - }, - pso::Descriptor::Image(image, _layout) => RegisterData { - c: ptr::null_mut(), - t: image.srv_handle.map_or(ptr::null_mut(), |h| h as *mut _), - u: image.uav_handle.map_or(ptr::null_mut(), |h| h as *mut _), - s: ptr::null_mut(), - }, - pso::Descriptor::Sampler(sampler) => RegisterData { - c: ptr::null_mut(), - t: ptr::null_mut(), - u: ptr::null_mut(), - s: sampler.sampler_handle.as_raw() as *mut _, + // If we're skipping array indices in the current binding, we need to add them to get the correct binding offset + if array_index > 0 { + let binding: &pso::DescriptorSetLayoutBinding = &op.set.layout.bindings[binding_index]; + let content = DescriptorContent::from(binding.ty); + mapping.add_content_many(content, binding.stage_flags, array_index as _); + } + + // Iterate over the descriptors, figuring out the corresponding binding, and adding + // it to the set of bindings. + // + // When we hit the end of an array of descriptors and there are still descriptors left + // over, we will spill into writing the next binding. + for descriptor in op.descriptors { + let binding: &pso::DescriptorSetLayoutBinding = &op.set.layout.bindings[binding_index]; + + let handles = match *descriptor.borrow() { + pso::Descriptor::Buffer(buffer, ref _sub) => RegisterData { + c: match buffer.internal.disjoint_cb { + Some(dj_buf) => dj_buf as *mut _, + None => buffer.internal.raw as *mut _, }, - pso::Descriptor::CombinedImageSampler(image, _layout, sampler) => { - RegisterData { - c: ptr::null_mut(), - t: image.srv_handle.map_or(ptr::null_mut(), |h| h as *mut _), - u: image.uav_handle.map_or(ptr::null_mut(), |h| h as *mut _), - s: sampler.sampler_handle.as_raw() as *mut _, - } - } - pso::Descriptor::TexelBuffer(_buffer_view) => unimplemented!(), - }; + t: buffer.internal.srv.map_or(ptr::null_mut(), |p| p as *mut _), + u: buffer.internal.uav.map_or(ptr::null_mut(), |p| p as *mut _), + s: ptr::null_mut(), + }, + pso::Descriptor::Image(image, _layout) => RegisterData { + c: ptr::null_mut(), + t: image.srv_handle.map_or(ptr::null_mut(), |h| h as *mut _), + u: image.uav_handle.map_or(ptr::null_mut(), |h| h as *mut _), + s: ptr::null_mut(), + }, + pso::Descriptor::Sampler(sampler) => RegisterData { + c: ptr::null_mut(), + t: ptr::null_mut(), + u: ptr::null_mut(), + s: sampler.sampler_handle.as_raw() as *mut _, + }, + pso::Descriptor::CombinedImageSampler(image, _layout, sampler) => RegisterData { + c: ptr::null_mut(), + t: image.srv_handle.map_or(ptr::null_mut(), |h| h as *mut _), + u: image.uav_handle.map_or(ptr::null_mut(), |h| h as *mut _), + s: sampler.sampler_handle.as_raw() as *mut _, + }, + pso::Descriptor::TexelBuffer(_buffer_view) => unimplemented!(), + }; - let content = DescriptorContent::from(binding.ty); - if content.contains(DescriptorContent::CBV) { - let offsets = mapping.map_other(|map| map.c); - write - .set - .assign_stages(&offsets, binding.stage_flags, handles.c); - }; - if content.contains(DescriptorContent::SRV) { - let offsets = mapping.map_other(|map| map.t); - write - .set - .assign_stages(&offsets, binding.stage_flags, handles.t); + let content = DescriptorContent::from(binding.ty); + if content.contains(DescriptorContent::CBV) { + let offsets = mapping.map_other(|map| map.c); + op.set + .assign_stages(&offsets, binding.stage_flags, handles.c); + }; + if content.contains(DescriptorContent::SRV) { + let offsets = mapping.map_other(|map| map.t); + op.set + .assign_stages(&offsets, binding.stage_flags, handles.t); + }; + if content.contains(DescriptorContent::UAV) { + // If this binding is used by the graphics pipeline and is a UAV, it belongs to the "Output Merger" + // stage, so we only put them in the fragment stage to save redundant descriptor allocations. + let stage_flags = if binding + .stage_flags + .intersects(pso::ShaderStageFlags::ALL - pso::ShaderStageFlags::COMPUTE) + { + let mut stage_flags = pso::ShaderStageFlags::FRAGMENT; + stage_flags.set( + pso::ShaderStageFlags::COMPUTE, + binding.stage_flags.contains(pso::ShaderStageFlags::COMPUTE), + ); + stage_flags + } else { + binding.stage_flags }; - if content.contains(DescriptorContent::UAV) { - // If this binding is used by the graphics pipeline and is a UAV, it belongs to the "Output Merger" - // stage, so we only put them in the fragment stage to save redundant descriptor allocations. - let stage_flags = if binding - .stage_flags - .intersects(pso::ShaderStageFlags::ALL - pso::ShaderStageFlags::COMPUTE) - { - let mut stage_flags = pso::ShaderStageFlags::FRAGMENT; - stage_flags.set( - pso::ShaderStageFlags::COMPUTE, - binding.stage_flags.contains(pso::ShaderStageFlags::COMPUTE), - ); - stage_flags - } else { - binding.stage_flags - }; - let offsets = mapping.map_other(|map| map.u); - write.set.assign_stages(&offsets, stage_flags, handles.u); - }; - if content.contains(DescriptorContent::SAMPLER) { - let offsets = mapping.map_other(|map| map.s); - write - .set - .assign_stages(&offsets, binding.stage_flags, handles.s); - }; + let offsets = mapping.map_other(|map| map.u); + op.set.assign_stages(&offsets, stage_flags, handles.u); + }; + if content.contains(DescriptorContent::SAMPLER) { + let offsets = mapping.map_other(|map| map.s); + op.set + .assign_stages(&offsets, binding.stage_flags, handles.s); + }; - mapping.add_content_many(content, binding.stage_flags, 1); + mapping.add_content_many(content, binding.stage_flags, 1); - array_index += 1; - if array_index >= binding.count { - // We've run out of array to write to, we should overflow to the next binding. - array_index = 0; - binding_index += 1; - } + array_index += 1; + if array_index >= binding.count { + // We've run out of array to write to, we should overflow to the next binding. + array_index = 0; + binding_index += 1; } } } - unsafe fn copy_descriptor_sets<'a, I>(&self, copy_iter: I) - where - I: IntoIterator>, - { - for _copy in copy_iter { - //TODO - /* - for offset in 0 .. copy.count { - let (dst_ty, dst_handle_offset, dst_second_handle_offset) = copy - .dst_set - .get_handle_offset(copy.dst_binding + offset as u32); - let (src_ty, src_handle_offset, src_second_handle_offset) = copy - .src_set - .get_handle_offset(copy.src_binding + offset as u32); - assert_eq!(dst_ty, src_ty); - - let dst_handle = copy.dst_set.handles.offset(dst_handle_offset as isize); - let src_handle = copy.dst_set.handles.offset(src_handle_offset as isize); - - match dst_ty { - pso::DescriptorType::Image { - ty: pso::ImageDescriptorType::Sampled { with_sampler: true } - } => { - let dst_second_handle = copy - .dst_set - .handles - .offset(dst_second_handle_offset as isize); - let src_second_handle = copy - .dst_set - .handles - .offset(src_second_handle_offset as isize); - - *dst_handle = *src_handle; - *dst_second_handle = *src_second_handle; - } - _ => *dst_handle = *src_handle, + unsafe fn copy_descriptor_set<'a>(&self, _op: pso::DescriptorSetCopy<'a, Backend>) { + unimplemented!() + /* + for offset in 0 .. copy.count { + let (dst_ty, dst_handle_offset, dst_second_handle_offset) = copy + .dst_set + .get_handle_offset(copy.dst_binding + offset as u32); + let (src_ty, src_handle_offset, src_second_handle_offset) = copy + .src_set + .get_handle_offset(copy.src_binding + offset as u32); + assert_eq!(dst_ty, src_ty); + + let dst_handle = copy.dst_set.handles.offset(dst_handle_offset as isize); + let src_handle = copy.dst_set.handles.offset(src_handle_offset as isize); + + match dst_ty { + pso::DescriptorType::Image { + ty: pso::ImageDescriptorType::Sampled { with_sampler: true } + } => { + let dst_second_handle = copy + .dst_set + .handles + .offset(dst_second_handle_offset as isize); + let src_second_handle = copy + .dst_set + .handles + .offset(src_second_handle_offset as isize); + + *dst_handle = *src_handle; + *dst_second_handle = *src_second_handle; } - }*/ - } + _ => *dst_handle = *src_handle, + } + }*/ } unsafe fn map_memory( diff --git a/src/backend/dx11/src/lib.rs b/src/backend/dx11/src/lib.rs index 22bc98d8c7b..58a915e1e11 100644 --- a/src/backend/dx11/src/lib.rs +++ b/src/backend/dx11/src/lib.rs @@ -4161,6 +4161,8 @@ impl DescriptorSet { #[derive(Debug)] pub struct DescriptorPool { + //TODO: do we need this in the pool? + // if the sets owned their data, we could make this just `Vec` handles: Vec, allocator: RangeAllocator, } diff --git a/src/backend/dx12/src/command.rs b/src/backend/dx12/src/command.rs index 72545a0fa79..56cde0d50e6 100644 --- a/src/backend/dx12/src/command.rs +++ b/src/backend/dx12/src/command.rs @@ -243,7 +243,7 @@ impl PipelineCache { } // Bind Sampler descriptor tables. - if let Some(gpu) = set.first_gpu_sampler.get() { + if let Some(gpu) = set.first_gpu_sampler { assert!(element.table.ty.contains(r::SAMPLERS)); // Cast is safe as offset **must** be in u32 range. Unable to @@ -259,8 +259,7 @@ impl PipelineCache { // Requires changes then in the descriptor update process. for binding in &set.binding_infos { // It's not valid to modify the descriptor sets during recording -> access if safe. - let dynamic_descriptors = unsafe { &*binding.dynamic_descriptors.get() }; - for descriptor in dynamic_descriptors { + for descriptor in binding.dynamic_descriptors.iter() { let gpu_offset = descriptor.gpu_buffer_location + offsets.next().unwrap(); self.user_data.set_descriptor_cbv(root_offset, gpu_offset); root_offset += 2; diff --git a/src/backend/dx12/src/device.rs b/src/backend/dx12/src/device.rs index cf15856575f..c28b1bf9905 100644 --- a/src/backend/dx12/src/device.rs +++ b/src/backend/dx12/src/device.rs @@ -2997,260 +2997,236 @@ impl d::Device for Device { }) } - unsafe fn write_descriptor_sets<'a, I, J>(&self, write_iter: I) + unsafe fn write_descriptor_set<'a, I>(&self, op: pso::DescriptorSetWrite<'a, B, I>) where - I: IntoIterator>, - J: IntoIterator, - J::Item: Borrow>, + I: IntoIterator, + I::Item: Borrow>, { let mut descriptor_updater = self.descriptor_updater.lock(); descriptor_updater.reset(); let mut accum = descriptors_cpu::MultiCopyAccumulator::default(); - debug!("write_descriptor_sets"); - - for write in write_iter { - let mut offset = write.array_offset as u64; - let mut target_binding = write.binding as usize; - let mut bind_info = &write.set.binding_infos[target_binding]; - debug!( - "\t{:?} binding {} array offset {}", - bind_info, target_binding, offset - ); - let base_sampler_offset = write.set.sampler_offset(write.binding, write.array_offset); - trace!("\tsampler offset {}", base_sampler_offset); - let mut sampler_offset = base_sampler_offset; - let mut desc_samplers = write.set.sampler_origins.borrow_mut(); - - for descriptor in write.descriptors { - // spill over the writes onto the next binding - while offset >= bind_info.count { - assert_eq!(offset, bind_info.count); - target_binding += 1; - bind_info = &write.set.binding_infos[target_binding]; - offset = 0; - } - let mut src_cbv = None; - let mut src_srv = None; - let mut src_uav = None; - - match *descriptor.borrow() { - pso::Descriptor::Buffer(buffer, ref sub) => { - let buffer = buffer.expect_bound(); - - if bind_info.content.is_dynamic() { - // Root Descriptor - let buffer_address = (*buffer.resource).GetGPUVirtualAddress(); - // Descriptor sets need to be externally synchronized according to specification - let dynamic_descriptors = &mut *bind_info.dynamic_descriptors.get(); - dynamic_descriptors[offset as usize].gpu_buffer_location = - buffer_address + sub.offset; - } else { - // Descriptor table - let size = sub.size_to(buffer.requirements.size); - - if bind_info.content.contains(r::DescriptorContent::CBV) { - // Making the size field of buffer requirements for uniform - // buffers a multiple of 256 and setting the required offset - // alignment to 256 allows us to patch the size here. - // We can always enforce the size to be aligned to 256 for - // CBVs without going out-of-bounds. - let mask = - d3d12::D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT - 1; - let desc = d3d12::D3D12_CONSTANT_BUFFER_VIEW_DESC { - BufferLocation: (*buffer.resource).GetGPUVirtualAddress() - + sub.offset, - SizeInBytes: (size as u32 + mask) as u32 & !mask, - }; - let handle = descriptor_updater.alloc_handle(self.raw); - self.raw.CreateConstantBufferView(&desc, handle); - src_cbv = Some(handle); - } - if bind_info.content.contains(r::DescriptorContent::SRV) { - assert_eq!(size % 4, 0); - let mut desc = d3d12::D3D12_SHADER_RESOURCE_VIEW_DESC { - Format: dxgiformat::DXGI_FORMAT_R32_TYPELESS, - Shader4ComponentMapping: IDENTITY_MAPPING, - ViewDimension: d3d12::D3D12_SRV_DIMENSION_BUFFER, - u: mem::zeroed(), - }; - *desc.u.Buffer_mut() = d3d12::D3D12_BUFFER_SRV { - FirstElement: sub.offset as _, - NumElements: (size / 4) as _, - StructureByteStride: 0, - Flags: d3d12::D3D12_BUFFER_SRV_FLAG_RAW, - }; - let handle = descriptor_updater.alloc_handle(self.raw); - self.raw.CreateShaderResourceView( - buffer.resource.as_mut_ptr(), - &desc, - handle, - ); - src_srv = Some(handle); - } - if bind_info.content.contains(r::DescriptorContent::UAV) { - assert_eq!(size % 4, 0); - let mut desc = d3d12::D3D12_UNORDERED_ACCESS_VIEW_DESC { - Format: dxgiformat::DXGI_FORMAT_R32_TYPELESS, - ViewDimension: d3d12::D3D12_UAV_DIMENSION_BUFFER, - u: mem::zeroed(), - }; - *desc.u.Buffer_mut() = d3d12::D3D12_BUFFER_UAV { - FirstElement: sub.offset as _, - NumElements: (size / 4) as _, - StructureByteStride: 0, - CounterOffsetInBytes: 0, - Flags: d3d12::D3D12_BUFFER_UAV_FLAG_RAW, - }; - let handle = descriptor_updater.alloc_handle(self.raw); - self.raw.CreateUnorderedAccessView( - buffer.resource.as_mut_ptr(), - ptr::null_mut(), - &desc, - handle, - ); - src_uav = Some(handle); - } + debug!("write_descriptor_set"); + + let mut offset = op.array_offset as u64; + let mut target_binding = op.binding as usize; + let base_sampler_offset = op.set.sampler_offset(op.binding, op.array_offset); + trace!("\tsampler offset {}", base_sampler_offset); + let mut sampler_offset = base_sampler_offset; + debug!("\tbinding {} array offset {}", target_binding, offset); + + for descriptor in op.descriptors { + // spill over the writes onto the next binding + while offset >= op.set.binding_infos[target_binding].count { + target_binding += 1; + offset = 0; + } + let mut bind_info = &mut op.set.binding_infos[target_binding]; + let mut src_cbv = None; + let mut src_srv = None; + let mut src_uav = None; + + match *descriptor.borrow() { + pso::Descriptor::Buffer(buffer, ref sub) => { + let buffer = buffer.expect_bound(); + + if bind_info.content.is_dynamic() { + // Root Descriptor + let buffer_address = (*buffer.resource).GetGPUVirtualAddress(); + // Descriptor sets need to be externally synchronized according to specification + bind_info.dynamic_descriptors[offset as usize].gpu_buffer_location = + buffer_address + sub.offset; + } else { + // Descriptor table + let size = sub.size_to(buffer.requirements.size); + + if bind_info.content.contains(r::DescriptorContent::CBV) { + // Making the size field of buffer requirements for uniform + // buffers a multiple of 256 and setting the required offset + // alignment to 256 allows us to patch the size here. + // We can always enforce the size to be aligned to 256 for + // CBVs without going out-of-bounds. + let mask = d3d12::D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT - 1; + let desc = d3d12::D3D12_CONSTANT_BUFFER_VIEW_DESC { + BufferLocation: (*buffer.resource).GetGPUVirtualAddress() + + sub.offset, + SizeInBytes: (size as u32 + mask) as u32 & !mask, + }; + let handle = descriptor_updater.alloc_handle(self.raw); + self.raw.CreateConstantBufferView(&desc, handle); + src_cbv = Some(handle); } - } - pso::Descriptor::Image(image, _layout) => { if bind_info.content.contains(r::DescriptorContent::SRV) { - src_srv = image.handle_srv.map(|h| h.raw); + assert_eq!(size % 4, 0); + let mut desc = d3d12::D3D12_SHADER_RESOURCE_VIEW_DESC { + Format: dxgiformat::DXGI_FORMAT_R32_TYPELESS, + Shader4ComponentMapping: IDENTITY_MAPPING, + ViewDimension: d3d12::D3D12_SRV_DIMENSION_BUFFER, + u: mem::zeroed(), + }; + *desc.u.Buffer_mut() = d3d12::D3D12_BUFFER_SRV { + FirstElement: sub.offset as _, + NumElements: (size / 4) as _, + StructureByteStride: 0, + Flags: d3d12::D3D12_BUFFER_SRV_FLAG_RAW, + }; + let handle = descriptor_updater.alloc_handle(self.raw); + self.raw.CreateShaderResourceView( + buffer.resource.as_mut_ptr(), + &desc, + handle, + ); + src_srv = Some(handle); } if bind_info.content.contains(r::DescriptorContent::UAV) { - src_uav = image.handle_uav.map(|h| h.raw); + assert_eq!(size % 4, 0); + let mut desc = d3d12::D3D12_UNORDERED_ACCESS_VIEW_DESC { + Format: dxgiformat::DXGI_FORMAT_R32_TYPELESS, + ViewDimension: d3d12::D3D12_UAV_DIMENSION_BUFFER, + u: mem::zeroed(), + }; + *desc.u.Buffer_mut() = d3d12::D3D12_BUFFER_UAV { + FirstElement: sub.offset as _, + NumElements: (size / 4) as _, + StructureByteStride: 0, + CounterOffsetInBytes: 0, + Flags: d3d12::D3D12_BUFFER_UAV_FLAG_RAW, + }; + let handle = descriptor_updater.alloc_handle(self.raw); + self.raw.CreateUnorderedAccessView( + buffer.resource.as_mut_ptr(), + ptr::null_mut(), + &desc, + handle, + ); + src_uav = Some(handle); } } - pso::Descriptor::CombinedImageSampler(image, _layout, sampler) => { + } + pso::Descriptor::Image(image, _layout) => { + if bind_info.content.contains(r::DescriptorContent::SRV) { src_srv = image.handle_srv.map(|h| h.raw); - desc_samplers[sampler_offset] = sampler.handle.raw; - sampler_offset += 1; - } - pso::Descriptor::Sampler(sampler) => { - desc_samplers[sampler_offset] = sampler.handle.raw; - sampler_offset += 1; } - pso::Descriptor::TexelBuffer(buffer_view) => { - if bind_info.content.contains(r::DescriptorContent::SRV) { - let handle = buffer_view.handle_srv - .expect("SRV handle of the storage texel buffer is zero (not supported by specified format)"); - src_srv = Some(handle.raw); - } - if bind_info.content.contains(r::DescriptorContent::UAV) { - let handle = buffer_view.handle_uav - .expect("UAV handle of the storage texel buffer is zero (not supported by specified format)"); - src_uav = Some(handle.raw); - } + if bind_info.content.contains(r::DescriptorContent::UAV) { + src_uav = image.handle_uav.map(|h| h.raw); } } - - if let Some(handle) = src_cbv { - trace!("\tcbv offset {}", offset); - accum.src_views.add(handle, 1); - accum - .dst_views - .add(bind_info.view_range.as_ref().unwrap().at(offset), 1); + pso::Descriptor::CombinedImageSampler(image, _layout, sampler) => { + src_srv = image.handle_srv.map(|h| h.raw); + op.set.sampler_origins[sampler_offset] = sampler.handle.raw; + sampler_offset += 1; } - if let Some(handle) = src_srv { - trace!("\tsrv offset {}", offset); - accum.src_views.add(handle, 1); - accum - .dst_views - .add(bind_info.view_range.as_ref().unwrap().at(offset), 1); + pso::Descriptor::Sampler(sampler) => { + op.set.sampler_origins[sampler_offset] = sampler.handle.raw; + sampler_offset += 1; } - if let Some(handle) = src_uav { - let uav_offset = if bind_info.content.contains(r::DescriptorContent::SRV) { - bind_info.count + offset - } else { - offset - }; - trace!("\tuav offset {}", uav_offset); - accum.src_views.add(handle, 1); - accum - .dst_views - .add(bind_info.view_range.as_ref().unwrap().at(uav_offset), 1); + pso::Descriptor::TexelBuffer(buffer_view) => { + if bind_info.content.contains(r::DescriptorContent::SRV) { + let handle = buffer_view.handle_srv + .expect("SRV handle of the storage texel buffer is zero (not supported by specified format)"); + src_srv = Some(handle.raw); + } + if bind_info.content.contains(r::DescriptorContent::UAV) { + let handle = buffer_view.handle_uav + .expect("UAV handle of the storage texel buffer is zero (not supported by specified format)"); + src_uav = Some(handle.raw); + } } - - offset += 1; } - if sampler_offset != base_sampler_offset { - drop(desc_samplers); - write - .set - .update_samplers(&self.samplers.heap, &self.samplers.origins, &mut accum); + if let Some(handle) = src_cbv { + trace!("\tcbv offset {}", offset); + accum.src_views.add(handle, 1); + accum + .dst_views + .add(bind_info.view_range.as_ref().unwrap().at(offset), 1); + } + if let Some(handle) = src_srv { + trace!("\tsrv offset {}", offset); + accum.src_views.add(handle, 1); + accum + .dst_views + .add(bind_info.view_range.as_ref().unwrap().at(offset), 1); } + if let Some(handle) = src_uav { + let uav_offset = if bind_info.content.contains(r::DescriptorContent::SRV) { + bind_info.count + offset + } else { + offset + }; + trace!("\tuav offset {}", uav_offset); + accum.src_views.add(handle, 1); + accum + .dst_views + .add(bind_info.view_range.as_ref().unwrap().at(uav_offset), 1); + } + + offset += 1; + } + + if sampler_offset != base_sampler_offset { + op.set + .update_samplers(&self.samplers.heap, &self.samplers.origins, &mut accum); } accum.flush(self.raw); } - unsafe fn copy_descriptor_sets<'a, I>(&self, copy_iter: I) - where - I: IntoIterator>, - { + unsafe fn copy_descriptor_set<'a>(&self, op: pso::DescriptorSetCopy<'a, B>) { let mut accum = descriptors_cpu::MultiCopyAccumulator::default(); - for copy in copy_iter { - let src_info = ©.src_set.binding_infos[copy.src_binding as usize]; - let dst_info = ©.dst_set.binding_infos[copy.dst_binding as usize]; + let src_info = &op.src_set.binding_infos[op.src_binding as usize]; + let dst_info = &op.dst_set.binding_infos[op.dst_binding as usize]; - if let (Some(src_range), Some(dst_range)) = - (src_info.view_range.as_ref(), dst_info.view_range.as_ref()) + if let (Some(src_range), Some(dst_range)) = + (src_info.view_range.as_ref(), dst_info.view_range.as_ref()) + { + assert!(op.src_array_offset + op.count <= src_range.handle.size as usize); + assert!(op.dst_array_offset + op.count <= dst_range.handle.size as usize); + let count = op.count as u32; + accum + .src_views + .add(src_range.at(op.src_array_offset as _), count); + accum + .dst_views + .add(dst_range.at(op.dst_array_offset as _), count); + + if (src_info.content & dst_info.content) + .contains(r::DescriptorContent::SRV | r::DescriptorContent::UAV) { - assert!(copy.src_array_offset + copy.count <= src_range.handle.size as usize); - assert!(copy.dst_array_offset + copy.count <= dst_range.handle.size as usize); - let count = copy.count as u32; - accum - .src_views - .add(src_range.at(copy.src_array_offset as _), count); - accum - .dst_views - .add(dst_range.at(copy.dst_array_offset as _), count); - - if (src_info.content & dst_info.content) - .contains(r::DescriptorContent::SRV | r::DescriptorContent::UAV) - { - assert!( - src_info.count as usize + copy.src_array_offset + copy.count - <= src_range.handle.size as usize - ); - assert!( - dst_info.count as usize + copy.dst_array_offset + copy.count - <= dst_range.handle.size as usize - ); - accum.src_views.add( - src_range.at(src_info.count + copy.src_array_offset as u64), - count, - ); - accum.dst_views.add( - dst_range.at(dst_info.count + copy.dst_array_offset as u64), - count, - ); - } - } - - if dst_info.content.contains(r::DescriptorContent::SAMPLER) { - let src_offset = copy - .src_set - .sampler_offset(copy.src_binding, copy.src_array_offset); - let dst_offset = copy - .dst_set - .sampler_offset(copy.dst_binding, copy.dst_array_offset); - let src_samplers = copy.src_set.sampler_origins.borrow(); - let mut dst_samplers = copy.dst_set.sampler_origins.borrow_mut(); - dst_samplers[dst_offset..dst_offset + copy.count] - .copy_from_slice(&src_samplers[src_offset..src_offset + copy.count]); - drop(dst_samplers); - - copy.dst_set.update_samplers( - &self.samplers.heap, - &self.samplers.origins, - &mut accum, + assert!( + src_info.count as usize + op.src_array_offset + op.count + <= src_range.handle.size as usize + ); + assert!( + dst_info.count as usize + op.dst_array_offset + op.count + <= dst_range.handle.size as usize + ); + accum.src_views.add( + src_range.at(src_info.count + op.src_array_offset as u64), + count, + ); + accum.dst_views.add( + dst_range.at(dst_info.count + op.dst_array_offset as u64), + count, ); } } + if dst_info.content.contains(r::DescriptorContent::SAMPLER) { + let src_offset = op + .src_set + .sampler_offset(op.src_binding, op.src_array_offset); + let dst_offset = op + .dst_set + .sampler_offset(op.dst_binding, op.dst_array_offset); + op.dst_set.sampler_origins[dst_offset..dst_offset + op.count] + .copy_from_slice(&op.src_set.sampler_origins[src_offset..src_offset + op.count]); + + op.dst_set + .update_samplers(&self.samplers.heap, &self.samplers.origins, &mut accum); + } + accum.flush(self.raw.clone()); } diff --git a/src/backend/dx12/src/resource.rs b/src/backend/dx12/src/resource.rs index d44fa7de76a..c4f4b30eefd 100644 --- a/src/backend/dx12/src/resource.rs +++ b/src/backend/dx12/src/resource.rs @@ -7,14 +7,7 @@ use winapi::{ um::d3d12, }; -use std::{ - cell::{Cell, RefCell, UnsafeCell}, - collections::BTreeMap, - fmt, - ops::Range, - slice, - sync::Arc, -}; +use std::{collections::BTreeMap, fmt, ops::Range, slice, sync::Arc}; use crate::{ descriptors_cpu::{Handle, MultiCopyAccumulator}, @@ -559,7 +552,7 @@ pub(crate) struct DynamicDescriptor { pub struct DescriptorBindingInfo { pub(crate) count: u64, pub(crate) view_range: Option, - pub(crate) dynamic_descriptors: UnsafeCell>, + pub(crate) dynamic_descriptors: Vec, pub(crate) content: DescriptorContent, } @@ -596,9 +589,9 @@ pub struct DescriptorSet { // Required for binding at command buffer pub(crate) heap_srv_cbv_uav: native::DescriptorHeap, pub(crate) heap_samplers: native::DescriptorHeap, - pub(crate) sampler_origins: RefCell>, + pub(crate) sampler_origins: Box<[native::CpuDescriptor]>, pub(crate) binding_infos: Vec, - pub(crate) first_gpu_sampler: Cell>, + pub(crate) first_gpu_sampler: Option, pub(crate) first_gpu_view: Option, pub(crate) raw_name: Vec, } @@ -639,35 +632,34 @@ impl DescriptorSet { } pub fn update_samplers( - &self, + &mut self, heap: &DescriptorHeap, origins: &RwLock, accum: &mut MultiCopyAccumulator, ) { - let desc_origins = self.sampler_origins.borrow(); let start_index = if let Some(index) = { // explicit variable allows to limit the lifetime of that borrow let borrow = origins.read(); - borrow.find(&*desc_origins) + borrow.find(&self.sampler_origins) } { Some(index) - } else if desc_origins.iter().any(|desc| desc.ptr == 0) { + } else if self.sampler_origins.iter().any(|desc| desc.ptr == 0) { // set is incomplete, don't try to build it None } else { - let base = origins.write().grow(&*desc_origins); + let base = origins.write().grow(&self.sampler_origins); // copy the descriptors from their origins into the new location - accum - .dst_samplers - .add(heap.cpu_descriptor_at(base), desc_origins.len() as u32); - for &origin in desc_origins.iter() { + accum.dst_samplers.add( + heap.cpu_descriptor_at(base), + self.sampler_origins.len() as u32, + ); + for &origin in self.sampler_origins.iter() { accum.src_samplers.add(origin, 1); } Some(base) }; - self.first_gpu_sampler - .set(start_index.map(|index| heap.gpu_descriptor_at(index))); + self.first_gpu_sampler = start_index.map(|index| heap.gpu_descriptor_at(index)); } } @@ -834,7 +826,7 @@ impl pso::DescriptorPool for DescriptorPool { binding_infos[binding.binding as usize] = DescriptorBindingInfo { count: binding.count as _, view_range, - dynamic_descriptors: UnsafeCell::new(dynamic_descriptors), + dynamic_descriptors, content, }; } @@ -842,11 +834,10 @@ impl pso::DescriptorPool for DescriptorPool { Ok(DescriptorSet { heap_srv_cbv_uav: self.heap_srv_cbv_uav.heap, heap_samplers: self.heap_raw_sampler, - sampler_origins: RefCell::new( - vec![native::CpuDescriptor { ptr: 0 }; num_samplers].into_boxed_slice(), - ), + sampler_origins: vec![native::CpuDescriptor { ptr: 0 }; num_samplers] + .into_boxed_slice(), binding_infos, - first_gpu_sampler: Cell::new(None), + first_gpu_sampler: None, first_gpu_view, raw_name: Vec::new(), }) diff --git a/src/backend/empty/src/lib.rs b/src/backend/empty/src/lib.rs index 3fd2114c52a..9ae432efd91 100644 --- a/src/backend/empty/src/lib.rs +++ b/src/backend/empty/src/lib.rs @@ -410,18 +410,14 @@ impl device::Device for Device { Ok(layout) } - unsafe fn write_descriptor_sets<'a, I, J>(&self, _: I) + unsafe fn write_descriptor_set<'a, I>(&self, _: pso::DescriptorSetWrite<'a, Backend, I>) where - I: IntoIterator>, - J: IntoIterator, - J::Item: Borrow>, + I: IntoIterator, + I::Item: Borrow>, { } - unsafe fn copy_descriptor_sets<'a, I>(&self, _: I) - where - I: IntoIterator>, - { + unsafe fn copy_descriptor_set<'a>(&self, _: pso::DescriptorSetCopy<'a, Backend>) { unimplemented!("{}", NOT_SUPPORTED_MESSAGE) } diff --git a/src/backend/gl/src/command.rs b/src/backend/gl/src/command.rs index 21b3e8c9caa..992d7660cc9 100644 --- a/src/backend/gl/src/command.rs +++ b/src/backend/gl/src/command.rs @@ -706,8 +706,9 @@ impl CommandBuffer { let mut set = first_set as usize; for desc_set in sets { let desc_set = desc_set.borrow(); - let bindings = desc_set.bindings.lock(); - for (binding_layout, new_binding) in desc_set.layout.iter().zip(bindings.iter()) { + for (binding_layout, new_binding) in + desc_set.layout.iter().zip(desc_set.bindings.iter()) + { let binding = layout.sets[set].bindings[binding_layout.binding as usize] as u32; match *new_binding { n::DescSetBindings::Buffer { diff --git a/src/backend/gl/src/device.rs b/src/backend/gl/src/device.rs index 088828ca280..f1e16638a3a 100644 --- a/src/backend/gl/src/device.rs +++ b/src/backend/gl/src/device.rs @@ -1676,124 +1676,106 @@ impl d::Device for Device { Ok(Arc::new(bindings)) } - unsafe fn write_descriptor_sets<'a, I, J>(&self, writes: I) + unsafe fn write_descriptor_set<'a, I>(&self, op: pso::DescriptorSetWrite<'a, B, I>) where - I: IntoIterator>, - J: IntoIterator, - J::Item: Borrow>, + I: IntoIterator, + I::Item: Borrow>, { - for write in writes { - let mut bindings = write.set.bindings.lock(); - let mut layout_index = write - .set - .layout - .binary_search_by_key(&write.binding, |b| b.binding) - .unwrap(); - let mut array_offset = write.array_offset; - - for descriptor in write.descriptors { - let binding_layout = &write.set.layout[layout_index]; - match *descriptor.borrow() { - pso::Descriptor::Buffer(buffer, ref sub) => { - let (raw_buffer, buffer_range) = buffer.as_bound(); - let range = crate::resolve_sub_range(sub, buffer_range); - - let register = match binding_layout.ty { - pso::DescriptorType::Buffer { ty, .. } => match ty { - pso::BufferDescriptorType::Uniform => { - n::BindingRegister::UniformBuffers - } - pso::BufferDescriptorType::Storage { .. } => { - n::BindingRegister::StorageBuffers - } - }, - other => { - panic!("Can't write buffer into descriptor of type {:?}", other) - } - }; - - bindings.push(n::DescSetBindings::Buffer { - register, - buffer: raw_buffer, - offset: range.start as i32, - size: (range.end - range.start) as i32, - }); - } - pso::Descriptor::CombinedImageSampler(view, _layout, sampler) => { - match *view { - n::ImageView::Texture { target, raw, .. } => { - bindings.push(n::DescSetBindings::Texture(raw, target)) - } - n::ImageView::Renderbuffer { .. } => { - panic!("Texture doesn't support shader binding") + let mut layout_index = op + .set + .layout + .binary_search_by_key(&op.binding, |b| b.binding) + .unwrap(); + let mut array_offset = op.array_offset; + + for descriptor in op.descriptors { + let binding_layout = &op.set.layout[layout_index]; + let binding = match *descriptor.borrow() { + pso::Descriptor::Buffer(buffer, ref sub) => { + let (raw_buffer, buffer_range) = buffer.as_bound(); + let range = crate::resolve_sub_range(sub, buffer_range); + + let register = match binding_layout.ty { + pso::DescriptorType::Buffer { ty, .. } => match ty { + pso::BufferDescriptorType::Uniform => { + n::BindingRegister::UniformBuffers } - } - match *sampler { - n::FatSampler::Sampler(sampler) => { - bindings.push(n::DescSetBindings::Sampler(sampler)) - } - n::FatSampler::Info(ref info) => { - bindings.push(n::DescSetBindings::SamplerDesc(info.clone())) + pso::BufferDescriptorType::Storage { .. } => { + n::BindingRegister::StorageBuffers } + }, + other => { + panic!("Can't write buffer into descriptor of type {:?}", other) } + }; + + n::DescSetBindings::Buffer { + register, + buffer: raw_buffer, + offset: range.start as i32, + size: (range.end - range.start) as i32, } - pso::Descriptor::Image(view, _layout) => match *view { - n::ImageView::Texture { target, raw, .. } => { - bindings.push(n::DescSetBindings::Texture(raw, target)) - } + } + pso::Descriptor::CombinedImageSampler(view, _layout, sampler) => { + match *view { + n::ImageView::Texture { target, raw, .. } => op + .set + .bindings + .push(n::DescSetBindings::Texture(raw, target)), n::ImageView::Renderbuffer { .. } => { panic!("Texture doesn't support shader binding") } - }, - pso::Descriptor::Sampler(sampler) => match *sampler { - n::FatSampler::Sampler(sampler) => { - bindings.push(n::DescSetBindings::Sampler(sampler)) - } + } + match *sampler { + n::FatSampler::Sampler(sampler) => n::DescSetBindings::Sampler(sampler), n::FatSampler::Info(ref info) => { - bindings.push(n::DescSetBindings::SamplerDesc(info.clone())) + n::DescSetBindings::SamplerDesc(info.clone()) } - }, - pso::Descriptor::TexelBuffer(_view) => unimplemented!(), + } } + pso::Descriptor::Image(view, _layout) => match *view { + n::ImageView::Texture { target, raw, .. } => { + n::DescSetBindings::Texture(raw, target) + } + n::ImageView::Renderbuffer { .. } => { + panic!("Texture doesn't support shader binding") + } + }, + pso::Descriptor::Sampler(sampler) => match *sampler { + n::FatSampler::Sampler(sampler) => n::DescSetBindings::Sampler(sampler), + n::FatSampler::Info(ref info) => n::DescSetBindings::SamplerDesc(info.clone()), + }, + pso::Descriptor::TexelBuffer(_view) => unimplemented!(), + }; - array_offset += 1; - if array_offset == binding_layout.count { - array_offset = 0; - layout_index += 1; - } + //TODO: overwrite instead of pushing on top + op.set.bindings.push(binding); + + array_offset += 1; + if array_offset == binding_layout.count { + array_offset = 0; + layout_index += 1; } } } - unsafe fn copy_descriptor_sets<'a, I>(&self, copies: I) - where - I: IntoIterator>, - { - for copy in copies { - let src_set = ©.src_set; - let dst_set = ©.dst_set; - if std::ptr::eq(src_set, dst_set) { - panic!("copying within same descriptor set is not currently supported"); - } - - let src_bindings = src_set.bindings.lock(); - let mut dst_bindings = dst_set.bindings.lock(); - - let count = copy.count; + unsafe fn copy_descriptor_set<'a>(&self, op: pso::DescriptorSetCopy<'a, B>) { + if std::ptr::eq(op.src_set, &*op.dst_set) { + panic!("copying within same descriptor set is not currently supported"); + } - // TODO: add support for array bindings when the OpenGL backend gets them - let src_start = copy.src_binding as usize; - let src_end = src_start + count; - assert!(src_end <= src_bindings.len()); + // TODO: add support for array bindings when the OpenGL backend gets them + let src_start = op.src_binding as usize; + let src_end = src_start + op.count; + assert!(src_end <= op.src_set.bindings.len()); - let src_slice = &src_bindings[src_start..src_end]; + let src_slice = &op.src_set.bindings[src_start..src_end]; - let dst_start = copy.dst_binding as usize; - let dst_end = dst_start + count; - assert!(dst_end <= dst_bindings.len()); + let dst_start = op.dst_binding as usize; + let dst_end = dst_start + op.count; + assert!(dst_end <= op.dst_set.bindings.len()); - dst_bindings[dst_start..dst_end].clone_from_slice(src_slice); - } + op.dst_set.bindings[dst_start..dst_end].clone_from_slice(src_slice); } fn create_semaphore(&self) -> Result { diff --git a/src/backend/gl/src/native.rs b/src/backend/gl/src/native.rs index 2e21ad5106c..5cd1130851e 100644 --- a/src/backend/gl/src/native.rs +++ b/src/backend/gl/src/native.rs @@ -6,8 +6,6 @@ use hal::{ pass, pso, window as w, }; -use parking_lot::Mutex; - use std::{borrow::Borrow, cell::Cell, ops::Range, sync::Arc}; pub type TextureTarget = u32; @@ -242,7 +240,7 @@ pub(crate) enum DescSetBindings { pub struct DescriptorSet { pub(crate) layout: DescriptorSetLayout, //TODO: use `UnsafeCell` instead - pub(crate) bindings: Arc>>, + pub(crate) bindings: Vec, } #[derive(Debug)] @@ -255,7 +253,7 @@ impl pso::DescriptorPool for DescriptorPool { ) -> Result { Ok(DescriptorSet { layout: Arc::clone(layout), - bindings: Arc::new(Mutex::new(Vec::new())), + bindings: Vec::new(), }) } diff --git a/src/backend/metal/src/device.rs b/src/backend/metal/src/device.rs index 17a11a20bb0..144fd79dba1 100644 --- a/src/backend/metal/src/device.rs +++ b/src/backend/metal/src/device.rs @@ -2200,167 +2200,156 @@ impl hal::device::Device for Device { } } - unsafe fn write_descriptor_sets<'a, I, J>(&self, write_iter: I) + unsafe fn write_descriptor_set<'a, I>(&self, op: pso::DescriptorSetWrite<'a, Backend, I>) where - I: IntoIterator>, - J: IntoIterator, - J::Item: Borrow>, + I: IntoIterator, + I::Item: Borrow>, { - debug!("write_descriptor_sets"); - for write in write_iter { - match *write.set { - n::DescriptorSet::Emulated { - ref pool, - ref layouts, - ref resources, - } => { - let mut counters = resources.map(|r| r.start); - let mut start = None; //TODO: can pre-compute this - for (i, layout) in layouts.iter().enumerate() { - if layout.binding == write.binding - && layout.array_index == write.array_offset - { - start = Some(i); - break; - } - counters.add(layout.content); + debug!("write_descriptor_set"); + match *op.set { + n::DescriptorSet::Emulated { + ref pool, + ref layouts, + ref resources, + } => { + let mut counters = resources.map(|r| r.start); + let mut start = None; //TODO: can pre-compute this + for (i, layout) in layouts.iter().enumerate() { + if layout.binding == op.binding && layout.array_index == op.array_offset { + start = Some(i); + break; } - let mut data = pool.write(); - - for (layout, descriptor) in - layouts[start.unwrap()..].iter().zip(write.descriptors) - { - trace!("\t{:?}", layout); - match *descriptor.borrow() { - pso::Descriptor::Sampler(sam) => { - debug_assert!(!layout - .content - .contains(n::DescriptorContent::IMMUTABLE_SAMPLER)); + counters.add(layout.content); + } + let mut data = pool.write(); + + for (layout, descriptor) in layouts[start.unwrap()..].iter().zip(op.descriptors) { + trace!("\t{:?}", layout); + match *descriptor.borrow() { + pso::Descriptor::Sampler(sam) => { + debug_assert!(!layout + .content + .contains(n::DescriptorContent::IMMUTABLE_SAMPLER)); + data.samplers[counters.samplers as usize] = ( + layout.stages, + Some(AsNative::from(sam.raw.as_ref().unwrap().as_ref())), + ); + } + pso::Descriptor::Image(view, il) => { + data.textures[counters.textures as usize] = ( + layout.stages, + Some(AsNative::from(view.texture.as_ref())), + il, + ); + } + pso::Descriptor::CombinedImageSampler(view, il, sam) => { + if !layout + .content + .contains(n::DescriptorContent::IMMUTABLE_SAMPLER) + { data.samplers[counters.samplers as usize] = ( layout.stages, Some(AsNative::from(sam.raw.as_ref().unwrap().as_ref())), ); } - pso::Descriptor::Image(view, il) => { - data.textures[counters.textures as usize] = ( - layout.stages, - Some(AsNative::from(view.texture.as_ref())), - il, - ); - } - pso::Descriptor::CombinedImageSampler(view, il, sam) => { - if !layout - .content - .contains(n::DescriptorContent::IMMUTABLE_SAMPLER) - { - data.samplers[counters.samplers as usize] = ( - layout.stages, - Some(AsNative::from(sam.raw.as_ref().unwrap().as_ref())), - ); - } - data.textures[counters.textures as usize] = ( - layout.stages, - Some(AsNative::from(view.texture.as_ref())), - il, - ); - } - pso::Descriptor::TexelBuffer(view) => { - data.textures[counters.textures as usize] = ( - layout.stages, - Some(AsNative::from(view.raw.as_ref())), - image::Layout::General, - ); - } - pso::Descriptor::Buffer(buf, ref sub) => { - let (raw, range) = buf.as_bound(); - debug_assert!( - range.start + sub.offset + sub.size.unwrap_or(0) <= range.end - ); - data.buffers[counters.buffers as usize] = ( - layout.stages, - Some(AsNative::from(raw)), - range.start + sub.offset, - ); - } + data.textures[counters.textures as usize] = ( + layout.stages, + Some(AsNative::from(view.texture.as_ref())), + il, + ); + } + pso::Descriptor::TexelBuffer(view) => { + data.textures[counters.textures as usize] = ( + layout.stages, + Some(AsNative::from(view.raw.as_ref())), + image::Layout::General, + ); + } + pso::Descriptor::Buffer(buf, ref sub) => { + let (raw, range) = buf.as_bound(); + debug_assert!( + range.start + sub.offset + sub.size.unwrap_or(0) <= range.end + ); + data.buffers[counters.buffers as usize] = ( + layout.stages, + Some(AsNative::from(raw)), + range.start + sub.offset, + ); } - counters.add(layout.content); } + counters.add(layout.content); } - n::DescriptorSet::ArgumentBuffer { - ref raw, - raw_offset, - ref pool, - ref range, - ref encoder, - ref bindings, - .. - } => { - debug_assert!(self.shared.private_caps.argument_buffers); - - encoder.set_argument_buffer(raw, raw_offset); - let mut arg_index = { - let binding = &bindings[&write.binding]; - debug_assert!((write.array_offset as usize) < binding.count); - (binding.res_offset as NSUInteger) + (write.array_offset as NSUInteger) - }; - - for (data, descriptor) in pool.write().resources - [range.start as usize + arg_index as usize..range.end as usize] - .iter_mut() - .zip(write.descriptors) - { - match *descriptor.borrow() { - pso::Descriptor::Sampler(sampler) => { - debug_assert!(!bindings[&write.binding] - .content - .contains(n::DescriptorContent::IMMUTABLE_SAMPLER)); - encoder.set_sampler_state(arg_index, sampler.raw.as_ref().unwrap()); - arg_index += 1; - } - pso::Descriptor::Image(image, _layout) => { - let tex_ref = image.texture.as_ref(); - encoder.set_texture(arg_index, tex_ref); - data.ptr = (&**tex_ref).as_ptr(); - arg_index += 1; - } - pso::Descriptor::CombinedImageSampler(image, _il, sampler) => { - let binding = &bindings[&write.binding]; - if !binding - .content - .contains(n::DescriptorContent::IMMUTABLE_SAMPLER) - { - //TODO: supporting arrays of combined image-samplers can be tricky. - // We need to scan both sampler and image sections of the encoder - // at the same time. - assert!( - arg_index - < (binding.res_offset as NSUInteger) - + (binding.count as NSUInteger) - ); - encoder.set_sampler_state( - arg_index + binding.count as NSUInteger, - sampler.raw.as_ref().unwrap(), - ); - } - let tex_ref = image.texture.as_ref(); - encoder.set_texture(arg_index, tex_ref); - data.ptr = (&**tex_ref).as_ptr(); - } - pso::Descriptor::TexelBuffer(view) => { - encoder.set_texture(arg_index, &view.raw); - data.ptr = (&**view.raw).as_ptr(); - arg_index += 1; - } - pso::Descriptor::Buffer(buffer, ref sub) => { - let (buf_raw, buf_range) = buffer.as_bound(); - encoder.set_buffer( - arg_index, - buf_raw, - buf_range.start + sub.offset, + } + n::DescriptorSet::ArgumentBuffer { + ref raw, + raw_offset, + ref pool, + ref range, + ref encoder, + ref bindings, + .. + } => { + debug_assert!(self.shared.private_caps.argument_buffers); + + encoder.set_argument_buffer(raw, raw_offset); + let mut arg_index = { + let binding = &bindings[&op.binding]; + debug_assert!((op.array_offset as usize) < binding.count); + (binding.res_offset as NSUInteger) + (op.array_offset as NSUInteger) + }; + + for (data, descriptor) in pool.write().resources + [range.start as usize + arg_index as usize..range.end as usize] + .iter_mut() + .zip(op.descriptors) + { + match *descriptor.borrow() { + pso::Descriptor::Sampler(sampler) => { + debug_assert!(!bindings[&op.binding] + .content + .contains(n::DescriptorContent::IMMUTABLE_SAMPLER)); + encoder.set_sampler_state(arg_index, sampler.raw.as_ref().unwrap()); + arg_index += 1; + } + pso::Descriptor::Image(image, _layout) => { + let tex_ref = image.texture.as_ref(); + encoder.set_texture(arg_index, tex_ref); + data.ptr = (&**tex_ref).as_ptr(); + arg_index += 1; + } + pso::Descriptor::CombinedImageSampler(image, _il, sampler) => { + let binding = &bindings[&op.binding]; + if !binding + .content + .contains(n::DescriptorContent::IMMUTABLE_SAMPLER) + { + //TODO: supporting arrays of combined image-samplers can be tricky. + // We need to scan both sampler and image sections of the encoder + // at the same time. + assert!( + arg_index + < (binding.res_offset as NSUInteger) + + (binding.count as NSUInteger) + ); + encoder.set_sampler_state( + arg_index + binding.count as NSUInteger, + sampler.raw.as_ref().unwrap(), ); - data.ptr = (&**buf_raw).as_ptr(); - arg_index += 1; } + let tex_ref = image.texture.as_ref(); + encoder.set_texture(arg_index, tex_ref); + data.ptr = (&**tex_ref).as_ptr(); + } + pso::Descriptor::TexelBuffer(view) => { + encoder.set_texture(arg_index, &view.raw); + data.ptr = (&**view.raw).as_ptr(); + arg_index += 1; + } + pso::Descriptor::Buffer(buffer, ref sub) => { + let (buf_raw, buf_range) = buffer.as_bound(); + encoder.set_buffer(arg_index, buf_raw, buf_range.start + sub.offset); + data.ptr = (&**buf_raw).as_ptr(); + arg_index += 1; } } } @@ -2368,13 +2357,8 @@ impl hal::device::Device for Device { } } - unsafe fn copy_descriptor_sets<'a, I>(&self, copies: I) - where - I: IntoIterator>, - { - for _copy in copies { - unimplemented!() - } + unsafe fn copy_descriptor_set<'a>(&self, _op: pso::DescriptorSetCopy<'a, Backend>) { + unimplemented!() } unsafe fn destroy_descriptor_pool(&self, _pool: n::DescriptorPool) {} diff --git a/src/backend/metal/src/native.rs b/src/backend/metal/src/native.rs index 9188282b4b5..a790837abc5 100644 --- a/src/backend/metal/src/native.rs +++ b/src/backend/metal/src/native.rs @@ -879,6 +879,8 @@ pub struct UsedResource { #[derive(Debug)] pub enum DescriptorSet { Emulated { + //TODO: consider storing the descriptors right here, + // to reduce the amount of locking, e.g. in descriptor binding. pool: Arc>, layouts: Arc>, resources: ResourceData>, diff --git a/src/backend/vulkan/src/device.rs b/src/backend/vulkan/src/device.rs index 5d44636bcac..f085d7275d6 100644 --- a/src/backend/vulkan/src/device.rs +++ b/src/backend/vulkan/src/device.rs @@ -1615,102 +1615,101 @@ impl d::Device for Device { } } - unsafe fn write_descriptor_sets<'a, I, J>(&self, write_iter: I) + unsafe fn write_descriptor_set<'a, I>(&self, op: pso::DescriptorSetWrite<'a, B, I>) where - I: IntoIterator>, - J: IntoIterator, - J::Item: Borrow>, + I: IntoIterator, + I::IntoIter: ExactSizeIterator, + I::Item: Borrow>, { - let mut raw_writes = Vec::::new(); + let descriptors = op.descriptors.into_iter(); + let mut raw_writes = Vec::::with_capacity(descriptors.len()); let mut image_infos = Vec::new(); let mut buffer_infos = Vec::new(); let mut texel_buffer_views = Vec::new(); - for sw in write_iter { - // gfx-hal allows the type and stages to be different between the descriptor - // in a single write, while Vulkan requires them to be the same. - let mut last_type = vk::DescriptorType::SAMPLER; - let mut last_stages = pso::ShaderStageFlags::empty(); - - let mut binding_pos = sw - .set - .bindings - .binary_search_by_key(&sw.binding, |b| b.binding) - .expect("Descriptor set writes don't match the set layout!"); - let mut array_offset = sw.array_offset; - - for descriptor in sw.descriptors { - let layout_binding = &sw.set.bindings[binding_pos]; - array_offset += 1; - if array_offset == layout_binding.count { - array_offset = 0; - binding_pos += 1; - } + // gfx-hal allows the type and stages to be different between the descriptor + // in a single write, while Vulkan requires them to be the same. + let mut last_type = vk::DescriptorType::SAMPLER; + let mut last_stages = pso::ShaderStageFlags::empty(); + + let mut binding_pos = op + .set + .bindings + .binary_search_by_key(&op.binding, |b| b.binding) + .expect("Descriptor set writes don't match the set layout!"); + let mut array_offset = op.array_offset; + + for descriptor in descriptors { + let layout_binding = &op.set.bindings[binding_pos]; + array_offset += 1; + if array_offset == layout_binding.count { + array_offset = 0; + binding_pos += 1; + } - let descriptor_type = conv::map_descriptor_type(layout_binding.ty); - if descriptor_type == last_type && layout_binding.stage_flags == last_stages { - raw_writes.last_mut().unwrap().descriptor_count += 1; - } else { - last_type = descriptor_type; - last_stages = layout_binding.stage_flags; - raw_writes.push(vk::WriteDescriptorSet { - s_type: vk::StructureType::WRITE_DESCRIPTOR_SET, - p_next: ptr::null(), - dst_set: sw.set.raw, - dst_binding: layout_binding.binding, - dst_array_element: if layout_binding.binding == sw.binding { - sw.array_offset as _ - } else { - 0 - }, - descriptor_count: 1, - descriptor_type, - p_image_info: image_infos.len() as _, - p_buffer_info: buffer_infos.len() as _, - p_texel_buffer_view: texel_buffer_views.len() as _, - }); - } + let descriptor_type = conv::map_descriptor_type(layout_binding.ty); + if descriptor_type == last_type && layout_binding.stage_flags == last_stages { + raw_writes.last_mut().unwrap().descriptor_count += 1; + } else { + last_type = descriptor_type; + last_stages = layout_binding.stage_flags; + raw_writes.push(vk::WriteDescriptorSet { + s_type: vk::StructureType::WRITE_DESCRIPTOR_SET, + p_next: ptr::null(), + dst_set: op.set.raw, + dst_binding: layout_binding.binding, + dst_array_element: if layout_binding.binding == op.binding { + op.array_offset as _ + } else { + 0 + }, + descriptor_count: 1, + descriptor_type, + p_image_info: image_infos.len() as _, + p_buffer_info: buffer_infos.len() as _, + p_texel_buffer_view: texel_buffer_views.len() as _, + }); + } - match *descriptor.borrow() { - pso::Descriptor::Sampler(sampler) => { - image_infos.push( - vk::DescriptorImageInfo::builder() - .sampler(sampler.0) - .image_view(vk::ImageView::null()) - .image_layout(vk::ImageLayout::GENERAL) - .build(), - ); - } - pso::Descriptor::Image(view, layout) => { - image_infos.push( - vk::DescriptorImageInfo::builder() - .sampler(vk::Sampler::null()) - .image_view(view.view) - .image_layout(conv::map_image_layout(layout)) - .build(), - ); - } - pso::Descriptor::CombinedImageSampler(view, layout, sampler) => { - image_infos.push( - vk::DescriptorImageInfo::builder() - .sampler(sampler.0) - .image_view(view.view) - .image_layout(conv::map_image_layout(layout)) - .build(), - ); - } - pso::Descriptor::Buffer(buffer, ref sub) => { - buffer_infos.push( - vk::DescriptorBufferInfo::builder() - .buffer(buffer.raw) - .offset(sub.offset) - .range(sub.size.unwrap_or(vk::WHOLE_SIZE)) - .build(), - ); - } - pso::Descriptor::TexelBuffer(view) => { - texel_buffer_views.push(view.raw); - } + match *descriptor.borrow() { + pso::Descriptor::Sampler(sampler) => { + image_infos.push( + vk::DescriptorImageInfo::builder() + .sampler(sampler.0) + .image_view(vk::ImageView::null()) + .image_layout(vk::ImageLayout::GENERAL) + .build(), + ); + } + pso::Descriptor::Image(view, layout) => { + image_infos.push( + vk::DescriptorImageInfo::builder() + .sampler(vk::Sampler::null()) + .image_view(view.view) + .image_layout(conv::map_image_layout(layout)) + .build(), + ); + } + pso::Descriptor::CombinedImageSampler(view, layout, sampler) => { + image_infos.push( + vk::DescriptorImageInfo::builder() + .sampler(sampler.0) + .image_view(view.view) + .image_layout(conv::map_image_layout(layout)) + .build(), + ); + } + pso::Descriptor::Buffer(buffer, ref sub) => { + buffer_infos.push( + vk::DescriptorBufferInfo::builder() + .buffer(buffer.raw) + .offset(sub.offset) + .range(sub.size.unwrap_or(vk::WHOLE_SIZE)) + .build(), + ); + } + pso::Descriptor::TexelBuffer(view) => { + texel_buffer_views.push(view.raw); } } } @@ -1749,28 +1748,18 @@ impl d::Device for Device { self.shared.raw.update_descriptor_sets(&raw_writes, &[]); } - unsafe fn copy_descriptor_sets<'a, I>(&self, copies: I) - where - I: IntoIterator>, - I::IntoIter: ExactSizeIterator, - { - let copies = copies.into_iter().map(|c| { - vk::CopyDescriptorSet::builder() - .src_set(c.src_set.raw) - .src_binding(c.src_binding as u32) - .src_array_element(c.src_array_offset as u32) - .dst_set(c.dst_set.raw) - .dst_binding(c.dst_binding as u32) - .dst_array_element(c.dst_array_offset as u32) - .descriptor_count(c.count as u32) - .build() - }); - - inplace_it::inplace_or_alloc_array(copies.len(), |uninit_guard| { - let copies = uninit_guard.init_with_iter(copies); + unsafe fn copy_descriptor_set<'a>(&self, op: pso::DescriptorSetCopy<'a, B>) { + let copy = vk::CopyDescriptorSet::builder() + .src_set(op.src_set.raw) + .src_binding(op.src_binding as u32) + .src_array_element(op.src_array_offset as u32) + .dst_set(op.dst_set.raw) + .dst_binding(op.dst_binding as u32) + .dst_array_element(op.dst_array_offset as u32) + .descriptor_count(op.count as u32) + .build(); - self.shared.raw.update_descriptor_sets(&[], &copies); - }); + self.shared.raw.update_descriptor_sets(&[], &[copy]); } unsafe fn map_memory( diff --git a/src/hal/src/device.rs b/src/hal/src/device.rs index 23ff3a69047..026e94e1031 100644 --- a/src/hal/src/device.rs +++ b/src/hal/src/device.rs @@ -554,18 +554,15 @@ pub trait Device: fmt::Debug + Any + Send + Sync { /// Destroy a descriptor set layout object unsafe fn destroy_descriptor_set_layout(&self, layout: B::DescriptorSetLayout); - /// Specifying the parameters of a descriptor set write operation - unsafe fn write_descriptor_sets<'a, I, J>(&self, write_iter: I) + /// Specifying the parameters of a descriptor set write operation. + unsafe fn write_descriptor_set<'a, I>(&self, op: pso::DescriptorSetWrite<'a, B, I>) where - I: IntoIterator>, - J: IntoIterator, - J::Item: Borrow>; + I: IntoIterator, + I::IntoIter: ExactSizeIterator, + I::Item: Borrow>; - /// Structure specifying a copy descriptor set operation - unsafe fn copy_descriptor_sets<'a, I>(&self, copy_iter: I) - where - I: IntoIterator>, - I::IntoIter: ExactSizeIterator; + /// Structure specifying a copy descriptor set operation. + unsafe fn copy_descriptor_set<'a>(&self, op: pso::DescriptorSetCopy<'a, B>); /// Map a memory object into application address space /// diff --git a/src/hal/src/pso/descriptor.rs b/src/hal/src/pso/descriptor.rs index a2961e35fd3..d7d0df8bc80 100644 --- a/src/hal/src/pso/descriptor.rs +++ b/src/hal/src/pso/descriptor.rs @@ -225,7 +225,7 @@ where WI::Item: Borrow>, { /// The descriptor set to modify. - pub set: &'a B::DescriptorSet, + pub set: &'a mut B::DescriptorSet, /// Binding index to start writing at. /// /// *Note*: when there are more descriptors provided than @@ -256,7 +256,7 @@ pub enum Descriptor<'a, B: Backend> { /// Copies a range of descriptors to be bound from one descriptor set to another. /// /// Should be provided to the `copy_descriptor_sets` method of a `Device`. -#[derive(Clone, Copy, Debug)] +#[derive(Debug)] pub struct DescriptorSetCopy<'a, B: Backend> { /// Descriptor set to copy from. pub src_set: &'a B::DescriptorSet, @@ -270,7 +270,7 @@ pub struct DescriptorSetCopy<'a, B: Backend> { /// Offset into the descriptor array to start copying from. pub src_array_offset: DescriptorArrayIndex, /// Descriptor set to copy to. - pub dst_set: &'a B::DescriptorSet, + pub dst_set: &'a mut B::DescriptorSet, /// Binding to copy to. /// /// *Note*: when there are more descriptors provided than diff --git a/src/warden/src/gpu.rs b/src/warden/src/gpu.rs index 431acb7dedf..ae431e02b7e 100644 --- a/src/warden/src/gpu.rs +++ b/src/warden/src/gpu.rs @@ -774,7 +774,7 @@ impl Scene { } => { // create a descriptor set let (ref binding_indices, ref set_layout) = resources.desc_set_layouts[layout]; - let desc_set = unsafe { + let mut desc_set = unsafe { resources .desc_pools .get_mut(pool) @@ -786,11 +786,10 @@ impl Scene { set_layout )); // fill it up - let mut writes = Vec::new(); let mut views = Vec::new(); for (&binding, range) in binding_indices.iter().zip(data) { - writes.push(hal::pso::DescriptorSetWrite { - set: &desc_set, + let write = hal::pso::DescriptorSetWrite { + set: &mut desc_set, binding, array_offset: 0, descriptors: match *range { @@ -831,10 +830,10 @@ impl Scene { }) .collect::>(), }, - }); - } - unsafe { - device.write_descriptor_sets(writes); + }; + unsafe { + device.write_descriptor_set(write); + } } resources.desc_sets.insert( name.clone(), From 7b5247017379c37261c93e33247945315e93f875 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Thu, 24 Dec 2020 17:42:36 -0500 Subject: [PATCH 06/15] Update Queue::wait_idle, Device::set/reset event --- src/backend/dx11/src/device.rs | 4 ++-- src/backend/dx11/src/lib.rs | 2 +- src/backend/dx12/src/device.rs | 20 ++++++-------------- src/backend/dx12/src/lib.rs | 28 ++++++++++++++++------------ src/backend/empty/src/lib.rs | 6 +++--- src/backend/gl/src/device.rs | 4 ++-- src/backend/gl/src/queue.rs | 2 +- src/backend/metal/src/command.rs | 2 +- src/backend/metal/src/device.rs | 4 ++-- src/backend/metal/src/native.rs | 1 + src/backend/vulkan/src/device.rs | 4 ++-- src/backend/vulkan/src/lib.rs | 2 +- src/backend/webgpu/src/device.rs | 6 +++--- src/hal/src/device.rs | 4 ++-- src/hal/src/queue/mod.rs | 2 +- 15 files changed, 44 insertions(+), 47 deletions(-) diff --git a/src/backend/dx11/src/device.rs b/src/backend/dx11/src/device.rs index 675d4756426..e5d90797823 100644 --- a/src/backend/dx11/src/device.rs +++ b/src/backend/dx11/src/device.rs @@ -2259,11 +2259,11 @@ impl device::Device for Device { unimplemented!() } - unsafe fn set_event(&self, _event: &()) -> Result<(), device::OutOfMemory> { + unsafe fn set_event(&self, _event: &mut ()) -> Result<(), device::OutOfMemory> { unimplemented!() } - unsafe fn reset_event(&self, _event: &()) -> Result<(), device::OutOfMemory> { + unsafe fn reset_event(&self, _event: &mut ()) -> Result<(), device::OutOfMemory> { unimplemented!() } diff --git a/src/backend/dx11/src/lib.rs b/src/backend/dx11/src/lib.rs index 58a915e1e11..e238a6d03f5 100644 --- a/src/backend/dx11/src/lib.rs +++ b/src/backend/dx11/src/lib.rs @@ -1184,7 +1184,7 @@ impl queue::CommandQueue for CommandQueue { Ok(None) } - fn wait_idle(&self) -> Result<(), hal::device::OutOfMemory> { + fn wait_idle(&mut self) -> Result<(), hal::device::OutOfMemory> { // unimplemented!() Ok(()) } diff --git a/src/backend/dx12/src/device.rs b/src/backend/dx12/src/device.rs index c28b1bf9905..73f6be25102 100644 --- a/src/backend/dx12/src/device.rs +++ b/src/backend/dx12/src/device.rs @@ -18,16 +18,8 @@ use winapi::{ use auxil::{spirv_cross_specialize_ast, ShaderStage}; use hal::{ - buffer, device as d, format, - format::Aspects, - image, memory, - memory::Requirements, - pass, - pool::CommandPoolCreateFlags, - pso, - pso::VertexInputRate, - query, - queue::{CommandQueue as _, QueueFamilyId}, + buffer, device as d, format, format::Aspects, image, memory, memory::Requirements, pass, + pool::CommandPoolCreateFlags, pso, pso::VertexInputRate, query, queue::QueueFamilyId, window as w, }; @@ -3021,7 +3013,7 @@ impl d::Device for Device { target_binding += 1; offset = 0; } - let mut bind_info = &mut op.set.binding_infos[target_binding]; + let bind_info = &mut op.set.binding_infos[target_binding]; let mut src_cbv = None; let mut src_srv = None; let mut src_uav = None; @@ -3406,11 +3398,11 @@ impl d::Device for Device { unimplemented!() } - unsafe fn set_event(&self, _event: &()) -> Result<(), d::OutOfMemory> { + unsafe fn set_event(&self, _event: &mut ()) -> Result<(), d::OutOfMemory> { unimplemented!() } - unsafe fn reset_event(&self, _event: &()) -> Result<(), d::OutOfMemory> { + unsafe fn reset_event(&self, _event: &mut ()) -> Result<(), d::OutOfMemory> { unimplemented!() } @@ -3572,7 +3564,7 @@ impl d::Device for Device { fn wait_idle(&self) -> Result<(), d::OutOfMemory> { for queue in &self.queues { - queue.wait_idle()?; + queue.wait_idle_impl()?; } Ok(()) } diff --git a/src/backend/dx12/src/lib.rs b/src/backend/dx12/src/lib.rs index ec82c62f672..3c650d42d6a 100644 --- a/src/backend/dx12/src/lib.rs +++ b/src/backend/dx12/src/lib.rs @@ -471,6 +471,20 @@ impl CommandQueue { self.idle_fence.destroy(); self.raw.destroy(); } + + fn wait_idle_impl(&self) -> Result<(), hal::device::OutOfMemory> { + self.raw.signal(self.idle_fence, 1); + assert_eq!( + winerror::S_OK, + self.idle_fence.set_event_on_completion(self.idle_event, 1) + ); + + unsafe { + synchapi::WaitForSingleObject(self.idle_event.0, winbase::INFINITE); + } + + Ok(()) + } } unsafe impl Send for CommandQueue {} @@ -516,18 +530,8 @@ impl q::CommandQueue for CommandQueue { surface.present(image).map(|()| None) } - fn wait_idle(&self) -> Result<(), hal::device::OutOfMemory> { - self.raw.signal(self.idle_fence, 1); - assert_eq!( - winerror::S_OK, - self.idle_fence.set_event_on_completion(self.idle_event, 1) - ); - - unsafe { - synchapi::WaitForSingleObject(self.idle_event.0, winbase::INFINITE); - } - - Ok(()) + fn wait_idle(&mut self) -> Result<(), hal::device::OutOfMemory> { + self.wait_idle_impl() } } diff --git a/src/backend/empty/src/lib.rs b/src/backend/empty/src/lib.rs index 9ae432efd91..9cbed44723e 100644 --- a/src/backend/empty/src/lib.rs +++ b/src/backend/empty/src/lib.rs @@ -184,7 +184,7 @@ impl queue::CommandQueue for CommandQueue { Ok(None) } - fn wait_idle(&self) -> Result<(), device::OutOfMemory> { + fn wait_idle(&mut self) -> Result<(), device::OutOfMemory> { unimplemented!("{}", NOT_SUPPORTED_MESSAGE) } } @@ -441,11 +441,11 @@ impl device::Device for Device { unimplemented!("{}", NOT_SUPPORTED_MESSAGE) } - unsafe fn set_event(&self, _: &()) -> Result<(), device::OutOfMemory> { + unsafe fn set_event(&self, _: &mut ()) -> Result<(), device::OutOfMemory> { unimplemented!("{}", NOT_SUPPORTED_MESSAGE) } - unsafe fn reset_event(&self, _: &()) -> Result<(), device::OutOfMemory> { + unsafe fn reset_event(&self, _: &mut ()) -> Result<(), device::OutOfMemory> { unimplemented!("{}", NOT_SUPPORTED_MESSAGE) } diff --git a/src/backend/gl/src/device.rs b/src/backend/gl/src/device.rs index f1e16638a3a..9dfbd8aff3f 100644 --- a/src/backend/gl/src/device.rs +++ b/src/backend/gl/src/device.rs @@ -1904,11 +1904,11 @@ impl d::Device for Device { unimplemented!() } - unsafe fn set_event(&self, _event: &()) -> Result<(), d::OutOfMemory> { + unsafe fn set_event(&self, _event: &mut ()) -> Result<(), d::OutOfMemory> { unimplemented!() } - unsafe fn reset_event(&self, _event: &()) -> Result<(), d::OutOfMemory> { + unsafe fn reset_event(&self, _event: &mut ()) -> Result<(), d::OutOfMemory> { unimplemented!() } diff --git a/src/backend/gl/src/queue.rs b/src/backend/gl/src/queue.rs index 74b73eab5fe..1ac763c325c 100644 --- a/src/backend/gl/src/queue.rs +++ b/src/backend/gl/src/queue.rs @@ -1120,7 +1120,7 @@ impl hal::queue::CommandQueue for CommandQueue { surface.present(image, &self.share.context) } - fn wait_idle(&self) -> Result<(), hal::device::OutOfMemory> { + fn wait_idle(&mut self) -> Result<(), hal::device::OutOfMemory> { unsafe { self.share.context.finish(); } diff --git a/src/backend/metal/src/command.rs b/src/backend/metal/src/command.rs index 82e1127f1ba..7d2046207d4 100644 --- a/src/backend/metal/src/command.rs +++ b/src/backend/metal/src/command.rs @@ -2434,7 +2434,7 @@ impl hal::queue::CommandQueue for CommandQueue { Ok(None) } - fn wait_idle(&self) -> Result<(), OutOfMemory> { + fn wait_idle(&mut self) -> Result<(), OutOfMemory> { QueueInner::wait_idle(&self.shared.queue); Ok(()) } diff --git a/src/backend/metal/src/device.rs b/src/backend/metal/src/device.rs index 144fd79dba1..3a53bd739dc 100644 --- a/src/backend/metal/src/device.rs +++ b/src/backend/metal/src/device.rs @@ -3014,13 +3014,13 @@ impl hal::device::Device for Device { Ok(event.0.load(Ordering::Acquire)) } - unsafe fn set_event(&self, event: &n::Event) -> Result<(), d::OutOfMemory> { + unsafe fn set_event(&self, event: &mut n::Event) -> Result<(), d::OutOfMemory> { event.0.store(true, Ordering::Release); self.shared.queue_blocker.lock().triage(); Ok(()) } - unsafe fn reset_event(&self, event: &n::Event) -> Result<(), d::OutOfMemory> { + unsafe fn reset_event(&self, event: &mut n::Event) -> Result<(), d::OutOfMemory> { Ok(event.0.store(false, Ordering::Release)) } diff --git a/src/backend/metal/src/native.rs b/src/backend/metal/src/native.rs index a790837abc5..175fb6f88ec 100644 --- a/src/backend/metal/src/native.rs +++ b/src/backend/metal/src/native.rs @@ -1009,6 +1009,7 @@ unsafe impl Send for Fence {} unsafe impl Sync for Fence {} //TODO: review the atomic ordering +//TODO: reconsider if Arc is needed #[derive(Debug)] pub struct Event(pub(crate) Arc); diff --git a/src/backend/vulkan/src/device.rs b/src/backend/vulkan/src/device.rs index f085d7275d6..932da235107 100644 --- a/src/backend/vulkan/src/device.rs +++ b/src/backend/vulkan/src/device.rs @@ -1936,7 +1936,7 @@ impl d::Device for Device { } } - unsafe fn set_event(&self, event: &n::Event) -> Result<(), d::OutOfMemory> { + unsafe fn set_event(&self, event: &mut n::Event) -> Result<(), d::OutOfMemory> { let result = self.shared.raw.set_event(event.0); match result { Ok(()) => Ok(()), @@ -1946,7 +1946,7 @@ impl d::Device for Device { } } - unsafe fn reset_event(&self, event: &n::Event) -> Result<(), d::OutOfMemory> { + unsafe fn reset_event(&self, event: &mut n::Event) -> Result<(), d::OutOfMemory> { let result = self.shared.raw.reset_event(event.0); match result { Ok(()) => Ok(()), diff --git a/src/backend/vulkan/src/lib.rs b/src/backend/vulkan/src/lib.rs index c3d59ebaa00..538333ad53d 100644 --- a/src/backend/vulkan/src/lib.rs +++ b/src/backend/vulkan/src/lib.rs @@ -1502,7 +1502,7 @@ impl queue::CommandQueue for CommandQueue { } } - fn wait_idle(&self) -> Result<(), OutOfMemory> { + fn wait_idle(&mut self) -> Result<(), OutOfMemory> { match unsafe { self.device.raw.queue_wait_idle(*self.raw) } { Ok(()) => Ok(()), Err(vk::Result::ERROR_OUT_OF_HOST_MEMORY) => Err(OutOfMemory::Host), diff --git a/src/backend/webgpu/src/device.rs b/src/backend/webgpu/src/device.rs index 045a578866e..308d343f918 100644 --- a/src/backend/webgpu/src/device.rs +++ b/src/backend/webgpu/src/device.rs @@ -465,14 +465,14 @@ impl hal::device::Device for Device { unsafe fn set_event( &self, - _event: &::Event, + _event: &mut ::Event, ) -> Result<(), OutOfMemory> { todo!() } unsafe fn reset_event( &self, - _event: &::Event, + _event: &mut ::Event, ) -> Result<(), OutOfMemory> { todo!() } @@ -500,7 +500,7 @@ impl hal::device::Device for Device { todo!() } - fn wait_idle(&self) -> Result<(), OutOfMemory> { + fn wait_idle(&mut self) -> Result<(), OutOfMemory> { todo!() } diff --git a/src/hal/src/device.rs b/src/hal/src/device.rs index 026e94e1031..39ac1685e62 100644 --- a/src/hal/src/device.rs +++ b/src/hal/src/device.rs @@ -706,10 +706,10 @@ pub trait Device: fmt::Debug + Any + Send + Sync { unsafe fn get_event_status(&self, event: &B::Event) -> Result; /// Sets an event. - unsafe fn set_event(&self, event: &B::Event) -> Result<(), OutOfMemory>; + unsafe fn set_event(&self, event: &mut B::Event) -> Result<(), OutOfMemory>; /// Resets an event. - unsafe fn reset_event(&self, event: &B::Event) -> Result<(), OutOfMemory>; + unsafe fn reset_event(&self, event: &mut B::Event) -> Result<(), OutOfMemory>; /// Create a new query pool object /// diff --git a/src/hal/src/queue/mod.rs b/src/hal/src/queue/mod.rs index 9bf4d4e0d26..fd59c0382d7 100644 --- a/src/hal/src/queue/mod.rs +++ b/src/hal/src/queue/mod.rs @@ -135,5 +135,5 @@ pub trait CommandQueue: fmt::Debug + Any + Send + Sync { ) -> Result, PresentError>; /// Wait for the queue to be idle. - fn wait_idle(&self) -> Result<(), OutOfMemory>; + fn wait_idle(&mut self) -> Result<(), OutOfMemory>; } From ecc6caa9341a757f5a3e602fb9b8784ea6734fab Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Thu, 24 Dec 2020 18:40:25 -0500 Subject: [PATCH 07/15] Update Queue::submit with mutable fence borrows --- examples/compute/main.rs | 4 ++-- examples/mesh-shading/main.rs | 2 +- examples/quad/main.rs | 2 +- src/backend/dx11/src/lib.rs | 2 +- src/backend/dx12/src/lib.rs | 2 +- src/backend/empty/src/lib.rs | 2 +- src/backend/gl/src/device.rs | 39 +++++++++++-------------------- src/backend/gl/src/native.rs | 9 +++---- src/backend/gl/src/queue.rs | 10 ++++---- src/backend/metal/src/command.rs | 9 +++---- src/backend/metal/src/device.rs | 30 +++++++++--------------- src/backend/metal/src/native.rs | 9 ++----- src/backend/vulkan/src/lib.rs | 2 +- src/backend/webgpu/src/command.rs | 15 ++---------- src/hal/src/queue/mod.rs | 4 ++-- src/warden/src/gpu.rs | 8 +++---- 16 files changed, 53 insertions(+), 96 deletions(-) diff --git a/examples/compute/main.rs b/examples/compute/main.rs index f708e4b8bc6..5da9f4b586b 100644 --- a/examples/compute/main.rs +++ b/examples/compute/main.rs @@ -182,7 +182,7 @@ fn main() { let mut command_pool = unsafe { device.create_command_pool(family.id(), pool::CommandPoolCreateFlags::empty()) } .expect("Can't create command pool"); - let fence = device.create_fence(false).unwrap(); + let mut fence = device.create_fence(false).unwrap(); unsafe { let mut command_buffer = command_pool.allocate_one(command::Level::Primary); command_buffer.begin_primary(command::CommandBufferFlags::ONE_TIME_SUBMIT); @@ -231,7 +231,7 @@ fn main() { ); command_buffer.finish(); - queue_group.queues[0].submit_without_semaphores(Some(&command_buffer), Some(&fence)); + queue_group.queues[0].submit_without_semaphores(Some(&command_buffer), Some(&mut fence)); device.wait_for_fence(&fence, !0).unwrap(); command_pool.free(Some(command_buffer)); diff --git a/examples/mesh-shading/main.rs b/examples/mesh-shading/main.rs index 4cbce701bfb..248bacb9d38 100644 --- a/examples/mesh-shading/main.rs +++ b/examples/mesh-shading/main.rs @@ -630,7 +630,7 @@ where }; self.queue_group.queues[0].submit( submission, - Some(&self.submission_complete_fences[frame_idx]), + Some(&mut self.submission_complete_fences[frame_idx]), ); // present frame diff --git a/examples/quad/main.rs b/examples/quad/main.rs index ecaa1849e7b..6c00985d721 100644 --- a/examples/quad/main.rs +++ b/examples/quad/main.rs @@ -859,7 +859,7 @@ where }; self.queue_group.queues[0].submit( submission, - Some(&self.submission_complete_fences[frame_idx]), + Some(&mut self.submission_complete_fences[frame_idx]), ); // present frame diff --git a/src/backend/dx11/src/lib.rs b/src/backend/dx11/src/lib.rs index e238a6d03f5..c24eef941d8 100644 --- a/src/backend/dx11/src/lib.rs +++ b/src/backend/dx11/src/lib.rs @@ -1124,7 +1124,7 @@ impl queue::CommandQueue for CommandQueue { unsafe fn submit<'a, T, Ic, S, Iw, Is>( &mut self, submission: queue::Submission, - fence: Option<&Fence>, + fence: Option<&mut Fence>, ) where T: 'a + Borrow, Ic: IntoIterator, diff --git a/src/backend/dx12/src/lib.rs b/src/backend/dx12/src/lib.rs index 3c650d42d6a..82febee3cde 100644 --- a/src/backend/dx12/src/lib.rs +++ b/src/backend/dx12/src/lib.rs @@ -494,7 +494,7 @@ impl q::CommandQueue for CommandQueue { unsafe fn submit<'a, T, Ic, S, Iw, Is>( &mut self, submission: q::Submission, - fence: Option<&resource::Fence>, + fence: Option<&mut resource::Fence>, ) where T: 'a + Borrow, Ic: IntoIterator, diff --git a/src/backend/empty/src/lib.rs b/src/backend/empty/src/lib.rs index 9cbed44723e..2a319c58423 100644 --- a/src/backend/empty/src/lib.rs +++ b/src/backend/empty/src/lib.rs @@ -165,7 +165,7 @@ impl queue::CommandQueue for CommandQueue { unsafe fn submit<'a, T, Ic, S, Iw, Is>( &mut self, _: queue::Submission, - _: Option<&()>, + _: Option<&mut ()>, ) where T: 'a + Borrow, Ic: IntoIterator, diff --git a/src/backend/gl/src/device.rs b/src/backend/gl/src/device.rs index 9dfbd8aff3f..65d19488c12 100644 --- a/src/backend/gl/src/device.rs +++ b/src/backend/gl/src/device.rs @@ -1783,12 +1783,11 @@ impl d::Device for Device { } fn create_fence(&self, signaled: bool) -> Result { - let cell = Cell::new(n::FenceInner::Idle { signaled }); - Ok(n::Fence(cell)) + Ok(n::Fence::Idle { signaled }) } unsafe fn reset_fence(&self, fence: &mut n::Fence) -> Result<(), d::OutOfMemory> { - fence.0.replace(n::FenceInner::Idle { signaled: false }); + *fence = n::Fence::Idle { signaled: false }; Ok(()) } @@ -1802,17 +1801,14 @@ impl d::Device for Device { // access to a resource. How much does this call costs ? The status of the fence // could be cached to avoid calling this more than once (in core or in the backend ?). let gl = &self.share.context; - match fence.0.get() { - n::FenceInner::Idle { signaled } => { + match *fence { + n::Fence::Idle { signaled } => { if !signaled { - warn!( - "Fence ptr {:?} is not pending, waiting not possible", - fence.0.as_ptr() - ); + warn!("Fence ptr {:?} is not pending, waiting not possible", fence); } Ok(signaled) } - n::FenceInner::Pending(Some(sync)) => { + n::Fence::Pending(sync) => { // TODO: Could `wait_sync` be used here instead? match gl.client_wait_sync(sync, glow::SYNC_FLUSH_COMMANDS_BIT, timeout_ns as i32) { glow::TIMEOUT_EXPIRED => Ok(false), @@ -1823,18 +1819,12 @@ impl d::Device for Device { Ok(false) } glow::CONDITION_SATISFIED | glow::ALREADY_SIGNALED => { - fence.0.set(n::FenceInner::Idle { signaled: true }); + //fence.0.set(n::Fence::Idle { signaled: true }); Ok(true) } _ => unreachable!(), } } - n::FenceInner::Pending(None) => { - // No sync capability, we fallback to waiting for *everything* to finish - gl.flush(); - fence.0.set(n::FenceInner::Idle { signaled: true }); - Ok(true) - } } } @@ -1887,12 +1877,9 @@ impl d::Device for Device { } unsafe fn get_fence_status(&self, fence: &n::Fence) -> Result { - Ok(match fence.0.get() { - n::FenceInner::Pending(Some(sync)) => { - self.share.context.get_sync_status(sync) == glow::SIGNALED - } - n::FenceInner::Pending(None) => false, - n::FenceInner::Idle { signaled } => signaled, + Ok(match *fence { + n::Fence::Idle { signaled } => signaled, + n::Fence::Pending(sync) => self.share.context.get_sync_status(sync) == glow::SIGNALED, }) } @@ -2007,11 +1994,11 @@ impl d::Device for Device { } unsafe fn destroy_fence(&self, fence: n::Fence) { - match fence.0.get() { - n::FenceInner::Pending(Some(sync)) => { + match fence { + n::Fence::Idle { .. } => {} + n::Fence::Pending(sync) => { self.share.context.delete_sync(sync); } - _ => {} } } diff --git a/src/backend/gl/src/native.rs b/src/backend/gl/src/native.rs index 5cd1130851e..9555b01c152 100644 --- a/src/backend/gl/src/native.rs +++ b/src/backend/gl/src/native.rs @@ -57,15 +57,12 @@ impl Buffer { #[derive(Debug)] pub struct BufferView; -#[derive(Copy, Clone, Debug)] -pub(crate) enum FenceInner { +#[derive(Debug)] +pub enum Fence { Idle { signaled: bool }, - Pending(Option<::Fence>), + Pending(::Fence), } -//TODO: reconsider the use of `Cell` -#[derive(Debug)] -pub struct Fence(pub(crate) Cell); unsafe impl Send for Fence {} unsafe impl Sync for Fence {} diff --git a/src/backend/gl/src/queue.rs b/src/backend/gl/src/queue.rs index 1ac763c325c..09f7b988094 100644 --- a/src/backend/gl/src/queue.rs +++ b/src/backend/gl/src/queue.rs @@ -1064,7 +1064,7 @@ impl hal::queue::CommandQueue for CommandQueue { unsafe fn submit<'a, T, Ic, S, Iw, Is>( &mut self, submit_info: hal::queue::Submission, - fence: Option<&native::Fence>, + fence: Option<&mut native::Fence>, ) where T: 'a + Borrow, Ic: IntoIterator, @@ -1097,16 +1097,16 @@ impl hal::queue::CommandQueue for CommandQueue { } if let Some(fence) = fence { - if self.share.private_caps.sync { - fence.0.set(native::FenceInner::Pending(Some( + *fence = if self.share.private_caps.sync { + native::Fence::Pending( self.share .context .fence_sync(glow::SYNC_GPU_COMMANDS_COMPLETE, 0) .unwrap(), - ))); + ) } else { self.share.context.flush(); - fence.0.set(native::FenceInner::Idle { signaled: true }); + native::Fence::Idle { signaled: true } } } } diff --git a/src/backend/metal/src/command.rs b/src/backend/metal/src/command.rs index 7d2046207d4..d8a0349308b 100644 --- a/src/backend/metal/src/command.rs +++ b/src/backend/metal/src/command.rs @@ -2199,7 +2199,7 @@ impl hal::queue::CommandQueue for CommandQueue { wait_semaphores, signal_semaphores, }: hal::queue::Submission, - fence: Option<&native::Fence>, + fence: Option<&mut native::Fence>, ) where T: 'a + Borrow, Ic: IntoIterator, @@ -2376,11 +2376,8 @@ impl hal::queue::CommandQueue for CommandQueue { blocker.submit_impl(cmd_buffer); if let Some(fence) = fence { - debug!( - "\tmarking fence ptr {:?} as pending", - fence.0.raw() as *const _ - ); - *fence.0.lock() = native::FenceInner::PendingSubmission(cmd_buffer.to_owned()); + debug!("\tmarking fence as pending"); + *fence = native::Fence::PendingSubmission(cmd_buffer.to_owned()); } } else if let Some(cmd_buffer) = deferred_cmd_buffer { blocker.submit_impl(cmd_buffer); diff --git a/src/backend/metal/src/device.rs b/src/backend/metal/src/device.rs index 3a53bd739dc..c2fc1ce203d 100644 --- a/src/backend/metal/src/device.rs +++ b/src/backend/metal/src/device.rs @@ -2937,18 +2937,13 @@ impl hal::device::Device for Device { unsafe fn destroy_image_view(&self, _view: n::ImageView) {} fn create_fence(&self, signaled: bool) -> Result { - let mutex = Mutex::new(n::FenceInner::Idle { signaled }); - debug!( - "Creating fence ptr {:?} with signal={}", - unsafe { mutex.raw() } as *const _, - signaled - ); - Ok(n::Fence(mutex)) + debug!("Creating fence with signal={}", signaled); + Ok(n::Fence::Idle { signaled }) } unsafe fn reset_fence(&self, fence: &mut n::Fence) -> Result<(), d::OutOfMemory> { - debug!("Resetting fence ptr {:?}", fence.0.raw() as *const _); - *fence.0.lock() = n::FenceInner::Idle { signaled: false }; + debug!("Resetting fence ptr {:?}", fence); + *fence = n::Fence::Idle { signaled: false }; Ok(()) } @@ -2962,17 +2957,14 @@ impl hal::device::Device for Device { } debug!("wait_for_fence {:?} for {} ms", fence, timeout_ns); - match *fence.0.lock() { - n::FenceInner::Idle { signaled } => { + match *fence { + n::Fence::Idle { signaled } => { if !signaled { - warn!( - "Fence ptr {:?} is not pending, waiting not possible", - fence.0.raw() as *const _ - ); + warn!("Fence ptr {:?} is not pending, waiting not possible", fence); } Ok(signaled) } - n::FenceInner::PendingSubmission(ref cmd_buf) => { + n::Fence::PendingSubmission(ref cmd_buf) => { if timeout_ns == !0 { cmd_buf.wait_until_completed(); return Ok(true); @@ -2993,9 +2985,9 @@ impl hal::device::Device for Device { } unsafe fn get_fence_status(&self, fence: &n::Fence) -> Result { - Ok(match *fence.0.lock() { - n::FenceInner::Idle { signaled } => signaled, - n::FenceInner::PendingSubmission(ref cmd_buf) => match cmd_buf.status() { + Ok(match *fence { + n::Fence::Idle { signaled } => signaled, + n::Fence::PendingSubmission(ref cmd_buf) => match cmd_buf.status() { metal::MTLCommandBufferStatus::Completed => true, _ => false, }, diff --git a/src/backend/metal/src/native.rs b/src/backend/metal/src/native.rs index 175fb6f88ec..0f7e7d18334 100644 --- a/src/backend/metal/src/native.rs +++ b/src/backend/metal/src/native.rs @@ -17,7 +17,7 @@ use range_alloc::RangeAllocator; use arrayvec::ArrayVec; use cocoa_foundation::foundation::NSRange; use metal; -use parking_lot::{Mutex, RwLock}; +use parking_lot::RwLock; use spirv_cross::{msl, spirv}; use std::{ @@ -995,16 +995,11 @@ pub enum QueryPool { } #[derive(Debug)] -pub enum FenceInner { +pub enum Fence { Idle { signaled: bool }, PendingSubmission(metal::CommandBuffer), } -//TODO: reconsider the `Mutex` -// it's only used in `submit()` -#[derive(Debug)] -pub struct Fence(pub(crate) Mutex); - unsafe impl Send for Fence {} unsafe impl Sync for Fence {} diff --git a/src/backend/vulkan/src/lib.rs b/src/backend/vulkan/src/lib.rs index 538333ad53d..0d5fc62aa3b 100644 --- a/src/backend/vulkan/src/lib.rs +++ b/src/backend/vulkan/src/lib.rs @@ -1421,7 +1421,7 @@ impl queue::CommandQueue for CommandQueue { unsafe fn submit<'a, T, Ic, S, Iw, Is>( &mut self, submission: queue::Submission, - fence: Option<&native::Fence>, + fence: Option<&mut native::Fence>, ) where T: 'a + Borrow, Ic: IntoIterator, diff --git a/src/backend/webgpu/src/command.rs b/src/backend/webgpu/src/command.rs index c3a709420fd..bc94585447a 100644 --- a/src/backend/webgpu/src/command.rs +++ b/src/backend/webgpu/src/command.rs @@ -26,7 +26,7 @@ impl hal::queue::CommandQueue for CommandQueue { unsafe fn submit<'a, T, Ic, S, Iw, Is>( &mut self, _submission: Submission, - _fence: Option<&::Fence>, + _fence: Option<&mut ::Fence>, ) where T: 'a + Borrow<::CommandBuffer>, Ic: IntoIterator, @@ -37,17 +37,6 @@ impl hal::queue::CommandQueue for CommandQueue { todo!() } - unsafe fn submit_without_semaphores<'a, T, Ic>( - &mut self, - _command_buffers: Ic, - _fence: Option<&::Fence>, - ) where - T: 'a + Borrow<::CommandBuffer>, - Ic: IntoIterator, - { - todo!() - } - unsafe fn present( &mut self, _surface: &mut ::Surface, @@ -57,7 +46,7 @@ impl hal::queue::CommandQueue for CommandQueue { todo!() } - fn wait_idle(&self) -> Result<(), OutOfMemory> { + fn wait_idle(&mut self) -> Result<(), OutOfMemory> { todo!() } } diff --git a/src/hal/src/queue/mod.rs b/src/hal/src/queue/mod.rs index fd59c0382d7..2dac254da86 100644 --- a/src/hal/src/queue/mod.rs +++ b/src/hal/src/queue/mod.rs @@ -96,7 +96,7 @@ pub trait CommandQueue: fmt::Debug + Any + Send + Sync { unsafe fn submit<'a, T, Ic, S, Iw, Is>( &mut self, submission: Submission, - fence: Option<&B::Fence>, + fence: Option<&mut B::Fence>, ) where T: 'a + Borrow, Ic: IntoIterator, @@ -108,7 +108,7 @@ pub trait CommandQueue: fmt::Debug + Any + Send + Sync { unsafe fn submit_without_semaphores<'a, T, Ic>( &mut self, command_buffers: Ic, - fence: Option<&B::Fence>, + fence: Option<&mut B::Fence>, ) where T: 'a + Borrow, Ic: IntoIterator, diff --git a/src/warden/src/gpu.rs b/src/warden/src/gpu.rs index ae431e02b7e..7a0668dd342 100644 --- a/src/warden/src/gpu.rs +++ b/src/warden/src/gpu.rs @@ -1568,13 +1568,13 @@ impl Scene { cmd_buffer.finish() } - let copy_fence = self + let mut copy_fence = self .device .create_fence(false) .expect("Can't create copy-fence"); unsafe { self.queue_group.queues[0] - .submit_without_semaphores(iter::once(&cmd_buffer), Some(©_fence)); + .submit_without_semaphores(iter::once(&cmd_buffer), Some(&mut copy_fence)); self.device.wait_for_fence(©_fence, !0).unwrap(); self.device.destroy_fence(copy_fence); self.device.destroy_command_pool(command_pool); @@ -1707,13 +1707,13 @@ impl Scene { cmd_buffer.finish(); } - let copy_fence = self + let mut copy_fence = self .device .create_fence(false) .expect("Can't create copy-fence"); unsafe { self.queue_group.queues[0] - .submit_without_semaphores(iter::once(&cmd_buffer), Some(©_fence)); + .submit_without_semaphores(iter::once(&cmd_buffer), Some(&mut copy_fence)); self.device.wait_for_fence(©_fence, !0).unwrap(); self.device.destroy_fence(copy_fence); self.device.destroy_command_pool(command_pool); From 15e5a159d0f3154a1d09c5c5108815f6470bfb62 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Thu, 24 Dec 2020 19:06:39 -0500 Subject: [PATCH 08/15] Update map/unmap memory to use &mut on it --- examples/colour-uniform/main.rs | 14 ++++++------- examples/compute/main.rs | 10 ++++----- examples/mesh-shading/main.rs | 6 +++--- examples/quad/main.rs | 12 +++++------ src/backend/dx11/src/device.rs | 4 ++-- src/backend/dx12/src/device.rs | 4 ++-- src/backend/empty/src/lib.rs | 4 ++-- src/backend/gl/src/device.rs | 20 +++++++++--------- src/backend/gl/src/native.rs | 4 ++-- src/backend/metal/src/device.rs | 4 ++-- src/backend/vulkan/src/device.rs | 4 ++-- src/backend/webgpu/src/device.rs | 20 +++++++----------- src/hal/src/device.rs | 8 +++++-- src/warden/src/gpu.rs | 36 +++++++++++++++++++------------- 14 files changed, 78 insertions(+), 72 deletions(-) diff --git a/examples/colour-uniform/main.rs b/examples/colour-uniform/main.rs index 39e948aee41..eb0a3abdabc 100644 --- a/examples/colour-uniform/main.rs +++ b/examples/colour-uniform/main.rs @@ -691,7 +691,7 @@ impl BufferState { where T: Copy, { - let memory: B::Memory; + let mut memory: B::Memory; let mut buffer: B::Buffer; let size: u64; @@ -726,9 +726,9 @@ impl BufferState { size = mem_req.size; // TODO: check transitions: read/write mapping and vertex buffer read - let mapping = device.map_memory(&memory, m::Segment::ALL).unwrap(); + let mapping = device.map_memory(&mut memory, m::Segment::ALL).unwrap(); ptr::copy_nonoverlapping(data_source.as_ptr() as *const u8, mapping, upload_size); - device.unmap_memory(&memory); + device.unmap_memory(&mut memory); } BufferState { @@ -749,7 +749,7 @@ impl BufferState { let upload_size = data_source.len() * stride; assert!(offset + upload_size as u64 <= self.size); - let memory = self.memory.as_ref().unwrap(); + let memory = self.memory.as_mut().unwrap(); unsafe { let mapping = device @@ -775,7 +775,7 @@ impl BufferState { let row_pitch = (width * stride as u32 + row_alignment_mask) & !row_alignment_mask; let upload_size = (height * row_pitch) as u64; - let memory: B::Memory; + let mut memory: B::Memory; let mut buffer: B::Buffer; let size: u64; @@ -801,7 +801,7 @@ impl BufferState { size = mem_reqs.size; // copy image data into staging buffer - let mapping = device.map_memory(&memory, m::Segment::ALL).unwrap(); + let mapping = device.map_memory(&mut memory, m::Segment::ALL).unwrap(); for y in 0..height as usize { let data_source_slice = &(**img)[y * (width as usize) * stride..(y + 1) * (width as usize) * stride]; @@ -811,7 +811,7 @@ impl BufferState { data_source_slice.len(), ); } - device.unmap_memory(&memory); + device.unmap_memory(&mut memory); } ( diff --git a/examples/compute/main.rs b/examples/compute/main.rs index 5da9f4b586b..8316eb4167d 100644 --- a/examples/compute/main.rs +++ b/examples/compute/main.rs @@ -131,7 +131,7 @@ fn main() { (pipeline_layout, pipeline, set_layout, desc_pool) }; - let (staging_memory, staging_buffer, _staging_size) = unsafe { + let (mut staging_memory, staging_buffer, _staging_size) = unsafe { create_buffer::( &device, &memory_properties.memory_types, @@ -144,14 +144,14 @@ fn main() { unsafe { let mapping = device - .map_memory(&staging_memory, memory::Segment::ALL) + .map_memory(&mut staging_memory, memory::Segment::ALL) .unwrap(); ptr::copy_nonoverlapping( numbers.as_ptr() as *const u8, mapping, numbers.len() * stride as usize, ); - device.unmap_memory(&staging_memory); + device.unmap_memory(&mut staging_memory); } let (device_memory, device_buffer, _device_buffer_size) = unsafe { @@ -239,13 +239,13 @@ fn main() { unsafe { let mapping = device - .map_memory(&staging_memory, memory::Segment::ALL) + .map_memory(&mut staging_memory, memory::Segment::ALL) .unwrap(); println!( "Times: {:?}", slice::from_raw_parts::(mapping as *const u8 as *const u32, numbers.len()), ); - device.unmap_memory(&staging_memory); + device.unmap_memory(&mut staging_memory); } unsafe { diff --git a/examples/mesh-shading/main.rs b/examples/mesh-shading/main.rs index 248bacb9d38..2f7902a534e 100644 --- a/examples/mesh-shading/main.rs +++ b/examples/mesh-shading/main.rs @@ -307,13 +307,13 @@ where // TODO: check transitions: read/write mapping and vertex buffer read let buffer_memory = unsafe { - let memory = device + let mut memory = device .allocate_memory(upload_type, buffer_req.size) .unwrap(); device .bind_buffer_memory(&memory, 0, &mut positions_buffer) .unwrap(); - let mapping = device.map_memory(&memory, m::Segment::ALL).unwrap(); + let mapping = device.map_memory(&mut memory, m::Segment::ALL).unwrap(); ptr::copy_nonoverlapping( positions.as_ptr() as *const u8, mapping, @@ -322,7 +322,7 @@ where device .flush_mapped_memory_ranges(iter::once((&memory, m::Segment::ALL))) .unwrap(); - device.unmap_memory(&memory); + device.unmap_memory(&mut memory); ManuallyDrop::new(memory) }; diff --git a/examples/quad/main.rs b/examples/quad/main.rs index 6c00985d721..a1f05b6e852 100644 --- a/examples/quad/main.rs +++ b/examples/quad/main.rs @@ -323,18 +323,18 @@ where // TODO: check transitions: read/write mapping and vertex buffer read let buffer_memory = unsafe { - let memory = device + let mut memory = device .allocate_memory(upload_type, buffer_req.size) .unwrap(); device .bind_buffer_memory(&memory, 0, &mut vertex_buffer) .unwrap(); - let mapping = device.map_memory(&memory, m::Segment::ALL).unwrap(); + let mapping = device.map_memory(&mut memory, m::Segment::ALL).unwrap(); ptr::copy_nonoverlapping(QUAD.as_ptr() as *const u8, mapping, buffer_len as usize); device .flush_mapped_memory_ranges(iter::once((&memory, m::Segment::ALL))) .unwrap(); - device.unmap_memory(&memory); + device.unmap_memory(&mut memory); ManuallyDrop::new(memory) }; @@ -362,13 +362,13 @@ where // copy image data into staging buffer let image_upload_memory = unsafe { - let memory = device + let mut memory = device .allocate_memory(upload_type, image_mem_reqs.size) .unwrap(); device .bind_buffer_memory(&memory, 0, &mut image_upload_buffer) .unwrap(); - let mapping = device.map_memory(&memory, m::Segment::ALL).unwrap(); + let mapping = device.map_memory(&mut memory, m::Segment::ALL).unwrap(); for y in 0..height as usize { let row = &(*img)[y * (width as usize) * image_stride ..(y + 1) * (width as usize) * image_stride]; @@ -381,7 +381,7 @@ where device .flush_mapped_memory_ranges(iter::once((&memory, m::Segment::ALL))) .unwrap(); - device.unmap_memory(&memory); + device.unmap_memory(&mut memory); ManuallyDrop::new(memory) }; diff --git a/src/backend/dx11/src/device.rs b/src/backend/dx11/src/device.rs index e5d90797823..a63f9ecbbd8 100644 --- a/src/backend/dx11/src/device.rs +++ b/src/backend/dx11/src/device.rs @@ -2139,13 +2139,13 @@ impl device::Device for Device { unsafe fn map_memory( &self, - memory: &Memory, + memory: &mut Memory, segment: memory::Segment, ) -> Result<*mut u8, device::MapError> { Ok(memory.host_ptr.offset(segment.offset as isize)) } - unsafe fn unmap_memory(&self, _memory: &Memory) { + unsafe fn unmap_memory(&self, _memory: &mut Memory) { // persistent mapping FTW } diff --git a/src/backend/dx12/src/device.rs b/src/backend/dx12/src/device.rs index 73f6be25102..7f82ba84356 100644 --- a/src/backend/dx12/src/device.rs +++ b/src/backend/dx12/src/device.rs @@ -3224,7 +3224,7 @@ impl d::Device for Device { unsafe fn map_memory( &self, - memory: &r::Memory, + memory: &mut r::Memory, segment: memory::Segment, ) -> Result<*mut u8, d::MapError> { let mem = memory @@ -3239,7 +3239,7 @@ impl d::Device for Device { Ok(ptr as *mut _) } - unsafe fn unmap_memory(&self, memory: &r::Memory) { + unsafe fn unmap_memory(&self, memory: &mut r::Memory) { if let Some(mem) = memory.resource { (*mem).Unmap(0, &d3d12::D3D12_RANGE { Begin: 0, End: 0 }); } diff --git a/src/backend/empty/src/lib.rs b/src/backend/empty/src/lib.rs index 2a319c58423..b83a51eaf5b 100644 --- a/src/backend/empty/src/lib.rs +++ b/src/backend/empty/src/lib.rs @@ -470,13 +470,13 @@ impl device::Device for Device { unsafe fn map_memory( &self, - memory: &Memory, + memory: &mut Memory, segment: hal::memory::Segment, ) -> Result<*mut u8, device::MapError> { memory.map(segment) } - unsafe fn unmap_memory(&self, _memory: &Memory) {} + unsafe fn unmap_memory(&self, _memory: &mut Memory) {} unsafe fn flush_mapped_memory_ranges<'a, I>(&self, _: I) -> Result<(), device::OutOfMemory> where diff --git a/src/backend/gl/src/device.rs b/src/backend/gl/src/device.rs index 65d19488c12..b0281110319 100644 --- a/src/backend/gl/src/device.rs +++ b/src/backend/gl/src/device.rs @@ -19,7 +19,7 @@ use glow::HasContext; use parking_lot::Mutex; use spirv_cross::{glsl, spirv, ErrorCode as SpirvErrorCode}; -use std::{borrow::Borrow, cell::Cell, ops::Range, slice, sync::Arc}; +use std::{borrow::Borrow, ops::Range, slice, sync::Arc}; /// Emit error during shader module creation. Used if we don't expect an error /// but might panic due to an exception in SPIRV-Cross. @@ -758,7 +758,7 @@ impl d::Device for Device { buffer: Some((raw, target)), size, map_flags, - emulate_map_allocation: Cell::new(None), + emulate_map_allocation: None, }) } @@ -769,7 +769,7 @@ impl d::Device for Device { buffer: None, size, map_flags: 0, - emulate_map_allocation: Cell::new(None), + emulate_map_allocation: None, }) } } @@ -1261,7 +1261,7 @@ impl d::Device for Device { unsafe fn map_memory( &self, - memory: &n::Memory, + memory: &mut n::Memory, segment: memory::Segment, ) -> Result<*mut u8, d::MapError> { let gl = &self.share.context; @@ -1272,12 +1272,12 @@ impl d::Device for Device { let (buffer, target) = memory.buffer.expect("cannot map image memory"); let ptr = if caps.emulate_map { - let ptr: *mut u8 = if let Some(ptr) = memory.emulate_map_allocation.get() { + let ptr: *mut u8 = if let Some(ptr) = memory.emulate_map_allocation { ptr } else { let ptr = Box::into_raw(vec![0; memory.size as usize].into_boxed_slice()) as *mut u8; - memory.emulate_map_allocation.set(Some(ptr)); + memory.emulate_map_allocation = Some(ptr); ptr }; @@ -1296,14 +1296,14 @@ impl d::Device for Device { Ok(ptr) } - unsafe fn unmap_memory(&self, memory: &n::Memory) { + unsafe fn unmap_memory(&self, memory: &mut n::Memory) { let gl = &self.share.context; let (buffer, target) = memory.buffer.expect("cannot unmap image memory"); gl.bind_buffer(target, Some(buffer)); if self.share.private_caps.emulate_map { - let ptr = memory.emulate_map_allocation.replace(None).unwrap(); + let ptr = memory.emulate_map_allocation.take().unwrap(); let _ = Box::from_raw(slice::from_raw_parts_mut(ptr, memory.size as usize)); } else { gl.unmap_buffer(target); @@ -1332,7 +1332,7 @@ impl d::Device for Device { let size = segment.size.unwrap_or(mem.size - segment.offset); if self.share.private_caps.emulate_map { - let ptr = mem.emulate_map_allocation.get().unwrap(); + let ptr = mem.emulate_map_allocation.unwrap(); let slice = slice::from_raw_parts_mut(ptr.offset(offset as isize), size as usize); gl.buffer_sub_data_u8_slice(target, offset as i32, slice); } else { @@ -1366,7 +1366,7 @@ impl d::Device for Device { let size = segment.size.unwrap_or(mem.size - segment.offset); if self.share.private_caps.emulate_map { - let ptr = mem.emulate_map_allocation.get().unwrap(); + let ptr = mem.emulate_map_allocation.unwrap(); let slice = slice::from_raw_parts_mut(ptr.offset(offset as isize), size as usize); gl.get_buffer_sub_data(target, offset as i32, slice); } else { diff --git a/src/backend/gl/src/native.rs b/src/backend/gl/src/native.rs index 9555b01c152..eec3569f37f 100644 --- a/src/backend/gl/src/native.rs +++ b/src/backend/gl/src/native.rs @@ -6,7 +6,7 @@ use hal::{ pass, pso, window as w, }; -use std::{borrow::Borrow, cell::Cell, ops::Range, sync::Arc}; +use std::{borrow::Borrow, ops::Range, sync::Arc}; pub type TextureTarget = u32; pub type TextureFormat = u32; @@ -285,7 +285,7 @@ pub struct Memory { /// Allocation size pub(crate) size: u64, pub(crate) map_flags: u32, - pub(crate) emulate_map_allocation: Cell>, + pub(crate) emulate_map_allocation: Option<*mut u8>, } unsafe impl Send for Memory {} diff --git a/src/backend/metal/src/device.rs b/src/backend/metal/src/device.rs index c2fc1ce203d..d1aaf39fb8e 100644 --- a/src/backend/metal/src/device.rs +++ b/src/backend/metal/src/device.rs @@ -1893,7 +1893,7 @@ impl hal::device::Device for Device { unsafe fn map_memory( &self, - memory: &n::Memory, + memory: &mut n::Memory, segment: memory::Segment, ) -> Result<*mut u8, d::MapError> { let range = memory.resolve(&segment); @@ -1906,7 +1906,7 @@ impl hal::device::Device for Device { Ok(base_ptr.offset(range.start as _)) } - unsafe fn unmap_memory(&self, memory: &n::Memory) { + unsafe fn unmap_memory(&self, memory: &mut n::Memory) { debug!("unmap_memory of size {}", memory.size); } diff --git a/src/backend/vulkan/src/device.rs b/src/backend/vulkan/src/device.rs index 932da235107..716baee4ff8 100644 --- a/src/backend/vulkan/src/device.rs +++ b/src/backend/vulkan/src/device.rs @@ -1764,7 +1764,7 @@ impl d::Device for Device { unsafe fn map_memory( &self, - memory: &n::Memory, + memory: &mut n::Memory, segment: Segment, ) -> Result<*mut u8, d::MapError> { let result = self.shared.raw.map_memory( @@ -1783,7 +1783,7 @@ impl d::Device for Device { } } - unsafe fn unmap_memory(&self, memory: &n::Memory) { + unsafe fn unmap_memory(&self, memory: &mut n::Memory) { self.shared.raw.unmap_memory(memory.raw) } diff --git a/src/backend/webgpu/src/device.rs b/src/backend/webgpu/src/device.rs index 308d343f918..5e6e371b83e 100644 --- a/src/backend/webgpu/src/device.rs +++ b/src/backend/webgpu/src/device.rs @@ -342,25 +342,21 @@ impl hal::device::Device for Device { todo!() } - unsafe fn write_descriptor_sets<'a, I, J>(&self, _write_iter: I) + unsafe fn write_descriptor_set<'a, I>(&self, _op: pso::DescriptorSetWrite<'a, Backend, I>) where - I: IntoIterator>, - J: IntoIterator, - J::Item: Borrow>, + I: IntoIterator, + I::Item: Borrow>, { todo!() } - unsafe fn copy_descriptor_sets<'a, I>(&self, _copy_iter: I) - where - I: IntoIterator>, - { + unsafe fn copy_descriptor_set<'a>(&self, _op: pso::DescriptorSetCopy<'a, Backend>) { todo!() } unsafe fn map_memory( &self, - _memory: &::Memory, + _memory: &mut ::Memory, _segment: Segment, ) -> Result<*mut u8, MapError> { todo!() @@ -382,7 +378,7 @@ impl hal::device::Device for Device { todo!() } - unsafe fn unmap_memory(&self, _memory: &::Memory) { + unsafe fn unmap_memory(&self, _memory: &mut ::Memory) { todo!() } @@ -494,13 +490,13 @@ impl hal::device::Device for Device { _pool: &::QueryPool, _queries: Range, _data: &mut [u8], - _stride: buffer::Offset, + _stride: buffer::Stride, _flags: query::ResultFlags, ) -> Result { todo!() } - fn wait_idle(&mut self) -> Result<(), OutOfMemory> { + fn wait_idle(&self) -> Result<(), OutOfMemory> { todo!() } diff --git a/src/hal/src/device.rs b/src/hal/src/device.rs index 39ac1685e62..0030f073c61 100644 --- a/src/hal/src/device.rs +++ b/src/hal/src/device.rs @@ -567,7 +567,11 @@ pub trait Device: fmt::Debug + Any + Send + Sync { /// Map a memory object into application address space /// /// Call `map_memory()` to retrieve a host virtual address pointer to a region of a mappable memory object - unsafe fn map_memory(&self, memory: &B::Memory, segment: Segment) -> Result<*mut u8, MapError>; + unsafe fn map_memory( + &self, + memory: &mut B::Memory, + segment: Segment, + ) -> Result<*mut u8, MapError>; /// Flush mapped memory ranges unsafe fn flush_mapped_memory_ranges<'a, I>(&self, ranges: I) -> Result<(), OutOfMemory> @@ -582,7 +586,7 @@ pub trait Device: fmt::Debug + Any + Send + Sync { I::Item: Borrow<(&'a B::Memory, Segment)>; /// Unmap a memory object once host access to it is no longer needed by the application - unsafe fn unmap_memory(&self, memory: &B::Memory); + unsafe fn unmap_memory(&self, memory: &mut B::Memory); /// Create a new semaphore object. fn create_semaphore(&self) -> Result; diff --git a/src/warden/src/gpu.rs b/src/warden/src/gpu.rs index 7a0668dd342..2383967b5cc 100644 --- a/src/warden/src/gpu.rs +++ b/src/warden/src/gpu.rs @@ -34,9 +34,9 @@ impl<'a, B: hal::Backend> FetchGuard<'a, B> { impl<'a, B: hal::Backend> Drop for FetchGuard<'a, B> { fn drop(&mut self) { let buffer = self.buffer.take().unwrap(); - let memory = self.memory.take().unwrap(); + let mut memory = self.memory.take().unwrap(); unsafe { - self.device.unmap_memory(&memory); + self.device.unmap_memory(&mut memory); self.device.destroy_buffer(buffer); self.device.free_memory(memory); } @@ -342,7 +342,7 @@ impl Scene { .iter() .find(|i| upload_req.type_mask & (1 << i.0) != 0) .unwrap(); - let upload_memory = + let mut upload_memory = unsafe { device.allocate_memory(upload_type, upload_req.size) } .unwrap(); @@ -351,13 +351,13 @@ impl Scene { // write the data unsafe { let mapping = device - .map_memory(&upload_memory, memory::Segment::ALL) + .map_memory(&mut upload_memory, memory::Segment::ALL) .unwrap(); File::open(data_path.join(data)) .unwrap() .read_exact(slice::from_raw_parts_mut(mapping, size)) .unwrap(); - device.unmap_memory(&upload_memory); + device.unmap_memory(&mut upload_memory); } // add init commands let final_state = b::Access::SHADER_READ; @@ -500,7 +500,7 @@ impl Scene { .iter() .find(|i| upload_req.type_mask & (1 << i.0) != 0) .unwrap(); - let upload_memory = + let mut upload_memory = unsafe { device.allocate_memory(upload_type, upload_req.size) } .unwrap(); unsafe { device.bind_buffer_memory(&upload_memory, 0, &mut upload_buffer) } @@ -509,7 +509,7 @@ impl Scene { unsafe { let mut file = File::open(data_path.join(data)).unwrap(); let mapping = device - .map_memory(&upload_memory, memory::Segment::ALL) + .map_memory(&mut upload_memory, memory::Segment::ALL) .unwrap(); for y in 0..(h as usize * d as usize) { let slice = slice::from_raw_parts_mut( @@ -518,7 +518,7 @@ impl Scene { ); file.read_exact(slice).unwrap(); } - device.unmap_memory(&upload_memory); + device.unmap_memory(&mut upload_memory); } // add init commands let final_state = @@ -1517,7 +1517,7 @@ impl Scene { .iter() .find(|i| down_req.type_mask & (1 << i.0) != 0) .unwrap(); - let down_memory = + let mut down_memory = unsafe { self.device.allocate_memory(download_type, down_req.size) }.unwrap(); unsafe { @@ -1580,8 +1580,11 @@ impl Scene { self.device.destroy_command_pool(command_pool); } - let mapping = - unsafe { self.device.map_memory(&down_memory, memory::Segment::ALL) }.unwrap(); + let mapping = unsafe { + self.device + .map_memory(&mut down_memory, memory::Segment::ALL) + } + .unwrap(); FetchGuard { device: &mut self.device, @@ -1629,11 +1632,11 @@ impl Scene { .iter() .find(|i| down_req.type_mask & (1 << i.0) != 0) .unwrap(); - let down_memory = + let mut down_memory = unsafe { self.device.allocate_memory(download_type, down_req.size) }.unwrap(); unsafe { self.device - .bind_buffer_memory(&down_memory, 0, &mut down_buffer) + .bind_buffer_memory(&mut down_memory, 0, &mut down_buffer) } .unwrap(); @@ -1719,8 +1722,11 @@ impl Scene { self.device.destroy_command_pool(command_pool); } - let mapping = - unsafe { self.device.map_memory(&down_memory, memory::Segment::ALL) }.unwrap(); + let mapping = unsafe { + self.device + .map_memory(&mut down_memory, memory::Segment::ALL) + } + .unwrap(); FetchGuard { device: &mut self.device, From 57dbb5858ef67866bfcec24739c4444f1839a27d Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Sun, 27 Dec 2020 17:45:00 -0500 Subject: [PATCH 09/15] Check for GL program linking before using it --- src/backend/gl/src/device.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/backend/gl/src/device.rs b/src/backend/gl/src/device.rs index b468ada9be5..ad03de5f16e 100644 --- a/src/backend/gl/src/device.rs +++ b/src/backend/gl/src/device.rs @@ -151,6 +151,16 @@ impl Device { panic!("Error linking program: {:?}", err); } + let linked_ok = unsafe { gl.get_program_link_status(program) }; + let log = unsafe { gl.get_program_info_log(program) }; + if !linked_ok { + error!("\tLog: {}", log); + return Err(pso::CreationError::Other); + } + if !log.is_empty() { + warn!("\tLog: {}", log); + } + if !self .share .legacy_features @@ -177,17 +187,7 @@ impl Device { } } - let linked_ok = unsafe { gl.get_program_link_status(program) }; - let log = unsafe { gl.get_program_info_log(program) }; - if linked_ok { - if !log.is_empty() { - warn!("\tLog: {}", log); - } - Ok((program, sampler_map)) - } else { - error!("\tLog: {}", log); - Err(pso::CreationError::Other) - } + Ok((program, sampler_map)) } fn bind_target_compat(gl: &GlContainer, point: u32, attachment: u32, view: &n::ImageView) { From 7b5b29a1c0cd3bbf7c54d471cbc1c33c0c95b416 Mon Sep 17 00:00:00 2001 From: Michael Tang Date: Sun, 27 Dec 2020 17:07:18 -0800 Subject: [PATCH 10/15] Fix WebGPU build from #3546 --- src/backend/webgpu/src/command.rs | 2 +- src/backend/webgpu/src/device.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/backend/webgpu/src/command.rs b/src/backend/webgpu/src/command.rs index 9f298775504..f8eb432f342 100644 --- a/src/backend/webgpu/src/command.rs +++ b/src/backend/webgpu/src/command.rs @@ -519,7 +519,7 @@ impl hal::command::CommandBuffer for CommandBuffer { _queries: Range, _buffer: &::Buffer, _offset: buffer::Offset, - _stride: buffer::Offset, + _stride: buffer::Stride, _flags: query::ResultFlags, ) { todo!() diff --git a/src/backend/webgpu/src/device.rs b/src/backend/webgpu/src/device.rs index 26ec1856261..923ec0da53f 100644 --- a/src/backend/webgpu/src/device.rs +++ b/src/backend/webgpu/src/device.rs @@ -492,7 +492,7 @@ impl hal::device::Device for Device { _pool: &::QueryPool, _queries: Range, _data: &mut [u8], - _stride: buffer::Offset, + _stride: buffer::Stride, _flags: query::ResultFlags, ) -> Result { todo!() From f45763107a64ee8ee259e24b49a86c124e1d45e4 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Sat, 2 Jan 2021 19:48:28 -0500 Subject: [PATCH 11/15] [metal] use metal-rs github, implement transparency --- src/backend/dx11/src/lib.rs | 4 +++- src/backend/metal/Cargo.toml | 3 ++- src/backend/metal/src/command.rs | 4 ++-- src/backend/metal/src/device.rs | 6 ++++-- src/backend/metal/src/lib.rs | 20 ++++++++++---------- src/backend/metal/src/native.rs | 5 ++--- src/backend/metal/src/window.rs | 28 ++++++++++++++++++---------- 7 files changed, 41 insertions(+), 29 deletions(-) diff --git a/src/backend/dx11/src/lib.rs b/src/backend/dx11/src/lib.rs index d68c1e116cb..1a2a379843f 100644 --- a/src/backend/dx11/src/lib.rs +++ b/src/backend/dx11/src/lib.rs @@ -971,7 +971,9 @@ impl window::PresentationSurface for Surface { // We must also delete the image data. // // This should not panic as all images must be deleted before - let mut present_image = Arc::try_unwrap(present.image).expect("Not all acquired images were deleted before the swapchain was reconfigured."); + let mut present_image = Arc::try_unwrap(present.image).expect( + "Not all acquired images were deleted before the swapchain was reconfigured.", + ); present_image.internal.release_resources(); let result = present.swapchain.ResizeBuffers( diff --git a/src/backend/metal/Cargo.toml b/src/backend/metal/Cargo.toml index 535f81deaa3..78e148944a9 100644 --- a/src/backend/metal/Cargo.toml +++ b/src/backend/metal/Cargo.toml @@ -30,10 +30,11 @@ bitflags = "1.0" copyless = "0.1.4" log = { version = "0.4" } dispatch = { version = "0.2", optional = true } -metal = { version = "0.20", features = ["private"] } +metal = { git = "https://github.com/gfx-rs/metal-rs", rev="ba08f5f98c70ab941020b8997936c9c75363b9aa", features = ["private"] } foreign-types = "0.3" objc = "0.2.5" block = "0.1" +#TODO: remove this, only used for `Id` cocoa-foundation = "0.1" spirv_cross = { version = "0.22", features = ["msl"] } parking_lot = "0.11" diff --git a/src/backend/metal/src/command.rs b/src/backend/metal/src/command.rs index 198de893a7a..ef4c243a2e9 100644 --- a/src/backend/metal/src/command.rs +++ b/src/backend/metal/src/command.rs @@ -21,12 +21,12 @@ use hal::{ use arrayvec::ArrayVec; use auxil::{FastHashMap, ShaderStage}; use block::ConcreteBlock; -use cocoa_foundation::foundation::{NSRange, NSUInteger}; +use cocoa_foundation::foundation::NSUInteger; use copyless::VecHelper; #[cfg(feature = "dispatch")] use dispatch; use foreign_types::ForeignType; -use metal::{self, MTLIndexType, MTLPrimitiveType, MTLScissorRect, MTLSize, MTLViewport}; +use metal::{self, MTLIndexType, MTLPrimitiveType, MTLScissorRect, MTLSize, MTLViewport, NSRange}; use objc::rc::autoreleasepool; use parking_lot::Mutex; diff --git a/src/backend/metal/src/device.rs b/src/backend/metal/src/device.rs index 3d6a0ccb3b2..555c101a1fd 100644 --- a/src/backend/metal/src/device.rs +++ b/src/backend/metal/src/device.rs @@ -7,7 +7,7 @@ use crate::{ use arrayvec::ArrayVec; use auxil::{spirv_cross_specialize_ast, FastHashMap, ShaderStage}; -use cocoa_foundation::foundation::{NSRange, NSUInteger}; +use cocoa_foundation::foundation::NSUInteger; use copyless::VecHelper; use foreign_types::{ForeignType, ForeignTypeRef}; use hal::{ @@ -23,7 +23,7 @@ use hal::{ use metal::{ CaptureManager, MTLCPUCacheMode, MTLLanguageVersion, MTLPrimitiveTopologyClass, MTLPrimitiveType, MTLResourceOptions, MTLSamplerMipFilter, MTLStorageMode, MTLTextureType, - MTLVertexStepFunction, + MTLVertexStepFunction, NSRange, }; use objc::{ rc::autoreleasepool, @@ -1301,6 +1301,7 @@ impl hal::device::Device for Device { MTLLanguageVersion::V2_0 => (2, 0), MTLLanguageVersion::V2_1 => (2, 1), MTLLanguageVersion::V2_2 => (2, 2), + MTLLanguageVersion::V2_3 => (2, 3), }, spirv_cross_compatibility: true, binding_map: res_overrides @@ -1336,6 +1337,7 @@ impl hal::device::Device for Device { MTLLanguageVersion::V2_0 => msl::Version::V2_0, MTLLanguageVersion::V2_1 => msl::Version::V2_1, MTLLanguageVersion::V2_2 => msl::Version::V2_2, + MTLLanguageVersion::V2_3 => msl::Version::V2_2, //TODO: update this! }; shader_compiler_options.enable_point_size_builtin = false; shader_compiler_options.vertex.invert_y = !self.features.contains(hal::Features::NDC_Y_UP); diff --git a/src/backend/metal/src/lib.rs b/src/backend/metal/src/lib.rs index 5c51cb91a75..987ca896651 100644 --- a/src/backend/metal/src/lib.rs +++ b/src/backend/metal/src/lib.rs @@ -71,7 +71,7 @@ use foreign_types::ForeignTypeRef; use lazy_static::lazy_static; use metal::MTLFeatureSet; use metal::MTLLanguageVersion; -use metal::{CGFloat, CGSize, CoreAnimationLayer, CoreAnimationLayerRef}; +use metal::{CGFloat, CGSize, MetalLayer, MetalLayerRef}; use objc::{ declare::ClassDecl, runtime::{Class, Object, Sel, BOOL, YES}, @@ -361,11 +361,11 @@ impl Instance { let class = class!(CAMetalLayer); let is_valid_layer: BOOL = msg_send![main_layer, isKindOfClass: class]; let render_layer = if is_valid_layer == YES { - mem::transmute::<_, &CoreAnimationLayerRef>(main_layer).to_owned() + mem::transmute::<_, &MetalLayerRef>(main_layer).to_owned() } else { // If the main layer is not a CAMetalLayer, we create a CAMetalLayer sublayer and use it instead. // Unlike on macOS, we cannot replace the main view as UIView does not allow it (when NSView does). - let new_layer: CoreAnimationLayer = msg_send![class, new]; + let new_layer: MetalLayer = msg_send![class, new]; let bounds: CGRect = msg_send![main_layer, bounds]; let () = msg_send![new_layer.as_ref(), setFrame: bounds]; let () = msg_send![main_layer, addSublayer: new_layer.as_ref()]; @@ -407,10 +407,10 @@ impl Instance { result == YES }; - let render_layer: CoreAnimationLayer = if use_current { - mem::transmute::<_, &CoreAnimationLayerRef>(existing).to_owned() + let render_layer: MetalLayer = if use_current { + mem::transmute::<_, &MetalLayerRef>(existing).to_owned() } else { - let layer: CoreAnimationLayer = msg_send![class, new]; + let layer: MetalLayer = msg_send![class, new]; let () = msg_send![view, setLayer: layer.as_ref()]; let () = msg_send![view, setWantsLayer: YES]; let bounds: CGRect = msg_send![view, bounds]; @@ -429,14 +429,14 @@ impl Instance { Surface::new(NonNull::new(view), render_layer) } - unsafe fn create_from_layer(&self, layer: &CoreAnimationLayerRef) -> Surface { + unsafe fn create_from_layer(&self, layer: &MetalLayerRef) -> Surface { let class = class!(CAMetalLayer); let proper_kind: BOOL = msg_send![layer, isKindOfClass: class]; assert_eq!(proper_kind, YES); Surface::new(None, layer.to_owned()) } - pub fn create_surface_from_layer(&self, layer: &CoreAnimationLayerRef) -> Surface { + pub fn create_surface_from_layer(&self, layer: &MetalLayerRef) -> Surface { unsafe { self.create_from_layer(layer) } } @@ -785,10 +785,10 @@ impl PrivateCapabilities { let os_is_mac = device.supports_feature_set(MTLFeatureSet::macOS_GPUFamily1_v1); let mut sample_count_mask: u8 = 1 | 4; // 1 and 4 samples are supported on all devices - if device.supports_sample_count(2) { + if device.supports_texture_sample_count(2) { sample_count_mask |= 2; } - if device.supports_sample_count(8) { + if device.supports_texture_sample_count(8) { sample_count_mask |= 8; } diff --git a/src/backend/metal/src/native.rs b/src/backend/metal/src/native.rs index c25e8cabe63..2c6609070a2 100644 --- a/src/backend/metal/src/native.rs +++ b/src/backend/metal/src/native.rs @@ -15,7 +15,6 @@ use hal::{ use range_alloc::RangeAllocator; use arrayvec::ArrayVec; -use cocoa_foundation::foundation::NSRange; use metal; use parking_lot::{Mutex, RwLock}; use spirv_cross::{msl, spirv}; @@ -352,11 +351,11 @@ impl Image { Some(raw.new_texture_view_from_slice( self.mtl_format, metal::MTLTextureType::D2Array, - NSRange { + metal::NSRange { location: 0, length: raw.mipmap_level_count(), }, - NSRange { + metal::NSRange { location: 0, length: self.kind.num_layers() as _, }, diff --git a/src/backend/metal/src/window.rs b/src/backend/metal/src/window.rs index fe3276a6e48..07af7c2a21c 100644 --- a/src/backend/metal/src/window.rs +++ b/src/backend/metal/src/window.rs @@ -7,7 +7,6 @@ use crate::{ use hal::{format, image, window as w}; use crate::CGRect; -use metal::{CGFloat, CGSize, CoreAnimationDrawable}; use objc::rc::autoreleasepool; use objc::runtime::Object; use parking_lot::Mutex; @@ -19,7 +18,7 @@ use std::thread; #[derive(Debug)] pub struct Surface { view: Option>, - render_layer: Mutex, + render_layer: Mutex, swapchain_format: metal::MTLPixelFormat, swapchain_format_desc: format::FormatDesc, main_thread_id: thread::ThreadId, @@ -29,7 +28,7 @@ unsafe impl Send for Surface {} unsafe impl Sync for Surface {} impl Surface { - pub fn new(view: Option>, layer: metal::CoreAnimationLayer) -> Self { + pub fn new(view: Option>, layer: metal::MetalLayer) -> Self { Surface { view, render_layer: Mutex::new(layer), @@ -68,7 +67,14 @@ impl Surface { caps.has_version_at_least(11, 0) }; let can_set_display_sync = is_mac && caps.has_version_at_least(10, 13); - let drawable_size = CGSize::new(config.extent.width as f64, config.extent.height as f64); + let drawable_size = + metal::CGSize::new(config.extent.width as f64, config.extent.height as f64); + + match config.composite_alpha_mode { + w::CompositeAlphaMode::OPAQUE => render_layer.set_opaque(true), + w::CompositeAlphaMode::POSTMULTIPLIED => render_layer.set_opaque(false), + _ => (), + } let device_raw = shared.device.lock(); unsafe { @@ -104,7 +110,7 @@ impl Surface { } fn dimensions(&self) -> w::Extent2D { - let (size, scale): (CGSize, CGFloat) = match self.view { + let (size, scale): (metal::CGSize, metal::CGFloat) = match self.view { Some(view) if !cfg!(target_os = "macos") => unsafe { let bounds: CGRect = msg_send![view.as_ptr(), bounds]; let window: Option> = msg_send![view.as_ptr(), window]; @@ -115,7 +121,7 @@ impl Surface { Some(screen) => { let screen_space: *mut Object = msg_send![screen.as_ptr(), coordinateSpace]; let rect: CGRect = msg_send![view.as_ptr(), convertRect:bounds toCoordinateSpace:screen_space]; - let scale_factor: CGFloat = msg_send![screen.as_ptr(), nativeScale]; + let scale_factor: metal::CGFloat = msg_send![screen.as_ptr(), nativeScale]; (rect.size, scale_factor) } None => (bounds.size, 1.0), @@ -125,7 +131,7 @@ impl Surface { let render_layer_borrow = self.render_layer.lock(); let render_layer = render_layer_borrow.as_ref(); let bounds: CGRect = msg_send![render_layer, bounds]; - let contents_scale: CGFloat = msg_send![render_layer, contentsScale]; + let contents_scale: metal::CGFloat = msg_send![render_layer, contentsScale]; (bounds.size, contents_scale) }, }; @@ -152,14 +158,14 @@ impl Default for AcquireMode { pub struct SwapchainImage { image: native::Image, view: native::ImageView, - drawable: metal::CoreAnimationDrawable, + drawable: metal::MetalDrawable, } unsafe impl Send for SwapchainImage {} unsafe impl Sync for SwapchainImage {} impl SwapchainImage { - pub(crate) fn into_drawable(self) -> CoreAnimationDrawable { + pub(crate) fn into_drawable(self) -> metal::MetalDrawable { self.drawable } } @@ -203,7 +209,9 @@ impl w::Surface for Surface { } else { w::PresentMode::FIFO }, - composite_alpha_modes: w::CompositeAlphaMode::OPAQUE, //TODO + composite_alpha_modes: w::CompositeAlphaMode::OPAQUE + | w::CompositeAlphaMode::POSTMULTIPLIED + | w::CompositeAlphaMode::INHERIT, //Note: this is hardcoded in `CAMetalLayer` documentation image_count: if can_set_maximum_drawables_count { 2..=3 From 79cdd6f779db39f21737b432fd783d62a09155ca Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 29 Dec 2020 12:47:44 +0100 Subject: [PATCH 12/15] Distinguish STORAGE from STORAGE_READ_WRITE image feature Implemented for Vulkan/DX12/DX11 In Vulkan, storage image implies simultaneous read/write access. Other APIs in contrast may allow STORAGE writeonly while disallowing reading. --- src/backend/dx11/src/lib.rs | 7 +++++++ src/backend/dx12/src/lib.rs | 5 +++++ src/backend/vulkan/src/conv.rs | 7 ++++++- src/hal/src/format.rs | 6 ++++-- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/backend/dx11/src/lib.rs b/src/backend/dx11/src/lib.rs index c24eef941d8..bbd0cec17d8 100644 --- a/src/backend/dx11/src/lib.rs +++ b/src/backend/dx11/src/lib.rs @@ -410,7 +410,14 @@ fn get_format_properties( props.buffer_features |= format::BufferFeature::STORAGE_TEXEL; } if can_image { + // Since read-only storage is exposed as SRV, we can guarantee read-only storage without checking D3D11_FORMAT_SUPPORT2_UAV_TYPED_LOAD first. props.optimal_tiling |= format::ImageFeature::STORAGE; + + if support_2.OutFormatSupport2 & d3d11::D3D11_FORMAT_SUPPORT2_UAV_TYPED_LOAD + != 0 + { + props.optimal_tiling |= format::ImageFeature::STORAGE_READ_WRITE; + } } } } diff --git a/src/backend/dx12/src/lib.rs b/src/backend/dx12/src/lib.rs index 82febee3cde..84bdad1eeb5 100644 --- a/src/backend/dx12/src/lib.rs +++ b/src/backend/dx12/src/lib.rs @@ -1456,7 +1456,12 @@ impl FormatProperties { props.buffer_features |= f::BufferFeature::STORAGE_TEXEL; } if can_image { + // Since read-only storage is exposed as SRV, we can guarantee read-only storage without checking D3D11_FORMAT_SUPPORT2_UAV_TYPED_LOAD first. props.optimal_tiling |= f::ImageFeature::STORAGE; + + if data.Support2 & d3d12::D3D12_FORMAT_SUPPORT2_UAV_TYPED_LOAD != 0 { + props.optimal_tiling |= f::ImageFeature::STORAGE_READ_WRITE; + } } } //TODO: blits, linear tiling diff --git a/src/backend/vulkan/src/conv.rs b/src/backend/vulkan/src/conv.rs index 342b18738ac..e70fdd8636f 100644 --- a/src/backend/vulkan/src/conv.rs +++ b/src/backend/vulkan/src/conv.rs @@ -378,7 +378,12 @@ pub fn map_query_result_flags(flags: query::ResultFlags) -> vk::QueryResultFlags } pub fn map_image_features(features: vk::FormatFeatureFlags) -> format::ImageFeature { - format::ImageFeature::from_bits_truncate(features.as_raw()) + let bits = format::ImageFeature::from_bits_truncate(features.as_raw()); + if bits.contains(format::ImageFeature::STORAGE) { + bits | format::ImageFeature::STORAGE_READ_WRITE + } else { + bits + } } pub fn map_buffer_features(features: vk::FormatFeatureFlags) -> format::BufferFeature { diff --git a/src/hal/src/format.rs b/src/hal/src/format.rs index 2415b57cf23..e5d0d40749d 100644 --- a/src/hal/src/format.rs +++ b/src/hal/src/format.rs @@ -148,10 +148,12 @@ bitflags!( pub struct ImageFeature: u32 { /// Image view can be sampled. const SAMPLED = 0x1; - /// Image view can be used as storage image. + /// Image view can be used as storage image with exclusive read & write access. const STORAGE = 0x2; - /// Image view can be used as storage image (with atomics). + /// Image view can be used as storage image with atomics. const STORAGE_ATOMIC = 0x4; + /// Image view can be used as storage image with simultaneous read/write access. + const STORAGE_READ_WRITE = 0x8; /// Image view can be used as color and input attachment. const COLOR_ATTACHMENT = 0x80; /// Image view can be used as color (with blending) and input attachment. From c1dc045b99795e74a448efffa695c33637495517 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Thu, 31 Dec 2020 10:35:32 +0100 Subject: [PATCH 13/15] Detach ImageFeature flags from Vulkan's definition --- src/backend/vulkan/src/conv.rs | 40 +++++++++++++++++++++++++++++----- src/hal/src/format.rs | 27 ++++++++++++----------- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/backend/vulkan/src/conv.rs b/src/backend/vulkan/src/conv.rs index e70fdd8636f..e08a84d12ae 100644 --- a/src/backend/vulkan/src/conv.rs +++ b/src/backend/vulkan/src/conv.rs @@ -378,12 +378,42 @@ pub fn map_query_result_flags(flags: query::ResultFlags) -> vk::QueryResultFlags } pub fn map_image_features(features: vk::FormatFeatureFlags) -> format::ImageFeature { - let bits = format::ImageFeature::from_bits_truncate(features.as_raw()); - if bits.contains(format::ImageFeature::STORAGE) { - bits | format::ImageFeature::STORAGE_READ_WRITE - } else { - bits + let mut mapped_flags = format::ImageFeature::empty(); + if features.contains(vk::FormatFeatureFlags::SAMPLED_IMAGE) { + mapped_flags |= format::ImageFeature::SAMPLED; + + if features.contains(vk::FormatFeatureFlags::SAMPLED_IMAGE_FILTER_LINEAR) { + mapped_flags |= format::ImageFeature::SAMPLED_LINEAR; + } + } + + if features.contains(vk::FormatFeatureFlags::STORAGE_IMAGE) { + mapped_flags |= format::ImageFeature::STORAGE_READ_WRITE; + + if features.contains(vk::FormatFeatureFlags::STORAGE_IMAGE_ATOMIC) { + mapped_flags |= format::ImageFeature::STORAGE_ATOMIC; + } + } + + if features.contains(vk::FormatFeatureFlags::COLOR_ATTACHMENT) { + mapped_flags |= format::ImageFeature::COLOR_ATTACHMENT; + + if features.contains(vk::FormatFeatureFlags::COLOR_ATTACHMENT_BLEND) { + mapped_flags |= format::ImageFeature::COLOR_ATTACHMENT_BLEND; + } + } + if features.contains(vk::FormatFeatureFlags::DEPTH_STENCIL_ATTACHMENT) { + mapped_flags |= format::ImageFeature::DEPTH_STENCIL_ATTACHMENT; } + + if features.contains(vk::FormatFeatureFlags::BLIT_SRC) { + mapped_flags |= format::ImageFeature::BLIT_SRC; + } + if features.contains(vk::FormatFeatureFlags::BLIT_DST) { + mapped_flags |= format::ImageFeature::BLIT_DST; + } + + mapped_flags } pub fn map_buffer_features(features: vk::FormatFeatureFlags) -> format::BufferFeature { diff --git a/src/hal/src/format.rs b/src/hal/src/format.rs index e5d0d40749d..f8e021fceae 100644 --- a/src/hal/src/format.rs +++ b/src/hal/src/format.rs @@ -148,26 +148,27 @@ bitflags!( pub struct ImageFeature: u32 { /// Image view can be sampled. const SAMPLED = 0x1; + /// Image can be sampled with a (mipmap) linear sampler or as blit source with linear sampling. + const SAMPLED_LINEAR = 0x10000 | ImageFeature::SAMPLED.bits() | ImageFeature::BLIT_SRC.bits(); + /// Image view can be used as storage image with exclusive read & write access. - const STORAGE = 0x2; - /// Image view can be used as storage image with atomics. - const STORAGE_ATOMIC = 0x4; + const STORAGE = 0x10; /// Image view can be used as storage image with simultaneous read/write access. - const STORAGE_READ_WRITE = 0x8; + const STORAGE_READ_WRITE = 0x20 | ImageFeature::STORAGE.bits(); + /// Image view can be used as storage image with atomics. + const STORAGE_ATOMIC = 0x40 | ImageFeature::STORAGE_READ_WRITE.bits(); + /// Image view can be used as color and input attachment. - const COLOR_ATTACHMENT = 0x80; + const COLOR_ATTACHMENT = 0x100; /// Image view can be used as color (with blending) and input attachment. - const COLOR_ATTACHMENT_BLEND = 0x100; + const COLOR_ATTACHMENT_BLEND = 0x200 | ImageFeature::COLOR_ATTACHMENT.bits(); /// Image view can be used as depth-stencil and input attachment. - const DEPTH_STENCIL_ATTACHMENT = 0x200; + const DEPTH_STENCIL_ATTACHMENT = 0x400; + /// Image can be used as source for blit commands. - const BLIT_SRC = 0x400; + const BLIT_SRC = 0x1000; /// Image can be used as destination for blit commands. - const BLIT_DST = 0x800; - /// Image can be sampled with a (mipmap) linear sampler or as blit source - /// with linear sampling. - /// Requires `SAMPLED` or `BLIT_SRC` flag. - const SAMPLED_LINEAR = 0x1000; + const BLIT_DST = 0x2000; } ); From c3fd228e7ff57722d5e5bc29ed51ff4f02bea0c2 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 3 Jan 2021 11:45:34 +0100 Subject: [PATCH 14/15] Metal format mapping support for STORAGE_READ_WRITE --- src/backend/metal/src/conversions.rs | 41 +++++++++++++++++++--------- src/backend/metal/src/lib.rs | 2 ++ 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/backend/metal/src/conversions.rs b/src/backend/metal/src/conversions.rs index 02be6319310..7311b6315ac 100644 --- a/src/backend/metal/src/conversions.rs +++ b/src/backend/metal/src/conversions.rs @@ -179,6 +179,15 @@ impl PrivateCapabilities { let compressed_if = color_if | If::SAMPLED_LINEAR; let depth_if = color_if | If::DEPTH_STENCIL_ATTACHMENT; + // Affected formats documented at: + // https://developer.apple.com/documentation/metal/mtlreadwritetexturetier/mtlreadwritetexturetier1?language=objc + // https://developer.apple.com/documentation/metal/mtlreadwritetexturetier/mtlreadwritetexturetier2?language=objc + let (read_write_tier1_if, read_write_tier2_if) = match self.read_write_texture_tier { + MTLReadWriteTextureTier::TierNone => (If::empty(), If::empty()), + MTLReadWriteTextureTier::Tier1 => (If::STORAGE_READ_WRITE, If::empty()), + MTLReadWriteTextureTier::Tier2 => (If::STORAGE_READ_WRITE, If::STORAGE_READ_WRITE), + }; + match self.map_format(format) { Some(A8Unorm) => Properties { optimal_tiling: compressed_if, @@ -187,6 +196,7 @@ impl PrivateCapabilities { }, Some(R8Unorm) => Properties { optimal_tiling: color_if + | read_write_tier2_if | If::SAMPLED_LINEAR | If::STORAGE | If::COLOR_ATTACHMENT @@ -221,12 +231,12 @@ impl PrivateCapabilities { ..Properties::default() }, Some(R8Uint) => Properties { - optimal_tiling: color_if | If::STORAGE | If::COLOR_ATTACHMENT, + optimal_tiling: color_if | read_write_tier2_if | If::STORAGE | If::COLOR_ATTACHMENT, buffer_features, ..Properties::default() }, Some(R8Sint) => Properties { - optimal_tiling: color_if | If::STORAGE | If::COLOR_ATTACHMENT, + optimal_tiling: color_if | read_write_tier2_if | If::STORAGE | If::COLOR_ATTACHMENT, buffer_features, ..Properties::default() }, @@ -249,17 +259,18 @@ impl PrivateCapabilities { ..Properties::default() }, Some(R16Uint) => Properties { - optimal_tiling: color_if | If::STORAGE | If::COLOR_ATTACHMENT, + optimal_tiling: color_if | read_write_tier2_if | If::STORAGE | If::COLOR_ATTACHMENT, buffer_features, ..Properties::default() }, Some(R16Sint) => Properties { - optimal_tiling: color_if | If::STORAGE | If::COLOR_ATTACHMENT, + optimal_tiling: color_if | read_write_tier2_if | If::STORAGE | If::COLOR_ATTACHMENT, buffer_features, ..Properties::default() }, Some(R16Float) => Properties { optimal_tiling: color_if + | read_write_tier2_if | If::SAMPLED_LINEAR | If::STORAGE | If::COLOR_ATTACHMENT @@ -345,7 +356,7 @@ impl PrivateCapabilities { ..Properties::default() }, Some(R32Uint) if self.format_r32_all => Properties { - optimal_tiling: color_if | If::STORAGE | If::COLOR_ATTACHMENT, + optimal_tiling: color_if | read_write_tier1_if | If::STORAGE | If::COLOR_ATTACHMENT, buffer_features, ..Properties::default() }, @@ -355,7 +366,7 @@ impl PrivateCapabilities { ..Properties::default() }, Some(R32Sint) if self.format_r32_all => Properties { - optimal_tiling: color_if | If::STORAGE | If::COLOR_ATTACHMENT, + optimal_tiling: color_if | read_write_tier1_if | If::STORAGE | If::COLOR_ATTACHMENT, buffer_features, ..Properties::default() }, @@ -379,6 +390,7 @@ impl PrivateCapabilities { }, Some(R32Float) if self.format_r32float_all => Properties { optimal_tiling: color_if + | read_write_tier1_if | If::SAMPLED_LINEAR | If::STORAGE | If::COLOR_ATTACHMENT @@ -415,6 +427,7 @@ impl PrivateCapabilities { }, Some(RGBA8Unorm) => Properties { optimal_tiling: color_if + | read_write_tier2_if | If::SAMPLED_LINEAR | If::STORAGE | If::COLOR_ATTACHMENT @@ -449,12 +462,12 @@ impl PrivateCapabilities { ..Properties::default() }, Some(RGBA8Uint) => Properties { - optimal_tiling: color_if | If::STORAGE | If::COLOR_ATTACHMENT, + optimal_tiling: color_if | read_write_tier2_if | If::STORAGE | If::COLOR_ATTACHMENT, buffer_features, ..Properties::default() }, Some(RGBA8Sint) => Properties { - optimal_tiling: color_if | If::STORAGE | If::COLOR_ATTACHMENT, + optimal_tiling: color_if | read_write_tier2_if | If::STORAGE | If::COLOR_ATTACHMENT, buffer_features, ..Properties::default() }, @@ -611,17 +624,18 @@ impl PrivateCapabilities { ..Properties::default() }, Some(RGBA16Uint) => Properties { - optimal_tiling: color_if | If::STORAGE | If::COLOR_ATTACHMENT, + optimal_tiling: color_if | read_write_tier2_if | If::STORAGE | If::COLOR_ATTACHMENT, buffer_features, ..Properties::default() }, Some(RGBA16Sint) => Properties { - optimal_tiling: color_if | If::STORAGE | If::COLOR_ATTACHMENT, + optimal_tiling: color_if | read_write_tier2_if | If::STORAGE | If::COLOR_ATTACHMENT, buffer_features, ..Properties::default() }, Some(RGBA16Float) => Properties { optimal_tiling: color_if + | read_write_tier2_if | If::SAMPLED_LINEAR | If::STORAGE | If::COLOR_ATTACHMENT @@ -635,7 +649,7 @@ impl PrivateCapabilities { ..Properties::default() }, Some(RGBA32Uint) if self.format_rgba32int_color_write => Properties { - optimal_tiling: color_if | If::COLOR_ATTACHMENT | If::STORAGE, + optimal_tiling: color_if | read_write_tier2_if | If::COLOR_ATTACHMENT | If::STORAGE, buffer_features, ..Properties::default() }, @@ -645,12 +659,13 @@ impl PrivateCapabilities { ..Properties::default() }, Some(RGBA32Sint) if self.format_rgba32int_color_write => Properties { - optimal_tiling: color_if | If::COLOR_ATTACHMENT | If::STORAGE, + optimal_tiling: color_if | read_write_tier2_if | If::COLOR_ATTACHMENT | If::STORAGE, buffer_features, ..Properties::default() }, Some(RGBA32Float) if self.format_rgba32float_all => Properties { optimal_tiling: color_if + | read_write_tier2_if | If::SAMPLED_LINEAR | If::STORAGE | If::COLOR_ATTACHMENT @@ -664,7 +679,7 @@ impl PrivateCapabilities { ..Properties::default() }, Some(RGBA32Float) if self.format_rgba32float_color_write => Properties { - optimal_tiling: color_if | If::COLOR_ATTACHMENT | If::STORAGE, + optimal_tiling: color_if | read_write_tier2_if | If::COLOR_ATTACHMENT | If::STORAGE, buffer_features, ..Properties::default() }, diff --git a/src/backend/metal/src/lib.rs b/src/backend/metal/src/lib.rs index 987ca896651..f2d28c4c79f 100644 --- a/src/backend/metal/src/lib.rs +++ b/src/backend/metal/src/lib.rs @@ -680,6 +680,7 @@ struct PrivateCapabilities { os_version: (u32, u32), msl_version: metal::MTLLanguageVersion, exposed_queues: usize, + read_write_texture_tier: metal::MTLReadWriteTextureTier, // if TRUE, we'll report `NON_FILL_POLYGON_MODE` feature without the points support expose_line_mode: bool, resource_heaps: bool, @@ -823,6 +824,7 @@ impl PrivateCapabilities { MTLLanguageVersion::V1_0 }, exposed_queues: 1, + read_write_texture_tier: device.read_write_texture_support(), expose_line_mode: true, resource_heaps: Self::supports_any(&device, RESOURCE_HEAP_SUPPORT), argument_buffers: experiments.argument_buffers From 8b902f5d7abd805b8f35f3b6b9ac567cbb634990 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sun, 3 Jan 2021 10:10:41 +0100 Subject: [PATCH 15/15] Untangeled ImageFeature bit flags --- src/backend/vulkan/src/conv.rs | 22 ++++++++++------------ src/hal/src/format.rs | 9 +++++---- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/backend/vulkan/src/conv.rs b/src/backend/vulkan/src/conv.rs index e08a84d12ae..3f808a3ecf5 100644 --- a/src/backend/vulkan/src/conv.rs +++ b/src/backend/vulkan/src/conv.rs @@ -381,26 +381,24 @@ pub fn map_image_features(features: vk::FormatFeatureFlags) -> format::ImageFeat let mut mapped_flags = format::ImageFeature::empty(); if features.contains(vk::FormatFeatureFlags::SAMPLED_IMAGE) { mapped_flags |= format::ImageFeature::SAMPLED; - - if features.contains(vk::FormatFeatureFlags::SAMPLED_IMAGE_FILTER_LINEAR) { - mapped_flags |= format::ImageFeature::SAMPLED_LINEAR; - } + } + if features.contains(vk::FormatFeatureFlags::SAMPLED_IMAGE_FILTER_LINEAR) { + mapped_flags |= format::ImageFeature::SAMPLED_LINEAR; } if features.contains(vk::FormatFeatureFlags::STORAGE_IMAGE) { + mapped_flags |= format::ImageFeature::STORAGE; mapped_flags |= format::ImageFeature::STORAGE_READ_WRITE; - - if features.contains(vk::FormatFeatureFlags::STORAGE_IMAGE_ATOMIC) { - mapped_flags |= format::ImageFeature::STORAGE_ATOMIC; - } + } + if features.contains(vk::FormatFeatureFlags::STORAGE_IMAGE_ATOMIC) { + mapped_flags |= format::ImageFeature::STORAGE_ATOMIC; } if features.contains(vk::FormatFeatureFlags::COLOR_ATTACHMENT) { mapped_flags |= format::ImageFeature::COLOR_ATTACHMENT; - - if features.contains(vk::FormatFeatureFlags::COLOR_ATTACHMENT_BLEND) { - mapped_flags |= format::ImageFeature::COLOR_ATTACHMENT_BLEND; - } + } + if features.contains(vk::FormatFeatureFlags::COLOR_ATTACHMENT_BLEND) { + mapped_flags |= format::ImageFeature::COLOR_ATTACHMENT_BLEND; } if features.contains(vk::FormatFeatureFlags::DEPTH_STENCIL_ATTACHMENT) { mapped_flags |= format::ImageFeature::DEPTH_STENCIL_ATTACHMENT; diff --git a/src/hal/src/format.rs b/src/hal/src/format.rs index f8e021fceae..7aff481b3e0 100644 --- a/src/hal/src/format.rs +++ b/src/hal/src/format.rs @@ -149,19 +149,20 @@ bitflags!( /// Image view can be sampled. const SAMPLED = 0x1; /// Image can be sampled with a (mipmap) linear sampler or as blit source with linear sampling. - const SAMPLED_LINEAR = 0x10000 | ImageFeature::SAMPLED.bits() | ImageFeature::BLIT_SRC.bits(); + /// (implies SAMPLED and BLIT_SRC support) + const SAMPLED_LINEAR = 0x2; /// Image view can be used as storage image with exclusive read & write access. const STORAGE = 0x10; /// Image view can be used as storage image with simultaneous read/write access. - const STORAGE_READ_WRITE = 0x20 | ImageFeature::STORAGE.bits(); + const STORAGE_READ_WRITE = 0x20; /// Image view can be used as storage image with atomics. - const STORAGE_ATOMIC = 0x40 | ImageFeature::STORAGE_READ_WRITE.bits(); + const STORAGE_ATOMIC = 0x40; /// Image view can be used as color and input attachment. const COLOR_ATTACHMENT = 0x100; /// Image view can be used as color (with blending) and input attachment. - const COLOR_ATTACHMENT_BLEND = 0x200 | ImageFeature::COLOR_ATTACHMENT.bits(); + const COLOR_ATTACHMENT_BLEND = 0x200; /// Image view can be used as depth-stencil and input attachment. const DEPTH_STENCIL_ATTACHMENT = 0x400;