From 4ac5916ee8ec23c87a6308e11e52a1bd109eb7ed Mon Sep 17 00:00:00 2001 From: lucasmerlin Date: Mon, 23 Mar 2026 15:34:39 +0100 Subject: [PATCH] Address review comments --- .github/workflows/rust.yml | 2 +- Cargo.toml | 2 +- crates/eframe/src/web/web_painter_wgpu.rs | 56 ++++-------------- crates/egui-wgpu/src/lib.rs | 70 ++++------------------- crates/egui-wgpu/src/setup.rs | 7 +-- crates/egui-wgpu/src/winit.rs | 47 +++------------ scripts/setup_web.sh | 4 +- 7 files changed, 35 insertions(+), 153 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 35f6f3b7c..2122e5b99 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -94,7 +94,7 @@ jobs: - name: wasm-bindgen uses: jetli/wasm-bindgen-action@v0.1.0 with: - version: "0.2.114" # Keep wasm-bindgen version in sync in: setup_web.sh, Cargo.toml, Cargo.lock, rust.yml + version: "0.2.108" # Keep wasm-bindgen version in sync in: setup_web.sh, Cargo.toml, Cargo.lock, rust.yml - run: ./scripts/wasm_bindgen_check.sh --skip-setup diff --git a/Cargo.toml b/Cargo.toml index 3dc17d98f..8471edaa8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -138,7 +138,7 @@ type-map = "0.5.1" unicode_names2 = { version = "2.0.0", default-features = false } unicode-segmentation = "1.12.0" vello_cpu = { version = "0.0.6", default-features = false, features = ["std", "u8_pipeline", "f32_pipeline"] } -wasm-bindgen = "0.2.114" # Keep wasm-bindgen version in sync in: setup_web.sh, Cargo.toml, Cargo.lock, rust.yml +wasm-bindgen = "0.2.108" # Keep wasm-bindgen version in sync in: setup_web.sh, Cargo.toml, Cargo.lock, rust.yml wasm-bindgen-futures = "0.4.0" wayland-cursor = { version = "0.31.11", default-features = false } web-sys = "0.3.77" diff --git a/crates/eframe/src/web/web_painter_wgpu.rs b/crates/eframe/src/web/web_painter_wgpu.rs index b82b33bf7..fb808caab 100644 --- a/crates/eframe/src/web/web_painter_wgpu.rs +++ b/crates/eframe/src/web/web_painter_wgpu.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use egui::{Event, UserData, ViewportId}; use egui_wgpu::{ - RenderState, SurfaceErrorAction, SurfaceStatus, + RenderState, SurfaceErrorAction, capture::{CaptureReceiver, CaptureSender, CaptureState, capture_channel}, }; use wasm_bindgen::JsValue; @@ -15,7 +15,7 @@ pub(crate) struct WebPainterWgpu { surface: wgpu::Surface<'static>, surface_configuration: wgpu::SurfaceConfiguration, render_state: Option, - on_surface_error: Arc SurfaceErrorAction>, + on_surface_status: Arc SurfaceErrorAction>, depth_stencil_format: Option, depth_texture_view: Option, screen_capture_state: Option, @@ -105,7 +105,7 @@ impl WebPainterWgpu { surface_configuration, depth_stencil_format, depth_texture_view: None, - on_surface_error: Arc::clone(&options.wgpu_options.on_surface_error) as _, + on_surface_status: Arc::clone(&options.wgpu_options.on_surface_status) as _, screen_capture_state: None, capture_tx, capture_rx, @@ -196,50 +196,14 @@ impl WebPainter for WebPainterWgpu { } let output_frame = match self.surface.get_current_texture() { - wgpu::CurrentSurfaceTexture::Success(frame) - | wgpu::CurrentSurfaceTexture::Suboptimal(frame) => frame, - wgpu::CurrentSurfaceTexture::Timeout => { - match (*self.on_surface_error)(SurfaceStatus::Timeout) { - SurfaceErrorAction::RecreateSurface => { - self.surface - .configure(&render_state.device, &self.surface_configuration); - } - SurfaceErrorAction::SkipFrame => {} - } - return Ok(()); + wgpu::CurrentSurfaceTexture::Success(frame) => frame, + wgpu::CurrentSurfaceTexture::Suboptimal(frame) => { + self.surface + .configure(&render_state.device, &self.surface_configuration); + frame } - wgpu::CurrentSurfaceTexture::Outdated => { - match (*self.on_surface_error)(SurfaceStatus::Outdated) { - SurfaceErrorAction::RecreateSurface => { - self.surface - .configure(&render_state.device, &self.surface_configuration); - } - SurfaceErrorAction::SkipFrame => {} - } - return Ok(()); - } - wgpu::CurrentSurfaceTexture::Lost => { - match (*self.on_surface_error)(SurfaceStatus::Lost) { - SurfaceErrorAction::RecreateSurface => { - self.surface - .configure(&render_state.device, &self.surface_configuration); - } - SurfaceErrorAction::SkipFrame => {} - } - return Ok(()); - } - wgpu::CurrentSurfaceTexture::Occluded => { - match (*self.on_surface_error)(SurfaceStatus::Occluded) { - SurfaceErrorAction::RecreateSurface => { - self.surface - .configure(&render_state.device, &self.surface_configuration); - } - SurfaceErrorAction::SkipFrame => {} - } - return Ok(()); - } - wgpu::CurrentSurfaceTexture::Validation => { - match (*self.on_surface_error)(SurfaceStatus::Validation) { + ref other => { + match (*self.on_surface_status)(other) { SurfaceErrorAction::RecreateSurface => { self.surface .configure(&render_state.device, &self.surface_configuration); diff --git a/crates/egui-wgpu/src/lib.rs b/crates/egui-wgpu/src/lib.rs index 4f2840024..2992d3583 100644 --- a/crates/egui-wgpu/src/lib.rs +++ b/crates/egui-wgpu/src/lib.rs @@ -276,58 +276,7 @@ fn describe_adapters(adapters: &[wgpu::Adapter]) -> String { } } -/// Describes a surface error when acquiring a texture for rendering. -/// -/// These correspond to the error variants of [`wgpu::CurrentSurfaceTexture`] — everything -/// except `Success` and `Suboptimal`, which contain usable frames. This enum is passed to the -/// [`WgpuConfiguration::on_surface_error`] callback so you can decide how to respond. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum SurfaceStatus { - /// Timed out waiting for the next frame. - /// - /// This is usually transient. Skip the current frame and try again next frame. - Timeout, - - /// The surface configuration no longer matches the underlying display surface. - /// - /// The surface should be reconfigured (egui will do this automatically if you return - /// [`SurfaceErrorAction::RecreateSurface`]). This commonly happens after a window resize - /// or display settings change. - Outdated, - - /// The surface has been lost and must be recreated from scratch. - /// - /// This is more severe than [`Self::Outdated`] — the entire surface is invalid. Return - /// [`SurfaceErrorAction::RecreateSurface`] to recover. - Lost, - - /// The window is not visible (e.g. minimized or fully behind another window). - /// - /// Skip the current frame. There is nothing to render to. - Occluded, - - /// A GPU validation error occurred. - /// - /// This should not be reachable under normal circumstances, unless - /// you wrap the [`winit::Painter::paint_and_update_textures`] in a wgpu - /// error scope, and there is a validation error in the - /// [`wgpu::Surface::get_current_texture`] call. - Validation, -} - -impl std::fmt::Display for SurfaceStatus { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Timeout => write!(f, "Surface timed out"), - Self::Outdated => write!(f, "Surface outdated"), - Self::Lost => write!(f, "Surface lost"), - Self::Occluded => write!(f, "Surface occluded"), - Self::Validation => write!(f, "Surface validation error"), - } - } -} - -/// Specifies which action should be taken as consequence of a [`SurfaceStatus`] +/// Specifies which action should be taken as consequence of a surface error. pub enum SurfaceErrorAction { /// Do nothing and skip the current frame. SkipFrame, @@ -354,8 +303,13 @@ pub struct WgpuConfiguration { /// How to create the wgpu adapter & device pub wgpu_setup: WgpuSetup, - /// Callback for surface errors. - pub on_surface_error: Arc SurfaceErrorAction + Send + Sync>, + /// Callback for surface status changes. + /// + /// Called with the [`wgpu::CurrentSurfaceTexture`] result whenever acquiring a frame + /// does not return [`wgpu::CurrentSurfaceTexture::Success`]. For + /// [`wgpu::CurrentSurfaceTexture::Suboptimal`], egui automatically reconfigures the + /// surface and uses the frame — the callback is not invoked in that case. + pub on_surface_status: Arc SurfaceErrorAction + Send + Sync>, } #[test] @@ -370,7 +324,7 @@ impl std::fmt::Debug for WgpuConfiguration { present_mode, desired_maximum_frame_latency, wgpu_setup, - on_surface_error: _, + on_surface_status: _, } = self; f.debug_struct("WgpuConfiguration") .field("present_mode", &present_mode) @@ -391,13 +345,13 @@ impl Default for WgpuConfiguration { // No display handle available at this point — callers should replace this with // `WgpuSetup::from_display_handle(...)` before creating the instance if one is available. wgpu_setup: WgpuSetup::without_display_handle(), - on_surface_error: Arc::new(|err| { - if err == SurfaceStatus::Outdated { + on_surface_status: Arc::new(|status| { + if matches!(status, wgpu::CurrentSurfaceTexture::Outdated) { // This error occurs when the app is minimized on Windows. // Silently return here to prevent spamming the console with: // "The underlying surface has changed, and therefore the swap chain must be updated" } else { - log::warn!("Dropped frame with error: {err}"); + log::warn!("Dropped frame with error: {status:?}"); } SurfaceErrorAction::SkipFrame }), diff --git a/crates/egui-wgpu/src/setup.rs b/crates/egui-wgpu/src/setup.rs index 749d3cf84..19e385ea2 100644 --- a/crates/egui-wgpu/src/setup.rs +++ b/crates/egui-wgpu/src/setup.rs @@ -132,16 +132,13 @@ impl WgpuSetup { log::debug!("Creating wgpu instance with backends {backends:?}"); let desc = &create_new.instance_descriptor; - let mut descriptor = wgpu::InstanceDescriptor { + let descriptor = wgpu::InstanceDescriptor { backends: desc.backends, flags: desc.flags, backend_options: desc.backend_options.clone(), memory_budget_thresholds: desc.memory_budget_thresholds, - display: None, + display: create_new.display_handle.as_ref().map(|handle| handle.clone_for_wgpu()), }; - if let Some(handle) = &create_new.display_handle { - descriptor.display = Some(handle.clone_for_wgpu()); - } wgpu::util::new_instance_with_webgpu_detection(descriptor).await } Self::Existing(existing) => existing.instance.clone(), diff --git a/crates/egui-wgpu/src/winit.rs b/crates/egui-wgpu/src/winit.rs index f9ef19cc4..aa3eecd7c 100644 --- a/crates/egui-wgpu/src/winit.rs +++ b/crates/egui-wgpu/src/winit.rs @@ -3,7 +3,7 @@ #![expect(clippy::unwrap_used)] // TODO(emilk): avoid unwraps #![expect(unsafe_code)] -use crate::{RenderState, SurfaceErrorAction, SurfaceStatus, WgpuConfiguration, renderer}; +use crate::{RenderState, SurfaceErrorAction, WgpuConfiguration, renderer}; use crate::{ RendererOptions, capture::{CaptureReceiver, CaptureSender, CaptureState, capture_channel}, @@ -501,46 +501,13 @@ impl Painter { }; let output_frame = match output_frame { - wgpu::CurrentSurfaceTexture::Success(frame) - | wgpu::CurrentSurfaceTexture::Suboptimal(frame) => frame, - wgpu::CurrentSurfaceTexture::Timeout => { - match (*self.configuration.on_surface_error)(SurfaceStatus::Timeout) { - SurfaceErrorAction::RecreateSurface => { - Self::configure_surface(surface_state, render_state, &self.configuration); - } - SurfaceErrorAction::SkipFrame => {} - } - return vsync_sec; + wgpu::CurrentSurfaceTexture::Success(frame) => frame, + wgpu::CurrentSurfaceTexture::Suboptimal(frame) => { + Self::configure_surface(surface_state, render_state, &self.configuration); + frame } - wgpu::CurrentSurfaceTexture::Outdated => { - match (*self.configuration.on_surface_error)(SurfaceStatus::Outdated) { - SurfaceErrorAction::RecreateSurface => { - Self::configure_surface(surface_state, render_state, &self.configuration); - } - SurfaceErrorAction::SkipFrame => {} - } - return vsync_sec; - } - wgpu::CurrentSurfaceTexture::Lost => { - match (*self.configuration.on_surface_error)(SurfaceStatus::Lost) { - SurfaceErrorAction::RecreateSurface => { - Self::configure_surface(surface_state, render_state, &self.configuration); - } - SurfaceErrorAction::SkipFrame => {} - } - return vsync_sec; - } - wgpu::CurrentSurfaceTexture::Occluded => { - match (*self.configuration.on_surface_error)(SurfaceStatus::Occluded) { - SurfaceErrorAction::RecreateSurface => { - Self::configure_surface(surface_state, render_state, &self.configuration); - } - SurfaceErrorAction::SkipFrame => {} - } - return vsync_sec; - } - wgpu::CurrentSurfaceTexture::Validation => { - match (*self.configuration.on_surface_error)(SurfaceStatus::Validation) { + ref other => { + match (*self.configuration.on_surface_status)(other) { SurfaceErrorAction::RecreateSurface => { Self::configure_surface(surface_state, render_state, &self.configuration); } diff --git a/scripts/setup_web.sh b/scripts/setup_web.sh index 67ff80e38..7945484b5 100755 --- a/scripts/setup_web.sh +++ b/scripts/setup_web.sh @@ -10,6 +10,6 @@ rustup target add wasm32-unknown-unknown # For generating JS bindings: # Keep wasm-bindgen version in sync in: setup_web.sh, Cargo.toml, Cargo.lock, rust.yml -if ! cargo install --list | grep -q 'wasm-bindgen-cli v0.2.114'; then - cargo install --force --quiet wasm-bindgen-cli --version 0.2.114 +if ! cargo install --list | grep -q 'wasm-bindgen-cli v0.2.108'; then + cargo install --force --quiet wasm-bindgen-cli --version 0.2.108 fi