Skip to content
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
38 changes: 29 additions & 9 deletions crates/eframe/src/native/wgpu_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ pub struct SharedState {
painter: egui_wgpu::winit::Painter,
viewport_from_window: HashMap<WindowId, ViewportId>,
focused_viewport: Option<ViewportId>,
resized_viewport: Option<ViewportId>,
}

pub type Viewports = egui::OrderedViewportIdMap<Viewport>;
Expand Down Expand Up @@ -302,6 +303,7 @@ impl<'app> WgpuWinitApp<'app> {
viewports,
painter,
focused_viewport: Some(ViewportId::ROOT),
resized_viewport: None,
}));

{
Expand Down Expand Up @@ -763,20 +765,34 @@ impl WgpuWinitRunning<'_> {
let viewport_id = shared.viewport_from_window.get(&window_id).copied();

// On Windows, if a window is resized by the user, it should repaint synchronously, inside the
// event handler.
//
// If this is not done, the compositor will assume that the window does not want to redraw,
// and continue ahead.
// event handler. If this is not done, the compositor will assume that the window does not want
// to redraw and continue ahead.
//
// In eframe's case, that causes the window to rapidly flicker, as it struggles to deliver
// new frames to the compositor in time.
//
// The flickering is technically glutin or glow's fault, but we should be responding properly
// new frames to the compositor in time. The flickering is technically glutin or glow's fault, but we should be responding properly
// to resizes anyway, as doing so avoids dropping frames.
//
// See: https://github.com/emilk/egui/issues/903
let mut repaint_asap = false;

// On MacOS the asap repaint is not enough. The drawn frames must be synchronized with
// the CoreAnimation transactions driving the window resize process.
//
// Thus, Painter, responsible for wgpu surfaces and their resize, has to be notified of the
// resize lifecycle, yet winit does not provide any events for that. To work around,
// the last resized viewport is tracked until any next non-resize event is received.
//
// Accidental state change during the resize process due to an unexpected event fire
// is ok, state will switch back upon next resize event.
//
// See: https://github.com/emilk/egui/issues/903
if let Some(id) = viewport_id
&& shared.resized_viewport == viewport_id
{
shared.painter.on_window_resize_state_change(id, false);
shared.resized_viewport = None;
}

match event {
winit::event::WindowEvent::Focused(focused) => {
let focused = if cfg!(target_os = "macos")
Expand All @@ -799,14 +815,18 @@ impl WgpuWinitRunning<'_> {
// Resize with 0 width and height is used by winit to signal a minimize event on Windows.
// See: https://github.com/rust-windowing/winit/issues/208
// This solves an issue where the app would panic when minimizing on Windows.
if let Some(viewport_id) = viewport_id
if let Some(id) = viewport_id
&& let (Some(width), Some(height)) = (
NonZeroU32::new(physical_size.width),
NonZeroU32::new(physical_size.height),
)
{
if shared.resized_viewport != viewport_id {
shared.resized_viewport = viewport_id;
shared.painter.on_window_resize_state_change(id, true);
}
shared.painter.on_window_resized(id, width, height);
repaint_asap = true;
shared.painter.on_window_resized(viewport_id, width, height);
}
}

Expand Down
5 changes: 4 additions & 1 deletion crates/egui-wgpu/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ all-features = true
rustdoc-args = ["--generate-link-to-definition"]

[features]
default = ["fragile-send-sync-non-atomic-wasm", "wgpu/default"]
default = ["fragile-send-sync-non-atomic-wasm", "macos-window-resize-jitter-fix", "wgpu/default"]

## Enable [`winit`](https://docs.rs/winit) integration. On Linux, requires either `wayland` or `x11`
winit = ["dep:winit", "winit/rwh_06"]
Expand All @@ -43,6 +43,9 @@ x11 = ["winit?/x11"]
## Thus that usage is guarded against with compiler errors in wgpu.
fragile-send-sync-non-atomic-wasm = ["wgpu/fragile-send-sync-non-atomic-wasm"]

## Enables `present_with_transaction` surface flag temporary during window resize on MacOS.
macos-window-resize-jitter-fix = ["wgpu/metal"]

[dependencies]
egui = { workspace = true, default-features = false }
epaint = { workspace = true, default-features = false, features = ["bytemuck"] }
Expand Down
55 changes: 55 additions & 0 deletions crates/egui-wgpu/src/winit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ struct SurfaceState {
alpha_mode: wgpu::CompositeAlphaMode,
width: u32,
height: u32,
resizing: bool,
}

/// Everything you need to paint egui with [`wgpu`] on [`winit`].
Expand Down Expand Up @@ -230,6 +231,7 @@ impl Painter {
width: size.width,
height: size.height,
alpha_mode,
resizing: false,
},
);
let Some(width) = NonZeroU32::new(size.width) else {
Expand Down Expand Up @@ -326,6 +328,59 @@ impl Painter {
}
}

/// Handles changes of the resizing state.
///
/// Should be called prior to the first [`Painter::on_window_resized`] call and after the last in
/// the chain. Used to apply platform-specific logic, e.g. OSX Metal window resize jitter fix.
pub fn on_window_resize_state_change(&mut self, viewport_id: ViewportId, resizing: bool) {
profiling::function_scope!();

let Some(state) = self.surfaces.get_mut(&viewport_id) else {
return;
};
if state.resizing == resizing {
if resizing {
log::debug!(
"Painter::on_window_resize_state_change() redundant call while resizing"
);
} else {
log::debug!(
"Painter::on_window_resize_state_change() redundant call after resizing"
);
}
return;
}

// Resizing is a bit tricky on macOS.
// It requires enabling ["present_with_transaction"](https://developer.apple.com/documentation/quartzcore/cametallayer/presentswithtransaction)
// flag to avoid jittering during the resize. Even though resize jittering on macOS
// is common across rendering backends, the solution for wgpu/metal is known.
//
// See https://github.com/emilk/egui/issues/903
#[cfg(all(target_os = "macos", feature = "macos-window-resize-jitter-fix"))]
{
// SAFETY: The cast is checked with if condition. If the used backend is not metal
// it gracefully fails. The pointer casts are valid as it's 1-to-1 type mapping.
// This is how wgpu currently exposes this backend-specific flag.
unsafe {
if let Some(hal_surface) = state.surface.as_hal::<wgpu::hal::api::Metal>() {
let raw =
std::ptr::from_ref::<wgpu::hal::metal::Surface>(&*hal_surface).cast_mut();

(*raw).present_with_transaction = resizing;

Self::configure_surface(
state,
self.render_state.as_ref().unwrap(),
&self.configuration,
);
Comment on lines +372 to +376
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this part can go outside of the unsafe scope, no need to have that longer than necessary

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

.. but it's actually annoying to do so with the condition an all. Whatever (:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's exactly what I have been moving-back-n-forth for 15minutes w/o any even remote satisfaction, agree.

}
}
}

state.resizing = resizing;
}

pub fn on_window_resized(
&mut self,
viewport_id: ViewportId,
Expand Down
Loading