xwm: Simplify override-redirect handling

src/input/mod.rs

@@ -8,7 +8,7 @@ use crate::{
             floating::SeatMoveGrabState,
             tiling::{Direction, FocusResult},
         },
-        Ordering, OverrideRedirectWindow, Workspace,
+        Workspace,
     }, // shell::grabs::SeatMoveGrabState
     state::Common,
     utils::prelude::*,
@@ -16,12 +16,9 @@ use crate::{
 };
 use cosmic_protocols::screencopy::v1::server::zcosmic_screencopy_session_v1::InputType;
 use smithay::{
-    backend::{
+    backend::input::{
-        input::{
+        Axis, AxisSource, Device, DeviceCapability, InputBackend, InputEvent, KeyState,
-            Axis, AxisSource, Device, DeviceCapability, InputBackend, InputEvent, KeyState,
+        PointerAxisEvent,
-            PointerAxisEvent,
-        },
-        renderer::element::Id,
     },
     desktop::{layer_map_for_output, space::SpaceElement, WindowSurfaceType},
     input::{
@@ -36,6 +33,7 @@ use smithay::{
         keyboard_shortcuts_inhibit::KeyboardShortcutsInhibitorSeat, seat::WaylandFocus,
         shell::wlr_layer::Layer as WlrLayer,
     },
+    xwayland::X11Surface,
 };
 
 use std::{cell::RefCell, collections::HashMap};
@@ -932,7 +930,7 @@ impl State {
         relative_pos: Point<f64, Logical>,
         output: &Output,
         output_geo: Rectangle<i32, Logical>,
-        override_redirect_windows: &[OverrideRedirectWindow],
+        override_redirect_windows: &[X11Surface],
         workspace: &Workspace,
     ) -> Option<(PointerFocusTarget, Point<i32, Logical>)> {
         let layers = layer_map_for_output(output);
@@ -946,6 +944,12 @@ impl State {
                     return Some((layer.clone().into(), output_geo.loc + layer_loc));
                 }
             }
+            if let Some(or) = override_redirect_windows
+                .iter()
+                .find(|or| or.is_in_input_region(&(global_pos - or.geometry().loc.to_f64())))
+            {
+                return Some((or.clone().into(), or.geometry().loc));
+            }
             Some((window.clone().into(), output_geo.loc))
         } else {
             if let Some(layer) = layers
@@ -960,54 +964,18 @@ impl State {
                     return Some((layer.clone().into(), output_geo.loc + layer_loc));
                 }
             }
-            if let Some(or) = override_redirect_windows.iter().find(|or| {
+            if let Some(or) = override_redirect_windows
-                or.above == Ordering::Above
+                .iter()
-                    && or
+                .find(|or| or.is_in_input_region(&(global_pos - or.geometry().loc.to_f64())))
-                        .surface
+            {
-                        .is_in_input_region(&(global_pos - or.surface.geometry().loc.to_f64()))
+                return Some((or.clone().into(), or.geometry().loc));
-            }) {
-                return Some((or.surface.clone().into(), or.surface.geometry().loc));
             }
             if let Some((mapped, loc)) = workspace.element_under(relative_pos) {
-                let filter = workspace
-                    .mapped()
-                    .skip_while(|m| *m != mapped)
-                    .collect::<Vec<_>>();
-                if let Some(or) = override_redirect_windows
-                    .iter()
-                    .filter(|or| {
-                        if let Ordering::AboveWindow(w) = &or.above {
-                            !filter.iter().any(|f| {
-                                f.wl_surface()
-                                    .map(|s| Id::from_wayland_resource(&s))
-                                    .as_ref()
-                                    == Some(&w)
-                            })
-                        } else {
-                            false
-                        }
-                    })
-                    .find(|or| {
-                        or.surface
-                            .is_in_input_region(&(global_pos - or.surface.geometry().loc.to_f64()))
-                    })
-                {
-                    return Some((or.surface.clone().into(), or.surface.geometry().loc));
-                }
-
                 return Some((
                     mapped.clone().into(),
                     loc + (global_pos - relative_pos).to_i32_round(),
                 ));
             }
-            if let Some(or) = override_redirect_windows.iter().find(|or| {
-                or.above == Ordering::Below
-                    && or
-                        .surface
-                        .is_in_input_region(&(global_pos - or.surface.geometry().loc.to_f64()))
-            }) {
-                return Some((or.surface.clone().into(), or.surface.geometry().loc));
-            }
             if let Some(layer) = layers
                 .layer_under(WlrLayer::Bottom, relative_pos)
                 .or_else(|| layers.layer_under(WlrLayer::Background, relative_pos))

src/shell/mod.rs

@@ -3,7 +3,6 @@ use std::{cell::RefCell, collections::HashMap};
 
 use cosmic_protocols::workspace::v1::server::zcosmic_workspace_handle_v1::State as WState;
 use smithay::{
-    backend::renderer::element::Id,
     desktop::{
         layer_map_for_output, space::SpaceElement, LayerSurface, PopupManager, WindowSurfaceType,
     },
@@ -61,7 +60,7 @@ pub struct Shell {
     pub tiling_enabled: bool,
     pub pending_windows: Vec<(CosmicSurface, Seat<State>)>,
     pub pending_layers: Vec<(LayerSurface, Output, Seat<State>)>,
-    pub override_redirect_windows: Vec<OverrideRedirectWindow>,
+    pub override_redirect_windows: Vec<X11Surface>,
 
     // wayland_state
     pub layer_shell_state: WlrLayerShellState,
@@ -71,19 +70,6 @@ pub struct Shell {
     pub workspace_state: WorkspaceState<State>,
 }
 
-#[derive(Debug)]
-pub struct OverrideRedirectWindow {
-    pub surface: X11Surface,
-    pub above: Ordering,
-}
-
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub enum Ordering {
-    Above,
-    AboveWindow(Id),
-    Below,
-}
-
 #[derive(Debug)]
 pub struct WorkspaceSet {
     active: usize,
@@ -891,8 +877,8 @@ impl Shell {
                 self.outputs()
                     .filter(|o| {
                         self.override_redirect_windows.iter().any(|or| {
-                            if or.surface.wl_surface().as_ref() == Some(surface) {
+                            if or.wl_surface().as_ref() == Some(surface) {
-                                or.surface.geometry().intersection(o.geometry()).is_some()
+                                or.geometry().intersection(o.geometry()).is_some()
                             } else {
                                 false
                             }
@@ -1038,11 +1024,10 @@ impl Shell {
             map.cleanup();
         }
 
-        self.override_redirect_windows
+        self.override_redirect_windows.retain(|or| or.alive());
-            .retain(|or| or.surface.alive());
         self.override_redirect_windows
             .iter()
-            .for_each(|or| or.surface.refresh());
+            .for_each(|or| or.refresh());
 
         self.toplevel_info_state
             .refresh(Some(&self.workspace_state));
@@ -1122,14 +1107,7 @@ impl Shell {
             window.output_enter(&output, overlap);
         }
 
-        state
+        state.common.shell.override_redirect_windows.push(window);
-            .common
-            .shell
-            .override_redirect_windows
-            .push(OverrideRedirectWindow {
-                surface: window,
-                above: Ordering::Above,
-            });
     }
 
     pub fn map_layer(state: &mut State, layer_surface: &LayerSurface) {

src/shell/workspace.rs

@@ -29,6 +29,7 @@ use smithay::{
     reexports::wayland_server::protocol::wl_surface::WlSurface,
     utils::{Buffer as BufferCoords, IsAlive, Logical, Physical, Point, Rectangle, Scale, Size},
     wayland::{seat::WaylandFocus, shell::wlr_layer::Layer},
+    xwayland::X11Surface,
 };
 use std::collections::HashMap;
 
@@ -36,7 +37,7 @@ use super::{
     element::CosmicMapped,
     focus::{FocusStack, FocusStackMut},
     grabs::{ResizeEdge, ResizeGrab},
-    CosmicMappedRenderElement, CosmicSurface, Ordering,
+    CosmicMappedRenderElement, CosmicSurface,
 };
 
 #[derive(Debug)]
@@ -395,7 +396,7 @@ impl Workspace {
         &self,
         renderer: &mut R,
         output: &Output,
-        override_redirect_windows: &[super::OverrideRedirectWindow],
+        override_redirect_windows: &[X11Surface],
         xwm_state: impl Iterator<Item = &'a mut XWaylandState>,
     ) -> Result<Vec<WorkspaceRenderElement<R>>, OutputNotMapped>
     where
@@ -433,19 +434,12 @@ impl Workspace {
             render_elements.extend(
                 override_redirect_windows
                     .iter()
-                    .filter(|or| {
+                    .filter(|or| or.geometry().intersection(output.geometry()).is_some())
-                        or.above != Ordering::Below
-                            && or
-                                .surface
-                                .geometry()
-                                .intersection(output.geometry())
-                                .is_some()
-                    })
                     .flat_map(|or| {
                         AsRenderElements::<R>::render_elements::<WorkspaceRenderElement<R>>(
-                            &or.surface,
+                            or,
                             renderer,
-                            (or.surface.geometry().loc - output.geometry().loc)
+                            (or.geometry().loc - output.geometry().loc)
                                 .to_physical_precise_round(output_scale),
                             Scale::from(output_scale),
                         )
@@ -503,25 +497,16 @@ impl Workspace {
                 lower
             };
 
-            let mut window_elements = Vec::new();
-
             // OR windows above all
-            window_elements.extend(
+            render_elements.extend(
                 override_redirect_windows
                     .iter()
-                    .filter(|or| {
+                    .filter(|or| or.geometry().intersection(output.geometry()).is_some())
-                        or.above == Ordering::Above
-                            && or
-                                .surface
-                                .geometry()
-                                .intersection(output.geometry())
-                                .is_some()
-                    })
                     .flat_map(|or| {
                         AsRenderElements::<R>::render_elements::<WorkspaceRenderElement<R>>(
-                            &or.surface,
+                            or,
                             renderer,
-                            (or.surface.geometry().loc - output.geometry().loc)
+                            (or.geometry().loc - output.geometry().loc)
                                 .to_physical_precise_round(output_scale),
                             Scale::from(output_scale),
                         )
@@ -529,7 +514,7 @@ impl Workspace {
             );
 
             // floating surfaces
-            window_elements.extend(
+            render_elements.extend(
                 self.floating_layer
                     .render_output::<R>(renderer, output)?
                     .into_iter()
@@ -537,66 +522,16 @@ impl Workspace {
             );
 
             //tiling surfaces
-            window_elements.extend(
+            render_elements.extend(
                 self.tiling_layer
                     .render_output::<R>(renderer, output)?
                     .into_iter()
                     .map(WorkspaceRenderElement::from),
             );
 
-            // Sort other OR windows in between
-            for or in override_redirect_windows.iter().filter(|or| {
-                matches!(or.above, Ordering::AboveWindow(_))
-                    && or
-                        .surface
-                        .geometry()
-                        .intersection(output.geometry())
-                        .is_some()
-            }) {
-                let pos = window_elements
-                    .iter()
-                    .position(|w| Ordering::AboveWindow(w.id().clone()) == or.above)
-                    .unwrap_or(0);
-                for element in AsRenderElements::<R>::render_elements::<WorkspaceRenderElement<R>>(
-                    &or.surface,
-                    renderer,
-                    (or.surface.geometry().loc - output.geometry().loc)
-                        .to_physical_precise_round(output_scale),
-                    Scale::from(output_scale),
-                )
-                .into_iter()
-                .rev()
-                {
-                    window_elements.insert(pos, element);
-                }
-            }
-
-            // OR windows below all
-            window_elements.extend(
-                override_redirect_windows
-                    .iter()
-                    .filter(|or| {
-                        or.above == Ordering::Below
-                            && or
-                                .surface
-                                .geometry()
-                                .intersection(output.geometry())
-                                .is_some()
-                    })
-                    .flat_map(|or| {
-                        AsRenderElements::<R>::render_elements::<WorkspaceRenderElement<R>>(
-                            &or.surface,
-                            renderer,
-                            (or.surface.geometry().loc - output.geometry().loc)
-                                .to_physical_precise_round(output_scale),
-                            Scale::from(output_scale),
-                        )
-                    }),
-            );
-
             for xwm in xwm_state.flat_map(|state| state.xwm.as_mut()) {
                 if let Err(err) =
-                    xwm.update_stacking_order_upwards(window_elements.iter().rev().map(|e| e.id()))
+                    xwm.update_stacking_order_upwards(render_elements.iter().rev().map(|e| e.id()))
                 {
                     slog_scope::warn!(
                         "Failed to update Xwm ({:?}) stacking order: {}",
@@ -606,8 +541,6 @@ impl Workspace {
                 }
             }
 
-            render_elements.extend(window_elements.into_iter());
-
             // bottom and background layer surfaces
             {
                 render_elements.extend(

src/state.rs

@@ -421,7 +421,7 @@ impl Common {
         }
 
         self.shell.override_redirect_windows.iter().for_each(|or| {
-            if let Some(wl_surface) = or.surface.wl_surface() {
+            if let Some(wl_surface) = or.wl_surface() {
                 with_surfaces_surface_tree(&wl_surface, |surface, states| {
                     update_surface_primary_scanout_output(
                         surface,
@@ -475,7 +475,7 @@ impl Common {
         });
 
         self.shell.override_redirect_windows.iter().for_each(|or| {
-            if let Some(wl_surface) = or.surface.wl_surface() {
+            if let Some(wl_surface) = or.wl_surface() {
                 take_presentation_feedback_surface_tree(
                     &wl_surface,
                     &mut output_presentation_feedback,

src/xwayland.rs

@@ -2,13 +2,13 @@ use std::ffi::OsString;
 
 use crate::{
     backend::render::cursor::Cursor,
-    shell::{CosmicSurface, Ordering, Shell},
+    shell::{CosmicSurface, Shell},
     state::{Data, State},
     utils::prelude::*,
     wayland::{handlers::screencopy::PendingScreencopyBuffers, protocols::screencopy::SessionType},
 };
 use smithay::{
-    backend::{drm::DrmNode, renderer::element::Id},
+    backend::drm::DrmNode,
     desktop::space::SpaceElement,
     reexports::x11rb::protocol::xproto::Window as X11Window,
     utils::{Logical, Point, Rectangle, Size},
@@ -190,7 +190,7 @@ impl XwmHandler for Data {
             .shell
             .override_redirect_windows
             .iter()
-            .any(|or| or.surface == window)
+            .any(|or| or == &window)
         {
             return;
         }
@@ -203,7 +203,7 @@ impl XwmHandler for Data {
                 .common
                 .shell
                 .override_redirect_windows
-                .retain(|or| or.surface != window);
+                .retain(|or| or != &window);
         } else if let Some((element, space)) = self
             .state
             .common
@@ -326,54 +326,20 @@ impl XwmHandler for Data {
         above: Option<X11Window>,
     ) {
         if window.is_override_redirect() {
-            let ordering = match above {
+            if let Some(id) = above {
-                None => Ordering::Below,
+                let or_windows = &mut self.state.common.shell.override_redirect_windows;
-                Some(id) => self
+                if let Some(own_pos) = or_windows.iter().position(|or| or == &window) {
-                    .state
+                    let compare_pos = or_windows
-                    .common
+                        .iter()
-                    .shell
+                        .position(|or| or.window_id() == id)
-                    .override_redirect_windows
+                        .unwrap_or(0);
-                    .iter()
+                    if compare_pos > own_pos {
-                    .find_map(|or| {
+                        let this = or_windows.remove(own_pos);
-                        if or.surface.window_id() == id {
+                        or_windows.insert(compare_pos, this);
-                            or.surface
+                    }
-                                .wl_surface()
+                }
-                                .map(|s| Id::from_wayland_resource(&s))
-                        } else {
-                            None
-                        }
-                    })
-                    .or_else(|| {
-                        self.state
-                            .common
-                            .shell
-                            .workspaces
-                            .spaces()
-                            .flat_map(|s| s.windows())
-                            .find_map(|w| {
-                                if let CosmicSurface::X11(w) = w {
-                                    if w.window_id() == id || w.mapped_window_id() == Some(id) {
-                                        return w
-                                            .wl_surface()
-                                            .map(|s| Id::from_wayland_resource(&s));
-                                    }
-                                }
-                                None
-                            })
-                    })
-                    .map(Ordering::AboveWindow)
-                    .unwrap_or(Ordering::Above),
-            };
-            if let Some(or) = self
-                .state
-                .common
-                .shell
-                .override_redirect_windows
-                .iter_mut()
-                .find(|or| or.surface == window)
-            {
-                or.above = ordering;
             }
+
             let geo = window.geometry();
             for (output, overlap) in self.state.common.shell.outputs().cloned().map(|o| {
                 let intersection = o.geometry().intersection(geo);