From 70d03e484135ef56d5f6a308bab3b70e5f468f82 Mon Sep 17 00:00:00 2001 From: apricotbucket28 <71973804+apricotbucket28@users.noreply.github.com> Date: Fri, 26 Apr 2024 17:53:49 -0300 Subject: [PATCH] x11: Fix window close (#11008) Fixes https://github.com/zed-industries/zed/issues/10483 on X11 Also calls the `should_close` callback before closing the window (needed for the "Do you want to save?" dialog). Release Notes: - N/A --- crates/gpui/src/platform/linux/x11/client.rs | 48 ++++--- crates/gpui/src/platform/linux/x11/window.rs | 132 +++++++++++++------ 2 files changed, 123 insertions(+), 57 deletions(-) diff --git a/crates/gpui/src/platform/linux/x11/client.rs b/crates/gpui/src/platform/linux/x11/client.rs index bc1a1b527e2c5ef3a06f171b63538480ba723426..d4a0044f799c6766f96c077b54cf2abc81244d6e 100644 --- a/crates/gpui/src/platform/linux/x11/client.rs +++ b/crates/gpui/src/platform/linux/x11/client.rs @@ -1,6 +1,6 @@ use std::cell::RefCell; use std::ops::Deref; -use std::rc::Rc; +use std::rc::{Rc, Weak}; use std::time::{Duration, Instant}; use calloop::{EventLoop, LoopHandle}; @@ -23,10 +23,10 @@ use crate::platform::linux::LinuxClient; use crate::platform::{LinuxCommon, PlatformWindow}; use crate::{ px, AnyWindowHandle, Bounds, CursorStyle, DisplayId, Modifiers, ModifiersChangedEvent, Pixels, - PlatformDisplay, PlatformInput, Point, ScrollDelta, Size, TouchPhase, WindowParams, + PlatformDisplay, PlatformInput, Point, ScrollDelta, Size, TouchPhase, WindowParams, X11Window, }; -use super::{super::SCROLL_LINES, X11Display, X11Window, XcbAtoms}; +use super::{super::SCROLL_LINES, X11Display, X11WindowStatePtr, XcbAtoms}; use super::{button_of_key, modifiers_from_state}; use crate::platform::linux::is_within_click_distance; use crate::platform::linux::platform::DOUBLE_CLICK_INTERVAL; @@ -36,12 +36,12 @@ use calloop::{ }; pub(crate) struct WindowRef { - window: X11Window, + window: X11WindowStatePtr, refresh_event_token: RegistrationToken, } impl Deref for WindowRef { - type Target = X11Window; + type Target = X11WindowStatePtr; fn deref(&self) -> &Self::Target { &self.window @@ -68,6 +68,24 @@ pub struct X11ClientState { pub(crate) primary: X11ClipboardContext, } +#[derive(Clone)] +pub struct X11ClientStatePtr(pub Weak>); + +impl X11ClientStatePtr { + pub fn drop_window(&self, x_window: u32) { + let client = X11Client(self.0.upgrade().expect("client already dropped")); + let mut state = client.0.borrow_mut(); + + if let Some(window_ref) = state.windows.remove(&x_window) { + state.loop_handle.remove(window_ref.refresh_event_token); + } + + if state.windows.is_empty() { + state.common.signal.stop(); + } + } +} + #[derive(Clone)] pub(crate) struct X11Client(Rc>); @@ -171,7 +189,7 @@ impl X11Client { }))) } - fn get_window(&self, win: xproto::Window) -> Option { + fn get_window(&self, win: xproto::Window) -> Option { let state = self.0.borrow(); state .windows @@ -182,18 +200,16 @@ impl X11Client { fn handle_event(&self, event: Event) -> Option<()> { match event { Event::ClientMessage(event) => { + let window = self.get_window(event.window)?; let [atom, ..] = event.data.as_data32(); let mut state = self.0.borrow_mut(); if atom == state.atoms.WM_DELETE_WINDOW { - // window "x" button clicked by user, we gracefully exit - let window_ref = state.windows.remove(&event.window)?; - - state.loop_handle.remove(window_ref.refresh_event_token); - window_ref.window.destroy(); - - if state.windows.is_empty() { - state.common.signal.stop(); + // window "x" button clicked by user + if window.should_close() { + let window_ref = state.windows.remove(&event.window)?; + state.loop_handle.remove(window_ref.refresh_event_token); + // Rest of the close logic is handled in drop_window() } } } @@ -424,6 +440,8 @@ impl LinuxClient for X11Client { let x_window = state.xcb_connection.generate_id().unwrap(); let window = X11Window::new( + X11ClientStatePtr(Rc::downgrade(&self.0)), + state.common.foreground_executor.clone(), params, &state.xcb_connection, state.x_root_index, @@ -492,7 +510,7 @@ impl LinuxClient for X11Client { .expect("Failed to initialize refresh timer"); let window_ref = WindowRef { - window: window.clone(), + window: window.0.clone(), refresh_event_token, }; diff --git a/crates/gpui/src/platform/linux/x11/window.rs b/crates/gpui/src/platform/linux/x11/window.rs index a1d8532f71072f66ad7e9e75fd6d02e18d6c1198..5bbedffe1711c31c94c55403a7fe39fca2b1e289 100644 --- a/crates/gpui/src/platform/linux/x11/window.rs +++ b/crates/gpui/src/platform/linux/x11/window.rs @@ -2,10 +2,10 @@ #![allow(unused)] use crate::{ - platform::blade::BladeRenderer, size, Bounds, DevicePixels, Modifiers, Pixels, PlatformAtlas, - PlatformDisplay, PlatformInput, PlatformInputHandler, PlatformWindow, Point, PromptLevel, - Scene, Size, WindowAppearance, WindowBackgroundAppearance, WindowOptions, WindowParams, - X11Client, X11ClientState, + platform::blade::BladeRenderer, size, Bounds, DevicePixels, ForegroundExecutor, Modifiers, + Pixels, Platform, PlatformAtlas, PlatformDisplay, PlatformInput, PlatformInputHandler, + PlatformWindow, Point, PromptLevel, Scene, Size, WindowAppearance, WindowBackgroundAppearance, + WindowOptions, WindowParams, X11Client, X11ClientState, X11ClientStatePtr, }; use blade_graphics as gpu; use parking_lot::Mutex; @@ -77,6 +77,8 @@ pub struct Callbacks { } pub(crate) struct X11WindowState { + client: X11ClientStatePtr, + executor: ForegroundExecutor, atoms: XcbAtoms, raw: RawWindow, bounds: Bounds, @@ -88,7 +90,7 @@ pub(crate) struct X11WindowState { } #[derive(Clone)] -pub(crate) struct X11Window { +pub(crate) struct X11WindowStatePtr { pub(crate) state: Rc>, pub(crate) callbacks: Rc>, xcb_connection: Rc, @@ -124,6 +126,8 @@ impl rwh::HasDisplayHandle for X11Window { impl X11WindowState { pub fn new( + client: X11ClientStatePtr, + executor: ForegroundExecutor, params: WindowParams, xcb_connection: &Rc, x_main_screen_index: usize, @@ -224,6 +228,8 @@ impl X11WindowState { let gpu_extent = query_render_extent(xcb_connection, x_window); Self { + client, + executor, display: Rc::new(X11Display::new(xcb_connection, x_screen_index).unwrap()), raw, bounds: params.bounds.map(|v| v.0), @@ -244,16 +250,47 @@ impl X11WindowState { } } +pub(crate) struct X11Window(pub X11WindowStatePtr); + +impl Drop for X11Window { + fn drop(&mut self) { + let mut state = self.0.state.borrow_mut(); + state.renderer.destroy(); + + self.0.xcb_connection.unmap_window(self.0.x_window).unwrap(); + self.0 + .xcb_connection + .destroy_window(self.0.x_window) + .unwrap(); + self.0.xcb_connection.flush().unwrap(); + + let this_ptr = self.0.clone(); + let client_ptr = state.client.clone(); + state + .executor + .spawn(async move { + this_ptr.close(); + client_ptr.drop_window(this_ptr.x_window); + }) + .detach(); + drop(state); + } +} + impl X11Window { pub fn new( + client: X11ClientStatePtr, + executor: ForegroundExecutor, params: WindowParams, xcb_connection: &Rc, x_main_screen_index: usize, x_window: xproto::Window, atoms: &XcbAtoms, ) -> Self { - X11Window { + Self(X11WindowStatePtr { state: Rc::new(RefCell::new(X11WindowState::new( + client, + executor, params, xcb_connection, x_main_screen_index, @@ -263,20 +300,27 @@ impl X11Window { callbacks: Rc::new(RefCell::new(Callbacks::default())), xcb_connection: xcb_connection.clone(), x_window, - } + }) } +} - pub fn destroy(&self) { - let mut state = self.state.borrow_mut(); - state.renderer.destroy(); - drop(state); +impl X11WindowStatePtr { + pub fn should_close(&self) -> bool { + let mut cb = self.callbacks.borrow_mut(); + if let Some(mut should_close) = cb.should_close.take() { + let result = (should_close)(); + cb.should_close = Some(should_close); + result + } else { + true + } + } - self.xcb_connection.unmap_window(self.x_window).unwrap(); - self.xcb_connection.destroy_window(self.x_window).unwrap(); - if let Some(fun) = self.callbacks.borrow_mut().close.take() { - fun(); + pub fn close(&self) { + let mut callbacks = self.callbacks.borrow_mut(); + if let Some(fun) = callbacks.close.take() { + fun() } - self.xcb_connection.flush().unwrap(); } pub fn refresh(&self) { @@ -345,7 +389,7 @@ impl X11Window { impl PlatformWindow for X11Window { fn bounds(&self) -> Bounds { - self.state.borrow_mut().bounds.map(|v| v.into()) + self.0.state.borrow_mut().bounds.map(|v| v.into()) } // todo(linux) @@ -359,11 +403,11 @@ impl PlatformWindow for X11Window { } fn content_size(&self) -> Size { - self.state.borrow_mut().content_size() + self.0.state.borrow_mut().content_size() } fn scale_factor(&self) -> f32 { - self.state.borrow_mut().scale_factor + self.0.state.borrow_mut().scale_factor } // todo(linux) @@ -372,13 +416,14 @@ impl PlatformWindow for X11Window { } fn display(&self) -> Rc { - self.state.borrow().display.clone() + self.0.state.borrow().display.clone() } fn mouse_position(&self) -> Point { let reply = self + .0 .xcb_connection - .query_pointer(self.x_window) + .query_pointer(self.0.x_window) .unwrap() .reply() .unwrap(); @@ -395,11 +440,11 @@ impl PlatformWindow for X11Window { } fn set_input_handler(&mut self, input_handler: PlatformInputHandler) { - self.state.borrow_mut().input_handler = Some(input_handler); + self.0.state.borrow_mut().input_handler = Some(input_handler); } fn take_input_handler(&mut self) -> Option { - self.state.borrow_mut().input_handler.take() + self.0.state.borrow_mut().input_handler.take() } fn prompt( @@ -414,8 +459,9 @@ impl PlatformWindow for X11Window { fn activate(&self) { let win_aux = xproto::ConfigureWindowAux::new().stack_mode(xproto::StackMode::ABOVE); - self.xcb_connection - .configure_window(self.x_window, &win_aux) + self.0 + .xcb_connection + .configure_window(self.0.x_window, &win_aux) .log_err(); } @@ -425,22 +471,24 @@ impl PlatformWindow for X11Window { } fn set_title(&mut self, title: &str) { - self.xcb_connection + self.0 + .xcb_connection .change_property8( xproto::PropMode::REPLACE, - self.x_window, + self.0.x_window, xproto::AtomEnum::WM_NAME, xproto::AtomEnum::STRING, title.as_bytes(), ) .unwrap(); - self.xcb_connection + self.0 + .xcb_connection .change_property8( xproto::PropMode::REPLACE, - self.x_window, - self.state.borrow().atoms._NET_WM_NAME, - self.state.borrow().atoms.UTF8_STRING, + self.0.x_window, + self.0.state.borrow().atoms._NET_WM_NAME, + self.0.state.borrow().atoms.UTF8_STRING, title.as_bytes(), ) .unwrap(); @@ -484,39 +532,39 @@ impl PlatformWindow for X11Window { } fn on_request_frame(&self, callback: Box) { - self.callbacks.borrow_mut().request_frame = Some(callback); + self.0.callbacks.borrow_mut().request_frame = Some(callback); } fn on_input(&self, callback: Box crate::DispatchEventResult>) { - self.callbacks.borrow_mut().input = Some(callback); + self.0.callbacks.borrow_mut().input = Some(callback); } fn on_active_status_change(&self, callback: Box) { - self.callbacks.borrow_mut().active_status_change = Some(callback); + self.0.callbacks.borrow_mut().active_status_change = Some(callback); } fn on_resize(&self, callback: Box, f32)>) { - self.callbacks.borrow_mut().resize = Some(callback); + self.0.callbacks.borrow_mut().resize = Some(callback); } fn on_fullscreen(&self, callback: Box) { - self.callbacks.borrow_mut().fullscreen = Some(callback); + self.0.callbacks.borrow_mut().fullscreen = Some(callback); } fn on_moved(&self, callback: Box) { - self.callbacks.borrow_mut().moved = Some(callback); + self.0.callbacks.borrow_mut().moved = Some(callback); } fn on_should_close(&self, callback: Box bool>) { - self.callbacks.borrow_mut().should_close = Some(callback); + self.0.callbacks.borrow_mut().should_close = Some(callback); } fn on_close(&self, callback: Box) { - self.callbacks.borrow_mut().close = Some(callback); + self.0.callbacks.borrow_mut().close = Some(callback); } fn on_appearance_changed(&self, callback: Box) { - self.callbacks.borrow_mut().appearance_changed = Some(callback); + self.0.callbacks.borrow_mut().appearance_changed = Some(callback); } // todo(linux) @@ -525,12 +573,12 @@ impl PlatformWindow for X11Window { } fn draw(&self, scene: &Scene) { - let mut inner = self.state.borrow_mut(); + let mut inner = self.0.state.borrow_mut(); inner.renderer.draw(scene); } fn sprite_atlas(&self) -> sync::Arc { - let inner = self.state.borrow(); + let inner = self.0.state.borrow(); inner.renderer.sprite_atlas().clone() } }