From f2156d4654b9c598c331f7d641e2647bbeb6fb18 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 23 Dec 2024 16:22:41 +0100 Subject: [PATCH] graphics: Optimize the damage buffer api It is now possible to write multiple damage locations in a single write call. Vesad also no longer processes damage for background vt's. The entire framebuffer will be redrawn once the vt switches to the foreground anyway. And finally fsync now consistently redraws the entire screen rather than having different behavior between vesad and virtio-gpud. --- graphics/fbcond/src/display.rs | 21 ++++++++--------- graphics/vesad/src/scheme.rs | 10 +++++---- graphics/vesad/src/screen.rs | 28 ++++++++++++----------- graphics/virtio-gpud/src/scheme.rs | 36 ++++++++++++++++-------------- 4 files changed, 49 insertions(+), 46 deletions(-) diff --git a/graphics/fbcond/src/display.rs b/graphics/fbcond/src/display.rs index ceaf48da44..bf6c435182 100644 --- a/graphics/fbcond/src/display.rs +++ b/graphics/fbcond/src/display.rs @@ -129,19 +129,16 @@ impl Display { } } DisplayCommand::SyncRects(sync_rects) => { - for sync_rect in sync_rects { - unsafe { - libredox::call::write( - display_file.as_raw_fd() as usize, - slice::from_raw_parts( - &sync_rect as *const Damage as *const u8, - mem::size_of::(), - ), - ) - .unwrap(); - } + unsafe { + libredox::call::write( + display_file.as_raw_fd() as usize, + slice::from_raw_parts( + sync_rects.as_ptr() as *const u8, + sync_rects.len() * mem::size_of::(), + ), + ) + .unwrap(); } - libredox::call::fsync(display_file.as_raw_fd() as usize).unwrap(); } } } diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 84dcb3ec39..d2127e3a68 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -213,7 +213,7 @@ impl SchemeBlockMut for DisplayScheme { if let Some(screens) = self.vts.get_mut(&handle.vt) { if let Some(screen) = screens.get_mut(&handle.screen) { if handle.vt == self.active { - screen.sync(&mut self.framebuffers[handle.screen.0]); + screen.redraw(&mut self.framebuffers[handle.screen.0]); } return Ok(Some(0)); } @@ -260,11 +260,13 @@ impl SchemeBlockMut for DisplayScheme { if let Some(screens) = self.vts.get_mut(&handle.vt) { if let Some(screen) = screens.get_mut(&handle.screen) { - let count = screen.write(buf)?; if handle.vt == self.active { - screen.sync(&mut self.framebuffers[handle.screen.0]); + screen + .write(buf, Some(&mut self.framebuffers[handle.screen.0])) + .map(|count| Some(count)) + } else { + screen.write(buf, None).map(|count| Some(count)) } - Ok(Some(count)) } else { Err(Error::new(EBADF)) } diff --git a/graphics/vesad/src/screen.rs b/graphics/vesad/src/screen.rs index 2b86e83d28..d4a26c07c2 100644 --- a/graphics/vesad/src/screen.rs +++ b/graphics/vesad/src/screen.rs @@ -14,7 +14,6 @@ pub struct GraphicScreen { pub height: usize, pub offscreen: OffscreenBuffer, pub input: VecDeque, - pub sync_rects: Vec, } impl GraphicScreen { @@ -24,7 +23,6 @@ impl GraphicScreen { height, offscreen: OffscreenBuffer::new(width * height), input: VecDeque::new(), - sync_rects: Vec::new(), } } } @@ -105,7 +103,7 @@ impl GraphicScreen { !self.input.is_empty() } - pub fn write(&mut self, buf: &[u8]) -> Result { + pub fn write(&mut self, buf: &[u8], framebuffer: Option<&mut FrameBuffer>) -> Result { let sync_rects = unsafe { slice::from_raw_parts( buf.as_ptr() as *const Damage, @@ -113,13 +111,15 @@ impl GraphicScreen { ) }; - self.sync_rects.extend_from_slice(sync_rects); + if let Some(framebuffer) = framebuffer { + self.sync(framebuffer, sync_rects); + } Ok(sync_rects.len() * mem::size_of::()) } - pub fn sync(&mut self, framebuffer: &mut FrameBuffer) { - for sync_rect in self.sync_rects.drain(..) { + pub fn sync(&mut self, framebuffer: &mut FrameBuffer, sync_rects: &[Damage]) { + for sync_rect in sync_rects { let x = sync_rect.x.try_into().unwrap_or(0); let y = sync_rect.y.try_into().unwrap_or(0); let w = sync_rect.width.try_into().unwrap_or(0); @@ -152,12 +152,14 @@ impl GraphicScreen { pub fn redraw(&mut self, framebuffer: &mut FrameBuffer) { let width = self.width.try_into().unwrap(); let height = self.height.try_into().unwrap(); - self.sync_rects.push(Damage { - x: 0, - y: 0, - width, - height, - }); - self.sync(framebuffer); + self.sync( + framebuffer, + &[Damage { + x: 0, + y: 0, + width, + height, + }], + ); } } diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 97c4a0f031..d34c20cda1 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -177,7 +177,7 @@ impl<'a> Display<'a> { } /// If `damage` is `None`, the entire screen is flushed. - async fn flush(&self, vt: usize, damage: Option<&Damage>) -> Result<(), Error> { + async fn flush(&self, vt: usize, damage: Option<&[Damage]>) -> Result<(), Error> { let Some(&res_id) = self.vts_res.borrow().get(&vt) else { // The resource is not yet created. Ignore the damage. We will write the entire backing // storage to the resource once we create the resource, which is equivalent to damaging @@ -185,17 +185,6 @@ impl<'a> Display<'a> { return Ok(()); }; - let damage = if let Some(damage) = damage { - damage.into() - } else { - GpuRect { - x: 0, - y: 0, - width: self.width, - height: self.height, - } - }; - let req = Dma::new(XferToHost2d::new( res_id, GpuRect { @@ -208,8 +197,23 @@ impl<'a> Display<'a> { let header = self.send_request(req).await?; assert_eq!(header.ty.get(), CommandTy::RespOkNodata); - self.flush_resource(ResourceFlush::new(res_id, damage.clone())) + if let Some(damage) = damage { + for damage in damage { + self.flush_resource(ResourceFlush::new(res_id, damage.into())) + .await?; + } + } else { + self.flush_resource(ResourceFlush::new( + res_id, + GpuRect { + x: 0, + y: 0, + width: self.width, + height: self.height, + }, + )) .await?; + } Ok(()) } } @@ -387,16 +391,14 @@ impl<'a> SchemeMut for Scheme<'a> { ) -> syscall::Result { let handle = self.handles.get(&id).ok_or(SysError::new(EINVAL))?; - let damages = unsafe { + let damage = unsafe { core::slice::from_raw_parts( buf.as_ptr() as *const Damage, buf.len() / core::mem::size_of::(), ) }; - for damage in damages { - futures::executor::block_on(handle.display.flush(handle.vt, Some(damage))).unwrap(); - } + futures::executor::block_on(handle.display.flush(handle.vt, Some(damage))).unwrap(); Ok(buf.len()) }