space: Let downstream `RenderElements` hash `(&Space, &Output)`

src/backend/renderer/mod.rs

@@ -32,6 +32,7 @@ use crate::backend::egl::{
     Error as EglError,
 };
 
+#[cfg(feature = "wayland_frontend")]
 pub mod utils;
 
 #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]

src/backend/renderer/utils.rs

@@ -15,7 +15,7 @@ pub(crate) struct SurfaceState {
     pub(crate) buffer_scale: i32,
     pub(crate) buffer: Option<WlBuffer>,
     pub(crate) texture: Option<Box<dyn std::any::Any + 'static>>,
-    pub(crate) damage_seen: HashSet<(usize, *const ())>,
+    pub(crate) damage_seen: HashSet<crate::desktop::space::SpaceOutputHash>,
 }
 
 impl SurfaceState {
@@ -69,7 +69,6 @@ pub fn on_commit_buffer_handler(surface: &WlSurface) {
     }
 }
 
-/// TODO
 pub fn draw_surface_tree<R, E, F, T>(
     renderer: &mut R,
     frame: &mut F,

src/desktop/layer.rs

@@ -84,11 +84,14 @@ impl LayerMap {
         self.zone
     }
 
-    pub fn layer_geometry(&self, layer: &LayerSurface) -> Rectangle<i32, Logical> {
+    pub fn layer_geometry(&self, layer: &LayerSurface) -> Option<Rectangle<i32, Logical>> {
+        if !self.layers.contains(layer) {
+            return None;
+        }
         let mut bbox = layer.bbox_with_popups();
         let state = layer_state(layer);
         bbox.loc += state.location;
-        bbox
+        Some(bbox)
     }
 
     pub fn layer_under<P: Into<Point<f64, Logical>>>(
@@ -98,7 +101,7 @@ impl LayerMap {
     ) -> Option<&LayerSurface> {
         let point = point.into();
         self.layers_on(layer).rev().find(|l| {
-            let bbox = self.layer_geometry(l);
+            let bbox = self.layer_geometry(l).unwrap();
             bbox.to_f64().contains(point)
         })
     }

src/desktop/space/element.rs

@@ -4,7 +4,10 @@ use crate::{
     utils::{Logical, Point, Rectangle},
     wayland::output::Output,
 };
-use std::any::{Any, TypeId};
+use std::{
+    any::{Any, TypeId},
+    hash::{Hash, Hasher},
+};
 use wayland_server::protocol::wl_surface::WlSurface;
 
 pub trait RenderElement<R, F, E, T>
@@ -21,13 +24,15 @@ where
         std::any::Any::type_id(self)
     }
     fn geometry(&self) -> Rectangle<i32, Logical>;
-    fn accumulated_damage(&self, for_values: Option<(&Space, &Output)>) -> Vec<Rectangle<i32, Logical>>;
+    fn accumulated_damage(
+        &self,
+        for_values: Option<SpaceOutputTuple<'_, '_>>,
+    ) -> Vec<Rectangle<i32, Logical>>;
     fn draw(
         &self,
         renderer: &mut R,
         frame: &mut F,
         scale: f64,
-        location: Point<i32, Logical>,
         damage: &[Rectangle<i32, Logical>],
         log: &slog::Logger,
     ) -> Result<(), R::Error>;
@@ -77,7 +82,7 @@ where
         (&**self as &dyn RenderElement<R, F, E, T>).geometry()
     }
     fn accumulated_damage(&self, for_values: Option<(&Space, &Output)>) -> Vec<Rectangle<i32, Logical>> {
-        (&**self as &dyn RenderElement<R, F, E, T>).accumulated_damage(for_values)
+        (&**self as &dyn RenderElement<R, F, E, T>).accumulated_damage(for_values.map(SpaceOutputTuple::from))
     }
     fn draw(
         &self,
@@ -85,11 +90,11 @@ where
         renderer: &mut R,
         frame: &mut F,
         scale: f64,
-        location: Point<i32, Logical>,
+        _location: Point<i32, Logical>,
         damage: &[Rectangle<i32, Logical>],
         log: &slog::Logger,
     ) -> Result<(), R::Error> {
-        (&**self as &dyn RenderElement<R, F, E, T>).draw(renderer, frame, scale, location, damage, log)
+        (&**self as &dyn RenderElement<R, F, E, T>).draw(renderer, frame, scale, damage, log)
     }
 }
 
@@ -116,8 +121,11 @@ where
         bbox
     }
 
-    fn accumulated_damage(&self, for_values: Option<(&Space, &Output)>) -> Vec<Rectangle<i32, Logical>> {
+    fn accumulated_damage(
-        damage_from_surface_tree(&self.surface, (0, 0), for_values)
+        &self,
+        for_values: Option<SpaceOutputTuple<'_, '_>>,
+    ) -> Vec<Rectangle<i32, Logical>> {
+        damage_from_surface_tree(&self.surface, (0, 0), for_values.map(|x| (x.0, x.1)))
     }
 
     fn draw(
@@ -125,7 +133,6 @@ where
         renderer: &mut R,
         frame: &mut F,
         scale: f64,
-        location: Point<i32, Logical>,
         damage: &[Rectangle<i32, Logical>],
         log: &slog::Logger,
     ) -> Result<(), R::Error> {
@@ -134,9 +141,40 @@ where
             frame,
             &self.surface,
             scale,
-            location,
+            self.position,
             damage,
             log,
         )
     }
 }
