winit
winit copied to clipboard
rust-windowing
Skip releasing displays that are unavailable
Prevent assertion error when trying to close a fullscreen window that was on a display that got disconnected.
- [ ] Tested on all platforms changed
- [ ] Added an entry to the
changelogmodule if knowledge of this change could be valuable to users - [ ] Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
- [ ] Created or updated an example program if it would help users understand this functionality
- [ ] Updated feature matrix, if new features were added or implemented
While investigating https://github.com/bevyengine/bevy/pull/13669#issuecomment-2160283863 I saw that there was chance that this method is called with a display handle that relates to a display (monitor) that was disconnected. This led to a panic in the example added in the bevy PR.
What is the exact assertion error that you get? I'm asking because I'd like to know the error type that you're hitting as the result of
CGDisplayRelease?
Nvmd, I just saw your logs in the linked Bevy issue, it sounds like you're hitting kCGErrorIllegalArgument? That's weird, could you try to debug-log video_mode?
Thanks for looking at this!
Yeah that's the error code I saw. Here is the full log with a few dbg calls right before the ffi calls -- it doesn't look like it's outputting anything strange, these are the specs for the screen that I disconnect.
2024-06-17T08:13:55.528582Z INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "MacOS 14.4.1 ", kernel: "23.4.0", cpu: "Apple M1 Pro", core_count: "10", memory: "32.0 GiB" }
2024-06-17T08:13:55.626080Z INFO bevy_render::renderer: AdapterInfo { name: "Apple M1 Pro", vendor: 0, device: 0, device_type: IntegratedGpu, driver: "", driver_info: "", backend: Metal }
2024-06-17T08:13:56.242942Z INFO bevy_winit::system: Creating new window "Monitor #41039" (Entity { index: 2, generation: 1 })
2024-06-17T08:13:56.726636Z INFO bevy_winit::system: Creating new window "Monitor #16901" (Entity { index: 5, generation: 1 })
2024-06-17T08:13:57.045025Z WARN bevy_time: time_system did not receive the time from the render world! Calculations depending on the time may be incorrect.
2024-06-17T08:14:01.719370Z INFO bevy_winit::system: Monitor removed Entity { index: 0, generation: 1 }
2024-06-17T08:14:01.749790Z INFO bevy_winit::system: Closing window Entity { index: 5, generation: 1 }
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1397:17] video_mode = VideoModeHandle {
size: PhysicalSize {
width: 5120,
height: 2160,
},
bit_depth: 32,
refresh_rate_millihertz: 60000,
monitor: MonitorHandle {
name: Some(
"Monitor #16901",
),
native_identifier: 2,
size: PhysicalSize {
width: 1,
height: 1,
},
position: PhysicalPosition {
x: 0,
y: 0,
},
scale_factor: 1.0,
refresh_rate_millihertz: Some(
60000,
),
..
},
}
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1398:17] video_mode.monitor() = MonitorHandle {
name: Some(
"Monitor #16901",
),
native_identifier: 2,
size: PhysicalSize {
width: 1,
height: 1,
},
position: PhysicalPosition {
x: 0,
y: 0,
},
scale_factor: 1.0,
refresh_rate_millihertz: Some(
60000,
),
..
}
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1399:17] video_mode.monitor().native_identifier() = 2
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1400:17] video_mode.monitor() = MonitorHandle {
name: Some(
"Monitor #16901",
),
native_identifier: 2,
size: PhysicalSize {
width: 1,
height: 1,
},
position: PhysicalPosition {
x: 0,
y: 0,
},
scale_factor: 1.0,
refresh_rate_millihertz: Some(
60000,
),
..
}
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1401:17] video_mode.native_mode.0 = 0x00000001268378f0
thread 'main' panicked at /Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1407:25:
assertion `left == right` failed
left: 1001
right: 0
stack backtrace:
0: rust_begin_unwind
at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/std/src/panicking.rs:645:5
1: core::panicking::panic_fmt
at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/panicking.rs:72:14
2: core::panicking::assert_failed_inner
3: core::panicking::assert_failed
at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/panicking.rs:298:5
4: winit::platform_impl::platform::window_delegate::WindowDelegate::set_fullscreen
at /Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1407:25
5: winit::window::Window::set_fullscreen::{{closure}}
at /Users/pascal/code/rust-windowing/winit/src/window.rs:1138:50
6: winit::platform_impl::platform::window::Window::maybe_wait_on_main::{{closure}}
at /Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window.rs:49:46
7: objc2_foundation::thread::MainThreadBound<T>::get_on_main::{{closure}}
at /Users/pascal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-foundation-0.2.2/src/thread.rs:440:27
8: objc2_foundation::thread::run_on_main
at /Users/pascal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-foundation-0.2.2/src/thread.rs:113:9
9: objc2_foundation::thread::MainThreadBound<T>::get_on_main
at /Users/pascal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/objc2-foundation-0.2.2/src/thread.rs:440:9
10: winit::platform_impl::platform::window::Window::maybe_wait_on_main
at /Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window.rs:49:9
11: winit::platform_impl::platform::window::Window::maybe_queue_on_main
at /Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window.rs:42:9
12: winit::window::Window::set_fullscreen
at /Users/pascal/code/rust-windowing/winit/src/window.rs:1138:9
13: bevy_winit::system::despawn_windows
at ./crates/bevy_winit/src/system.rs:215:17
14: core::ops::function::FnMut::call_mut
at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/ops/function.rs:166:5
15: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/ops/function.rs:294:13
16: <Func as bevy_ecs::system::function_system::SystemParamFunction<fn(F0,F1,F2,F3,F4,F5,F6,F7) .> Out>>::run::call_inner
at ./crates/bevy_ecs/src/system/function_system.rs:704:21
17: <Func as bevy_ecs::system::function_system::SystemParamFunction<fn(F0,F1,F2,F3,F4,F5,F6,F7) .> Out>>::run
at ./crates/bevy_ecs/src/system/function_system.rs:707:17
18: <bevy_ecs::system::function_system::FunctionSystem<Marker,F> as bevy_ecs::system::system::System>::run_unsafe
at ./crates/bevy_ecs/src/system/function_system.rs:534:19
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Encountered a panic in system `bevy_winit::system::despawn_windows`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!
Thanks, seems like the CGDirectDisplayID is 2, which definitely sounds like a valid value, and the other CGDisplay functions that we call on it seem to work :/ (I was expecting something like 0).
Could you try running CGDisplayIsCaptured(native_identifier)? You might have to define it first with extern "C" { fn CGDisplayIsCaptured(display: CGDirectDisplayID) -> c_int; }.
Will probably take a closer look myself in a few days when I get the time and have access to an external monitor (especially if reminded). In any case, I still currently think the correct solution is to log the error instead of asserting it.
Sure! I did this now:
+ dbg!(video_mode);
+ dbg!(video_mode.monitor());
+ dbg!(video_mode.monitor().native_identifier());
+ dbg!(video_mode.monitor());
+ dbg!(video_mode.native_mode.0);
+ let available_monitors = monitor::available_monitors();
+ dbg!(available_monitors);
+
+ let monitor = &video_mode.monitor();
+ dbg!(unsafe { CGDisplayIsCaptured(monitor.native_identifier()) });
unsafe {
ffi::CGRestorePermanentDisplayConfiguration();
assert_eq!(
- ffi::CGDisplayRelease(video_mode.monitor().native_identifier()),
+ ffi::CGDisplayRelease(monitor.native_identifier()),
ffi::kCGErrorSuccess
);
};
and it yields
2024-06-17T08:52:20.221625Z INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "MacOS 14.4.1 ", kernel: "23.4.0", cpu: "Apple M1 Pro", core_count: "10", memory: "32.0 GiB" }
2024-06-17T08:52:20.345355Z INFO bevy_render::renderer: AdapterInfo { name: "Apple M1 Pro", vendor: 0, device: 0, device_type: IntegratedGpu, driver: "", driver_info: "", backend: Metal }
2024-06-17T08:52:20.935595Z INFO bevy_winit::system: Creating new window "Monitor #41039" (Entity { index: 2, generation: 1 })
2024-06-17T08:52:21.383400Z INFO bevy_winit::system: Creating new window "Monitor #16901" (Entity { index: 5, generation: 1 })
2024-06-17T08:52:21.767103Z WARN bevy_time: time_system did not receive the time from the render world! Calculations depending on the time may be incorrect.
2024-06-17T08:52:26.558842Z INFO bevy_winit::system: Monitor removed Entity { index: 0, generation: 1 }
2024-06-17T08:52:26.587391Z INFO bevy_winit::system: Closing window Entity { index: 5, generation: 1 }
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1398:17] video_mode = VideoModeHandle {
size: PhysicalSize {
width: 5120,
height: 2160,
},
bit_depth: 32,
refresh_rate_millihertz: 60000,
monitor: MonitorHandle {
name: Some(
"Monitor #16901",
),
native_identifier: 2,
size: PhysicalSize {
width: 1,
height: 1,
},
position: PhysicalPosition {
x: 0,
y: 0,
},
scale_factor: 1.0,
refresh_rate_millihertz: Some(
60000,
),
..
},
}
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1399:17] video_mode.monitor() = MonitorHandle {
name: Some(
"Monitor #16901",
),
native_identifier: 2,
size: PhysicalSize {
width: 1,
height: 1,
},
position: PhysicalPosition {
x: 0,
y: 0,
},
scale_factor: 1.0,
refresh_rate_millihertz: Some(
60000,
),
..
}
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1400:17] video_mode.monitor().native_identifier() = 2
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1401:17] video_mode.monitor() = MonitorHandle {
name: Some(
"Monitor #16901",
),
native_identifier: 2,
size: PhysicalSize {
width: 1,
height: 1,
},
position: PhysicalPosition {
x: 0,
y: 0,
},
scale_factor: 1.0,
refresh_rate_millihertz: Some(
60000,
),
..
}
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1402:17] video_mode.native_mode.0 = 0x000000014194fd30
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1404:17] available_monitors = [
MonitorHandle {
name: Some(
"Monitor #41039",
),
native_identifier: 1,
size: PhysicalSize {
width: 3024,
height: 1964,
},
position: PhysicalPosition {
x: 0,
y: 0,
},
scale_factor: 2.0,
refresh_rate_millihertz: Some(
120000,
),
..
},
]
[/Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1407:17] unsafe { CGDisplayIsCaptured(monitor.native_identifier()) } = 0
thread 'main' panicked at /Users/pascal/code/rust-windowing/winit/src/platform_impl/macos/window_delegate.rs:1410:21:
assertion `left == right` failed
left: 1001
right: 0
stack backtrace: …
Curious how CGDisplayIsCaptured(2) returns no error but available_monitors does not contain that display.
Rebase now.
@madsmtm did you have time to look at this? Is there anything else I should check?
This is obviously outside the scope of this PR, but ideally we'd have a way to react to the missing monitor in user code, i.e. by returning an error. But the log is definitely an upgrade over just panicking, which feels like it's almost never the right move.
Just thought about this again. If it makes sense, I could change it so that instead of the assert we parse the return code into a proper Result<(), CGError> type. Initially to not introduce a breaking change we can still unwrap or log it, but later on return it (maybe wrapped in an even more meaningful type).