Separate presentation facts from surface eligibility policy

compositor/src/backends/direct.rs

@@ -1560,6 +1560,7 @@ fn deliver_frame_callbacks(state: &mut Ferese, node: DrmNode, crtc: crtc::Handle
 }
 
 fn send_frame_callbacks(state: &mut Ferese, output: &Output) {
+    let eligible = state.callback_outputs();
     if state.session_lock.active {
         state.lock_frame_callbacks(output);
         return;
@@ -1576,13 +1577,13 @@ fn send_frame_callbacks(state: &mut Ferese, output: &Output) {
         })
         .for_each(|window| {
             window.send_frame(output, state.start_time.elapsed(), None, |surface, _| {
-                state.display_presentation.callback_output(&surface.into())
+                eligible.get(&surface.into()).cloned()
             });
         });
     let layers = layer_map_for_output(output).layers().cloned().collect::<Vec<_>>();
     layers.iter().for_each(|layer| {
         layer.send_frame(output, state.start_time.elapsed(), None, |surface, _| {
-            state.display_presentation.callback_output(&surface.into())
+            eligible.get(&surface.into()).cloned()
         });
     });
     state.send_cursor_frame(output);

compositor/src/display_presentation.rs

@@ -43,28 +43,16 @@ impl DisplayPresentation {
         }
     }
 
+    #[cfg(test)]
     pub fn visible(&self, surface: &Id) -> bool {
         self.outputs.values().any(|record| record.displayed.contains(surface))
     }
 
