From 96b0a9d09784227b66204bef9717fd0ea0878ef1 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Fri, 2 Oct 2026 08:16:00 +0100 Subject: [PATCH] Separate presentation facts from surface eligibility policy --- compositor/src/backends/direct.rs | 5 +- compositor/src/display_presentation.rs | 38 ++------------- compositor/src/handlers/compositor.rs | 19 +------- compositor/src/main.rs | 1 + compositor/src/portal_session.rs | 8 +++- compositor/src/presentation_policy.rs | 64 ++++++++++++++++++++++++++ compositor/src/session_lock.rs | 5 +- compositor/src/state/hit_testing.rs | 3 +- compositor/src/state/layout.rs | 3 -- compositor/src/winit.rs | 9 ++-- 10 files changed, 88 insertions(+), 67 deletions(-) create mode 100644 compositor/src/presentation_policy.rs diff --git a/compositor/src/backends/direct.rs b/compositor/src/backends/direct.rs index f022e3d..db938cf 100644 --- a/compositor/src/backends/direct.rs +++ b/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::>(); 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); diff --git a/compositor/src/display_presentation.rs b/compositor/src/display_presentation.rs index fe56ce3..e4b8d3d 100644 --- a/compositor/src/display_presentation.rs +++ b/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. - // A stable tie-break avoids alternating owners when refresh rates match. - pub fn callback_output(&self, surface: &Id) -> Option { + pub fn outputs_for<'a>(&'a self, surface: &'a Id) -> impl Iterator { 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); - } } diff --git a/compositor/src/handlers/compositor.rs b/compositor/src/handlers/compositor.rs index 757204b..fe50e41 100644 --- a/compositor/src/handlers/compositor.rs +++ b/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(); - } + 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 diff --git a/compositor/src/main.rs b/compositor/src/main.rs index e56b3a1..6927da1 100644 --- a/compositor/src/main.rs +++ b/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; diff --git a/compositor/src/portal_session.rs b/compositor/src/portal_session.rs index dd317e7..2fb84fa 100644 --- a/compositor/src/portal_session.rs +++ b/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(), ); } diff --git a/compositor/src/presentation_policy.rs b/compositor/src/presentation_policy.rs new file mode 100644 index 0000000..6d2b13c --- /dev/null +++ b/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 { + 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() + } +} diff --git a/compositor/src/session_lock.rs b/compositor/src/session_lock.rs index cbcd927..20859a2 100644 --- a/compositor/src/session_lock.rs +++ b/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); diff --git a/compositor/src/state/hit_testing.rs b/compositor/src/state/hit_testing.rs index a278f3b..8c89479 100644 --- a/compositor/src/state/hit_testing.rs +++ b/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() }); } diff --git a/compositor/src/state/layout.rs b/compositor/src/state/layout.rs index c95f92f..114a81f 100644 --- a/compositor/src/state/layout.rs +++ b/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(); diff --git a/compositor/src/winit.rs b/compositor/src/winit.rs index fc10920..263f5d5 100644 --- a/compositor/src/winit.rs +++ b/compositor/src/winit.rs @@ -225,6 +225,7 @@ pub fn init(event_loop: &mut EventLoop, 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, _| { - state.display_presentation.callback_output(&surface.into()) - }); + window.send_frame(output, time, None, |surface, _| eligible.get(&surface.into()).cloned()); } for layer in layer_surfaces(output) { - layer.send_frame(output, time, None, |surface, _| { - state.display_presentation.callback_output(&surface.into()) - }); + layer.send_frame(output, time, None, |surface, _| eligible.get(&surface.into()).cloned()); } state.send_cursor_frame(output); }