Fix issues where screen and window sizes contained Pixels, but were declared as DevicePixels (#12991)

On most platforms, things were working correctly, but had the wrong
type. On X11, there were some problems with window and display size
calculations.

Release Notes:

- Fixed issues with window positioning on X11

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
Co-authored-by: Mikayla <mikayla@zed.dev>

crates/gpui/src/platform/linux/wayland/client.rs

@@ -557,7 +557,7 @@ impl LinuxClient for WaylandClient {
                 Rc::new(WaylandDisplay {
                     id: id.clone(),
                     name: output.name.clone(),
-                    bounds: output.bounds,
+                    bounds: output.bounds.to_pixels(output.scale as f32),
                 }) as Rc<dyn PlatformDisplay>
             })
             .collect()
@@ -573,7 +573,7 @@ impl LinuxClient for WaylandClient {
                     Rc::new(WaylandDisplay {
                         id: object_id.clone(),
                         name: output.name.clone(),
-                        bounds: output.bounds,
+                        bounds: output.bounds.to_pixels(output.scale as f32),
                     }) as Rc<dyn PlatformDisplay>
                 })
             })

crates/gpui/src/platform/linux/wayland/display.rs

@@ -6,14 +6,14 @@ use std::{
 use uuid::Uuid;
 use wayland_backend::client::ObjectId;
 
-use crate::{Bounds, DevicePixels, DisplayId, PlatformDisplay};
+use crate::{Bounds, DisplayId, Pixels, PlatformDisplay};
 
 #[derive(Debug, Clone)]
 pub(crate) struct WaylandDisplay {
     /// The ID of the wl_output object
     pub id: ObjectId,
     pub name: Option<String>,
-    pub bounds: Bounds<DevicePixels>,
+    pub bounds: Bounds<Pixels>,
 }
 
 impl Hash for WaylandDisplay {
@@ -35,7 +35,7 @@ impl PlatformDisplay for WaylandDisplay {
         }
     }
 
-    fn bounds(&self) -> Bounds<DevicePixels> {
+    fn bounds(&self) -> Bounds<Pixels> {
         self.bounds
     }
 }

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

@@ -1,6 +1,5 @@
 use std::cell::{Ref, RefCell, RefMut};
 use std::ffi::c_void;
-use std::num::NonZeroU32;
 use std::ptr::NonNull;
 use std::rc::Rc;
 use std::sync::Arc;
@@ -26,9 +25,9 @@ use crate::platform::linux::wayland::serial::SerialKind;
 use crate::platform::{PlatformAtlas, PlatformInputHandler, PlatformWindow};
 use crate::scene::Scene;
 use crate::{
-    px, size, AnyWindowHandle, Bounds, DevicePixels, Globals, Modifiers, Output, Pixels,
-    PlatformDisplay, PlatformInput, Point, PromptLevel, Size, WaylandClientStatePtr,
-    WindowAppearance, WindowBackgroundAppearance, WindowBounds, WindowParams,
+    px, size, AnyWindowHandle, Bounds, Globals, Modifiers, Output, Pixels, PlatformDisplay,
+    PlatformInput, Point, PromptLevel, Size, WaylandClientStatePtr, WindowAppearance,
+    WindowBackgroundAppearance, WindowBounds, WindowParams,
 };
 
 #[derive(Default)]
@@ -76,13 +75,13 @@ pub struct WaylandWindowState {
     display: Option<(ObjectId, Output)>,
     globals: Globals,
     renderer: BladeRenderer,
-    bounds: Bounds<u32>,
+    bounds: Bounds<Pixels>,
     scale: f32,
     input_handler: Option<PlatformInputHandler>,
     decoration_state: WaylandDecorationState,
     fullscreen: bool,
     maximized: bool,
-    windowed_bounds: Bounds<DevicePixels>,
+    windowed_bounds: Bounds<Pixels>,
     client: WaylandClientStatePtr,
     handle: AnyWindowHandle,
     active: bool,
@@ -108,8 +107,6 @@ impl WaylandWindowState {
         globals: Globals,
         options: WindowParams,
     ) -> anyhow::Result<Self> {
-        let bounds = options.bounds.map(|p| p.0 as u32);
-
         let raw = RawWindow {
             window: surface.id().as_ptr().cast::<c_void>(),
             display: surface
@@ -134,8 +131,8 @@ impl WaylandWindowState {
         );
         let config = BladeSurfaceConfig {
             size: gpu::Extent {
-                width: bounds.size.width,
-                height: bounds.size.height,
+                width: options.bounds.size.width.0 as u32,
+                height: options.bounds.size.height.0 as u32,
                 depth: 1,
             },
             transparent: options.window_background != WindowBackgroundAppearance::Opaque,
@@ -153,7 +150,7 @@ impl WaylandWindowState {
             outputs: HashMap::default(),
             display: None,
             renderer: BladeRenderer::new(gpu, config),
-            bounds,
+            bounds: options.bounds,
             scale: 1.0,
             input_handler: None,
             decoration_state: WaylandDecorationState::Client,
@@ -349,10 +346,16 @@ impl WaylandWindowStatePtr {
     pub fn handle_toplevel_event(&self, event: xdg_toplevel::Event) -> bool {
         match event {
             xdg_toplevel::Event::Configure {
-                mut width,
-                mut height,
+                width,
+                height,
                 states,
             } => {
+                let mut size = if width == 0 || height == 0 {
+                    None
+                } else {
+                    Some(size(px(width as f32), px(height as f32)))
+                };
+
                 let fullscreen = states.contains(&(xdg_toplevel::State::Fullscreen as u8));
                 let maximized = states.contains(&(xdg_toplevel::State::Maximized as u8));
 
@@ -362,19 +365,20 @@ impl WaylandWindowStatePtr {
                 state.maximized = maximized;
 
                 if got_unmaximized {
-                    width = state.windowed_bounds.size.width.0;
-                    height = state.windowed_bounds.size.height.0;
-                } else if width != 0 && height != 0 && !fullscreen && !maximized {
-                    state.windowed_bounds = Bounds {
-                        origin: Point::default(),
-                        size: size(width.into(), height.into()),
-                    };
+                    size = Some(state.windowed_bounds.size);
+                } else if !fullscreen && !maximized {
+                    if let Some(size) = size {
+                        state.windowed_bounds = Bounds {
+                            origin: Point::default(),
+                            size,
+                        };
+                    }
                 }
 
-                let width = NonZeroU32::new(width as u32);
-                let height = NonZeroU32::new(height as u32);
                 drop(state);
-                self.resize(width, height);
+                if let Some(size) = size {
+                    self.resize(size);
+                }
 
                 false
             }
@@ -483,63 +487,43 @@ impl WaylandWindowStatePtr {
         bounds
     }
 
-    pub fn set_size_and_scale(
-        &self,
-        width: Option<NonZeroU32>,
-        height: Option<NonZeroU32>,
-        scale: Option<f32>,
-    ) {
-        let (width, height, scale) = {
+    pub fn set_size_and_scale(&self, size: Option<Size<Pixels>>, scale: Option<f32>) {
+        let (size, scale) = {
             let mut state = self.state.borrow_mut();
-            if width.map_or(true, |width| width.get() == state.bounds.size.width)
-                && height.map_or(true, |height| height.get() == state.bounds.size.height)
+            if size.map_or(true, |size| size == state.bounds.size)
                 && scale.map_or(true, |scale| scale == state.scale)
             {
                 return;
             }
-            if let Some(width) = width {
-                state.bounds.size.width = width.get();
-            }
-            if let Some(height) = height {
-                state.bounds.size.height = height.get();
+            if let Some(size) = size {
+                state.bounds.size = size;
             }
             if let Some(scale) = scale {
                 state.scale = scale;
             }
-            let width = state.bounds.size.width;
-            let height = state.bounds.size.height;
-            let scale = state.scale;
-            state.renderer.update_drawable_size(size(
-                width as f64 * scale as f64,
-                height as f64 * scale as f64,
-            ));
-            (width, height, scale)
+            let device_bounds = state.bounds.to_device_pixels(state.scale);
+            state.renderer.update_drawable_size(device_bounds.size);
+            (state.bounds.size, state.scale)
         };
 
         if let Some(ref mut fun) = self.callbacks.borrow_mut().resize {
-            fun(
-                Size {
-                    width: px(width as f32),
-                    height: px(height as f32),
-                },
-                scale,
-            );
+            fun(size, scale);
         }
 
         {
             let state = self.state.borrow();
             if let Some(viewport) = &state.viewport {
-                viewport.set_destination(width as i32, height as i32);
+                viewport.set_destination(size.width.0 as i32, size.height.0 as i32);
             }
         }
     }
 
-    pub fn resize(&self, width: Option<NonZeroU32>, height: Option<NonZeroU32>) {
-        self.set_size_and_scale(width, height, None);
+    pub fn resize(&self, size: Size<Pixels>) {
+        self.set_size_and_scale(Some(size), None);
     }
 
     pub fn rescale(&self, scale: f32) {
-        self.set_size_and_scale(None, None, Some(scale));
+        self.set_size_and_scale(None, Some(scale));
     }
 
     /// Notifies the window of the state of the decorations.
@@ -625,8 +609,8 @@ impl rwh::HasDisplayHandle for WaylandWindow {
 }
 
 impl PlatformWindow for WaylandWindow {
-    fn bounds(&self) -> Bounds<DevicePixels> {
-        self.borrow().bounds.map(|p| DevicePixels(p as i32))
+    fn bounds(&self) -> Bounds<Pixels> {
+        self.borrow().bounds
     }
 
     fn is_maximized(&self) -> bool {
@@ -640,16 +624,13 @@ impl PlatformWindow for WaylandWindow {
         } else if state.maximized {
             WindowBounds::Maximized(state.windowed_bounds)
         } else {
-            WindowBounds::Windowed(state.bounds.map(|p| DevicePixels(p as i32)))
+            drop(state);
+            WindowBounds::Windowed(self.bounds())
         }
     }
 
     fn content_size(&self) -> Size<Pixels> {
-        let state = self.borrow();
-        Size {
-            width: Pixels(state.bounds.size.width as f32),
-            height: Pixels(state.bounds.size.height as f32),
-        }
+        self.borrow().bounds.size
     }
 
     fn scale_factor(&self) -> f32 {
@@ -661,11 +642,12 @@ impl PlatformWindow for WaylandWindow {
     }
 
     fn display(&self) -> Option<Rc<dyn PlatformDisplay>> {
-        self.borrow().display.as_ref().map(|(id, display)| {
+        let state = self.borrow();
+        state.display.as_ref().map(|(id, display)| {
             Rc::new(WaylandDisplay {
                 id: id.clone(),
                 name: display.name.clone(),
-                bounds: display.bounds,
+                bounds: display.bounds.to_pixels(state.scale),
             }) as Rc<dyn PlatformDisplay>
         })
     }

crates/gpui/src/platform/linux/x11/client.rs

@@ -908,8 +908,11 @@ impl LinuxClient for X11Client {
             .iter()
             .enumerate()
             .filter_map(|(root_id, _)| {
-                Some(Rc::new(X11Display::new(&state.xcb_connection, root_id)?)
-                    as Rc<dyn PlatformDisplay>)
+                Some(Rc::new(X11Display::new(
+                    &state.xcb_connection,
+                    state.scale_factor,
+                    root_id,
+                )?) as Rc<dyn PlatformDisplay>)
             })
             .collect()
     }
@@ -918,8 +921,12 @@ impl LinuxClient for X11Client {
         let state = self.0.borrow();
 
         Some(Rc::new(
-            X11Display::new(&state.xcb_connection, state.x_root_index)
-                .expect("There should always be a root index"),
+            X11Display::new(
+                &state.xcb_connection,
+                state.scale_factor,
+                state.x_root_index,
+            )
+            .expect("There should always be a root index"),
         ))
     }
 
@@ -928,6 +935,7 @@ impl LinuxClient for X11Client {
 
         Some(Rc::new(X11Display::new(
             &state.xcb_connection,
+            state.scale_factor,
             id.0 as usize,
         )?))
     }

crates/gpui/src/platform/linux/x11/display.rs

@@ -2,25 +2,29 @@ use anyhow::Result;
 use uuid::Uuid;
 use x11rb::{connection::Connection as _, xcb_ffi::XCBConnection};
 
-use crate::{Bounds, DevicePixels, DisplayId, PlatformDisplay, Size};
+use crate::{px, Bounds, DisplayId, Pixels, PlatformDisplay, Size};
 
 #[derive(Debug)]
 pub(crate) struct X11Display {
     x_screen_index: usize,
-    bounds: Bounds<DevicePixels>,
+    bounds: Bounds<Pixels>,
     uuid: Uuid,
 }
 
 impl X11Display {
-    pub(crate) fn new(xc: &XCBConnection, x_screen_index: usize) -> Option<Self> {
+    pub(crate) fn new(
+        xc: &XCBConnection,
+        scale_factor: f32,
+        x_screen_index: usize,
+    ) -> Option<Self> {
         let screen = xc.setup().roots.get(x_screen_index).unwrap();
         Some(Self {
-            x_screen_index: x_screen_index,
+            x_screen_index,
             bounds: Bounds {
                 origin: Default::default(),
                 size: Size {
-                    width: DevicePixels(screen.width_in_pixels as i32),
-                    height: DevicePixels(screen.height_in_pixels as i32),
+                    width: px(screen.width_in_pixels as f32 / scale_factor),
+                    height: px(screen.height_in_pixels as f32 / scale_factor),
                 },
             },
             uuid: Uuid::from_bytes([0; 16]),
@@ -37,7 +41,7 @@ impl PlatformDisplay for X11Display {
         Ok(self.uuid)
     }
 
-    fn bounds(&self) -> Bounds<DevicePixels> {
+    fn bounds(&self) -> Bounds<Pixels> {
         self.bounds
     }
 }

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

@@ -1,6 +1,6 @@
 use crate::{
     platform::blade::{BladeRenderer, BladeSurfaceConfig},
-    size, AnyWindowHandle, Bounds, DevicePixels, ForegroundExecutor, Modifiers, Pixels,
+    px, size, AnyWindowHandle, Bounds, DevicePixels, ForegroundExecutor, Modifiers, Pixels,
     PlatformAtlas, PlatformDisplay, PlatformInput, PlatformInputHandler, PlatformWindow, Point,
     PromptLevel, Scene, Size, WindowAppearance, WindowBackgroundAppearance, WindowBounds,
     WindowKind, WindowParams, X11ClientStatePtr,
@@ -162,7 +162,7 @@ pub struct X11WindowState {
     atoms: XcbAtoms,
     x_root_window: xproto::Window,
     _raw: RawWindow,
-    bounds: Bounds<i32>,
+    bounds: Bounds<Pixels>,
     scale_factor: f32,
     renderer: BladeRenderer,
     display: Rc<dyn PlatformDisplay>,
@@ -273,10 +273,10 @@ impl X11WindowState {
                 visual.depth,
                 x_window,
                 visual_set.root,
-                params.bounds.origin.x.0 as i16,
-                params.bounds.origin.y.0 as i16,
-                params.bounds.size.width.0 as u16,
-                params.bounds.size.height.0 as u16,
+                (params.bounds.origin.x.0 * scale_factor) as i16,
+                (params.bounds.origin.y.0 * scale_factor) as i16,
+                (params.bounds.size.width.0 * scale_factor) as u16,
+                (params.bounds.size.height.0 * scale_factor) as u16,
                 0,
                 xproto::WindowClass::INPUT_OUTPUT,
                 visual.id,
@@ -370,10 +370,12 @@ impl X11WindowState {
         Ok(Self {
             client,
             executor,
-            display: Rc::new(X11Display::new(xcb_connection, x_screen_index).unwrap()),
+            display: Rc::new(
+                X11Display::new(xcb_connection, scale_factor, x_screen_index).unwrap(),
+            ),
             _raw: raw,
             x_root_window: visual_set.root,
-            bounds: params.bounds.map(|v| v.0),
+            bounds: params.bounds,
             scale_factor,
             renderer: BladeRenderer::new(gpu, config),
             atoms: *atoms,
@@ -627,6 +629,7 @@ impl X11WindowStatePtr {
         let is_resize;
         {
             let mut state = self.state.borrow_mut();
+            let bounds = bounds.map(|f| px(f as f32 / state.scale_factor));
 
             is_resize = bounds.size.width != state.bounds.size.width
                 || bounds.size.height != state.bounds.size.height;
@@ -641,9 +644,10 @@ impl X11WindowStatePtr {
 
             let gpu_size = query_render_extent(&self.xcb_connection, self.x_window);
             if state.renderer.viewport_size() != gpu_size {
-                state
-                    .renderer
-                    .update_drawable_size(size(gpu_size.width as f64, gpu_size.height as f64));
+                state.renderer.update_drawable_size(size(
+                    DevicePixels(gpu_size.width as i32),
+                    DevicePixels(gpu_size.height as i32),
+                ));
                 resize_args = Some((state.content_size(), state.scale_factor));
             }
         }
@@ -678,8 +682,8 @@ impl X11WindowStatePtr {
 }
 
 impl PlatformWindow for X11Window {
-    fn bounds(&self) -> Bounds<DevicePixels> {
-        self.0.state.borrow().bounds.map(|v| v.into())
+    fn bounds(&self) -> Bounds<Pixels> {
+        self.0.state.borrow().bounds
     }
 
     fn is_maximized(&self) -> bool {
@@ -693,7 +697,11 @@ impl PlatformWindow for X11Window {
 
     fn window_bounds(&self) -> WindowBounds {
         let state = self.0.state.borrow();
-        WindowBounds::Windowed(state.bounds.map(|p| DevicePixels(p)))
+        if self.is_maximized() {
+            WindowBounds::Maximized(state.bounds)
+        } else {
+            WindowBounds::Windowed(state.bounds)
+        }
     }
 
     fn content_size(&self) -> Size<Pixels> {