Merge pull request #103 from pinnacle-comp/even_better_rendering

Improve rendering

src/backend/udev.rs

@@ -1,16 +1,15 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 
-mod utils;
-
 use std::{
     collections::{HashMap, HashSet},
     ffi::OsString,
     os::fd::FromRawFd,
     path::Path,
-    time::Duration,
+    time::{Duration, Instant},
 };
 
 use anyhow::Context;
+use calloop::Idle;
 use smithay::{
     backend::{
         allocator::{
@@ -28,10 +27,7 @@ use smithay::{
         libinput::{LibinputInputBackend, LibinputSessionInterface},
         renderer::{
             damage::{self},
-            element::{
+            element::{texture::TextureBuffer, RenderElement, RenderElementStates},
-                surface::WaylandSurfaceRenderElement, texture::TextureBuffer, RenderElement,
-                RenderElementStates,
-            },
             gles::{GlesRenderer, GlesTexture},
             multigpu::{gbm::GbmGlesBackend, GpuManager, MultiRenderer, MultiTexture},
             Bind, ExportMem, ImportDma, ImportEgl, ImportMemWl, Offscreen, Renderer,
@@ -53,10 +49,7 @@ use smithay::{
     output::{Output, PhysicalProperties, Subpixel},
     reexports::{
         ash::vk::ExtPhysicalDeviceDrmFn,
-        calloop::{
+        calloop::{EventLoop, LoopHandle, RegistrationToken},
-            timer::{TimeoutAction, Timer},
-            EventLoop, LoopHandle, RegistrationToken,
-        },
         drm::{
             self,
             control::{connector, crtc, ModeTypeFlags},
@@ -88,7 +81,7 @@ use crate::{
         ConnectorSavedState,
     },
     output::OutputName,
-    render::{pointer::PointerElement, take_presentation_feedback, CustomRenderElements},
+    render::{pointer::PointerElement, take_presentation_feedback},
     state::{CalloopData, State, SurfaceDmabufFeedback, WithState},
     window::WindowElement,
 };
@@ -128,6 +121,44 @@ pub struct Udev {
     pointer_images: Vec<(xcursor::parser::Image, TextureBuffer<MultiTexture>)>,
     pointer_element: PointerElement<MultiTexture>,
     pointer_image: crate::cursor::Cursor,
+
+    last_vblank_time: Instant,
+}
+
+impl Backend {
+    fn udev(&self) -> &Udev {
+        let Backend::Udev(udev) = self else { unreachable!() };
+        udev
+    }
+
+    fn udev_mut(&mut self) -> &mut Udev {
+        let Backend::Udev(udev) = self else { unreachable!() };
+        udev
+    }
+}
+
+impl Udev {
+    pub fn schedule_render(&mut self, loop_handle: &LoopHandle<CalloopData>, output: &Output) {
+        let Some(surface) = render_surface_for_output(output, &mut self.backends) else {
+            return;
+        };
+        // tracing::debug!(state = ?surface.render_state, "scheduling render");
+
+        match &surface.render_state {
+            RenderState::Idle => {
+                let output = output.clone();
+                let token = loop_handle.insert_idle(move |data| {
+                    data.state.render_surface(&output);
+                });
+
+                surface.render_state = RenderState::Scheduled(token);
+            }
+            RenderState::Scheduled(_) => (),
+            RenderState::WaitingForVblank { dirty: _ } => {
+                surface.render_state = RenderState::WaitingForVblank { dirty: true }
+            }
+        }
+    }
 }
 
 impl BackendData for Udev {
@@ -193,6 +224,8 @@ pub fn run_udev() -> anyhow::Result<()> {
         pointer_image: crate::cursor::Cursor::load(),
         pointer_images: Vec::new(),
         pointer_element: PointerElement::default(),
+
+        last_vblank_time: Instant::now(),
     };
 
     let display_handle = display.handle();
@@ -217,9 +250,7 @@ pub fn run_udev() -> anyhow::Result<()> {
         }
     }
 
-    let Backend::Udev(udev) = &mut state.backend else {
+    let udev = state.backend.udev_mut();
-        unreachable!()
-    };
 
     event_loop
         .handle()
@@ -272,9 +303,8 @@ pub fn run_udev() -> anyhow::Result<()> {
     event_loop
         .handle()
         .insert_source(notifier, move |event, _, data| {
-            let Backend::Udev(udev) = &mut data.state.backend else {
+            let udev = data.state.backend.udev_mut();
-                unreachable!()
+
-            };
             match event {
                 session::Event::PauseSession => {
                     libinput_context.suspend();
@@ -290,11 +320,7 @@ pub fn run_udev() -> anyhow::Result<()> {
                     if let Err(err) = libinput_context.resume() {
                         tracing::error!("Failed to resume libinput context: {:?}", err);
                     }
-                    for (node, backend) in udev
+                    for backend in udev.backends.values_mut() {
-                        .backends
-                        .iter_mut()
-                        .map(|(handle, backend)| (*handle, backend))
-                    {
                         backend.drm.activate();
                         for surface in backend.surfaces.values_mut() {
                             if let Err(err) = surface.compositor.surface().reset_state() {
@@ -306,9 +332,13 @@ pub fn run_udev() -> anyhow::Result<()> {
                             // otherwise
                             surface.compositor.reset_buffers();
                         }
-                        data.state
+                    }
-                            .loop_handle
+
-                            .insert_idle(move |data| data.state.render(node, None));
+                    for output in data.state.space.outputs().cloned().collect::<Vec<_>>() {
+                        data.state.schedule_render(&output);
+                        // data.state
+                        //     .loop_handle
+                        //     .insert_idle(move |data| data.state.render_surface(&output));
                     }
                 }
             }
@@ -429,6 +459,8 @@ pub fn run_udev() -> anyhow::Result<()> {
             data.display_handle
                 .flush_clients()
                 .expect("failed to flush_clients");
+
+            data.state.focus_state.fix_up_focus(&mut data.state.space);
         },
     )?;
 
@@ -436,7 +468,7 @@ pub fn run_udev() -> anyhow::Result<()> {
 }
 
 struct UdevBackendData {
-    surfaces: HashMap<crtc::Handle, SurfaceData>,
+    surfaces: HashMap<crtc::Handle, RenderSurface>,
     gbm: GbmDevice<DrmDeviceFd>,
     drm: DrmDevice,
     drm_scanner: DrmScanner,
@@ -525,17 +557,47 @@ struct DrmSurfaceDmabufFeedback {
     scanout_feedback: DmabufFeedback,
 }
 
-/// Data associated with a crtc.
+/// The state of a [`RenderSurface`].
-struct SurfaceData {
+#[derive(Debug)]
-    global: Option<GlobalId>,
+enum RenderState {
-    display_handle: DisplayHandle,
+    /// No render is scheduled.
-    device_id: DrmNode,
+    Idle,
-    render_node: DrmNode,
+    /// A render has been queued.
-    compositor: GbmDrmCompositor,
+    Scheduled(
-    dmabuf_feedback: Option<DrmSurfaceDmabufFeedback>,
+        /// The idle token from a render being scheduled.
+        /// This is used to cancel renders if, for example,
+        /// the output being rendered is removed.
+        Idle<'static>,
+    ),
+    /// A frame was rendered and scheduled and we are waiting for vblank.
+    WaitingForVblank {
+        /// A render was scheduled while waiting for vblank.
+        /// In this case, another render will be scheduled once vblank happens.
+        dirty: bool,
+    },
 }
 
-impl Drop for SurfaceData {
+/// Render surface for an output.
+struct RenderSurface {
+    /// The output global id.
+    global: Option<GlobalId>,
+    /// A display handle used to remove the global on drop.
+    display_handle: DisplayHandle,
+    /// The node from `connector_connected`.
+    device_id: DrmNode,
+    /// The node rendering to the screen? idk
+    ///
+    /// If this is equal to the primary gpu node then it does the rendering operations.
+    /// If it's not it is the node the composited buffer ends up on.
+    render_node: DrmNode,
+    /// The thing rendering elements and queueing frames.
+    compositor: GbmDrmCompositor,
+    dmabuf_feedback: Option<DrmSurfaceDmabufFeedback>,
+    render_state: RenderState,
+}
+
+impl Drop for RenderSurface {
+    // Stop advertising this output to clients on drop.
     fn drop(&mut self) {
         if let Some(global) = self.global.take() {
             self.display_handle.remove_global::<State>(global);
@@ -550,13 +612,15 @@ type GbmDrmCompositor = DrmCompositor<
     DrmDeviceFd,
 >;
 
-/// The result of a frame render from `GbmDrmCompositor::render_frame`.
+/// The result of a frame render from `render_frame`.
 struct SurfaceCompositorRenderResult {
     rendered: bool,
     states: RenderElementStates,
 }
 
 /// Render a frame with the given elements.
+///
+/// This frame needs to be queued for scanout afterwards.
 fn render_frame<R, E, Target>(
     compositor: &mut GbmDrmCompositor,
     renderer: &mut R,
@@ -569,6 +633,8 @@ where
     <R as Renderer>::Error: Into<SwapBuffersError>,
     E: RenderElement<R>,
 {
+    use smithay::backend::drm::compositor::RenderFrameError;
+
     compositor
         .render_frame(renderer, elements, clear_color)
         .map(|render_frame_result| {
@@ -581,10 +647,8 @@ where
             }
         })
         .map_err(|err| match err {
-            smithay::backend::drm::compositor::RenderFrameError::PrepareFrame(err) => err.into(),
+            RenderFrameError::PrepareFrame(err) => err.into(),
-            smithay::backend::drm::compositor::RenderFrameError::RenderFrame(
+            RenderFrameError::RenderFrame(damage::Error::Rendering(err)) => err.into(),
-                damage::Error::Rendering(err),
-            ) => err.into(),
             _ => unreachable!(),
         })
 }
@@ -592,9 +656,7 @@ where
 impl State {
     /// A GPU was plugged in.
     fn device_added(&mut self, node: DrmNode, path: &Path) -> Result<(), DeviceAddError> {
-        let Backend::Udev(udev) = &mut self.backend else {
+        let udev = self.backend.udev_mut();
-            unreachable!()
-        };
 
         // Try to open the device
         let fd = udev
@@ -613,17 +675,22 @@ impl State {
 
         let registration_token = self
             .loop_handle
-            .insert_source(
+            .insert_source(notifier, move |event, metadata, data| match event {
-                notifier,
+                DrmEvent::VBlank(crtc) => {
-                move |event, metadata, data: &mut CalloopData| match event {
+                    // { TODO:
-                    DrmEvent::VBlank(crtc) => {
+                    //     let udev = data.state.backend.udev_mut();
-                        data.state.frame_finish(node, crtc, metadata);
+                    //     let then = udev.last_vblank_time;
-                    }
+                    //     let now = Instant::now();
-                    DrmEvent::Error(error) => {
+                    //     let diff = now.duration_since(then);
-                        tracing::error!("{:?}", error);
+                    //     // tracing::debug!(time = diff.as_secs_f64(), "Time since last vblank");
-                    }
+                    //     udev.last_vblank_time = now;
-                },
+                    // }
-            )
+                    data.state.on_vblank(node, crtc, metadata);
+                }
+                DrmEvent::Error(error) => {
+                    tracing::error!("{:?}", error);
+                }
+            })
             .expect("failed to insert drm notifier into event loop");
 
         let render_node = EGLDevice::device_for_display(
@@ -663,9 +730,7 @@ impl State {
         connector: connector::Info,
         crtc: crtc::Handle,
     ) {
-        let Backend::Udev(udev) = &mut self.backend else {
+        let udev = self.backend.udev_mut();
-            unreachable!()
-        };
 
         let device = if let Some(device) = udev.backends.get_mut(&node) {
             device
@@ -845,19 +910,18 @@ impl State {
             &compositor,
         );
 
-        let surface = SurfaceData {
+        let surface = RenderSurface {
             display_handle: udev.display_handle.clone(),
             device_id: node,
             render_node: device.render_node,
             global: Some(global),
             compositor,
             dmabuf_feedback,
+            render_state: RenderState::Idle,
         };
 
         device.surfaces.insert(crtc, surface);
 
-        self.schedule_initial_render(node, crtc, self.loop_handle.clone());
-
         // If there is saved connector state, the connector was previously plugged in.
         // In this case, restore its tags and location.
         // TODO: instead of checking the connector, check the monitor's edid info instead
@@ -911,9 +975,7 @@ impl State {
     ) {
         tracing::debug!(?crtc, "connector_disconnected");
 
-        let Backend::Udev(udev) = &mut self.backend else {
+        let udev = self.backend.udev_mut();
-            unreachable!()
-        };
 
         let device = if let Some(device) = udev.backends.get_mut(&node) {
             device
@@ -948,9 +1010,7 @@ impl State {
     }
 
     fn device_changed(&mut self, node: DrmNode) {
-        let Backend::Udev(udev) = &mut self.backend else {
+        let udev = self.backend.udev_mut();
-            unreachable!()
-        };
 
         let device = if let Some(device) = udev.backends.get_mut(&node) {
             device
@@ -980,11 +1040,9 @@ impl State {
     /// A GPU was unplugged.
     fn device_removed(&mut self, node: DrmNode) {
         let crtcs = {
-            let Backend::Udev(udev) = &mut self.backend else {
+            let udev = self.backend.udev();
-                unreachable!()
-            };
 
-            let Some(device) = udev.backends.get_mut(&node) else {
+            let Some(device) = udev.backends.get(&node) else {
                 return;
             };
 
@@ -1001,9 +1059,7 @@ impl State {
 
         tracing::debug!("Surfaces dropped");
 
-        let Backend::Udev(udev) = &mut self.backend else {
+        let udev = self.backend.udev_mut();
-            unreachable!()
-        };
 
         // drop the backends on this side
         if let Some(backend_data) = udev.backends.remove(&node) {
@@ -1018,15 +1074,13 @@ impl State {
     }
 
     /// Mark [`OutputPresentationFeedback`]s as presented and schedule a new render on idle.
-    fn frame_finish(
+    fn on_vblank(
         &mut self,
         dev_id: DrmNode,
         crtc: crtc::Handle,
         metadata: &mut Option<DrmEventMetadata>,
     ) {
-        let Backend::Udev(udev) = &mut self.backend else {
+        let udev = self.backend.udev_mut();
-            unreachable!()
-        };
 
         let Some(surface) = udev
             .backends
@@ -1047,7 +1101,7 @@ impl State {
             return;
         };
 
-        let schedule_render = match surface
+        match surface
             .compositor
             .frame_submitted()
             .map_err(SwapBuffersError::from)
@@ -1084,83 +1138,43 @@ impl State {
                         flags,
                     );
                 }
-
-                true
             }
             Err(err) => {
                 tracing::warn!("Error during rendering: {:?}", err);
-                match err {
+                if let SwapBuffersError::ContextLost(err) = err {
-                    SwapBuffersError::AlreadySwapped => true,
+                    panic!("Rendering loop lost: {}", err)
-                    // If the device has been deactivated do not reschedule, this will be done
-                    // by session resume
-                    SwapBuffersError::TemporaryFailure(err)
-                        if matches!(
-                            err.downcast_ref::<DrmError>(),
-                            Some(&DrmError::DeviceInactive)
-                        ) =>
-                    {
-                        false
-                    }
-                    SwapBuffersError::TemporaryFailure(err) => matches!(
-                        err.downcast_ref::<DrmError>(),
-                        Some(&DrmError::Access {
-                            source: drm::SystemError::PermissionDenied,
-                            ..
-                        })
-                    ),
-                    SwapBuffersError::ContextLost(err) => panic!("Rendering loop lost: {}", err),
                 }
             }
         };
 
-        if schedule_render {
+        let RenderState::WaitingForVblank { dirty } = surface.render_state else {
-            // Anvil had some stuff here about delaying a render to reduce latency,
+            unreachable!();
-            // but it introduces visible hitching when scrolling, so I'm removing it here.
+        };
-            //
+
-            // If latency is a problem then future me can deal with it :)
+        surface.render_state = RenderState::Idle;
-            self.loop_handle.insert_idle(move |data| {
+
-                data.state.render(dev_id, Some(crtc));
+        if dirty {
-            });
+            self.schedule_render(&output);
+        } else {
+            for window in self.windows.iter() {
+                window.send_frame(&output, self.clock.now(), Some(Duration::ZERO), |_, _| {
+                    Some(output.clone())
+                });
+            }
         }
     }
 
-    /// Render using the gpu on `node` to the provided `crtc`, or all available crtcs if `None`.
+    /// Render to the [`RenderSurface`] associated with the given `output`.
-    fn render(&mut self, node: DrmNode, crtc: Option<crtc::Handle>) {
+    #[tracing::instrument(level = "debug", skip(self), fields(output = output.name()))]
-        let Backend::Udev(udev) = &mut self.backend else {
+    fn render_surface(&mut self, output: &Output) {
-            unreachable!()
+        let udev = self.backend.udev_mut();
-        };
 
-        let device_backend = match udev.backends.get_mut(&node) {
+        let Some(surface) = render_surface_for_output(output, &mut udev.backends) else {
-            Some(backend) => backend,
-            None => {
-                tracing::error!("Trying to render on non-existent backend {}", node);
-                return;
-            }
-        };
-
-        if let Some(crtc) = crtc {
-            self.render_surface(node, crtc);
-        } else {
-            let crtcs: Vec<_> = device_backend.surfaces.keys().copied().collect();
-            for crtc in crtcs {
-                self.render_surface(node, crtc);
-            }
-        };
-    }
-
-    fn render_surface(&mut self, node: DrmNode, crtc: crtc::Handle) {
-        let Backend::Udev(udev) = &mut self.backend else {
-            unreachable!()
-        };
-
-        let Some(surface) = udev
-            .backends
-            .get_mut(&node)
-            .and_then(|device| device.surfaces.get_mut(&crtc))
-        else {
             return;
         };
 
+        assert!(matches!(surface.render_state, RenderState::Scheduled(_)));
+
         // TODO get scale from the rendersurface when supporting HiDPI
         let frame = udev.pointer_image.get_image(
             1, /*scale*/
@@ -1219,17 +1233,6 @@ impl State {
                 texture
             });
 
-        let output = if let Some(output) = self.space.outputs().find(|o| {
-            let udev_op_data = o.user_data().get::<UdevOutputData>();
-            udev_op_data
-                .is_some_and(|data| data.device_id == surface.device_id && data.crtc == crtc)
-        }) {
-            output.clone()
-        } else {
-            // somehow we got called with an invalid output
-            return;
-        };
-
         let windows = self
             .focus_state
             .focus_stack
@@ -1239,137 +1242,65 @@ impl State {
             .collect::<Vec<_>>();
 
         let result = render_surface(
-            &mut self.cursor_status,
+            surface,
+            &mut renderer,
+            output,
             &self.space,
             &windows,
             &self.override_redirect_windows,
             self.dnd_icon.as_ref(),
-            surface,
+            &mut self.cursor_status,
-            &mut renderer,
-            &output,
-            // self.seat.input_method(),
             &pointer_image,
             &mut udev.pointer_element,
             self.pointer_location,
             &self.clock,
         );
 
-        let reschedule = match &result {
+        match result {
-            Ok(has_rendered) => !has_rendered,
+            Ok(true) => surface.render_state = RenderState::WaitingForVblank { dirty: false },
-            Err(err) => {
+            Ok(false) | Err(_) => surface.render_state = RenderState::Idle,
-                tracing::warn!("Error during rendering: {:?}", err);
-                match err {
-                    SwapBuffersError::AlreadySwapped => false,
-                    SwapBuffersError::TemporaryFailure(err) => !matches!(
-                        err.downcast_ref::<DrmError>(),
-                        Some(&DrmError::DeviceInactive)
-                            | Some(&DrmError::Access {
-                                source: drm::SystemError::PermissionDenied,
-                                ..
-                            })
-                    ),
-                    SwapBuffersError::ContextLost(err) => panic!("Rendering loop lost: {}", err),
-                }
-            }
-        };
-
-        if reschedule {
-            let Some(data) = output.user_data().get::<UdevOutputData>() else {
-                unreachable!()
-            };
-
-            // Literally no idea if this refresh time calculation is doing anything, but we're
-            // gonna keep it here because I already added the stuff for it
-            let refresh_time = if let Some(mode) = data.mode {
-                self::utils::refresh_time(mode)
-            } else {
-                let output_refresh = match output.current_mode() {
-                    Some(mode) => mode.refresh,
-                    None => {
-                        return;
-                    }
-                };
-                Duration::from_millis((1_000_000f32 / output_refresh as f32) as u64)
-            };
-
-            // If reschedule is true we either hit a temporary failure or more likely rendering
-            // did not cause any damage on the output. In this case we just re-schedule a repaint
-            // after approx. one frame to re-test for damage.
-            tracing::trace!(
-                "reschedule repaint timer with delay {:?} on {:?}",
-                refresh_time,
-                crtc,
-            );
-            let timer = Timer::from_duration(refresh_time);
-            self.loop_handle
-                .insert_source(timer, move |_, _, data| {
-                    data.state.render(node, Some(crtc));
-                    TimeoutAction::Drop
-                })
-                .expect("failed to schedule frame timer");
-        }
-    }
-
-    fn schedule_initial_render(
-        &mut self,
-        node: DrmNode,
-        crtc: crtc::Handle,
-        evt_handle: LoopHandle<'static, CalloopData>,
-    ) {
-        let Backend::Udev(udev) = &mut self.backend else {
-            unreachable!()
-        };
-
-        let Some(surface) = udev
-            .backends
-            .get_mut(&node)
-            .and_then(|device| device.surfaces.get_mut(&crtc))
-        else {
-            return;
-        };
-
-        let node = surface.render_node;
-        let result = {
-            let mut renderer = udev
-                .gpu_manager
-                .single_renderer(&node)
-                .expect("failed to create MultiRenderer");
-            initial_render(surface, &mut renderer)
-        };
-
-        if let Err(err) = result {
-            match err {
-                SwapBuffersError::AlreadySwapped => {}
-                SwapBuffersError::TemporaryFailure(err) => {
-                    // TODO dont reschedule after 3(?) retries
-                    tracing::warn!("Failed to submit page_flip: {}", err);
-                    let handle = evt_handle.clone();
-                    evt_handle.insert_idle(move |data| {
-                        data.state.schedule_initial_render(node, crtc, handle)
-                    });
-                }
-                SwapBuffersError::ContextLost(err) => panic!("Rendering loop lost: {}", err),
-            }
         }
     }
 }
 
+fn render_surface_for_output<'a>(
+    output: &Output,
+    backends: &'a mut HashMap<DrmNode, UdevBackendData>,
+) -> Option<&'a mut RenderSurface> {
+    let UdevOutputData {
+        device_id,
+        crtc,
+        mode: _,
+    } = output.user_data().get()?;
+
+    backends
+        .get_mut(device_id)
+        .and_then(|device| device.surfaces.get_mut(crtc))
+}
+
+/// Render windows, layers, and everything else needed to the given [`RenderSurface`].
+/// Also queues the frame for scanout.
 #[allow(clippy::too_many_arguments)]
-fn render_surface<'a>(
+fn render_surface(
-    cursor_status: &mut CursorImageStatus,
+    surface: &mut RenderSurface,
+    renderer: &mut UdevRenderer<'_, '_>,
+    output: &Output,
+
     space: &Space<WindowElement>,
     windows: &[WindowElement],
     override_redirect_windows: &[X11Surface],
+
     dnd_icon: Option<&WlSurface>,
-    surface: &'a mut SurfaceData,
+    cursor_status: &mut CursorImageStatus,
-    renderer: &mut UdevRenderer<'a, '_>,
+
-    output: &Output,
-    // input_method: &InputMethodHandle,
     pointer_image: &TextureBuffer<MultiTexture>,
     pointer_element: &mut PointerElement<MultiTexture>,
     pointer_location: Point<f64, Logical>,
+
     clock: &Clock<Monotonic>,
 ) -> Result<bool, SwapBuffersError> {
+    use smithay::reexports::wayland_protocols::xdg::shell::server::xdg_toplevel;
+
     let pending_wins = windows
         .iter()
         .filter(|win| win.alive())
@@ -1383,6 +1314,16 @@ fn render_surface<'a>(
             };
             pending_size || win.with_state(|state| !state.loc_request_state.is_idle())
         })
+        .filter(|win| {
+            if let WindowElement::Wayland(win) = win {
+                !win.toplevel()
+                    .current_state()
+                    .states
+                    .contains(xdg_toplevel::State::Resizing)
+            } else {
+                true
+            }
+        })
         .map(|win| {
             (
                 win.class().unwrap_or("None".to_string()),
@@ -1405,7 +1346,10 @@ fn render_surface<'a>(
             .queue_frame(None)
             .map_err(Into::<SwapBuffersError>::into)?;
 
+        tracing::debug!("queued no frame");
+
         // TODO: still draw the cursor here
+        surface.render_state = RenderState::WaitingForVblank { dirty: false };
 
         return Ok(true);
     }
@@ -1433,7 +1377,6 @@ fn render_surface<'a>(
 
     let time = clock.now();
 
-    // Send frames to the cursor surface to get it to update correctly
     if let CursorImageStatus::Surface(surf) = cursor_status {
         send_frames_surface_tree(surf, output, time, Some(Duration::ZERO), |_, _| None);
     }
@@ -1462,19 +1405,3 @@ fn render_surface<'a>(
 
     Ok(res.rendered)
 }
-
-fn initial_render(
-    surface: &mut SurfaceData,
-    renderer: &mut UdevRenderer<'_, '_>,
-) -> Result<(), SwapBuffersError> {
-    render_frame::<_, CustomRenderElements<_, WaylandSurfaceRenderElement<_>>, GlesTexture>(
-        &mut surface.compositor,
-        renderer,
-        &[],
-        [0.6, 0.6, 0.6, 1.0],
-    )?;
-    surface.compositor.queue_frame(None)?;
-    surface.compositor.reset_buffers();
-
-    Ok(())
-}

src/backend/udev/utils.rs

@@ -1,31 +0,0 @@
-// SPDX-License-Identifier: GPL-3.0-or-later
-
-use std::time::Duration;
-
-use smithay::reexports::drm::control::{Mode, ModeFlags};
-
-// From niri:
-// https://github.com/YaLTeR/niri/blob/ba0a6d6b8868cc6348ad1b20f683a95d5909df6b/src/backend/tty.rs#L900-L922
-pub fn refresh_time(mode: Mode) -> Duration {
-    let clock = mode.clock() as u64;
-    let htotal = mode.hsync().2 as u64;
-    let vtotal = mode.vsync().2 as u64;
-
-    let mut numerator = htotal * vtotal * 1_000_000;
-    let mut denominator = clock;
-
-    if mode.flags().contains(ModeFlags::INTERLACE) {
-        denominator *= 2;
-    }
-
-    if mode.flags().contains(ModeFlags::DBLSCAN) {
-        numerator *= 2;
-    }
-
-    if mode.vscan() > 1 {
-        numerator *= mode.vscan() as u64;
-    }
-
-    let refresh_interval = (numerator + denominator / 2) / denominator;
-    Duration::from_nanos(refresh_interval)
-}

src/backend/winit.rs

@@ -23,7 +23,10 @@ use smithay::{
             timer::{TimeoutAction, Timer},
             EventLoop,
         },
-        wayland_protocols::wp::presentation_time::server::wp_presentation_feedback,
+        wayland_protocols::{
+            wp::presentation_time::server::wp_presentation_feedback,
+            xdg::shell::server::xdg_toplevel,
+        },
         wayland_server::{protocol::wl_surface::WlSurface, Display},
     },
     utils::{IsAlive, Transform},
@@ -57,6 +60,13 @@ impl BackendData for Winit {
     fn early_import(&mut self, _surface: &WlSurface) {}
 }
 
+impl Backend {
+    fn winit_mut(&mut self) -> &mut Winit {
+        let Backend::Winit(winit) = self else { unreachable!() };
+        winit
+    }
+}
+
 /// Start Pinnacle as a window in a graphical environment.
 pub fn run_winit() -> anyhow::Result<()> {
     let mut event_loop: EventLoop<CalloopData> = EventLoop::try_new()?;
@@ -163,13 +173,13 @@ pub fn run_winit() -> anyhow::Result<()> {
 
     state.focus_state.focused_output = Some(output.clone());
 
-    let Backend::Winit(backend) = &mut state.backend else {
+    let winit = state.backend.winit_mut();
-        unreachable!()
+
-    };
+    winit.backend.window().set_title("Pinnacle");
 
     state
         .shm_state
-        .update_formats(backend.backend.renderer().shm_formats());
+        .update_formats(winit.backend.renderer().shm_formats());
 
     state.space.map_output(&output, (0, 0));
 
@@ -183,13 +193,10 @@ pub fn run_winit() -> anyhow::Result<()> {
         tracing::error!("Failed to start XWayland: {err}");
     }
 
-    let mut pointer_element = PointerElement::<GlesTexture>::new();
-
     let insert_ret =
         state
             .loop_handle
             .insert_source(Timer::immediate(), move |_instant, _metadata, data| {
-                let display_handle = &mut data.display_handle;
                 let state = &mut data.state;
 
                 let result = winit_evt_loop.dispatch_new_events(|event| match event {
@@ -214,7 +221,9 @@ pub fn run_winit() -> anyhow::Result<()> {
                     WinitEvent::Input(input_evt) => {
                         state.process_input_event(input_evt);
                     }
-                    WinitEvent::Refresh => {}
+                    WinitEvent::Refresh => {
+                        state.render_window(&output);
+                    }
                 });
 
                 match result {
@@ -224,172 +233,10 @@ pub fn run_winit() -> anyhow::Result<()> {
                     }
                 };
 
-                if let CursorImageStatus::Surface(surface) = &state.cursor_status {
+                state.render_window(&output);
-                    if !surface.alive() {
-                        state.cursor_status = CursorImageStatus::default_named();
-                    }
-                }
-
-                let cursor_visible = !matches!(state.cursor_status, CursorImageStatus::Surface(_));
-
-                pointer_element.set_status(state.cursor_status.clone());
-
-                let pending_wins = state
-                    .windows
-                    .iter()
-                    .filter(|win| win.alive())
-                    .filter(|win| {
-                        let pending_size = if let WindowElement::Wayland(win) = win {
-                            let current_state = win.toplevel().current_state();
-                            win.toplevel()
-                                .with_pending_state(|state| state.size != current_state.size)
-                        } else {
-                            false
-                        };
-                        pending_size || win.with_state(|state| !state.loc_request_state.is_idle())
-                    })
-                    .map(|win| {
-                        (
-                            win.class().unwrap_or("None".to_string()),
-                            win.title().unwrap_or("None".to_string()),
-                            win.with_state(|state| state.loc_request_state.clone()),
-                        )
-                    })
-                    .collect::<Vec<_>>();
-
-                if !pending_wins.is_empty() {
-                    // tracing::debug!("Skipping frame, waiting on {pending_wins:?}");
-                    let op_clone = output.clone();
-                    state.loop_handle.insert_idle(move |dt| {
-                        for win in dt.state.windows.iter() {
-                            win.send_frame(
-                                &op_clone,
-                                dt.state.clock.now(),
-                                Some(Duration::ZERO),
-                                surface_primary_scanout_output,
-                            );
-                        }
-                    });
-
-                    state.space.refresh();
-                    state.popup_manager.cleanup();
-                    display_handle
-                        .flush_clients()
-                        .expect("failed to flush client buffers");
-
-                    // TODO: still draw the cursor here
-
-                    return TimeoutAction::ToDuration(Duration::from_millis(1));
-                }
-
-                let Backend::Winit(backend) = &mut state.backend else {
-                    unreachable!()
-                };
-                let full_redraw = &mut backend.full_redraw;
-                *full_redraw = full_redraw.saturating_sub(1);
-
-                state.focus_state.fix_up_focus(&mut state.space);
-
-                let output_render_elements = crate::render::generate_render_elements(
-                    &output,
-                    backend.backend.renderer(),
-                    &state.space,
-                    &state.focus_state.focus_stack,
-                    &state.override_redirect_windows,
-                    state.pointer_location,
-                    &mut state.cursor_status,
-                    state.dnd_icon.as_ref(),
-                    // state.seat.input_method(),
-                    &mut pointer_element,
-                    None,
-                );
-
-                let render_res = backend.backend.bind().and_then(|_| {
-                    let age = if *full_redraw > 0 {
-                        0
-                    } else {
-                        backend.backend.buffer_age().unwrap_or(0)
-                    };
-
-                    let renderer = backend.backend.renderer();
-
-                    backend
-                        .damage_tracker
-                        .render_output(renderer, age, &output_render_elements, [0.6, 0.6, 0.6, 1.0])
-                        .map_err(|err| match err {
-                            damage::Error::Rendering(err) => err.into(),
-                            damage::Error::OutputNoMode(_) => todo!(),
-                        })
-                });
-
-                match render_res {
-                    Ok(render_output_result) => {
-                        let has_rendered = render_output_result.damage.is_some();
-                        if let Some(damage) = render_output_result.damage {
-                            // tracing::debug!("damage rects are {damage:?}");
-                            if let Err(err) = backend.backend.submit(Some(&damage)) {
-                                tracing::warn!("{}", err);
-                            }
-                        }
-
-                        backend.backend.window().set_cursor_visible(cursor_visible);
-
-                        let time = state.clock.now();
-
-                        // Send frames to the cursor surface so it updates correctly
-                        if let CursorImageStatus::Surface(surf) = &state.cursor_status {
-                            if let Some(op) = state.focus_state.focused_output.as_ref() {
-                                send_frames_surface_tree(
-                                    surf,
-                                    op,
-                                    time,
-                                    Some(Duration::ZERO),
-                                    |_, _| None,
-                                );
-                            }
-                        }
-
-                        super::post_repaint(
-                            &output,
-                            &render_output_result.states,
-                            &state.space,
-                            None,
-                            time.into(),
-                        );
-
-                        if has_rendered {
-                            let mut output_presentation_feedback = take_presentation_feedback(
-                                &output,
-                                &state.space,
-                                &render_output_result.states,
-                            );
-                            output_presentation_feedback.presented(
-                                time,
-                                output
-                                    .current_mode()
-                                    .map(|mode| {
-                                        Duration::from_secs_f64(1000f64 / mode.refresh as f64)
-                                    })
-                                    .unwrap_or_default(),
-                                0,
-                                wp_presentation_feedback::Kind::Vsync,
-                            );
-                        }
-                    }
-                    Err(err) => {
-                        tracing::warn!("{}", err);
-                    }
-                }
-
-                state.space.refresh();
-                state.popup_manager.cleanup();
-                display_handle
-                    .flush_clients()
-                    .expect("failed to flush client buffers");
 
                 TimeoutAction::ToDuration(Duration::from_micros(((1.0 / 144.0) * 1000000.0) as u64))
             });
-
     if let Err(err) = insert_ret {
         anyhow::bail!("Failed to insert winit events into event loop: {err}");
     }
@@ -400,10 +247,170 @@ pub fn run_winit() -> anyhow::Result<()> {
             display_handle,
             state,
         },
-        |_data| {
+        |data| {
-            // println!("{}", _data.state.space.elements().count());
+            data.state.space.refresh();
+            data.state.popup_manager.cleanup();
+            data.display_handle
+                .flush_clients()
+                .expect("failed to flush client buffers");
         },
     )?;
 
     Ok(())
 }
+
+impl State {
+    fn render_window(&mut self, output: &Output) {
+        let winit = self.backend.winit_mut();
+
+        let pending_wins = self
+            .windows
+            .iter()
+            .filter(|win| win.alive())
+            .filter(|win| {
+                let pending_size = if let WindowElement::Wayland(win) = win {
+                    let current_state = win.toplevel().current_state();
+                    win.toplevel()
+                        .with_pending_state(|state| state.size != current_state.size)
+                } else {
+                    false
+                };
+                pending_size || win.with_state(|state| !state.loc_request_state.is_idle())
+            })
+            .filter(|win| {
+                if let WindowElement::Wayland(win) = win {
+                    !win.toplevel()
+                        .current_state()
+                        .states
+                        .contains(xdg_toplevel::State::Resizing)
+                } else {
+                    true
+                }
+            })
+            .map(|win| {
+                (
+                    win.class().unwrap_or("None".to_string()),
+                    win.title().unwrap_or("None".to_string()),
+                    win.with_state(|state| state.loc_request_state.clone()),
+                )
+            })
+            .collect::<Vec<_>>();
+
+        if !pending_wins.is_empty() {
+            // tracing::debug!("Skipping frame, waiting on {pending_wins:?}");
+            let op_clone = output.clone();
+            self.loop_handle.insert_idle(move |dt| {
+                for win in dt.state.windows.iter() {
+                    win.send_frame(
+                        &op_clone,
+                        dt.state.clock.now(),
+                        Some(Duration::ZERO),
+                        surface_primary_scanout_output,
+                    );
+                }
+            });
+
+            // TODO: still draw the cursor here
+
+            return;
+        }
+        let full_redraw = &mut winit.full_redraw;
+        *full_redraw = full_redraw.saturating_sub(1);
+
+        self.focus_state.fix_up_focus(&mut self.space);
+
+        if let CursorImageStatus::Surface(surface) = &self.cursor_status {
+            if !surface.alive() {
+                self.cursor_status = CursorImageStatus::default_named();
+            }
+        }
+
+        let cursor_visible = !matches!(self.cursor_status, CursorImageStatus::Surface(_));
+
+        let mut pointer_element = PointerElement::<GlesTexture>::new();
+        pointer_element.set_status(self.cursor_status.clone());
+
+        let output_render_elements = crate::render::generate_render_elements(
+            output,
+            winit.backend.renderer(),
+            &self.space,
+            &self.focus_state.focus_stack,
+            &self.override_redirect_windows,
+            self.pointer_location,
+            &mut self.cursor_status,
+            self.dnd_icon.as_ref(),
+            // self.seat.input_method(),
+            &mut pointer_element,
+            None,
+        );
+
+        let render_res = winit.backend.bind().and_then(|_| {
+            let age = if *full_redraw > 0 {
+                0
+            } else {
+                winit.backend.buffer_age().unwrap_or(0)
+            };
+
+            let renderer = winit.backend.renderer();
+
+            winit
+                .damage_tracker
+                .render_output(renderer, age, &output_render_elements, [0.6, 0.6, 0.6, 1.0])
+                .map_err(|err| match err {
+                    damage::Error::Rendering(err) => err.into(),
+                    damage::Error::OutputNoMode(_) => todo!(),
+                })
+        });
+
+        match render_res {
+            Ok(render_output_result) => {
+                let has_rendered = render_output_result.damage.is_some();
+                if let Some(damage) = render_output_result.damage {
+                    // tracing::debug!("damage rects are {damage:?}");
+                    if let Err(err) = winit.backend.submit(Some(&damage)) {
+                        tracing::warn!("{}", err);
+                    }
+                }
+
+                winit.backend.window().set_cursor_visible(cursor_visible);
+
+                let time = self.clock.now();
+
+                // Send frames to the cursor surface so it updates correctly
+                if let CursorImageStatus::Surface(surf) = &self.cursor_status {
+                    if let Some(op) = self.focus_state.focused_output.as_ref() {
+                        send_frames_surface_tree(surf, op, time, Some(Duration::ZERO), |_, _| None);
+                    }
+                }
+
+                super::post_repaint(
+                    output,
+                    &render_output_result.states,
+                    &self.space,
+                    None,
+                    time.into(),
+                );
+
+                if has_rendered {
+                    let mut output_presentation_feedback = take_presentation_feedback(
+                        output,
+                        &self.space,
+                        &render_output_result.states,
+                    );
+                    output_presentation_feedback.presented(
+                        time,
+                        output
+                            .current_mode()
+                            .map(|mode| Duration::from_secs_f64(1000f64 / mode.refresh as f64))
+                            .unwrap_or_default(),
+                        0,
+                        wp_presentation_feedback::Kind::Vsync,
+                    );
+                }
+            }
+            Err(err) => {
+                tracing::warn!("{}", err);
+            }
+        }
+    }
+}

src/handlers.rs

@@ -10,7 +10,7 @@ use smithay::{
     delegate_compositor, delegate_data_device, delegate_fractional_scale, delegate_layer_shell,
     delegate_output, delegate_presentation, delegate_primary_selection, delegate_relative_pointer,
     delegate_seat, delegate_shm, delegate_viewporter,
-    desktop::{self, layer_map_for_output, PopupKind, WindowSurfaceType},
+    desktop::{self, find_popup_root_surface, layer_map_for_output, PopupKind, WindowSurfaceType},
     input::{pointer::CursorImageStatus, Seat, SeatHandler, SeatState},
     output::Output,
     reexports::{
@@ -105,11 +105,12 @@ impl CompositorHandler for State {
         utils::on_commit_buffer_handler::<Self>(surface);
         self.backend.early_import(surface);
 
+        let mut root = surface.clone();
+        while let Some(parent) = compositor::get_parent(&root) {
+            root = parent;
+        }
+
         if !compositor::is_sync_subsurface(surface) {
-            let mut root = surface.clone();
-            while let Some(parent) = compositor::get_parent(&root) {
-                root = parent;
-            }
             if let Some(win @ WindowElement::Wayland(window)) = &self.window_for_surface(&root) {
                 // tracing::debug!("window commit thing {:?}", win.class());
                 window.on_commit();
@@ -129,9 +130,41 @@ impl CompositorHandler for State {
 
         crate::grab::resize_grab::handle_commit(self, surface);
 
-        // if let Some(window) = self.window_for_surface(surface) {
+        // `surface` is a root window
-        //     tracing::debug!("commit on window {:?}", window.class());
+        let Some(output) = self
-        // }
+            .window_for_surface(surface)
+            .and_then(|win| win.output(self))
+            .or_else(|| {
+                // `surface` is a descendant of a root window
+                self.window_for_surface(&root)
+                    .and_then(|win| win.output(self))
+            })
+            .or_else(|| {
+                // `surface` is a popup
+                self.popup_manager
+                    .find_popup(surface)
+                    .and_then(|popup| find_popup_root_surface(&popup).ok())
+                    .and_then(|surf| self.window_for_surface(&surf))
+                    .and_then(|win| win.output(self))
+            })
+            .or_else(|| {
+                // `surface` is a layer surface
+                self.space
+                    .outputs()
+                    .find(|op| {
+                        let layer_map = layer_map_for_output(op);
+                        layer_map
+                            .layer_for_surface(surface, WindowSurfaceType::ALL)
+                            .is_some()
+                    })
+                    .cloned()
+            })
+        // TODO: cursor surface and dnd icon
+        else {
+            return;
+        };
+
+        self.schedule_render(&output);
     }
 
     fn client_compositor_state<'a>(&self, client: &'a Client) -> &'a CompositorClientState {

src/handlers/xdg_shell.rs

@@ -1,5 +1,3 @@
-use std::time::Duration;
-
 use smithay::{
     delegate_xdg_shell,
     desktop::{
@@ -150,6 +148,8 @@ impl XdgShellHandler for State {
                 .get_keyboard()
                 .expect("Seat had no keyboard")
                 .set_focus(self, focus, SERIAL_COUNTER.next_serial());
+
+            self.schedule_render(&output);
         }
     }
 
@@ -749,14 +749,6 @@ impl XdgShellHandler for State {
                                 state.loc_request_state =
                                     LocationRequestState::Acknowledged(new_loc);
                             });
-                            if let Some(op) = window.output(self) {
-                                window.send_frame(
-                                    &op,
-                                    self.clock.now(),
-                                    Some(Duration::ZERO),
-                                    |_, _| Some(op.clone()),
-                                );
-                            }
                         }
                     }
                     Configure::Popup(_) => todo!(),

src/handlers/xwayland.rs

@@ -218,6 +218,8 @@ impl XwmHandler for CalloopData {
                     .get_keyboard()
                     .expect("Seat had no keyboard")
                     .set_focus(&mut self.state, focus, SERIAL_COUNTER.next_serial());
+
+                self.state.schedule_render(&output);
             }
         }
         if !window.is_override_redirect() {
@@ -267,6 +269,8 @@ impl XwmHandler for CalloopData {
                     .get_keyboard()
                     .expect("Seat had no keyboard")
                     .set_focus(&mut self.state, focus, SERIAL_COUNTER.next_serial());
+
+                self.state.schedule_render(&output);
             }
         }
         tracing::debug!("destroyed x11 window");
@@ -313,7 +317,6 @@ impl XwmHandler for CalloopData {
         };
         tracing::debug!("configure notify with geo: {geometry:?}");
         self.state.space.map_element(win, geometry.loc, true);
-        // TODO: anvil has a TODO here
     }
 
     fn maximize_request(&mut self, _xwm: XwmId, window: X11Surface) {

src/input.rs

@@ -657,8 +657,8 @@ impl State {
         let surface_under = self.surface_under(self.pointer_location);
 
         // tracing::info!("{:?}", self.pointer_location);
-        if let Some(ptr) = self.seat.get_pointer() {
+        if let Some(pointer) = self.seat.get_pointer() {
-            ptr.motion(
+            pointer.motion(
                 self,
                 surface_under.clone(),
                 &MotionEvent {
@@ -668,7 +668,7 @@ impl State {
                 },
             );
 
-            ptr.relative_motion(
+            pointer.relative_motion(
                 self,
                 surface_under,
                 &RelativeMotionEvent {
@@ -678,7 +678,15 @@ impl State {
                 },
             );
 
-            ptr.frame(self);
+            pointer.frame(self);
+
+            self.schedule_render(
+                &self
+                    .focus_state
+                    .focused_output
+                    .clone()
+                    .expect("no focused output"),
+            );
         }
     }
 }

src/render.rs

@@ -31,7 +31,11 @@ use smithay::{
     xwayland::X11Surface,
 };
 
-use crate::{state::WithState, window::WindowElement};
+use crate::{
+    backend::Backend,
+    state::{State, WithState},
+    window::WindowElement,
+};
 
 use self::pointer::{PointerElement, PointerRenderElement};
 
@@ -400,3 +404,12 @@ pub fn take_presentation_feedback(
 
     output_presentation_feedback
 }
+
+impl State {
+    pub fn schedule_render(&mut self, output: &Output) {
+        // I'm relegating winit to render every frame because it's not my priority right now
+        if let Backend::Udev(udev) = &mut self.backend {
+            udev.schedule_render(&self.loop_handle, output);
+        }
+    }
+}

src/render/pointer.rs

@@ -10,7 +10,7 @@ use smithay::{
         },
         ImportAll, Renderer, Texture,
     },
-    input::pointer::{CursorIcon, CursorImageStatus},
+    input::pointer::CursorImageStatus,
     render_elements,
     utils::{Physical, Point, Scale},
 };

src/state/api_handlers.rs

@@ -113,6 +113,7 @@ impl State {
                 window.change_geometry(Rectangle::from_loc_and_size(window_loc, window_size));
                 if let Some(output) = window.output(self) {
                     self.update_windows(&output);
+                    self.schedule_render(&output);
                 }
             }
             Msg::MoveWindowToTag { window_id, tag_id } => {
@@ -123,7 +124,7 @@ impl State {
                 });
                 let Some(output) = tag.output(self) else { return };
                 self.update_windows(&output);
-                // self.re_layout(&output);
+                self.schedule_render(&output);
             }
             Msg::ToggleTagOnWindow { window_id, tag_id } => {
                 let Some(window) = window_id.window(self) else { return };
@@ -139,7 +140,7 @@ impl State {
 
                 let Some(output) = tag.output(self) else { return };
                 self.update_windows(&output);
-                // self.re_layout(&output);
+                self.schedule_render(&output);
             }
             Msg::ToggleFloating { window_id } => {
                 let Some(window) = window_id.window(self) else { return };
@@ -147,6 +148,12 @@ impl State {
 
                 let Some(output) = window.output(self) else { return };
                 self.update_windows(&output);
+
+                // Sometimes toggling won't change the window size,
+                // causing no commit.
+                //
+                // Schedule a render in case the window moves.
+                self.schedule_render(&output);
             }
             Msg::ToggleFullscreen { window_id } => {
                 let Some(window) = window_id.window(self) else { return };
@@ -154,6 +161,7 @@ impl State {
 
                 let Some(output) = window.output(self) else { return };
                 self.update_windows(&output);
+                self.schedule_render(&output);
             }
             Msg::ToggleMaximized { window_id } => {
                 let Some(window) = window_id.window(self) else { return };
@@ -161,6 +169,7 @@ impl State {
 
                 let Some(output) = window.output(self) else { return };
                 self.update_windows(&output);
+                self.schedule_render(&output);
             }
             Msg::AddWindowRule { cond, rule } => {
                 self.config.window_rules.push((cond, rule));
@@ -251,6 +260,7 @@ impl State {
                     if let Some(output) = tag.output(self) {
                         self.update_windows(&output);
                         self.update_focus(&output);
+                        self.schedule_render(&output);
                     }
                 }
             }
@@ -265,6 +275,7 @@ impl State {
                 });
                 self.update_windows(&output);
                 self.update_focus(&output);
+                self.schedule_render(&output);
             }
             Msg::AddTags {
                 output_name,