diff --git a/Cargo.lock b/Cargo.lock index f8c06aa04a..5a62bc852c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6951,6 +6951,7 @@ dependencies = [ "vello", "web-sys", "wgpu", + "wgpu-sync", ] [[package]] @@ -7017,6 +7018,13 @@ dependencies = [ "wgpu-types", ] +[[package]] +name = "wgpu-sync" +version = "0.0.0" +dependencies = [ + "wgpu", +] + [[package]] name = "wgpu-types" version = "29.0.3" diff --git a/Cargo.toml b/Cargo.toml index 59b46705cb..98ef2d9eee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ members = [ "frontend/wrapper", "libraries/dyn-any", "libraries/math-parser", + "libraries/wgpu-sync", "node-graph/libraries/graphene-hash", "node-graph/libraries/*", "node-graph/nodes/*", @@ -98,6 +99,7 @@ graphene-std = { path = "node-graph/nodes/gstd" } interpreted-executor = { path = "node-graph/interpreted-executor" } node-macro = { path = "node-graph/node-macro" } wgpu-executor = { path = "node-graph/libraries/wgpu-executor" } +wgpu-sync = { path = "libraries/wgpu-sync" } graphite-proc-macros = { path = "proc-macros" } graphite-editor = { path = "editor" } graphene-canvas-utils = { path = "node-graph/libraries/canvas-utils" } diff --git a/desktop/src/render/state.rs b/desktop/src/render/state.rs index 804809df63..6907c03466 100644 --- a/desktop/src/render/state.rs +++ b/desktop/src/render/state.rs @@ -1,13 +1,12 @@ use wgpu::PresentMode; use crate::window::Window; -use crate::wrapper::{WgpuContext, WgpuExecutor}; +use crate::wrapper::{WgpuContext, WgpuCurrentSurfaceTexture, WgpuExecutor, WgpuSurface}; #[derive(derivative::Derivative)] #[derivative(Debug)] pub(crate) struct RenderState { - surface: wgpu::Surface<'static>, - context: WgpuContext, + surface: WgpuSurface, executor: WgpuExecutor, config: wgpu::SurfaceConfiguration, render_pipeline: wgpu::RenderPipeline, @@ -162,12 +161,11 @@ impl RenderState { cache: None, }); - let wgpu_executor = WgpuExecutor::with_context(context.clone()).expect("Failed to create WgpuExecutor"); + let executor = WgpuExecutor::with_context(context).expect("Failed to create WgpuExecutor"); Self { surface, - context, - executor: wgpu_executor, + executor, config, render_pipeline, transparent_texture, @@ -193,12 +191,6 @@ impl RenderState { self.desired_width = width; self.desired_height = height; self.surface_outdated = true; - - if width > 0 && height > 0 && (self.config.width != width || self.config.height != height) { - self.config.width = width; - self.config.height = height; - self.surface.configure(&self.context.device, &self.config); - } } pub(crate) fn bind_viewport_texture(&mut self, viewport_texture: std::sync::Arc) { @@ -254,6 +246,14 @@ impl RenderState { if !self.surface_outdated { return Ok(()); } + + // Apply resize once per presented frame. + if self.desired_width > 0 && self.desired_height > 0 && (self.config.width != self.desired_width || self.config.height != self.desired_height) { + self.config.width = self.desired_width; + self.config.height = self.desired_height; + self.surface.configure(&self.executor.context().device, &self.config); + } + let ui_scale = if let Some(ui_texture) = &self.ui_texture && (self.desired_width != ui_texture.width() || self.desired_height != ui_texture.height()) { @@ -266,21 +266,19 @@ impl RenderState { self.render_overlays(scene); } - let (output, suboptimal) = match self.surface.get_current_texture() { - wgpu::CurrentSurfaceTexture::Success(t) => (t, false), - // wgpu reports the swapchain no longer matches the underlying surface; present this frame and reconfigure after present, since `Surface::configure` panics while an acquired `SurfaceTexture` is still alive - wgpu::CurrentSurfaceTexture::Suboptimal(t) => (t, true), - // Window is minimized or behind another window: skip the frame silently and try again once it becomes visible - wgpu::CurrentSurfaceTexture::Occluded => return Ok(()), - wgpu::CurrentSurfaceTexture::Lost => return Err(RenderError::SurfaceLost), - wgpu::CurrentSurfaceTexture::Outdated => return Err(RenderError::SurfaceOutdated), - wgpu::CurrentSurfaceTexture::Timeout => return Err(RenderError::SurfaceTimeout), - wgpu::CurrentSurfaceTexture::Validation => return Err(RenderError::SurfaceValidation), + let (surface_texture, suboptimal) = match self.surface.get_current_texture(&self.executor.context().queue) { + WgpuCurrentSurfaceTexture::Success(t) => (t, false), + WgpuCurrentSurfaceTexture::Suboptimal(t) => (t, true), + WgpuCurrentSurfaceTexture::Occluded => return Ok(()), + WgpuCurrentSurfaceTexture::Lost => return Err(RenderError::SurfaceLost), + WgpuCurrentSurfaceTexture::Outdated => return Err(RenderError::SurfaceOutdated), + WgpuCurrentSurfaceTexture::Timeout => return Err(RenderError::SurfaceTimeout), + WgpuCurrentSurfaceTexture::Validation => return Err(RenderError::SurfaceValidation), }; - let view = output.texture.create_view(&wgpu::TextureViewDescriptor::default()); + let view = surface_texture.texture.create_view(&wgpu::TextureViewDescriptor::default()); - let mut encoder = self.context.device.create_command_encoder(&wgpu::CommandEncoderDescriptor { label: Some("Render Encoder") }); + let mut encoder = self.executor.context().device.create_command_encoder(&wgpu::CommandEncoderDescriptor { label: Some("Render Encoder") }); { let mut render_pass = encoder.begin_render_pass(&wgpu::RenderPassDescriptor { @@ -318,12 +316,12 @@ impl RenderState { tracing::warn!("No bind group available - showing clear color only"); } } - self.context.queue.submit(std::iter::once(encoder.finish())); + surface_texture.queue.submit(std::iter::once(encoder.finish())); window.pre_present_notify(); - output.present(); + surface_texture.present(); if suboptimal { - self.surface.configure(&self.context.device, &self.config); + self.surface.configure(&self.executor.context().device, &self.config); } if ui_scale.is_some() { @@ -340,7 +338,7 @@ impl RenderState { let overlays_texture_view = self.overlays_texture.as_ref().unwrap_or(&self.transparent_texture).create_view(&wgpu::TextureViewDescriptor::default()); let ui_texture_view = self.ui_texture.as_ref().unwrap_or(&self.transparent_texture).create_view(&wgpu::TextureViewDescriptor::default()); - let bind_group = self.context.device.create_bind_group(&wgpu::BindGroupDescriptor { + let bind_group = self.executor.context().device.create_bind_group(&wgpu::BindGroupDescriptor { layout: &self.render_pipeline.get_bind_group_layout(0), entries: &[ wgpu::BindGroupEntry { diff --git a/desktop/src/window.rs b/desktop/src/window.rs index e5eff063ff..4783db16c5 100644 --- a/desktop/src/window.rs +++ b/desktop/src/window.rs @@ -1,6 +1,7 @@ use crate::consts::APP_NAME; use crate::event::AppEventScheduler; use crate::wrapper::messages::MenuItem; +use crate::wrapper::{WgpuInstance, WgpuSurface}; use std::collections::HashMap; use std::sync::Arc; use winit::cursor::{CursorIcon, CustomCursor, CustomCursorSource}; @@ -86,8 +87,8 @@ impl Window { self.winit_window.request_redraw(); } - pub(crate) fn create_surface(&self, instance: &wgpu::Instance) -> wgpu::Surface<'static> { - instance.create_surface(self.winit_window.clone()).unwrap() + pub(crate) fn create_surface(&self, instance: &WgpuInstance) -> WgpuSurface { + instance.create_surface(self.winit_window.clone()).expect("Failed to create surface") } pub(crate) fn pre_present_notify(&self) { diff --git a/desktop/wrapper/src/lib.rs b/desktop/wrapper/src/lib.rs index e49a552208..475ed93dc7 100644 --- a/desktop/wrapper/src/lib.rs +++ b/desktop/wrapper/src/lib.rs @@ -11,8 +11,11 @@ pub use graphite_editor::consts::{DOUBLE_CLICK_MILLISECONDS, FILE_EXTENSION}; pub use wgpu_executor::WgpuBackends; pub use wgpu_executor::WgpuContext; pub use wgpu_executor::WgpuContextBuilder; +pub use wgpu_executor::WgpuCurrentSurfaceTexture; pub use wgpu_executor::WgpuExecutor; pub use wgpu_executor::WgpuFeatures; +pub use wgpu_executor::WgpuInstance; +pub use wgpu_executor::WgpuSurface; mod handle_desktop_wrapper_message; mod intercept_editor_message; diff --git a/libraries/wgpu-sync/Cargo.toml b/libraries/wgpu-sync/Cargo.toml new file mode 100644 index 0000000000..d8ecfd0352 --- /dev/null +++ b/libraries/wgpu-sync/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "wgpu-sync" +description = "Helper for working with wgpu in a multi-threaded context" +version.workspace = true +edition.workspace = true +authors.workspace = true +license.workspace = true + +[dependencies] +wgpu = { workspace = true } diff --git a/libraries/wgpu-sync/src/lib.rs b/libraries/wgpu-sync/src/lib.rs new file mode 100644 index 0000000000..cce1d0f240 --- /dev/null +++ b/libraries/wgpu-sync/src/lib.rs @@ -0,0 +1,206 @@ +//! Wraps wgpu types to provide synchronization against surface configuration. +//! Everything sharing a [`Instance`] is synchronized against that instance's [`Surface`]s. +//! [`Surface::configure`] takes a write lock, and all other operations take a read lock. +//! +//! [`wgpu::Surface::configure`] recreates the swapchain and waits for the GPU to idle. +//! A concurrent `submit`, `get_current_texture`, or `present` makes that +//! wait fail (validation error, panic, or driver crash on the unsafe hal usage). +//! +//! [`Instance`] and [`Adapter`] wrapper types can be dereferenced to the underlying wgpu type. +//! Their `create_surface`/`request_adapter`/`request_device` methods shadow the wgpu ones. +//! These methods return wrapper types that synchronize against the parent [`Instance`]. +//! Be aware that using the underlying wgpu versions directly (through deref) results in unsynchronized objects. +//! +//! Guards hold their read lock for their whole lifetime. +//! While holding a [`SurfaceTextureGuard`] reuse its [`QueueGuard`] via [`SurfaceTextureGuard::queue`] to avoid deadlock. + +use std::ops::Deref; +use std::sync::{Arc, PoisonError, RwLock, RwLockReadGuard, RwLockWriteGuard}; + +#[derive(Clone, Debug)] +struct Lock(Arc>); + +impl Lock { + fn new() -> Self { + Self(Arc::new(RwLock::new(()))) + } + + fn read(&self) -> RwLockReadGuard<'_, ()> { + self.0.read().unwrap_or_else(PoisonError::into_inner) + } + + fn write(&self) -> RwLockWriteGuard<'_, ()> { + self.0.write().unwrap_or_else(PoisonError::into_inner) + } +} + +#[derive(Clone, Debug)] +pub struct Instance { + raw: wgpu::Instance, + lock: Lock, +} + +impl Instance { + pub fn new(raw: wgpu::Instance) -> Self { + Self { raw, lock: Lock::new() } + } + + pub fn create_surface(&self, target: impl Into>) -> Result { + Ok(Surface { + raw: self.raw.create_surface(target)?, + lock: self.lock.clone(), + }) + } + + pub async fn request_adapter(&self, options: &wgpu::RequestAdapterOptions<'_, '_>) -> Result { + Ok(Adapter { + raw: self.raw.request_adapter(options).await?, + lock: self.lock.clone(), + }) + } + + pub async fn enumerate_adapters(&self, backends: wgpu::Backends) -> Vec { + self.raw + .enumerate_adapters(backends) + .await + .into_iter() + .map(|adapter| Adapter { + raw: adapter, + lock: self.lock.clone(), + }) + .collect() + } +} + +impl Deref for Instance { + type Target = wgpu::Instance; + fn deref(&self) -> &wgpu::Instance { + &self.raw + } +} + +#[derive(Clone, Debug)] +pub struct Adapter { + raw: wgpu::Adapter, + lock: Lock, +} + +impl Adapter { + pub async fn request_device(&self, desc: &wgpu::DeviceDescriptor<'_>) -> Result<(wgpu::Device, Queue), wgpu::RequestDeviceError> { + let (device, queue) = self.raw.request_device(desc).await?; + Ok((device, Queue { raw: queue, lock: self.lock.clone() })) + } +} + +impl Deref for Adapter { + type Target = wgpu::Adapter; + fn deref(&self) -> &wgpu::Adapter { + &self.raw + } +} + +#[derive(Clone, Debug)] +pub struct Queue { + raw: wgpu::Queue, + lock: Lock, +} + +impl Queue { + pub fn submit>(&self, command_buffers: I) -> wgpu::SubmissionIndex { + self.lock().submit(command_buffers) + } + + pub fn lock(&self) -> QueueGuard<'_> { + QueueGuard { + raw: &self.raw, + _guard: self.lock.read(), + } + } + + pub fn write_buffer(&self, buffer: &wgpu::Buffer, offset: wgpu::BufferAddress, data: &[u8]) { + self.lock().write_buffer(buffer, offset, data); + } + + pub fn write_texture(&self, texture: wgpu::TexelCopyTextureInfo<'_>, data: &[u8], data_layout: wgpu::TexelCopyBufferLayout, size: wgpu::Extent3d) { + self.lock().write_texture(texture, data, data_layout, size); + } +} + +pub struct QueueGuard<'a> { + raw: &'a wgpu::Queue, + _guard: RwLockReadGuard<'a, ()>, +} + +impl Deref for QueueGuard<'_> { + type Target = wgpu::Queue; + fn deref(&self) -> &wgpu::Queue { + self.raw + } +} + +#[derive(Debug)] +pub struct Surface { + raw: wgpu::Surface<'static>, + lock: Lock, +} + +impl Surface { + pub fn configure(&self, device: &wgpu::Device, config: &wgpu::SurfaceConfiguration) { + let _guard = self.lock.write(); + self.raw.configure(device, config); + } + + pub fn get_current_texture<'a>(&self, queue: &'a Queue) -> CurrentSurfaceTexture<'a> { + debug_assert!(Arc::ptr_eq(&self.lock.0, &queue.lock.0), "queue must come from the same `Instance` as this surface"); + let guard = queue.lock(); + let raw = self.raw.get_current_texture(); + match raw { + wgpu::CurrentSurfaceTexture::Success(raw) => CurrentSurfaceTexture::Success(SurfaceTextureGuard { raw, queue: guard }), + wgpu::CurrentSurfaceTexture::Suboptimal(raw) => CurrentSurfaceTexture::Suboptimal(SurfaceTextureGuard { raw, queue: guard }), + wgpu::CurrentSurfaceTexture::Occluded => CurrentSurfaceTexture::Occluded, + wgpu::CurrentSurfaceTexture::Lost => CurrentSurfaceTexture::Lost, + wgpu::CurrentSurfaceTexture::Outdated => CurrentSurfaceTexture::Outdated, + wgpu::CurrentSurfaceTexture::Timeout => CurrentSurfaceTexture::Timeout, + wgpu::CurrentSurfaceTexture::Validation => CurrentSurfaceTexture::Validation, + } + } + + pub fn get_capabilities(&self, adapter: &wgpu::Adapter) -> wgpu::SurfaceCapabilities { + self.raw.get_capabilities(adapter) + } +} + +#[derive(Debug)] +pub enum CurrentSurfaceTexture<'a> { + Success(SurfaceTextureGuard<'a>), + Suboptimal(SurfaceTextureGuard<'a>), + Occluded, + Lost, + Outdated, + Timeout, + Validation, +} + +pub struct SurfaceTextureGuard<'a> { + raw: wgpu::SurfaceTexture, + pub queue: QueueGuard<'a>, +} + +impl SurfaceTextureGuard<'_> { + pub fn present(self) { + self.raw.present(); + } +} + +impl Deref for SurfaceTextureGuard<'_> { + type Target = wgpu::SurfaceTexture; + fn deref(&self) -> &Self::Target { + &self.raw + } +} + +impl std::fmt::Debug for SurfaceTextureGuard<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SurfaceTexture").field("raw", &self.raw).finish() + } +} diff --git a/node-graph/libraries/canvas-utils/src/wasm.rs b/node-graph/libraries/canvas-utils/src/wasm.rs index 4b4bd305ff..068144b8be 100644 --- a/node-graph/libraries/canvas-utils/src/wasm.rs +++ b/node-graph/libraries/canvas-utils/src/wasm.rs @@ -7,7 +7,7 @@ use web_sys::js_sys::{Object, Reflect}; use web_sys::wasm_bindgen::{JsCast, JsValue}; use web_sys::{CanvasRenderingContext2d, HtmlCanvasElement, window}; #[cfg(feature = "wgpu")] -use wgpu_executor::WgpuExecutor; +use wgpu_executor::{WgpuCurrentSurfaceTexture, WgpuExecutor, WgpuSurface}; const CANVASES_OBJECT_KEY: &str = "imageCanvases"; @@ -52,13 +52,13 @@ impl Canvas for CanvasHandle { } #[cfg(feature = "wgpu")] -pub struct CanvasSurfaceHandle(CanvasHandle, Option>>); +pub struct CanvasSurfaceHandle(CanvasHandle, Option>); #[cfg(feature = "wgpu")] impl CanvasSurfaceHandle { pub fn new() -> Self { Self(CanvasHandle::new(), None) } - fn surface(&mut self, executor: &WgpuExecutor) -> &wgpu::Surface<'_> { + fn surface(&mut self, executor: &WgpuExecutor) -> &WgpuSurface { if self.1.is_none() { let canvas = self.0.get().canvas.clone(); let surface = executor @@ -115,9 +115,9 @@ impl CanvasSurface for CanvasSurfaceHandle { }, ); - let surface_texture = match surface.get_current_texture() { - wgpu::CurrentSurfaceTexture::Success(t) | wgpu::CurrentSurfaceTexture::Suboptimal(t) => t, - other => panic!("Failed to get surface texture: {other:?}"), + let surface_texture = match surface.get_current_texture(&context.queue) { + WgpuCurrentSurfaceTexture::Success(t) | WgpuCurrentSurfaceTexture::Suboptimal(t) => t, + _ => panic!("Failed to get surface texture"), }; encoder.copy_texture_to_texture( @@ -136,7 +136,7 @@ impl CanvasSurface for CanvasSurfaceHandle { source_texture.size(), ); - context.queue.submit([encoder.finish()]); + surface_texture.queue.submit([encoder.finish()]); surface_texture.present(); } } diff --git a/node-graph/libraries/wgpu-executor/Cargo.toml b/node-graph/libraries/wgpu-executor/Cargo.toml index 8565d8682d..9c4236f043 100644 --- a/node-graph/libraries/wgpu-executor/Cargo.toml +++ b/node-graph/libraries/wgpu-executor/Cargo.toml @@ -18,6 +18,7 @@ node-macro = { workspace = true } glam = { workspace = true } anyhow = { workspace = true } wgpu = { workspace = true } +wgpu-sync = { workspace = true } futures = { workspace = true } web-sys = { workspace = true } vello = { workspace = true } diff --git a/node-graph/libraries/wgpu-executor/src/context.rs b/node-graph/libraries/wgpu-executor/src/context.rs index 7006d7e78a..cb83597fef 100644 --- a/node-graph/libraries/wgpu-executor/src/context.rs +++ b/node-graph/libraries/wgpu-executor/src/context.rs @@ -1,4 +1,5 @@ -use wgpu::{Adapter, Backends, Device, Features, Instance, Queue}; +use wgpu::{Backends, Device, Features}; +use wgpu_sync::{Adapter, Instance, Queue}; #[derive(Debug, Clone)] pub struct Context { @@ -77,10 +78,10 @@ impl ContextBuilder { } impl ContextBuilder { fn build_instance(&self) -> Instance { - Instance::new(wgpu::InstanceDescriptor { + Instance::new(wgpu::Instance::new(wgpu::InstanceDescriptor { backends: self.backends, ..wgpu::InstanceDescriptor::new_without_display_handle() - }) + })) } #[cfg(target_family = "wasm")] async fn request_adapter(&self, instance: &Instance) -> Option { diff --git a/node-graph/libraries/wgpu-executor/src/lib.rs b/node-graph/libraries/wgpu-executor/src/lib.rs index 22b6e95ebd..661236a238 100644 --- a/node-graph/libraries/wgpu-executor/src/lib.rs +++ b/node-graph/libraries/wgpu-executor/src/lib.rs @@ -4,8 +4,6 @@ pub mod shader_runtime; mod texture_cache; pub mod texture_conversion; -use std::sync::Arc; - use crate::shader_runtime::ShaderRuntime; use crate::texture_cache::TextureCache; use anyhow::Result; @@ -14,6 +12,7 @@ use core_types::color::SRGBA8; use futures::lock::Mutex; use glam::UVec2; use graphene_application_io::{ApplicationIo, EditorApi}; +use std::sync::Arc; use vello::{AaConfig, AaSupport, RenderParams, Renderer, RendererOptions, Scene}; use wgpu::{Origin3d, TextureAspect}; @@ -25,6 +24,10 @@ pub use pipeline::PipelineCache as WgpuPipelineCache; pub use rendering::RenderContext; pub use wgpu::Backends as WgpuBackends; pub use wgpu::Features as WgpuFeatures; +pub use wgpu_sync::CurrentSurfaceTexture as WgpuCurrentSurfaceTexture; +pub use wgpu_sync::Instance as WgpuInstance; +pub use wgpu_sync::Queue as WgpuQueue; +pub use wgpu_sync::Surface as WgpuSurface; const TEXTURE_CACHE_SIZE: u64 = 256 * 1024 * 1024; // 256 MiB @@ -88,7 +91,11 @@ impl WgpuExecutor { }; renderer.override_image(&image_brush.image, Some(texture_view)); } - renderer.render_to_texture(&self.context().device, &self.context().queue, scene, &texture_view, &render_params)?; + + { + let queue = self.context().queue.lock(); + renderer.render_to_texture(&self.context().device, &queue, scene, &texture_view, &render_params)?; + } for (image_brush, _) in context.resource_overrides.iter() { renderer.override_image(&image_brush.image, None); } diff --git a/node-graph/libraries/wgpu-executor/src/texture_conversion.rs b/node-graph/libraries/wgpu-executor/src/texture_conversion.rs index afecb7a62d..ed9107cffc 100644 --- a/node-graph/libraries/wgpu-executor/src/texture_conversion.rs +++ b/node-graph/libraries/wgpu-executor/src/texture_conversion.rs @@ -150,12 +150,12 @@ impl<'i> Convert>, &'i WgpuExecutor> for List> { impl<'i> Convert>, &'i WgpuExecutor> for List> { async fn convert(self, _: Footprint, executor: &'i WgpuExecutor) -> List> { let device = &executor.context().device; - let queue = &executor.context().queue; + let queue = executor.context().queue.lock(); let list = self .into_iter() .map(|row| { let (image, attributes) = row.into_parts(); - let texture = upload_to_texture(device, queue, &image); + let texture = upload_to_texture(device, &queue, &image); Item::from_parts(Raster::new_gpu(texture), attributes) }) @@ -170,8 +170,8 @@ impl<'i> Convert>, &'i WgpuExecutor> for List> { impl<'i> Convert, &'i WgpuExecutor> for Raster { async fn convert(self, _: Footprint, executor: &'i WgpuExecutor) -> Raster { let device = &executor.context().device; - let queue = &executor.context().queue; - let texture = upload_to_texture(device, queue, &self); + let queue = executor.context().queue.lock(); + let texture = upload_to_texture(device, &queue, &self); queue.submit([]); Raster::new_gpu(texture)