Merge pull request #475 from Smithay/fix/popup_output_events

Send popups and layers output enter/leave events as well

src/desktop/layer.rs

@@ -3,7 +3,7 @@ use crate::{
     desktop::{utils::*, PopupManager, Space},
     utils::{user_data::UserDataMap, Logical, Point, Rectangle},
     wayland::{
-        compositor::with_states,
+        compositor::{with_states, with_surface_tree_downward, TraversalAction},
         output::{Inner as OutputInner, Output},
         shell::wlr_layer::{
             Anchor, ExclusiveZone, KeyboardInteractivity, Layer as WlrLayer, LayerSurface as WlrLayerSurface,
@@ -29,6 +29,9 @@ pub struct LayerMap {
     layers: IndexSet<LayerSurface>,
     output: Weak<(Mutex<OutputInner>, wayland_server::UserDataMap)>,
     zone: Rectangle<i32, Logical>,
+    // surfaces for tracking enter and leave events
+    surfaces: Vec<WlSurface>,
+    logger: ::slog::Logger,
 }
 
 /// Retrieve a [`LayerMap`] for a given [`Output`].
@@ -53,6 +56,10 @@ pub fn layer_map_for_output(o: &Output) -> RefMut<'_, LayerMap> {
                     .map(|mode| mode.size.to_logical(o.current_scale()))
                     .unwrap_or_else(|| (0, 0).into()),
             ),
+            surfaces: Vec::new(),
+            logger: (*o.inner.0.lock().unwrap())
+                .log
+                .new(slog::o!("smithay_module" => "layer_map")),
         })
     });
     userdata.get::<RefCell<LayerMap>>().unwrap().borrow_mut()
@@ -90,6 +97,34 @@ impl LayerMap {
             let _ = layer.user_data().get::<LayerUserdata>().take();
             self.arrange();
         }
+        if let (Some(output), Some(surface)) = (self.output(), layer.get_surface()) {
+            with_surface_tree_downward(
+                surface,
+                (),
+                |_, _, _| TraversalAction::DoChildren(()),
+                |wl_surface, _, _| {
+                    output_leave(&output, &mut self.surfaces, wl_surface, &self.logger);
+                },
+                |_, _, _| true,
+            );
+            for (popup, _) in PopupManager::popups_for_surface(surface)
+                .ok()
+                .into_iter()
+                .flatten()
+            {
+                if let Some(surface) = popup.get_surface() {
+                    with_surface_tree_downward(
+                        surface,
+                        (),
+                        |_, _, _| TraversalAction::DoChildren(()),
+                        |wl_surface, _, _| {
+                            output_leave(&output, &mut self.surfaces, wl_surface, &self.logger);
+                        },
+                        |_, _, _| true,
+                    )
+                }
+            }
+        }
     }
 
     /// Return the area of this output, that is not exclusive to any [`LayerSurface`]s.
@@ -160,11 +195,7 @@ impl LayerMap {
                     .unwrap_or_else(|| (0, 0).into()),
             );
             let mut zone = output_rect;
