Merge branch 'Smithay:master' into master

CHANGELOG.md

@@ -13,6 +13,7 @@
 - `PointerButtonEvent::button` now returns an `Option<MouseButton>`.
 - `MouseButton` is now non-exhaustive.
 - Remove `Other` and add `Forward` and `Back` variants to `MouseButton`. Use the new `PointerButtonEvent::button_code` in place of `Other`.
+- `GrabStartData` has been renamed to `PointerGrabStartData`
 
 #### Backends
 
@@ -49,6 +50,7 @@
 - Support for `xdg_wm_base` protocol version 3
 - Added the option to initialize the dmabuf global with a client filter
 - `wayland::output::Output` now has user data attached to it and more functions to query its properties
+- Added a `KeyboardGrab` similar to the existing `PointerGrab`
 
 #### Backends
 

Cargo.toml

@@ -25,9 +25,9 @@ cgmath = "0.18.0"
 dbus = { version = "0.9.0", optional = true }
 downcast-rs = "1.2.0"
 drm-fourcc = "^2.1.1"
-drm = { version = "0.5.0", optional = true }
+drm = { version = "0.6.1", optional = true }
-drm-ffi = { version = "0.2.0", optional = true }
+drm-ffi = { version = "0.2.1", optional = true }
-gbm = { version = "0.7.0", optional = true, default-features = false, features = ["drm-support"] }
+gbm = { version = "0.8.0", optional = true, default-features = false, features = ["drm-support"] }
 input = { version = "0.7", default-features = false, features=["libinput_1_14"], optional = true }
 indexmap = { version = "1.7", optional = true }
 lazy_static = "1"

anvil/src/drawing.rs

@@ -7,9 +7,11 @@ use image::{ImageBuffer, Rgba};
 use slog::Logger;
 #[cfg(feature = "image")]
 use smithay::backend::renderer::gles2::{Gles2Error, Gles2Renderer, Gles2Texture};
