Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wgpu resources are no longer wrapped in Arc (since they are now Clone) #5612

Merged
merged 3 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 11 additions & 23 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: Vec<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 @@ -258,7 +246,7 @@ impl RenderState {
Ok(Self {
adapter,
#[cfg(not(target_arch = "wasm32"))]
available_adapters: available_adapters.into(),
available_adapters,
device,
queue,
target_format,
Expand All @@ -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
7 changes: 3 additions & 4 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 @@ -75,16 +74,16 @@ impl<State> HarnessBuilder<State> {

/// Enable wgpu rendering with a default setup suitable for testing.
///
/// This sets up a [`WgpuTestRenderer`] with the default setup.
/// This sets up a [`crate::wgpu::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"))]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bacon was unhappy before. understandable!

pub fn render(&mut self) -> Result<image::RgbaImage, String> {
self.renderer.render(&self.ctx, &self.output)
}
Expand Down
17 changes: 13 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,12 @@ 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: &egui::Context,
output: &egui::FullOutput,
) -> Result<image::RgbaImage, String> {
match self {
Self::Uninitialized {
texture_ops,
Expand Down
Loading