-            slog::debug!(
+            slog::trace!(self.logger, "Arranging layers into {:?}", output_rect.size);
-                crate::slog_or_fallback(None),
-                "Arranging layers into {:?}",
-                output_rect.size
-            );
 
             for layer in self.layers.iter() {
                 let surface = if let Some(surface) = layer.get_surface() {
@@ -173,6 +204,35 @@ impl LayerMap {
                     continue;
                 };
 
+                let logger_ref = &self.logger;
+                let surfaces_ref = &mut self.surfaces;
+                with_surface_tree_downward(
+                    surface,
+                    (),
+                    |_, _, _| TraversalAction::DoChildren(()),
+                    |wl_surface, _, _| {
+                        output_enter(&output, surfaces_ref, wl_surface, logger_ref);
+                    },
+                    |_, _, _| true,
+                );
+                for (popup, _) in PopupManager::popups_for_surface(surface)
+                    .ok()
+                    .into_iter()
+                    .flatten()
+                {
+                    if let Some(surface) = popup.get_surface() {
+                        with_surface_tree_downward(
+                            surface,
+                            (),
+                            |_, _, _| TraversalAction::DoChildren(()),
+                            |wl_surface, _, _| {
+                                output_enter(&output, surfaces_ref, wl_surface, logger_ref);
+                            },
+                            |_, _, _| true,
+                        )
+                    }
+                }
+
                 let data = with_states(surface, |states| {
                     *states.cached_state.current::<LayerSurfaceCachedState>()
                 })
@@ -233,8 +293,8 @@ impl LayerMap {
                     }
                 }
 
-                slog::debug!(
+                slog::trace!(
-                    crate::slog_or_fallback(None),
+                    self.logger,
                     "Setting layer to pos {:?} and size {:?}",
                     location,
                     size
@@ -253,7 +313,7 @@ impl LayerMap {
                 layer_state(layer).location = location;
             }
 
-            slog::debug!(crate::slog_or_fallback(None), "Remaining zone {:?}", zone);
+            slog::trace!(self.logger, "Remaining zone {:?}", zone);
             self.zone = zone;
         }
     }
@@ -267,7 +327,8 @@ impl LayerMap {
     /// This function needs to be called periodically (though not necessarily frequently)
     /// to be able cleanup internally used resources.
     pub fn cleanup(&mut self) {
-        self.layers.retain(|layer| layer.alive())
+        self.layers.retain(|layer| layer.alive());
+        self.surfaces.retain(|s| s.as_ref().is_alive());
     }
 }
 

src/desktop/space/mod.rs

@@ -2,23 +2,22 @@
 //! rendering helpers to add custom elements or different clients to a space.
 
 use crate::{
-    backend::renderer::{utils::SurfaceState, Frame, ImportAll, Renderer},
+    backend::renderer::{Frame, ImportAll, Renderer},
     desktop::{
         layer::{layer_map_for_output, LayerSurface},
+        popup::PopupManager,
+        utils::{output_leave, output_update},
         window::Window,
     },
     utils::{Logical, Point, Rectangle, Transform},
     wayland::{
-        compositor::{
+        compositor::{get_parent, is_sync_subsurface},
-            get_parent, is_sync_subsurface, with_surface_tree_downward, SubsurfaceCachedState,
-            TraversalAction,
-        },
         output::Output,
         shell::wlr_layer::Layer as WlrLayer,
     },
 };
 use indexmap::{IndexMap, IndexSet};
-use std::{cell::RefCell, collections::VecDeque, fmt};
+use std::{collections::VecDeque, fmt};
 use wayland_server::protocol::wl_surface::WlSurface;
 
 mod element;
@@ -326,97 +325,39 @@ impl Space {
                 // the output.
                 if !output_geometry.overlaps(bbox) {
                     if let Some(surface) = kind.get_surface() {
-                        with_surface_tree_downward(
+                        output_leave(output, &mut output_state.surfaces, surface, &self.logger);
-                            surface,
-                            (),
-                            |_, _, _| TraversalAction::DoChildren(()),
-                            |wl_surface, _, _| {
-                                if output_state.surfaces.contains(wl_surface) {
-                                    slog::trace!(
-                                        self.logger,
-                                        "surface ({:?}) leaving output {:?}",
-                                        wl_surface,
-                                        output.name()
-                                    );
-                                    output.leave(wl_surface);
-                                    output_state.surfaces.retain(|s| s != wl_surface);
-                                }
-                            },
-                            |_, _, _| true,
-                        )
                     }
                     continue;
                 }
 
                 if let Some(surface) = kind.get_surface() {
-                    with_surface_tree_downward(
+                    output_update(
+                        output,
+                        output_geometry,
+                        &mut output_state.surfaces,
                         surface,
                         window_loc(window, &self.id),
-                        |_, states, location| {
+                        &self.logger,
-                            let mut location = *location;
-                            let data = states.data_map.get::<RefCell<SurfaceState>>();
-
-                            if data.is_some() {
-                                if states.role == Some("subsurface") {
-                                    let current = states.cached_state.current::<SubsurfaceCachedState>();
-                                    location += current.location;
-                                }
-
-                                TraversalAction::DoChildren(location)
-                            } else {
-                                // If the parent surface is unmapped, then the child surfaces are hidden as
-                                // well, no need to consider them here.
-                                TraversalAction::SkipChildren
-                            }
-                        },
-                        |wl_surface, states, &loc| {
-                            let data = states.data_map.get::<RefCell<SurfaceState>>();
-
-                            if let Some(size) = data.and_then(|d| d.borrow().surface_size()) {
-                                let surface_rectangle = Rectangle { loc, size };
-
-                                if output_geometry.overlaps(surface_rectangle) {
-                                    // We found a matching output, check if we already sent enter
-                                    if !output_state.surfaces.contains(wl_surface) {
-                                        slog::trace!(
-                                            self.logger,
-                                            "surface ({:?}) entering output {:?}",
-                                            wl_surface,
-                                            output.name()
                     );
-                                        output.enter(wl_surface);
+
-                                        output_state.surfaces.push(wl_surface.clone());
+                    for (popup, location) in PopupManager::popups_for_surface(surface)
-                                    }
+                        .ok()
-                                } else {
+                        .into_iter()
-                                    // Surface does not match output, if we sent enter earlier
+                        .flatten()
-                                    // we should now send leave
+                    {
-                                    if output_state.surfaces.contains(wl_surface) {
+                        if let Some(surface) = popup.get_surface() {
-                                        slog::trace!(
+                            let location = window_loc(window, &self.id) + window.geometry().loc + location
-                                            self.logger,
+                                - popup.geometry().loc;
-                                            "surface ({:?}) leaving output {:?}",
+                            output_update(
-                                            wl_surface,
+                                output,
-                                            output.name()
+                                output_geometry,
+                                &mut output_state.surfaces,
+                                surface,
+                                location,
+                                &self.logger,
                             );
-                                        output.leave(wl_surface);
-                                        output_state.surfaces.retain(|s| s != wl_surface);
                         }
                     }
-                            } else {
-                                // Maybe the the surface got unmapped, send leave on output
-                                if output_state.surfaces.contains(wl_surface) {
-                                    slog::trace!(
-                                        self.logger,
-                                        "surface ({:?}) leaving output {:?}",
-                                        wl_surface,
-                                        output.name()
-                                    );
-                                    output.leave(wl_surface);
-                                    output_state.surfaces.retain(|s| s != wl_surface);
-                                }
-                            }
-                        },
-                        |_, _, _| true,
-                    )
                 }
             }
         }

src/desktop/utils.rs

@@ -231,3 +231,89 @@ pub fn send_frames_surface_tree(surface: &wl_surface::WlSurface, time: u32) {
         |_, _, &()| true,
     );
 }
+
+pub(crate) fn output_update(
+    output: &Output,
+    output_geometry: Rectangle<i32, Logical>,
+    surface_list: &mut Vec<wl_surface::WlSurface>,
+    surface: &wl_surface::WlSurface,
+    location: Point<i32, Logical>,
+    logger: &slog::Logger,
+) {
+    with_surface_tree_downward(
+        surface,
+        location,
+        |_, states, location| {
+            let mut location = *location;
+            let data = states.data_map.get::<RefCell<SurfaceState>>();
+
+            if data.is_some() {
+                if states.role == Some("subsurface") {
+                    let current = states.cached_state.current::<SubsurfaceCachedState>();
+                    location += current.location;
+                }
+
+                TraversalAction::DoChildren(location)
+            } else {
+                // If the parent surface is unmapped, then the child surfaces are hidden as
+                // well, no need to consider them here.
+                TraversalAction::SkipChildren
+            }
+        },
+        |wl_surface, states, &loc| {
+            let data = states.data_map.get::<RefCell<SurfaceState>>();
+
+            if let Some(size) = data.and_then(|d| d.borrow().surface_size()) {
+                let surface_rectangle = Rectangle { loc, size };
+                if output_geometry.overlaps(surface_rectangle) {
+                    // We found a matching output, check if we already sent enter
+                    output_enter(output, surface_list, wl_surface, logger);
+                } else {
+                    // Surface does not match output, if we sent enter earlier
+                    // we should now send leave
+                    output_leave(output, surface_list, wl_surface, logger);
+                }
+            } else {
+                // Maybe the the surface got unmapped, send leave on output
+                output_leave(output, surface_list, wl_surface, logger);
+            }
+        },
+        |_, _, _| true,
+    );
+}
+
+pub(crate) fn output_enter(
+    output: &Output,
+    surface_list: &mut Vec<wl_surface::WlSurface>,
+    surface: &wl_surface::WlSurface,
+    logger: &slog::Logger,
+) {
+    if !surface_list.contains(surface) {
+        slog::debug!(
+            logger,
+            "surface ({:?}) entering output {:?}",
+            surface,
+            output.name()
+        );
+        output.enter(surface);
+        surface_list.push(surface.clone());
+    }
+}
+
+pub(crate) fn output_leave(
+    output: &Output,
+    surface_list: &mut Vec<wl_surface::WlSurface>,
+    surface: &wl_surface::WlSurface,
+    logger: &slog::Logger,
+) {
+    if surface_list.contains(surface) {
+        slog::debug!(
+            logger,
+            "surface ({:?}) leaving output {:?}",
+            surface,
+            output.name()
+        );
+        output.leave(surface);
+        surface_list.retain(|s| s != surface);
+    }
+}

src/wayland/output/mod.rs

@@ -102,7 +102,7 @@ pub struct PhysicalProperties {
 #[derive(Debug)]
 pub(crate) struct Inner {
     name: String,
-    log: ::slog::Logger,
+    pub(crate) log: ::slog::Logger,
     instances: Vec<WlOutput>,
     physical: PhysicalProperties,
     location: Point<i32, Logical>,