X11: Fix window creation hangs when another application is fullscreen (#1248)

* X11: Fix window creation hangs when another application is fullscreen

Previously, the X11 backend would block until a `VisibilityNotify` event
is received when creating a Window that is visible or when calling
`set_visible(true)` on a Window that is not currently visible. This
could cause winit to hang in situations where the WM does not quickly
send this event to the application, such as another window being
fullscreen at the time.

This behavior existed to prevent an X protocol error caused by setting
fullscreen state on an invisible window. This fix instead stores desired
fullscreen state when `set_fullscreen` is called (iff the window is not
visible or not yet visible) and issues X commands to set fullscreen
state when a `VisibilityNotify` event is received through the normal
processing of events in the event loop.

* Add window_debug example to facilitate testing

* Add a CHANGELOG entry

* Call `XUnmapWindow` if `VisibilityNotify` is received on an invisible window

CHANGELOG.md

@@ -4,6 +4,7 @@
 - On X11, fix misreporting DPI factor at startup.
 - On X11, fix events not being reported when using `run_return`.
 - On X11, fix key modifiers being incorrectly reported.
+- On X11, fix window creation hanging when another window is fullscreen.
 
 # 0.20.0 Alpha 4 (2019-10-18)
 

examples/timer.rs

@@ -17,10 +17,7 @@ fn main() {
     let timer_length = Duration::new(1, 0);
 
     event_loop.run(move |event, _, control_flow| {
-        if let Event::WindowEvent { event, .. } = &event {
-            // Print only Window events to reduce noise
-            println!("{:?}", event);
-        }
+        println!("{:?}", event);
 
         match event {
             Event::NewEvents(StartCause::Init) => {

examples/window_debug.rs

@@ -0,0 +1,121 @@
+// This example is used by developers to test various window functions.
+
+use winit::{
+    dpi::{LogicalSize, PhysicalSize},
+    event::{DeviceEvent, ElementState, Event, KeyboardInput, VirtualKeyCode, WindowEvent},
+    event_loop::{ControlFlow, EventLoop},
+    window::{Fullscreen, WindowBuilder},
+};
+
+fn main() {
+    let event_loop = EventLoop::new();
+
+    let window = WindowBuilder::new()
+        .with_title("A fantastic window!")
+        .with_inner_size(LogicalSize::from((100, 100)))
+        .build(&event_loop)
+        .unwrap();
+
+    eprintln!("debugging keys:");
+    eprintln!("  (E) Enter exclusive fullscreen");
+    eprintln!("  (F) Toggle borderless fullscreen");
+    #[cfg(waiting_for_set_minimized)]
+    eprintln!("  (M) Toggle minimized");
+    eprintln!("  (Q) Quit event loop");
+    eprintln!("  (V) Toggle visibility");
+    eprintln!("  (X) Toggle maximized");
+
+    #[cfg(waiting_for_set_minimized)]
+    let mut minimized = false;
+    let mut maximized = false;
+    let mut visible = true;
+
+    event_loop.run(move |event, _, control_flow| {
+        *control_flow = ControlFlow::Wait;
+
+        match event {
+            Event::DeviceEvent {
+                event:
+                    DeviceEvent::Key(KeyboardInput {
+                        virtual_keycode: Some(key),
+                        state: ElementState::Pressed,
+                        ..
+                    }),
+                ..
+            } => match key {
+                #[cfg(waiting_for_set_minimized)]
+                VirtualKeyCode::M => {
+                    if minimized {
+                        minimized = !minimized;
+                        window.set_minimized(minimized);
+                    }
+                }
+                VirtualKeyCode::V => {
+                    if !visible {
+                        visible = !visible;
+                        window.set_visible(visible);
+                    }
+                }
+                _ => (),
+            },
+            Event::WindowEvent {
+                event: WindowEvent::KeyboardInput { input, .. },
+                ..
+            } => match input {
+                KeyboardInput {
+                    virtual_keycode: Some(key),
+                    state: ElementState::Pressed,
+                    ..
+                } => match key {
+                    VirtualKeyCode::E => {
+                        fn area(size: PhysicalSize) -> f64 {
+                            size.width * size.height
+                        }
+
+                        let monitor = window.current_monitor();
+                        if let Some(mode) = monitor.video_modes().max_by(|a, b| {
+                            area(a.size())
+                                .partial_cmp(&area(b.size()))
+                                .expect("NaN in video mode size")
+                        }) {
+                            window.set_fullscreen(Some(Fullscreen::Exclusive(mode)));
+                        } else {
+                            eprintln!("no video modes available");
+                        }
+                    }
+                    VirtualKeyCode::F => {
+                        if window.fullscreen().is_some() {
+                            window.set_fullscreen(None);
+                        } else {
+                            let monitor = window.current_monitor();
+                            window.set_fullscreen(Some(Fullscreen::Borderless(monitor)));
+                        }
+                    }
+                    #[cfg(waiting_for_set_minimized)]
+                    VirtualKeyCode::M => {
+                        minimized = !minimized;
+                        window.set_minimized(minimized);
+                    }
+                    VirtualKeyCode::Q => {
+                        *control_flow = ControlFlow::Exit;
+                    }
+                    VirtualKeyCode::V => {
+                        visible = !visible;
+                        window.set_visible(visible);
+                    }
+                    VirtualKeyCode::X => {
+                        maximized = !maximized;
+                        window.set_maximized(maximized);
+                    }
+                    _ => (),
+                },
+                _ => (),
+            },
+            Event::WindowEvent {
+                event: WindowEvent::CloseRequested,
+                window_id,
+            } if window_id == window.id() => *control_flow = ControlFlow::Exit,
+            _ => (),
+        }
+    });
+}

src/platform_impl/linux/x11/event_processor.rs

@@ -497,6 +497,13 @@ impl<T: 'static> EventProcessor<T> {
                 });
             }
 
+            ffi::VisibilityNotify => {
+                let xev: &ffi::XVisibilityEvent = xev.as_ref();
+                let xwindow = xev.window;
+
+                self.with_window(xwindow, |window| window.visibility_notify());
+            }
+
             ffi::Expose => {
                 let xev: &ffi::XExposeEvent = xev.as_ref();
 

src/platform_impl/linux/x11/window.rs

@@ -28,16 +28,6 @@ use crate::{
 
 use super::{ffi, util, EventLoopWindowTarget, ImeSender, WindowId, XConnection, XError};
 
-unsafe extern "C" fn visibility_predicate(
-    _display: *mut ffi::Display,
-    event: *mut ffi::XEvent,
-    arg: ffi::XPointer, // We populate this with the window ID (by value) when we call XIfEvent
-) -> ffi::Bool {
-    let event: &ffi::XAnyEvent = (*event).as_ref();
-    let window = arg as ffi::Window;
-    (event.window == window && event.type_ == ffi::VisibilityNotify) as _
-}
-
 #[derive(Debug)]
 pub struct SharedState {
     pub cursor_pos: Option<(f64, f64)>,
@@ -48,6 +38,8 @@ pub struct SharedState {
     pub last_monitor: X11MonitorHandle,
     pub dpi_adjusted: Option<(f64, f64)>,
     pub fullscreen: Option<Fullscreen>,
+    // Set when application calls `set_fullscreen` when window is not visible
+    pub desired_fullscreen: Option<Option<Fullscreen>>,
     // Used to restore position after exiting fullscreen
     pub restore_position: Option<(i32, i32)>,
     // Used to restore video mode after exiting fullscreen
@@ -55,14 +47,28 @@ pub struct SharedState {
     pub frame_extents: Option<util::FrameExtentsHeuristic>,
     pub min_inner_size: Option<LogicalSize>,
     pub max_inner_size: Option<LogicalSize>,
-    pub is_visible: bool,
+    pub visibility: Visibility,
+}
+
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub enum Visibility {
+    No,
+    Yes,
+    // Waiting for VisibilityNotify
+    YesWait,
 }
 
 impl SharedState {
     fn new(last_monitor: X11MonitorHandle, is_visible: bool) -> Mutex<Self> {
+        let visibility = if is_visible {
+            Visibility::YesWait
+        } else {
+            Visibility::No
+        };
+
         Mutex::new(SharedState {
             last_monitor,
-            is_visible,
+            visibility,
 
             cursor_pos: None,
             size: None,
@@ -71,6 +77,7 @@ impl SharedState {
             inner_position_rel_parent: None,
             dpi_adjusted: None,
             fullscreen: None,
+            desired_fullscreen: None,
             restore_position: None,
             desktop_video_mode: None,
             frame_extents: None,
@@ -360,8 +367,6 @@ impl UnownedWindow {
                 unsafe {
                     (xconn.xlib.XMapRaised)(xconn.display, window.xwindow);
                 } //.queue();
-
-                window.wait_for_visibility_notify();
             }
 
             // Attempt to make keyboard input repeat detectable
@@ -419,8 +424,7 @@ impl UnownedWindow {
             if window_attrs.fullscreen.is_some() {
                 window
                     .set_fullscreen_inner(window_attrs.fullscreen.clone())
-                    .unwrap()
-                    .queue();
+                    .map(|flusher| flusher.queue());
             }
             if window_attrs.always_on_top {
                 window
@@ -577,9 +581,13 @@ impl UnownedWindow {
     fn set_fullscreen_inner(&self, fullscreen: Option<Fullscreen>) -> Option<util::Flusher<'_>> {
         let mut shared_state_lock = self.shared_state.lock();
 
-        if !shared_state_lock.is_visible {
+        match shared_state_lock.visibility {
             // Setting fullscreen on a window that is not visible will generate an error.
-            return None;
+            Visibility::No | Visibility::YesWait => {
+                shared_state_lock.desired_fullscreen = Some(fullscreen);
+                return None;
+            }
+            Visibility::Yes => (),
         }
 
         let old_fullscreen = shared_state_lock.fullscreen.clone();
@@ -690,7 +698,12 @@ impl UnownedWindow {
 
     #[inline]
     pub fn fullscreen(&self) -> Option<Fullscreen> {
-        self.shared_state.lock().fullscreen.clone()
+        let shared_state = self.shared_state.lock();
+
+        shared_state
+            .desired_fullscreen
+            .clone()
+            .unwrap_or_else(|| shared_state.fullscreen.clone())
     }
 
     #[inline]
@@ -703,6 +716,26 @@ impl UnownedWindow {
         }
     }
 
+    // Called by EventProcessor when a VisibilityNotify event is received
+    pub(crate) fn visibility_notify(&self) {
+        let mut shared_state = self.shared_state.lock();
+
+        match shared_state.visibility {
+            Visibility::No => unsafe {
+                (self.xconn.xlib.XUnmapWindow)(self.xconn.display, self.xwindow);
+            },
+            Visibility::Yes => (),
+            Visibility::YesWait => {
+                shared_state.visibility = Visibility::Yes;
+
+                if let Some(fullscreen) = shared_state.desired_fullscreen.take() {
+                    drop(shared_state);
+                    self.set_fullscreen(fullscreen);
+                }
+            }
+        }
+    }
+
     #[inline]
     pub fn current_monitor(&self) -> X11MonitorHandle {
         self.shared_state.lock().last_monitor.clone()
@@ -838,44 +871,31 @@ impl UnownedWindow {
 
     #[inline]
     pub fn set_visible(&self, visible: bool) {
-        let is_visible = self.shared_state.lock().is_visible;
+        let mut shared_state = self.shared_state.lock();
 
-        if visible == is_visible {
-            return;
+        match (visible, shared_state.visibility) {
+            (true, Visibility::Yes) | (true, Visibility::YesWait) | (false, Visibility::No) => {
+                return
+            }
+            _ => (),
         }
 
-        match visible {
-            true => unsafe {
+        if visible {
+            unsafe {
                 (self.xconn.xlib.XMapRaised)(self.xconn.display, self.xwindow);
-                self.xconn
-                    .flush_requests()
-                    .expect("Failed to call XMapRaised");
-
-                // Some X requests may generate an error if the window is not
-                // visible, so we must wait until the window becomes visible.
-                self.wait_for_visibility_notify();
-            },
-            false => unsafe {
+            }
+            self.xconn
+                .flush_requests()
+                .expect("Failed to call XMapRaised");
+            shared_state.visibility = Visibility::YesWait;
+        } else {
+            unsafe {
                 (self.xconn.xlib.XUnmapWindow)(self.xconn.display, self.xwindow);
-                self.xconn
-                    .flush_requests()
-                    .expect("Failed to call XUnmapWindow");
-            },
-        }
-
-        self.shared_state.lock().is_visible = visible;
-    }
-
-    fn wait_for_visibility_notify(&self) {
-        unsafe {
-            let mut event = MaybeUninit::uninit();
-
-            (self.xconn.xlib.XIfEvent)(
-                self.xconn.display,
-                event.as_mut_ptr(),
-                Some(visibility_predicate),
-                self.xwindow as _,
-            );
+            }
+            self.xconn
+                .flush_requests()
+                .expect("Failed to call XUnmapWindow");
+            shared_state.visibility = Visibility::No;
         }
     }