wl_surface: dispatch frame requests from vblank event handlers

src/backends/metal.rs

@@ -221,7 +221,6 @@ impl Backend for MetalBackend {
                 connector.crtc.take();
                 connector.on_change.clear();
                 connector.present_trigger.clear();
-                connector.render_result.take();
                 connector.active_framebuffer.take();
                 connector.next_framebuffer.take();
             }

src/backends/metal/present.rs

@@ -486,8 +486,6 @@ impl MetalConnector {
         if damage == 0 {
             return None;
         }
-        let mut rr = self.render_result.borrow_mut();
-        rr.output_id = node.id;
         let render_hw_cursor = !self.cursor_enabled.get();
         let mode = node.global.mode.get();
         let pass = create_render_pass(
@@ -495,7 +493,6 @@ impl MetalConnector {
             &**node,
             &self.state,
             Some(node.global.pos.get()),
-            Some(&mut rr),
             node.global.persistent.scale.get(),
             true,
             render_hw_cursor,
@@ -503,7 +500,6 @@ impl MetalConnector {
             node.global.persistent.transform.get(),
             Some(&self.state.damage_visualizer),
         );
-        rr.dispatch_frame_requests(self.state.now_msec());
         Some(Latched { pass, damage })
     }
 

src/backends/metal/video.rs

@@ -26,7 +26,6 @@ use {
             wl_output::OutputId,
             wp_presentation_feedback::{KIND_HW_COMPLETION, KIND_VSYNC},
         },
-        renderer::RenderResult,
         state::State,
         udev::UdevDevice,
         utils::{
@@ -447,8 +446,6 @@ pub struct MetalConnector {
 
     pub present_trigger: AsyncEvent,
 
-    pub render_result: RefCell<RenderResult>,
-
     pub cursor_x: Cell<i32>,
     pub cursor_y: Cell<i32>,
     pub cursor_enabled: Cell<bool>,
@@ -1044,7 +1041,6 @@ fn create_connector(
         crtc: Default::default(),
         on_change: Default::default(),
         present_trigger: Default::default(),
-        render_result: RefCell::new(Default::default()),
         cursor_x: Cell::new(0),
         cursor_y: Cell::new(0),
         cursor_enabled: Cell::new(false),
@@ -1912,6 +1908,7 @@ impl MetalBackend {
         };
         self.update_sequence(&connector, sequence);
         connector.queue_sequence();
+        self.state.vblank(connector.connector_id);
         let dd = connector.display.borrow();
         connector
             .next_flip_nsec

src/backends/x.rs

@@ -12,7 +12,6 @@ use {
         format::XRGB8888,
         gfx_api::{GfxContext, GfxError, GfxFramebuffer, GfxTexture},
         ifs::wl_output::OutputId,
-        renderer::RenderResult,
         state::State,
         utils::{
             clonecell::CloneCell, copyhashmap::CopyHashMap, errorfmt::ErrorFmt, numcell::NumCell,
@@ -239,7 +238,6 @@ pub async fn create(state: &Rc<State>) -> Result<Rc<XBackend>, XBackendError> {
         root,
         scheduled_present: Default::default(),
         grab_requests: Default::default(),
-        render_result: Default::default(),
         drm_device_id: state.drm_dev_ids.next(),
         drm_dev,
     });
@@ -274,7 +272,6 @@ pub struct XBackend {
     root: u32,
     scheduled_present: AsyncQueue<Rc<XOutput>>,
     grab_requests: AsyncQueue<(Rc<XSeat>, bool)>,
-    render_result: RefCell<RenderResult>,
     drm_device_id: DrmDeviceId,
     drm_dev: dev_t,
 }
@@ -702,6 +699,7 @@ impl XBackend {
         } else {
             image.render_on_idle.set(true);
         }
+        self.state.vblank(output.id);
         Ok(())
     }
 
@@ -745,13 +743,9 @@ impl XBackend {
         image.last_serial.set(serial);
 
         if let Some(node) = self.state.root.outputs.get(&output.id) {
-            let res = self.state.present_output(
+            let res = self
-                &node,
+                .state
-                &image.fb.get(),
+                .present_output(&node, &image.fb.get(), &image.tex.get(), true);
-                &image.tex.get(),
-                &mut self.render_result.borrow_mut(),
-                true,
-            );
             if let Err(e) = res {
                 log::error!("Could not render screen: {}", ErrorFmt(e));
                 return;

src/compositor.rs

@@ -504,6 +504,7 @@ fn create_dummy_output(state: &Rc<State>) {
         screencopies: Default::default(),
         title_visible: Cell::new(false),
         schedule,
+        vblank_event: Default::default(),
         latch_event: Default::default(),
         presentation_event: Default::default(),
     });

src/gfx_api.rs

@@ -8,7 +8,7 @@ use {
         fixed::Fixed,
         format::Format,
         rect::{Rect, Region},
-        renderer::{renderer_base::RendererBase, RenderResult, Renderer},
+        renderer::{renderer_base::RendererBase, Renderer},
         scale::Scale,
         state::State,
         theme::Color,
@@ -345,7 +345,6 @@ impl dyn GfxFramebuffer {
         node: &dyn Node,
         state: &State,
         cursor_rect: Option<Rect>,
-        result: Option<&mut RenderResult>,
         scale: Scale,
         render_cursor: bool,
         render_hardware_cursor: bool,
@@ -358,7 +357,6 @@ impl dyn GfxFramebuffer {
             node,
             state,
             cursor_rect,
-            result,
             scale,
             render_cursor,
             render_hardware_cursor,
@@ -377,7 +375,6 @@ impl dyn GfxFramebuffer {
         node: &OutputNode,
         state: &State,
         cursor_rect: Option<Rect>,
-        result: Option<&mut RenderResult>,
         scale: Scale,
         render_hardware_cursor: bool,
     ) -> Result<Option<SyncFile>, GfxError> {
@@ -385,7 +382,6 @@ impl dyn GfxFramebuffer {
             node,
             state,
             cursor_rect,
-            result,
             scale,
             true,
             render_hardware_cursor,
@@ -399,7 +395,6 @@ impl dyn GfxFramebuffer {
         node: &dyn Node,
         state: &State,
         cursor_rect: Option<Rect>,
-        result: Option<&mut RenderResult>,
         scale: Scale,
         render_cursor: bool,
         render_hardware_cursor: bool,
@@ -410,7 +405,6 @@ impl dyn GfxFramebuffer {
             node,
             state,
             cursor_rect,
-            result,
             scale,
             render_cursor,
             render_hardware_cursor,
@@ -432,7 +426,6 @@ impl dyn GfxFramebuffer {
         let mut renderer = Renderer {
             base: self.renderer_base(&mut ops, scale, transform),
             state,
-            result: None,
             logical_extents: Rect::new_empty(0, 0),
             pixel_extents: {
                 let (width, height) = self.logical_size(transform);
@@ -638,7 +631,6 @@ pub fn create_render_pass(
     node: &dyn Node,
     state: &State,
     cursor_rect: Option<Rect>,
-    result: Option<&mut RenderResult>,
     scale: Scale,
     render_cursor: bool,
     render_hardware_cursor: bool,
@@ -650,7 +642,6 @@ pub fn create_render_pass(
     let mut renderer = Renderer {
         base: renderer_base(physical_size, &mut ops, scale, transform),
         state,
-        result,
         logical_extents: node.node_absolute_position().at_point(0, 0),
         pixel_extents: {
             let (width, height) = logical_size(physical_size, transform);

src/ifs/jay_screencast.rs

@@ -192,7 +192,6 @@ impl JayScreencast {
                     tl.tl_as_node(),
                     &self.client.state,
                     Some(tl.node_absolute_position()),
-                    None,
                     scale,
                     true,
                     true,

src/ifs/wl_surface.rs

@@ -64,8 +64,8 @@ use {
         renderer::Renderer,
         tree::{
             ContainerNode, FindTreeResult, FoundNode, LatchListener, Node, NodeId, NodeVisitor,
-            NodeVisitorBase, OutputNode, OutputNodeId, PlaceholderNode, PresentationListener,
+            NodeVisitorBase, OutputNode, PlaceholderNode, PresentationListener, ToplevelNode,
-            ToplevelNode,
+            VblankListener,
         },
         utils::{
             cell_ext::CellExt, clonecell::CloneCell, copyhashmap::CopyHashMap,
@@ -276,14 +276,12 @@ pub struct WlSurface {
     pub buffer_abs_pos: Cell<Rect>,
     pub need_extents_update: Cell<bool>,
     pub buffer: CloneCell<Option<Rc<SurfaceBuffer>>>,
-    buffer_presented: Cell<bool>,
-    buffer_had_frame_request: Cell<bool>,
     pub shm_textures: DoubleBuffered<SurfaceShmTexture>,
     pub buf_x: NumCell<i32>,
     pub buf_y: NumCell<i32>,
     pub children: RefCell<Option<Box<ParentData>>>,
     ext: CloneCell<Rc<dyn SurfaceExt>>,
-    pub frame_requests: RefCell<Vec<Rc<WlCallback>>>,
+    frame_requests: RefCell<Vec<Rc<WlCallback>>>,
     presentation_feedback: RefCell<Vec<Rc<WpPresentationFeedback>>>,
     latched_presentation_feedback: RefCell<Vec<Rc<WpPresentationFeedback>>>,
     seat_state: NodeSeatState,
@@ -309,6 +307,7 @@ pub struct WlSurface {
     alpha_modifier: CloneCell<Option<Rc<WpAlphaModifierSurfaceV1>>>,
     alpha: Cell<Option<f32>>,
     pub text_input_connections: SmallMap<SeatId, Rc<TextInputConnection>, 1>,
+    vblank_listener: EventListener<dyn VblankListener>,
     latch_listener: EventListener<dyn LatchListener>,
     presentation_listener: EventListener<dyn PresentationListener>,
     commit_version: NumCell<u64>,
@@ -594,8 +593,6 @@ impl WlSurface {
             buffer_abs_pos: Cell::new(Default::default()),
             need_extents_update: Default::default(),
             buffer: Default::default(),
-            buffer_presented: Default::default(),
-            buffer_had_frame_request: Default::default(),
             shm_textures: DoubleBuffered::new(DamageQueue::new().map(|damage| SurfaceShmTexture {
                 tex: Default::default(),
                 damage,
@@ -630,6 +627,7 @@ impl WlSurface {
             alpha_modifier: Default::default(),
             alpha: Default::default(),
             text_input_connections: Default::default(),
+            vblank_listener: EventListener::new(slf.clone()),
             latch_listener: EventListener::new(slf.clone()),
             presentation_listener: EventListener::new(slf.clone()),
             commit_version: Default::default(),
@@ -1122,10 +1120,6 @@ impl WlSurface {
                     release,
                 };
                 self.buffer.set(Some(Rc::new(surface_buffer)));
-                if pending.has_damage() {
-                    self.buffer_presented.set(false);
-                    self.buffer_had_frame_request.set(false);
-                }
             } else {
                 self.reset_shm_textures();
                 self.buf_x.set(0);
@@ -1237,14 +1231,11 @@ impl WlSurface {
                 damage_full = true;
             }
         }
-        let had_frame_requests = self.buffer_had_frame_request.get();
         let has_frame_requests = {
             let frs = &mut *self.frame_requests.borrow_mut();
             frs.append(&mut pending.frame_request);
             frs.is_not_empty()
         };
-        self.buffer_had_frame_request
-            .set(had_frame_requests || has_frame_requests);
         let has_presentation_feedback = {
             let mut fbs = self.presentation_feedback.borrow_mut();
             for fb in fbs.drain(..) {
@@ -1295,6 +1286,9 @@ impl WlSurface {
         self.ext.get().after_apply_commit();
         if self.visible.get() {
             let output = self.output.get();
+            if has_frame_requests {
+                self.vblank_listener.attach(&output.vblank_event);
+            }
             if has_presentation_feedback {
                 self.latch_listener.attach(&output.latch_event);
             }
@@ -1306,35 +1300,16 @@ impl WlSurface {
                     damage = damage.intersect(tl.node_absolute_position());
                 }
                 self.client.state.damage(damage);
-            } else if self.buffer_presented.get() {
+            } else if pending.has_damage() {
-                // If the currently attached buffer has already been fully presented ...
-                if has_frame_requests {
-                    // ... and there are new frame requests ...
-                    if had_frame_requests {
-                        // ... and we've already dispatched frame requests for that buffer,
-                        //     then schedule new presentation of the primary output and
-                        //     unset the buffer_presented flag. This is for clients that
-                        //     send frame requests at an uncapped rate and expect the
-                        //     compositor to dispatch frame requests at the monitor
-                        //     refresh rate (e.g. firefox). This is very inefficient and
-                        //     disables VRR from working correctly. But since only firefox
-                        //     is affected, I'm ok with this.
-                        let rect = self.output.get().global.pos.get();
-                        self.client.state.damage(rect);
-                        self.buffer_presented.set(false);
-                    } else {
-                        // ... and this is the first commit that attaches a frame request
-                        //     for that buffer, then dispatch the frame requests
-                        //     immediately.
-                        let now = self.client.state.now_msec() as _;
-                        for fr in self.frame_requests.borrow_mut().drain(..) {
-                            fr.send_done(now);
-                            let _ = fr.client.remove_obj(&*fr);
-                        }
-                    }
-                }
-            } else {
                 self.apply_damage(pending);
+                if has_frame_requests {
+                    output.global.connector.damage();
+                }
+            } else if has_frame_requests && output.schedule.vrr_enabled() {
+                // Frame requests must be dispatched at the highest possible frame rate.
+                // Therefore we must trigger a vsync of the output as soon as possible.
+                let rect = output.global.pos.get();
+                self.client.state.damage(rect);
             }
         }
         pending.buffer_damage.clear();
@@ -1491,6 +1466,7 @@ impl WlSurface {
     }
 
     fn attach_events_to_output(&self, output: &OutputNode) {
+        self.vblank_listener.attach(&output.vblank_event);
         self.latch_listener.attach(&output.latch_event);
     }
 
@@ -1519,12 +1495,6 @@ impl WlSurface {
         self.seat_state.set_visible(self, visible);
     }
 
-    pub fn presented(&self, on: OutputNodeId) {
-        if on == self.output.get().id {
-            self.buffer_presented.set(true);
-        }
-    }
-
     pub fn detach_node(&self, set_invisible: bool) {
         for (_, constraint) in &self.constraints {
             constraint.deactivate();
@@ -2083,6 +2053,19 @@ impl DamageMatrix {
     }
 }
 
+impl VblankListener for WlSurface {
+    fn after_vblank(self: Rc<Self>) {
+        if self.visible.get() {
+            let now = self.client.state.now_usec();
+            for fr in self.frame_requests.borrow_mut().drain(..) {
+                fr.send_done(now as _);
+                let _ = fr.client.remove_obj(&*fr);
+            }
+        }
+        self.vblank_listener.detach();
+    }
+}
+
 impl LatchListener for WlSurface {
     fn after_latch(self: Rc<Self>) {
         if self.visible.get() {

src/renderer.rs

@@ -1,14 +1,11 @@
 use {
     crate::{
         gfx_api::{AcquireSync, GfxApiOpt, ReleaseSync, SampleRect},
-        ifs::{
+        ifs::wl_surface::{
-            wl_callback::WlCallback,
+            x_surface::xwindow::Xwindow,
-            wl_surface::{
+            xdg_surface::{xdg_toplevel::XdgToplevel, XdgSurface},
-                x_surface::xwindow::Xwindow,
+            zwlr_layer_surface_v1::ZwlrLayerSurfaceV1,
-                xdg_surface::{xdg_toplevel::XdgToplevel, XdgSurface},
+            SurfaceBuffer, WlSurface,
-                zwlr_layer_surface_v1::ZwlrLayerSurfaceV1,
-                SurfaceBuffer, WlSurface,
-            },
         },
         rect::Rect,
         renderer::renderer_base::RendererBase,
@@ -16,53 +13,18 @@ use {
         state::State,
         theme::Color,
         tree::{
-            ContainerNode, DisplayNode, FloatNode, OutputNode, OutputNodeId, PlaceholderNode,
+            ContainerNode, DisplayNode, FloatNode, OutputNode, PlaceholderNode, ToplevelData,
-            ToplevelData, ToplevelNodeBase, WorkspaceNode,
+            ToplevelNodeBase, WorkspaceNode,
         },
     },
-    std::{
+    std::{ops::Deref, rc::Rc, slice},
-        fmt::{Debug, Formatter},
-        ops::Deref,
-        rc::Rc,
-        slice,
-    },
 };
 
 pub mod renderer_base;
 
-pub struct RenderResult {
-    pub frame_requests: Vec<Rc<WlCallback>>,
-    pub output_id: OutputNodeId,
-}
-
-impl Default for RenderResult {
-    fn default() -> Self {
-        Self {
-            frame_requests: Default::default(),
-            output_id: OutputNodeId::none(),
-        }
-    }
-}
-
-impl RenderResult {
-    pub fn dispatch_frame_requests(&mut self, now: u64) {
-        for fr in self.frame_requests.drain(..) {
-            fr.send_done(now as _);
-            let _ = fr.client.remove_obj(&*fr);
-        }
-    }
-}
-
-impl Debug for RenderResult {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("RenderResult").finish_non_exhaustive()
-    }
-}
-
 pub struct Renderer<'a> {
     pub base: RendererBase<'a>,
     pub state: &'a State,
-    pub result: Option<&'a mut RenderResult>,
     pub logical_extents: Rect,
     pub pixel_extents: Rect,
 }
@@ -427,13 +389,6 @@ impl Renderer<'_> {
         } else {
             self.render_buffer(surface, &buffer, alpha, x, y, *tpoints, size, bounds);
         }
-        if let Some(result) = self.result.as_deref_mut() {
-            {
-                let mut fr = surface.frame_requests.borrow_mut();
-                result.frame_requests.extend(fr.drain(..));
-            }
-            surface.presented(result.output_id);
-        }
     }
 
     pub fn render_buffer(

src/screenshoter.rs

@@ -80,7 +80,6 @@ pub fn take_screenshot(
         state.root.deref(),
         state,
         Some(state.root.extents.get()),
-        None,
         Scale::from_int(1),
         include_cursor,
         true,

src/state.rs

@@ -63,7 +63,7 @@ use {
         leaks::Tracker,
         logger::Logger,
         rect::Rect,
-        renderer::{RenderResult, Renderer},
+        renderer::Renderer,
         scale::Scale,
         security_context_acceptor::SecurityContextAcceptors,
         theme::{Color, Theme},
@@ -901,20 +901,17 @@ impl State {
         output: &OutputNode,
         fb: &Rc<dyn GfxFramebuffer>,
         tex: &Rc<dyn GfxTexture>,
-        rr: &mut RenderResult,
         render_hw_cursor: bool,
     ) -> Result<Option<SyncFile>, GfxError> {
         let sync_file = fb.render_output(
             output,
             self,
             Some(output.global.pos.get()),
-            Some(rr),
             output.global.persistent.scale.get(),
             render_hw_cursor,
         )?;
         output.latched();
         output.perform_screencopies(tex, !render_hw_cursor, 0, 0, None);
-        rr.dispatch_frame_requests(self.now_msec());
         Ok(sync_file)
     }
 
@@ -933,7 +930,6 @@ impl State {
         let mut renderer = Renderer {
             base: target.renderer_base(&mut ops, Scale::from_int(1), Transform::None),
             state: self,
-            result: None,
             logical_extents: position.at_point(0, 0),
             pixel_extents: {
                 let (width, height) = target.logical_size(Transform::None);
@@ -1183,6 +1179,12 @@ impl State {
             self.ei_acceptor_future.take();
         }
     }
+
+    pub fn vblank(&self, connector: ConnectorId) {
+        if let Some(output) = self.root.outputs.get(&connector) {
+            output.vblank();
+        }
+    }
 }
 
 #[derive(Debug, Error)]

src/tasks/connector.rs

@@ -177,6 +177,7 @@ impl ConnectorHandler {
             title_visible: Default::default(),
             schedule,
             latch_event: Default::default(),
+            vblank_event: Default::default(),
             presentation_event: Default::default(),
         });
         on.update_visible();

src/tree/output.rs

@@ -81,6 +81,7 @@ pub struct OutputNode {
     pub title_visible: Cell<bool>,
     pub schedule: Rc<OutputSchedule>,
     pub latch_event: EventSource<dyn LatchListener>,
+    pub vblank_event: EventSource<dyn VblankListener>,
     pub presentation_event: EventSource<dyn PresentationListener>,
 }
 
@@ -88,6 +89,10 @@ pub trait LatchListener {
     fn after_latch(self: Rc<Self>);
 }
 
+pub trait VblankListener {
+    fn after_vblank(self: Rc<Self>);
+}
+
 pub trait PresentationListener {
     fn presented(
         self: Rc<Self>,
@@ -126,6 +131,12 @@ impl OutputNode {
         }
     }
 
+    pub fn vblank(&self) {
+        for listener in self.vblank_event.iter() {
+            listener.after_vblank();
+        }
+    }
+
     pub fn presented(&self, tv_sec: u64, tv_nsec: u32, refresh: u32, seq: u64, flags: u32) {
         for listener in self.presentation_event.iter() {
             listener.presented(self, tv_sec, tv_nsec, refresh, seq, flags);