+
+/// Newtype for (&Space, &Output) to provide a `Hash` implementation for damage tracking
+#[derive(Debug, PartialEq)]
+pub struct SpaceOutputTuple<'a, 'b>(pub &'a Space, pub &'b Output);
+
+impl<'a, 'b> Hash for SpaceOutputTuple<'a, 'b> {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.0.id.hash(state);
+        (std::sync::Arc::as_ptr(&self.1.inner) as *const () as usize).hash(state);
+    }
+}
+
+impl<'a, 'b> SpaceOutputTuple<'a, 'b> {
+    /// Returns an owned version that produces and equivalent hash
+    pub fn owned_hash(&self) -> SpaceOutputHash {
+        SpaceOutputHash(
+            self.0.id,
+            std::sync::Arc::as_ptr(&self.1.inner) as *const () as usize,
+        )
+    }
+}
+
+impl<'a, 'b> From<(&'a Space, &'b Output)> for SpaceOutputTuple<'a, 'b> {
+    fn from((space, output): (&'a Space, &'b Output)) -> SpaceOutputTuple<'a, 'b> {
+        SpaceOutputTuple(space, output)
+    }
+}
+
+/// Type to use as an owned hashable value equal to [`SpaceOutputTuple`]
+#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
+pub struct SpaceOutputHash(usize, usize);

src/desktop/space/mod.rs

@@ -30,12 +30,7 @@ use self::window::*;
 
 crate::utils::ids::id_gen!(next_space_id, SPACE_ID, SPACE_IDS);
 
-#[derive(Debug, thiserror::Error)]
+/// Represents two dimensional plane to map windows and outputs upon.
-pub enum SpaceError {
-    #[error("Window is not mapped to this space")]
-    UnknownWindow,
-}
-
 #[derive(Debug)]
 pub struct Space {
     pub(super) id: usize,
@@ -48,6 +43,12 @@ pub struct Space {
 pub type DynamicRenderElements<R> =
     Box<dyn RenderElement<R, <R as Renderer>::Frame, <R as Renderer>::Error, <R as Renderer>::TextureId>>;
 
+impl PartialEq for Space {
+    fn eq(&self, other: &Space) -> bool {
+        self.id == other.id
+    }
+}
+
 impl Drop for Space {
     fn drop(&mut self) {
         SPACE_IDS.lock().unwrap().remove(&self.id);
@@ -390,6 +391,10 @@ impl Space {
         R::Error: 'static,
         R::Frame: 'static,
     {
+        if !self.outputs.contains(output) {
+            return Err(RenderError::UnmappedOutput);
+        }
+
         type SpaceElem<R> =
             dyn SpaceElement<R, <R as Renderer>::Frame, <R as Renderer>::Error, <R as Renderer>::TextureId>;
 
@@ -474,14 +479,7 @@ impl Space {
         damage.dedup();
         damage.retain(|rect| rect.overlaps(output_geo));
         damage.retain(|rect| rect.size.h > 0 && rect.size.w > 0);
-        for rect in damage.clone().iter() {
+        // merge overlapping rectangles
-            // if this rect was already removed, because it was smaller as another one,
-            // there is no reason to evaluate this.
-            if damage.contains(rect) {
-                // remove every rectangle that is contained in this rectangle
-                damage.retain(|other| !rect.contains_rect(*other));
-            }
-        }
         damage = damage.into_iter().fold(Vec::new(), |mut new_damage, rect| {
             if let Some(existing) = new_damage.iter_mut().find(|other| rect.overlaps(**other)) {
                 *existing = existing.merge(rect);
@@ -614,4 +612,6 @@ pub enum RenderError<R: Renderer> {
     Rendering(R::Error),
     #[error("Output has no active mode")]
     OutputNoMode,
+    #[error("Output was not mapped to this space")]
+    UnmappedOutput,
 }

src/desktop/utils.rs

@@ -12,7 +12,7 @@ use crate::{
 };
 use wayland_server::protocol::wl_surface;
 
-use std::{cell::RefCell, sync::Arc};
+use std::cell::RefCell;
 
 impl SurfaceState {
     /// Returns the size of the surface.
@@ -96,8 +96,10 @@ pub fn damage_from_surface_tree<P>(
 where
     P: Into<Point<i32, Logical>>,
 {
+    use super::space::SpaceOutputTuple;
+
     let mut damage = Vec::new();
-    let key = key.map(|(space, output)| (space.id, Arc::as_ptr(&output.inner) as *const ()));
+    let key = key.map(|x| SpaceOutputTuple::from(x).owned_hash());
     with_surface_tree_upward(
         surface,
         location.into(),