Skip to content

Commit

Permalink
No longer wrap wgpu resources in Arc
Browse files Browse the repository at this point in the history
  • Loading branch information
Wumpf committed Jan 16, 2025
1 parent a5d7cf5 commit 6ab0c0e
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 43 deletions.
32 changes: 10 additions & 22 deletions crates/egui-wgpu/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,20 @@ pub enum WgpuError {
#[derive(Clone)]
pub struct RenderState {
/// Wgpu adapter used for rendering.
pub adapter: Arc<wgpu::Adapter>,
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<wgpu::Adapter>]>,
pub available_adapters: Arc<[wgpu::Adapter]>,

/// Wgpu device used for rendering, created from the adapter.
pub device: Arc<wgpu::Device>,
pub device: wgpu::Device,

/// Wgpu queue used for rendering, created from the adapter.
pub queue: Arc<wgpu::Queue>,
pub queue: wgpu::Queue,

/// The target texture format used for presenting to the window.
pub target_format: wgpu::TextureFormat,
Expand All @@ -90,8 +89,8 @@ async fn request_adapter(
instance: &wgpu::Instance,
power_preference: wgpu::PowerPreference,
compatible_surface: Option<&wgpu::Surface<'_>>,
_available_adapters: &[Arc<wgpu::Adapter>],
) -> Result<Arc<wgpu::Adapter>, WgpuError> {
_available_adapters: &[wgpu::Adapter],
) -> Result<wgpu::Adapter, WgpuError> {
profiling::function_scope!();

let adapter = instance
Expand Down Expand Up @@ -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 {
Expand All @@ -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::<Vec<_>>()
instance.enumerate_adapters(backends)
};

let (adapter, device, queue) = match config.wgpu_setup.clone() {
Expand Down Expand Up @@ -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: _,
Expand Down Expand Up @@ -268,7 +256,7 @@ impl RenderState {
}

#[cfg(not(target_arch = "wasm32"))]
fn describe_adapters(adapters: &[Arc<wgpu::Adapter>]) -> String {
fn describe_adapters(adapters: &[wgpu::Adapter]) -> String {
if adapters.is_empty() {
"(none)".to_owned()
} else if adapters.len() == 1 {
Expand Down
21 changes: 8 additions & 13 deletions crates/egui-wgpu/src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<wgpu::Instance> {
pub async fn new_instance(&self) -> wgpu::Instance {
match self {
Self::CreateNew(create_new) => {
#[allow(unused_mut)]
Expand All @@ -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(),
}
Expand All @@ -93,9 +89,8 @@ impl From<WgpuSetupExisting> 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<wgpu::Adapter>], Option<&wgpu::Surface<'_>>) -> Result<Arc<wgpu::Adapter>, String>
dyn Fn(&[wgpu::Adapter], Option<&wgpu::Surface<'_>>) -> Result<wgpu::Adapter, String>
+ Send
+ Sync,
>;
Expand Down Expand Up @@ -215,8 +210,8 @@ impl Default for WgpuSetupCreateNew {
/// Used for [`WgpuSetup::Existing`].
#[derive(Clone)]
pub struct WgpuSetupExisting {
pub instance: Arc<wgpu::Instance>,
pub adapter: Arc<wgpu::Adapter>,
pub device: Arc<wgpu::Device>,
pub queue: Arc<wgpu::Queue>,
pub instance: wgpu::Instance,
pub adapter: wgpu::Adapter,
pub device: wgpu::Device,
pub queue: wgpu::Queue,
}
2 changes: 1 addition & 1 deletion crates/egui-wgpu/src/winit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub struct Painter {
depth_format: Option<wgpu::TextureFormat>,
screen_capture_state: Option<CaptureState>,

instance: Arc<wgpu::Instance>,
instance: wgpu::Instance,
render_state: Option<RenderState>,

// Per viewport/window:
Expand Down
5 changes: 2 additions & 3 deletions crates/egui_kittest/src/builder.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -78,13 +77,13 @@ impl<State> HarnessBuilder<State> {
/// 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.
Expand Down
1 change: 1 addition & 0 deletions crates/egui_kittest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<image::RgbaImage, String> {
self.renderer.render(&self.ctx, &self.output)
}
Expand Down
13 changes: 9 additions & 4 deletions crates/egui_kittest/src/renderer.rs
Original file line number Diff line number Diff line change
@@ -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`].
Expand All @@ -13,7 +12,12 @@ pub trait TestRenderer {
///
/// # Errors
/// Returns an error if the rendering fails.
fn render(&mut self, ctx: &Context, output: &FullOutput) -> Result<RgbaImage, String>;
#[cfg(any(feature = "wgpu", feature = "snapshot"))]
fn render(
&mut self,
ctx: &egui::Context,
output: &egui::FullOutput,
) -> Result<image::RgbaImage, String>;
}

/// A lazy renderer that initializes the renderer on the first render call.
Expand Down Expand Up @@ -58,7 +62,8 @@ impl TestRenderer for LazyRenderer {
}
}

fn render(&mut self, ctx: &Context, output: &FullOutput) -> Result<RgbaImage, String> {
#[cfg(any(feature = "wgpu", feature = "snapshot"))]
fn render(&mut self, ctx: &Context, output: &FullOutput) -> Result<image::RgbaImage, String> {
match self {
Self::Uninitialized {
texture_ops,
Expand Down

0 comments on commit 6ab0c0e

Please sign in to comment.