From f48ae933fffef5dc63ff335a152212e29b9f73e3 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 6 Mar 2025 20:36:54 +0100 Subject: [PATCH] graphics: Only allow passing a single damage area at a time Currently only fbbootlogd and fbcond make use of support for multiple damage areas and they are not all that performance critical anyway. Orbital always passes a single damage area per write call. Out of all graphics drivers we have and are likely to get only vesad could potentially benefit from fine-grained damage areas. This commit doesn't significantly impact performance of fbcond. --- graphics/console-draw/src/lib.rs | 63 +++++++++++++---------------- graphics/driver-graphics/src/lib.rs | 19 +++------ graphics/fbbootlogd/src/scheme.rs | 2 +- graphics/fbcond/src/display.rs | 4 +- graphics/fbcond/src/text.rs | 2 +- graphics/graphics-ipc/src/v1.rs | 6 +-- graphics/vesad/src/scheme.rs | 45 +++++++++------------ graphics/virtio-gpud/src/scheme.rs | 21 ++++------ 8 files changed, 66 insertions(+), 96 deletions(-) diff --git a/graphics/console-draw/src/lib.rs b/graphics/console-draw/src/lib.rs index 59aea00469..82d180501f 100644 --- a/graphics/console-draw/src/lib.rs +++ b/graphics/console-draw/src/lib.rs @@ -1,6 +1,6 @@ extern crate ransid; -use std::collections::{BTreeSet, VecDeque}; +use std::collections::VecDeque; use std::convert::{TryFrom, TryInto}; use std::{cmp, ptr}; @@ -15,7 +15,6 @@ pub struct DisplayMap { pub struct TextScreen { console: ransid::Console, - changed: BTreeSet, } impl TextScreen { @@ -23,7 +22,6 @@ impl TextScreen { TextScreen { // Width and height will be filled in on the next write to the console console: ransid::Console::new(0, 0), - changed: BTreeSet::new(), } } @@ -118,18 +116,24 @@ impl TextScreen { } impl TextScreen { - pub fn write( - &mut self, - map: &mut DisplayMap, - buf: &[u8], - input: &mut VecDeque, - ) -> Vec { + pub fn write(&mut self, map: &mut DisplayMap, buf: &[u8], input: &mut VecDeque) -> Damage { + let mut min_changed = usize::MAX; + let mut max_changed = 0; + let mut line_changed = |line| { + if line < min_changed { + min_changed = line; + } + if line > max_changed { + max_changed = line; + } + }; + self.console.resize(map.width / 8, map.height / 16); - if self.console.state.x > self.console.state.w { - self.console.state.x = self.console.state.w; + if self.console.state.x >= self.console.state.w { + self.console.state.x = self.console.state.w - 1; } - if self.console.state.y > self.console.state.h { - self.console.state.y = self.console.state.h; + if self.console.state.y >= self.console.state.h { + self.console.state.y = self.console.state.h - 1; } if self.console.state.cursor @@ -139,7 +143,7 @@ impl TextScreen { let x = self.console.state.x; let y = self.console.state.y; Self::invert(map, x * 8, y * 16, 8, 16); - self.changed.insert(y); + line_changed(y); } self.console.write(buf, |event| match event { @@ -152,13 +156,13 @@ impl TextScreen { .. } => { Self::char(map, x * 8, y * 16, c, color.as_rgb(), bold, false); - self.changed.insert(y); + line_changed(y); } ransid::Event::Input { data } => input.extend(data), ransid::Event::Rect { x, y, w, h, color } => { Self::rect(map, x * 8, y * 16, w * 8, h * 16, color.as_rgb()); for y2 in y..y + h { - self.changed.insert(y2); + line_changed(y2); } } ransid::Event::ScreenBuffer { .. } => (), @@ -195,7 +199,7 @@ impl TextScreen { } } - self.changed.insert(to_y + y); + line_changed(to_y + y); } } ransid::Event::Resize { .. } => (), @@ -209,27 +213,16 @@ impl TextScreen { let x = self.console.state.x; let y = self.console.state.y; Self::invert(map, x * 8, y * 16, 8, 16); - self.changed.insert(y); + line_changed(y); } let width = map.width.try_into().unwrap(); - let mut damage: Vec = vec![]; - let mut last_change = usize::MAX - 1; - for &change in &self.changed { - if change == last_change + 1 { - damage.last_mut().unwrap().height += 16; - } else { - damage.push(Damage { - x: 0, - y: u32::try_from(change).unwrap() * 16, - width, - height: 16, - }); - } - last_change = change; - } - - self.changed.clear(); + let damage = Damage { + x: 0, + y: u32::try_from(min_changed).unwrap() * 16, + width, + height: u32::try_from(max_changed.saturating_sub(min_changed)).unwrap() * 16, + }; damage } diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index cfe0089928..03cab9c6e7 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -16,12 +16,7 @@ pub trait GraphicsAdapter { fn create_dumb_framebuffer(&mut self, width: u32, height: u32) -> Self::Framebuffer; fn map_dumb_framebuffer(&mut self, framebuffer: &Self::Framebuffer) -> *mut u8; - fn update_plane( - &mut self, - display_id: usize, - framebuffer: &Self::Framebuffer, - damage: &[Damage], - ); + fn update_plane(&mut self, display_id: usize, framebuffer: &Self::Framebuffer, damage: Damage); } pub trait Framebuffer { @@ -137,12 +132,12 @@ impl GraphicsScheme { adapter.update_plane( screen, framebuffer, - &[Damage { + Damage { x: 0, y: 0, width: framebuffer.width(), height: framebuffer.height(), - }], + }, ); } } @@ -227,12 +222,8 @@ impl Scheme for GraphicsScheme { let framebuffer = &self.vts_fb[vt][screen]; - let damage = unsafe { - core::slice::from_raw_parts( - buf.as_ptr() as *const Damage, - buf.len() / core::mem::size_of::(), - ) - }; + assert_eq!(buf.len(), std::mem::size_of::()); + let damage = unsafe { *buf.as_ptr().cast::() }; self.adapter.update_plane(*screen, framebuffer, damage); diff --git a/graphics/fbbootlogd/src/scheme.rs b/graphics/fbbootlogd/src/scheme.rs index 3b66ae5dca..17166c43af 100644 --- a/graphics/fbbootlogd/src/scheme.rs +++ b/graphics/fbbootlogd/src/scheme.rs @@ -102,7 +102,7 @@ impl Scheme for FbbootlogScheme { ); if let Some(map) = &self.display_map { - map.display_handle.sync_rects(&damage).unwrap(); + map.display_handle.sync_rect(damage).unwrap(); } } diff --git a/graphics/fbcond/src/display.rs b/graphics/fbcond/src/display.rs index 60d7a18218..d95d0c7789 100644 --- a/graphics/fbcond/src/display.rs +++ b/graphics/fbcond/src/display.rs @@ -55,9 +55,9 @@ impl Display { V1GraphicsHandle::from_file(display_file) } - pub fn sync_rects(&mut self, sync_rects: Vec) { + pub fn sync_rect(&mut self, sync_rect: Damage) { if let Some(map) = &self.map { - map.display_handle.sync_rects(&sync_rects).unwrap(); + map.display_handle.sync_rect(sync_rect).unwrap(); } } } diff --git a/graphics/fbcond/src/text.rs b/graphics/fbcond/src/text.rs index 11d4c7ca19..e503488bfc 100644 --- a/graphics/fbcond/src/text.rs +++ b/graphics/fbcond/src/text.rs @@ -132,7 +132,7 @@ impl TextScreen { &mut self.input, ); - self.display.sync_rects(damage); + self.display.sync_rect(damage); } Ok(buf.len()) diff --git a/graphics/graphics-ipc/src/v1.rs b/graphics/graphics-ipc/src/v1.rs index ddebae9c1a..2ff0ddf900 100644 --- a/graphics/graphics-ipc/src/v1.rs +++ b/graphics/graphics-ipc/src/v1.rs @@ -64,11 +64,11 @@ impl V1GraphicsHandle { Ok(()) } - pub fn sync_rects(&self, sync_rects: &[Damage]) -> io::Result<()> { + pub fn sync_rect(&self, sync_rect: Damage) -> io::Result<()> { libredox::call::write(self.file.as_raw_fd() as usize, unsafe { slice::from_raw_parts( - sync_rects.as_ptr() as *const u8, - sync_rects.len() * mem::size_of::(), + ptr::addr_of!(sync_rect).cast::(), + mem::size_of::(), ) })?; Ok(()) diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 32e3a22436..d89b5e600a 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -34,12 +34,7 @@ impl GraphicsAdapter for FbAdapter { framebuffer.ptr.as_ptr().cast::() } - fn update_plane( - &mut self, - display_id: usize, - framebuffer: &Self::Framebuffer, - damage: &[Damage], - ) { + fn update_plane(&mut self, display_id: usize, framebuffer: &Self::Framebuffer, damage: Damage) { framebuffer.sync(&mut self.framebuffers[display_id], damage) } } @@ -89,29 +84,27 @@ impl Framebuffer for GraphicScreen { } impl GraphicScreen { - fn sync(&self, framebuffer: &mut FrameBuffer, sync_rects: &[Damage]) { - for sync_rect in sync_rects { - let sync_rect = sync_rect.clip( - self.width.try_into().unwrap(), - self.height.try_into().unwrap(), - ); + fn sync(&self, framebuffer: &mut FrameBuffer, sync_rect: Damage) { + let sync_rect = sync_rect.clip( + self.width.try_into().unwrap(), + self.height.try_into().unwrap(), + ); - let start_x: usize = sync_rect.x.try_into().unwrap(); - let start_y: usize = sync_rect.y.try_into().unwrap(); - let w: usize = sync_rect.width.try_into().unwrap(); - let h: usize = sync_rect.height.try_into().unwrap(); + let start_x: usize = sync_rect.x.try_into().unwrap(); + let start_y: usize = sync_rect.y.try_into().unwrap(); + let w: usize = sync_rect.width.try_into().unwrap(); + let h: usize = sync_rect.height.try_into().unwrap(); - let offscreen_ptr = self.ptr.as_ptr() as *mut u32; - let onscreen_ptr = framebuffer.onscreen as *mut u32; // FIXME use as_mut_ptr once stable + let offscreen_ptr = self.ptr.as_ptr() as *mut u32; + let onscreen_ptr = framebuffer.onscreen as *mut u32; // FIXME use as_mut_ptr once stable - for row in start_y..start_y + h { - unsafe { - ptr::copy( - offscreen_ptr.add(row * self.width + start_x), - onscreen_ptr.add(row * framebuffer.stride + start_x), - w, - ); - } + for row in start_y..start_y + h { + unsafe { + ptr::copy( + offscreen_ptr.add(row * self.width + start_x), + onscreen_ptr.add(row * framebuffer.stride + start_x), + w, + ); } } } diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index b6eda2e2b0..41ee3c31ca 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -170,12 +170,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { framebuffer.sgl.as_ptr() } - fn update_plane( - &mut self, - display_id: usize, - framebuffer: &Self::Framebuffer, - damage: &[Damage], - ) { + fn update_plane(&mut self, display_id: usize, framebuffer: &Self::Framebuffer, damage: Damage) { futures::executor::block_on(async { let req = Dma::new(XferToHost2d::new( framebuffer.id, @@ -204,14 +199,12 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { self.displays[display_id].active_resource = Some(framebuffer.id); } - for damage in damage { - let flush = ResourceFlush::new( - framebuffer.id, - damage.clip(framebuffer.width, framebuffer.height).into(), - ); - let header = self.send_request(Dma::new(flush).unwrap()).await.unwrap(); - assert_eq!(header.ty, CommandTy::RespOkNodata); - } + let flush = ResourceFlush::new( + framebuffer.id, + damage.clip(framebuffer.width, framebuffer.height).into(), + ); + let header = self.send_request(Dma::new(flush).unwrap()).await.unwrap(); + assert_eq!(header.ty, CommandTy::RespOkNodata); }); } }