From 6ab0c0e0967742903624a49c7320c0a89936495f Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Thu, 16 Jan 2025 17:13:01 +0100 Subject: [PATCH] No longer wrap wgpu resources in Arc --- crates/egui-wgpu/src/lib.rs | 32 +++++++++-------------------- crates/egui-wgpu/src/setup.rs | 21 ++++++++----------- crates/egui-wgpu/src/winit.rs | 2 +- crates/egui_kittest/src/builder.rs | 5 ++--- crates/egui_kittest/src/lib.rs | 1 + crates/egui_kittest/src/renderer.rs | 13 ++++++++---- 6 files changed, 31 insertions(+), 43 deletions(-) diff --git a/crates/egui-wgpu/src/lib.rs b/crates/egui-wgpu/src/lib.rs index 0d795783aa1..6b3d5881757 100644 --- a/crates/egui-wgpu/src/lib.rs +++ b/crates/egui-wgpu/src/lib.rs @@ -63,21 +63,20 @@ pub enum WgpuError { #[derive(Clone)] pub struct RenderState { /// Wgpu adapter used for rendering. - pub adapter: Arc, + pub adapter: wgpu::Adapter, /// All the available adapters. /// /// This is not available on web. /// On web, we always select WebGPU is available, then fall back to WebGL if not. - // TODO(gfx-rs/wgpu#6665): Remove layer of `Arc` here once we update to wgpu 24 #[cfg(not(target_arch = "wasm32"))] - pub available_adapters: Arc<[Arc]>, + pub available_adapters: Arc<[wgpu::Adapter]>, /// Wgpu device used for rendering, created from the adapter. - pub device: Arc, + pub device: wgpu::Device, /// Wgpu queue used for rendering, created from the adapter. - pub queue: Arc, + pub queue: wgpu::Queue, /// The target texture format used for presenting to the window. pub target_format: wgpu::TextureFormat, @@ -90,8 +89,8 @@ async fn request_adapter( instance: &wgpu::Instance, power_preference: wgpu::PowerPreference, compatible_surface: Option<&wgpu::Surface<'_>>, - _available_adapters: &[Arc], -) -> Result, WgpuError> { + _available_adapters: &[wgpu::Adapter], +) -> Result { profiling::function_scope!(); let adapter = instance @@ -149,10 +148,7 @@ async fn request_adapter( ); } - // On wasm, depending on feature flags, wgpu objects may or may not implement sync. - // It doesn't make sense to switch to Rc for that special usecase, so simply disable the lint. - #[allow(clippy::arc_with_non_send_sync)] - Ok(Arc::new(adapter)) + Ok(adapter) } impl RenderState { @@ -179,12 +175,7 @@ impl RenderState { wgpu::Backends::all() }; - instance - .enumerate_adapters(backends) - // TODO(gfx-rs/wgpu#6665): Remove layer of `Arc` here once we update to wgpu 24. - .into_iter() - .map(Arc::new) - .collect::>() + instance.enumerate_adapters(backends) }; let (adapter, device, queue) = match config.wgpu_setup.clone() { @@ -222,10 +213,7 @@ impl RenderState { .await? }; - // On wasm, depending on feature flags, wgpu objects may or may not implement sync. - // It doesn't make sense to switch to Rc for that special usecase, so simply disable the lint. - #[allow(clippy::arc_with_non_send_sync)] - (adapter, Arc::new(device), Arc::new(queue)) + (adapter, device, queue) } WgpuSetup::Existing(WgpuSetupExisting { instance: _, @@ -268,7 +256,7 @@ impl RenderState { } #[cfg(not(target_arch = "wasm32"))] -fn describe_adapters(adapters: &[Arc]) -> String { +fn describe_adapters(adapters: &[wgpu::Adapter]) -> String { if adapters.is_empty() { "(none)".to_owned() } else if adapters.len() == 1 { diff --git a/crates/egui-wgpu/src/setup.rs b/crates/egui-wgpu/src/setup.rs index 45e15ea22cc..1f70b6bb78a 100644 --- a/crates/egui-wgpu/src/setup.rs +++ b/crates/egui-wgpu/src/setup.rs @@ -45,7 +45,7 @@ impl WgpuSetup { /// /// Does *not* store the wgpu instance, so calling this repeatedly may /// create a new instance every time! - pub async fn new_instance(&self) -> Arc { + pub async fn new_instance(&self) -> wgpu::Instance { match self { Self::CreateNew(create_new) => { #[allow(unused_mut)] @@ -65,12 +65,8 @@ impl WgpuSetup { } log::debug!("Creating wgpu instance with backends {:?}", backends); - - #[allow(clippy::arc_with_non_send_sync)] - Arc::new( - wgpu::util::new_instance_with_webgpu_detection(&create_new.instance_descriptor) - .await, - ) + wgpu::util::new_instance_with_webgpu_detection(&create_new.instance_descriptor) + .await } Self::Existing(existing) => existing.instance.clone(), } @@ -93,9 +89,8 @@ impl From for WgpuSetup { /// /// This can be used for fully custom adapter selection. /// If available, `wgpu::Surface` is passed to allow checking for surface compatibility. -// TODO(gfx-rs/wgpu#6665): Remove layer of `Arc` here. pub type NativeAdapterSelectorMethod = Arc< - dyn Fn(&[Arc], Option<&wgpu::Surface<'_>>) -> Result, String> + dyn Fn(&[wgpu::Adapter], Option<&wgpu::Surface<'_>>) -> Result + Send + Sync, >; @@ -215,8 +210,8 @@ impl Default for WgpuSetupCreateNew { /// Used for [`WgpuSetup::Existing`]. #[derive(Clone)] pub struct WgpuSetupExisting { - pub instance: Arc, - pub adapter: Arc, - pub device: Arc, - pub queue: Arc, + pub instance: wgpu::Instance, + pub adapter: wgpu::Adapter, + pub device: wgpu::Device, + pub queue: wgpu::Queue, } diff --git a/crates/egui-wgpu/src/winit.rs b/crates/egui-wgpu/src/winit.rs index 86f7bd84d84..cc3a7db2c79 100644 --- a/crates/egui-wgpu/src/winit.rs +++ b/crates/egui-wgpu/src/winit.rs @@ -27,7 +27,7 @@ pub struct Painter { depth_format: Option, screen_capture_state: Option, - instance: Arc, + instance: wgpu::Instance, render_state: Option, // Per viewport/window: diff --git a/crates/egui_kittest/src/builder.rs b/crates/egui_kittest/src/builder.rs index d21558113b8..bda1fcb2ac4 100644 --- a/crates/egui_kittest/src/builder.rs +++ b/crates/egui_kittest/src/builder.rs @@ -1,5 +1,4 @@ use crate::app_kind::AppKind; -use crate::wgpu::WgpuTestRenderer; use crate::{Harness, LazyRenderer, TestRenderer}; use egui::{Pos2, Rect, Vec2}; use std::marker::PhantomData; @@ -78,13 +77,13 @@ impl HarnessBuilder { /// This sets up a [`WgpuTestRenderer`] with the default setup. #[cfg(feature = "wgpu")] pub fn wgpu(self) -> Self { - self.renderer(WgpuTestRenderer::default()) + self.renderer(crate::wgpu::WgpuTestRenderer::default()) } /// Enable wgpu rendering with the given setup. #[cfg(feature = "wgpu")] pub fn wgpu_setup(self, setup: egui_wgpu::WgpuSetup) -> Self { - self.renderer(WgpuTestRenderer::from_setup(setup)) + self.renderer(crate::wgpu::WgpuTestRenderer::from_setup(setup)) } /// Create a new Harness with the given app closure and a state. diff --git a/crates/egui_kittest/src/lib.rs b/crates/egui_kittest/src/lib.rs index fdb31372c8a..661cb92c3b2 100644 --- a/crates/egui_kittest/src/lib.rs +++ b/crates/egui_kittest/src/lib.rs @@ -402,6 +402,7 @@ impl<'a, State> Harness<'a, State> { /// /// # Errors /// Returns an error if the rendering fails. + #[cfg(any(feature = "wgpu", feature = "snapshot"))] pub fn render(&mut self) -> Result { self.renderer.render(&self.ctx, &self.output) } diff --git a/crates/egui_kittest/src/renderer.rs b/crates/egui_kittest/src/renderer.rs index cbd789a0c33..e95cfd0d1e4 100644 --- a/crates/egui_kittest/src/renderer.rs +++ b/crates/egui_kittest/src/renderer.rs @@ -1,5 +1,4 @@ -use egui::{Context, FullOutput, TexturesDelta}; -use image::RgbaImage; +use egui::TexturesDelta; pub trait TestRenderer { /// We use this to pass the glow / wgpu render state to [`eframe::Frame`]. @@ -13,7 +12,12 @@ pub trait TestRenderer { /// /// # Errors /// Returns an error if the rendering fails. - fn render(&mut self, ctx: &Context, output: &FullOutput) -> Result; + #[cfg(any(feature = "wgpu", feature = "snapshot"))] + fn render( + &mut self, + ctx: &egui::Context, + output: &egui::FullOutput, + ) -> Result; } /// A lazy renderer that initializes the renderer on the first render call. @@ -58,7 +62,8 @@ impl TestRenderer for LazyRenderer { } } - fn render(&mut self, ctx: &Context, output: &FullOutput) -> Result { + #[cfg(any(feature = "wgpu", feature = "snapshot"))] + fn render(&mut self, ctx: &Context, output: &FullOutput) -> Result { match self { Self::Uninitialized { texture_ops,