Crop windows, also pause rendering on a pending size

This is a series of attempts at preventing flickering. We wrap every window render element in a CropRenderElement so that windows don't render at the incorrect size for a frame. Additionally, we also pause rendering also when the window has a pending size different from the current. Fun fact: Firefox (and by extension Librewolf) renders content to child subsurfaces, so the root surface doesn't get most commits.

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,
@@ -1480,7 +1483,15 @@ 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| {
+            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
+            }
+        })
         .map(|win| {
             (
                 win.class().unwrap_or("None".to_string()),
@@ -1491,7 +1502,7 @@ 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())
@@ -1509,14 +1520,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),
@@ -1567,7 +1578,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();
+                    state.loop_handle.insert_idle(move |dt| {
+                        for win in dt.state.windows.iter() {
                             win.send_frame(
-                            &output,
+                                &op_clone,
-                            state.clock.now(),
+                                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) => {
                         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!(),
                 }
             }
-            });
         }
     }
 

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
@@ -136,27 +137,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 +174,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 +196,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 +326,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 +343,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),