wayland: Explicitly handle destroyed surfaces

src/main.rs

@@ -9,11 +9,7 @@ use smithay::{
 };
 
 use anyhow::{Context, Result};
-use std::{
+use std::{ffi::OsString, os::unix::prelude::AsRawFd, sync::Arc};
-    ffi::OsString,
-    os::unix::prelude::AsRawFd,
-    sync::{atomic::Ordering, Arc},
-};
 
 pub mod backend;
 pub mod config;
@@ -71,18 +67,6 @@ fn main() -> Result<()> {
         data.state.common.shell.refresh();
         state::Common::refresh_focus(&mut data.state);
 
-        // do we need to trigger another render
-        if data.state.common.dirty_flag.swap(false, Ordering::SeqCst) {
-            // TODO: Render workspace sessions
-            for output in data.state.common.shell.outputs() {
-                data.state.backend.schedule_render(
-                    &data.state.common.event_loop_handle,
-                    output,
-                    None,
-                )
-            }
-        }
-
         // send out events
         let _ = data.display.flush_clients();
     })?;

src/state.rs

@@ -39,7 +39,6 @@ use smithay::{
 use std::{
     cell::RefCell,
     ffi::OsString,
-    sync::{atomic::AtomicBool, Arc},
     time::{Duration, Instant},
 };
 #[cfg(feature = "debug")]
@@ -75,7 +74,6 @@ pub struct Common {
 
     //pub output_conf: ConfigurationManager,
     pub shell: Shell,
-    pub dirty_flag: Arc<AtomicBool>,
 
     seats: Vec<Seat<State>>,
     last_active_seat: Option<Seat<State>>,
@@ -242,11 +240,6 @@ impl State {
 
         let shell = Shell::new(&config, dh);
 
-        #[cfg(not(feature = "debug"))]
-        let dirty_flag = Arc::new(AtomicBool::new(false));
-        #[cfg(feature = "debug")]
-        let dirty_flag = log.dirty_flag.clone();
-
         State {
             common: Common {
                 config,
@@ -256,7 +249,6 @@ impl State {
                 event_loop_signal: signal,
 
                 shell,
-                dirty_flag,
 
                 seats: Vec::new(),
                 last_active_seat: None,

src/wayland/handlers/compositor.rs

@@ -1,19 +1,11 @@
 // SPDX-License-Identifier: GPL-3.0-only
 
-use crate::{
+use crate::{state::BackendData, utils::prelude::*, wayland::protocols::screencopy::SessionType};
-    state::BackendData,
-    utils::prelude::*,
-    wayland::{
-        handlers::screencopy::UserdataExt,
-        protocols::screencopy::{BufferParams, Session as ScreencopySession, SessionType},
-    },
-};
 use smithay::{
     backend::renderer::utils::{on_commit_buffer_handler, with_renderer_surface_state},
     delegate_compositor,
     desktop::{layer_map_for_output, Kind, LayerSurface, PopupKind, WindowSurfaceType},
     reexports::wayland_server::protocol::wl_surface::WlSurface,
-    utils::IsAlive,
     wayland::{
         compositor::{with_states, CompositorHandler, CompositorState},
         shell::{
@@ -26,7 +18,7 @@ use smithay::{
 };
 use std::sync::Mutex;
 
-use super::screencopy::{self, PendingScreencopyBuffers};
+use super::screencopy::PendingScreencopyBuffers;
 
 impl State {
     fn early_import_surface(&mut self, surface: &WlSurface) {
@@ -170,36 +162,11 @@ impl CompositorHandler for State {
                 );
                 workspace.commit(surface);
             }
-
-            // handle window screencopy sessions
-            let active = element.active_window();
-            if active.toplevel().wl_surface() == surface {
-                for (session, params) in active.pending_buffers() {
-                    let window = active.clone();
-                    self.common.event_loop_handle.insert_idle(move |data| {
-                        if !session.alive() {
-                            return;
-                        }
-
-                        match screencopy::render_window_to_buffer(
-                            &mut data.state,
-                            &session,
-                            params.clone(),
-                            &window,
-                        ) {
-                            // rendering yielded no damage, buffer is still pending
-                            Ok(false) => data.state.common.still_pending(session, params),
-                            Ok(true) => {} // success
-                            Err((reason, err)) => {
-                                slog_scope::warn!("Screencopy session failed: {}", err);
-                                session.failed(reason);
-                            }
-                        }
-                    });
-                }
-            }
         }
 
+        //handle window screencopy sessions
+        self.schedule_window_session(surface);
+
         // We need to know every potential output for importing to the right gpu and scheduling a render,
         // so call this only after every potential surface map operation has been done.
         self.early_import_surface(surface);
@@ -216,66 +183,7 @@ impl CompositorHandler for State {
             layer_map_for_output(output).arrange();
         }
 
-        // here we store additional workspace_sessions, we should handle, when rendering the corresponding output anyway
+        let mut scheduled_sessions = self.schedule_workspace_sessions(surface);
-        let mut scheduled_sessions: Option<Vec<(ScreencopySession, BufferParams)>> = None;
-
-        // lets check which workspaces this surface belongs to
-        let active_spaces = self
-            .common
-            .shell
-            .outputs()
-            .map(|o| (o.clone(), self.common.shell.active_space(o).handle.clone()))
-            .collect::<Vec<_>>();
-        for (handle, output) in self.common.shell.workspaces_for_surface(surface) {
-            let workspace = self.common.shell.space_for_handle_mut(&handle).unwrap();
-            if !workspace.pending_buffers.is_empty() {
-                // TODO: replace with drain_filter....
-                let mut i = 0;
-                while i < workspace.pending_buffers.len() {
-                    if let SessionType::Workspace(o, w) =
-                        workspace.pending_buffers[i].0.session_type()
-                    {
-                        if active_spaces.contains(&(o.clone(), w)) {
-                            // surface is on an active workspace/output combo, add to workspace_sessions
-                            let (session, params) = workspace.pending_buffers.remove(i);
-                            scheduled_sessions
-                                .get_or_insert_with(Vec::new)
-                                .push((session, params));
-                        } else if handle == w && output == o {
-                            // surface is visible on an offscreen workspace session, schedule a new render
-                            let (session, params) = workspace.pending_buffers.remove(i);
-                            let output = output.clone();
-                            self.common.event_loop_handle.insert_idle(move |data| {
-                                if !session.alive() {
-                                    return;
-                                }
-                                match screencopy::render_workspace_to_buffer(
-                                    &mut data.state,
-                                    &session,
-                                    params.clone(),
-                                    &output,
-                                    &handle,
-                                ) {
-                                    Ok(false) => {
-                                        // rendering yielded no new damage, buffer still pending
-                                        data.state.common.still_pending(session, params);
-                                    }
-                                    Ok(true) => {}
-                                    Err((reason, err)) => {
-                                        slog_scope::warn!("Screencopy session failed: {}", err);
-                                        session.failed(reason);
-                                    }
-                                }
-                            });
-                        } else {
-                            i += 1;
-                        }
-                    } else {
-                        unreachable!();
-                    }
-                }
-            }
-        }
 
         // schedule a new render
         for output in self.common.shell.visible_outputs_for_surface(surface) {

src/wayland/handlers/layer_shell.rs

@@ -3,7 +3,7 @@
 use crate::utils::prelude::*;
 use smithay::{
     delegate_layer_shell,
-    desktop::{LayerSurface, PopupKind},
+    desktop::{layer_map_for_output, LayerSurface, PopupKind, WindowSurfaceType},
     output::Output,
     reexports::wayland_server::protocol::wl_output::WlOutput,
     wayland::shell::{
@@ -14,6 +14,8 @@ use smithay::{
     },
 };
 
+use super::screencopy::PendingScreencopyBuffers;
+
 impl WlrLayerShellHandler for State {
     fn shell_state(&mut self) -> &mut WlrLayerShellState {
         &mut self.common.shell.layer_shell_state
@@ -26,7 +28,6 @@ impl WlrLayerShellHandler for State {
         _layer: Layer,
         namespace: String,
     ) {
-        super::mark_dirty_on_drop(&self.common, surface.wl_surface());
         let seat = self.common.last_active_seat().clone();
         let output = wl_output
             .as_ref()
@@ -51,6 +52,42 @@ impl WlrLayerShellHandler for State {
                 .unwrap();
         }
     }
+
+    fn layer_destroyed(&mut self, surface: WlrLayerSurface) {
+        let maybe_output = self
+            .common
+            .shell
+            .outputs()
+            .find(|o| {
+                let map = layer_map_for_output(o);
+                map.layer_for_surface(surface.wl_surface(), WindowSurfaceType::TOPLEVEL)
+                    .is_some()
+            })
+            .cloned();
+
+        if let Some(output) = maybe_output {
+            let mut map = layer_map_for_output(&output);
+            let layer = map
+                .layer_for_surface(surface.wl_surface(), WindowSurfaceType::TOPLEVEL)
+                .unwrap()
+                .clone();
+            map.unmap_layer(&layer);
+
+            // collect screencopy sessions needing an update
+            let mut scheduled_sessions = self.schedule_workspace_sessions(surface.wl_surface());
+            if let Some(sessions) = output.user_data().get::<PendingScreencopyBuffers>() {
+                scheduled_sessions
+                    .get_or_insert_with(Vec::new)
+                    .extend(sessions.borrow_mut().drain(..));
+            }
+
+            self.backend.schedule_render(
+                &self.common.event_loop_handle,
+                &output,
+                scheduled_sessions,
+            );
+        }
+    }
 }
 
 delegate_layer_shell!(State);

src/wayland/handlers/mod.rs

@@ -19,28 +19,3 @@ pub mod viewporter;
 pub mod wl_drm;
 pub mod workspace;
 pub mod xdg_shell;
-
-use crate::state::Common;
-use smithay::{
-    reexports::wayland_server::protocol::wl_surface::WlSurface,
-    wayland::compositor::{add_destruction_hook, with_states},
-};
-
-fn mark_dirty_on_drop(state: &Common, wl_surface: &WlSurface) {
-    use std::sync::{
-        atomic::{AtomicBool, Ordering},
-        Arc,
-    };
-
-    let dirty = state.dirty_flag.clone();
-    struct DirtyFlag(Arc<AtomicBool>);
-
-    with_states(wl_surface, |data| {
-        data.data_map.insert_if_missing(|| DirtyFlag(dirty));
-    });
-    add_destruction_hook(wl_surface, |data| {
-        if let Some(DirtyFlag(dirty)) = data.data_map.get::<DirtyFlag>() {
-            dirty.store(true, Ordering::SeqCst);
-        }
-    })
-}

src/wayland/handlers/screencopy.rs

@@ -26,10 +26,10 @@ use smithay::{
     desktop::Window,
     output::Output,
     reexports::wayland_server::{
-        protocol::{wl_buffer::WlBuffer, wl_shm::Format as ShmFormat},
+        protocol::{wl_buffer::WlBuffer, wl_shm::Format as ShmFormat, wl_surface::WlSurface},
         Resource,
     },
-    utils::{Physical, Rectangle, Scale, Transform},
+    utils::{IsAlive, Physical, Rectangle, Scale, Transform},
     wayland::{
         dmabuf::get_dmabuf,
         shm::{with_buffer_contents, with_buffer_contents_mut},
@@ -869,4 +869,105 @@ impl UserdataExt for Window {
     }
 }
 
+impl State {
+    pub fn schedule_window_session(&mut self, surface: &WlSurface) {
+        if let Some(element) = self.common.shell.element_for_surface(surface).cloned() {
+            let active = element.active_window();
+            if active.toplevel().wl_surface() == surface {
+                for (session, params) in active.pending_buffers() {
+                    let window = active.clone();
+                    self.common.event_loop_handle.insert_idle(move |data| {
+                        if !session.alive() {
+                            return;
+                        }
+
+                        match render_window_to_buffer(
+                            &mut data.state,
+                            &session,
+                            params.clone(),
+                            &window,
+                        ) {
+                            // rendering yielded no damage, buffer is still pending
+                            Ok(false) => data.state.common.still_pending(session, params),
+                            Ok(true) => {} // success
+                            Err((reason, err)) => {
+                                slog_scope::warn!("Screencopy session failed: {}", err);
+                                session.failed(reason);
+                            }
+                        }
+                    });
+                }
+            }
+        }
+    }
+
+    pub fn schedule_workspace_sessions(
+        &mut self,
+        surface: &WlSurface,
+    ) -> Option<Vec<(Session, BufferParams)>> {
+        // here we store additional workspace_sessions, we should handle, when rendering the corresponding output anyway
+        let mut output_sessions: Option<Vec<(Session, BufferParams)>> = None;
+
+        // lets check which workspaces this surface belongs to
+        let active_spaces = self
+            .common
+            .shell
+            .outputs()
+            .map(|o| (o.clone(), self.common.shell.active_space(o).handle.clone()))
+            .collect::<Vec<_>>();
+        for (handle, output) in self.common.shell.workspaces_for_surface(surface) {
+            let workspace = self.common.shell.space_for_handle_mut(&handle).unwrap();
+            if !workspace.pending_buffers.is_empty() {
+                // TODO: replace with drain_filter....
+                let mut i = 0;
+                while i < workspace.pending_buffers.len() {
+                    if let SessionType::Workspace(o, w) =
+                        workspace.pending_buffers[i].0.session_type()
+                    {
+                        if active_spaces.contains(&(o.clone(), w)) {
+                            // surface is on an active workspace/output combo, add to workspace_sessions
+                            let (session, params) = workspace.pending_buffers.remove(i);
+                            output_sessions
+                                .get_or_insert_with(Vec::new)
+                                .push((session, params));
+                        } else if handle == w && output == o {
+                            // surface is visible on an offscreen workspace session, schedule a new render
+                            let (session, params) = workspace.pending_buffers.remove(i);
+                            let output = output.clone();
+                            self.common.event_loop_handle.insert_idle(move |data| {
+                                if !session.alive() {
+                                    return;
+                                }
+                                match render_workspace_to_buffer(
+                                    &mut data.state,
+                                    &session,
+                                    params.clone(),
+                                    &output,
+                                    &handle,
+                                ) {
+                                    Ok(false) => {
+                                        // rendering yielded no new damage, buffer still pending
+                                        data.state.common.still_pending(session, params);
+                                    }
+                                    Ok(true) => {}
+                                    Err((reason, err)) => {
+                                        slog_scope::warn!("Screencopy session failed: {}", err);
+                                        session.failed(reason);
+                                    }
+                                }
+                            });
+                        } else {
+                            i += 1;
+                        }
+                    } else {
+                        unreachable!();
+                    }
+                }
+            }
+        }
+
+        output_sessions
+    }
+}
+
 delegate_screencopy!(State);

src/wayland/handlers/xdg_shell/mod.rs

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-3.0-only
 
-use crate::utils::prelude::*;
+use crate::{utils::prelude::*, wayland::protocols::screencopy::SessionType};
 use smithay::{
     delegate_xdg_shell,
     desktop::{
@@ -26,6 +26,8 @@ use smithay::{
 };
 use std::cell::Cell;
 
+use super::screencopy::PendingScreencopyBuffers;
+
 pub mod popup;
 
 pub type PopupGrabData = Cell<Option<PopupGrab<State>>>;
@@ -36,8 +38,6 @@ impl XdgShellHandler for State {
     }
 
     fn new_toplevel(&mut self, surface: ToplevelSurface) {
-        super::mark_dirty_on_drop(&self.common, surface.wl_surface());
-
         let seat = self.common.last_active_seat().clone();
         let window = Window::new(Kind::Xdg(surface));
         self.common.shell.toplevel_info_state.new_toplevel(&window);
@@ -46,8 +46,6 @@ impl XdgShellHandler for State {
     }
 
     fn new_popup(&mut self, surface: PopupSurface, positioner: PositionerState) {
-        super::mark_dirty_on_drop(&self.common, surface.wl_surface());
-
         surface.with_pending_state(|state| {
             state.geometry = positioner.get_geometry();
             state.positioner = positioner;
@@ -280,6 +278,45 @@ impl XdgShellHandler for State {
             }
         }
     }
+
+    fn toplevel_destroyed(&mut self, surface: ToplevelSurface) {
+        let outputs = self
+            .common
+            .shell
+            .visible_outputs_for_surface(surface.wl_surface())
+            .collect::<Vec<_>>();
+        for output in outputs.iter() {
+            self.common.shell.active_space_mut(output).refresh();
+        }
+
+        // screencopy
+        let mut scheduled_sessions = self.schedule_workspace_sessions(surface.wl_surface());
+        for output in outputs.into_iter() {
+            if let Some(sessions) = output.user_data().get::<PendingScreencopyBuffers>() {
+                scheduled_sessions
+                    .get_or_insert_with(Vec::new)
+                    .extend(sessions.borrow_mut().drain(..));
+            }
+            self.backend.schedule_render(
+                &self.common.event_loop_handle,
+                &output,
+                scheduled_sessions.as_ref().map(|sessions| {
+                    sessions
+                        .iter()
+                        .filter(|(s, _)| match s.session_type() {
+                            SessionType::Output(o) | SessionType::Workspace(o, _)
+                                if o == output =>
+                            {
+                                true
+                            }
+                            _ => false,
+                        })
+                        .cloned()
+                        .collect::<Vec<_>>()
+                }),
+            );
+        }
+    }
 }
 
 fn check_grab_preconditions(