Merge pull request #95 from pinnacle-comp/render

Flickering mitigations

Cargo.toml

@@ -14,7 +14,7 @@ tracing = { git = "https://github.com/tokio-rs/tracing", rev = "b8c45cc" }
 tracing-subscriber = { git = "https://github.com/tokio-rs/tracing", rev = "b8c45cc", features = ["env-filter"] }
 tracing-appender = { git = "https://github.com/tokio-rs/tracing", rev = "b8c45cc" }
 smithay = { git = "https://github.com/Smithay/smithay", rev = "1a61e1c", features = ["desktop", "wayland_frontend"] }
-smithay-drm-extras = { git = "https://github.com/Smithay/smithay", optional = true }
+smithay-drm-extras = { git = "https://github.com/Smithay/smithay", rev = "1a61e1c", optional = true }
 thiserror = "1.0.48"
 xcursor = { version = "0.3.4", optional = true }
 image = { version = "0.24.7", default-features = false, optional = true }

src/backend/udev.rs

@@ -29,7 +29,10 @@ use smithay::{
         libinput::{LibinputInputBackend, LibinputSessionInterface},
         renderer::{
             damage::{self, OutputDamageTracker},
-            element::{texture::TextureBuffer, RenderElement, RenderElementStates},
+            element::{
+                surface::WaylandSurfaceRenderElement, texture::TextureBuffer, RenderElement,
+                RenderElementStates,
+            },
             gles::{GlesRenderer, GlesTexture},
             multigpu::{gbm::GbmGlesBackend, GpuManager, MultiRenderer, MultiTexture},
             sync::SyncPoint,
@@ -422,7 +425,7 @@ pub fn run_udev() -> anyhow::Result<()> {
     }
 
     event_loop.run(
-        Some(Duration::from_millis(1)),
+        Some(Duration::from_micros(((1.0 / 144.0) * 1000000.0) as u64)),
         &mut CalloopData { state, display },
         |data| {
             data.state.space.refresh();
@@ -756,6 +759,8 @@ impl State {
         Ok(())
     }
 
+    /// A display was plugged in.
+    // TODO: better edid info from cosmic-comp
     fn connector_connected(
         &mut self,
         node: DrmNode,
@@ -969,6 +974,7 @@ impl State {
         }
     }
 
+    /// A display was unplugged.
     fn connector_disconnected(
         &mut self,
         node: DrmNode,
@@ -1036,6 +1042,7 @@ impl State {
         // crate::shell::fixup_positions(&mut self.space);
     }
 
+    /// A GPU was unplugged.
     fn device_removed(&mut self, node: DrmNode) {
         let crtcs = {
             let Backend::Udev(backend) = &mut self.backend else {
@@ -1120,7 +1127,7 @@ impl State {
         let schedule_render = match surface
             .compositor
             .frame_submitted()
-            .map_err(Into::<SwapBuffersError>::into)
+            .map_err(SwapBuffersError::from)
         {
             Ok(user_data) => {
                 if let Some(mut feedback) = user_data {
@@ -1243,7 +1250,7 @@ impl State {
         }
     }
 
-    // If crtc is `Some()`, render it, else render all crtcs
+    /// Render using the gpu on `node` to the provided `crtc`, or all available crtcs if `None`.
     fn render(&mut self, node: DrmNode, crtc: Option<crtc::Handle>) {
         let Backend::Udev(backend) = &mut self.backend else {
             unreachable!()
@@ -1480,7 +1487,16 @@ fn render_surface<'a>(
     let pending_wins = windows
         .iter()
         .filter(|win| win.alive())
-        .filter(|win| win.with_state(|state| !state.loc_request_state.is_idle()))
+        .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()),
@@ -1491,13 +1507,14 @@ fn render_surface<'a>(
         .collect::<Vec<_>>();
 
     if !pending_wins.is_empty() {
-        // tracing::debug!("Skipping frame, waiting on {pending_wins:?}");
+        tracing::debug!("Skipping frame, waiting on {pending_wins:?}");
         for win in windows.iter() {
             win.send_frame(output, clock.now(), Some(Duration::ZERO), |_, _| {
                 Some(output.clone())
             });
         }
 
+        // TODO: set waiting for vblank to true here
         surface
             .compositor
             .queue_frame(None, None, None)
@@ -1509,14 +1526,14 @@ fn render_surface<'a>(
     }
 
     let output_render_elements = crate::render::generate_render_elements(
+        output,
+        renderer,
         space,
         windows,
         override_redirect_windows,
         pointer_location,
         cursor_status,
         dnd_icon,
-        renderer,
-        output,
         input_method,
         pointer_element,
         Some(pointer_image),
@@ -1552,10 +1569,11 @@ fn render_surface<'a>(
 
     if res.rendered {
         let output_presentation_feedback = take_presentation_feedback(output, space, &res.states);
+        // TODO: set waiting for vblank to true here
         surface
             .compositor
             .queue_frame(res.sync, res.damage, Some(output_presentation_feedback))
-            .map_err(Into::<SwapBuffersError>::into)?;
+            .map_err(SwapBuffersError::from)?;
     }
 
     Ok(res.rendered)
@@ -1567,7 +1585,7 @@ fn initial_render(
 ) -> Result<(), SwapBuffersError> {
     surface
         .compositor
-        .render_frame::<_, CustomRenderElements<_>, GlesTexture>(
+        .render_frame::<_, CustomRenderElements<_, WaylandSurfaceRenderElement<_>>, GlesTexture>(
             renderer,
             &[],
             [0.6, 0.6, 0.6, 1.0],

src/backend/winit.rs

@@ -36,6 +36,7 @@ use smithay::{
 use crate::{
     render::{pointer::PointerElement, take_presentation_feedback},
     state::{Backend, CalloopData, State, WithState},
+    window::WindowElement,
 };
 
 use super::BackendData;
@@ -241,7 +242,16 @@ pub fn run_winit() -> anyhow::Result<()> {
                     .windows
                     .iter()
                     .filter(|win| win.alive())
-                    .filter(|win| win.with_state(|state| !state.loc_request_state.is_idle()))
+                    .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()),
@@ -253,14 +263,17 @@ pub fn run_winit() -> anyhow::Result<()> {
 
                 if !pending_wins.is_empty() {
                     // tracing::debug!("Skipping frame, waiting on {pending_wins:?}");
-                    for win in state.windows.iter() {
+                    let op_clone = output.clone();
-                        win.send_frame(
+                    state.loop_handle.insert_idle(move |dt| {
-                            &output,
+                        for win in dt.state.windows.iter() {
-                            state.clock.now(),
+                            win.send_frame(
-                            Some(Duration::ZERO),
+                                &op_clone,
-                            surface_primary_scanout_output,
+                                dt.state.clock.now(),
-                        );
+                                Some(Duration::ZERO),
-                    }
+                                surface_primary_scanout_output,
+                            );
+                        }
+                    });
 
                     state.space.refresh();
                     state.popup_manager.cleanup();
@@ -282,14 +295,14 @@ pub fn run_winit() -> anyhow::Result<()> {
                 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(),
-                    backend.backend.renderer(),
-                    &output,
                     state.seat.input_method(),
                     &mut pointer_element,
                     None,
@@ -306,7 +319,7 @@ pub fn run_winit() -> anyhow::Result<()> {
 
                     backend
                         .damage_tracker
-                        .render_output(renderer, age, &output_render_elements, [0.5, 0.5, 0.5, 1.0])
+                        .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!(),
@@ -327,7 +340,7 @@ pub fn run_winit() -> anyhow::Result<()> {
 
                         let time = state.clock.now();
 
-                        // Send frames to the cursor surface so it updates in xwayland
+                        // 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(
@@ -378,7 +391,7 @@ pub fn run_winit() -> anyhow::Result<()> {
                     .flush_clients()
                     .expect("failed to flush client buffers");
 
-                TimeoutAction::ToDuration(Duration::from_millis(1))
+                TimeoutAction::ToDuration(Duration::from_micros(((1.0 / 144.0) * 1000000.0) as u64))
             });
 
     if let Err(err) = insert_ret {
@@ -386,7 +399,7 @@ pub fn run_winit() -> anyhow::Result<()> {
     }
 
     event_loop.run(
-        Some(Duration::from_millis(1)),
+        Some(Duration::from_micros(((1.0 / 144.0) * 1000000.0) as u64)),
         &mut CalloopData { display, state },
         |_data| {
             // println!("{}", _data.state.space.elements().count());

src/grab/move_grab.rs

@@ -86,6 +86,19 @@ impl PointerGrab<State> for MoveSurfaceGrab<State> {
                     return;
                 }
 
+                if state
+                    .space
+                    .element_geometry(&self.window)
+                    .is_some_and(|geo| {
+                        state
+                            .space
+                            .element_geometry(&window_under)
+                            .is_some_and(|geo2| geo.overlaps(geo2))
+                    })
+                {
+                    return;
+                }
+
                 let is_floating =
                     window_under.with_state(|state| state.floating_or_tiled.is_floating());
 
@@ -101,6 +114,7 @@ impl PointerGrab<State> for MoveSurfaceGrab<State> {
                     return;
                 }
 
+                tracing::debug!("Swapping window positions");
                 state.swap_window_positions(&self.window, &window_under);
             }
         } else {

src/handlers.rs

@@ -95,8 +95,7 @@ impl CompositorHandler for State {
     }
 
     fn commit(&mut self, surface: &WlSurface) {
-        // tracing::debug!("commit on surface {:?}", surface);
+        // tracing::debug!("commit on surface {surface:?}");
-
         X11Wm::commit_hook::<CalloopData>(surface);
 
         utils::on_commit_buffer_handler::<Self>(surface);
@@ -107,8 +106,16 @@ impl CompositorHandler for State {
             while let Some(parent) = compositor::get_parent(&root) {
                 root = parent;
             }
-            if let Some(WindowElement::Wayland(window)) = self.window_for_surface(surface) {
+            if let Some(win @ WindowElement::Wayland(window)) = &self.window_for_surface(&root) {
+                // tracing::debug!("window commit thing {:?}", win.class());
                 window.on_commit();
+                win.with_state(|state| {
+                    if let LocationRequestState::Acknowledged(new_pos) = state.loc_request_state {
+                        tracing::debug!("Mapping Acknowledged window");
+                        state.loc_request_state = LocationRequestState::Idle;
+                        self.space.map_element(win.clone(), new_pos, false);
+                    }
+                });
             }
         };
 
@@ -118,15 +125,9 @@ impl CompositorHandler for State {
 
         crate::grab::resize_grab::handle_commit(self, surface);
 
-        if let Some(window) = self.window_for_surface(surface) {
+        // if let Some(window) = self.window_for_surface(surface) {
-            window.with_state(|state| {
+        //     tracing::debug!("commit on window {:?}", window.class());
-                if let LocationRequestState::Acknowledged(new_pos) = state.loc_request_state {
+        // }
-                    tracing::debug!("Mapping Acknowledged window");
-                    state.loc_request_state = LocationRequestState::Idle;
-                    self.space.map_element(window.clone(), new_pos, false);
-                }
-            });
-        }
     }
 
     fn client_compositor_state<'a>(&self, client: &'a Client) -> &'a CompositorClientState {

src/handlers/xdg_shell.rs

@@ -1,3 +1,5 @@
+use std::time::Duration;
+
 use smithay::{
     delegate_xdg_shell,
     desktop::{
@@ -736,20 +738,30 @@ impl XdgShellHandler for State {
 
     fn ack_configure(&mut self, surface: WlSurface, configure: Configure) {
         if let Some(window) = self.window_for_surface(&surface) {
-            window.with_state(|state| {
+            if let LocationRequestState::Requested(serial, new_loc) =
-                if let LocationRequestState::Requested(serial, new_loc) = state.loc_request_state {
+                window.with_state(|state| state.loc_request_state.clone())
-                    match &configure {
+            {
-                        Configure::Toplevel(configure) => {
+                match &configure {
-                            if configure.serial >= serial {
+                    Configure::Toplevel(configure) => {
-                                tracing::debug!("acked configure, new loc is {:?}", new_loc);
+                        if configure.serial >= serial {
+                            tracing::debug!("acked configure, new loc is {:?}", new_loc);
+                            window.with_state(|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!(),
                     }
+                    Configure::Popup(_) => todo!(),
                 }
-            });
+            }
         }
     }
 

src/render.rs

@@ -5,8 +5,8 @@ use std::sync::Mutex;
 use smithay::{
     backend::renderer::{
         element::{
-            self, surface::WaylandSurfaceRenderElement, texture::TextureBuffer, AsRenderElements,
+            self, surface::WaylandSurfaceRenderElement, texture::TextureBuffer,
-            RenderElementStates, Wrap,
+            utils::CropRenderElement, AsRenderElements, RenderElementStates, Wrap,
         },
         ImportAll, ImportMem, Renderer, Texture,
     },
@@ -38,16 +38,17 @@ use self::pointer::{PointerElement, PointerRenderElement};
 pub mod pointer;
 
 render_elements! {
-    pub CustomRenderElements<R> where R: ImportAll + ImportMem;
+    pub CustomRenderElements<R, E> where R: ImportAll + ImportMem;
-    Pointer=PointerRenderElement<R>,
+    Pointer = PointerRenderElement<R>,
-    Surface=WaylandSurfaceRenderElement<R>,
+    Surface = WaylandSurfaceRenderElement<R>,
+    Crop = CropRenderElement<E>,
 }
 
 render_elements! {
     pub OutputRenderElements<R, E> where R: ImportAll + ImportMem;
     Space=SpaceRenderElements<R, E>,
     Window=Wrap<E>,
-    Custom=CustomRenderElements<R>,
+    Custom=CustomRenderElements<R, E>,
 }
 
 impl<R> AsRenderElements<R> for WindowElement
@@ -64,7 +65,6 @@ where
         scale: Scale<f64>,
         alpha: f32,
     ) -> Vec<C> {
-        // let window_bbox = self.bbox();
         match self {
             WindowElement::Wayland(window) => {
                 window.render_elements(renderer, location, scale, alpha)
@@ -111,7 +111,7 @@ where
         .map(|(surface, loc)| {
             let render_elements = surface.render_elements::<WaylandSurfaceRenderElement<R>>(
                 renderer,
-                loc.to_physical(1),
+                loc.to_physical((scale.x.round() as i32, scale.x.round() as i32)),
                 scale,
                 1.0,
             );
@@ -136,27 +136,35 @@ where
 }
 
 /// Get the render_elements for the provided tags.
-fn tag_render_elements<R, C>(
+fn tag_render_elements<R>(
     windows: &[WindowElement],
     space: &Space<WindowElement>,
     renderer: &mut R,
     scale: Scale<f64>,
-) -> Vec<C>
+) -> Vec<CustomRenderElements<R, WaylandSurfaceRenderElement<R>>>
 where
     R: Renderer + ImportAll + ImportMem,
     <R as Renderer>::TextureId: 'static,
-    C: From<WaylandSurfaceRenderElement<R>>,
 {
     let elements = windows
         .iter()
         .rev() // rev because I treat the focus stack backwards vs how the renderer orders it
         .filter(|win| win.is_on_active_tag(space.outputs()))
-        .flat_map(|win| {
+        .map(|win| {
             // subtract win.geometry().loc to align decorations correctly
             let loc = (space.element_location(win).unwrap_or((0, 0).into())
                 - win.geometry().loc)
-                .to_physical(1);
+                .to_physical((scale.x.round() as i32, scale.x.round() as i32));
-            win.render_elements::<C>(renderer, loc, scale, 1.0)
+            (win.render_elements::<WaylandSurfaceRenderElement<R>>(renderer, loc, scale, 1.0), space.element_geometry(win))
+        }).flat_map(|(elems, rect)| {
+            match rect {
+                Some(rect) => {
+                    elems.into_iter().filter_map(|elem| {
+                        CropRenderElement::from_element(elem, scale, rect.to_physical_precise_down(scale))
+                    }).map(CustomRenderElements::from).collect::<Vec<_>>()
+                },
+                None => elems.into_iter().map(CustomRenderElements::from).collect(),
+            }
         })
         .collect::<Vec<_>>();
 
@@ -165,14 +173,14 @@ where
 
 #[allow(clippy::too_many_arguments)]
 pub fn generate_render_elements<R, T>(
+    output: &Output,
+    renderer: &mut R,
     space: &Space<WindowElement>,
     windows: &[WindowElement],
     override_redirect_windows: &[X11Surface],
     pointer_location: Point<f64, Logical>,
     cursor_status: &mut CursorImageStatus,
     dnd_icon: Option<&WlSurface>,
-    renderer: &mut R,
-    output: &Output,
     input_method: &InputMethodHandle,
     pointer_element: &mut PointerElement<T>,
     pointer_image: Option<&TextureBuffer<T>>,
@@ -187,7 +195,7 @@ where
         .expect("called output_geometry on an unmapped output");
     let scale = Scale::from(output.current_scale().fractional_scale());
 
-    let mut custom_render_elements: Vec<CustomRenderElements<_>> = Vec::new();
+    let mut custom_render_elements: Vec<CustomRenderElements<_, _>> = Vec::new();
     // draw input method surface if any
     let rectangle = input_method.coordinates();
     let position = Point::from((
@@ -317,8 +325,7 @@ where
                 overlay,
             } = layer_render_elements(output, renderer, scale);
 
-            let window_render_elements: Vec<WaylandSurfaceRenderElement<R>> =
+            let window_render_elements = tag_render_elements::<R>(windows, space, renderer, scale);
-                tag_render_elements(windows, space, renderer, scale);
 
             let mut output_render_elements =
                 Vec::<OutputRenderElements<R, WaylandSurfaceRenderElement<R>>>::new();
@@ -335,8 +342,19 @@ where
                 overlay
                     .into_iter()
                     .chain(top)
-                    .chain(window_render_elements)
+                    .map(CustomRenderElements::from)
-                    .chain(bottom)
+                    .map(OutputRenderElements::from),
+            );
+
+            output_render_elements.extend(
+                window_render_elements
+                    .into_iter()
+                    .map(OutputRenderElements::from),
+            );
+
+            output_render_elements.extend(
+                bottom
+                    .into_iter()
                     .chain(background)
                     .map(CustomRenderElements::from)
                     .map(OutputRenderElements::from),