kms: Limit frame-callbacks to one per refresh

src/backend/kms/surface/mod.rs

@@ -77,7 +77,7 @@ use std::{
     collections::{HashMap, HashSet},
     mem,
     sync::{
-        atomic::{AtomicBool, AtomicUsize, Ordering},
+        atomic::{AtomicBool, Ordering},
         mpsc::{Receiver, SyncSender},
         Arc, Condvar, Mutex, RwLock,
     },
@@ -104,7 +104,6 @@ pub struct Surface {
     known_nodes: HashSet<DrmNode>,
 
     active: Arc<AtomicBool>,
-    frame_callback_seq: Arc<AtomicUsize>,
     feedback: HashMap<DrmNode, SurfaceDmabufFeedback>,
     plane_formats: HashSet<Format>,
 
@@ -123,7 +122,7 @@ pub struct SurfaceThreadState {
 
     state: QueueState,
     timings: Timings,
-    frame_callback_seq: Arc<AtomicUsize>,
+    frame_callback_seq: usize,
     thread_sender: Sender<SurfaceCommand>,
 
     output: Output,
@@ -239,7 +238,7 @@ pub enum ThreadCommand {
 
 #[derive(Debug)]
 pub enum SurfaceCommand {
-    SendFrames,
+    SendFrames(usize),
     RenderStates(RenderElementStates),
 }
 
@@ -255,14 +254,11 @@ impl Surface {
         shell: Arc<RwLock<Shell>>,
         startup_done: Arc<(Mutex<bool>, Condvar)>,
     ) -> Result<Self> {
-        let frame_callback_seq = Arc::new(AtomicUsize::new(0));
-
         let (tx, rx) = channel::<ThreadCommand>();
         let (tx2, rx2) = channel::<SurfaceCommand>();
         let active = Arc::new(AtomicBool::new(false));
 
         let active_clone = active.clone();
-        let frame_callback_seq_clone = frame_callback_seq.clone();
         let output_clone = output.clone();
 
         std::thread::Builder::new()
@@ -274,7 +270,6 @@ impl Surface {
                     target_node,
                     shell,
                     active_clone,
-                    frame_callback_seq_clone,
                     tx2,
                     rx,
                     startup_done,
@@ -287,8 +282,8 @@ impl Surface {
         let output_clone = output.clone();
         let thread_token = evlh
             .insert_source(rx2, move |command, _, state| match command {
-                Event::Msg(SurfaceCommand::SendFrames) => {
+                Event::Msg(SurfaceCommand::SendFrames(sequence)) => {
-                    state.common.send_frames(&output_clone);
+                    state.common.send_frames(&output_clone, Some(sequence));
                 }
                 Event::Msg(SurfaceCommand::RenderStates(states)) => {
                     state.common.update_primary_output(&output_clone, &states);
@@ -340,7 +335,6 @@ impl Surface {
             connector,
             crtc,
             output: output.clone(),
-            frame_callback_seq,
             known_nodes: HashSet::new(),
             active,
             feedback: HashMap::new(),
@@ -445,7 +439,6 @@ fn surface_thread(
     target_node: DrmNode,
     shell: Arc<RwLock<Shell>>,
     active: Arc<AtomicBool>,
-    frame_callback_seq: Arc<AtomicUsize>,
     thread_sender: Sender<SurfaceCommand>,
     thread_receiver: Channel<ThreadCommand>,
     startup_done: Arc<(Mutex<bool>, Condvar)>,
@@ -484,7 +477,7 @@ fn surface_thread(
 
         state: QueueState::Idle,
         timings: Timings::new(None, false),
-        frame_callback_seq,
+        frame_callback_seq: 0,
         thread_sender,
 
         output,
@@ -759,7 +752,7 @@ impl SurfaceThreadState {
             }
         }
 
-        self.frame_callback_seq.fetch_add(1, Ordering::SeqCst);
+        self.frame_callback_seq = self.frame_callback_seq.wrapping_add(1);
 
         if self.shell.read().unwrap().animations_going() {
             self.queue_redraw(false);
@@ -1232,7 +1225,7 @@ impl SurfaceThreadState {
                             };
 
                             if self.mirroring.is_none() {
-                                self.frame_callback_seq.fetch_add(1, Ordering::SeqCst);
+                                self.frame_callback_seq = self.frame_callback_seq.wrapping_add(1);
 
                                 let states = frame_result.states;
                                 self.send_frame_callbacks();
@@ -1311,7 +1304,9 @@ impl SurfaceThreadState {
 
     fn send_frame_callbacks(&mut self) {
         if self.mirroring.is_none() {
-            let _ = self.thread_sender.send(SurfaceCommand::SendFrames);
+            let _ = self
+                .thread_sender
+                .send(SurfaceCommand::SendFrames(self.frame_callback_seq));
         }
     }
 

src/backend/winit.rs

@@ -71,7 +71,7 @@ impl WinitState {
                 self.backend
                     .submit(damage.map(|x| x.as_slice()))
                     .with_context(|| "Failed to submit buffer for display")?;
-                state.send_frames(&self.output);
+                state.send_frames(&self.output, None);
                 state.update_primary_output(&self.output, &states);
                 state.send_dmabuf_feedback(&self.output, &states, |_| None);
                 if damage.is_some() {

src/backend/x11.rs

@@ -221,7 +221,7 @@ impl Surface {
                 self.surface
                     .submit()
                     .with_context(|| "Failed to submit buffer for display")?;
-                state.send_frames(&self.output);
+                state.send_frames(&self.output, None);
                 state.update_primary_output(&self.output, &states);
                 state.send_dmabuf_feedback(&self.output, &states, |_| None);
                 if damage.is_some() {

src/state.rs

@@ -60,7 +60,7 @@ use smithay::{
     utils::{Clock, IsAlive, Monotonic},
     wayland::{
         alpha_modifier::AlphaModifierState,
-        compositor::{CompositorClientState, CompositorState},
+        compositor::{CompositorClientState, CompositorState, SurfaceData},
         dmabuf::{DmabufFeedback, DmabufGlobal, DmabufState},
         fractional_scale::{with_fractional_scale, FractionalScaleManagerState},
         idle_inhibit::IdleInhibitManagerState,
@@ -233,6 +233,18 @@ pub struct SurfaceDmabufFeedback {
     pub scanout_feedback: DmabufFeedback,
 }
 
+#[derive(Debug)]
+struct SurfaceFrameThrottlingState {
+    last_sent_at: RefCell<Option<(Output, usize)>>,
+}
+impl Default for SurfaceFrameThrottlingState {
+    fn default() -> Self {
+        SurfaceFrameThrottlingState {
+            last_sent_at: RefCell::new(None),
+        }
+    }
+}
+
 impl BackendData {
     pub fn kms(&mut self) -> &mut KmsState {
         match self {
@@ -836,9 +848,45 @@ impl Common {
         }
     }
 
-    pub fn send_frames(&self, output: &Output) {
+    pub fn send_frames(&self, output: &Output, sequence: Option<usize>) {
         let time = self.clock.now();
-        let throttle = Some(Duration::from_secs(1));
+        let should_send = |surface: &WlSurface, states: &SurfaceData| {
+            // Do the standard primary scanout output check. For pointer surfaces it deduplicates
+            // the frame callbacks across potentially multiple outputs, and for regular windows and
+            // layer-shell surfaces it avoids sending frame callbacks to invisible surfaces.
+            let current_primary_output = surface_primary_scanout_output(surface, states);
+            if current_primary_output.as_ref() != Some(output) {
+                return None;
+            }
+
+            let Some(sequence) = sequence else {
+                return Some(output.clone());
+            };
+
+            // Next, check the throttling status.
+            let frame_throttling_state = states
+                .data_map
+                .get_or_insert(SurfaceFrameThrottlingState::default);
+            let mut last_sent_at = frame_throttling_state.last_sent_at.borrow_mut();
+
+            let mut send = true;
+
+            // If we already sent a frame callback to this surface this output refresh
+            // cycle, don't send one again to prevent empty-damage commit busy loops.
+            if let Some((last_output, last_sequence)) = &*last_sent_at {
+                if last_output == output && *last_sequence == sequence {
+                    send = false;
+                }
+            }
+
+            if send {
+                *last_sent_at = Some((output.clone(), sequence));
+                Some(output.clone())
+            } else {
+                None
+            }
+        };
+        let throttle = Some(Duration::from_millis(995));
         let shell = self.shell.read().unwrap();
 
         if let Some(session_lock) = shell.session_lock.as_ref() {
@@ -849,9 +897,13 @@ impl Common {
                             .set_preferred_scale(output.current_scale().fractional_scale());
                     });
                 });
-                send_frames_surface_tree(lock_surface.wl_surface(), output, time, None, |_, _| {
+                send_frames_surface_tree(
-                    Some(output.clone())
+                    lock_surface.wl_surface(),
-                });
+                    output,
+                    time,
+                    None,
+                    should_send,
+                );
             }
         }
 
@@ -875,15 +927,19 @@ impl Common {
                 .unwrap_or(CursorImageStatus::default_named());
 
             if let CursorImageStatus::Surface(wl_surface) = cursor_status {
-                send_frames_surface_tree(&wl_surface, output, time, Some(Duration::ZERO), |_, _| {
+                send_frames_surface_tree(
-                    None
+                    &wl_surface,
-                })
+                    output,
+                    time,
+                    Some(Duration::ZERO),
+                    should_send,
+                )
             }
 
             if let Some(move_grab) = seat.user_data().get::<SeatMoveGrabState>() {
                 if let Some(grab_state) = move_grab.lock().unwrap().as_ref() {
                     for (window, _) in grab_state.element().windows() {
-                        window.send_frame(output, time, throttle, surface_primary_scanout_output);
+                        window.send_frame(output, time, throttle, should_send);
                     }
                 }
             }
@@ -898,14 +954,14 @@ impl Common {
             .mapped()
             .for_each(|mapped| {
                 for (window, _) in mapped.windows() {
-                    window.send_frame(output, time, throttle, surface_primary_scanout_output);
+                    window.send_frame(output, time, throttle, should_send);
                 }
             });
 
         let active = shell.active_space(output);
         active.mapped().for_each(|mapped| {
             for (window, _) in mapped.windows() {
-                window.send_frame(output, time, throttle, surface_primary_scanout_output);
+                window.send_frame(output, time, throttle, should_send);
             }
         });
 
@@ -934,19 +990,13 @@ impl Common {
 
         shell.override_redirect_windows.iter().for_each(|or| {
             if let Some(wl_surface) = or.wl_surface() {
-                send_frames_surface_tree(
+                send_frames_surface_tree(&wl_surface, output, time, throttle, should_send);
-                    &wl_surface,
-                    output,
-                    time,
-                    throttle,
-                    surface_primary_scanout_output,
-                );
             }
         });
 
         let map = smithay::desktop::layer_map_for_output(output);
         for layer_surface in map.layers() {
-            layer_surface.send_frame(output, time, throttle, surface_primary_scanout_output);
+            layer_surface.send_frame(output, time, throttle, should_send);
         }
     }
 }