From 03cf0955d240ecdebc6c03f263207238a18b5a69 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 4 Apr 2025 20:04:14 +0200 Subject: [PATCH 1/4] Move cursor resource writing to driver-graphics --- graphics/driver-graphics/src/lib.rs | 31 +++++++++++++++++++++++++++-- graphics/vesad/src/scheme.rs | 4 ++++ graphics/virtio-gpud/src/scheme.rs | 29 +++++---------------------- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index f79d2c2d35..9f0b9c7a60 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -21,6 +21,7 @@ pub trait GraphicsAdapter { fn supports_hw_cursor(&self) -> bool; fn create_cursor_framebuffer(&mut self) -> Self::Cursor; + fn map_cursor_framebuffer(&mut self, cursor: &Self::Cursor) -> *mut u8; fn handle_cursor(&mut self, cursor_damage: CursorDamage, cursor_resource: &mut Self::Cursor); } @@ -247,10 +248,36 @@ impl Scheme for GraphicsScheme { let cursor_damage = unsafe { *buf.as_ptr().cast::() }; //There is always expected to be cursor_resource if supports_hw_cursor returns true - if let Some(cursor_resource) = self.cursor_resources.get_mut(vt) { - self.adapter.handle_cursor(cursor_damage, cursor_resource); + let cursor_resource = self.cursor_resources.get_mut(vt).unwrap(); + + if cursor_damage.header != 0 { + let w: i32 = cursor_damage.width; + let h: i32 = cursor_damage.height; + let cursor_image = cursor_damage.cursor_img_bytes; + let cursor_ptr = self.adapter.map_cursor_framebuffer(cursor_resource); + + //Clear previous image from backing storage + unsafe { + core::ptr::write_bytes(cursor_ptr as *mut u8, 0, 64 * 64 * 4); + } + + //Write image to backing storage + for row in 0..h { + let start: usize = (w * row) as usize; + let end: usize = (w * row + w) as usize; + + unsafe { + core::ptr::copy_nonoverlapping( + cursor_image[start..end].as_ptr(), + cursor_ptr.cast::().offset(64 * row as isize), + w as usize, + ); + } + } } + self.adapter.handle_cursor(cursor_damage, cursor_resource); + return Ok(buf.len()); } diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 4b23b27893..2336272cb0 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -51,6 +51,10 @@ impl GraphicsAdapter for FbAdapter { unimplemented!("Vesad does not support this function"); } + fn map_cursor_framebuffer(&mut self, _cursor: &Self::Cursor) -> *mut u8 { + unimplemented!("Vesad does not support this function"); + } + fn handle_cursor(&mut self, _cursor_damage: CursorDamage, _cursor_resource: &mut VesadCursor) { unimplemented!("Vesad does not support this function"); } diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 459e29c367..2657df38e4 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -101,35 +101,12 @@ impl VirtGpuAdapter<'_> { Ok(response) } - fn update_cursor(&mut self, cursor_damage: CursorDamage, cursor: &mut VirtGpuCursor) { + fn update_cursor(&mut self, cursor_damage: CursorDamage, cursor: &VirtGpuCursor) { let x = cursor_damage.x; let y = cursor_damage.y; let hot_x = cursor_damage.hot_x; let hot_y = cursor_damage.hot_y; - let w: i32 = cursor_damage.width; - let h: i32 = cursor_damage.height; - let cursor_image = cursor_damage.cursor_img_bytes; - - //Clear previous image from backing storage - unsafe { - core::ptr::write_bytes(cursor.sgl.as_ptr() as *mut u8, 0, 64 * 64 * 4); - } - - //Write image to backing storage - for row in 0..h { - let start: usize = (w * row) as usize; - let end: usize = (w * row + w) as usize; - - unsafe { - core::ptr::copy_nonoverlapping( - cursor_image[start..end].as_ptr(), - (cursor.sgl.as_ptr() as *mut u32).offset(64 * row as isize), - w as usize, - ); - } - } - //Transfering cursor resource to host futures::executor::block_on(async { let transfer_request = Dma::new(XferToHost2d::new( @@ -367,6 +344,10 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { } } + fn map_cursor_framebuffer(&mut self, cursor: &Self::Cursor) -> *mut u8 { + cursor.sgl.as_ptr() + } + fn handle_cursor(&mut self, cursor_damage: CursorDamage, cursor: &mut VirtGpuCursor) { if !cursor.set { cursor.set = true; From abab5c9a3c66d58969b0c5fde6b05d3db8c87061 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 4 Apr 2025 20:25:57 +0200 Subject: [PATCH 2/4] Avoid passing unused fields to VIRTIO_GPU_CMD_MOVE_CURSOR --- graphics/virtio-gpud/src/main.rs | 8 ++++---- graphics/virtio-gpud/src/scheme.rs | 18 +++--------------- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/graphics/virtio-gpud/src/main.rs b/graphics/virtio-gpud/src/main.rs index ea2b67eafe..7f95d9d572 100644 --- a/graphics/virtio-gpud/src/main.rs +++ b/graphics/virtio-gpud/src/main.rs @@ -416,13 +416,13 @@ pub struct MoveCursor { } impl MoveCursor { - pub fn move_cursor(x: i32, y: i32, hot_x: i32, hot_y: i32, resource_id: ResourceId) -> Self { + pub fn move_cursor(x: i32, y: i32) -> Self { Self { header: ControlHeader::with_ty(CommandTy::MoveCursor), pos: CursorPos::new(0, x, y), - resource_id, - hot_x, - hot_y, + resource_id: ResourceId(0), + hot_x: 0, + hot_y: 0, _padding: 0, } } diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 2657df38e4..606a768c3c 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -139,20 +139,8 @@ impl VirtGpuAdapter<'_> { }); } - fn move_cursor(&self, cursor_damage: CursorDamage, cursor: &mut VirtGpuCursor) { - let x = cursor_damage.x; - let y = cursor_damage.y; - let hot_x = cursor_damage.hot_x; - let hot_y = cursor_damage.hot_y; - - let request = Dma::new(MoveCursor::move_cursor( - x, - y, - hot_x, - hot_y, - cursor.resource_id, - )) - .unwrap(); + fn move_cursor(&self, cursor_damage: CursorDamage) { + let request = Dma::new(MoveCursor::move_cursor(cursor_damage.x, cursor_damage.y)).unwrap(); futures::executor::block_on(async { let command = ChainBuilder::new().chain(Buffer::new(&request)).build(); @@ -355,7 +343,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { } if cursor_damage.header == 0 { - self.move_cursor(cursor_damage, cursor); + self.move_cursor(cursor_damage); } else { self.update_cursor(cursor_damage, cursor); } From 215b8185c243d8de60b22062aeb36480aeaceb3a Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 4 Apr 2025 20:34:42 +0200 Subject: [PATCH 3/4] Handle vt switching for cursors without requiring a cursor update This requires storing the cursor state on the driver-graphics side to be be able to restore the correct state for the target vt. --- graphics/driver-graphics/src/lib.rs | 70 ++++++++++++++++++++++------- graphics/vesad/src/scheme.rs | 8 ++-- graphics/virtio-gpud/src/scheme.rs | 42 ++++++++--------- 3 files changed, 77 insertions(+), 43 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index 9f0b9c7a60..b52dfb5039 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -9,7 +9,7 @@ use syscall::{Error, MapFlags, Result, EAGAIN, EBADF, EINVAL}; pub trait GraphicsAdapter { type Framebuffer: Framebuffer; - type Cursor: Cursor; + type Cursor: CursorFramebuffer; fn displays(&self) -> Vec; fn display_size(&self, display_id: usize) -> (u32, u32); @@ -22,7 +22,7 @@ pub trait GraphicsAdapter { fn supports_hw_cursor(&self) -> bool; fn create_cursor_framebuffer(&mut self) -> Self::Cursor; fn map_cursor_framebuffer(&mut self, cursor: &Self::Cursor) -> *mut u8; - fn handle_cursor(&mut self, cursor_damage: CursorDamage, cursor_resource: &mut Self::Cursor); + fn handle_cursor(&mut self, cursor: &mut CursorPlane, dirty_fb: bool); } pub trait Framebuffer { @@ -30,7 +30,15 @@ pub trait Framebuffer { fn height(&self) -> u32; } -pub trait Cursor {} +pub struct CursorPlane { + pub x: i32, + pub y: i32, + pub hot_x: i32, + pub hot_y: i32, + pub framebuffer: C, +} + +pub trait CursorFramebuffer {} pub struct GraphicsScheme { adapter: T, @@ -42,7 +50,7 @@ pub struct GraphicsScheme { active_vt: usize, vts_fb: HashMap>, - cursor_resources: HashMap, + cursor_planes: HashMap>, } enum Handle { @@ -62,7 +70,7 @@ impl GraphicsScheme { handles: BTreeMap::new(), active_vt: 0, vts_fb: HashMap::new(), - cursor_resources: HashMap::new(), + cursor_planes: HashMap::new(), } } @@ -96,6 +104,15 @@ impl GraphicsScheme { Self::update_whole_screen(&mut self.adapter, display_id, framebuffer); self.active_vt = vt_event.vt; + + if self.adapter.supports_hw_cursor() { + let cursor_plane = Self::cursor_plane_for_vt( + &mut self.adapter, + &mut self.cursor_planes, + self.active_vt, + ); + self.adapter.handle_cursor(cursor_plane, true); + } } } @@ -150,6 +167,20 @@ impl GraphicsScheme { }, ); } + + fn cursor_plane_for_vt<'a>( + adapter: &mut T, + cursor_planes: &'a mut HashMap>, + vt: usize, + ) -> &'a mut CursorPlane { + cursor_planes.entry(vt).or_insert_with(|| CursorPlane { + x: 0, + y: 0, + hot_x: 0, + hot_y: 0, + framebuffer: adapter.create_cursor_framebuffer(), + }) + } } impl Scheme for GraphicsScheme { @@ -177,11 +208,6 @@ impl Scheme for GraphicsScheme { self.adapter.create_dumb_framebuffer(width, height) }); - if self.adapter.supports_hw_cursor() { - self.cursor_resources - .insert(vt, self.adapter.create_cursor_framebuffer()); - } - self.next_id += 1; self.handles .insert(self.next_id, Handle::Screen { vt, screen: id }); @@ -247,14 +273,25 @@ impl Scheme for GraphicsScheme { { let cursor_damage = unsafe { *buf.as_ptr().cast::() }; - //There is always expected to be cursor_resource if supports_hw_cursor returns true - let cursor_resource = self.cursor_resources.get_mut(vt).unwrap(); + let cursor_plane = Self::cursor_plane_for_vt( + &mut self.adapter, + &mut self.cursor_planes, + self.active_vt, + ); + + cursor_plane.x = cursor_damage.x; + cursor_plane.y = cursor_damage.y; + + if cursor_damage.header == 0 { + self.adapter.handle_cursor(cursor_plane, false); + } else { + cursor_plane.hot_x = cursor_damage.hot_x; + cursor_plane.hot_y = cursor_damage.hot_y; - if cursor_damage.header != 0 { let w: i32 = cursor_damage.width; let h: i32 = cursor_damage.height; let cursor_image = cursor_damage.cursor_img_bytes; - let cursor_ptr = self.adapter.map_cursor_framebuffer(cursor_resource); + let cursor_ptr = self.adapter.map_cursor_framebuffer(&cursor_plane.framebuffer); //Clear previous image from backing storage unsafe { @@ -274,9 +311,10 @@ impl Scheme for GraphicsScheme { ); } } - } - self.adapter.handle_cursor(cursor_damage, cursor_resource); + self.adapter + .handle_cursor(cursor_plane, true); + } return Ok(buf.len()); } diff --git a/graphics/vesad/src/scheme.rs b/graphics/vesad/src/scheme.rs index 2336272cb0..f8c4249ee5 100644 --- a/graphics/vesad/src/scheme.rs +++ b/graphics/vesad/src/scheme.rs @@ -2,8 +2,8 @@ use std::alloc::{self, Layout}; use std::convert::TryInto; use std::ptr::{self, NonNull}; -use driver_graphics::{Cursor, Framebuffer, GraphicsAdapter}; -use graphics_ipc::v1::{CursorDamage, Damage}; +use driver_graphics::{CursorFramebuffer, CursorPlane, Framebuffer, GraphicsAdapter}; +use graphics_ipc::v1::Damage; use syscall::PAGE_SIZE; use crate::framebuffer::FrameBuffer; @@ -14,7 +14,7 @@ pub struct FbAdapter { pub enum VesadCursor {} -impl Cursor for VesadCursor {} +impl CursorFramebuffer for VesadCursor {} impl GraphicsAdapter for FbAdapter { type Framebuffer = GraphicScreen; @@ -55,7 +55,7 @@ impl GraphicsAdapter for FbAdapter { unimplemented!("Vesad does not support this function"); } - fn handle_cursor(&mut self, _cursor_damage: CursorDamage, _cursor_resource: &mut VesadCursor) { + fn handle_cursor(&mut self, _cursor: &mut CursorPlane, _dirty_fb: bool) { unimplemented!("Vesad does not support this function"); } } diff --git a/graphics/virtio-gpud/src/scheme.rs b/graphics/virtio-gpud/src/scheme.rs index 606a768c3c..86fc9c9fc9 100644 --- a/graphics/virtio-gpud/src/scheme.rs +++ b/graphics/virtio-gpud/src/scheme.rs @@ -1,8 +1,10 @@ use std::sync::Arc; use common::{dma::Dma, sgl}; -use driver_graphics::{Cursor, Framebuffer, GraphicsAdapter, GraphicsScheme}; -use graphics_ipc::v1::{CursorDamage, Damage}; +use driver_graphics::{ + CursorFramebuffer, CursorPlane, Framebuffer, GraphicsAdapter, GraphicsScheme, +}; +use graphics_ipc::v1::Damage; use inputd::DisplayHandle; use syscall::PAGE_SIZE; @@ -43,10 +45,9 @@ impl Framebuffer for VirtGpuFramebuffer { pub struct VirtGpuCursor { resource_id: ResourceId, sgl: sgl::Sgl, - set: bool, } -impl Cursor for VirtGpuCursor {} +impl CursorFramebuffer for VirtGpuCursor {} #[derive(Debug, Copy, Clone)] pub struct Display { @@ -101,12 +102,7 @@ impl VirtGpuAdapter<'_> { Ok(response) } - fn update_cursor(&mut self, cursor_damage: CursorDamage, cursor: &VirtGpuCursor) { - let x = cursor_damage.x; - let y = cursor_damage.y; - let hot_x = cursor_damage.hot_x; - let hot_y = cursor_damage.hot_y; - + fn update_cursor(&mut self, cursor: &VirtGpuCursor, x: i32, y: i32, hot_x: i32, hot_y: i32) { //Transfering cursor resource to host futures::executor::block_on(async { let transfer_request = Dma::new(XferToHost2d::new( @@ -139,8 +135,8 @@ impl VirtGpuAdapter<'_> { }); } - fn move_cursor(&self, cursor_damage: CursorDamage) { - let request = Dma::new(MoveCursor::move_cursor(cursor_damage.x, cursor_damage.y)).unwrap(); + fn move_cursor(&mut self, x: i32, y: i32) { + let request = Dma::new(MoveCursor::move_cursor(x, y)).unwrap(); futures::executor::block_on(async { let command = ChainBuilder::new().chain(Buffer::new(&request)).build(); @@ -327,8 +323,7 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { VirtGpuCursor { resource_id: res_id, - sgl: sgl, - set: false, + sgl, } } @@ -336,16 +331,17 @@ impl GraphicsAdapter for VirtGpuAdapter<'_> { cursor.sgl.as_ptr() } - fn handle_cursor(&mut self, cursor_damage: CursorDamage, cursor: &mut VirtGpuCursor) { - if !cursor.set { - cursor.set = true; - self.update_cursor(cursor_damage, cursor); - } - - if cursor_damage.header == 0 { - self.move_cursor(cursor_damage); + fn handle_cursor(&mut self, cursor: &mut CursorPlane, dirty_fb: bool) { + if dirty_fb { + self.update_cursor( + &cursor.framebuffer, + cursor.x, + cursor.y, + cursor.hot_x, + cursor.hot_y, + ); } else { - self.update_cursor(cursor_damage, cursor); + self.move_cursor(cursor.x, cursor.y); } } } From f1b5ba52eda2ac308b53f88f940218c78ca718c5 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 4 Apr 2025 20:41:31 +0200 Subject: [PATCH 4/4] Rustfmt --- graphics/driver-graphics/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/graphics/driver-graphics/src/lib.rs b/graphics/driver-graphics/src/lib.rs index b52dfb5039..3ef5c0874e 100644 --- a/graphics/driver-graphics/src/lib.rs +++ b/graphics/driver-graphics/src/lib.rs @@ -291,7 +291,9 @@ impl Scheme for GraphicsScheme { let w: i32 = cursor_damage.width; let h: i32 = cursor_damage.height; let cursor_image = cursor_damage.cursor_img_bytes; - let cursor_ptr = self.adapter.map_cursor_framebuffer(&cursor_plane.framebuffer); + let cursor_ptr = self + .adapter + .map_cursor_framebuffer(&cursor_plane.framebuffer); //Clear previous image from backing storage unsafe { @@ -312,8 +314,7 @@ impl Scheme for GraphicsScheme { } } - self.adapter - .handle_cursor(cursor_plane, true); + self.adapter.handle_cursor(cursor_plane, true); } return Ok(buf.len());