-    // Membership describes every output; callback ownership is a separate choice.
+    pub fn outputs_for<'a>(&'a self, surface: &'a Id) -> impl Iterator<Item = &'a Output> {
-    // A stable tie-break avoids alternating owners when refresh rates match.
-    pub fn callback_output(&self, surface: &Id) -> Option<Output> {
         self.outputs
             .iter()
             .filter(|(_, record)| record.displayed.contains(surface))
             .map(|(output, _)| output)
-            .max_by_key(|output| (output.current_mode().map_or(0, |mode| mode.refresh), output.name()))
-            .cloned()
-    }
-
-    pub fn remove_surface(&mut self, surface: &Id) {
-        for record in self.outputs.values_mut() {
-            record.displayed.remove(surface);
-            if let Some(queued) = &mut record.queued {
-                queued.remove(surface);
-            }
-        }
     }
 
     pub fn remove_output(&mut self, output: &Output) {
@@ -126,7 +114,7 @@ mod tests {
     }
 
     #[test]
-    fn outputs_are_independent_and_removal_cannot_resurrect_a_surface() {
+    fn outputs_are_independent_and_queued_history_survives_until_presentation() {
         let mut record = DisplayPresentation::default();
         let a = output("a");
         let b = output("b");
@@ -138,9 +126,9 @@ mod tests {
         record.remove_output(&a);
         assert!(record.visible(&id));
         record.queued(&b, &scene(&id));
-        record.remove_surface(&id);
+        assert!(record.visible(&id));
         record.presented(&b);
-        assert!(!record.visible(&id));
+        assert!(record.visible(&id));
         record.clear();
         assert!(!record.visible(&id));
     }
@@ -155,20 +143,4 @@ mod tests {
         states.states.get_mut(&id).unwrap().presentation_state = RenderElementPresentationState::Skipped;
         assert!(visible(&states).is_empty());
     }
-    #[test]
-    fn callback_ownership_is_stable_and_moves_when_an_output_disappears() {
-        let mut record = DisplayPresentation::default();
-        let a = output("a");
-        let b = output("b");
-        let id = Id::new();
-        for output in [&a, &b] {
-            record.queued(output, &scene(&id));
-            record.presented(output);
-        }
-        assert_eq!(record.callback_output(&id), Some(b.clone()));
-        record.remove_output(&b);
-        assert_eq!(record.callback_output(&id), Some(a.clone()));
-        record.remove_surface(&id);
-        assert_eq!(record.callback_output(&id), None);
-    }
 }

compositor/src/handlers/compositor.rs

@@ -71,22 +71,7 @@ impl CompositorHandler for Ferese {
                 self.record_client_commit(&window);
             }
         }
-        if !is_sync_subsurface(surface) {
-            let mut surfaces = Vec::new();
-            smithay::desktop::utils::with_surfaces_surface_tree(surface, |surface, _| surfaces.push(surface.clone()));
-            for surface in surfaces {
-                let mapped = smithay::backend::renderer::utils::with_renderer_surface_state(&surface, |state| {
-                    state.buffer().is_some()
-                })
-                .unwrap_or(false);
-                if !mapped {
-                    smithay::desktop::utils::with_surfaces_surface_tree(&surface, |child, _| {
-                        self.display_presentation.remove_surface(&child.into());
-                    });
-                }
-            }
         self.refresh_idle_inhibition();
-        }
 
         layer_shell::handle_commit(self, surface);
         xdg_shell::handle_commit(&mut self.popups, &mut self.space, surface);
@@ -94,8 +79,6 @@ impl CompositorHandler for Ferese {
     }
 
     fn destroyed(&mut self, surface: &WlSurface) {
-        self.display_presentation.remove_surface(&surface.into());
-
         if self.session_lock.active {
             self.session_lock
                 .surfaces

compositor/src/main.rs

@@ -20,6 +20,7 @@ mod overview;
 mod portal_session;
 mod portal_shortcuts;
 mod presentation;
+mod presentation_policy;
 mod private_client;
 mod reload;
 mod render;

compositor/src/portal_session.rs

@@ -235,10 +235,16 @@ impl PortalSession {
 
 impl crate::Ferese {
     pub(crate) fn refresh_idle_inhibition(&mut self) {
+        let eligible = if self.idle_inhibitors.is_empty() {
+            Default::default()
+        } else {
+            self.callback_outputs()
+        };
+
         self.idle_notifier_state.set_is_inhibited(
             self.idle_inhibitors
                 .keys()
-                .any(|surface| self.display_presentation.visible(&surface.into()))
+                .any(|surface| eligible.contains_key(&surface.into()))
                 || self.portal_session.idle_inhibited(),
         );
     }

compositor/src/presentation_policy.rs

@@ -0,0 +1,64 @@
+//! Eligibility and callback ownership are policy, not presentation history.
+use std::collections::HashMap;
+
+use smithay::backend::renderer::element::Id;
+use smithay::backend::renderer::utils::with_renderer_surface_state;
+use smithay::desktop::layer_map_for_output;
+use smithay::output::Output;
+use smithay::reexports::wayland_server::Resource;
+use smithay::reexports::wayland_server::protocol::wl_surface::WlSurface;
+use smithay::wayland::compositor::get_parent;
+
+use crate::Ferese;
+
+fn mapped_tree(surface: &WlSurface) -> bool {
+    let mut current = Some(surface.clone());
+    while let Some(surface) = current {
+        if !surface.is_alive()
+            || !with_renderer_surface_state(&surface, |state| state.buffer().is_some()).unwrap_or(false)
+        {
+            return false;
+        }
+        current = get_parent(&surface);
+    }
+    true
+}
+
+impl Ferese {
+    pub(crate) fn callback_outputs(&self) -> HashMap<Id, Output> {
+        let mut surfaces = Vec::new();
+        if self.session_lock.active {
+            for lock in self.session_lock.surfaces.values() {
+                smithay::desktop::utils::with_surfaces_surface_tree(lock.wl_surface(), |surface, _| {
+                    surfaces.push(surface.clone());
+                });
+            }
+        } else {
+            for window in self.space.elements() {
+                window.with_surfaces(|surface, _| surfaces.push(surface.clone()));
+            }
+            for output in self.space.outputs() {
+                for layer in layer_map_for_output(output).layers() {
+                    layer.with_surfaces(|surface, _| surfaces.push(surface.clone()));
+                }
+            }
+            if let smithay::input::pointer::CursorImageStatus::Surface(surface) = &self.cursor_status {
+                surfaces.push(surface.clone());
+            }
+        }
+
+        surfaces
+            .into_iter()
+            .filter(|surface| mapped_tree(surface))
+            .filter_map(|surface| {
+                let id = Id::from(&surface);
+                let output = self
+                    .display_presentation
+                    .outputs_for(&id)
+                    .max_by_key(|output| (output.current_mode().map_or(0, |mode| mode.refresh), output.name()))?
+                    .clone();
+                Some((id, output))
+            })
+            .collect()
+    }
+}

compositor/src/session_lock.rs

@@ -213,13 +213,14 @@ impl Ferese {
     }
 
     pub(crate) fn lock_frame_callbacks(&self, output: &Output) {
+        let eligible = self.callback_outputs();
         if let Some(surface) = self.session_lock.surfaces.get(output).filter(|surface| surface.alive()) {
             smithay::desktop::utils::send_frames_surface_tree(
                 surface.wl_surface(),
                 output,
                 self.start_time.elapsed(),
                 None,
-                |surface, _| self.display_presentation.callback_output(&surface.into()),
+                |surface, _| eligible.get(&surface.into()).cloned(),
             );
         }
     }
@@ -236,7 +237,6 @@ impl SessionLockHandler for Ferese {
         }
         self.cancel_logout_confirmation();
         self.session_lock.active = true;
-        self.display_presentation.clear();
         self.refresh_idle_inhibition();
         self.session_lock.idle_since = Some(Instant::now());
         match self
@@ -304,7 +304,6 @@ impl SessionLockHandler for Ferese {
         }
         let sleeping = self.session_lock.sleeping;
         self.session_lock = Lock::default();
-        self.display_presentation.clear();
         self.refresh_idle_inhibition();
         if sleeping {
             crate::backends::direct::wake_locked_outputs(self);

compositor/src/state/hit_testing.rs

@@ -109,6 +109,7 @@ impl Ferese {
     }
 
     pub fn send_cursor_frame(&self, output: &Output) {
+        let eligible = self.callback_outputs();
         let CursorImageStatus::Surface(surface) = &self.cursor_status else {
             return;
         };
@@ -120,7 +121,7 @@ impl Ferese {
         }
 
         send_frames_surface_tree(surface, output, self.start_time.elapsed(), None, |surface, _| {
-            self.display_presentation.callback_output(&surface.into())
+            eligible.get(&surface.into()).cloned()
         });
     }
 

compositor/src/state/layout.rs

@@ -44,9 +44,6 @@ impl Ferese {
                 geometry.settle_presentation();
             }
 
-            window.with_surfaces(|surface, _| {
-                self.display_presentation.remove_surface(&surface.into());
-            });
             self.space.unmap_elem(&window);
         }
         self.refresh_idle_inhibition();

compositor/src/winit.rs

@@ -225,6 +225,7 @@ pub fn init(event_loop: &mut EventLoop<Ferese>, state: &mut Ferese) -> Result<()
 }
 
 fn send_nested_frame_callbacks(state: &mut Ferese, output: &Output) {
+    let eligible = state.callback_outputs();
     if state.session_lock.active {
         state.lock_frame_callbacks(output);
         return;
@@ -237,14 +238,10 @@ fn send_nested_frame_callbacks(state: &mut Ferese, output: &Output) {
             .get(*window)
             .is_some_and(|id| state.window_belongs_to_output(*id, output))
     }) {
-        window.send_frame(output, time, None, |surface, _| {
+        window.send_frame(output, time, None, |surface, _| eligible.get(&surface.into()).cloned());
-            state.display_presentation.callback_output(&surface.into())
-        });
     }
     for layer in layer_surfaces(output) {
-        layer.send_frame(output, time, None, |surface, _| {
+        layer.send_frame(output, time, None, |surface, _| eligible.get(&surface.into()).cloned());
-            state.display_presentation.callback_output(&surface.into())
-        });
     }
     state.send_cursor_frame(output);
 }