From 0e98890de6fa53f3d8da47a6166064d35a6f3130 Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Sun, 28 Jul 2024 17:52:52 +0200 Subject: [PATCH 1/2] jay_screenshot: add support for multiple planes --- src/cli/screenshot.rs | 89 +++++++++++++++++++------- src/ifs/jay_compositor.rs | 38 +++++++---- src/ifs/jay_screenshot.rs | 40 +++++++++++- src/it/test_client.rs | 10 +-- src/it/test_ifs/test_jay_compositor.rs | 11 +++- src/it/test_ifs/test_registry.rs | 2 +- src/it/test_ifs/test_screenshot.rs | 65 +++++++++++++++++-- src/it/testrun.rs | 2 +- src/tools/tool_client.rs | 2 +- wire/jay_screenshot.txt | 19 +++++- 10 files changed, 217 insertions(+), 61 deletions(-) diff --git a/src/cli/screenshot.rs b/src/cli/screenshot.rs index 42379582..047f30b3 100644 --- a/src/cli/screenshot.rs +++ b/src/cli/screenshot.rs @@ -11,13 +11,14 @@ use { }, wire::{ jay_compositor::TakeScreenshot, - jay_screenshot::{Dmabuf, Error}, + jay_screenshot::{Dmabuf, Error, Format, Plane}, }, }, chrono::Local, jay_algorithms::qoi::xrgb8888_encode_qoi, png::{BitDepth, ColorType, Encoder, SrgbRenderingIntent}, - std::rc::Rc, + std::{cell::RefCell, mem, rc::Rc}, + uapi::OwnedFd, }; pub fn main(global: GlobalArgs, args: ScreenshotArgs) { @@ -48,16 +49,60 @@ async fn run(screenshot: Rc) { res.push(Err(err.msg.to_owned())); }); Dmabuf::handle(tc, sid, result.clone(), |res, buf| { - res.push(Ok(buf)); + let mut planes = PlaneVec::new(); + planes.push(DmaBufPlane { + offset: buf.offset, + stride: buf.stride, + fd: buf.fd, + }); + let dmabuf = DmaBuf { + id: DmaBufIds::default().next(), + width: buf.width as _, + height: buf.height as _, + format: XRGB8888, + modifier: buf.modifier_lo as u64 | ((buf.modifier_hi as u64) << 32), + planes, + }; + res.push(Ok(ScreenshotWithDevice { + dev: buf.drm_dev, + buf: dmabuf, + })); + }); + let planes = Rc::new(RefCell::new(PlaneVec::new())); + Plane::handle(tc, sid, planes.clone(), |planes, buf| { + planes.borrow_mut().push(DmaBufPlane { + offset: buf.offset, + stride: buf.stride, + fd: buf.fd, + }); }); - let buf = match result.pop().await { + Format::handle( + tc, + sid, + (planes.clone(), result.clone()), + |(planes, res), buf| { + let dmabuf = DmaBuf { + id: DmaBufIds::default().next(), + width: buf.width as _, + height: buf.height as _, + format: XRGB8888, + modifier: buf.modifier_lo as u64 | ((buf.modifier_hi as u64) << 32), + planes: mem::take(&mut *planes.borrow_mut()), + }; + res.push(Ok(ScreenshotWithDevice { + dev: buf.drm_dev, + buf: dmabuf, + })); + }, + ); + let shot = match result.pop().await { Ok(b) => b, Err(e) => { fatal!("Could not take a screenshot: {}", e); } }; let format = screenshot.args.format; - let data = buf_to_bytes(&DmaBufIds::default(), &buf, format); + let data = buf_to_bytes(&shot.dev, &shot.buf, format); let filename = match &screenshot.args.filename { Some(f) => f.clone(), _ => { @@ -74,8 +119,13 @@ async fn run(screenshot: Rc) { } } -pub fn buf_to_bytes(dma_buf_ids: &DmaBufIds, buf: &Dmabuf, format: ScreenshotFormat) -> Vec { - let drm = match Drm::reopen(buf.drm_dev.raw(), false) { +pub struct ScreenshotWithDevice { + pub dev: Rc, + pub buf: DmaBuf, +} + +pub fn buf_to_bytes(dev: &Rc, buf: &DmaBuf, format: ScreenshotFormat) -> Vec { + let drm = match Drm::reopen(dev.raw(), false) { Ok(drm) => drm, Err(e) => { fatal!("Could not open the drm device: {}", ErrorFmt(e)); @@ -87,21 +137,7 @@ pub fn buf_to_bytes(dma_buf_ids: &DmaBufIds, buf: &Dmabuf, format: ScreenshotFor fatal!("Could not create a gbm device: {}", ErrorFmt(e)); } }; - let mut planes = PlaneVec::new(); - planes.push(DmaBufPlane { - offset: buf.offset, - stride: buf.stride, - fd: buf.fd.clone(), - }); - let dmabuf = DmaBuf { - id: dma_buf_ids.next(), - width: buf.width as _, - height: buf.height as _, - format: XRGB8888, - modifier: (buf.modifier_hi as u64) << 32 | (buf.modifier_lo as u64), - planes, - }; - let bo = match gbm.import_dmabuf(&dmabuf, GBM_BO_USE_LINEAR | GBM_BO_USE_RENDERING) { + let bo = match gbm.import_dmabuf(&buf, GBM_BO_USE_LINEAR | GBM_BO_USE_RENDERING) { Ok(bo) => Rc::new(bo), Err(e) => { fatal!("Could not import screenshot dmabuf: {}", ErrorFmt(e)); @@ -115,7 +151,12 @@ pub fn buf_to_bytes(dma_buf_ids: &DmaBufIds, buf: &Dmabuf, format: ScreenshotFor }; let data = unsafe { bo_map.data() }; if format == ScreenshotFormat::Qoi { - return xrgb8888_encode_qoi(data, buf.width, buf.height, bo_map.stride() as u32); + return xrgb8888_encode_qoi( + data, + buf.width as _, + buf.height as _, + bo_map.stride() as u32, + ); } let mut out = vec![]; @@ -128,7 +169,7 @@ pub fn buf_to_bytes(dma_buf_ids: &DmaBufIds, buf: &Dmabuf, format: ScreenshotFor image_data.extend_from_slice(&[pixel[2], pixel[1], pixel[0], 255]) } } - let mut encoder = Encoder::new(&mut out, buf.width, buf.height); + let mut encoder = Encoder::new(&mut out, buf.width as _, buf.height as _); encoder.set_color(ColorType::Rgba); encoder.set_depth(BitDepth::Eight); encoder.set_srgb(SrgbRenderingIntent::Perceptual); diff --git a/src/ifs/jay_compositor.rs b/src/ifs/jay_compositor.rs index dd778d0c..be718763 100644 --- a/src/ifs/jay_compositor.rs +++ b/src/ifs/jay_compositor.rs @@ -13,7 +13,7 @@ use { jay_randr::JayRandr, jay_render_ctx::JayRenderCtx, jay_screencast::JayScreencast, - jay_screenshot::JayScreenshot, + jay_screenshot::{JayScreenshot, PLANES_SINCE}, jay_seat_events::JaySeatEvents, jay_select_toplevel::{JaySelectToplevel, JayToplevelSelector}, jay_select_workspace::{JaySelectWorkspace, JayWorkspaceSelector}, @@ -69,7 +69,7 @@ impl Global for JayCompositorGlobal { } fn version(&self) -> u32 { - 5 + 6 } fn required_caps(&self) -> ClientCaps { @@ -111,29 +111,41 @@ impl JayCompositor { id, client: self.client.clone(), tracker: Default::default(), + version: self.version, + bo: Cell::new(None), }); track!(self.client, ss); self.client.add_client_obj(&ss)?; match take_screenshot(&self.client.state, include_cursor) { Ok(s) => { let dmabuf = s.bo.dmabuf(); - let plane = &dmabuf.planes[0]; - ss.send_dmabuf( - &s.drm, - &plane.fd, - dmabuf.width, - dmabuf.height, - plane.offset, - plane.stride, - dmabuf.modifier, - ); + if ss.version < PLANES_SINCE { + let plane = &dmabuf.planes[0]; + ss.send_dmabuf( + &s.drm, + &plane.fd, + dmabuf.width, + dmabuf.height, + plane.offset, + plane.stride, + dmabuf.modifier, + ); + } else { + for plane in &dmabuf.planes { + ss.send_plane(&plane); + } + ss.send_format(&s.drm, dmabuf); + } + ss.bo.set(Some(s.bo)); } Err(e) => { let msg = ErrorFmt(e).to_string(); ss.send_error(&msg); } } - self.client.remove_obj(ss.deref())?; + if ss.version < PLANES_SINCE { + self.client.remove_obj(ss.deref())?; + } Ok(()) } } diff --git a/src/ifs/jay_screenshot.rs b/src/ifs/jay_screenshot.rs index 5695067f..39a0b878 100644 --- a/src/ifs/jay_screenshot.rs +++ b/src/ifs/jay_screenshot.rs @@ -1,18 +1,26 @@ use { crate::{ - client::Client, + client::{Client, ClientError}, leaks::Tracker, object::{Object, Version}, + video::{ + dmabuf::{DmaBuf, DmaBufPlane}, + gbm::GbmBo, + }, wire::{jay_screenshot::*, JayScreenshotId}, }, - std::{convert::Infallible, rc::Rc}, + std::{cell::Cell, rc::Rc}, uapi::OwnedFd, }; +pub const PLANES_SINCE: Version = Version(6); + pub struct JayScreenshot { pub id: JayScreenshotId, pub client: Rc, pub tracker: Tracker, + pub version: Version, + pub bo: Cell>, } impl JayScreenshot { @@ -45,10 +53,36 @@ impl JayScreenshot { msg, }); } + + pub fn send_plane(&self, plane: &DmaBufPlane) { + self.client.event(Plane { + self_id: self.id, + fd: plane.fd.clone(), + offset: plane.offset, + stride: plane.stride, + }); + } + + pub fn send_format(&self, drm_dev: &Rc, dmabuf: &DmaBuf) { + self.client.event(Format { + self_id: self.id, + drm_dev: drm_dev.clone(), + format: dmabuf.format.drm, + width: dmabuf.width, + height: dmabuf.height, + modifier_lo: dmabuf.modifier as u32, + modifier_hi: (dmabuf.modifier >> 32) as u32, + }); + } } impl JayScreenshotRequestHandler for JayScreenshot { - type Error = Infallible; + type Error = ClientError; + + fn destroy(&self, _req: Destroy, _slf: &Rc) -> Result<(), Self::Error> { + self.client.remove_obj(self)?; + Ok(()) + } } object_base! { diff --git a/src/it/test_client.rs b/src/it/test_client.rs index fdfbad9b..a4e68983 100644 --- a/src/it/test_client.rs +++ b/src/it/test_client.rs @@ -25,7 +25,7 @@ use { pub struct TestClient { pub run: Rc, - pub server: Rc, + pub _server: Rc, pub tran: Rc, pub registry: Rc, pub jc: Rc, @@ -92,12 +92,8 @@ impl TestClient { } pub async fn take_screenshot(&self, include_cursor: bool) -> Result, TestError> { - let dmabuf = self.jc.take_screenshot(include_cursor).await?; - let qoi = buf_to_bytes( - &self.server.state.dma_buf_ids, - &dmabuf, - ScreenshotFormat::Qoi, - ); + let (shot, _ts) = self.jc.take_screenshot(include_cursor).await?; + let qoi = buf_to_bytes(&shot.dev, &shot.buf, ScreenshotFormat::Qoi); Ok(qoi) } diff --git a/src/it/test_ifs/test_jay_compositor.rs b/src/it/test_ifs/test_jay_compositor.rs index 53887664..2f57c939 100644 --- a/src/it/test_ifs/test_jay_compositor.rs +++ b/src/it/test_ifs/test_jay_compositor.rs @@ -1,5 +1,6 @@ use { crate::{ + cli::screenshot::ScreenshotWithDevice, client::ClientId, it::{ test_error::{TestError, TestResult}, @@ -11,7 +12,6 @@ use { utils::{buffd::MsgParser, cell_ext::CellExt}, wire::{ jay_compositor::{self, *}, - jay_screenshot::Dmabuf, JayCompositorId, }, }, @@ -49,10 +49,15 @@ impl TestJayCompositor { Ok(()) } - pub async fn take_screenshot(&self, include_cursor: bool) -> Result { + pub async fn take_screenshot( + &self, + include_cursor: bool, + ) -> Result<(ScreenshotWithDevice, Rc), TestError> { let js = Rc::new(TestJayScreenshot { + tran: self.tran.clone(), id: self.tran.id(), result: Cell::new(None), + planes: Default::default(), }); self.tran.send(TakeScreenshot2 { self_id: self.id, @@ -62,7 +67,7 @@ impl TestJayCompositor { self.tran.add_obj(js.clone())?; self.tran.sync().await; match js.result.take() { - Some(Ok(res)) => Ok(res), + Some(Ok(res)) => Ok((res, js)), Some(Err(res)) => bail!("Compositor could not take a screenshot: {}", res), None => bail!("Compositor did not send a screenshot"), } diff --git a/src/it/test_ifs/test_registry.rs b/src/it/test_ifs/test_registry.rs index 13286ee4..cead68b6 100644 --- a/src/it/test_ifs/test_registry.rs +++ b/src/it/test_ifs/test_registry.rs @@ -165,7 +165,7 @@ impl TestRegistry { get_jay_compositor, jay_compositor, jay_compositor, - 1, + 6, TestJayCompositor ); create_singleton!(get_compositor, compositor, wl_compositor, 6, TestCompositor); diff --git a/src/it/test_ifs/test_screenshot.rs b/src/it/test_ifs/test_screenshot.rs index e3d21d32..18c840f9 100644 --- a/src/it/test_ifs/test_screenshot.rs +++ b/src/it/test_ifs/test_screenshot.rs @@ -1,22 +1,35 @@ use { crate::{ - it::{test_error::TestError, test_object::TestObject, testrun::ParseFull}, + cli::screenshot::ScreenshotWithDevice, + format::formats, + it::{ + test_error::TestError, test_object::TestObject, test_transport::TestTransport, + testrun::ParseFull, + }, utils::buffd::MsgParser, + video::dmabuf::{DmaBuf, DmaBufPlane, PlaneVec}, wire::{jay_screenshot::*, JayScreenshotId}, }, - std::cell::Cell, + std::{ + cell::{Cell, RefCell}, + rc::Rc, + }, }; pub struct TestJayScreenshot { + pub tran: Rc, pub id: JayScreenshotId, - pub result: Cell>>, + pub result: Cell>>, + pub planes: RefCell>, } impl TestJayScreenshot { - fn handle_dmabuf(&self, parser: MsgParser<'_, '_>) -> Result<(), TestError> { - let ev = Dmabuf::parse_full(parser)?; - self.result.set(Some(Ok(ev))); - Ok(()) + fn destroy(&self) -> Result<(), TestError> { + self.tran.send(Destroy { self_id: self.id }) + } + + fn handle_dmabuf(&self, _parser: MsgParser<'_, '_>) -> Result<(), TestError> { + bail!("got dmabuf message") } fn handle_error(&self, parser: MsgParser<'_, '_>) -> Result<(), TestError> { @@ -24,6 +37,36 @@ impl TestJayScreenshot { self.result.set(Some(Err(ev.msg.to_string()))); Ok(()) } + + fn handle_plane(&self, parser: MsgParser<'_, '_>) -> Result<(), TestError> { + let ev = Plane::parse_full(parser)?; + self.planes.borrow_mut().push(DmaBufPlane { + offset: ev.offset, + stride: ev.stride, + fd: ev.fd, + }); + Ok(()) + } + + fn handle_format(&self, parser: MsgParser<'_, '_>) -> Result<(), TestError> { + let ev = Format::parse_full(parser)?; + let Some(format) = formats().get(&ev.format) else { + bail!("Unknown screenshot format {}", ev.format); + }; + let res = ScreenshotWithDevice { + dev: ev.drm_dev, + buf: DmaBuf { + id: self.tran.run.state.dma_buf_ids.next(), + width: ev.width, + height: ev.height, + format, + modifier: ev.modifier_lo as u64 | ((ev.modifier_hi as u64) << 32), + planes: self.planes.borrow_mut().take(), + }, + }; + self.result.set(Some(Ok(res))); + Ok(()) + } } test_object! { @@ -31,6 +74,14 @@ test_object! { DMABUF => handle_dmabuf, ERROR => handle_error, + PLANE => handle_plane, + FORMAT => handle_format, } impl TestObject for TestJayScreenshot {} + +impl Drop for TestJayScreenshot { + fn drop(&mut self) { + let _ = self.destroy(); + } +} diff --git a/src/it/testrun.rs b/src/it/testrun.rs index ad3a5b6f..9ad6b7f2 100644 --- a/src/it/testrun.rs +++ b/src/it/testrun.rs @@ -79,7 +79,7 @@ impl TestRun { let client = self.state.clients.get(client_id)?; Ok(Rc::new(TestClient { run: self.clone(), - server: client, + _server: client, tran, jc, comp: registry.get_compositor().await?, diff --git a/src/tools/tool_client.rs b/src/tools/tool_client.rs index 5468132a..1b489374 100644 --- a/src/tools/tool_client.rs +++ b/src/tools/tool_client.rs @@ -330,7 +330,7 @@ impl ToolClient { self_id: s.registry, name: s.jay_compositor.0, interface: JayCompositor.name(), - version: s.jay_compositor.1.min(4), + version: s.jay_compositor.1.min(6), id: id.into(), }); self.jay_compositor.set(Some(id)); diff --git a/wire/jay_screenshot.txt b/wire/jay_screenshot.txt index a4f77050..90947852 100644 --- a/wire/jay_screenshot.txt +++ b/wire/jay_screenshot.txt @@ -1,4 +1,6 @@ -# events +request destroy { + +} event dmabuf { drm_dev: fd, @@ -14,3 +16,18 @@ event dmabuf { event error { msg: str, } + +event plane { + fd: fd, + offset: u32, + stride: u32, +} + +event format { + drm_dev: fd, + format: u32, + width: i32, + height: i32, + modifier_lo: u32, + modifier_hi: u32, +} From 1520232858174000352756800a183cebb5979d4b Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Sun, 28 Jul 2024 18:17:11 +0200 Subject: [PATCH 2/2] it: don't require linear modifier for screenshots --- src/cli/screenshot.rs | 4 ++-- src/it/test_gfx_api.rs | 4 ++-- src/screenshoter.rs | 20 ++++++++------------ 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/cli/screenshot.rs b/src/cli/screenshot.rs index 047f30b3..eeeae8e9 100644 --- a/src/cli/screenshot.rs +++ b/src/cli/screenshot.rs @@ -7,7 +7,7 @@ use { video::{ dmabuf::{DmaBuf, DmaBufIds, DmaBufPlane, PlaneVec}, drm::Drm, - gbm::{GbmDevice, GBM_BO_USE_LINEAR, GBM_BO_USE_RENDERING}, + gbm::GbmDevice, }, wire::{ jay_compositor::TakeScreenshot, @@ -137,7 +137,7 @@ pub fn buf_to_bytes(dev: &Rc, buf: &DmaBuf, format: ScreenshotFormat) - fatal!("Could not create a gbm device: {}", ErrorFmt(e)); } }; - let bo = match gbm.import_dmabuf(&buf, GBM_BO_USE_LINEAR | GBM_BO_USE_RENDERING) { + let bo = match gbm.import_dmabuf(&buf, 0) { Ok(bo) => Rc::new(bo), Err(e) => { fatal!("Could not import screenshot dmabuf: {}", ErrorFmt(e)); diff --git a/src/it/test_gfx_api.rs b/src/it/test_gfx_api.rs index 4dd2f5b4..b4f8d2c9 100644 --- a/src/it/test_gfx_api.rs +++ b/src/it/test_gfx_api.rs @@ -11,7 +11,7 @@ use { dmabuf::DmaBuf, drm::{sync_obj::SyncObjCtx, Drm, DrmError}, gbm::{GbmBo, GbmDevice, GbmError}, - LINEAR_MODIFIER, + INVALID_MODIFIER, }, }, ahash::AHashMap, @@ -65,7 +65,7 @@ impl TestGfxCtx { let gbm = GbmDevice::new(drm).map_err(TestGfxError::CreateGbmDevice)?; let ctx = Rc::new(SyncObjCtx::new(drm.fd())); let mut modifiers = IndexSet::new(); - modifiers.insert(LINEAR_MODIFIER); + modifiers.insert(INVALID_MODIFIER); let mut formats = AHashMap::new(); for f in [XRGB8888, ARGB8888] { formats.insert( diff --git a/src/screenshoter.rs b/src/screenshoter.rs index 4f9454c9..52c6b678 100644 --- a/src/screenshoter.rs +++ b/src/screenshoter.rs @@ -6,8 +6,7 @@ use { state::State, video::{ drm::DrmError, - gbm::{GbmBo, GbmError, GBM_BO_USE_LINEAR, GBM_BO_USE_RENDERING}, - INVALID_MODIFIER, LINEAR_MODIFIER, + gbm::{GbmBo, GbmError, GBM_BO_USE_RENDERING}, }, }, jay_config::video::Transform, @@ -30,8 +29,8 @@ pub enum ScreenshooterError { DrmError(#[from] DrmError), #[error("Render context does not support XRGB8888")] XRGB8888, - #[error("Render context supports neither linear nor invalid modifier for XRGB8888 rendering")] - Linear, + #[error("Render context cannot render to XRGB8888")] + NoModifiers, } pub struct Screenshot { @@ -52,16 +51,13 @@ pub fn take_screenshot( return Err(ScreenshooterError::EmptyDisplay); } let formats = ctx.formats(); - let mut usage = GBM_BO_USE_RENDERING; let modifiers = match formats.get(&XRGB8888.drm) { None => return Err(ScreenshooterError::XRGB8888), - Some(f) if f.write_modifiers.contains(&LINEAR_MODIFIER) => &[LINEAR_MODIFIER], - Some(f) if f.write_modifiers.contains(&INVALID_MODIFIER) => { - usage |= GBM_BO_USE_LINEAR; - &[INVALID_MODIFIER] - } - Some(_) => return Err(ScreenshooterError::Linear), + Some(f) => &f.write_modifiers, }; + if modifiers.is_empty() { + return Err(ScreenshooterError::NoModifiers); + } let gbm = ctx.gbm(); let bo = gbm.create_bo( &state.dma_buf_ids, @@ -69,7 +65,7 @@ pub fn take_screenshot( extents.height(), XRGB8888, modifiers, - usage, + GBM_BO_USE_RENDERING, )?; let fb = ctx.clone().dmabuf_fb(bo.dmabuf())?; fb.render_node(