Fix window restoration bugs (#9358)

Mikayla Maki created

- [x] fixes https://github.com/zed-industries/zed/issues/9349
- [x] fixes https://github.com/zed-industries/zed/issues/4656
- [x] fixes https://github.com/zed-industries/zed/issues/8345

Release Notes:

- TODO

Change summary

crates/collab_ui/src/collab_titlebar_item.rs     |  2 
crates/gpui/src/platform.rs                      |  6 
crates/gpui/src/platform/linux/wayland/window.rs |  4 
crates/gpui/src/platform/linux/x11/window.rs     |  4 
crates/gpui/src/platform/mac/status_item.rs      |  2 
crates/gpui/src/platform/mac/window.rs           |  4 
crates/gpui/src/platform/test/window.rs          |  4 
crates/gpui/src/platform/windows/window.rs       |  4 
crates/gpui/src/window.rs                        | 13 +-
crates/workspace/src/persistence.rs              | 37 +++++++
crates/workspace/src/persistence/model.rs        |  1 
crates/workspace/src/workspace.rs                | 79 +++++++++++------
crates/zed/src/zed.rs                            |  2 
13 files changed, 106 insertions(+), 56 deletions(-)

Detailed changes

crates/collab_ui/src/collab_titlebar_item.rs 🔗

@@ -64,7 +64,7 @@ impl Render for CollabTitlebarItem {
             .w_full()
             .h(titlebar_height(cx))
             .map(|this| {
-                if cx.is_full_screen() {
+                if cx.is_fullscreen() {
                     this.pl_2()
                 } else {
                     // Use pixels here instead of a rem-based size because the macOS traffic

crates/gpui/src/platform.rs 🔗

@@ -191,8 +191,8 @@ pub(crate) trait PlatformWindow: HasWindowHandle + HasDisplayHandle {
     fn show_character_palette(&self);
     fn minimize(&self);
     fn zoom(&self);
-    fn toggle_full_screen(&self);
-    fn is_full_screen(&self) -> bool;
+    fn toggle_fullscreen(&self);
+    fn is_fullscreen(&self) -> bool;
     fn on_request_frame(&self, callback: Box<dyn FnMut()>);
     fn on_input(&self, callback: Box<dyn FnMut(PlatformInput) -> bool>);
     fn on_active_status_change(&self, callback: Box<dyn FnMut(bool)>);
@@ -596,7 +596,7 @@ pub enum WindowKind {
 
 /// Platform level interface
 /// bounds: Bounds<GlobalPixels>
-/// full_screen: bool
+/// fullscreen: bool
 
 /// The appearance of the window, as defined by the operating system.
 ///

crates/gpui/src/platform/linux/wayland/window.rs 🔗

@@ -362,7 +362,7 @@ impl PlatformWindow for WaylandWindow {
         // todo(linux)
     }
 
-    fn toggle_full_screen(&self) {
+    fn toggle_fullscreen(&self) {
         if !self.0.inner.borrow().fullscreen {
             self.0.toplevel.set_fullscreen(None);
         } else {
@@ -370,7 +370,7 @@ impl PlatformWindow for WaylandWindow {
         }
     }
 
-    fn is_full_screen(&self) -> bool {
+    fn is_fullscreen(&self) -> bool {
         self.0.inner.borrow_mut().fullscreen
     }
 

crates/gpui/src/platform/linux/x11/window.rs 🔗

@@ -438,12 +438,12 @@ impl PlatformWindow for X11Window {
     }
 
     // todo(linux)
-    fn toggle_full_screen(&self) {
+    fn toggle_fullscreen(&self) {
         unimplemented!()
     }
 
     // todo(linux)
-    fn is_full_screen(&self) -> bool {
+    fn is_fullscreen(&self) -> bool {
         false
     }
 

crates/gpui/src/platform/mac/window.rs 🔗

@@ -941,7 +941,7 @@ impl PlatformWindow for MacWindow {
             .detach();
     }
 
-    fn toggle_full_screen(&self) {
+    fn toggle_fullscreen(&self) {
         let this = self.0.lock();
         let window = this.native_window;
         this.executor
@@ -953,7 +953,7 @@ impl PlatformWindow for MacWindow {
             .detach();
     }
 
-    fn is_full_screen(&self) -> bool {
+    fn is_fullscreen(&self) -> bool {
         let this = self.0.lock();
         let window = this.native_window;
 

crates/gpui/src/platform/test/window.rs 🔗

@@ -197,12 +197,12 @@ impl PlatformWindow for TestWindow {
         unimplemented!()
     }
 
-    fn toggle_full_screen(&self) {
+    fn toggle_fullscreen(&self) {
         let mut lock = self.0.lock();
         lock.is_fullscreen = !lock.is_fullscreen;
     }
 
-    fn is_full_screen(&self) -> bool {
+    fn is_fullscreen(&self) -> bool {
         self.0.lock().is_fullscreen
     }
 

crates/gpui/src/platform/windows/window.rs 🔗

@@ -909,10 +909,10 @@ impl PlatformWindow for WindowsWindow {
     fn zoom(&self) {}
 
     // todo(windows)
-    fn toggle_full_screen(&self) {}
+    fn toggle_fullscreen(&self) {}
 
     // todo(windows)
-    fn is_full_screen(&self) -> bool {
+    fn is_fullscreen(&self) -> bool {
         false
     }
 

crates/gpui/src/window.rs 🔗

@@ -338,9 +338,6 @@ fn default_bounds(cx: &mut AppContext) -> Bounds<GlobalPixels> {
         })
 }
 
-// Fixed, Maximized, Fullscreen, and 'Inherent / default'
-// Platform part, you don't, you only need Fixed, Maximized, Fullscreen
-
 impl Window {
     pub(crate) fn new(
         handle: AnyWindowHandle,
@@ -386,7 +383,7 @@ impl Window {
         let last_input_timestamp = Rc::new(Cell::new(Instant::now()));
 
         if fullscreen {
-            platform_window.toggle_full_screen();
+            platform_window.toggle_fullscreen();
         }
 
         platform_window.on_close(Box::new({
@@ -807,8 +804,8 @@ impl<'a> WindowContext<'a> {
     }
 
     /// Retusn whether or not the window is currently fullscreen
-    pub fn is_full_screen(&self) -> bool {
-        self.window.platform_window.is_full_screen()
+    pub fn is_fullscreen(&self) -> bool {
+        self.window.platform_window.is_fullscreen()
     }
 
     fn appearance_changed(&mut self) {
@@ -1481,8 +1478,8 @@ impl<'a> WindowContext<'a> {
     }
 
     /// Toggle full screen status on the current window at the platform level.
-    pub fn toggle_full_screen(&self) {
-        self.window.platform_window.toggle_full_screen();
+    pub fn toggle_fullscreen(&self) {
+        self.window.platform_window.toggle_fullscreen();
     }
 
     /// Present a platform dialog.

crates/workspace/src/persistence.rs 🔗

@@ -138,6 +138,7 @@ define_connection! {
     //   window_width: Option<f32>, // WindowBounds::Fixed RectF width
     //   window_height: Option<f32>, // WindowBounds::Fixed RectF height
     //   display: Option<Uuid>, // Display id
+    //   fullscreen: Option<bool>, // Is the window fullscreen?
     // )
     //
     // pane_groups(
@@ -274,7 +275,11 @@ define_connection! {
     // Add pane group flex data
     sql!(
         ALTER TABLE pane_groups ADD COLUMN flexes TEXT;
-    )
+    ),
+    // Add fullscreen field to workspace
+    sql!(
+        ALTER TABLE workspaces ADD COLUMN fullscreen INTEGER; //bool
+    ),
     ];
 }
 
@@ -290,11 +295,12 @@ impl WorkspaceDb {
 
         // Note that we re-assign the workspace_id here in case it's empty
         // and we've grabbed the most recent workspace
-        let (workspace_id, workspace_location, bounds, display, docks): (
+        let (workspace_id, workspace_location, bounds, display, fullscreen, docks): (
             WorkspaceId,
             WorkspaceLocation,
             Option<SerializedWindowsBounds>,
             Option<Uuid>,
+            Option<bool>,
             DockStructure,
         ) = self
             .select_row_bound(sql! {
@@ -307,6 +313,7 @@ impl WorkspaceDb {
                     window_width,
                     window_height,
                     display,
+                    fullscreen,
                     left_dock_visible,
                     left_dock_active_panel,
                     left_dock_zoom,
@@ -332,6 +339,7 @@ impl WorkspaceDb {
                 .context("Getting center group")
                 .log_err()?,
             bounds: bounds.map(|bounds| bounds.0),
+            fullscreen: fullscreen.unwrap_or(false),
             display,
             docks,
         })
@@ -412,6 +420,16 @@ impl WorkspaceDb {
         }
     }
 
+    query! {
+        pub fn last_monitor() -> Result<Option<Uuid>> {
+            SELECT display
+            FROM workspaces
+            WHERE workspace_location IS NOT NULL
+            ORDER BY timestamp DESC
+            LIMIT 1
+        }
+    }
+
     query! {
         pub async fn delete_workspace_by_id(id: WorkspaceId) -> Result<()> {
             DELETE FROM workspaces
@@ -648,6 +666,14 @@ impl WorkspaceDb {
             WHERE workspace_id = ?1
         }
     }
+
+    query! {
+        pub(crate) async fn set_fullscreen(workspace_id: WorkspaceId, fullscreen: bool) -> Result<()> {
+            UPDATE workspaces
+            SET fullscreen = ?2
+            WHERE workspace_id = ?1
+        }
+    }
 }
 
 #[cfg(test)]
@@ -733,6 +759,7 @@ mod tests {
             bounds: Default::default(),
             display: Default::default(),
             docks: Default::default(),
+            fullscreen: false,
         };
 
         let workspace_2 = SerializedWorkspace {
@@ -742,6 +769,7 @@ mod tests {
             bounds: Default::default(),
             display: Default::default(),
             docks: Default::default(),
+            fullscreen: false,
         };
 
         db.save_workspace(workspace_1.clone()).await;
@@ -840,6 +868,7 @@ mod tests {
             bounds: Default::default(),
             display: Default::default(),
             docks: Default::default(),
+            fullscreen: false,
         };
 
         db.save_workspace(workspace.clone()).await;
@@ -868,6 +897,7 @@ mod tests {
             bounds: Default::default(),
             display: Default::default(),
             docks: Default::default(),
+            fullscreen: false,
         };
 
         let mut workspace_2 = SerializedWorkspace {
@@ -877,6 +907,7 @@ mod tests {
             bounds: Default::default(),
             display: Default::default(),
             docks: Default::default(),
+            fullscreen: false,
         };
 
         db.save_workspace(workspace_1.clone()).await;
@@ -913,6 +944,7 @@ mod tests {
             bounds: Default::default(),
             display: Default::default(),
             docks: Default::default(),
+            fullscreen: false,
         };
 
         db.save_workspace(workspace_3.clone()).await;
@@ -946,6 +978,7 @@ mod tests {
             bounds: Default::default(),
             display: Default::default(),
             docks: Default::default(),
+            fullscreen: false,
         }
     }
 

crates/workspace/src/persistence/model.rs 🔗

@@ -70,6 +70,7 @@ pub(crate) struct SerializedWorkspace {
     pub(crate) location: WorkspaceLocation,
     pub(crate) center_group: SerializedPaneGroup,
     pub(crate) bounds: Option<Bounds<GlobalPixels>>,
+    pub(crate) fullscreen: bool,
     pub(crate) display: Option<Uuid>,
     pub(crate) docks: DockStructure,
 }

crates/workspace/src/workspace.rs 🔗

@@ -694,15 +694,28 @@ impl Workspace {
                     let display_bounds = display.bounds();
                     window_bounds.origin.x -= display_bounds.origin.x;
                     window_bounds.origin.y -= display_bounds.origin.y;
+                    let fullscreen = cx.is_fullscreen();
 
                     if let Some(display_uuid) = display.uuid().log_err() {
-                        cx.background_executor()
-                            .spawn(DB.set_window_bounds(
-                                workspace_id,
-                                SerializedWindowsBounds(window_bounds),
-                                display_uuid,
-                            ))
-                            .detach_and_log_err(cx);
+                        // Only update the window bounds when not full screen,
+                        // so we can remember the last non-fullscreen bounds
+                        // across restarts
+                        if fullscreen {
+                            cx.background_executor()
+                                .spawn(DB.set_fullscreen(workspace_id, true))
+                                .detach_and_log_err(cx);
+                        } else {
+                            cx.background_executor()
+                                .spawn(DB.set_fullscreen(workspace_id, false))
+                                .detach_and_log_err(cx);
+                            cx.background_executor()
+                                .spawn(DB.set_window_bounds(
+                                    workspace_id,
+                                    SerializedWindowsBounds(window_bounds),
+                                    display_uuid,
+                                ))
+                                .detach_and_log_err(cx);
+                        }
                     }
                 }
                 cx.notify();
@@ -834,36 +847,41 @@ impl Workspace {
                 window
             } else {
                 let window_bounds_override = window_bounds_env_override(&cx);
-                let (bounds, display) = if let Some(bounds) = window_bounds_override {
-                    (Some(bounds), None)
-                } else {
-                    serialized_workspace
-                        .as_ref()
-                        .and_then(|serialized_workspace| {
-                            let serialized_display = serialized_workspace.display?;
-                            let mut bounds = serialized_workspace.bounds?;
-
-                            // Stored bounds are relative to the containing display.
-                            // So convert back to global coordinates if that screen still exists
-                            let screen = cx
-                                .update(|cx| {
-                                    cx.displays().into_iter().find(|display| {
-                                        display.uuid().ok() == Some(serialized_display)
-                                    })
-                                })
-                                .ok()??;
-                            let screen_bounds = screen.bounds();
-                            bounds.origin.x += screen_bounds.origin.x;
-                            bounds.origin.y += screen_bounds.origin.y;
 
-                            Some((bounds, serialized_display))
+                let (bounds, display, fullscreen) = if let Some(bounds) = window_bounds_override {
+                    (Some(bounds), None, false)
+                } else if let Some((serialized_display, mut bounds, fullscreen)) =
+                    serialized_workspace.as_ref().and_then(|workspace| {
+                        Some((workspace.display?, workspace.bounds?, workspace.fullscreen))
+                    })
+                {
+                    // Stored bounds are relative to the containing display.
+                    // So convert back to global coordinates if that screen still exists
+                    let screen_bounds = cx
+                        .update(|cx| {
+                            cx.displays()
+                                .into_iter()
+                                .find(|display| display.uuid().ok() == Some(serialized_display))
                         })
-                        .unzip()
+                        .ok()
+                        .flatten()
+                        .map(|screen| screen.bounds());
+
+                    if let Some(screen_bounds) = screen_bounds {
+                        bounds.origin.x += screen_bounds.origin.x;
+                        bounds.origin.y += screen_bounds.origin.y;
+                    }
+
+                    (Some(bounds), Some(serialized_display), fullscreen)
+                } else {
+                    let display = DB.last_monitor().log_err().flatten();
+                    (None, display, false)
                 };
 
                 // Use the serialized workspace to construct the new window
                 let mut options = cx.update(|cx| (app_state.build_window_options)(display, cx))?;
                 options.bounds = bounds;
+                options.fullscreen = fullscreen;
                 cx.open_window(options, {
                     let app_state = app_state.clone();
                     let project_handle = project_handle.clone();
@@ -3420,6 +3438,7 @@ impl Workspace {
                     bounds: Default::default(),
                     display: Default::default(),
                     docks,
+                    fullscreen: cx.is_fullscreen(),
                 };
 
                 cx.spawn(|_| persistence::DB.save_workspace(serialized_workspace))

crates/zed/src/zed.rs 🔗

@@ -222,7 +222,7 @@ pub fn initialize_workspace(app_state: Arc<AppState>, cx: &mut AppContext) {
                 cx.zoom_window();
             })
             .register_action(|_, _: &ToggleFullScreen, cx| {
-                cx.toggle_full_screen();
+                cx.toggle_fullscreen();
             })
             .register_action(|_, action: &OpenZedUrl, cx| {
                 OpenListener::global(cx).open_urls(vec![action.url.clone()])