Refactor screencopy/screencast to render via the damage tracker, fix cursorless screencopy with damage

The damage tracker stores framebuffer effect cache, so we want anything
that renders repeatedly to render through a reused damage tracker. This
way, the cache persists and is reused across renders.

This will be important for the non-xray background effects.

src/niri.rs

@@ -1,4 +1,4 @@
-use std::cell::{Cell, OnceCell, RefCell};
+use std::cell::{Cell, RefCell};
 use std::collections::{HashMap, HashSet};
 use std::ffi::OsString;
 use std::os::unix::net::UnixStream;
@@ -28,7 +28,7 @@ use smithay::backend::renderer::element::utils::{
     RescaleRenderElement,
 };
 use smithay::backend::renderer::element::{
-    default_primary_scanout_output_compare, Element, Id, Kind, PrimaryScanoutOutput,
+    default_primary_scanout_output_compare, Element, Id, Kind, PrimaryScanoutOutput, RenderElement,
     RenderElementStates,
 };
 use smithay::backend::renderer::gles::GlesRenderer;
@@ -5194,57 +5194,65 @@ impl Niri {
         let _span = tracy_client::span!("Niri::render_for_screencopy_with_damage");
 
         let mut screencopy_state = mem::take(&mut self.screencopy_state);
-        let elements = OnceCell::new();
 
         screencopy_state.with_queues_mut(|queue| {
             let (damage_tracker, screencopy) = queue.split();
             if let Some(screencopy) = screencopy {
                 if screencopy.output() == output {
-                    let elements = elements.get_or_init(|| {
+                    let ctx = RenderCtx {
-                        let ctx = RenderCtx {
-                            renderer,
-                            target: RenderTarget::ScreenCapture,
-                            xray: None,
-                        };
-                        self.render_to_vec(ctx, output, true)
-                    });
-                    // FIXME: skip elements if not including pointers
-                    let render_result = Self::render_for_screencopy_internal(
                         renderer,
+                        target: RenderTarget::ScreenCapture,
+                        xray: None,
+                    };
+                    let offset = screencopy.region_loc().upscale(-1);
+                    let mut elements = Vec::new();
+                    self.render(ctx, output, screencopy.overlay_cursor(), &mut |elem| {
+                        let elem =
+                            RelocateRenderElement::from_element(elem, offset, Relocate::Relative);
+                        elements.push(elem);
+                    });
+
+                    let (damages, states) = Self::damage_screencopy_internal(
                         output,
-                        elements,
+                        &elements,
-                        true,
                         damage_tracker,
                         screencopy,
                     );
-                    match render_result {
+                    if let Some(damages) = damages {
-                        Ok((sync, damages)) => {
+                        // Convert from Physical coordinates back to Buffer coordinates.
-                            if let Some(damages) = damages {
+                        let transform = output.current_transform();
-                                // Convert from Physical coordinates back to Buffer coordinates.
+                        let physical_size = transform.transform_size(screencopy.buffer_size());
-                                let transform = output.current_transform();
+                        let damages = damages.iter().map(|dmg| {
-                                let physical_size =
+                            dmg.to_logical(1).to_buffer(
-                                    transform.transform_size(screencopy.buffer_size());
+                                1,
-                                let damages = damages.iter().map(|dmg| {
+                                transform.invert(),
-                                    dmg.to_logical(1).to_buffer(
+                                &physical_size.to_logical(1),
-                                        1,
+                            )
-                                        transform.invert(),
+                        });
-                                        &physical_size.to_logical(1),
-                                    )
-                                });
 
-                                screencopy.damage(damages);
+                        screencopy.damage(damages);
+
+                        let render_result = Self::render_for_screencopy_internal(
+                            renderer,
+                            damage_tracker,
+                            &elements,
+                            states,
+                            screencopy,
+                        );
+                        match render_result {
+                            Ok(sync) => {
                                 queue.pop().submit_after_sync(false, sync, &self.event_loop);
-                            } else {
+                            }
-                                trace!("no damage found, waiting till next redraw");
+                            Err(err) => {
+                                // Recreate damage tracker to report full damage next check.
+                                *damage_tracker =
+                                    OutputDamageTracker::new((0, 0), 1.0, Transform::Normal);
+                                queue.pop();
+                                warn!("error rendering for screencopy: {err:?}");
                             }
                         }
-                        Err(err) => {
+                    } else {
-                            // Recreate damage tracker to report full damage next check.
+                        trace!("no damage found, waiting till next redraw");
-                            *damage_tracker =
-                                OutputDamageTracker::new((0, 0), 1.0, Transform::Normal);
-                            queue.pop();
-                            warn!("error rendering for screencopy: {err:?}");
-                        }
                     }
                 };
             }
@@ -5274,24 +5282,28 @@ impl Niri {
             target: RenderTarget::ScreenCapture,
             xray: None,
         };
-        let elements = self.render_to_vec(ctx, output, screencopy.overlay_cursor());
+        let offset = screencopy.region_loc().upscale(-1);
+        let mut elements = Vec::new();
+        self.render(ctx, output, screencopy.overlay_cursor(), &mut |elem| {
+            let elem = RelocateRenderElement::from_element(elem, offset, Relocate::Relative);
+            elements.push(elem);
+        });
 
         let Some(damage_tracker) = self.screencopy_state.damage_tracker(manager) else {
             error!("screencopy queue must not be deleted as long as frames exist");
             bail!("screencopy queue missing");
         };
 
-        let render_result = Self::render_for_screencopy_internal(
+        let (_damages, states) =
+            Self::damage_screencopy_internal(output, &elements, damage_tracker, &screencopy);
+        let res = Self::render_for_screencopy_internal(
             renderer,
-            output,
-            &elements,
-            false,
             damage_tracker,
+            &elements,
+            states,
             &screencopy,
         );
-
+        let res = res.map(|sync| screencopy.submit_after_sync(false, sync, &self.event_loop));
-        let res = render_result
-            .map(|(sync, _damage)| screencopy.submit_after_sync(false, sync, &self.event_loop));
 
         if res.is_err() {
             // Recreate damage tracker to report full damage next check.
@@ -5301,15 +5313,15 @@ impl Niri {
         res
     }
 
-    #[allow(clippy::type_complexity)]
+    fn damage_screencopy_internal<'a>(
-    fn render_for_screencopy_internal<'a>(
-        renderer: &mut GlesRenderer,
         output: &Output,
-        elements: &[OutputRenderElements<GlesRenderer>],
+        elements: &[impl Element],
-        with_damage: bool,
         damage_tracker: &'a mut OutputDamageTracker,
         screencopy: &Screencopy,
-    ) -> anyhow::Result<(Option<SyncPoint>, Option<&'a Vec<Rectangle<i32, Physical>>>)> {
+    ) -> (
+        Option<&'a Vec<Rectangle<i32, Physical>>>,
+        RenderElementStates,
+    ) {
         let OutputModeSource::Static {
             size: last_size,
             scale: last_scale,
@@ -5327,41 +5339,33 @@ impl Niri {
             *damage_tracker = OutputDamageTracker::new(size, scale, transform);
         }
 
-        let region_loc = screencopy.region_loc();
-        let elements = elements
-            .iter()
-            .map(|element| {
-                RelocateRenderElement::from_element(
-                    element,
-                    region_loc.upscale(-1),
-                    Relocate::Relative,
-                )
-            })
-            .collect::<Vec<_>>();
-
         // Just checked damage tracker has static mode
-        let damages = damage_tracker.damage_output(1, &elements).unwrap().0;
+        damage_tracker.damage_output(1, elements).unwrap()
-        if with_damage && damages.is_none() {
+    }
-            return Ok((None, None));
-        }
-
-        let elements = elements.iter().rev();
 
+    #[allow(clippy::type_complexity)]
+    fn render_for_screencopy_internal(
+        renderer: &mut GlesRenderer,
+        damage_tracker: &mut OutputDamageTracker,
+        elements: &[impl RenderElement<GlesRenderer>],
+        states: RenderElementStates,
+        screencopy: &Screencopy,
+    ) -> anyhow::Result<Option<SyncPoint>> {
         let sync = match screencopy.buffer() {
             ScreencopyBuffer::Dmabuf(dmabuf) => {
                 let sync =
-                    render_to_dmabuf(renderer, dmabuf.clone(), size, scale, transform, elements)
+                    render_to_dmabuf(renderer, damage_tracker, dmabuf.clone(), elements, states)
                         .context("error rendering to screencopy dmabuf")?;
                 Some(sync)
             }
             ScreencopyBuffer::Shm(wl_buffer) => {
-                render_to_shm(renderer, wl_buffer, size, scale, transform, elements)
+                render_to_shm(renderer, damage_tracker, wl_buffer, elements, states)
                     .context("error rendering to screencopy shm buffer")?;
                 None
             }
         };
 
-        Ok((sync, damages))
+        Ok(sync)
     }
 
     #[cfg(not(feature = "xdp-gnome-screencast"))]

src/render_helpers/mod.rs

@@ -4,8 +4,9 @@ use anyhow::{ensure, Context as _};
 use niri_config::BlockOutFrom;
 use smithay::backend::allocator::dmabuf::Dmabuf;
 use smithay::backend::allocator::{Buffer, Fourcc};
+use smithay::backend::renderer::damage::OutputDamageTracker;
 use smithay::backend::renderer::element::utils::{Relocate, RelocateRenderElement};
-use smithay::backend::renderer::element::{Element, Kind, RenderElement};
+use smithay::backend::renderer::element::{Element, Kind, RenderElement, RenderElementStates};
 use smithay::backend::renderer::gles::{
     GlesError, GlesMapping, GlesRenderer, GlesTarget, GlesTexture,
 };
@@ -269,33 +270,44 @@ pub fn render_to_vec(
 
 pub fn render_to_dmabuf(
     renderer: &mut GlesRenderer,
+    damage_tracker: &mut OutputDamageTracker,
     mut dmabuf: Dmabuf,
-    size: Size<i32, Physical>,
+    elements: &[impl RenderElement<GlesRenderer>],
-    scale: Scale<f64>,
+    states: RenderElementStates,
-    transform: Transform,
-    elements: impl Iterator<Item = impl RenderElement<GlesRenderer>>,
 ) -> anyhow::Result<SyncPoint> {
     let _span = tracy_client::span!();
+    let (size, _scale, _transform) = damage_tracker.mode().try_into().unwrap();
     ensure!(
         dmabuf.width() == size.w as u32 && dmabuf.height() == size.h as u32,
         "invalid buffer size"
     );
-    let mut target = renderer
+
-        .bind(&mut dmabuf)
+    let mut target = renderer.bind(&mut dmabuf).context("error binding dmabuf")?;
-        .context("error binding texture")?;
+    let res = damage_tracker
-    render_elements(renderer, &mut target, size, scale, transform, elements)
+        .render_output_with_states(
+            renderer,
+            &mut target,
+            0,
+            elements,
+            Color32F::TRANSPARENT,
+            states,
+        )
+        .context("error rendering to dmabuf")?;
+    Ok(res.sync)
 }
 
 pub fn render_to_shm(
     renderer: &mut GlesRenderer,
+    damage_tracker: &mut OutputDamageTracker,
     buffer: &WlBuffer,
-    size: Size<i32, Physical>,
+    elements: &[impl RenderElement<GlesRenderer>],
-    scale: Scale<f64>,
+    states: RenderElementStates,
-    transform: Transform,
-    elements: impl Iterator<Item = impl RenderElement<GlesRenderer>>,
 ) -> anyhow::Result<()> {
     let _span = tracy_client::span!();
     shm::with_buffer_contents_mut(buffer, |shm_buffer, shm_len, buffer_data| {
+        let (size, _scale, _transform) = damage_tracker.mode().try_into().unwrap();
+        let fourcc = Fourcc::Xrgb8888;
+
         ensure!(
             // The buffer prefers pixels in little endian ...
             buffer_data.format == wl_shm::Format::Xrgb8888
@@ -305,9 +317,26 @@ pub fn render_to_shm(
                 && shm_len == buffer_data.stride as usize * buffer_data.height as usize,
             "invalid buffer format or size"
         );
-        let mapping =
-            render_and_download(renderer, size, scale, transform, Fourcc::Xrgb8888, elements)?;
 
+        let mut texture =
+            create_texture(renderer, size, fourcc).context("error creating texture")?;
+        let mut target = renderer
+            .bind(&mut texture)
+            .context("error binding texture")?;
+
+        let _res = damage_tracker
+            .render_output_with_states(
+                renderer,
+                &mut target,
+                0,
+                elements,
+                Color32F::TRANSPARENT,
+                states,
+            )
+            .context("error rendering")?;
+
+        let mapping =
+            copy_framebuffer(renderer, &target, fourcc).context("error copying framebuffer")?;
         let bytes = renderer
             .map_texture(&mapping)
             .context("error mapping texture")?;

src/screencasting/mod.rs

@@ -201,11 +201,6 @@ impl State {
 
             self.backend.with_primary_renderer(|renderer| {
                 let mut elements = Vec::new();
-                mapped.render_for_screen_cast(renderer, scale, &mut |elem| {
-                    elements.push(CastRenderElement::from(elem))
-                });
-
-                let mut pointer_elements = Vec::new();
                 let mut pointer_location = Point::default();
 
                 if self.niri.pointer_visibility.is_visible() {
@@ -225,11 +220,18 @@ impl State {
                         self.niri.render_pointer(renderer, output, &mut |elem| {
                             let elem =
                                 RelocateRenderElement::from_element(elem, pos, Relocate::Relative);
-                            pointer_elements.push(CastRenderElement::from(elem));
+                            elements.push(CastRenderElement::from(elem));
                         });
                     }
                 }
-                let cursor_data = CursorData::compute(&pointer_elements, pointer_location, scale);
+
+                let main_start = elements.len();
+                mapped.render_for_screen_cast(renderer, scale, &mut |elem| {
+                    elements.push(CastRenderElement::from(elem))
+                });
+
+                let cursor_data =
+                    CursorData::compute(&elements, main_start, pointer_location, scale);
 
                 if cast.dequeue_buffer_and_render(
                     renderer,
@@ -546,7 +548,6 @@ impl Niri {
         let scale = Scale::from(output.current_scale().fractional_scale());
 
         let mut elements = Vec::new();
-        let mut pointer = Vec::new();
         let mut cursor_data = None;
 
         let mut casts_to_stop = vec![];
@@ -575,13 +576,6 @@ impl Niri {
             }
 
             if cursor_data.is_none() {
-                let ctx = RenderCtx {
-                    renderer,
-                    target: RenderTarget::Screencast,
-                    xray: None,
-                };
-                self.render(ctx, output, false, &mut |elem| elements.push(elem.into()));
-
                 let mut pointer_pos = Point::default();
                 if self.pointer_visibility.is_visible() {
                     let output_geo = self.global_space.output_geometry(output).unwrap().to_f64();
@@ -593,12 +587,25 @@ impl Niri {
                     if output_geo.contains(pointer_loc) {
                         pointer_pos = pointer_loc - output_geo.loc;
                         self.render_pointer(renderer, output, &mut |elem| {
-                            pointer.push(elem.into())
+                            elements.push(elem.into())
                         });
                     }
                 }
 
-                cursor_data = Some(CursorData::compute(&pointer, pointer_pos, scale));
+                let main_start = elements.len();
+                let ctx = RenderCtx {
+                    renderer,
+                    target: RenderTarget::Screencast,
+                    xray: None,
+                };
+                self.render(ctx, output, false, &mut |elem| elements.push(elem.into()));
+
+                cursor_data = Some(CursorData::compute(
+                    &elements,
+                    main_start,
+                    pointer_pos,
+                    scale,
+                ));
             }
             let cursor_data = cursor_data.as_ref().unwrap();
 
@@ -659,11 +666,6 @@ impl Niri {
             }
 
             let mut elements = Vec::new();
-            mapped.render_for_screen_cast(renderer, scale, &mut |elem| {
-                elements.push(CastRenderElement::from(elem))
-            });
-
-            let mut pointer_elements = Vec::new();
             let mut pointer_location = Point::default();
 
             if self.pointer_visibility.is_visible() {
@@ -680,11 +682,17 @@ impl Niri {
                     self.render_pointer(renderer, output, &mut |elem| {
                         let elem =
                             RelocateRenderElement::from_element(elem, pos, Relocate::Relative);
-                        pointer_elements.push(CastRenderElement::from(elem));
+                        elements.push(CastRenderElement::from(elem));
                     });
                 }
             }
-            let cursor_data = CursorData::compute(&pointer_elements, pointer_location, scale);
+
+            let main_start = elements.len();
+            mapped.render_for_screen_cast(renderer, scale, &mut |elem| {
+                elements.push(CastRenderElement::from(elem))
+            });
+
+            let cursor_data = CursorData::compute(&elements, main_start, pointer_location, scale);
 
             if cast.dequeue_buffer_and_render(renderer, &elements, &cursor_data, bbox.size, scale) {
                 cast.last_frame_time = target_presentation_time;

src/screencasting/pw_utils.rs

@@ -153,15 +153,19 @@ pub enum CastSizeChange {
 
 /// Data for drawing a cursor either as metadata or embedded.
 ///
+/// The cursor elements are expected to be at the start of the main elements slice. `elem_count` is
+/// the count of the pointer elements. This way, the full slice includes both main and cursor
+/// elements for embedded mode, and `&elements[elem_count..]` gives just the main elements for
+/// metadata mode.
+///
 /// We have weird borrowed references here in order to support both metadata and embedded cases.
 /// The cursor damage tracker needs a slice of impl Element at (0, 0), so we pass it `relocated`
-/// (luckily, &impl Element also impls Element). Then, if we need to embed the cursor, we chain the
+/// (luckily, &impl Element also impls Element). Then, if we need to embed the cursor, we use the
-/// elements to the main video buffer elements, so we need the same type. We use `original` for
+/// full elements slice which starts with non-relocated pointer elements (that we borrow from).
-/// this; `E` is expected to match the type of the main video buffer elements.
 #[derive(Debug)]
 pub struct CursorData<'a, E> {
-    /// Cursor elements at their original location.
+    /// Count of the pointer elements in the slice (index of the first non-pointer element).
-    original: &'a [E],
+    elem_count: usize,
     /// Cursor elements relocated to (0, 0).
     relocated: Vec<RelocateRenderElement<&'a E>>,
     /// Location of the cursor's hotspot in the video buffer.
@@ -175,16 +179,22 @@ pub struct CursorData<'a, E> {
 }
 
 impl<'a, E: Element> CursorData<'a, E> {
-    pub fn compute(elements: &'a [E], location: Point<f64, Logical>, scale: Scale<f64>) -> Self {
+    pub fn compute(
+        elements: &'a [E],
+        elem_count: usize,
+        location: Point<f64, Logical>,
+        scale: Scale<f64>,
+    ) -> Self {
+        let pointer_elements = &elements[..elem_count];
         let location = location.to_physical_precise_round(scale);
 
-        let geo = encompassing_geo(scale, elements.iter());
+        let geo = encompassing_geo(scale, pointer_elements.iter());
-        let relocated = Vec::from_iter(elements.iter().map(|elem| {
+        let relocated = Vec::from_iter(pointer_elements.iter().map(|elem| {
             RelocateRenderElement::from_element(elem, geo.loc.upscale(-1), Relocate::Relative)
         }));
 
         Self {
-            original: elements,
+            elem_count,
             relocated,
             location,
             hotspot: location - geo.loc,
@@ -1052,7 +1062,7 @@ impl Cast {
     pub fn dequeue_buffer_and_render(
         &mut self,
         renderer: &mut GlesRenderer,
-        elements: &[CastRenderElement<GlesRenderer>],
+        mut elements: &[CastRenderElement<GlesRenderer>],
         cursor_data: &CursorData<CastRenderElement<GlesRenderer>>,
         size: Size<i32, Physical>,
         scale: Scale<f64>,
@@ -1092,11 +1102,17 @@ impl Cast {
             );
         }
 
-        let (damage, _states) = damage_tracker.damage_output(1, elements).unwrap();
-
         let mut has_cursor_update = false;
         let mut redraw_cursor = false;
-        if self.cursor_mode != CursorMode::Hidden {
+
+        // For embedded cursor, pass the full slice (cursor + main) to the damage tracker.
+        // For metadata or hidden cursor, pass only the main elements.
+        if self.cursor_mode == CursorMode::Metadata || self.cursor_mode == CursorMode::Hidden {
+            elements = &elements[cursor_data.elem_count..];
+        }
+        let (damage, states) = damage_tracker.damage_output(1, elements).unwrap();
+
+        if self.cursor_mode == CursorMode::Metadata {
             let (damage, _states) = cursor_damage_tracker
                 .damage_output(1, &cursor_data.relocated)
                 .unwrap();
@@ -1118,33 +1134,30 @@ impl Cast {
         };
         let buffer = pw_buffer.as_ptr();
 
+        let mut inner = self.inner.borrow_mut();
+        let inner_ = &mut *inner;
+        let CastState::Ready { damage_tracker, .. } = &mut inner_.state else {
+            unreachable!()
+        };
+        let damage_tracker = damage_tracker.as_mut().unwrap();
+
         unsafe {
             let spa_buffer = (*buffer).buffer;
 
-            let mut pointer_elements = None;
             if self.cursor_mode == CursorMode::Metadata {
                 add_cursor_metadata(renderer, spa_buffer, cursor_data, redraw_cursor);
-            } else if self.cursor_mode != CursorMode::Hidden {
-                // Embed the cursor into the main render.
-                pointer_elements = Some(cursor_data.original.iter());
             }
-            let pointer_elements = pointer_elements.into_iter().flatten();
-            let elements = pointer_elements.chain(elements);
 
             // FIXME: would be good to skip rendering the full frame if only the pointer changed.
             // Unfortunately, I think the OBS PipeWire code needs to be updated first to cleanly
             // allow for that codepath.
             let fd = (*(*spa_buffer).datas).fd;
-            let dmabuf = self.inner.borrow().dmabufs[&fd].clone();
+            let dmabuf = inner_.dmabufs[&fd].clone();
 
-            match render_to_dmabuf(
+            let res = render_to_dmabuf(renderer, damage_tracker, dmabuf, elements, states);
-                renderer,
+            drop(inner);
-                dmabuf,
+
-                size,
+            match res {
-                scale,
-                Transform::Normal,
-                elements.rev(),
-            ) {
                 Ok(sync_point) => {
                     mark_buffer_as_good(pw_buffer, &mut self.sequence_counter);
                     trace!("queueing buffer with seq={}", self.sequence_counter);