Skip to content

Commit 6f5ea1a

Browse files
aspcartmanemilk
authored andcommitted
Fix jittering during window resize on MacOS for WGPU/Metal (emilk#7641)
Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
1 parent 8c94227 commit 6f5ea1a

3 files changed

Lines changed: 88 additions & 10 deletions

File tree

crates/eframe/src/native/wgpu_integration.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ pub struct SharedState {
7171
painter: egui_wgpu::winit::Painter,
7272
viewport_from_window: HashMap<WindowId, ViewportId>,
7373
focused_viewport: Option<ViewportId>,
74+
resized_viewport: Option<ViewportId>,
7475
}
7576

7677
pub type Viewports = egui::OrderedViewportIdMap<Viewport>;
@@ -302,6 +303,7 @@ impl<'app> WgpuWinitApp<'app> {
302303
viewports,
303304
painter,
304305
focused_viewport: Some(ViewportId::ROOT),
306+
resized_viewport: None,
305307
}));
306308

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

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

778+
// On MacOS the asap repaint is not enough. The drawn frames must be synchronized with
779+
// the CoreAnimation transactions driving the window resize process.
780+
//
781+
// Thus, Painter, responsible for wgpu surfaces and their resize, has to be notified of the
782+
// resize lifecycle, yet winit does not provide any events for that. To work around,
783+
// the last resized viewport is tracked until any next non-resize event is received.
784+
//
785+
// Accidental state change during the resize process due to an unexpected event fire
786+
// is ok, state will switch back upon next resize event.
787+
//
788+
// See: https://github.com/emilk/egui/issues/903
789+
if let Some(id) = viewport_id
790+
&& shared.resized_viewport == viewport_id
791+
{
792+
shared.painter.on_window_resize_state_change(id, false);
793+
shared.resized_viewport = None;
794+
}
795+
780796
match event {
781797
winit::event::WindowEvent::Focused(focused) => {
782798
let focused = if cfg!(target_os = "macos")
@@ -799,14 +815,18 @@ impl WgpuWinitRunning<'_> {
799815
// Resize with 0 width and height is used by winit to signal a minimize event on Windows.
800816
// See: https://github.com/rust-windowing/winit/issues/208
801817
// This solves an issue where the app would panic when minimizing on Windows.
802-
if let Some(viewport_id) = viewport_id
818+
if let Some(id) = viewport_id
803819
&& let (Some(width), Some(height)) = (
804820
NonZeroU32::new(physical_size.width),
805821
NonZeroU32::new(physical_size.height),
806822
)
807823
{
824+
if shared.resized_viewport != viewport_id {
825+
shared.resized_viewport = viewport_id;
826+
shared.painter.on_window_resize_state_change(id, true);
827+
}
828+
shared.painter.on_window_resized(id, width, height);
808829
repaint_asap = true;
809-
shared.painter.on_window_resized(viewport_id, width, height);
810830
}
811831
}
812832

crates/egui-wgpu/Cargo.toml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ all-features = true
2525
rustdoc-args = ["--generate-link-to-definition"]
2626

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

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

46+
## Enables `present_with_transaction` surface flag temporary during window resize on MacOS.
47+
macos-window-resize-jitter-fix = ["wgpu/metal"]
48+
4649
[dependencies]
4750
egui = { workspace = true, default-features = false }
4851
epaint = { workspace = true, default-features = false, features = ["bytemuck"] }

crates/egui-wgpu/src/winit.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ struct SurfaceState {
1414
alpha_mode: wgpu::CompositeAlphaMode,
1515
width: u32,
1616
height: u32,
17+
resizing: bool,
1718
}
1819

1920
/// Everything you need to paint egui with [`wgpu`] on [`winit`].
@@ -230,6 +231,7 @@ impl Painter {
230231
width: size.width,
231232
height: size.height,
232233
alpha_mode,
234+
resizing: false,
233235
},
234236
);
235237
let Some(width) = NonZeroU32::new(size.width) else {
@@ -326,6 +328,59 @@ impl Painter {
326328
}
327329
}
328330

331+
/// Handles changes of the resizing state.
332+
///
333+
/// Should be called prior to the first [`Painter::on_window_resized`] call and after the last in
334+
/// the chain. Used to apply platform-specific logic, e.g. OSX Metal window resize jitter fix.
335+
pub fn on_window_resize_state_change(&mut self, viewport_id: ViewportId, resizing: bool) {
336+
profiling::function_scope!();
337+
338+
let Some(state) = self.surfaces.get_mut(&viewport_id) else {
339+
return;
340+
};
341+
if state.resizing == resizing {
342+
if resizing {
343+
log::debug!(
344+
"Painter::on_window_resize_state_change() redundant call while resizing"
345+
);
346+
} else {
347+
log::debug!(
348+
"Painter::on_window_resize_state_change() redundant call after resizing"
349+
);
350+
}
351+
return;
352+
}
353+
354+
// Resizing is a bit tricky on macOS.
355+
// It requires enabling ["present_with_transaction"](https://developer.apple.com/documentation/quartzcore/cametallayer/presentswithtransaction)
356+
// flag to avoid jittering during the resize. Even though resize jittering on macOS
357+
// is common across rendering backends, the solution for wgpu/metal is known.
358+
//
359+
// See https://github.com/emilk/egui/issues/903
360+
#[cfg(all(target_os = "macos", feature = "macos-window-resize-jitter-fix"))]
361+
{
362+
// SAFETY: The cast is checked with if condition. If the used backend is not metal
363+
// it gracefully fails. The pointer casts are valid as it's 1-to-1 type mapping.
364+
// This is how wgpu currently exposes this backend-specific flag.
365+
unsafe {
366+
if let Some(hal_surface) = state.surface.as_hal::<wgpu::hal::api::Metal>() {
367+
let raw =
368+
std::ptr::from_ref::<wgpu::hal::metal::Surface>(&*hal_surface).cast_mut();
369+
370+
(*raw).present_with_transaction = resizing;
371+
372+
Self::configure_surface(
373+
state,
374+
self.render_state.as_ref().unwrap(),
375+
&self.configuration,
376+
);
377+
}
378+
}
379+
}
380+
381+
state.resizing = resizing;
382+
}
383+
329384
pub fn on_window_resized(
330385
&mut self,
331386
viewport_id: ViewportId,

0 commit comments

Comments
 (0)