Refactor rendering to push-based instead of pull-based (#3113)

Our current rendering code constructs and returns complex
`-> impl Iterator<Item = SomeRenderElement>` types that are collected
into a vector at the top level Niri::render(). This causes some
problems:
- It's hard to write logic around returning iterators. Especially things
  like conditions, since the returned iterator must have a single type,
  you can't branch and return different iterators. This will be solved
  by gen fn but alas it's not here yet.
- In many cases, the returned `-> impl Iterator` will borrow from &self
  leading to complex lifetimes. In certain cases, it is also desirable
  for it to borrow the &mut NiriRenderer, which causes a lot of issues
  because it's exclusive (&mut).
- Sometimes those issues are too hard to deal with, leading to the
  escape hatch of allocating and returning a temporary
  Vec<SomeRenderElement>, like in
  Scrolling/FloatingSpace::render_elements(). These allocations are
  unfortunate because they are not really necessary.
- It's impossible to use some downstream combinators with this
  `-> impl Iterator` approach, leading to functions like Smithay's
  render_elements_from_surface_tree() returning a Vec. This is extra
  unfortunate because it results in a temporary allocation per Wayland
  toplevel/popup.
- It's hard to properly create profiling spans for the rendering
  functions since the spans are dropped when the (lazy) iterator is
  returned and not when all the code actually completes.
- The code compiles down to complex state machines in generated iterator
  types with logic located in Iterator::next(), which makes it annoying
  to follow in debuggers and profiling tools.

This refactor changes the code to push-based iteration: rendering
functions receive a push() closure that they call to push their render
elements. It solves all of the aforementioned problems:
- The logic becomes simpler. Just use conditionals and loops as normal.
- No borrowing and lifetimes since we're not returning anything.
- All temporary Vecs are removed because the problems they worked around
  no longer exist.
- The new push_elements_from_surface_tree() helper is the same as
  render_elements_from_surface_tree() but doesn't allocate a temporary
  Vec since it's not necessary; the push() closure can be passed down.
- Profiling spans work normally since the function returns when it ran
  all of the logic.
- The code compiles down to normal functions and calls as expected.

Generally, the iterator approach gives these advantages:
- You can wrap the returned items in the upstream logic. This is
  possible in exactly the same way with the push closure.
- You can decide to cut the iterator short in the upstream logic. This
  is not possible with push-based iteration, but we don't actually use
  it anywhere.

I chose the push closure type to be &mut dyn FnMut(SomeRenderElement).
It's deliberately not a generic impl FnMut() to avoid duplicating the
rendering logic when it's called from several different places. But it's
still a normal closure that can capture the outside context.

While my original idea for this refactor was to simplify the logic while
getting rid of temporary Vecs, it also appears to have brought a
consistent 2-3x speedup to the whole render list construction. On an old
Eee PC laptop I even observed a 8x speedup.

The refactor also results in smaller binary size, presumably due to
removing many iterator combinators and state tracking.
This commit is contained in:
Ivan Molodetskikh
2025-12-25 14:26:19 +03:00
committed by GitHub
parent 1a63089d67
commit 7f132ecf95
24 changed files with 764 additions and 782 deletions
+2 -5
View File
@@ -89,11 +89,8 @@ impl TestCase for GradientArea {
1.,
1.,
);
rv.extend(
self.border
.render(renderer, g_loc)
.map(|elem| Box::new(elem) as _),
);
self.border
.render(renderer, g_loc, &mut |elem| rv.push(Box::new(elem) as _));
rv.extend(
[BorderRenderElement::new(
+6 -4
View File
@@ -268,12 +268,14 @@ impl TestCase for Layout {
_size: Size<i32, Physical>,
) -> Vec<Box<dyn RenderElement<GlesRenderer>>> {
self.layout.update_render_elements(Some(&self.output));
let mut rv = Vec::new();
self.layout
.monitor_for_output(&self.output)
.unwrap()
.render_elements(renderer, RenderTarget::Output, true)
.flat_map(|(_, _, iter)| iter)
.map(|elem| Box::new(elem) as _)
.collect()
.render_workspaces(renderer, RenderTarget::Output, true, &mut |elem| {
rv.push(Box::new(elem) as _)
});
rv
}
}
+10 -4
View File
@@ -119,9 +119,15 @@ impl TestCase for Tile {
true,
Rectangle::new(Point::from((-location.x, -location.y)), size.to_logical(1.)),
);
self.tile
.render(renderer, location, true, RenderTarget::Output)
.map(|elem| Box::new(elem) as _)
.collect()
let mut rv = Vec::new();
self.tile.render(
renderer,
location,
true,
RenderTarget::Output,
&mut |elem| rv.push(Box::new(elem) as _),
);
rv
}
}
+10 -11
View File
@@ -52,16 +52,15 @@ impl TestCase for Window {
.to_f64()
.downscale(2.);
self.window
.render(
renderer,
location,
Scale::from(1.),
1.,
RenderTarget::Output,
)
.into_iter()
.map(|elem| Box::new(elem) as _)
.collect()
let mut rv = Vec::new();
self.window.render_normal(
renderer,
location,
Scale::from(1.),
1.,
RenderTarget::Output,
&mut |elem| rv.push(Box::new(elem) as _),
);
rv
}
}
+16 -22
View File
@@ -9,7 +9,7 @@ use niri::layout::{
use niri::render_helpers::offscreen::OffscreenData;
use niri::render_helpers::renderer::NiriRenderer;
use niri::render_helpers::solid_color::{SolidColorBuffer, SolidColorRenderElement};
use niri::render_helpers::{RenderTarget, SplitElements};
use niri::render_helpers::RenderTarget;
use niri::utils::transaction::Transaction;
use niri::window::ResolvedWindowRules;
use smithay::backend::renderer::element::Kind;
@@ -149,36 +149,30 @@ impl LayoutElement for TestWindow {
false
}
fn render<R: NiriRenderer>(
fn render_normal<R: NiriRenderer>(
&self,
_renderer: &mut R,
location: Point<f64, Logical>,
_scale: Scale<f64>,
alpha: f32,
_target: RenderTarget,
) -> SplitElements<LayoutElementRenderElement<R>> {
push: &mut dyn FnMut(LayoutElementRenderElement<R>),
) {
let inner = self.inner.borrow();
SplitElements {
normal: vec![
SolidColorRenderElement::from_buffer(
&inner.buffer,
location,
alpha,
Kind::Unspecified,
)
push(
SolidColorRenderElement::from_buffer(&inner.buffer, location, alpha, Kind::Unspecified)
.into(),
SolidColorRenderElement::from_buffer(
&inner.csd_shadow_buffer,
location
- Point::from((inner.csd_shadow_width, inner.csd_shadow_width)).to_f64(),
alpha,
Kind::Unspecified,
)
.into(),
],
popups: vec![],
}
);
push(
SolidColorRenderElement::from_buffer(
&inner.csd_shadow_buffer,
location - Point::from((inner.csd_shadow_width, inner.csd_shadow_width)).to_f64(),
alpha,
Kind::Unspecified,
)
.into(),
);
}
fn request_size(