Fix rendering efficiency and screencopy Y-flip

Rendering: Skip the full bind/clear/draw/finish/submit cycle when
nothing visual has changed (no new Electron frames, no dirty Wayland
surfaces, no cursor movement). In the DRM path, also skip when no
screencopy or portal streams are active. Frame callbacks are always
sent unconditionally to prevent Wayland client stalls.

Screencopy: Remove the incorrect row-flip after glReadPixels in both
the wlr-screencopy fulfillment and portal stream readback paths.
Smithay already applies a Y-correction when rendering to GBM-backed
FBOs, so glReadPixels returns top-down data. The manual flip was
inverting an already-correct image, producing upside-down output.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Stephen Hellings 2026-03-10 13:43:34 -04:00
parent 0bede114a6
commit 0a72ec60bf
2 changed files with 180 additions and 150 deletions

View file

@ -90,6 +90,8 @@ struct EventLoopData {
frame_timestamps: std::collections::HashMap<String, Instant>, frame_timestamps: std::collections::HashMap<String, Instant>,
gles_tex_cache: std::collections::HashMap<String, smithay::backend::renderer::gles::GlesTexture>, gles_tex_cache: std::collections::HashMap<String, smithay::backend::renderer::gles::GlesTexture>,
input_events_pending: bool, input_events_pending: bool,
/// Track previous cursor position to detect cursor movement for damage tracking.
last_cursor_pos: (f64, f64),
session: smithay::backend::session::libseat::LibSeatSession, session: smithay::backend::session::libseat::LibSeatSession,
active: bool, active: bool,
vblank_pending: bool, vblank_pending: bool,
@ -478,6 +480,32 @@ fn render_frame(data: &mut EventLoopData) {
data.first_render = false; data.first_render = false;
} }
// Detect cursor movement for damage tracking
let cursor_pos = (data.state.pointer_location.x, data.state.pointer_location.y);
let cursor_moved = cursor_pos != data.last_cursor_pos;
if cursor_moved {
data.last_cursor_pos = cursor_pos;
}
// Check for pending screencopy requests that require a fresh render
let has_screencopy = data.state.screencopy_state.as_ref()
.map_or(false, |s| s.has_pending_frames());
let has_portal_streams = data.state.portal.as_ref()
.map_or(false, |p| !p.get_active_sessions().is_empty());
// Skip the expensive render+flip cycle when nothing visual has changed.
// Dirty surface exports (SHM/DMABUF to Electron) are handled separately
// and still run when dirty_surfaces is non-empty.
let frame_dirty = !data.state.damaged_windows.is_empty()
|| !data.state.dirty_surfaces.is_empty()
|| cursor_moved
|| has_screencopy
|| has_portal_streams;
if !frame_dirty {
return;
}
// Acquire swapchain slot // Acquire swapchain slot
let slot = match data.swapchain.acquire() { let slot = match data.swapchain.acquire() {
Ok(Some(slot)) => slot, Ok(Some(slot)) => slot,
@ -966,15 +994,7 @@ fn render_frame(data: &mut EventLoopData) {
}); });
if read_ok.is_ok() { if read_ok.is_ok() {
// GL returns bottom-up rows; flip to top-down. // GBM-backed FBO stores pixels top-down; no row flip needed.
let stride = fb_w as usize * 4;
for y in 0..(fb_h as usize / 2) {
let top = y * stride;
let bot = (fb_h as usize - 1 - y) * stride;
for x in 0..stride {
fb_pixels.swap(top + x, bot + x);
}
}
// Convert RGBA→ARGB (wl_shm expects ARGB8888) // Convert RGBA→ARGB (wl_shm expects ARGB8888)
for pixel in fb_pixels.chunks_exact_mut(4) { for pixel in fb_pixels.chunks_exact_mut(4) {
@ -1060,14 +1080,7 @@ fn render_frame(data: &mut EventLoopData) {
); );
}); });
if read_ok.is_ok() { if read_ok.is_ok() {
let stride = fb_w as usize * 4; // GBM-backed FBO stores pixels top-down; no row flip needed.
for y in 0..(fb_h as usize / 2) {
let top = y * stride;
let bot = (fb_h as usize - 1 - y) * stride;
for x in 0..stride {
fb_pixels.swap(top + x, bot + x);
}
}
for pixel in fb_pixels.chunks_exact_mut(4) { for pixel in fb_pixels.chunks_exact_mut(4) {
let (r, g, b, a) = (pixel[0], pixel[1], pixel[2], pixel[3]); let (r, g, b, a) = (pixel[0], pixel[1], pixel[2], pixel[3]);
pixel[0] = b; pixel[0] = b;
@ -1869,6 +1882,7 @@ fn run_drm(mut config: Config) -> Result<(), anyhow::Error> {
wayland_socket, wayland_socket,
frame_timestamps: std::collections::HashMap::new(), frame_timestamps: std::collections::HashMap::new(),
input_events_pending: false, input_events_pending: false,
last_cursor_pos: (0.0, 0.0),
session, session,
active: true, active: true,
vblank_pending: false, vblank_pending: false,

View file

@ -296,6 +296,8 @@ pub fn run_nested(config: Config) -> Result<()> {
// Frame tracking for backpressure management // Frame tracking for backpressure management
let mut frame_timestamps: HashMap<String, Instant> = HashMap::new(); let mut frame_timestamps: HashMap<String, Instant> = HashMap::new();
let _max_frame_age = Duration::from_millis(33); // Drop frames older than 2 frames @60Hz let _max_frame_age = Duration::from_millis(33); // Drop frames older than 2 frames @60Hz
// Track previous cursor position to detect movement
let mut last_cursor_pos: (f64, f64) = (0.0, 0.0);
info!("Nested compositor running"); info!("Nested compositor running");
@ -551,6 +553,16 @@ pub fn run_nested(config: Config) -> Result<()> {
let output_rect: Rectangle<i32, Physical> = let output_rect: Rectangle<i32, Physical> =
Rectangle::new((0, 0).into(), (size.w as i32, size.h as i32).into()); Rectangle::new((0, 0).into(), (size.w as i32, size.h as i32).into());
// Determine if anything visual changed since last frame.
let cursor_pos = (state.pointer_location.x, state.pointer_location.y);
let cursor_moved = cursor_pos != last_cursor_pos;
if cursor_moved {
last_cursor_pos = cursor_pos;
}
let has_new_frames = !latest_frames.is_empty();
let has_dirty_surfaces = !state.dirty_surfaces.is_empty();
let frame_dirty = has_new_frames || has_dirty_surfaces || cursor_moved;
// Render in a scope to release borrows before submission. // Render in a scope to release borrows before submission.
// Use a labeled block so early exits (bind/render failures) still reach // Use a labeled block so early exits (bind/render failures) still reach
// the frame-callback section below — Wayland clients MUST receive frame // the frame-callback section below — Wayland clients MUST receive frame
@ -558,6 +570,12 @@ pub fn run_nested(config: Config) -> Result<()> {
let mut render_ok = false; let mut render_ok = false;
let render_t0 = std::time::Instant::now(); let render_t0 = std::time::Instant::now();
'render: { 'render: {
// Skip the expensive bind/render/submit cycle when nothing visual changed.
// Dirty surface exports (SHM/DMABUF) still happen via the separate path below.
if !frame_dirty {
break 'render;
}
// Bind backend for rendering (this returns (renderer, target)) // Bind backend for rendering (this returns (renderer, target))
let bind_t0 = std::time::Instant::now(); let bind_t0 = std::time::Instant::now();
let (renderer, mut target) = match backend.bind() { let (renderer, mut target) = match backend.bind() {
@ -602,6 +620,8 @@ pub fn run_nested(config: Config) -> Result<()> {
let imported_textures = &gles_tex_cache; let imported_textures = &gles_tex_cache;
// Only run the full compositing pass when the output needs updating.
if frame_dirty {
// Prepare cursor elements before starting the frame // Prepare cursor elements before starting the frame
let cursor_elements: Vec<WaylandSurfaceRenderElement<GlesRenderer>> = let cursor_elements: Vec<WaylandSurfaceRenderElement<GlesRenderer>> =
if let smithay::input::pointer::CursorImageStatus::Surface(ref cursor_surface) = state.cursor_status { if let smithay::input::pointer::CursorImageStatus::Surface(ref cursor_surface) = state.cursor_status {
@ -691,11 +711,6 @@ pub fn run_nested(config: Config) -> Result<()> {
} }
} }
// (Native rendering loop was removed)
// NOTE: Wayland surfaces are rendered via offscreen GL to SHM for comraw,
// allowing the client to fully control window compositing and layout.
// Render cursor on top of everything // Render cursor on top of everything
if !cursor_elements.is_empty() { if !cursor_elements.is_empty() {
for element in &cursor_elements { for element in &cursor_elements {
@ -739,6 +754,7 @@ pub fn run_nested(config: Config) -> Result<()> {
} }
} }
} }
} // end if frame_dirty
let render_ms = render_t0.elapsed().as_millis(); let render_ms = render_t0.elapsed().as_millis();
if render_ms > 50 { if render_ms > 50 {
warn!("render phase took {}ms!", render_ms); warn!("render phase took {}ms!", render_ms);
@ -1080,7 +1096,7 @@ pub fn run_nested(config: Config) -> Result<()> {
} }
state.dirty_surfaces.clear(); state.dirty_surfaces.clear();
} // 'render block ends here, backend borrow is released } // inner bind scope ends here, backend borrow is released
let did_bind = render_ok; let did_bind = render_ok;
@ -1105,7 +1121,10 @@ pub fn run_nested(config: Config) -> Result<()> {
warn!("backend.submit() (eglSwapBuffers) took {}ms! This may indicate a blocked vsync/EGL fence.", submit_ms); warn!("backend.submit() (eglSwapBuffers) took {}ms! This may indicate a blocked vsync/EGL fence.", submit_ms);
} }
} }
} // 'render block ends here
// Frame callbacks must be sent unconditionally so Wayland clients
// don't stall — even when the compositor skipped rendering.
let presented_at = std::time::SystemTime::now() let presented_at = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH) .duration_since(std::time::UNIX_EPOCH)
.unwrap_or_default(); .unwrap_or_default();
@ -1116,8 +1135,6 @@ pub fn run_nested(config: Config) -> Result<()> {
}); });
} }
// SHM exports already written directly to mmap'd memfds and broadcast above.
let render_total_ms = render_t_start.elapsed().as_millis(); let render_total_ms = render_t_start.elapsed().as_millis();
if render_total_ms > 50 { if render_total_ms > 50 {
warn!("render+submit+callbacks total took {}ms!", render_total_ms); warn!("render+submit+callbacks total took {}ms!", render_total_ms);
@ -1131,7 +1148,6 @@ pub fn run_nested(config: Config) -> Result<()> {
warn!("SLOW LOOP iter={} total={}ms", loop_iter, total_loop_ms); warn!("SLOW LOOP iter={} total={}ms", loop_iter, total_loop_ms);
} }
} }
}
// Clean up GL textures while context is still active // Clean up GL textures while context is still active
offscreen_tex_cache.clear(); offscreen_tex_cache.clear();