+#[cfg(feature = "debug")]
+use smithay::utils::Transform;
 use smithay::{
     backend::{
-        renderer::{buffer_type, BufferType, Frame, ImportAll, Renderer, Texture, Transform},
+        renderer::{buffer_type, BufferType, Frame, ImportAll, Renderer, Texture},
         SwapBuffersError,
     },
     reexports::wayland_server::protocol::{wl_buffer, wl_surface},
@@ -111,7 +113,11 @@ where
                             .map(|dmg| match dmg {
                                 Damage::Buffer(rect) => *rect,
                                 // TODO also apply transformations
-                                Damage::Surface(rect) => rect.to_buffer(attributes.buffer_scale),
+                                Damage::Surface(rect) => rect.to_buffer(
+                                    attributes.buffer_scale,
+                                    attributes.buffer_transform.into(),
+                                    &data.size().unwrap(),
+                                ),
                             })
                             .collect::<Vec<_>>();
 
@@ -161,6 +167,7 @@ where
             if let Some(data) = states.data_map.get::<RefCell<SurfaceData>>() {
                 let mut data = data.borrow_mut();
                 let buffer_scale = data.buffer_scale;
+                let buffer_transform = data.buffer_transform;
                 if let Some(texture) = data
                     .texture
                     .as_mut()
@@ -177,7 +184,7 @@ where
                         location.to_f64().to_physical(output_scale as f64).to_i32_round(),
                         buffer_scale,
                         output_scale as f64,
-                        Transform::Normal, /* TODO */
+                        buffer_transform,
                         &[Rectangle::from_loc_and_size((0, 0), (i32::MAX, i32::MAX))],
                         1.0,
                     ) {

anvil/src/shell.rs

@@ -13,13 +13,13 @@ use smithay::{
             Display,
         },
     },
-    utils::{Logical, Physical, Point, Rectangle, Size},
+    utils::{Buffer, Logical, Point, Rectangle, Size, Transform},
     wayland::{
         compositor::{
             compositor_init, is_sync_subsurface, with_states, with_surface_tree_upward, BufferAssignment,
             SurfaceAttributes, TraversalAction,
         },
-        seat::{AxisFrame, GrabStartData, PointerGrab, PointerInnerHandle, Seat},
+        seat::{AxisFrame, PointerGrab, PointerGrabStartData, PointerInnerHandle, Seat},
         shell::{
             legacy::{wl_shell_init, ShellRequest, ShellState as WlShellState, ShellSurfaceKind},
             wlr_layer::{LayerShellRequest, LayerSurfaceAttributes},
@@ -39,7 +39,7 @@ use crate::{
 };
 
 struct MoveSurfaceGrab {
-    start_data: GrabStartData,
+    start_data: PointerGrabStartData,
     window_map: Rc<RefCell<WindowMap>>,
     toplevel: SurfaceKind,
     initial_window_location: Point<i32, Logical>,
@@ -82,7 +82,7 @@ impl PointerGrab for MoveSurfaceGrab {
         handle.axis(details)
     }
 
-    fn start_data(&self) -> &GrabStartData {
+    fn start_data(&self) -> &PointerGrabStartData {
         &self.start_data
     }
 }
@@ -130,7 +130,7 @@ impl From<ResizeEdge> for xdg_toplevel::ResizeEdge {
 }
 
 struct ResizeSurfaceGrab {
-    start_data: GrabStartData,
+    start_data: PointerGrabStartData,
     toplevel: SurfaceKind,
     edges: ResizeEdge,
     initial_window_size: Size<i32, Logical>,
@@ -280,7 +280,7 @@ impl PointerGrab for ResizeSurfaceGrab {
         handle.axis(details)
     }
 
-    fn start_data(&self) -> &GrabStartData {
+    fn start_data(&self) -> &PointerGrabStartData {
         &self.start_data
     }
 }
@@ -947,8 +947,9 @@ pub struct SurfaceData {
     pub texture: Option<Box<dyn std::any::Any + 'static>>,
     pub geometry: Option<Rectangle<i32, Logical>>,
     pub resize_state: ResizeState,
-    pub buffer_dimensions: Option<Size<i32, Physical>>,
+    pub buffer_dimensions: Option<Size<i32, Buffer>>,
     pub buffer_scale: i32,
+    pub buffer_transform: Transform,
 }
 
 impl SurfaceData {
@@ -958,6 +959,7 @@ impl SurfaceData {
                 // new contents
                 self.buffer_dimensions = buffer_dimensions(&buffer);
                 self.buffer_scale = attrs.buffer_scale;
+                self.buffer_transform = attrs.buffer_transform.into();
                 if let Some(old_buffer) = std::mem::replace(&mut self.buffer, Some(buffer)) {
                     old_buffer.release();
                 }
@@ -976,7 +978,7 @@ impl SurfaceData {
     /// Returns the size of the surface.
     pub fn size(&self) -> Option<Size<i32, Logical>> {
         self.buffer_dimensions
-            .map(|dims| dims.to_logical(self.buffer_scale))
+            .map(|dims| dims.to_logical(self.buffer_scale, self.buffer_transform))
     }
 
     /// Checks if the surface's input region contains the point.

anvil/src/udev.rs

@@ -20,7 +20,7 @@ use smithay::{
         libinput::{LibinputInputBackend, LibinputSessionInterface},
         renderer::{
             gles2::{Gles2Renderer, Gles2Texture},
-            Bind, Frame, Renderer, Transform,
+            Bind, Frame, Renderer,
         },
         session::{auto::AutoSession, Session, Signal as SessionSignal},
         udev::{UdevBackend, UdevEvent},
@@ -50,7 +50,7 @@ use smithay::{
     },
     utils::{
         signaling::{Linkable, SignalToken, Signaler},
-        Logical, Point, Rectangle,
+        Logical, Point, Rectangle, Transform,
     },
     wayland::{
         output::{Mode, PhysicalProperties},
@@ -264,7 +264,7 @@ pub fn run_udev(log: Logger) {
     event_loop.handle().remove(udev_event_source);
 }
 
-pub type RenderSurface = GbmBufferedSurface<SessionFd>;
+pub type RenderSurface = GbmBufferedSurface<Rc<RefCell<GbmDevice<SessionFd>>>, SessionFd>;
 
 struct SurfaceData {
     surface: RenderSurface,
@@ -279,7 +279,7 @@ struct BackendData {
     #[cfg(feature = "debug")]
     fps_texture: Gles2Texture,
     renderer: Rc<RefCell<Gles2Renderer>>,
-    gbm: GbmDevice<SessionFd>,
+    gbm: Rc<RefCell<GbmDevice<SessionFd>>>,
     registration_token: RegistrationToken,
     event_dispatcher: Dispatcher<'static, DrmDevice<SessionFd>, AnvilState<UdevData>>,
     dev_id: u64,
@@ -287,7 +287,7 @@ struct BackendData {
 
 fn scan_connectors(
     device: &mut DrmDevice<SessionFd>,
-    gbm: &GbmDevice<SessionFd>,
+    gbm: &Rc<RefCell<GbmDevice<SessionFd>>>,
     renderer: &mut Gles2Renderer,
     output_map: &mut crate::output_map::OutputMap,
     signaler: &Signaler<SessionSignal>,
@@ -483,6 +483,7 @@ impl AnvilState<UdevData> {
                 }
             }
 
+            let gbm = Rc::new(RefCell::new(gbm));
             let backends = Rc::new(RefCell::new(scan_connectors(
                 &mut device,
                 &gbm,

anvil/src/winit.rs

@@ -9,7 +9,7 @@ use smithay::{
 };
 use smithay::{
     backend::{
-        renderer::{Renderer, Transform},
+        renderer::Renderer,
         winit::{self, WinitEvent},
         SwapBuffersError,
     },
@@ -17,6 +17,7 @@ use smithay::{
         calloop::EventLoop,
         wayland_server::{protocol::wl_output, Display},
     },
+    utils::Transform,
     wayland::{
         output::{Mode, PhysicalProperties},
         seat::CursorImageStatus,

anvil/src/x11.rs

@@ -11,7 +11,7 @@ use smithay::{backend::renderer::ImportDma, wayland::dmabuf::init_dmabuf_global}
 use smithay::{
     backend::{
         egl::{EGLContext, EGLDisplay},
-        renderer::{gles2::Gles2Renderer, Bind, ImportEgl, Renderer, Transform, Unbind},
+        renderer::{gles2::Gles2Renderer, Bind, ImportEgl, Renderer, Unbind},
         x11::{WindowBuilder, X11Backend, X11Event, X11Surface},
         SwapBuffersError,
     },
@@ -20,6 +20,7 @@ use smithay::{
         gbm,
         wayland_server::{protocol::wl_output, Display},
     },
+    utils::Transform,
     wayland::{
         output::{Mode, PhysicalProperties},
         seat::CursorImageStatus,

src/backend/allocator/swapchain.rs

@@ -154,11 +154,13 @@ where
     /// Mark a given buffer as submitted.
     ///
     /// This might effect internal data (e.g. buffer age) and may only be called,
-    /// if the buffer was actually used for display.
+    /// the buffer may not be used for rendering anymore.
+    /// You may hold on to it, if you require keeping it alive.
+    ///
     /// Buffers can always just be safely discarded by dropping them, but not
-    /// calling this function may affect performance characteristics
+    /// calling this function before may affect performance characteristics
     /// (e.g. by not tracking the buffer age).
-    pub fn submitted(&self, slot: Slot<B>) {
+    pub fn submitted(&self, slot: &Slot<B>) {
         // don't mess up the state, if the user submitted and old buffer, after e.g. a resize
         if !self.slots.iter().any(|other| Arc::ptr_eq(&slot.0, other)) {
             return;
@@ -167,7 +169,7 @@ where
         slot.0.age.store(1, Ordering::SeqCst);
         for other_slot in &self.slots {
             if !Arc::ptr_eq(other_slot, &slot.0) && other_slot.buffer.is_some() {
-                assert!(other_slot
+                let res = other_slot
                     .age
                     .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |age| {
                         if age > 0 {
@@ -175,8 +177,8 @@ where
                         } else {
                             Some(0)
                         }
-                    })
+                    });
-                    .is_ok());
+                assert!(res.is_ok());
             }
         }
     }

src/backend/drm/device/atomic.rs

@@ -242,7 +242,7 @@ impl<A: AsRawFd + 'static> AtomicDrmDevice<A> {
             req.add_property(*crtc, *mode_prop, property::Value::Unknown(0));
         }
         self.fd
-            .atomic_commit(&[AtomicCommitFlags::AllowModeset], req)
+            .atomic_commit(AtomicCommitFlags::ALLOW_MODESET, req)
             .map_err(|source| Error::Access {
                 errmsg: "Failed to disable connectors",
                 dev: self.fd.dev_path(),
@@ -282,7 +282,7 @@ impl<A: AsRawFd + 'static> Drop for AtomicDrmDevice<A> {
             add_multiple_props(&mut req, &self.old_state.2);
             add_multiple_props(&mut req, &self.old_state.3);
 
-            if let Err(err) = self.fd.atomic_commit(&[AtomicCommitFlags::AllowModeset], req) {
+            if let Err(err) = self.fd.atomic_commit(AtomicCommitFlags::ALLOW_MODESET, req) {
                 error!(self.logger, "Failed to restore previous state. Error: {}", err);
             }
         }

src/backend/drm/surface/atomic.rs

@@ -249,7 +249,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
             )?;
             self.fd
                 .atomic_commit(
-                    &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
+                    AtomicCommitFlags::ALLOW_MODESET | AtomicCommitFlags::TEST_ONLY,
                     req,
                 )
                 .map_err(|_| Error::TestFailed(self.crtc))?;
@@ -287,7 +287,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
         )?;
         self.fd
             .atomic_commit(
-                &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
+                AtomicCommitFlags::ALLOW_MODESET | AtomicCommitFlags::TEST_ONLY,
                 req,
             )
             .map_err(|_| Error::TestFailed(self.crtc))?;
@@ -327,7 +327,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
 
         self.fd
             .atomic_commit(
-                &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
+                AtomicCommitFlags::ALLOW_MODESET | AtomicCommitFlags::TEST_ONLY,
                 req,
             )
             .map_err(|_| Error::TestFailed(self.crtc))?;
@@ -367,7 +367,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
         if let Err(err) = self
             .fd
             .atomic_commit(
-                &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
+                AtomicCommitFlags::ALLOW_MODESET | AtomicCommitFlags::TEST_ONLY,
                 req,
             )
             .map_err(|_| Error::TestFailed(self.crtc))
@@ -428,7 +428,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
         )?;
         self.fd
             .atomic_commit(
-                &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
+                AtomicCommitFlags::ALLOW_MODESET | AtomicCommitFlags::TEST_ONLY,
                 req,
             )
             .map_err(|_| Error::TestFailed(self.crtc))?;
@@ -502,7 +502,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
             if let Err(err) = self
                 .fd
                 .atomic_commit(
-                    &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
+                    AtomicCommitFlags::ALLOW_MODESET | AtomicCommitFlags::TEST_ONLY,
                     req.clone(),
                 )
                 .map_err(|_| Error::TestFailed(self.crtc))
@@ -530,21 +530,18 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
             .fd
             .atomic_commit(
                 if event {
-                    &[
+                    // on the atomic api we can modeset and trigger a page_flip event on the same call!
-                        // on the atomic api we can modeset and trigger a page_flip event on the same call!
+                    AtomicCommitFlags::PAGE_FLIP_EVENT | AtomicCommitFlags::ALLOW_MODESET
-                        AtomicCommitFlags::PageFlipEvent,
+                    // we also *should* not need to wait for completion, like with `set_crtc`,
-                        AtomicCommitFlags::AllowModeset,
+                    // because we have tested this exact commit already, so we do not expect any errors later down the line.
-                        // we also *should* not need to wait for completion, like with `set_crtc`,
+                    //
-                        // because we have tested this exact commit already, so we do not expect any errors later down the line.
+                    // but there is always an exception and `amdgpu` can fail in interesting ways with this flag set...
-                        //
+                    // https://gitlab.freedesktop.org/drm/amd/-/issues?scope=all&utf8=%E2%9C%93&state=opened&search=drm_atomic_helper_wait_for_flip_done
-                        // but there is always an exception and `amdgpu` can fail in interesting ways with this flag set...
+                    //
-                        // https://gitlab.freedesktop.org/drm/amd/-/issues?scope=all&utf8=%E2%9C%93&state=opened&search=drm_atomic_helper_wait_for_flip_done
+                    // so we skip this flag:
-                        //
+                    // AtomicCommitFlags::Nonblock,
-                        // so we skip this flag:
-                        // AtomicCommitFlags::Nonblock,
-                    ]
                 } else {
-                    &[AtomicCommitFlags::AllowModeset]
+                    AtomicCommitFlags::ALLOW_MODESET
                 },
                 req,
             )
@@ -588,9 +585,9 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
         self.fd
             .atomic_commit(
                 if event {
-                    &[AtomicCommitFlags::PageFlipEvent, AtomicCommitFlags::Nonblock]
+                    AtomicCommitFlags::PAGE_FLIP_EVENT | AtomicCommitFlags::NONBLOCK
                 } else {
-                    &[AtomicCommitFlags::Nonblock]
+                    AtomicCommitFlags::NONBLOCK
                 },
                 req,
             )
@@ -638,7 +635,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
         let result = self
             .fd
             .atomic_commit(
-                &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
+                AtomicCommitFlags::ALLOW_MODESET | AtomicCommitFlags::TEST_ONLY,
                 req,
             )
             .is_ok();
@@ -676,7 +673,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
         let result = self
             .fd
             .atomic_commit(
-                &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
+                AtomicCommitFlags::ALLOW_MODESET | AtomicCommitFlags::TEST_ONLY,
                 req,
             )
             .is_ok();
@@ -937,7 +934,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurface<A> {
 
         let result = self
             .fd
-            .atomic_commit(&[AtomicCommitFlags::Nonblock], req)
+            .atomic_commit(AtomicCommitFlags::NONBLOCK, req)
             .map_err(|source| Error::Access {
                 errmsg: "Failed to commit on clear_plane",
                 dev: self.fd.dev_path(),
@@ -1029,7 +1026,7 @@ impl<A: AsRawFd + 'static> Drop for AtomicDrmSurface<A> {
 
         req.add_property(self.crtc, *active_prop, property::Value::Boolean(false));
         req.add_property(self.crtc, *mode_prop, property::Value::Unknown(0));
-        if let Err(err) = self.fd.atomic_commit(&[AtomicCommitFlags::AllowModeset], req) {
+        if let Err(err) = self.fd.atomic_commit(AtomicCommitFlags::ALLOW_MODESET, req) {
             warn!(self.logger, "Unable to disable connectors: {}", err);
         }
     }

src/backend/drm/surface/gbm.rs

@@ -4,12 +4,12 @@ use std::sync::Arc;
 
 use drm::buffer::PlanarBuffer;
 use drm::control::{connector, crtc, framebuffer, plane, Device, Mode};
-use gbm::{BufferObject, Device as GbmDevice};
+use gbm::BufferObject;
 
 use crate::backend::allocator::{
     dmabuf::{AsDmabuf, Dmabuf},
     gbm::GbmConvertError,
-    Format, Fourcc, Modifier, Slot, Swapchain,
+    Allocator, Format, Fourcc, Modifier, Slot, Swapchain,
 };
 use crate::backend::drm::{device::DevPath, surface::DrmSurfaceInternal, DrmError, DrmSurface};
 use crate::backend::SwapBuffersError;
@@ -18,17 +18,19 @@ use slog::{debug, error, o, trace, warn};
 
 /// Simplified abstraction of a swapchain for gbm-buffers displayed on a [`DrmSurface`].
 #[derive(Debug)]
-pub struct GbmBufferedSurface<D: AsRawFd + 'static> {
+pub struct GbmBufferedSurface<A: Allocator<BufferObject<()>> + 'static, D: AsRawFd + 'static> {
     current_fb: Slot<BufferObject<()>>,
     pending_fb: Option<Slot<BufferObject<()>>>,
     queued_fb: Option<Slot<BufferObject<()>>>,
     next_fb: Option<Slot<BufferObject<()>>>,
-    swapchain: Swapchain<GbmDevice<D>, BufferObject<()>>,
+    swapchain: Swapchain<A, BufferObject<()>>,
     drm: Arc<DrmSurface<D>>,
 }
 
-impl<D> GbmBufferedSurface<D>
+impl<A, D> GbmBufferedSurface<A, D>
 where
+    A: Allocator<BufferObject<()>>,
+    A::Error: std::error::Error + Send + Sync,
     D: AsRawFd + 'static,
 {
     /// Create a new `GbmBufferedSurface` from a given compatible combination
@@ -40,10 +42,10 @@ where
     #[allow(clippy::type_complexity)]
     pub fn new<L>(
         drm: DrmSurface<D>,
-        allocator: GbmDevice<D>,
+        allocator: A,
         mut renderer_formats: HashSet<Format>,
         log: L,
-    ) -> Result<GbmBufferedSurface<D>, Error>
+    ) -> Result<GbmBufferedSurface<A, D>, Error<A::Error>>
     where
         L: Into<Option<::slog::Logger>>,
     {
@@ -121,7 +123,7 @@ where
 
         let mode = drm.pending_mode();
 
-        let mut swapchain: Swapchain<GbmDevice<D>, BufferObject<()>> = Swapchain::new(
+        let mut swapchain: Swapchain<A, BufferObject<()>> = Swapchain::new(
             allocator,
             mode.size().0 as u32,
             mode.size().1 as u32,
@@ -130,7 +132,7 @@ where
         );
 
         // Test format
-        let buffer = swapchain.acquire()?.unwrap();
+        let buffer = swapchain.acquire().map_err(Error::GbmError)?.unwrap();
         let format = Format {
             code,
             modifier: buffer.modifier().unwrap(), // no guarantee
@@ -171,9 +173,13 @@ where
     ///
     /// *Note*: This function can be called multiple times and
     /// will return the same buffer until it is queued (see [`GbmBufferedSurface::queue_buffer`]).
-    pub fn next_buffer(&mut self) -> Result<(Dmabuf, u8), Error> {
+    pub fn next_buffer(&mut self) -> Result<(Dmabuf, u8), Error<A::Error>> {
         if self.next_fb.is_none() {
-            let slot = self.swapchain.acquire()?.ok_or(Error::NoFreeSlotsError)?;
+            let slot = self
+                .swapchain
+                .acquire()
+                .map_err(Error::GbmError)?
+                .ok_or(Error::NoFreeSlotsError)?;
 
             let maybe_buffer = slot.userdata().get::<Dmabuf>().cloned();
             if maybe_buffer.is_none() {
@@ -197,7 +203,7 @@ where
     /// *Note*: This function needs to be followed up with [`GbmBufferedSurface::frame_submitted`]
     /// when a vblank event is received, that denotes successful scanout of the buffer.
     /// Otherwise the underlying swapchain will eventually run out of buffers.
-    pub fn queue_buffer(&mut self) -> Result<(), Error> {
+    pub fn queue_buffer(&mut self) -> Result<(), Error<A::Error>> {
         self.queued_fb = self.next_fb.take();
         if self.pending_fb.is_none() && self.queued_fb.is_some() {
             self.submit()?;
@@ -210,10 +216,9 @@ where
     /// *Note*: Needs to be called, after the vblank event of the matching [`DrmDevice`](super::super::DrmDevice)
     /// was received after calling [`GbmBufferedSurface::queue_buffer`] on this surface.
     /// Otherwise the underlying swapchain will run out of buffers eventually.
-    pub fn frame_submitted(&mut self) -> Result<(), Error> {
+    pub fn frame_submitted(&mut self) -> Result<(), Error<A::Error>> {
         if let Some(mut pending) = self.pending_fb.take() {
             std::mem::swap(&mut pending, &mut self.current_fb);
-            self.swapchain.submitted(pending);
             if self.queued_fb.is_some() {
                 self.submit()?;
             }
@@ -222,7 +227,7 @@ where
         Ok(())
     }
 
-    fn submit(&mut self) -> Result<(), Error> {
+    fn submit(&mut self) -> Result<(), Error<A::Error>> {
         // yes it does not look like it, but both of these lines should be safe in all cases.
         let slot = self.queued_fb.take().unwrap();
         let fb = slot.userdata().get::<FbHandle<D>>().unwrap().fb;
@@ -233,6 +238,7 @@ where
             self.drm.page_flip([(fb, self.drm.plane())].iter(), true)
         };
         if flip.is_ok() {
+            self.swapchain.submitted(&slot);
             self.pending_fb = Some(slot);
         }
         flip.map_err(Error::DrmError)
@@ -276,13 +282,13 @@ where
     /// (e.g. no suitable [`encoder`](drm::control::encoder) may be found)
     /// or is not compatible with the currently pending
     /// [`Mode`](drm::control::Mode).
-    pub fn add_connector(&self, connector: connector::Handle) -> Result<(), Error> {
+    pub fn add_connector(&self, connector: connector::Handle) -> Result<(), Error<A::Error>> {
         self.drm.add_connector(connector).map_err(Error::DrmError)
     }
 
     /// Tries to mark a [`connector`](drm::control::connector)
     /// for removal on the next commit.    
-    pub fn remove_connector(&self, connector: connector::Handle) -> Result<(), Error> {
+    pub fn remove_connector(&self, connector: connector::Handle) -> Result<(), Error<A::Error>> {
         self.drm.remove_connector(connector).map_err(Error::DrmError)
     }
 
@@ -292,7 +298,7 @@ where
     /// (e.g. no suitable [`encoder`](drm::control::encoder) may be found)
     /// or is not compatible with the currently pending
     /// [`Mode`](drm::control::Mode).    
-    pub fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Error> {
+    pub fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Error<A::Error>> {
         self.drm.set_connectors(connectors).map_err(Error::DrmError)
     }
 
@@ -314,7 +320,7 @@ where
     /// Fails if the mode is not compatible with the underlying
     /// [`crtc`](drm::control::crtc) or any of the
     /// pending [`connector`](drm::control::connector)s.
-    pub fn use_mode(&mut self, mode: Mode) -> Result<(), Error> {
+    pub fn use_mode(&mut self, mode: Mode) -> Result<(), Error<A::Error>> {
         self.drm.use_mode(mode).map_err(Error::DrmError)?;
         let (w, h) = mode.size();
         self.swapchain.resize(w as _, h as _);
@@ -334,9 +340,10 @@ impl<A: AsRawFd + 'static> Drop for FbHandle<A> {
     }
 }
 
-fn attach_framebuffer<A>(drm: &Arc<DrmSurface<A>>, bo: &BufferObject<()>) -> Result<FbHandle<A>, Error>
+fn attach_framebuffer<E, D>(drm: &Arc<DrmSurface<D>>, bo: &BufferObject<()>) -> Result<FbHandle<D>, Error<E>>
 where
-    A: AsRawFd + 'static,
+    E: std::error::Error + Send + Sync,
+    D: AsRawFd + 'static,
 {
     let modifier = match bo.modifier().unwrap() {
         Modifier::Invalid => None,
@@ -385,7 +392,7 @@ where
 
 /// Errors thrown by a [`GbmBufferedSurface`]
 #[derive(Debug, thiserror::Error)]
-pub enum Error {
+pub enum Error<E: std::error::Error + Send + Sync + 'static> {
     /// No supported pixel format for the given plane could be determined
     #[error("No supported plane buffer format found")]
     NoSupportedPlaneFormat,
@@ -406,14 +413,14 @@ pub enum Error {
     DrmError(#[from] DrmError),
     /// Error importing the rendered buffer to libgbm for scan-out
     #[error("The underlying gbm device encounted an error: {0}")]
-    GbmError(#[from] std::io::Error),
+    GbmError(#[source] E),
     /// Error exporting as Dmabuf
     #[error("The allocated buffer could not be exported as a dmabuf: {0}")]
     AsDmabufError(#[from] GbmConvertError),
 }
 
-impl From<Error> for SwapBuffersError {
+impl<E: std::error::Error + Send + Sync + 'static> From<Error<E>> for SwapBuffersError {
-    fn from(err: Error) -> SwapBuffersError {
+    fn from(err: Error<E>) -> SwapBuffersError {
         match err {
             x @ Error::NoSupportedPlaneFormat
             | x @ Error::NoSupportedRendererFormat

src/backend/drm/surface/legacy.rs

@@ -289,18 +289,13 @@ impl<A: AsRawFd + 'static> LegacyDrmSurface<A> {
             // this will result in wasting a frame, because this flip will need to wait
             // for `set_crtc`, but is necessary to drive the event loop and thus provide
             // a more consistent api.
-            ControlDevice::page_flip(
+            ControlDevice::page_flip(&*self.fd, self.crtc, framebuffer, PageFlipFlags::EVENT, None).map_err(
-                &*self.fd,
+                |source| Error::Access {
-                self.crtc,
+                    errmsg: "Failed to queue page flip",
-                framebuffer,
+                    dev: self.fd.dev_path(),
-                &[PageFlipFlags::PageFlipEvent],
+                    source,
-                None,
+                },
-            )
+            )?;
-            .map_err(|source| Error::Access {
-                errmsg: "Failed to queue page flip",
-                dev: self.fd.dev_path(),
-                source,
-            })?;
         }
 
         Ok(())
@@ -318,9 +313,9 @@ impl<A: AsRawFd + 'static> LegacyDrmSurface<A> {
             self.crtc,
             framebuffer,
             if event {
-                &[PageFlipFlags::PageFlipEvent]
+                PageFlipFlags::EVENT
             } else {
-                &[]
+                PageFlipFlags::empty()
             },
             None,
         )

src/backend/egl/display.rs

@@ -839,7 +839,7 @@ impl EGLBufferReader {
     pub fn egl_buffer_dimensions(
         &self,
         buffer: &WlBuffer,
-    ) -> Option<crate::utils::Size<i32, crate::utils::Physical>> {
+    ) -> Option<crate::utils::Size<i32, crate::utils::Buffer>> {
         if !buffer.as_ref().is_alive() {
             debug!(self.logger, "Suplied buffer is no longer alive");
             return None;

src/backend/egl/surface.rs

@@ -81,18 +81,28 @@ impl EGLSurface {
     }
 
     /// Returns the buffer age of the underlying back buffer
-    pub fn buffer_age(&self) -> i32 {
+    pub fn buffer_age(&self) -> Option<i32> {
         let surface = self.surface.load(Ordering::SeqCst);
         let mut age = 0;
-        unsafe {
+        let ret = unsafe {
             ffi::egl::QuerySurface(
                 **self.display,
                 surface as *const _,
                 ffi::egl::BUFFER_AGE_EXT as i32,
                 &mut age as *mut _,
+            )
+        };
+        if ret == ffi::egl::FALSE {
+            slog::debug!(
+                self.logger,
+                "Failed to query buffer age value for surface {:?}: {}",
+                self,
+                EGLError::from_last_call().unwrap_err()
             );
+            None
+        } else {
+            Some(age)
         }
-        age
     }
 
     /// Swaps buffers at the end of a frame.

src/backend/renderer/gles2/mod.rs

@@ -13,7 +13,7 @@ use cgmath::{prelude::*, Matrix3, Vector2, Vector3};
 mod shaders;
 mod version;
 
-use super::{Bind, Frame, Renderer, Texture, TextureFilter, Transform, Unbind};
+use super::{Bind, Frame, Renderer, Texture, TextureFilter, Unbind};
 use crate::backend::allocator::{
     dmabuf::{Dmabuf, WeakDmabuf},
     Format,
@@ -23,7 +23,7 @@ use crate::backend::egl::{
     EGLContext, EGLSurface, MakeCurrentError,
 };
 use crate::backend::SwapBuffersError;
-use crate::utils::{Buffer, Physical, Rectangle, Size};
+use crate::utils::{Buffer, Physical, Rectangle, Size, Transform};
 
 #[cfg(all(feature = "wayland_frontend", feature = "use_system_lib"))]
 use super::ImportEgl;
@@ -874,6 +874,7 @@ impl Bind<Rc<EGLSurface>> for Gles2Renderer {
     fn bind(&mut self, surface: Rc<EGLSurface>) -> Result<(), Gles2Error> {
         self.unbind()?;
         self.target_surface = Some(surface);
+        self.make_current()?;
         Ok(())
     }
 }
@@ -881,9 +882,7 @@ impl Bind<Rc<EGLSurface>> for Gles2Renderer {
 impl Bind<Dmabuf> for Gles2Renderer {
     fn bind(&mut self, dmabuf: Dmabuf) -> Result<(), Gles2Error> {
         self.unbind()?;
-        unsafe {
+        self.make_current()?;
-            self.egl.make_current()?;
-        }
 
         // Free outdated buffer resources
         // TODO: Replace with `drain_filter` once it lands
@@ -1250,7 +1249,7 @@ impl Frame for Gles2Frame {
         texture: &Self::TextureId,
         src: Rectangle<i32, Buffer>,
         dest: Rectangle<f64, Physical>,
-        damage: &[Rectangle<i32, Physical>],
+        damage: &[Rectangle<i32, Buffer>],
         transform: Transform,
         alpha: f32,
     ) -> Result<(), Self::Error> {
@@ -1266,7 +1265,7 @@ impl Frame for Gles2Frame {
             assert_eq!(mat, mat * transform.invert().matrix());
             assert_eq!(transform.matrix(), Matrix3::<f32>::identity());
         }
-        mat = mat * transform.invert().matrix();
+        mat = mat * transform.matrix();
         mat = mat * Matrix3::from_translation(Vector2::new(-0.5, -0.5));
 
         // this matrix should be regular, we can expect invert to succeed
@@ -1290,18 +1289,22 @@ impl Frame for Gles2Frame {
         let damage = damage
             .iter()
             .map(|rect| {
+                let src = src.size.to_f64();
                 let rect = rect.to_f64();
 
                 let rect_constrained_loc = rect
                     .loc
-                    .constrain(Rectangle::from_extemities((0f64, 0f64), dest.size.to_point()));
+                    .constrain(Rectangle::from_extemities((0f64, 0f64), src.to_point()));
-                let rect_clamped_size = rect.size.clamp((0f64, 0f64), dest.size);
+                let rect_clamped_size = rect
+                    .size
+                    .clamp((0f64, 0f64), (src.to_point() - rect_constrained_loc).to_size());
 
+                let rect = Rectangle::from_loc_and_size(rect_constrained_loc, rect_clamped_size);
                 [
-                    (rect_constrained_loc.x / dest.size.w) as f32,
+                    (rect.loc.x / src.w) as f32,
-                    (rect_constrained_loc.y / dest.size.h) as f32,
+                    (rect.loc.y / src.h) as f32,
-                    (rect_clamped_size.w / dest.size.w) as f32,
+                    (rect.size.w / src.w) as f32,
-                    (rect_clamped_size.h / dest.size.h) as f32,
+                    (rect.size.h / src.h) as f32,
                 ]
             })
             .flatten()

src/backend/renderer/mod.rs

@@ -10,7 +10,7 @@
 use std::collections::HashSet;
 use std::error::Error;
 
-use crate::utils::{Buffer, Coordinate, Physical, Point, Rectangle, Size};
+use crate::utils::{Buffer, Physical, Point, Rectangle, Size, Transform};
 
 #[cfg(feature = "wayland_frontend")]
 use crate::wayland::compositor::SurfaceData;
@@ -35,27 +35,6 @@ use crate::backend::egl::{
 #[cfg(feature = "wayland_frontend")]
 pub mod utils;
 
-#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
-/// Possible transformations to two-dimensional planes
-pub enum Transform {
-    /// Identity transformation (plane is unaltered when applied)
-    Normal,
-    /// Plane is rotated by 90 degrees
-    _90,
-    /// Plane is rotated by 180 degrees
-    _180,
-    /// Plane is rotated by 270 degrees
-    _270,
-    /// Plane is flipped vertically
-    Flipped,
-    /// Plane is flipped vertically and rotated by 90 degrees
-    Flipped90,
-    /// Plane is flipped vertically and rotated by 180 degrees
-    Flipped180,
-    /// Plane is flipped vertically and rotated by 270 degrees
-    Flipped270,
-}
-
 #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
 /// Texture filtering methods
 pub enum TextureFilter {
@@ -74,70 +53,11 @@ impl Transform {
             Transform::_180 => Matrix3::new(-1.0, 0.0, 0.0, 0.0, -1.0, 0.0, 0.0, 0.0, 1.0),
             Transform::_270 => Matrix3::new(0.0, 1.0, 0.0, -1.0, 0.0, 0.0, 0.0, 0.0, 1.0),
             Transform::Flipped => Matrix3::new(-1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0),
-            Transform::Flipped90 => Matrix3::new(0.0, 1.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0),
+            Transform::Flipped90 => Matrix3::new(0.0, -1.0, 0.0, -1.0, 0.0, 0.0, 0.0, 0.0, 1.0),
             Transform::Flipped180 => Matrix3::new(1.0, 0.0, 0.0, 0.0, -1.0, 0.0, 0.0, 0.0, 1.0),
-            Transform::Flipped270 => Matrix3::new(0.0, -1.0, 0.0, -1.0, 0.0, 0.0, 0.0, 0.0, 1.0),
+            Transform::Flipped270 => Matrix3::new(0.0, 1.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0),
         }
     }
-
-    /// Inverts any 90-degree transformation into 270-degree transformations and vise versa.
-    ///
-    /// Flipping is preserved and 180/Normal transformation are uneffected.
-    pub fn invert(&self) -> Transform {
-        match self {
-            Transform::Normal => Transform::Normal,
-            Transform::Flipped => Transform::Flipped,
-            Transform::_90 => Transform::_270,
-            Transform::_180 => Transform::_180,
-            Transform::_270 => Transform::_90,
-            Transform::Flipped90 => Transform::Flipped270,
-            Transform::Flipped180 => Transform::Flipped180,
-            Transform::Flipped270 => Transform::Flipped90,
-        }
-    }
-
-    /// Transformed size after applying this transformation.
-    pub fn transform_size<N: Coordinate, Kind>(&self, size: Size<N, Kind>) -> Size<N, Kind> {
-        if *self == Transform::_90
-            || *self == Transform::_270
-            || *self == Transform::Flipped90
-            || *self == Transform::Flipped270
-        {
-            (size.h, size.w).into()
-        } else {
-            size
-        }
-    }
-
-    /// Transforms a rectangle inside an area of a given size by applying this transformation
-    pub fn transform_rect_in<N: Coordinate, Kind>(
-        &self,
-        rect: Rectangle<N, Kind>,
-        area: &Size<N, Kind>,
-    ) -> Rectangle<N, Kind> {
-        let size = self.transform_size(rect.size);
-
-        let loc = match *self {
-            Transform::Normal => rect.loc,
-            Transform::_90 => (area.h - rect.loc.y - rect.size.h, rect.loc.x).into(),
-            Transform::_180 => (
-                area.w - rect.loc.x - rect.size.w,
-                area.h - rect.loc.y - rect.size.h,
-            )
-                .into(),
-            Transform::_270 => (rect.loc.y, area.w - rect.loc.x - rect.size.w).into(),
-            Transform::Flipped => (area.w - rect.loc.x - rect.size.w, rect.loc.y).into(),
-            Transform::Flipped90 => (rect.loc.y, rect.loc.x).into(),
-            Transform::Flipped180 => (rect.loc.x, area.h - rect.loc.y - rect.size.h).into(),
-            Transform::Flipped270 => (
-                area.h - rect.loc.y - rect.size.h,
-                area.w - rect.loc.x - rect.size.w,
-            )
-                .into(),
-        };
-
-        Rectangle::from_loc_and_size(loc, size)
-    }
 }
 
 #[cfg(feature = "wayland_frontend")]
@@ -202,6 +122,9 @@ pub trait Frame {
 
     /// Clear the complete current target with a single given color.
     ///
+    /// The `at` parameter specifies a set of rectangles to clear in the current target. This allows partially
+    /// clearing the target which may be useful for damaged rendering.
+    ///
     /// This operation is only valid in between a `begin` and `finish`-call.
     /// If called outside this operation may error-out, do nothing or modify future rendering results in any way.
     fn clear(&mut self, color: [f32; 4], at: &[Rectangle<i32, Physical>]) -> Result<(), Self::Error>;
@@ -217,7 +140,7 @@ pub trait Frame {
         texture_scale: i32,
         output_scale: f64,
         src_transform: Transform,
-        damage: &[Rectangle<i32, Physical>],
+        damage: &[Rectangle<i32, Buffer>],
         alpha: f32,
     ) -> Result<(), Self::Error> {
         self.render_texture_from_to(
@@ -227,7 +150,7 @@ pub trait Frame {
                 pos,
                 texture
                     .size()
-                    .to_logical(texture_scale)
+                    .to_logical(texture_scale, src_transform)
                     .to_f64()
                     .to_physical(output_scale),
             ),
@@ -245,7 +168,7 @@ pub trait Frame {
         texture: &Self::TextureId,
         src: Rectangle<i32, Buffer>,
         dst: Rectangle<f64, Physical>,
-        damage: &[Rectangle<i32, Physical>],
+        damage: &[Rectangle<i32, Buffer>],
         src_transform: Transform,
         alpha: f32,
     ) -> Result<(), Self::Error>;
@@ -540,7 +463,7 @@ pub fn buffer_type(buffer: &wl_buffer::WlBuffer) -> Option<BufferType> {
 ///
 /// *Note*: This will only return dimensions for buffer types known to smithay (see [`buffer_type`])
 #[cfg(feature = "wayland_frontend")]
-pub fn buffer_dimensions(buffer: &wl_buffer::WlBuffer) -> Option<Size<i32, Physical>> {
+pub fn buffer_dimensions(buffer: &wl_buffer::WlBuffer) -> Option<Size<i32, Buffer>> {
     use crate::backend::allocator::Buffer;
 
     if let Some(buf) = buffer.as_ref().user_data().get::<Dmabuf>() {
@@ -560,102 +483,3 @@ pub fn buffer_dimensions(buffer: &wl_buffer::WlBuffer) -> Option<Size<i32, Physi
 
     crate::wayland::shm::with_buffer_contents(buffer, |_, data| (data.width, data.height).into()).ok()
 }
-
-#[cfg(test)]
-mod tests {
-    use super::Transform;
-    use crate::utils::{Logical, Rectangle, Size};
-
-    #[test]
-    fn transform_rect_ident() {
-        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
-        let size = Size::from((70, 90));
-        let transform = Transform::Normal;
-
-        assert_eq!(rect, transform.transform_rect_in(rect, &size))
-    }
-
-    #[test]
-    fn transform_rect_90() {
-        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
-        let size = Size::from((70, 90));
-        let transform = Transform::_90;
-
-        assert_eq!(
-            Rectangle::from_loc_and_size((30, 10), (40, 30)),
-            transform.transform_rect_in(rect, &size)
-        )
-    }
-
-    #[test]
-    fn transform_rect_180() {
-        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
-        let size = Size::from((70, 90));
-        let transform = Transform::_180;
-
-        assert_eq!(
-            Rectangle::from_loc_and_size((30, 30), (30, 40)),
-            transform.transform_rect_in(rect, &size)
-        )
-    }
-
-    #[test]
-    fn transform_rect_270() {
-        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
-        let size = Size::from((70, 90));
-        let transform = Transform::_270;
-
-        assert_eq!(
-            Rectangle::from_loc_and_size((20, 30), (40, 30)),
-            transform.transform_rect_in(rect, &size)
-        )
-    }
-
-    #[test]
-    fn transform_rect_f() {
-        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
-        let size = Size::from((70, 90));
-        let transform = Transform::Flipped;
-
-        assert_eq!(
-            Rectangle::from_loc_and_size((30, 20), (30, 40)),
-            transform.transform_rect_in(rect, &size)
-        )
-    }
-
-    #[test]
-    fn transform_rect_f90() {
-        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
-        let size = Size::from((70, 80));
-        let transform = Transform::Flipped90;
-
-        assert_eq!(
-            Rectangle::from_loc_and_size((20, 10), (40, 30)),
-            transform.transform_rect_in(rect, &size)
-        )
-    }
-
-    #[test]
-    fn transform_rect_f180() {
-        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
-        let size = Size::from((70, 90));
-        let transform = Transform::Flipped180;
-
-        assert_eq!(
-            Rectangle::from_loc_and_size((10, 30), (30, 40)),
-            transform.transform_rect_in(rect, &size)
-        )
-    }
-
-    #[test]
-    fn transform_rect_f270() {
-        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
-        let size = Size::from((70, 90));
-        let transform = Transform::Flipped270;
-
-        assert_eq!(
-            Rectangle::from_loc_and_size((30, 30), (40, 30)),
-            transform.transform_rect_in(rect, &size)
-        )
-    }
-}

src/backend/renderer/utils.rs

@@ -2,7 +2,7 @@
 
 use crate::{
     backend::renderer::{buffer_dimensions, Frame, ImportAll, Renderer, Texture},
-    utils::{Logical, Physical, Point, Rectangle, Size},
+    utils::{Buffer, Logical, Point, Rectangle, Size, Transform},
     wayland::compositor::{
         is_sync_subsurface, with_surface_tree_upward, BufferAssignment, Damage, SubsurfaceCachedState,
         SurfaceAttributes, TraversalAction,
@@ -15,8 +15,9 @@ 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_dimensions: Option<Size<i32, Buffer>>,
     pub(crate) buffer_scale: i32,
+    pub(crate) buffer_transform: Transform,
     pub(crate) buffer: Option<WlBuffer>,
     pub(crate) texture: Option<Box<dyn std::any::Any + 'static>>,
     #[cfg(feature = "desktop")]
@@ -30,6 +31,7 @@ impl SurfaceState {
                 // new contents
                 self.buffer_dimensions = buffer_dimensions(&buffer);
                 self.buffer_scale = attrs.buffer_scale;
+                self.buffer_transform = attrs.buffer_transform.into();
                 if let Some(old_buffer) = std::mem::replace(&mut self.buffer, Some(buffer)) {
                     if &old_buffer != self.buffer.as_ref().unwrap() {
                         old_buffer.release();
@@ -52,6 +54,13 @@ impl SurfaceState {
             None => {}
         }
     }
+
+    /// Returns the size of the surface.
+    pub fn surface_size(&self) -> Option<Size<i32, Logical>> {
+        self.buffer_dimensions
+            .as_ref()
+            .map(|dim| dim.to_logical(self.buffer_scale, self.buffer_transform))
+    }
 }
 
 /// Handler to let smithay take over buffer management.
@@ -110,10 +119,6 @@ where
     T: Texture + 'static,
 {
     let mut result = Ok(());
-    let damage = damage
-        .iter()
-        .map(|geo| geo.to_f64().to_physical(scale).to_i32_up())
-        .collect::<Vec<_>>();
     with_surface_tree_upward(
         surface,
         location,
@@ -125,17 +130,20 @@ where
                 // Import a new buffer if necessary
                 if data.texture.is_none() {
                     if let Some(buffer) = data.buffer.as_ref() {
-                        let damage = attributes
+                        let buffer_damage = attributes
                             .damage
                             .iter()
                             .map(|dmg| match dmg {
                                 Damage::Buffer(rect) => *rect,
-                                // TODO also apply transformations
+                                Damage::Surface(rect) => rect.to_buffer(
-                                Damage::Surface(rect) => rect.to_buffer(attributes.buffer_scale),
+                                    attributes.buffer_scale,
+                                    attributes.buffer_transform.into(),
+                                    &data.surface_size().unwrap(),
+                                ),
                             })
                             .collect::<Vec<_>>();
 
-                        match renderer.import_buffer(buffer, Some(states), &damage) {
+                        match renderer.import_buffer(buffer, Some(states), &buffer_damage) {
                             Some(Ok(m)) => {
                                 data.texture = Some(Box::new(m));
                             }
@@ -169,10 +177,12 @@ where
             let mut location = *location;
             if let Some(data) = states.data_map.get::<RefCell<SurfaceState>>() {
                 let mut data = data.borrow_mut();
+                let dimensions = data.surface_size();
                 let buffer_scale = data.buffer_scale;
-                let buffer_dimensions = data.buffer_dimensions;
+                let buffer_transform = data.buffer_transform;
                 let attributes = states.cached_state.current::<SurfaceAttributes>();
                 if let Some(texture) = data.texture.as_mut().and_then(|x| x.downcast_mut::<T>()) {
+                    let dimensions = dimensions.unwrap();
                     // we need to re-extract the subsurface offset, as the previous closure
                     // only passes it to our children
                     let mut surface_offset = (0, 0).into();
@@ -182,23 +192,19 @@ where
                         location += current.location;
                     }
 
-                    let rect = Rectangle::<i32, Physical>::from_loc_and_size(
+                    let damage = damage
-                        surface_offset.to_f64().to_physical(scale).to_i32_round(),
-                        buffer_dimensions
-                            .unwrap_or_default()
-                            .to_logical(buffer_scale)
-                            .to_f64()
-                            .to_physical(scale)
-                            .to_i32_round(),
-                    );
-                    let new_damage = damage
                         .iter()
                         .cloned()
-                        .flat_map(|geo| geo.intersection(rect))
+                        // first move the damage by the surface offset in logical space
                         .map(|mut geo| {
-                            geo.loc -= rect.loc;
+                            // make the damage relative to the surfaec
+                            geo.loc -= surface_offset;
                             geo
                         })
+                        // then clamp to surface size again in logical space
+                        .flat_map(|geo| geo.intersection(Rectangle::from_loc_and_size((0, 0), dimensions)))
+                        // lastly transform it into buffer space
+                        .map(|geo| geo.to_buffer(buffer_scale, buffer_transform, &dimensions))
                         .collect::<Vec<_>>();
 
                     // TODO: Take wp_viewporter into account
@@ -208,7 +214,7 @@ where
                         buffer_scale,
                         scale,
                         attributes.buffer_transform.into(),
-                        &new_damage,
+                        &damage,
                         1.0,
                     ) {
                         result = Err(err);

src/backend/winit/mod.rs

@@ -302,12 +302,19 @@ impl WinitGraphicsBackend {
         Ok(())
     }
 
-    /// Retrieve the buffer age of the current backbuffer of the window
+    /// Retrieve the buffer age of the current backbuffer of the window.
-    pub fn buffer_age(&self) -> usize {
+    ///
+    /// This will only return a meaningful value, if this `WinitGraphicsBackend`
+    /// is currently bound (by previously calling [`WinitGraphicsBackend::bind`]).
+    ///
+    /// Otherwise and on error this function returns `None`.
+    /// If you are using this value actively e.g. for damage-tracking you should
+    /// likely interpret an error just as if "0" was returned.
+    pub fn buffer_age(&self) -> Option<usize> {
         if self.damage_tracking {
-            self.egl.buffer_age() as usize
+            self.egl.buffer_age().map(|x| x as usize)
         } else {
-            0
+            Some(0)
         }
     }
 

src/backend/x11/surface.rs

@@ -123,7 +123,7 @@ impl X11Surface {
 
             // Now present the current buffer
             let _ = pixmap.present(&*connection, window.as_ref())?;
-            self.swapchain.submitted(next);
+            self.swapchain.submitted(&next);
 
             // Flush the connection after presenting to the window to ensure we don't run out of buffer space in the X11 connection.
             let _ = connection.flush();

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,
@@ -21,6 +21,8 @@ use std::{
     sync::{Arc, Mutex, Weak},
 };
 
+use super::WindowSurfaceType;
+
 crate::utils::ids::id_gen!(next_layer_id, LAYER_ID, LAYER_IDS);
 
 /// Map of [`LayerSurface`]s on an [`Output`]
@@ -29,6 +31,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 +58,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 +99,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 +197,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 +206,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 +295,8 @@ impl LayerMap {
                     }
                 }
 
-                slog::debug!(
+                slog::trace!(
-                    crate::slog_or_fallback(None),
+                    self.logger,
                     "Setting layer to pos {:?} and size {:?}",
                     location,
                     size
@@ -253,7 +315,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 +329,14 @@ 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());
+    }
+
+    /// Returns layers count
+    #[allow(clippy::len_without_is_empty)] //we don't need is_empty on that struct for now, mark as allow
+    pub fn len(&self) -> usize {
+        self.layers.len()
     }
 }
 
@@ -428,6 +497,7 @@ impl LayerSurface {
     pub fn surface_under<P: Into<Point<f64, Logical>>>(
         &self,
         point: P,
+        surface_type: WindowSurfaceType,
     ) -> Option<(WlSurface, Point<i32, Logical>)> {
         let point = point.into();
         if let Some(surface) = self.get_surface() {
@@ -438,13 +508,13 @@ impl LayerSurface {
             {
                 if let Some(result) = popup
                     .get_surface()
-                    .and_then(|surface| under_from_surface_tree(surface, point, location))
+                    .and_then(|surface| under_from_surface_tree(surface, point, location, surface_type))
                 {
                     return Some(result);
                 }
             }
 
-            under_from_surface_tree(surface, point, (0, 0))
+            under_from_surface_tree(surface, point, (0, 0), surface_type)
         } else {
             None
         }
@@ -485,7 +555,16 @@ impl LayerSurface {
     /// window that requested it
     pub fn send_frame(&self, time: u32) {
         if let Some(wl_surface) = self.0.surface.get_surface() {
-            send_frames_surface_tree(wl_surface, time)
+            send_frames_surface_tree(wl_surface, time);
+            for (popup, _) in PopupManager::popups_for_surface(wl_surface)
+                .ok()
+                .into_iter()
+                .flatten()
+            {
+                if let Some(surface) = popup.get_surface() {
+                    send_frames_surface_tree(surface, time);
+                }
+            }
         }
     }
 

src/desktop/popup/grab.rs

@@ -0,0 +1,501 @@
+use std::sync::{Arc, Mutex};
+
+use wayland_server::protocol::{wl_keyboard::KeyState, wl_pointer::ButtonState, wl_surface::WlSurface};
+
+use crate::{
+    utils::{DeadResource, Logical, Point},
+    wayland::{
+        compositor::get_role,
+        seat::{
+            AxisFrame, KeyboardGrab, KeyboardGrabStartData, KeyboardHandle, KeyboardInnerHandle, PointerGrab,
+            PointerGrabStartData, PointerInnerHandle,
+        },
+        shell::xdg::XDG_POPUP_ROLE,
+        Serial,
+    },
+};
+
+use thiserror::Error;
+
+use super::{PopupKind, PopupManager};
+
+/// Defines the possible errors that
+/// can be returned from [`PopupManager::grab_popup`]
+#[derive(Debug, Error)]
+pub enum PopupGrabError {
+    /// This resource has been destroyed and can no longer be used.
+    #[error(transparent)]
+    DeadResource(#[from] DeadResource),
+    /// The client tried to grab a popup after it's parent has been dismissed
+    #[error("the parent of the popup has been already dismissed")]
+    ParentDismissed,
+    /// The client tried to grab a popup after it has been mapped
+    #[error("tried to grab after being mapped")]
+    InvalidGrab,
+    /// The client tried to grab a popup which is not the topmost
+    #[error("popup was not created on the topmost popup")]
+    NotTheTopmostPopup,
+}
+
+/// Defines the possibly strategies
+/// for the [`PopupGrab::ungrab`] operation
+#[derive(Debug, Copy, Clone)]
+pub enum PopupUngrabStrategy {
+    /// Only ungrab the topmost popup
+    Topmost,
+    /// Ungrab all active popups
+    All,
+}
+
+#[derive(Debug, Default)]
+struct PopupGrabInternal {
+    serial: Option<Serial>,
+    active_grabs: Vec<(WlSurface, PopupKind)>,
+    dismissed_grabs: Vec<(WlSurface, PopupKind)>,
+}
+
+impl PopupGrabInternal {
+    fn alive(&self) -> bool {
+        !self.active_grabs.is_empty() || !self.dismissed_grabs.is_empty()
+    }
+
+    fn current_grab(&self) -> Option<&WlSurface> {
+        self.active_grabs
+            .iter()
+            .rev()
+            .find(|(_, p)| p.alive())
+            .map(|(s, _)| s)
+    }
+
+    fn is_dismissed(&self, surface: &WlSurface) -> bool {
+        self.dismissed_grabs.iter().any(|(s, _)| s == surface)
+    }
+
+    fn append_grab(&mut self, popup: &PopupKind) -> Result<(), DeadResource> {
+        let surface = popup.get_surface().ok_or(DeadResource)?;
+        self.active_grabs.push((surface.clone(), popup.clone()));
+        Ok(())
+    }
+
+    fn cleanup(&mut self) {
+        let mut i = 0;
+        while i < self.active_grabs.len() {
+            if !self.active_grabs[i].1.alive() {
+                let grab = self.active_grabs.remove(i);
+                self.dismissed_grabs.push(grab);
+            } else {
+                i += 1;
+            }
+        }
+
+        self.dismissed_grabs.retain(|(s, _)| s.as_ref().is_alive());
+    }
+}
+
+#[derive(Debug, Clone, Default)]
+pub(super) struct PopupGrabInner {
+    internal: Arc<Mutex<PopupGrabInternal>>,
+}
+
+impl PopupGrabInner {
+    pub(super) fn alive(&self) -> bool {
+        let guard = self.internal.lock().unwrap();
+        guard.alive()
+    }
+
+    fn current_grab(&self) -> Option<WlSurface> {
+        let guard = self.internal.lock().unwrap();
+        guard
+            .active_grabs
+            .iter()
+            .rev()
+            .find(|(_, p)| p.alive())
+            .map(|(s, _)| s)
+            .cloned()
+    }
+
+    pub(super) fn cleanup(&self) {
+        let mut guard = self.internal.lock().unwrap();
+        guard.cleanup();
+    }
+
+    pub(super) fn grab(&self, popup: &PopupKind, serial: Serial) -> Result<Option<Serial>, PopupGrabError> {
+        let parent = popup.parent().ok_or(DeadResource)?;
+        let parent_role = get_role(&parent);
+
+        self.cleanup();
+
+        let mut guard = self.internal.lock().unwrap();
+
+        match guard.current_grab() {
+            Some(grab) => {
+                if grab != &parent {
+                    // If the parent is a grabbing popup which has already been dismissed, this popup will be immediately dismissed.
+                    if guard.is_dismissed(&parent) {
+                        return Err(PopupGrabError::ParentDismissed);
+                    }
+
+                    // If the parent is a popup that did not take an explicit grab, an error will be raised.
+                    return Err(PopupGrabError::NotTheTopmostPopup);
+                }
+            }
+            None => {
+                if parent_role == Some(XDG_POPUP_ROLE) {
+                    return Err(PopupGrabError::NotTheTopmostPopup);
+                }
+            }
+        }
+
+        guard.append_grab(popup)?;
+
+        Ok(guard.serial.replace(serial))
+    }
+
+    fn ungrab(&self, root: &WlSurface, strategy: PopupUngrabStrategy) -> Option<WlSurface> {
+        let mut guard = self.internal.lock().unwrap();
+        let dismissed = match strategy {
+            PopupUngrabStrategy::Topmost => {
+                if let Some(grab) = guard.active_grabs.pop() {
+                    let dismissed = PopupManager::dismiss_popup(root, &grab.1);
+
+                    if dismissed.is_ok() {
+                        guard.dismissed_grabs.push(grab);
+                    }
+
+                    dismissed
+                } else {
+                    Ok(())
+                }
+            }
+            PopupUngrabStrategy::All => {
+                let grabs = guard.active_grabs.drain(..).collect::<Vec<_>>();
+
+                if let Some(grab) = grabs.first() {
+                    let dismissed = PopupManager::dismiss_popup(root, &grab.1);
+
+                    if dismissed.is_ok() {
+                        guard.dismissed_grabs.push(grab.clone());
+                        guard.dismissed_grabs.extend(grabs);
+                    }
+
+                    dismissed
+                } else {
+                    Ok(())
+                }
+            }
+        };
+
+        if dismissed.is_err() {
+            // If dismiss_popup returns Err(DeadResource) there is not much what
+            // can do about it here, we just remove all our grabs as they are dead now
+            // anyway. The pointer/keyboard grab will be unset automatically so we
+            // should be fine.
+            guard.active_grabs.drain(..);
+        }
+
+        guard.current_grab().cloned()
+    }
+}
+
+/// Represents the explicit grab a client requested for a popup
+///
+/// An explicit grab can be used by a client to redirect all keyboard
+/// input to a single popup. The focus of the keyboard will stay on
+/// the popup for as long as the grab is valid, that is as long as the
+/// compositor did not call [`ungrab`](PopupGrab::ungrab) or the client
+/// did not destroy the popup. A grab can be nested by requesting a grab
+/// on a popup who's parent is the currently grabbed popup. The grab will
+/// be returned to the parent after the popup has been dismissed.
+///
+/// This module also provides default implementations for [`KeyboardGrab`] and
+/// [`PointerGrab`] that implement the behavior described in the [`xdg-shell`](https://wayland.app/protocols/xdg-shell#xdg_popup:request:grab)
+/// specification. See [`PopupKeyboardGrab`] and [`PopupPointerGrab`] for more
+/// information on the default implementations.
+///
+/// In case the implemented behavior is not suited for your use-case the grab can be
+/// either decorated or a custom [`KeyboardGrab`]/[`PointerGrab`] can use the methods
+/// on the [`PopupGrab`] to implement a custom behavior.
+///
+/// One example would be to use a timer to automatically dismiss the popup after some
+/// timeout.
+///
+/// The grab is obtained by calling [`PopupManager::grap_popup`](super::PopupManager::grab_popup).
+#[derive(Debug, Clone)]
+pub struct PopupGrab {
+    root: WlSurface,
+    serial: Serial,
+    previous_serial: Option<Serial>,
+    toplevel_grab: PopupGrabInner,
+    keyboard_handle: Option<KeyboardHandle>,
+    keyboard_grab_start_data: KeyboardGrabStartData,
+    pointer_grab_start_data: PointerGrabStartData,
+}
+
+impl PopupGrab {
+    pub(super) fn new(
+        toplevel_popups: PopupGrabInner,
+        root: WlSurface,
+        serial: Serial,
+        previous_serial: Option<Serial>,
+        keyboard_handle: Option<KeyboardHandle>,
+    ) -> Self {
+        PopupGrab {
+            root: root.clone(),
+            serial,
+            previous_serial,
+            toplevel_grab: toplevel_popups,
+            keyboard_handle,
+            keyboard_grab_start_data: KeyboardGrabStartData {
+                // We set the focus to root as this will make
+                // sure the grab will stay alive until the
+                // toplevel is destroyed or the grab is unset
+                focus: Some(root.clone()),
+            },
+            pointer_grab_start_data: PointerGrabStartData {
+                button: 0,
+                // We set the focus to root as this will make
+                // sure the grab will stay alive until the
+                // toplevel is destroyed or the grab is unset
+                focus: Some((root, (0, 0).into())),
+                location: (0f64, 0f64).into(),
+            },
+        }
+    }
+
+    /// Returns the serial that was used to grab the popup
+    pub fn serial(&self) -> Serial {
+        self.serial
+    }
+
+    /// Returns the previous serial that was used to grab
+    /// the parent popup in case of nested grabs
+    pub fn previous_serial(&self) -> Option<Serial> {
+        self.previous_serial
+    }
+
+    /// Check if this grab has ended
+    ///
+    /// A grab has ended if either all popups
+    /// associated with the grab have been dismissed
+    /// by the server with [`PopupGrab::ungrab`] or by the client
+    /// by destroying the popup.
+    ///
+    /// This will also return [`false`] if the root
+    /// of the grab has been destroyed.
+    pub fn has_ended(&self) -> bool {
+        !self.root.as_ref().is_alive() || !self.toplevel_grab.alive()
+    }
+
+    /// Returns the current grabbed [`WlSurface`].
+    ///
+    /// If the grab has ended this will return the root surface
+    /// so that the client expected focus can be restored
+    pub fn current_grab(&self) -> Option<WlSurface> {
+        self.toplevel_grab
+            .current_grab()
+            .or_else(|| Some(self.root.clone()))
+    }
+
+    /// Ungrab and dismiss a popup
+    ///
+    /// This will dismiss either the topmost or all popups
+    /// according to the specified [`PopupUngrabStrategy`]
+    ///
+    /// Returns the new topmost popup in case of nested popups
+    /// or if the grab has ended the root surface
+    pub fn ungrab(&mut self, strategy: PopupUngrabStrategy) -> Option<WlSurface> {
+        self.toplevel_grab
+            .ungrab(&self.root, strategy)
+            .or_else(|| Some(self.root.clone()))
+    }
+
+    /// Convenience method for getting a [`KeyboardGrabStartData`] for this grab.
+    ///
+    /// The focus of the [`KeyboardGrabStartData`] will always be the root
+    /// of the popup grab, e.g. the surface of the toplevel, to make sure
+    /// the grab is not automatically unset.
+    pub fn keyboard_grab_start_data(&self) -> &KeyboardGrabStartData {
+        &self.keyboard_grab_start_data
+    }
+
+    /// Convenience method for getting a [`PointerGrabStartData`] for this grab.
+    ///
+    /// The focus of the [`PointerGrabStartData`] will always be the root
+    /// of the popup grab, e.g. the surface of the toplevel, to make sure
+    /// the grab is not automatically unset.
+    pub fn pointer_grab_start_data(&self) -> &PointerGrabStartData {
+        &self.pointer_grab_start_data
+    }
+
+    fn unset_keyboard_grab(&self, serial: Serial) {
+        if let Some(keyboard) = self.keyboard_handle.as_ref() {
+            if keyboard.is_grabbed()
+                && (keyboard.has_grab(self.serial)
+                    || keyboard.has_grab(self.previous_serial.unwrap_or(self.serial)))
+            {
+                keyboard.unset_grab();
+                keyboard.set_focus(Some(&self.root), serial);
+            }
+        }
+    }
+}
+
+/// Default implementation of a [`KeyboardGrab`] for [`PopupGrab`]
+///
+/// The [`PopupKeyboardGrab`] will keep the focus of the keyboard
+/// on the topmost popup until the grab has ended. If the
+/// grab has ended it will restore the focus on the root of the grab
+/// and unset the [`KeyboardGrab`]
+#[derive(Debug)]
+pub struct PopupKeyboardGrab {
+    popup_grab: PopupGrab,
+}
+
+impl PopupKeyboardGrab {
+    /// Create a [`PopupKeyboardGrab`] for the provided [`PopupGrab`]
+    pub fn new(popup_grab: &PopupGrab) -> Self {
+        PopupKeyboardGrab {
+            popup_grab: popup_grab.clone(),
+        }
+    }
+}
+
+impl KeyboardGrab for PopupKeyboardGrab {
+    fn input(
+        &mut self,
+        handle: &mut KeyboardInnerHandle<'_>,
+        keycode: u32,
+        key_state: KeyState,
+        modifiers: Option<(u32, u32, u32, u32)>,
+        serial: Serial,
+        time: u32,
+    ) {
+        // Check if the grab changed and update the focus
+        // If the grab has ended this will return the root
+        // surface to restore the client expected focus.
+        if let Some(surface) = self.popup_grab.current_grab() {
+            handle.set_focus(Some(&surface), serial);
+        }
+
+        if self.popup_grab.has_ended() {
+            handle.unset_grab(serial, false);
+        }
+
+        handle.input(keycode, key_state, modifiers, serial, time)
+    }
+
+    fn set_focus(&mut self, handle: &mut KeyboardInnerHandle<'_>, focus: Option<&WlSurface>, serial: Serial) {
+        // Ignore focus changes unless the grab has ended
+        if self.popup_grab.has_ended() {
+            handle.set_focus(focus, serial);
+            handle.unset_grab(serial, false);
+            return;
+        }
+
+        // Allow to set the focus to the current grab, this can
+        // happen if the user initially sets the focus to
+        // popup instead of relying on the grab behavior
+        if self.popup_grab.current_grab().as_ref() == focus {
+            handle.set_focus(focus, serial);
+        }
+    }
+
+    fn start_data(&self) -> &KeyboardGrabStartData {
+        self.popup_grab.keyboard_grab_start_data()
+    }
+}
+
+/// Default implementation of a [`PointerGrab`] for [`PopupGrab`]
+///
+/// The [`PopupPointerGrab`] will make sure that the pointer focus
+/// stays on the same client as the grabbed popup (similar to an
+/// "owner-events" grab in X11 parlance). If an input event happens
+/// outside of the grabbed [`WlSurface`] the popup will be dismissed
+/// and the grab ends. In case of a nested grab all parent grabs will
+/// also be dismissed.
+///
+/// If the grab has ended the pointer focus is restored and the
+/// [`PointerGrab`] is unset. Additional it will unset an active
+/// [`KeyboardGrab`] that matches the [`Serial`] of this grab and
+/// restore the keyboard focus like described in [`PopupKeyboardGrab`]
+#[derive(Debug)]
+pub struct PopupPointerGrab {
+    popup_grab: PopupGrab,
+}
+
+impl PopupPointerGrab {
+    /// Create a [`PopupPointerGrab`] for the provided [`PopupGrab`]
+    pub fn new(popup_grab: &PopupGrab) -> Self {
+        PopupPointerGrab {
+            popup_grab: popup_grab.clone(),
+        }
+    }
+}
+
+impl PointerGrab for PopupPointerGrab {
+    fn motion(
+        &mut self,
+        handle: &mut PointerInnerHandle<'_>,
+        location: Point<f64, Logical>,
+        focus: Option<(WlSurface, Point<i32, Logical>)>,
+        serial: Serial,
+        time: u32,
+    ) {
+        if self.popup_grab.has_ended() {
+            handle.unset_grab(serial, time);
+            self.popup_grab.unset_keyboard_grab(serial);
+            return;
+        }
+
+        // Check that the focus is of the same client as the grab
+        // If yes allow it, if not unset the focus.
+        let same_client = focus.as_ref().map(|(surface, _)| surface.as_ref().client())
+            == self
+                .popup_grab
+                .current_grab()
+                .map(|surface| surface.as_ref().client());
+
+        if same_client {
+            handle.motion(location, focus, serial, time);
+        } else {
+            handle.motion(location, None, serial, time);
+        }
+    }
+
+    fn button(
+        &mut self,
+        handle: &mut PointerInnerHandle<'_>,
+        button: u32,
+        state: ButtonState,
+        serial: Serial,
+        time: u32,
+    ) {
+        if self.popup_grab.has_ended() {
+            handle.unset_grab(serial, time);
+            handle.button(button, state, serial, time);
+            self.popup_grab.unset_keyboard_grab(serial);
+            return;
+        }
+
+        // Check if the the focused surface is still on the current grabbed surface,
+        // if not the popup will be dismissed
+        if state == ButtonState::Pressed
+            && handle.current_focus().map(|(surface, _)| surface) != self.popup_grab.current_grab().as_ref()
+        {
+            let _ = self.popup_grab.ungrab(PopupUngrabStrategy::All);
+            handle.unset_grab(serial, time);
+            handle.button(button, state, serial, time);
+            self.popup_grab.unset_keyboard_grab(serial);
+        }
+
+        handle.button(button, state, serial, time);
+    }
+
+    fn axis(&mut self, handle: &mut PointerInnerHandle<'_>, details: AxisFrame) {
+        handle.axis(details);
+    }
+
+    fn start_data(&self) -> &PointerGrabStartData {
+        self.popup_grab.pointer_grab_start_data()
+    }
+}

src/desktop/popup/manager.rs

@@ -1,18 +1,23 @@
 use crate::{
-    utils::{DeadResource, Logical, Point, Rectangle},
+    utils::{DeadResource, Logical, Point},
     wayland::{
         compositor::{get_role, with_states},
-        shell::xdg::{PopupSurface, SurfaceCachedState, XdgPopupSurfaceRoleAttributes, XDG_POPUP_ROLE},
+        seat::Seat,
+        shell::xdg::{XdgPopupSurfaceRoleAttributes, XDG_POPUP_ROLE},
+        Serial,
     },
 };
 use std::sync::{Arc, Mutex};
 use wayland_server::protocol::wl_surface::WlSurface;
 
+use super::{PopupGrab, PopupGrabError, PopupGrabInner, PopupKind};
+
 /// Helper to track popups.
 #[derive(Debug)]
 pub struct PopupManager {
     unmapped_popups: Vec<PopupKind>,
     popup_trees: Vec<PopupTree>,
+    popup_grabs: Vec<PopupGrabInner>,
     logger: ::slog::Logger,
 }
 
@@ -22,6 +27,7 @@ impl PopupManager {
         PopupManager {
             unmapped_popups: Vec::new(),
             popup_trees: Vec::new(),
+            popup_grabs: Vec::new(),
             logger: crate::slog_or_fallback(logger),
         }
     }
@@ -54,24 +60,89 @@ impl PopupManager {
         }
     }
 
-    fn add_popup(&mut self, popup: PopupKind) -> Result<(), DeadResource> {
+    /// Take an explicit grab for the provided [`PopupKind`]
-        let mut parent = popup.parent().unwrap();
+    ///
-        while get_role(&parent) == Some(XDG_POPUP_ROLE) {
+    /// Returns a [`PopupGrab`] on success or an [`PopupGrabError`]
-            parent = with_states(&parent, |states| {
+    /// if the grab has been denied.
-                states
+    pub fn grab_popup(
-                    .data_map
+        &mut self,
-                    .get::<Mutex<XdgPopupSurfaceRoleAttributes>>()
+        popup: PopupKind,
-                    .unwrap()
+        seat: &Seat,
-                    .lock()
+        serial: Serial,
-                    .unwrap()
+    ) -> Result<PopupGrab, PopupGrabError> {
-                    .parent
+        let surface = popup.get_surface().ok_or(DeadResource)?;
-                    .as_ref()
+        let root = find_popup_root_surface(&popup)?;
-                    .cloned()
+
-                    .unwrap()
+        match popup {
-            })?;
+            PopupKind::Xdg(ref xdg) => {
+                let surface = xdg.get_surface().ok_or(DeadResource)?;
+                let committed = with_states(surface, |states| {
+                    states
+                        .data_map
+                        .get::<Mutex<XdgPopupSurfaceRoleAttributes>>()
+                        .unwrap()
+                        .lock()
+                        .unwrap()
+                        .committed
+                })?;
+
+                if committed {
+                    surface.as_ref().post_error(
+                        wayland_protocols::xdg_shell::server::xdg_popup::Error::InvalidGrab as u32,
+                        "xdg_popup already is mapped".to_string(),
+                    );
+                    return Err(PopupGrabError::InvalidGrab);
+                }
+            }
         }
 
-        with_states(&parent, |states| {
+        // The primary store for the grab is the seat, additional we store it
+        // in the popupmanager for active cleanup
+        seat.user_data().insert_if_missing(PopupGrabInner::default);
+        let toplevel_popups = seat.user_data().get::<PopupGrabInner>().unwrap().clone();
+
+        // It the popup grab is not alive it is likely
+        // that it either is new and have never been
+        // added to the popupmanager or that it has been
+        // cleaned up.
+        if !toplevel_popups.alive() {
+            self.popup_grabs.push(toplevel_popups.clone());
+        }
+
+        let previous_serial = match toplevel_popups.grab(&popup, serial) {
+            Ok(serial) => serial,
+            Err(err) => {
+                match err {
+                    PopupGrabError::ParentDismissed => {
+                        let _ = PopupManager::dismiss_popup(&root, &popup);
+                    }
+                    PopupGrabError::NotTheTopmostPopup => {
+                        surface.as_ref().post_error(
+                            wayland_protocols::xdg_shell::server::xdg_wm_base::Error::NotTheTopmostPopup
+                                as u32,
+                            "xdg_popup was not created on the topmost popup".to_string(),
+                        );
+                    }
+                    _ => {}
+                }
+
+                return Err(err);
+            }
+        };
+
+        Ok(PopupGrab::new(
+            toplevel_popups,
+            root,
+            serial,
+            previous_serial,
+            seat.get_keyboard(),
+        ))
+    }
+
+    fn add_popup(&mut self, popup: PopupKind) -> Result<(), DeadResource> {
+        let root = find_popup_root_surface(&popup)?;
+
+        with_states(&root, |states| {
             let tree = PopupTree::default();
             if states.data_map.insert_if_missing(|| tree.clone()) {
                 self.popup_trees.push(tree);
@@ -81,7 +152,7 @@ impl PopupManager {
                 // 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);
+            slog::trace!(self.logger, "Adding popup {:?} to root {:?}", popup, root);
             tree.insert(popup);
         })
     }
@@ -116,16 +187,47 @@ impl PopupManager {
         })
     }
 
+    pub(crate) fn dismiss_popup(surface: &WlSurface, popup: &PopupKind) -> Result<(), DeadResource> {
+        with_states(surface, |states| {
+            let tree = states.data_map.get::<PopupTree>();
+
+            if let Some(tree) = tree {
+                tree.dismiss_popup(popup);
+            }
+        })
+    }
+
     /// Needs to be called periodically (but not necessarily frequently)
     /// to cleanup internal resources.
     pub fn cleanup(&mut self) {
         // retain_mut is sadly still unstable
+        self.popup_grabs.iter_mut().for_each(|grabs| grabs.cleanup());
+        self.popup_grabs.retain(|grabs| grabs.alive());
         self.popup_trees.iter_mut().for_each(|tree| tree.cleanup());
         self.popup_trees.retain(|tree| tree.alive());
         self.unmapped_popups.retain(|surf| surf.alive());
     }
 }
 
+fn find_popup_root_surface(popup: &PopupKind) -> Result<WlSurface, DeadResource> {
+    let mut parent = popup.parent().ok_or(DeadResource)?;
+    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()
+        })?;
+    }
+    Ok(parent)
+}
+
 #[derive(Debug, Default, Clone)]
 struct PopupTree(Arc<Mutex<Vec<PopupNode>>>);
 
@@ -157,6 +259,22 @@ impl PopupTree {
         children.push(PopupNode::new(popup));
     }
 
+    fn dismiss_popup(&self, popup: &PopupKind) {
+        let mut children = self.0.lock().unwrap();
+
+        let mut i = 0;
+        while i < children.len() {
+            let child = &mut children[i];
+
+            if child.dismiss_popup(popup) {
+                let _ = children.remove(i);
+                break;
+            } else {
+                i += 1;
+            }
+        }
+    }
+
     fn cleanup(&mut self) {
         let mut children = self.0.lock().unwrap();
         for child in children.iter_mut() {
@@ -209,80 +327,48 @@ impl PopupNode {
         }
     }
 
+    fn send_done(&self) {
+        for child in self.children.iter().rev() {
+            child.send_done();
+        }
+
+        self.surface.send_done();
+    }
+
+    fn dismiss_popup(&mut self, popup: &PopupKind) -> bool {
+        if self.surface.get_surface() == popup.get_surface() {
+            self.send_done();
+            return true;
+        }
+
+        let mut i = 0;
+        while i < self.children.len() {
+            let child = &mut self.children[i];
+
+            if child.dismiss_popup(popup) {
+                let _ = self.children.remove(i);
+                return false;
+            } else {
+                i += 1;
+            }
+        }
+
+        false
+    }
+
     fn cleanup(&mut self) {
         for child in &mut self.children {
             child.cleanup();
         }
+
+        if !self.surface.alive() && !self.children.is_empty() {
+            // TODO: The client destroyed a popup before
+            // destroying all children, this is a protocol
+            // error. As the surface is no longer alive we
+            // can not retrieve the client here to send
+            // the error.
+        }
+
         self.children.retain(|n| n.surface.alive());
     }
 }
-
-/// Represents a popup surface
-#[derive(Debug, Clone)]
-pub enum PopupKind {
-    /// xdg-shell [`PopupSurface`]
-    Xdg(PopupSurface),
-}
-
-impl PopupKind {
-    fn alive(&self) -> bool {
-        match *self {
-            PopupKind::Xdg(ref t) => t.alive(),
-        }
-    }
-
-    /// Retrieves the underlying [`WlSurface`]
-    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(),
-        }
-    }
-
-    /// Returns the surface geometry as set by the client using `xdg_surface::set_window_geometry`
-    pub fn geometry(&self) -> Rectangle<i32, Logical> {
-        let wl_surface = match self.get_surface() {
-            Some(s) => s,
-            None => return Rectangle::from_loc_and_size((0, 0), (0, 0)),
-        };
-
-        with_states(wl_surface, |states| {
-            states
-                .cached_state
-                .current::<SurfaceCachedState>()
-                .geometry
-                .unwrap_or_default()
-        })
-        .unwrap()
-    }
-
-    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/popup/mod.rs

@@ -0,0 +1,96 @@
+mod grab;
+mod manager;
+
+use std::sync::Mutex;
+
+pub use grab::*;
+pub use manager::*;
+use wayland_server::protocol::wl_surface::WlSurface;
+
+use crate::{
+    utils::{Logical, Point, Rectangle},
+    wayland::{
+        compositor::with_states,
+        shell::xdg::{PopupSurface, SurfaceCachedState, XdgPopupSurfaceRoleAttributes},
+    },
+};
+
+/// Represents a popup surface
+#[derive(Debug, Clone)]
+pub enum PopupKind {
+    /// xdg-shell [`PopupSurface`]
+    Xdg(PopupSurface),
+}
+
+impl PopupKind {
+    fn alive(&self) -> bool {
+        match *self {
+            PopupKind::Xdg(ref t) => t.alive(),
+        }
+    }
+
+    /// Retrieves the underlying [`WlSurface`]
+    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(),
+        }
+    }
+
+    /// Returns the surface geometry as set by the client using `xdg_surface::set_window_geometry`
+    pub fn geometry(&self) -> Rectangle<i32, Logical> {
+        let wl_surface = match self.get_surface() {
+            Some(s) => s,
+            None => return Rectangle::from_loc_and_size((0, 0), (0, 0)),
+        };
+
+        with_states(wl_surface, |states| {
+            states
+                .cached_state
+                .current::<SurfaceCachedState>()
+                .geometry
+                .unwrap_or_default()
+        })
+        .unwrap()
+    }
+
+    fn send_done(&self) {
+        if !self.alive() {
+            return;
+        }
+
+        match *self {
+            PopupKind::Xdg(ref t) => t.send_popup_done(),
+        }
+    }
+
+    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/element.rs

@@ -10,6 +10,33 @@ use std::{
 };
 use wayland_server::protocol::wl_surface::WlSurface;
 
+/// Indicates default values for some zindexs inside smithay
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[repr(u8)]
+pub enum RenderZindex {
+    /// WlrLayer::Background default zindex
+    Background = 10,
+    /// WlrLayer::Bottom default zindex
+    Bottom = 20,
+    /// Default zindex for Windows
+    Shell = 30,
+    /// WlrLayer::Top default zindex
+    Top = 40,
+    /// Default zindex for Windows PopUps
+    Popups = 50,
+    /// Default Layer for RenderElements
+    Overlay = 60,
+    /// Default Layer for Overlay PopUp
+    PopupsOverlay = 70,
+}
+
+/// Elements rendered by [`Space::render_output`] in addition to windows, layers and popups.
+pub type DynamicRenderElements<R> =
+    Box<dyn RenderElement<R, <R as Renderer>::Frame, <R as Renderer>::Error, <R as Renderer>::TextureId>>;
+
+pub(super) type SpaceElem<R> =
+    dyn SpaceElement<R, <R as Renderer>::Frame, <R as Renderer>::Error, <R as Renderer>::TextureId>;
+
 /// Trait for custom elements to be rendered during [`Space::render_output`].
 pub trait RenderElement<R, F, E, T>
 where
@@ -28,6 +55,7 @@ where
     /// Returns the bounding box of this element including its position in the space.
     fn geometry(&self) -> Rectangle<i32, Logical>;
     /// Returns the damage of the element since it's last update.
+    /// It should be relative to the elements coordinates.
     ///
     /// If you receive `Some(_)` for `for_values` you may cache that you
     /// send the damage for this `Space` and `Output` combination once
@@ -43,6 +71,8 @@ where
     /// Draws the element using the provided `Frame` and `Renderer`.
     ///
     /// - `scale` provides the current fractional scale value to render as
+    /// - `location` refers to the relative position in the bound buffer the element should be drawn at,
+    ///    so that it matches with the space-relative coordinates returned by [`RenderElement::geometry`].
     /// - `damage` provides the regions you need to re-draw and *may* not
     ///   be equivalent to the damage returned by `accumulated_damage`.
     ///   Redrawing other parts of the element is not valid and may cause rendering artifacts.
@@ -51,9 +81,15 @@ where
         renderer: &mut R,
         frame: &mut F,
         scale: f64,
+        location: Point<i32, Logical>,
         damage: &[Rectangle<i32, Logical>],
         log: &slog::Logger,
     ) -> Result<(), R::Error>;
+
+    /// Returns z_index of RenderElement, reverf too [`RenderZindex`] for default values
+    fn z_index(&self) -> u8 {
+        RenderZindex::Overlay as u8
+    }
 }
 
 pub(crate) trait SpaceElement<R, F, E, T>
@@ -81,6 +117,7 @@ where
         damage: &[Rectangle<i32, Logical>],
         log: &slog::Logger,
     ) -> Result<(), R::Error>;
+    fn z_index(&self) -> u8;
 }
 
 impl<R, F, E, T> SpaceElement<R, F, E, T> for Box<dyn RenderElement<R, F, E, T>>
@@ -108,11 +145,15 @@ 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, damage, log)
+        (&**self as &dyn RenderElement<R, F, E, T>).draw(renderer, frame, scale, location, damage, log)
+    }
+
+    fn z_index(&self) -> u8 {
+        RenderElement::z_index(self.as_ref())
     }
 }
 
@@ -161,6 +202,7 @@ where
         renderer: &mut R,
         frame: &mut F,
         scale: f64,
+        location: Point<i32, Logical>,
         damage: &[Rectangle<i32, Logical>],
         log: &slog::Logger,
     ) -> Result<(), R::Error> {
@@ -169,7 +211,7 @@ where
             frame,
             &self.surface,
             scale,
-            self.position,
+            location,
             damage,
             log,
         )

src/desktop/space/layer.rs

@@ -5,7 +5,7 @@ use crate::{
         space::{Space, SpaceElement},
     },
     utils::{Logical, Point, Rectangle},
-    wayland::output::Output,
+    wayland::{output::Output, shell::wlr_layer::Layer},
 };
 use std::{
     any::TypeId,
@@ -13,6 +13,8 @@ use std::{
     collections::HashMap,
 };
 
+use super::RenderZindex;
+
 #[derive(Default)]
 pub struct LayerState {
     pub drawn: bool,
@@ -69,4 +71,18 @@ where
         }
         res
     }
+
+    fn z_index(&self) -> u8 {
+        if let Some(layer) = self.layer() {
+            let z_index = match layer {
+                Layer::Background => RenderZindex::Background,
+                Layer::Bottom => RenderZindex::Bottom,
+                Layer::Top => RenderZindex::Top,
+                Layer::Overlay => RenderZindex::Overlay,
+            };
+            z_index as u8
+        } else {
+            0
+        }
+    }
 }

src/desktop/space/mod.rs

@@ -2,23 +2,21 @@
 //! rendering helpers to add custom elements or different clients to a space.
 
 use crate::{
-    backend::renderer::{utils::SurfaceState, Frame, ImportAll, Renderer, Transform},
+    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},
+    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;
@@ -44,10 +42,6 @@ pub struct Space {
     logger: ::slog::Logger,
 }
 
-/// Elements rendered by [`Space::render_output`] in addition to windows, layers and popups.
-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
@@ -326,97 +320,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() {
+                    for (popup, location) in PopupManager::popups_for_surface(surface)
-                                if states.role == Some("subsurface") {
+                        .ok()
-                                    let current = states.cached_state.current::<SubsurfaceCachedState>();
+                        .into_iter()
-                                    location += current.location;
+                        .flatten()
-                                }
+                    {
-
+                        if let Some(surface) = popup.get_surface() {
-                                TraversalAction::DoChildren(location)
+                            let location = window_loc(window, &self.id) + window.geometry().loc + location
-                            } else {
+                                - popup.geometry().loc;
-                                // If the parent surface is unmapped, then the child surfaces are hidden as
+                            output_update(
-                                // well, no need to consider them here.
+                                output,
-                                TraversalAction::SkipChildren
+                                output_geometry,
-                            }
+                                &mut output_state.surfaces,
-                        },
+                                surface,
-                        |wl_surface, states, &loc| {
+                                location,
-                            let data = states.data_map.get::<RefCell<SurfaceState>>();
+                                &self.logger,
-
+                            );
-                            if let Some(size) = data.and_then(|d| d.borrow().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());
-                                    }
-                                } else {
-                                    // Surface does not match output, if we sent enter earlier
-                                    // we should now send leave
-                                    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);
-                                    }
-                                }
-                            } 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,
-                    )
                 }
             }
         }
@@ -454,7 +390,8 @@ impl Space {
     /// trait and use `custom_elements` to provide them to this function. `custom_elements are rendered
     /// after every other element.
     ///
-    /// Returns a list of updated regions (or `None` if that list would be empty) in case of success.
+    /// Returns a list of updated regions relative to the rendered output
+    /// (or `None` if that list would be empty) in case of success.
     pub fn render_output<R>(
         &mut self,
         renderer: &mut R,
@@ -473,9 +410,6 @@ impl Space {
             return Err(RenderError::UnmappedOutput);
         }
 
-        type SpaceElem<R> =
-            dyn SpaceElement<R, <R as Renderer>::Frame, <R as Renderer>::Error, <R as Renderer>::TextureId>;
-
         let mut state = output_state(self.id, output);
         let output_size = output
             .current_mode()
@@ -497,23 +431,31 @@ impl Space {
             .flat_map(|l| l.popup_elements::<R>(self.id))
             .collect::<Vec<_>>();
 
+        let mut render_elements: Vec<&SpaceElem<R>> = Vec::with_capacity(
+            custom_elements.len()
+                + layer_map.len()
+                + self.windows.len()
+                + window_popups.len()
+                + layer_popups.len(),
+        );
+
+        render_elements.extend(custom_elements.iter().map(|l| l as &SpaceElem<R>));
+        render_elements.extend(self.windows.iter().map(|l| l as &SpaceElem<R>));
+        render_elements.extend(window_popups.iter().map(|l| l as &SpaceElem<R>));
+        render_elements.extend(layer_map.layers().map(|l| l as &SpaceElem<R>));
+        render_elements.extend(layer_popups.iter().map(|l| l as &SpaceElem<R>));
+
+        render_elements.sort_by_key(|e| e.z_index());
+
         // This will hold all the damage we need for this rendering step
         let mut damage = Vec::<Rectangle<i32, Logical>>::new();
         // First add damage for windows gone
+
         for old_toplevel in state
             .last_state
             .iter()
             .filter_map(|(id, geo)| {
-                if !self
+                if !render_elements.iter().any(|e| ToplevelId::from(*e) == *id) {
-                    .windows
-                    .iter()
-                    .map(|w| w as &SpaceElem<R>)
-                    .chain(window_popups.iter().map(|p| p as &SpaceElem<R>))
-                    .chain(layer_map.layers().map(|l| l as &SpaceElem<R>))
-                    .chain(layer_popups.iter().map(|p| p as &SpaceElem<R>))
-                    .chain(custom_elements.iter().map(|c| c as &SpaceElem<R>))
-                    .any(|e| ToplevelId::from(e) == *id)
-                {
                     Some(*geo)
                 } else {
                     None
@@ -526,17 +468,9 @@ impl Space {
         }
 
         // lets iterate front to back and figure out, what new windows or unmoved windows we have
-        for element in self
+        for element in &render_elements {
-            .windows
-            .iter()
-            .map(|w| w as &SpaceElem<R>)
-            .chain(window_popups.iter().map(|p| p as &SpaceElem<R>))
-            .chain(layer_map.layers().map(|l| l as &SpaceElem<R>))
-            .chain(layer_popups.iter().map(|p| p as &SpaceElem<R>))
-            .chain(custom_elements.iter().map(|c| c as &SpaceElem<R>))
-        {
             let geo = element.geometry(self.id);
-            let old_geo = state.last_state.get(&ToplevelId::from(element)).cloned();
+            let old_geo = state.last_state.get(&ToplevelId::from(*element)).cloned();
 
             // window was moved or resized
             if old_geo.map(|old_geo| old_geo != geo).unwrap_or(false) {
@@ -572,12 +506,15 @@ impl Space {
         damage.retain(|rect| rect.overlaps(output_geo));
         damage.retain(|rect| rect.size.h > 0 && rect.size.w > 0);
         // merge overlapping rectangles
-        damage = damage.into_iter().fold(Vec::new(), |mut new_damage, rect| {
+        damage = damage.into_iter().fold(Vec::new(), |new_damage, mut rect| {
-            if let Some(existing) = new_damage.iter_mut().find(|other| rect.overlaps(**other)) {
+            // replace with drain_filter, when that becomes stable to reuse the original Vec's memory
-                *existing = existing.merge(rect);
+            let (overlapping, mut new_damage): (Vec<_>, Vec<_>) =
-            } else {
+                new_damage.into_iter().partition(|other| other.overlaps(rect));
-                new_damage.push(rect);
+
+            for overlap in overlapping {
+                rect = rect.merge(overlap);
             }
+            new_damage.push(rect);
             new_damage
         });
 
@@ -600,37 +537,28 @@ impl Space {
                     clear_color,
                     &damage
                         .iter()
+                        // Map from global space to output space
+                        .map(|geo| Rectangle::from_loc_and_size(geo.loc - output_geo.loc, geo.size))
+                        // Map from logical to physical
                         .map(|geo| geo.to_f64().to_physical(state.render_scale).to_i32_round())
                         .collect::<Vec<_>>(),
                 )?;
-
                 // Then re-draw all windows & layers overlapping with a damage rect.
 
-                for element in layer_map
+                for element in &render_elements {
-                    .layers_on(WlrLayer::Background)
-                    .chain(layer_map.layers_on(WlrLayer::Bottom))
-                    .map(|l| l as &SpaceElem<R>)
-                    .chain(self.windows.iter().map(|w| w as &SpaceElem<R>))
-                    .chain(
-                        layer_map
-                            .layers_on(WlrLayer::Top)
-                            .chain(layer_map.layers_on(WlrLayer::Overlay))
-                            .map(|l| l as &SpaceElem<R>),
-                    )
-                    .chain(custom_elements.iter().map(|c| c as &SpaceElem<R>))
-                {
                     let geo = element.geometry(self.id);
                     if damage.iter().any(|d| d.overlaps(geo)) {
-                        let loc = element.location(self.id) - output_geo.loc;
+                        let loc = element.location(self.id);
                         let damage = damage
                             .iter()
                             .flat_map(|d| d.intersection(geo))
+                            // Map from output space to surface-relative coordinates
                             .map(|geo| Rectangle::from_loc_and_size(geo.loc - loc, geo.size))
                             .collect::<Vec<_>>();
                         slog::trace!(
                             self.logger,
                             "Rendering toplevel at {:?} with damage {:#?}",
-                            geo,
+                            Rectangle::from_loc_and_size(geo.loc - output_geo.loc, geo.size),
                             damage
                         );
                         element.draw(
@@ -638,7 +566,7 @@ impl Space {
                             renderer,
                             frame,
                             state.render_scale,
-                            loc,
+                            loc - output_geo.loc,
                             &damage,
                             &self.logger,
                         )?;
@@ -658,22 +586,24 @@ impl Space {
         }
 
         // If rendering was successful capture the state and add the damage
-        state.last_state = self
+        state.last_state = render_elements
-            .windows
             .iter()
-            .map(|w| w as &SpaceElem<R>)
-            .chain(window_popups.iter().map(|p| p as &SpaceElem<R>))
-            .chain(layer_map.layers().map(|l| l as &SpaceElem<R>))
-            .chain(layer_popups.iter().map(|p| p as &SpaceElem<R>))
-            .chain(custom_elements.iter().map(|c| c as &SpaceElem<R>))
             .map(|elem| {
                 let geo = elem.geometry(self.id);
-                (ToplevelId::from(elem), geo)
+                (ToplevelId::from(*elem), geo)
             })
             .collect();
         state.old_damage.push_front(new_damage.clone());
 
-        Ok(Some(new_damage))
+        Ok(Some(
+            new_damage
+                .into_iter()
+                .map(|mut geo| {
+                    geo.loc -= output_geo.loc;
+                    geo
+                })
+                .collect(),
+        ))
     }
 
     /// Sends the frame callback to mapped [`Window`]s and [`LayerSurface`]s.

src/desktop/space/popup.rs

@@ -8,14 +8,17 @@ use crate::{
         window::Window,
     },
     utils::{Logical, Point, Rectangle},
-    wayland::output::Output,
+    wayland::{output::Output, shell::wlr_layer::Layer},
 };
 use std::any::TypeId;
 
+use super::RenderZindex;
+
 #[derive(Debug)]
 pub struct RenderPopup {
     location: Point<i32, Logical>,
     popup: PopupKind,
+    z_index: u8,
 }
 
 impl Window {
@@ -39,6 +42,7 @@ impl Window {
                         RenderPopup {
                             location: offset,
                             popup,
+                            z_index: RenderZindex::Popups as u8,
                         }
                     })
             })
@@ -48,7 +52,7 @@ impl Window {
 }
 
 impl LayerSurface {
-    pub(super) fn popup_elements<R>(&self, space_id: usize) -> impl Iterator<Item = RenderPopup>
+    pub(super) fn popup_elements<R>(&self, space_id: usize) -> impl Iterator<Item = RenderPopup> + '_
     where
         R: Renderer + ImportAll + 'static,
         R::TextureId: 'static,
@@ -67,9 +71,20 @@ impl LayerSurface {
                     .flatten()
                     .map(move |(popup, location)| {
                         let offset = loc + location - popup.geometry().loc;
+                        let z_index = if let Some(layer) = self.layer() {
+                            if layer == Layer::Overlay {
+                                RenderZindex::PopupsOverlay as u8
+                            } else {
+                                RenderZindex::Popups as u8
+                            }
+                        } else {
+                            0
+                        };
+
                         RenderPopup {
                             location: offset,
                             popup,
+                            z_index,
                         }
                     })
             })
@@ -126,4 +141,8 @@ where
         // popups are special, we track them, but they render with their parents
         Ok(())
     }
+
+    fn z_index(&self) -> u8 {
+        self.z_index
+    }
 }

src/desktop/space/window.rs

@@ -13,6 +13,8 @@ use std::{
     collections::HashMap,
 };
 
+use super::RenderZindex;
+
 #[derive(Default)]
 pub struct WindowState {
     pub location: Point<i32, Logical>,
@@ -103,4 +105,8 @@ where
         }
         res
     }
+
+    fn z_index(&self) -> u8 {
+        RenderZindex::Shell as u8
+    }
 }

src/desktop/utils.rs

@@ -3,7 +3,7 @@
 use crate::{
     backend::renderer::utils::SurfaceState,
     desktop::Space,
-    utils::{Logical, Point, Rectangle, Size},
+    utils::{Logical, Point, Rectangle},
     wayland::{
         compositor::{
             with_surface_tree_downward, with_surface_tree_upward, Damage, SubsurfaceCachedState,
@@ -16,16 +16,12 @@ use wayland_server::protocol::wl_surface;
 
 use std::cell::RefCell;
 
-impl SurfaceState {
+use super::WindowSurfaceType;
-    /// Returns the size of the surface.
-    pub fn size(&self) -> Option<Size<i32, Logical>> {
-        self.buffer_dimensions
-            .map(|dims| dims.to_logical(self.buffer_scale))
-    }
 
+impl SurfaceState {
     fn contains_point<P: Into<Point<f64, Logical>>>(&self, attrs: &SurfaceAttributes, point: P) -> bool {
         let point = point.into();
-        let size = match self.size() {
+        let size = match self.surface_size() {
             None => return false, // If the surface has no size, it can't have an input region.
             Some(size) => size,
         };
@@ -71,7 +67,7 @@ where
             let mut loc = *loc;
             let data = states.data_map.get::<RefCell<SurfaceState>>();
 
-            if let Some(size) = data.and_then(|d| d.borrow().size()) {
+            if let Some(size) = data.and_then(|d| d.borrow().surface_size()) {
                 if states.role == Some("subsurface") {
                     let current = states.cached_state.current::<SubsurfaceCachedState>();
                     loc += current.location;
@@ -148,7 +144,11 @@ where
 
                     damage.extend(attributes.damage.iter().map(|dmg| {
                         let mut rect = match dmg {
-                            Damage::Buffer(rect) => rect.to_logical(attributes.buffer_scale),
+                            Damage::Buffer(rect) => rect.to_logical(
+                                attributes.buffer_scale,
+                                attributes.buffer_transform.into(),
+                                &data.buffer_dimensions.unwrap(),
+                            ),
                             Damage::Surface(rect) => *rect,
                         };
                         rect.loc += location;
@@ -174,6 +174,7 @@ pub fn under_from_surface_tree<P>(
     surface: &wl_surface::WlSurface,
     point: Point<f64, Logical>,
     location: P,
+    surface_type: WindowSurfaceType,
 ) -> Option<(wl_surface::WlSurface, Point<i32, Logical>)>
 where
     P: Into<Point<i32, Logical>>,
@@ -191,17 +192,23 @@ where
                 location += current.location;
             }
 
-            let contains_the_point = data
+            if states.role == Some("subsurface") || surface_type.contains(WindowSurfaceType::TOPLEVEL) {
-                .map(|data| {
+                let contains_the_point = data
-                    data.borrow()
+                    .map(|data| {
-                        .contains_point(&*states.cached_state.current(), point - location.to_f64())
+                        data.borrow()
-                })
+                            .contains_point(&*states.cached_state.current(), point - location.to_f64())
-                .unwrap_or(false);
+                    })
-            if contains_the_point {
+                    .unwrap_or(false);
-                *found.borrow_mut() = Some((wl_surface.clone(), location));
+                if contains_the_point {
+                    *found.borrow_mut() = Some((wl_surface.clone(), location));
+                }
             }
 
-            TraversalAction::DoChildren(location)
+            if surface_type.contains(WindowSurfaceType::SUBSURFACE) {
+                TraversalAction::DoChildren(location)
+            } else {
+                TraversalAction::SkipChildren
+            }
         },
         |_, _, _| {},
         |_, _, _| {
@@ -233,3 +240,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/desktop/window.rs

@@ -109,6 +109,21 @@ impl Hash for Window {
     }
 }
 
+bitflags::bitflags! {
+    /// Defines the surface types that can be
+    /// queried with [`Window::surface_under`]
+    pub struct WindowSurfaceType: u32 {
+        /// Include the toplevel surface
+        const TOPLEVEL = 1;
+        /// Include all subsurfaces
+        const SUBSURFACE = 2;
+        /// Include all popup surfaces
+        const POPUP = 4;
+        /// Query all surfaces
+        const ALL = Self::TOPLEVEL.bits | Self::SUBSURFACE.bits | Self::POPUP.bits;
+    }
+}
+
 impl Window {
     /// Construct a new [`Window`] from a given compatible toplevel surface
     pub fn new(toplevel: Kind) -> Window {
@@ -222,24 +237,27 @@ impl Window {
     pub fn surface_under<P: Into<Point<f64, Logical>>>(
         &self,
         point: P,
+        surface_type: WindowSurfaceType,
     ) -> Option<(wl_surface::WlSurface, Point<i32, Logical>)> {
         let point = point.into();
         if let Some(surface) = self.0.toplevel.get_surface() {
-            for (popup, location) in PopupManager::popups_for_surface(surface)
+            if surface_type.contains(WindowSurfaceType::POPUP) {
-                .ok()
+                for (popup, location) in PopupManager::popups_for_surface(surface)
-                .into_iter()
+                    .ok()
-                .flatten()
+                    .into_iter()
-            {
+                    .flatten()
-                let offset = self.geometry().loc + location - popup.geometry().loc;
-                if let Some(result) = popup
-                    .get_surface()
-                    .and_then(|surface| under_from_surface_tree(surface, point, offset))
                 {
-                    return Some(result);
+                    let offset = self.geometry().loc + location - popup.geometry().loc;
+                    if let Some(result) = popup
+                        .get_surface()
+                        .and_then(|surface| under_from_surface_tree(surface, point, offset, surface_type))
+                    {
+                        return Some(result);
+                    }
                 }
             }
 
-            under_from_surface_tree(surface, point, (0, 0))
+            under_from_surface_tree(surface, point, (0, 0), surface_type)
         } else {
             None
         }

src/utils/geometry.rs

@@ -406,10 +406,11 @@ impl<N: Coordinate> Point<N, Logical> {
 
     #[inline]
     /// Convert this logical point to buffer coordinate space according to given scale factor
-    pub fn to_buffer(self, scale: N) -> Point<N, Buffer> {
+    pub fn to_buffer(self, scale: N, transformation: Transform, area: &Size<N, Logical>) -> Point<N, Buffer> {
+        let point = transformation.transform_point_in(self, area);
         Point {
-            x: self.x.upscale(scale),
+            x: point.x.upscale(scale),
-            y: self.y.upscale(scale),
+            y: point.y.upscale(scale),
             _kind: std::marker::PhantomData,
         }
     }
@@ -430,10 +431,11 @@ impl<N: Coordinate> Point<N, Physical> {
 impl<N: Coordinate> Point<N, Buffer> {
     #[inline]
     /// Convert this physical point to logical coordinate space according to given scale factor
-    pub fn to_logical(self, scale: N) -> Point<N, Logical> {
+    pub fn to_logical(self, scale: N, transform: Transform, area: &Size<N, Buffer>) -> Point<N, Logical> {
+        let point = transform.invert().transform_point_in(self, area);
         Point {
-            x: self.x.downscale(scale),
+            x: point.x.downscale(scale),
-            y: self.y.downscale(scale),
+            y: point.y.downscale(scale),
             _kind: std::marker::PhantomData,
         }
     }
@@ -667,12 +669,12 @@ impl<N: Coordinate> Size<N, Logical> {
 
     #[inline]
     /// Convert this logical size to buffer coordinate space according to given scale factor
-    pub fn to_buffer(self, scale: N) -> Size<N, Buffer> {
+    pub fn to_buffer(self, scale: N, transformation: Transform) -> Size<N, Buffer> {
-        Size {
+        transformation.transform_size(Size {
             w: self.w.upscale(scale),
             h: self.h.upscale(scale),
             _kind: std::marker::PhantomData,
-        }
+        })
     }
 }
 
@@ -691,12 +693,12 @@ impl<N: Coordinate> Size<N, Physical> {
 impl<N: Coordinate> Size<N, Buffer> {
     #[inline]
     /// Convert this physical point to logical coordinate space according to given scale factor
-    pub fn to_logical(self, scale: N) -> Size<N, Logical> {
+    pub fn to_logical(self, scale: N, transformation: Transform) -> Size<N, Logical> {
-        Size {
+        transformation.invert().transform_size(Size {
             w: self.w.downscale(scale),
             h: self.h.downscale(scale),
             _kind: std::marker::PhantomData,
-        }
+        })
     }
 }
 
@@ -969,10 +971,24 @@ impl<N: Coordinate> Rectangle<N, Logical> {
 
     /// Convert this logical rectangle to buffer coordinate space according to given scale factor
     #[inline]
-    pub fn to_buffer(self, scale: N) -> Rectangle<N, Buffer> {
+    pub fn to_buffer(
+        self,
+        scale: N,
+        transformation: Transform,
+        area: &Size<N, Logical>,
+    ) -> Rectangle<N, Buffer> {
+        let rect = transformation.transform_rect_in(self, area);
         Rectangle {
-            loc: self.loc.to_buffer(scale),
+            loc: Point {
-            size: self.size.to_buffer(scale),
+                x: rect.loc.x.upscale(scale),
+                y: rect.loc.y.upscale(scale),
+                _kind: std::marker::PhantomData,
+            },
+            size: Size {
+                w: rect.size.w.upscale(scale),
+                h: rect.size.h.upscale(scale),
+                _kind: std::marker::PhantomData,
+            },
         }
     }
 }
@@ -991,10 +1007,24 @@ impl<N: Coordinate> Rectangle<N, Physical> {
 impl<N: Coordinate> Rectangle<N, Buffer> {
     /// Convert this physical rectangle to logical coordinate space according to given scale factor
     #[inline]
-    pub fn to_logical(self, scale: N) -> Rectangle<N, Logical> {
+    pub fn to_logical(
+        self,
+        scale: N,
+        transformation: Transform,
+        area: &Size<N, Buffer>,
+    ) -> Rectangle<N, Logical> {
+        let rect = transformation.invert().transform_rect_in(self, area);
         Rectangle {
-            loc: self.loc.to_logical(scale),
+            loc: Point {
-            size: self.size.to_logical(scale),
+                x: rect.loc.x.downscale(scale),
+                y: rect.loc.y.downscale(scale),
+                _kind: std::marker::PhantomData,
+            },
+            size: Size {
+                w: rect.size.w.downscale(scale),
+                h: rect.size.h.downscale(scale),
+                _kind: std::marker::PhantomData,
+            },
         }
     }
 }
@@ -1071,3 +1101,207 @@ impl<N: Default, Kind> Default for Rectangle<N, Kind> {
         }
     }
 }
+
+#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
+/// Possible transformations to two-dimensional planes
+pub enum Transform {
+    /// Identity transformation (plane is unaltered when applied)
+    Normal,
+    /// Plane is rotated by 90 degrees
+    _90,
+    /// Plane is rotated by 180 degrees
+    _180,
+    /// Plane is rotated by 270 degrees
+    _270,
+    /// Plane is flipped vertically
+    Flipped,
+    /// Plane is flipped vertically and rotated by 90 degrees
+    Flipped90,
+    /// Plane is flipped vertically and rotated by 180 degrees
+    Flipped180,
+    /// Plane is flipped vertically and rotated by 270 degrees
+    Flipped270,
+}
+
+impl Default for Transform {
+    fn default() -> Transform {
+        Transform::Normal
+    }
+}
+
+impl Transform {
+    /// Inverts any 90-degree transformation into 270-degree transformations and vise versa.
+    ///
+    /// Flipping is preserved and 180/Normal transformation are uneffected.
+    pub fn invert(&self) -> Transform {
+        match self {
+            Transform::Normal => Transform::Normal,
+            Transform::Flipped => Transform::Flipped,
+            Transform::_90 => Transform::_270,
+            Transform::_180 => Transform::_180,
+            Transform::_270 => Transform::_90,
+            Transform::Flipped90 => Transform::Flipped270,
+            Transform::Flipped180 => Transform::Flipped180,
+            Transform::Flipped270 => Transform::Flipped90,
+        }
+    }
+
+    /// Transforms a point inside an area of a given size by applying this transformation.
+    pub fn transform_point_in<N: Coordinate, Kind>(
+        &self,
+        point: Point<N, Kind>,
+        area: &Size<N, Kind>,
+    ) -> Point<N, Kind> {
+        match *self {
+            Transform::Normal => point,
+            Transform::_90 => (area.h - point.y, point.x).into(),
+            Transform::_180 => (area.w - point.x, area.h - point.y).into(),
+            Transform::_270 => (point.y, area.w - point.x).into(),
+            Transform::Flipped => (area.w - point.x, point.y).into(),
+            Transform::Flipped90 => (point.y, point.x).into(),
+            Transform::Flipped180 => (point.x, area.h - point.y).into(),
+            Transform::Flipped270 => (area.h - point.y, area.w - point.x).into(),
+        }
+    }
+
+    /// Transformed size after applying this transformation.
+    pub fn transform_size<N: Coordinate, Kind>(&self, size: Size<N, Kind>) -> Size<N, Kind> {
+        if *self == Transform::_90
+            || *self == Transform::_270
+            || *self == Transform::Flipped90
+            || *self == Transform::Flipped270
+        {
+            (size.h, size.w).into()
+        } else {
+            size
+        }
+    }
+
+    /// Transforms a rectangle inside an area of a given size by applying this transformation.
+    pub fn transform_rect_in<N: Coordinate, Kind>(
+        &self,
+        rect: Rectangle<N, Kind>,
+        area: &Size<N, Kind>,
+    ) -> Rectangle<N, Kind> {
+        let size = self.transform_size(rect.size);
+
+        let loc = match *self {
+            Transform::Normal => rect.loc,
+            Transform::_90 => (area.h - rect.loc.y - rect.size.h, rect.loc.x).into(),
+            Transform::_180 => (
+                area.w - rect.loc.x - rect.size.w,
+                area.h - rect.loc.y - rect.size.h,
+            )
+                .into(),
+            Transform::_270 => (rect.loc.y, area.w - rect.loc.x - rect.size.w).into(),
+            Transform::Flipped => (area.w - rect.loc.x - rect.size.w, rect.loc.y).into(),
+            Transform::Flipped90 => (rect.loc.y, rect.loc.x).into(),
+            Transform::Flipped180 => (rect.loc.x, area.h - rect.loc.y - rect.size.h).into(),
+            Transform::Flipped270 => (
+                area.h - rect.loc.y - rect.size.h,
+                area.w - rect.loc.x - rect.size.w,
+            )
+                .into(),
+        };
+
+        Rectangle::from_loc_and_size(loc, size)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{Logical, Rectangle, Size, Transform};
+
+    #[test]
+    fn transform_rect_ident() {
+        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
+        let size = Size::from((70, 90));
+        let transform = Transform::Normal;
+
+        assert_eq!(rect, transform.transform_rect_in(rect, &size))
+    }
+
+    #[test]
+    fn transform_rect_90() {
+        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
+        let size = Size::from((70, 90));
+        let transform = Transform::_90;
+
+        assert_eq!(
+            Rectangle::from_loc_and_size((30, 10), (40, 30)),
+            transform.transform_rect_in(rect, &size)
+        )
+    }
+
+    #[test]
+    fn transform_rect_180() {
+        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
+        let size = Size::from((70, 90));
+        let transform = Transform::_180;
+
+        assert_eq!(
+            Rectangle::from_loc_and_size((30, 30), (30, 40)),
+            transform.transform_rect_in(rect, &size)
+        )
+    }
+
+    #[test]
+    fn transform_rect_270() {
+        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
+        let size = Size::from((70, 90));
+        let transform = Transform::_270;
+
+        assert_eq!(
+            Rectangle::from_loc_and_size((20, 30), (40, 30)),
+            transform.transform_rect_in(rect, &size)
+        )
+    }
+
+    #[test]
+    fn transform_rect_f() {
+        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
+        let size = Size::from((70, 90));
+        let transform = Transform::Flipped;
+
+        assert_eq!(
+            Rectangle::from_loc_and_size((30, 20), (30, 40)),
+            transform.transform_rect_in(rect, &size)
+        )
+    }
+
+    #[test]
+    fn transform_rect_f90() {
+        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
+        let size = Size::from((70, 80));
+        let transform = Transform::Flipped90;
+
+        assert_eq!(
+            Rectangle::from_loc_and_size((20, 10), (40, 30)),
+            transform.transform_rect_in(rect, &size)
+        )
+    }
+
+    #[test]
+    fn transform_rect_f180() {
+        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
+        let size = Size::from((70, 90));
+        let transform = Transform::Flipped180;
+
+        assert_eq!(
+            Rectangle::from_loc_and_size((10, 30), (30, 40)),
+            transform.transform_rect_in(rect, &size)
+        )
+    }
+
+    #[test]
+    fn transform_rect_f270() {
+        let rect = Rectangle::<i32, Logical>::from_loc_and_size((10, 20), (30, 40));
+        let size = Size::from((70, 90));
+        let transform = Transform::Flipped270;
+
+        assert_eq!(
+            Rectangle::from_loc_and_size((30, 30), (40, 30)),
+            transform.transform_rect_in(rect, &size)
+        )
+    }
+}

src/utils/mod.rs

@@ -10,7 +10,7 @@ pub mod x11rb;
 pub(crate) mod ids;
 pub mod user_data;
 
-pub use self::geometry::{Buffer, Coordinate, Logical, Physical, Point, Raw, Rectangle, Size};
+pub use self::geometry::{Buffer, Coordinate, Logical, Physical, Point, Raw, Rectangle, Size, Transform};
 
 /// This resource is not managed by Smithay
 #[derive(Debug)]

src/wayland/data_device/dnd_grab.rs

@@ -8,7 +8,7 @@ use wayland_server::{
 use crate::{
     utils::{Logical, Point},
     wayland::{
-        seat::{AxisFrame, GrabStartData, PointerGrab, PointerInnerHandle, Seat},
+        seat::{AxisFrame, PointerGrab, PointerGrabStartData, PointerInnerHandle, Seat},
         Serial,
     },
 };
@@ -16,7 +16,7 @@ use crate::{
 use super::{with_source_metadata, DataDeviceData, SeatData};
 
 pub(crate) struct DnDGrab {
-    start_data: GrabStartData,
+    start_data: PointerGrabStartData,
     data_source: Option<wl_data_source::WlDataSource>,
     current_focus: Option<wl_surface::WlSurface>,
     pending_offers: Vec<wl_data_offer::WlDataOffer>,
@@ -29,7 +29,7 @@ pub(crate) struct DnDGrab {
 
 impl DnDGrab {
     pub(crate) fn new(
-        start_data: GrabStartData,
+        start_data: PointerGrabStartData,
         source: Option<wl_data_source::WlDataSource>,
         origin: wl_surface::WlSurface,
         seat: Seat,
@@ -222,7 +222,7 @@ impl PointerGrab for DnDGrab {
         handle.axis(details);
     }
 
-    fn start_data(&self) -> &GrabStartData {
+    fn start_data(&self) -> &PointerGrabStartData {
         &self.start_data
     }
 }

src/wayland/data_device/mod.rs

@@ -60,7 +60,7 @@ use slog::{debug, error, o};
 
 use crate::wayland::{
     compositor,
-    seat::{GrabStartData, Seat},
+    seat::{PointerGrabStartData, Seat},
     Serial,
 };
 
@@ -335,7 +335,7 @@ pub fn set_data_device_selection(seat: &Seat, mime_types: Vec<String>) {
 pub fn start_dnd<C>(
     seat: &Seat,
     serial: Serial,
-    start_data: GrabStartData,
+    start_data: PointerGrabStartData,
     metadata: SourceMetadata,
     callback: C,
 ) where

src/wayland/data_device/server_dnd_grab.rs

@@ -8,7 +8,7 @@ use wayland_server::{
 use crate::{
     utils::{Logical, Point},
     wayland::{
-        seat::{AxisFrame, GrabStartData, PointerGrab, PointerInnerHandle, Seat},
+        seat::{AxisFrame, PointerGrab, PointerGrabStartData, PointerInnerHandle, Seat},
         Serial,
     },
 };
@@ -42,7 +42,7 @@ pub enum ServerDndEvent {
 }
 
 pub(crate) struct ServerDnDGrab<C: 'static> {
-    start_data: GrabStartData,
+    start_data: PointerGrabStartData,
     metadata: super::SourceMetadata,
     current_focus: Option<wl_surface::WlSurface>,
     pending_offers: Vec<wl_data_offer::WlDataOffer>,
@@ -53,7 +53,7 @@ pub(crate) struct ServerDnDGrab<C: 'static> {
 
 impl<C: 'static> ServerDnDGrab<C> {
     pub(crate) fn new(
-        start_data: GrabStartData,
+        start_data: PointerGrabStartData,
         metadata: super::SourceMetadata,
         seat: Seat,
         callback: Rc<RefCell<C>>,
@@ -222,7 +222,7 @@ where
         handle.axis(details);
     }
 
-    fn start_data(&self) -> &GrabStartData {
+    fn start_data(&self) -> &PointerGrabStartData {
         &self.start_data
     }
 }

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>,

src/wayland/seat/keyboard.rs

@@ -86,9 +86,16 @@ pub struct XkbConfig<'a> {
     pub options: Option<String>,
 }
 
+enum GrabStatus {
+    None,
+    Active(Serial, Box<dyn KeyboardGrab>),
+    Borrowed,
+}
+
 struct KbdInternal {
     known_kbds: Vec<WlKeyboard>,
     focus: Option<WlSurface>,
+    pending_focus: Option<WlSurface>,
     pressed_keys: Vec<u32>,
     mods_state: ModifiersState,
     keymap: xkb::Keymap,
@@ -96,6 +103,7 @@ struct KbdInternal {
     repeat_rate: i32,
     repeat_delay: i32,
     focus_hook: Box<dyn FnMut(Option<&WlSurface>)>,
+    grab: GrabStatus,
 }
 
 // focus_hook does not implement debug, so we have to impl Debug manually
@@ -147,6 +155,7 @@ impl KbdInternal {
         Ok(KbdInternal {
             known_kbds: Vec::new(),
             focus: None,
+            pending_focus: None,
             pressed_keys: Vec::new(),
             mods_state: ModifiersState::default(),
             keymap,
@@ -154,6 +163,7 @@ impl KbdInternal {
             repeat_rate,
             repeat_delay,
             focus_hook,
+            grab: GrabStatus::None,
         })
     }
 
@@ -215,6 +225,35 @@ impl KbdInternal {
             }
         }
     }
+
+    fn with_grab<F>(&mut self, f: F, logger: ::slog::Logger)
+    where
+        F: FnOnce(KeyboardInnerHandle<'_>, &mut dyn KeyboardGrab),
+    {
+        let mut grab = ::std::mem::replace(&mut self.grab, GrabStatus::Borrowed);
+        match grab {
+            GrabStatus::Borrowed => panic!("Accessed a keyboard grab from within a keyboard grab access."),
+            GrabStatus::Active(_, ref mut handler) => {
+                // If this grab is associated with a surface that is no longer alive, discard it
+                if let Some(ref surface) = handler.start_data().focus {
+                    if !surface.as_ref().is_alive() {
+                        self.grab = GrabStatus::None;
+                        f(KeyboardInnerHandle { inner: self, logger }, &mut DefaultGrab);
+                        return;
+                    }
+                }
+                f(KeyboardInnerHandle { inner: self, logger }, &mut **handler);
+            }
+            GrabStatus::None => {
+                f(KeyboardInnerHandle { inner: self, logger }, &mut DefaultGrab);
+            }
+        }
+
+        if let GrabStatus::Borrowed = self.grab {
+            // the grab has not been ended nor replaced, put it back in place
+            self.grab = grab;
+        }
+    }
 }
 
 /// Errors that can be encountered when creating a keyboard handler
@@ -320,6 +359,47 @@ pub enum FilterResult<T> {
     Intercept(T),
 }
 
+/// Data about the event that started the grab.
+#[derive(Debug, Clone)]
+pub struct GrabStartData {
+    /// The focused surface, if any, at the start of the grab.
+    pub focus: Option<WlSurface>,
+}
+
+/// A trait to implement a keyboard grab
+///
+/// In some context, it is necessary to temporarily change the behavior of the keyboard. This is
+/// typically known as a keyboard grab. A example would be, during a popup grab the keyboard focus
+/// will not be changed and stay on the grabbed popup.
+///
+/// This trait is the interface to intercept regular keyboard events and change them as needed, its
+/// interface mimics the [`KeyboardHandle`] interface.
+///
+/// If your logic decides that the grab should end, both [`KeyboardInnerHandle`] and [`KeyboardHandle`] have
+/// a method to change it.
+///
+/// When your grab ends (either as you requested it or if it was forcefully cancelled by the server),
+/// the struct implementing this trait will be dropped. As such you should put clean-up logic in the destructor,
+/// rather than trying to guess when the grab will end.
+pub trait KeyboardGrab {
+    /// An input was reported
+    fn input(
+        &mut self,
+        handle: &mut KeyboardInnerHandle<'_>,
+        keycode: u32,
+        key_state: WlKeyState,
+        modifiers: Option<(u32, u32, u32, u32)>,
+        serial: Serial,
+        time: u32,
+    );
+
+    /// A focus change was requested
+    fn set_focus(&mut self, handle: &mut KeyboardInnerHandle<'_>, focus: Option<&WlSurface>, serial: Serial);
+
+    /// The data about the event that started the grab.
+    fn start_data(&self) -> &GrabStartData;
+}
+
 /// An handle to a keyboard handler
 ///
 /// It can be cloned and all clones manipulate the same internal state.
@@ -337,6 +417,42 @@ pub struct KeyboardHandle {
 }
 
 impl KeyboardHandle {
+    /// Change the current grab on this keyboard to the provided grab
+    ///
+    /// Overwrites any current grab.
+    pub fn set_grab<G: KeyboardGrab + 'static>(&self, grab: G, serial: Serial) {
+        self.arc.internal.borrow_mut().grab = GrabStatus::Active(serial, Box::new(grab));
+    }
+
+    /// Remove any current grab on this keyboard, resetting it to the default behavior
+    pub fn unset_grab(&self) {
+        self.arc.internal.borrow_mut().grab = GrabStatus::None;
+    }
+
+    /// Check if this keyboard is currently grabbed with this serial
+    pub fn has_grab(&self, serial: Serial) -> bool {
+        let guard = self.arc.internal.borrow_mut();
+        match guard.grab {
+            GrabStatus::Active(s, _) => s == serial,
+            _ => false,
+        }
+    }
+
+    /// Check if this keyboard is currently being grabbed
+    pub fn is_grabbed(&self) -> bool {
+        let guard = self.arc.internal.borrow_mut();
+        !matches!(guard.grab, GrabStatus::None)
+    }
+
+    /// Returns the start data for the grab, if any.
+    pub fn grab_start_data(&self) -> Option<GrabStartData> {
+        let guard = self.arc.internal.borrow();
+        match &guard.grab {
+            GrabStatus::Active(_, g) => Some(g.start_data().clone()),
+            _ => None,
+        }
+    }
+
     /// Handle a keystroke
     ///
     /// All keystrokes from the input backend should be fed _in order_ to this method of the
@@ -392,14 +508,12 @@ impl KeyboardHandle {
             KeyState::Pressed => WlKeyState::Pressed,
             KeyState::Released => WlKeyState::Released,
         };
-        guard.with_focused_kbds(|kbd, _| {
+        guard.with_grab(
-            // key event must be sent before modifers event for libxkbcommon
+            move |mut handle, grab| {
-            // to process them correctly
+                grab.input(&mut handle, keycode, wl_state, modifiers, serial, time);
-            kbd.key(serial.into(), time, keycode, wl_state);
+            },
-            if let Some((dep, la, lo, gr)) = modifiers {
+            self.arc.logger.clone(),
-                kbd.modifiers(serial.into(), dep, la, lo, gr);
+        );
-            }
-        });
         if guard.focus.is_some() {
             trace!(self.arc.logger, "Input forwarded to client");
         } else {
@@ -417,44 +531,13 @@ impl KeyboardHandle {
     /// a [`wl_keyboard::Event::Enter`](wayland_server::protocol::wl_keyboard::Event::Enter) event will be sent.
     pub fn set_focus(&self, focus: Option<&WlSurface>, serial: Serial) {
         let mut guard = self.arc.internal.borrow_mut();
-
+        guard.pending_focus = focus.cloned();
-        let same = guard
+        guard.with_grab(
-            .focus
+            move |mut handle, grab| {
-            .as_ref()
+                grab.set_focus(&mut handle, focus, serial);
-            .and_then(|f| focus.map(|s| s.as_ref().equals(f.as_ref())))
+            },
-            .unwrap_or(false);
+            self.arc.logger.clone(),
-
+        );
-        if !same {
-            // unset old focus
-            guard.with_focused_kbds(|kbd, s| {
-                kbd.leave(serial.into(), s);
-            });
-
-            // set new focus
-            guard.focus = focus.cloned();
-            let (dep, la, lo, gr) = guard.serialize_modifiers();
-            let keys = guard.serialize_pressed_keys();
-            guard.with_focused_kbds(|kbd, surface| {
-                kbd.enter(serial.into(), surface, keys.clone());
-                // Modifiers must be send after enter event.
-                kbd.modifiers(serial.into(), dep, la, lo, gr);
-            });
-            {
-                let KbdInternal {
-                    ref focus,
-                    ref mut focus_hook,
-                    ..
-                } = *guard;
-                focus_hook(focus.as_ref());
-            }
-            if guard.focus.is_some() {
-                trace!(self.arc.logger, "Focus set to new surface");
-            } else {
-                trace!(self.arc.logger, "Focus unset");
-            }
-        } else {
-            trace!(self.arc.logger, "Focus unchanged");
-        }
     }
 
     /// Check if given client currently has keyboard focus
@@ -543,3 +626,129 @@ pub(crate) fn implement_keyboard(keyboard: Main<WlKeyboard>, handle: Option<&Key
 
     keyboard.deref().clone()
 }
+
+/// This inner handle is accessed from inside a keyboard grab logic, and directly
+/// sends event to the client
+#[derive(Debug)]
+pub struct KeyboardInnerHandle<'a> {
+    inner: &'a mut KbdInternal,
+    logger: ::slog::Logger,
+}
+
+impl<'a> KeyboardInnerHandle<'a> {
+    /// Change the current grab on this keyboard to the provided grab
+    ///
+    /// Overwrites any current grab.
+    pub fn set_grab<G: KeyboardGrab + 'static>(&mut self, serial: Serial, grab: G) {
+        self.inner.grab = GrabStatus::Active(serial, Box::new(grab));
+    }
+
+    /// Remove any current grab on this keyboard, resetting it to the default behavior
+    ///
+    /// This will also restore the focus of the underlying keyboard if restore_focus
+    /// is [`true`]
+    pub fn unset_grab(&mut self, serial: Serial, restore_focus: bool) {
+        self.inner.grab = GrabStatus::None;
+        // restore the focus
+        if restore_focus {
+            let focus = self.inner.pending_focus.clone();
+            self.set_focus(focus.as_ref(), serial);
+        }
+    }
+
+    /// Access the current focus of this keyboard
+    pub fn current_focus(&self) -> Option<&WlSurface> {
+        self.inner.focus.as_ref()
+    }
+
+    /// Send the input to the focused keyboards
+    pub fn input(
+        &mut self,
+        keycode: u32,
+        key_state: WlKeyState,
+        modifiers: Option<(u32, u32, u32, u32)>,
+        serial: Serial,
+        time: u32,
+    ) {
+        self.inner.with_focused_kbds(|kbd, _| {
+            // key event must be sent before modifers event for libxkbcommon
+            // to process them correctly
+            kbd.key(serial.into(), time, keycode, key_state);
+            if let Some((dep, la, lo, gr)) = modifiers {
+                kbd.modifiers(serial.into(), dep, la, lo, gr);
+            }
+        });
+    }
+
+    /// Set the current focus of this keyboard
+    ///
+    /// If the new focus is different from the previous one, any previous focus
+    /// will be sent a [`wl_keyboard::Event::Leave`](wayland_server::protocol::wl_keyboard::Event::Leave)
+    /// event, and if the new focus is not `None`,
+    /// a [`wl_keyboard::Event::Enter`](wayland_server::protocol::wl_keyboard::Event::Enter) event will be sent.
+    pub fn set_focus(&mut self, focus: Option<&WlSurface>, serial: Serial) {
+        let same = self
+            .inner
+            .focus
+            .as_ref()
+            .and_then(|f| focus.map(|s| s.as_ref().equals(f.as_ref())))
+            .unwrap_or(false);
+
+        if !same {
+            // unset old focus
+            self.inner.with_focused_kbds(|kbd, s| {
+                kbd.leave(serial.into(), s);
+            });
+
+            // set new focus
+            self.inner.focus = focus.cloned();
+            let (dep, la, lo, gr) = self.inner.serialize_modifiers();
+            let keys = self.inner.serialize_pressed_keys();
+            self.inner.with_focused_kbds(|kbd, surface| {
+                kbd.enter(serial.into(), surface, keys.clone());
+                // Modifiers must be send after enter event.
+                kbd.modifiers(serial.into(), dep, la, lo, gr);
+            });
+            {
+                let KbdInternal {
+                    ref focus,
+                    ref mut focus_hook,
+                    ..
+                } = *self.inner;
+                focus_hook(focus.as_ref());
+            }
+            if self.inner.focus.is_some() {
+                trace!(self.logger, "Focus set to new surface");
+            } else {
+                trace!(self.logger, "Focus unset");
+            }
+        } else {
+            trace!(self.logger, "Focus unchanged");
+        }
+    }
+}
+
+// The default grab, the behavior when no particular grab is in progress
+struct DefaultGrab;
+
+impl KeyboardGrab for DefaultGrab {
+    fn input(
+        &mut self,
+        handle: &mut KeyboardInnerHandle<'_>,
+        keycode: u32,
+        key_state: WlKeyState,
+        modifiers: Option<(u32, u32, u32, u32)>,
+        serial: Serial,
+        time: u32,
+    ) {
+        handle.input(keycode, key_state, modifiers, serial, time)
+    }
+
+    fn set_focus(&mut self, handle: &mut KeyboardInnerHandle<'_>, focus: Option<&WlSurface>, serial: Serial) {
+        handle.set_focus(focus, serial)
+    }
+
+    fn start_data(&self) -> &GrabStartData {
+        unreachable!()
+    }
+}

src/wayland/seat/mod.rs

@@ -41,12 +41,12 @@ mod pointer;
 
 pub use self::{
     keyboard::{
-        keysyms, Error as KeyboardError, FilterResult, KeyboardHandle, Keysym, KeysymHandle, ModifiersState,
+        keysyms, Error as KeyboardError, FilterResult, GrabStartData as KeyboardGrabStartData, KeyboardGrab,
-        XkbConfig,
+        KeyboardHandle, KeyboardInnerHandle, Keysym, KeysymHandle, ModifiersState, XkbConfig,
     },
     pointer::{
-        AxisFrame, CursorImageAttributes, CursorImageStatus, GrabStartData, PointerGrab, PointerHandle,
+        AxisFrame, CursorImageAttributes, CursorImageStatus, GrabStartData as PointerGrabStartData,
-        PointerInnerHandle,
+        PointerGrab, PointerHandle, PointerInnerHandle,
     },
 };
 

src/wayland/seat/pointer.rs

@@ -677,16 +677,18 @@ impl PointerGrab for DefaultGrab {
         time: u32,
     ) {
         handle.button(button, state, serial, time);
-        handle.set_grab(
+        if state == ButtonState::Pressed {
-            serial,
+            handle.set_grab(
-            ClickGrab {
+                serial,
-                start_data: GrabStartData {
+                ClickGrab {
-                    focus: handle.current_focus().cloned(),
+                    start_data: GrabStartData {
-                    button,
+                        focus: handle.current_focus().cloned(),
-                    location: handle.current_location(),
+                        button,
+                        location: handle.current_location(),
+                    },
                 },
-            },
+            );
-        );
+        }
     }
     fn axis(&mut self, handle: &mut PointerInnerHandle<'_>, details: AxisFrame) {
         handle.axis(details);

src/wayland/shell/wlr_layer/types.rs

@@ -162,7 +162,7 @@ impl TryFrom<zwlr_layer_surface_v1::Anchor> for Anchor {
 }
 
 /// Exclusive zone descriptor
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum ExclusiveZone {
     /// Requests that the compositor avoids occluding an area with other surfaces.
     ///

src/wayland/shell/xdg/mod.rs

@@ -383,6 +383,14 @@ xdg_role!(
         /// It is a protocol error to call commit on a wl_surface with
         /// the xdg_popup role when no parent is set.
         pub parent: Option<wl_surface::WlSurface>,
+
+        /// Defines if the surface has received at least one commit
+        ///
+        /// This can be used to check for protocol errors, like
+        /// checking if a popup requested a grab after it has been
+        /// mapped.
+        pub committed: bool,
+
         popup_handle: Option<xdg_popup::XdgPopup>
     }
 );
@@ -1169,15 +1177,14 @@ impl ToplevelSurface {
     /// The parent must be another toplevel equivalent surface.
     ///
     /// If the parent is `None`, the parent-child relationship is removed.
-    pub fn set_parent(&self, parent: Option<wl_surface::WlSurface>) -> bool {
+    pub fn set_parent(&self, parent: Option<&wl_surface::WlSurface>) -> bool {
         if let Some(parent) = parent {
-            if !is_toplevel_equivalent(&parent) {
+            if !is_toplevel_equivalent(parent) {
                 return false;
             }
         }
 
-        // Unset the parent
+        xdg_handlers::set_parent(&self.shell_surface, parent.cloned());
-        xdg_handlers::set_parent(&self.shell_surface, None);
 
         true
     }
@@ -1399,6 +1406,7 @@ impl PopupSurface {
                 .unwrap()
                 .lock()
                 .unwrap();
+            attributes.committed = true;
             if attributes.initial_configure_sent {
                 if let Some(state) = attributes.last_acked {
                     if state != attributes.current {

src/wayland/xdg_foreign/mod.rs

@@ -447,7 +447,7 @@ fn imported_implementation(
                 .map(|export| export.surface.clone())
                 .unwrap();
 
-            toplevel_surface.set_parent(Some(imported_parent));
+            toplevel_surface.set_parent(Some(&imported_parent));
         }
 
         _ => unreachable!(),

wlcs_anvil/src/main_loop.rs

@@ -5,7 +5,7 @@ use std::{
 
 use smithay::{
     backend::{
-        renderer::{Frame, Renderer, Transform},
+        renderer::{Frame, Renderer},
         SwapBuffersError,
     },
     reexports::{
@@ -18,7 +18,7 @@ use smithay::{
             Client, Display,
         },
     },
-    utils::Rectangle,
+    utils::{Rectangle, Transform},
     wayland::{
         output::{Mode, PhysicalProperties},
         seat::CursorImageStatus,

wlcs_anvil/src/renderer.rs

@@ -3,11 +3,11 @@ use std::cell::Cell;
 use smithay::{
     backend::{
         allocator::dmabuf::Dmabuf,
-        renderer::{Frame, ImportDma, ImportShm, Renderer, Texture, TextureFilter, Transform},
+        renderer::{Frame, ImportDma, ImportShm, Renderer, Texture, TextureFilter},
         SwapBuffersError,
     },
     reexports::wayland_server::protocol::wl_buffer,
-    utils::{Buffer, Physical, Rectangle, Size},
+    utils::{Buffer, Physical, Rectangle, Size, Transform},
     wayland::compositor::SurfaceData,
 };
 
@@ -106,7 +106,7 @@ impl Frame for DummyFrame {
         _texture: &Self::TextureId,
         _src: Rectangle<i32, Buffer>,
         _dst: Rectangle<f64, Physical>,
-        _damage: &[Rectangle<i32, Physical>],
+        _damage: &[Rectangle<i32, Buffer>],
         _src_transform: Transform,
         _alpha: f32,
     ) -> Result<(), Self::Error> {