desktop: handle xdg-popups

src/backend/renderer/mod.rs

@@ -32,6 +32,8 @@ use crate::backend::egl::{
     Error as EglError,
 };
 
+pub mod utils;
+
 #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
 /// Possible transformations to two-dimensional planes
 pub enum Transform {

src/backend/renderer/utils/mod.rs

@@ -0,0 +1,167 @@
+use crate::{
+    backend::renderer::{buffer_dimensions, Frame, ImportAll, Renderer, Texture},
+    utils::{Logical, Physical, Point, Size},
+    wayland::compositor::{
+        is_sync_subsurface, with_surface_tree_upward, BufferAssignment, Damage, SubsurfaceCachedState,
+        SurfaceAttributes, TraversalAction,
+    },
+};
+use std::cell::RefCell;
+use wayland_server::protocol::{wl_buffer::WlBuffer, wl_surface::WlSurface};
+
+#[derive(Default)]
+pub(crate) struct SurfaceState {
+    pub(crate) buffer_dimensions: Option<Size<i32, Physical>>,
+    pub(crate) buffer_scale: i32,
+    pub(crate) buffer: Option<WlBuffer>,
+    pub(crate) texture: Option<Box<dyn std::any::Any + 'static>>,
+}
+
+impl SurfaceState {
+    pub fn update_buffer(&mut self, attrs: &mut SurfaceAttributes) {
+        match attrs.buffer.take() {
+            Some(BufferAssignment::NewBuffer { buffer, .. }) => {
+                // new contents
+                self.buffer_dimensions = buffer_dimensions(&buffer);
+                self.buffer_scale = attrs.buffer_scale;
+                if let Some(old_buffer) = std::mem::replace(&mut self.buffer, Some(buffer)) {
+                    if &old_buffer != self.buffer.as_ref().unwrap() {
+                        old_buffer.release();
+                    }
+                }
+                self.texture = None;
+            }
+            Some(BufferAssignment::Removed) => {
+                // remove the contents
+                self.buffer_dimensions = None;
+                if let Some(buffer) = self.buffer.take() {
+                    buffer.release();
+                };
+                self.texture = None;
+            }
+            None => {}
+        }
+    }
+}
+
+pub fn on_commit_buffer_handler(surface: &WlSurface) {
+    if !is_sync_subsurface(surface) {
+        with_surface_tree_upward(
+            surface,
+            (),
+            |_, _, _| TraversalAction::DoChildren(()),
+            |_surf, states, _| {
+                states
+                    .data_map
+                    .insert_if_missing(|| RefCell::new(SurfaceState::default()));
+                let mut data = states
+                    .data_map
+                    .get::<RefCell<SurfaceState>>()
+                    .unwrap()
+                    .borrow_mut();
+                data.update_buffer(&mut *states.cached_state.current::<SurfaceAttributes>());
+            },
+            |_, _, _| true,
+        );
+    }
+}
+
+pub fn draw_surface_tree<R, E, F, T>(
+    renderer: &mut R,
+    frame: &mut F,
+    surface: &WlSurface,
+    scale: f64,
+    location: Point<i32, Logical>,
+    log: &slog::Logger,
+) -> Result<(), R::Error>
+where
+    R: Renderer<Error = E, TextureId = T, Frame = F> + ImportAll,
+    F: Frame<Error = E, TextureId = T>,
+    E: std::error::Error,
+    T: Texture + 'static,
+{
+    let mut result = Ok(());
+    with_surface_tree_upward(
+        surface,
+        location,
+        |_surface, states, location| {
+            let mut location = *location;
+            if let Some(data) = states.data_map.get::<RefCell<SurfaceState>>() {
+                let mut data = data.borrow_mut();
+                let attributes = states.cached_state.current::<SurfaceAttributes>();
+                // Import a new buffer if necessary
+                if data.texture.is_none() {
+                    if let Some(buffer) = data.buffer.as_ref() {
+                        let damage = attributes
+                            .damage
+                            .iter()
+                            .map(|dmg| match dmg {
+                                Damage::Buffer(rect) => *rect,
+                                // TODO also apply transformations
+                                Damage::Surface(rect) => rect.to_buffer(attributes.buffer_scale),
+                            })
+                            .collect::<Vec<_>>();
+
+                        match renderer.import_buffer(buffer, Some(states), &damage) {
+                            Some(Ok(m)) => {
+                                data.texture = Some(Box::new(m));
+                            }
+                            Some(Err(err)) => {
+                                slog::warn!(log, "Error loading buffer: {}", err);
+                            }
+                            None => {
+                                slog::error!(log, "Unknown buffer format for: {:?}", buffer);
+                            }
+                        }
+                    }
+                }
+                // Now, should we be drawn ?
+                if data.texture.is_some() {
+                    // if yes, also process the children
+                    if states.role == Some("subsurface") {
+                        let current = states.cached_state.current::<SubsurfaceCachedState>();
+                        location += current.location;
+                    }
+                    TraversalAction::DoChildren(location)
+                } else {
+                    // we are not displayed, so our children are neither
+                    TraversalAction::SkipChildren
+                }
+            } else {
+                // we are not displayed, so our children are neither
+                TraversalAction::SkipChildren
+            }
+        },
+        |surface, states, location| {
+            let mut location = *location;
+            if let Some(data) = states.data_map.get::<RefCell<SurfaceState>>() {
+                let mut data = data.borrow_mut();
+                let buffer_scale = data.buffer_scale;
+                let attributes = states.cached_state.current::<SurfaceAttributes>();
+                if let Some(texture) = data.texture.as_mut().and_then(|x| x.downcast_mut::<T>()) {
+                    // we need to re-extract the subsurface offset, as the previous closure
+                    // only passes it to our children
+                    if states.role == Some("subsurface") {
+                        let current = states.cached_state.current::<SubsurfaceCachedState>();
+                        location += current.location;
+                    }
+
+                    // TODO: Take wp_viewporter into account
+                    if let Err(err) = frame.render_texture_at(
+                        texture,
+                        location.to_f64().to_physical(scale).to_i32_round(),
+                        buffer_scale,
+                        scale,
+                        attributes.buffer_transform.into(),
+                        1.0,
+                    ) {
+                        result = Err(err);
+                    }
+                }
+            }
+        },
+        |_, _, _| true,
+    );
+
+    result
+}

src/desktop/mod.rs

@@ -1,7 +1,9 @@
 // TODO: Remove - but for now, this makes sure these files are not completely highlighted with warnings
 #![allow(missing_docs, clippy::all)]
+mod popup;
 mod space;
 mod window;
 
+pub use self::popup::*;
 pub use self::space::*;
 pub use self::window::*;

src/desktop/popup.rs

@@ -0,0 +1,262 @@
+use crate::{
+    utils::{DeadResource, Logical, Point},
+    wayland::{
+        compositor::{get_role, with_states},
+        shell::xdg::{PopupSurface, XdgPopupSurfaceRoleAttributes, XDG_POPUP_ROLE},
+    },
+};
+use std::sync::{Arc, Mutex};
+use wayland_server::protocol::wl_surface::WlSurface;
+
+#[derive(Debug)]
+pub struct PopupManager {
+    unmapped_popups: Vec<PopupKind>,
+    popup_trees: Vec<PopupTree>,
+    logger: ::slog::Logger,
+}
+
+impl PopupManager {
+    pub fn new<L: Into<Option<::slog::Logger>>>(logger: L) -> Self {
+        PopupManager {
+            unmapped_popups: Vec::new(),
+            popup_trees: Vec::new(),
+            logger: crate::slog_or_fallback(logger),
+        }
+    }
+
+    pub fn track_popup(&mut self, kind: PopupKind) -> Result<(), DeadResource> {
+        if kind.parent().is_some() {
+            self.add_popup(kind)
+        } else {
+            slog::trace!(self.logger, "Adding unmapped popups: {:?}", kind);
+            self.unmapped_popups.push(kind);
+            Ok(())
+        }
+    }
+
+    pub fn commit(&mut self, surface: &WlSurface) {
+        if get_role(surface) == Some(XDG_POPUP_ROLE) {
+            if let Some(i) = self
+                .unmapped_popups
+                .iter()
+                .enumerate()
+                .find(|(_, p)| p.get_surface() == Some(surface))
+                .map(|(i, _)| i)
+            {
+                slog::trace!(self.logger, "Popup got mapped");
+                let popup = self.unmapped_popups.swap_remove(i);
+                // at this point the popup must have a parent,
+                // or it would have raised a protocol error
+                let _ = self.add_popup(popup);
+            }
+        }
+    }
+
+    fn add_popup(&mut self, popup: PopupKind) -> Result<(), DeadResource> {
+        let mut parent = popup.parent().unwrap();
+        while get_role(&parent) == Some(XDG_POPUP_ROLE) {
+            parent = with_states(&parent, |states| {
+                states
+                    .data_map
+                    .get::<Mutex<XdgPopupSurfaceRoleAttributes>>()
+                    .unwrap()
+                    .lock()
+                    .unwrap()
+                    .parent
+                    .as_ref()
+                    .cloned()
+                    .unwrap()
+            })?;
+        }
+
+        with_states(&parent, |states| {
+            let tree = PopupTree::default();
+            if states.data_map.insert_if_missing(|| tree.clone()) {
+                self.popup_trees.push(tree);
+            };
+            let tree = states.data_map.get::<PopupTree>().unwrap();
+            if !tree.alive() {
+                // if it previously had no popups, we likely removed it from our list already
+                self.popup_trees.push(tree.clone());
+            }
+            slog::trace!(self.logger, "Adding popup {:?} to parent {:?}", popup, parent);
+            tree.insert(popup);
+        })
+    }
+
+    pub fn find_popup(&self, surface: &WlSurface) -> Option<PopupKind> {
+        self.unmapped_popups
+            .iter()
+            .find(|p| p.get_surface() == Some(surface))
+            .cloned()
+            .or_else(|| {
+                self.popup_trees
+                    .iter()
+                    .map(|tree| tree.iter_popups())
+                    .flatten()
+                    .find(|(p, _)| p.get_surface() == Some(surface))
+                    .map(|(p, _)| p)
+            })
+    }
+
+    pub fn popups_for_surface(
+        surface: &WlSurface,
+    ) -> Result<impl Iterator<Item = (PopupKind, Point<i32, Logical>)>, DeadResource> {
+        with_states(surface, |states| {
+            states
+                .data_map
+                .get::<PopupTree>()
+                .map(|x| x.iter_popups())
+                .into_iter()
+                .flatten()
+        })
+    }
+
+    pub fn cleanup(&mut self) {
+        // retain_mut is sadly still unstable
+        self.popup_trees.iter_mut().for_each(|tree| tree.cleanup());
+        self.popup_trees.retain(|tree| tree.alive());
+        self.unmapped_popups.retain(|surf| surf.alive());
+    }
+}
+
+#[derive(Debug, Default, Clone)]
+struct PopupTree(Arc<Mutex<Vec<PopupNode>>>);
+
+#[derive(Debug, Clone)]
+struct PopupNode {
+    surface: PopupKind,
+    children: Vec<PopupNode>,
+}
+
+impl PopupTree {
+    fn iter_popups(&self) -> impl Iterator<Item = (PopupKind, Point<i32, Logical>)> {
+        self.0
+            .lock()
+            .unwrap()
+            .iter()
+            .map(|n| n.iter_popups_relative_to((0, 0)).map(|(p, l)| (p.clone(), l)))
+            .flatten()
+            .collect::<Vec<_>>()
+            .into_iter()
+    }
+
+    fn insert(&self, popup: PopupKind) {
+        let children = &mut *self.0.lock().unwrap();
+        for child in children.iter_mut() {
+            if child.insert(popup.clone()) {
+                return;
+            }
+        }
+        children.push(PopupNode::new(popup));
+    }
+
+    fn cleanup(&mut self) {
+        let mut children = self.0.lock().unwrap();
+        for child in children.iter_mut() {
+            child.cleanup();
+        }
+        children.retain(|n| n.surface.alive());
+    }
+
+    fn alive(&self) -> bool {
+        !self.0.lock().unwrap().is_empty()
+    }
+}
+
+impl PopupNode {
+    fn new(surface: PopupKind) -> Self {
+        PopupNode {
+            surface,
+            children: Vec::new(),
+        }
+    }
+
+    fn iter_popups_relative_to<P: Into<Point<i32, Logical>>>(
+        &self,
+        loc: P,
+    ) -> impl Iterator<Item = (&PopupKind, Point<i32, Logical>)> {
+        let relative_to = loc.into() + self.surface.location();
+        std::iter::once((&self.surface, relative_to)).chain(
+            self.children
+                .iter()
+                .map(move |x| {
+                    Box::new(x.iter_popups_relative_to(relative_to))
+                        as Box<dyn Iterator<Item = (&PopupKind, Point<i32, Logical>)>>
+                })
+                .flatten(),
+        )
+    }
+
+    fn insert(&mut self, popup: PopupKind) -> bool {
+        let parent = popup.parent().unwrap();
+        if self.surface.get_surface() == Some(&parent) {
+            self.children.push(PopupNode::new(popup));
+            true
+        } else {
+            for child in &mut self.children {
+                if child.insert(popup.clone()) {
+                    return true;
+                }
+            }
+            false
+        }
+    }
+
+    fn cleanup(&mut self) {
+        for child in &mut self.children {
+            child.cleanup();
+        }
+        self.children.retain(|n| n.surface.alive());
+    }
+}
+
+#[derive(Debug, Clone)]
+pub enum PopupKind {
+    Xdg(PopupSurface),
+}
+
+impl PopupKind {
+    fn alive(&self) -> bool {
+        match *self {
+            PopupKind::Xdg(ref t) => t.alive(),
+        }
+    }
+
+    pub fn get_surface(&self) -> Option<&WlSurface> {
+        match *self {
+            PopupKind::Xdg(ref t) => t.get_surface(),
+        }
+    }
+
+    fn parent(&self) -> Option<WlSurface> {
+        match *self {
+            PopupKind::Xdg(ref t) => t.get_parent_surface(),
+        }
+    }
+
+    fn location(&self) -> Point<i32, Logical> {
+        let wl_surface = match self.get_surface() {
+            Some(s) => s,
+            None => return (0, 0).into(),
+        };
+        with_states(wl_surface, |states| {
+            states
+                .data_map
+                .get::<Mutex<XdgPopupSurfaceRoleAttributes>>()
+                .unwrap()
+                .lock()
+                .unwrap()
+                .current
+                .geometry
+        })
+        .unwrap_or_default()
+        .loc
+    }
+}
+
+impl From<PopupSurface> for PopupKind {
+    fn from(p: PopupSurface) -> PopupKind {
+        PopupKind::Xdg(p)
+    }
+}

src/desktop/space.rs

@@ -1,6 +1,6 @@
-use super::{draw_window, SurfaceState, Window};
+use super::{draw_window, Window};
 use crate::{
-    backend::renderer::{Frame, ImportAll, Renderer, Transform},
+    backend::renderer::{utils::SurfaceState, Frame, ImportAll, Renderer, Transform},
     utils::{Logical, Point, Rectangle},
     wayland::{
         compositor::{with_surface_tree_downward, SubsurfaceCachedState, TraversalAction},
@@ -150,7 +150,7 @@ impl Space {
 
     /// Get a reference to the window under a given point, if any
     pub fn window_under(&self, point: Point<f64, Logical>) -> Option<&Window> {
-        self.windows.iter().find(|w| {
+        self.windows.iter().rev().find(|w| {
             let bbox = window_rect(w, &self.id);
             bbox.to_f64().contains(point)
         })
@@ -419,7 +419,7 @@ impl Space {
 
         // lets iterate front to back and figure out, what new windows or unmoved windows we have
         for window in self.windows.iter().rev() {
-            let geo = window_rect(window, &self.id);
+            let geo = window_rect_with_popups(window, &self.id);
             let old_geo = state.last_state.get(&window.0.id).cloned();
 
             // window was moved or resized
@@ -481,7 +481,7 @@ impl Space {
 
                 // Then re-draw all window overlapping with a damage rect.
                 for window in self.windows.iter() {
-                    let wgeo = window_rect(window, &self.id);
+                    let wgeo = window_rect_with_popups(window, &self.id);
                     let mut loc = window_loc(window, &self.id);
                     if damage.iter().any(|geo| wgeo.overlaps(*geo)) {
                         loc -= output_geo.loc;
@@ -506,7 +506,7 @@ impl Space {
             .windows
             .iter()
             .map(|window| {
-                let wgeo = window_rect(window, &self.id);
+                let wgeo = window_rect_with_popups(window, &self.id);
                 (window.0.id, wgeo)
             })
             .collect();
@@ -550,6 +550,13 @@ fn window_rect(window: &Window, space_id: &usize) -> Rectangle<i32, Logical> {
     wgeo
 }
 
+fn window_rect_with_popups(window: &Window, space_id: &usize) -> Rectangle<i32, Logical> {
+    let loc = window_loc(window, space_id);
+    let mut wgeo = window.bbox_with_popups();
+    wgeo.loc += loc;
+    wgeo
+}
+
 fn window_loc(window: &Window, space_id: &usize) -> Point<i32, Logical> {
     window
         .user_data()

src/desktop/window.rs

@@ -1,11 +1,14 @@
 use crate::{
-    backend::renderer::{buffer_dimensions, Frame, ImportAll, Renderer, Texture},
-    utils::{Logical, Physical, Point, Rectangle, Size},
+    backend::renderer::{
+        utils::{draw_surface_tree, SurfaceState},
+        Frame, ImportAll, Renderer, Texture,
+    },
+    desktop::PopupManager,
+    utils::{Logical, Point, Rectangle, Size},
     wayland::{
         compositor::{
-            add_commit_hook, is_sync_subsurface, with_states, with_surface_tree_downward,
-            with_surface_tree_upward, BufferAssignment, Damage, SubsurfaceCachedState, SurfaceAttributes,
-            TraversalAction,
+            with_states, with_surface_tree_downward, with_surface_tree_upward, Damage, SubsurfaceCachedState,
+            SurfaceAttributes, TraversalAction,
         },
         shell::xdg::{SurfaceCachedState, ToplevelSurface},
     },
@@ -98,62 +101,7 @@ impl Kind {
     }
 }
 
-#[derive(Default)]
-pub(super) struct SurfaceState {
-    buffer_dimensions: Option<Size<i32, Physical>>,
-    buffer_scale: i32,
-    buffer: Option<wl_buffer::WlBuffer>,
-    texture: Option<Box<dyn std::any::Any + 'static>>,
-}
-
-fn surface_commit(surface: &wl_surface::WlSurface) {
-    if !is_sync_subsurface(surface) {
-        with_surface_tree_upward(
-            surface,
-            (),
-            |_, _, _| TraversalAction::DoChildren(()),
-            |_surf, states, _| {
-                states
-                    .data_map
-                    .insert_if_missing(|| RefCell::new(SurfaceState::default()));
-                let mut data = states
-                    .data_map
-                    .get::<RefCell<SurfaceState>>()
-                    .unwrap()
-                    .borrow_mut();
-                data.update_buffer(&mut *states.cached_state.current::<SurfaceAttributes>());
-            },
-            |_, _, _| true,
-        );
-    }
-}
-
 impl SurfaceState {
-    pub fn update_buffer(&mut self, attrs: &mut SurfaceAttributes) {
-        match attrs.buffer.take() {
-            Some(BufferAssignment::NewBuffer { buffer, .. }) => {
-                // new contents
-                self.buffer_dimensions = buffer_dimensions(&buffer);
-                self.buffer_scale = attrs.buffer_scale;
-                if let Some(old_buffer) = std::mem::replace(&mut self.buffer, Some(buffer)) {
-                    if &old_buffer != self.buffer.as_ref().unwrap() {
-                        old_buffer.release();
-                    }
-                }
-                self.texture = None;
-            }
-            Some(BufferAssignment::Removed) => {
-                // remove the contents
-                self.buffer_dimensions = None;
-                if let Some(buffer) = self.buffer.take() {
-                    buffer.release();
-                };
-                self.texture = None;
-            }
-            None => {}
-        }
-    }
-
     /// Returns the size of the surface.
     pub fn size(&self) -> Option<Size<i32, Logical>> {
         self.buffer_dimensions
@@ -248,34 +196,25 @@ impl Window {
     /// A bounding box over this window and its children.
     // TODO: Cache and document when to trigger updates. If possible let space do it
     pub fn bbox(&self) -> Rectangle<i32, Logical> {
-        let mut bounding_box = Rectangle::from_loc_and_size((0, 0), (0, 0));
-        if let Some(wl_surface) = self.0.toplevel.get_surface() {
-            with_surface_tree_downward(
-                wl_surface,
-                (0, 0).into(),
-                |_, states, loc: &Point<i32, Logical>| {
-                    let mut loc = *loc;
-                    let data = states.data_map.get::<RefCell<SurfaceState>>();
+        if let Some(surface) = self.0.toplevel.get_surface() {
+            bbox_from_surface_tree(surface, (0, 0))
+        } else {
+            Rectangle::from_loc_and_size((0, 0), (0, 0))
+        }
+    }
 
-                    if let Some(size) = data.and_then(|d| d.borrow().size()) {
-                        if states.role == Some("subsurface") {
-                            let current = states.cached_state.current::<SubsurfaceCachedState>();
-                            loc += current.location;
-                        }
-
-                        // Update the bounding box.
-                        bounding_box = bounding_box.merge(Rectangle::from_loc_and_size(loc, size));
-
-                        TraversalAction::DoChildren(loc)
-                    } else {
-                        // If the parent surface is unmapped, then the child surfaces are hidden as
-                        // well, no need to consider them here.
-                        TraversalAction::SkipChildren
-                    }
-                },
-                |_, _, _| {},
-                |_, _, _| true,
-            );
+    pub fn bbox_with_popups(&self) -> Rectangle<i32, Logical> {
+        let mut bounding_box = self.bbox();
+        if let Some(surface) = self.0.toplevel.get_surface() {
+            for (popup, location) in PopupManager::popups_for_surface(surface)
+                .ok()
+                .into_iter()
+                .flatten()
+            {
+                if let Some(surface) = popup.get_surface() {
+                    bounding_box = bounding_box.merge(bbox_from_surface_tree(surface, location));
+                }
+            }
         }
         bounding_box
     }
@@ -310,25 +249,17 @@ impl Window {
     /// Sends the frame callback to all the subsurfaces in this
     /// window that requested it
     pub fn send_frame(&self, time: u32) {
-        if let Some(wl_surface) = self.0.toplevel.get_surface() {
-            with_surface_tree_downward(
-                wl_surface,
-                (),
-                |_, _, &()| TraversalAction::DoChildren(()),
-                |_surf, states, &()| {
-                    // the surface may not have any user_data if it is a subsurface and has not
-                    // yet been commited
-                    for callback in states
-                        .cached_state
-                        .current::<SurfaceAttributes>()
-                        .frame_callbacks
-                        .drain(..)
-                    {
-                        callback.done(time);
-                    }
-                },
-                |_, _, &()| true,
-            );
+        if let Some(surface) = self.0.toplevel.get_surface() {
+            send_frames_surface_tree(surface, time);
+            for (popup, _) in PopupManager::popups_for_surface(surface)
+                .ok()
+                .into_iter()
+                .flatten()
+            {
+                if let Some(surface) = popup.get_surface() {
+                    send_frames_surface_tree(surface, time);
+                }
+            }
         }
     }
 
@@ -338,89 +269,41 @@ impl Window {
         &self,
         point: Point<f64, Logical>,
     ) -> Option<(wl_surface::WlSurface, Point<i32, Logical>)> {
-        let found = RefCell::new(None);
-        if let Some(wl_surface) = self.0.toplevel.get_surface() {
-            with_surface_tree_downward(
-                wl_surface,
-                (0, 0).into(),
-                |wl_surface, states, location: &Point<i32, Logical>| {
-                    let mut location = *location;
-                    let data = states.data_map.get::<RefCell<SurfaceState>>();
+        if let Some(surface) = self.0.toplevel.get_surface() {
+            for (popup, location) in PopupManager::popups_for_surface(surface)
+                .ok()
+                .into_iter()
+                .flatten()
+            {
+                if let Some(result) = popup
+                    .get_surface()
+                    .and_then(|surface| under_from_surface_tree(surface, point, location))
+                {
+                    return Some(result);
+                }
+            }
 
-                    if states.role == Some("subsurface") {
-                        let current = states.cached_state.current::<SubsurfaceCachedState>();
-                        location += current.location;
-                    }
-
-                    let contains_the_point = data
-                        .map(|data| {
-                            data.borrow()
-                                .contains_point(&*states.cached_state.current(), point - location.to_f64())
-                        })
-                        .unwrap_or(false);
-                    if contains_the_point {
-                        *found.borrow_mut() = Some((wl_surface.clone(), location));
-                    }
-
-                    TraversalAction::DoChildren(location)
-                },
-                |_, _, _| {},
-                |_, _, _| {
-                    // only continue if the point is not found
-                    found.borrow().is_none()
-                },
-            );
+            under_from_surface_tree(surface, point, (0, 0))
+        } else {
+            None
         }
-        found.into_inner()
     }
 
     /// Damage of all the surfaces of this window
     pub(super) fn accumulated_damage(&self) -> Vec<Rectangle<i32, Logical>> {
         let mut damage = Vec::new();
-        let location = (0, 0).into();
         if let Some(surface) = self.0.toplevel.get_surface() {
-            with_surface_tree_upward(
-                surface,
-                location,
-                |_surface, states, location| {
-                    let mut location = *location;
-                    if let Some(data) = states.data_map.get::<RefCell<SurfaceState>>() {
-                        let data = data.borrow();
-                        if data.texture.is_none() {
-                            if states.role == Some("subsurface") {
-                                let current = states.cached_state.current::<SubsurfaceCachedState>();
-                                location += current.location;
-                            }
-                            return TraversalAction::DoChildren(location);
-                        }
-                    }
-                    TraversalAction::SkipChildren
-                },
-                |_surface, states, location| {
-                    let mut location = *location;
-                    if let Some(data) = states.data_map.get::<RefCell<SurfaceState>>() {
-                        let data = data.borrow();
-                        let attributes = states.cached_state.current::<SurfaceAttributes>();
-
-                        if data.texture.is_none() {
-                            if states.role == Some("subsurface") {
-                                let current = states.cached_state.current::<SubsurfaceCachedState>();
-                                location += current.location;
-                            }
-
-                            damage.extend(attributes.damage.iter().map(|dmg| {
-                                let mut rect = match dmg {
-                                    Damage::Buffer(rect) => rect.to_logical(attributes.buffer_scale),
-                                    Damage::Surface(rect) => *rect,
-                                };
-                                rect.loc += location;
-                                rect
-                            }));
-                        }
-                    }
-                },
-                |_, _, _| true,
-            )
+            damage.extend(damage_from_surface_tree(surface, (0, 0)));
+            for (popup, location) in PopupManager::popups_for_surface(surface)
+                .ok()
+                .into_iter()
+                .flatten()
+            {
+                if let Some(surface) = popup.get_surface() {
+                    let popup_damage = damage_from_surface_tree(surface, location);
+                    damage.extend(popup_damage);
+                }
+            }
         }
         damage
     }
@@ -432,32 +315,155 @@ impl Window {
     pub fn user_data(&self) -> &UserDataMap {
         &self.0.user_data
     }
-
-    /// Has to be called on commit - Window handles the buffer for you
-    pub fn commit(surface: &wl_surface::WlSurface) {
-        surface_commit(surface)
-    }
 }
 
-// TODO: This is basically `draw_surface_tree` from anvil.
-// Can we move this somewhere, where it is also usable for other things then windows?
-// Maybe add this as a helper function for surfaces to `backend::renderer`?
-// How do we handle SurfaceState in that case? Like we need a closure to
-// store and retrieve textures for arbitrary surface trees? Or leave this to the
-// compositor, but that feels like a lot of unnecessary code dublication.
+fn damage_from_surface_tree<P>(surface: &wl_surface::WlSurface, location: P) -> Vec<Rectangle<i32, Logical>>
+where
+    P: Into<Point<i32, Logical>>,
+{
+    let mut damage = Vec::new();
+    with_surface_tree_upward(
+        surface,
+        location.into(),
+        |_surface, states, location| {
+            let mut location = *location;
+            if let Some(data) = states.data_map.get::<RefCell<SurfaceState>>() {
+                let data = data.borrow();
+                if data.texture.is_none() {
+                    if states.role == Some("subsurface") {
+                        let current = states.cached_state.current::<SubsurfaceCachedState>();
+                        location += current.location;
+                    }
+                    return TraversalAction::DoChildren(location);
+                }
+            }
+            TraversalAction::SkipChildren
+        },
+        |_surface, states, location| {
+            let mut location = *location;
+            if let Some(data) = states.data_map.get::<RefCell<SurfaceState>>() {
+                let data = data.borrow();
+                let attributes = states.cached_state.current::<SurfaceAttributes>();
 
-// TODO: This does not handle ImportAll errors properly and uses only one texture slot.
-// This means it is *not* compatible with MultiGPU setups at all.
-// Current plan is to make sure it does not crash at least in that case and later add
-// A `MultiGpuManager` that opens gpus automatically, creates renderers for them,
-// implements `Renderer` and `ImportAll` itself and dispatches everything accordingly,
-// even copying buffers if necessary. This abstraction will likely also handle dmabuf-
-// protocol(s) (and maybe explicit sync?). Then this function will be fine and all the
-// gore of handling multiple gpus will be hidden away for most if not all cases.
+                if data.texture.is_none() {
+                    if states.role == Some("subsurface") {
+                        let current = states.cached_state.current::<SubsurfaceCachedState>();
+                        location += current.location;
+                    }
 
-// TODO: This function does not crop or scale windows to fit into a space.
-// How do we want to handle this? Add an additional size property to a window?
-// Let the user specify the max size and the method to handle it?
+                    damage.extend(attributes.damage.iter().map(|dmg| {
+                        let mut rect = match dmg {
+                            Damage::Buffer(rect) => rect.to_logical(attributes.buffer_scale),
+                            Damage::Surface(rect) => *rect,
+                        };
+                        rect.loc += location;
+                        rect
+                    }));
+                }
+            }
+        },
+        |_, _, _| true,
+    );
+    damage
+}
+
+fn bbox_from_surface_tree<P>(surface: &wl_surface::WlSurface, location: P) -> Rectangle<i32, Logical>
+where
+    P: Into<Point<i32, Logical>>,
+{
+    let location = location.into();
+    let mut bounding_box = Rectangle::from_loc_and_size(location, (0, 0));
+    with_surface_tree_downward(
+        surface,
+        location,
+        |_, states, loc: &Point<i32, Logical>| {
+            let mut loc = *loc;
+            let data = states.data_map.get::<RefCell<SurfaceState>>();
+
+            if let Some(size) = data.and_then(|d| d.borrow().size()) {
+                if states.role == Some("subsurface") {
+                    let current = states.cached_state.current::<SubsurfaceCachedState>();
+                    loc += current.location;
+                }
+
+                // Update the bounding box.
+                bounding_box = bounding_box.merge(Rectangle::from_loc_and_size(loc, size));
+
+                TraversalAction::DoChildren(loc)
+            } else {
+                // If the parent surface is unmapped, then the child surfaces are hidden as
+                // well, no need to consider them here.
+                TraversalAction::SkipChildren
+            }
+        },
+        |_, _, _| {},
+        |_, _, _| true,
+    );
+    bounding_box
+}
+
+pub fn under_from_surface_tree<P>(
+    surface: &wl_surface::WlSurface,
+    point: Point<f64, Logical>,
+    location: P,
+) -> Option<(wl_surface::WlSurface, Point<i32, Logical>)>
+where
+    P: Into<Point<i32, Logical>>,
+{
+    let found = RefCell::new(None);
+    with_surface_tree_downward(
+        surface,
+        location.into(),
+        |wl_surface, states, location: &Point<i32, Logical>| {
+            let mut location = *location;
+            let data = states.data_map.get::<RefCell<SurfaceState>>();
+
+            if states.role == Some("subsurface") {
+                let current = states.cached_state.current::<SubsurfaceCachedState>();
+                location += current.location;
+            }
+
+            let contains_the_point = data
+                .map(|data| {
+                    data.borrow()
+                        .contains_point(&*states.cached_state.current(), point - location.to_f64())
+                })
+                .unwrap_or(false);
+            if contains_the_point {
+                *found.borrow_mut() = Some((wl_surface.clone(), location));
+            }
+
+            TraversalAction::DoChildren(location)
+        },
+        |_, _, _| {},
+        |_, _, _| {
+            // only continue if the point is not found
+            found.borrow().is_none()
+        },
+    );
+    found.into_inner()
+}
+
+fn send_frames_surface_tree(surface: &wl_surface::WlSurface, time: u32) {
+    with_surface_tree_downward(
+        surface,
+        (),
+        |_, _, &()| TraversalAction::DoChildren(()),
+        |_surf, states, &()| {
+            // the surface may not have any user_data if it is a subsurface and has not
+            // yet been commited
+            for callback in states
+                .cached_state
+                .current::<SurfaceAttributes>()
+                .frame_callbacks
+                .drain(..)
+            {
+                callback.done(time);
+            }
+        },
+        |_, _, &()| true,
+    );
+}
 
 pub fn draw_window<R, E, F, T>(
     renderer: &mut R,
@@ -473,89 +479,17 @@ where
     E: std::error::Error,
     T: Texture + 'static,
 {
-    let mut result = Ok(());
-    if let Some(surface) = window.0.toplevel.get_surface() {
-        with_surface_tree_upward(
-            surface,
-            location,
-            |_surface, states, location| {
-                let mut location = *location;
-                if let Some(data) = states.data_map.get::<RefCell<SurfaceState>>() {
-                    let mut data = data.borrow_mut();
-                    let attributes = states.cached_state.current::<SurfaceAttributes>();
-                    // Import a new buffer if necessary
-                    if data.texture.is_none() {
-                        if let Some(buffer) = data.buffer.as_ref() {
-                            let damage = attributes
-                                .damage
-                                .iter()
-                                .map(|dmg| match dmg {
-                                    Damage::Buffer(rect) => *rect,
-                                    // TODO also apply transformations
-                                    Damage::Surface(rect) => rect.to_buffer(attributes.buffer_scale),
-                                })
-                                .collect::<Vec<_>>();
-
-                            match renderer.import_buffer(buffer, Some(states), &damage) {
-                                Some(Ok(m)) => {
-                                    data.texture = Some(Box::new(m));
-                                }
-                                Some(Err(err)) => {
-                                    slog::warn!(log, "Error loading buffer: {}", err);
-                                }
-                                None => {
-                                    slog::error!(log, "Unknown buffer format for: {:?}", buffer);
-                                }
-                            }
-                        }
-                    }
-                    // Now, should we be drawn ?
-                    if data.texture.is_some() {
-                        // if yes, also process the children
-                        if states.role == Some("subsurface") {
-                            let current = states.cached_state.current::<SubsurfaceCachedState>();
-                            location += current.location;
-                        }
-                        TraversalAction::DoChildren(location)
-                    } else {
-                        // we are not displayed, so our children are neither
-                        TraversalAction::SkipChildren
-                    }
-                } else {
-                    // we are not displayed, so our children are neither
-                    TraversalAction::SkipChildren
-                }
-            },
-            |_surface, states, location| {
-                let mut location = *location;
-                if let Some(data) = states.data_map.get::<RefCell<SurfaceState>>() {
-                    let mut data = data.borrow_mut();
-                    let buffer_scale = data.buffer_scale;
-                    let attributes = states.cached_state.current::<SurfaceAttributes>();
-                    if let Some(texture) = data.texture.as_mut().and_then(|x| x.downcast_mut::<T>()) {
-                        // we need to re-extract the subsurface offset, as the previous closure
-                        // only passes it to our children
-                        if states.role == Some("subsurface") {
-                            let current = states.cached_state.current::<SubsurfaceCachedState>();
-                            location += current.location;
-                        }
-                        // TODO: Take wp_viewporter into account
-                        if let Err(err) = frame.render_texture_at(
-                            texture,
-                            location.to_f64().to_physical(scale).to_i32_round(),
-                            buffer_scale,
-                            scale,
-                            attributes.buffer_transform.into(),
-                            1.0,
-                        ) {
-                            result = Err(err);
-                        }
-                    }
-                }
-            },
-            |_, _, _| true,
-        );
+    if let Some(surface) = window.toplevel().get_surface() {
+        draw_surface_tree(renderer, frame, surface, scale, location, log)?;
+        for (popup, p_location) in PopupManager::popups_for_surface(surface)
+            .ok()
+            .into_iter()
+            .flatten()
+        {
+            if let Some(surface) = popup.get_surface() {
+                draw_surface_tree(renderer, frame, surface, scale, location + p_location, log)?;
+            }
+        }
     }
-
-    result
+    Ok(())
 }