gfx: remove incompatible shm downloads

src/gfx_api.rs

@@ -265,16 +265,7 @@ pub trait GfxFramebuffer: Debug {
         clear: Option<&Color>,
     ) -> Result<Option<SyncFile>, GfxError>;
 
-    fn copy_to_shm(
+    fn copy_to_shm(self: Rc<Self>, shm: &[Cell<u8>]) -> Result<(), GfxError>;
-        self: Rc<Self>,
-        x: i32,
-        y: i32,
-        width: i32,
-        height: i32,
-        stride: i32,
-        format: &'static Format,
-        shm: &[Cell<u8>],
-    ) -> Result<(), GfxError>;
 
     fn format(&self) -> &'static Format;
 }

src/gfx_apis/gl/renderer/framebuffer.rs

@@ -34,29 +34,21 @@ impl Debug for Framebuffer {
 }
 
 impl Framebuffer {
-    pub fn copy_to_shm(
+    pub fn copy_to_shm(&self, shm: &[Cell<u8>]) -> Result<(), RenderError> {
-        &self,
+        let format = self.gl.rb.format;
-        x: i32,
-        y: i32,
-        width: i32,
-        height: i32,
-        format: &Format,
-        shm: &[Cell<u8>],
-    ) -> Result<(), RenderError> {
         let Some(shm_info) = &format.shm_info else {
             return Err(RenderError::UnsupportedShmFormat(format.name));
         };
         let gles = self.ctx.ctx.dpy.gles;
-        let y = self.gl.height - y - height;
         let _ = self.ctx.ctx.with_current(|| {
             unsafe {
                 (gles.glBindFramebuffer)(GL_FRAMEBUFFER, self.gl.fbo);
                 (gles.glViewport)(0, 0, self.gl.width, self.gl.height);
                 (gles.glReadnPixels)(
-                    x,
+                    0,
-                    y,
+                    0,
-                    width,
+                    self.gl.width,
-                    height,
+                    self.gl.height,
                     shm_info.gl_format as _,
                     shm_info.gl_type as _,
                     shm.len() as _,
@@ -112,19 +104,8 @@ impl GfxFramebuffer for Framebuffer {
         self.render(acquire_sync, ops, clear).map_err(|e| e.into())
     }
 
-    fn copy_to_shm(
+    fn copy_to_shm(self: Rc<Self>, shm: &[Cell<u8>]) -> Result<(), GfxError> {
-        self: Rc<Self>,
+        (*self).copy_to_shm(shm).map_err(|e| e.into())
-        x: i32,
-        y: i32,
-        width: i32,
-        height: i32,
-        _stride: i32,
-        format: &'static Format,
-        shm: &[Cell<u8>],
-    ) -> Result<(), GfxError> {
-        (*self)
-            .copy_to_shm(x, y, width, height, format, shm)
-            .map_err(|e| e.into())
     }
 
     fn format(&self) -> &'static Format {

src/gfx_apis/vulkan.rs

@@ -173,16 +173,6 @@ pub enum VulkanError {
     InvalidStride,
     #[error("Shm stride and height do not match buffer size")]
     InvalidBufferSize,
-    #[error("The shm parameters are invalid x={x}, y={y}, width={width}, height={height}, stride={stride}")]
-    InvalidShmParameters {
-        x: i32,
-        y: i32,
-        width: i32,
-        height: i32,
-        stride: i32,
-    },
-    #[error(transparent)]
-    GfxError(GfxError),
     #[error("Buffer format {0} is not supported for shm buffers in Vulkan context")]
     UnsupportedShmFormat(&'static str),
     #[error("Only BO_USE_RENDERING and BO_USE_WRITE are supported")]

src/gfx_apis/vulkan/image.rs

@@ -510,18 +510,9 @@ impl GfxFramebuffer for VulkanImage {
             .map_err(|e| e.into())
     }
 
-    fn copy_to_shm(
+    fn copy_to_shm(self: Rc<Self>, shm: &[Cell<u8>]) -> Result<(), GfxError> {
-        self: Rc<Self>,
-        x: i32,
-        y: i32,
-        width: i32,
-        height: i32,
-        stride: i32,
-        format: &'static Format,
-        shm: &[Cell<u8>],
-    ) -> Result<(), GfxError> {
         self.renderer
-            .read_pixels(&self, x, y, width, height, stride, format, shm)
+            .read_all_pixels(&self, shm)
             .map_err(|e| e.into())
     }
 

src/gfx_apis/vulkan/renderer.rs

@@ -2,10 +2,10 @@ use {
     crate::{
         async_engine::{AsyncEngine, SpawnedFuture},
         cpu_worker::PendingJob,
-        format::{Format, XRGB8888},
+        format::XRGB8888,
         gfx_api::{
-            AcquireSync, BufferResv, BufferResvUser, GfxApiOpt, GfxFormat, GfxFramebuffer,
+            AcquireSync, BufferResv, BufferResvUser, GfxApiOpt, GfxFormat, GfxTexture,
-            GfxTexture, GfxWriteModifier, ReleaseSync, SyncFile,
+            GfxWriteModifier, ReleaseSync, SyncFile,
         },
         gfx_apis::vulkan::{
             allocator::{VulkanAllocator, VulkanThreadedAllocator},
@@ -905,74 +905,20 @@ impl VulkanRenderer {
         frame.waiter.set(Some(future));
     }
 
-    pub fn read_pixels(
+    pub(super) fn read_all_pixels(
-        self: &Rc<Self>,
-        tex: &Rc<VulkanImage>,
-        x: i32,
-        y: i32,
-        width: i32,
-        height: i32,
-        stride: i32,
-        format: &'static Format,
-        dst: &[Cell<u8>],
-    ) -> Result<(), VulkanError> {
-        if x < 0 || y < 0 || width <= 0 || height <= 0 || stride <= 0 {
-            return Err(VulkanError::InvalidShmParameters {
-                x,
-                y,
-                width,
-                height,
-                stride,
-            });
-        }
-        let width = width as u32;
-        let height = height as u32;
-        let stride = stride as u32;
-        if x == 0 && y == 0 && width == tex.width && height == tex.height && format == tex.format {
-            return self.read_all_pixels(tex, stride, dst);
-        }
-        let tmp_tex = self.create_shm_texture(
-            format,
-            width as i32,
-            height as i32,
-            stride as i32,
-            &[],
-            true,
-            None,
-        )?;
-        (&*tmp_tex as &dyn GfxFramebuffer)
-            .copy_texture(
-                AcquireSync::None,
-                ReleaseSync::None,
-                &(tex.clone() as _),
-                None,
-                AcquireSync::None,
-                ReleaseSync::None,
-                x,
-                y,
-            )
-            .map_err(VulkanError::GfxError)?;
-        self.read_all_pixels(&tmp_tex, stride, dst)
-    }
-
-    fn read_all_pixels(
         self: &Rc<Self>,
         tex: &VulkanImage,
-        stride: u32,
         dst: &[Cell<u8>],
     ) -> Result<(), VulkanError> {
         let Some(shm_info) = &tex.format.shm_info else {
             return Err(VulkanError::UnsupportedShmFormat(tex.format.name));
         };
-        if stride < tex.width * shm_info.bpp || stride % shm_info.bpp != 0 {
+        let size = tex.stride as u64 * tex.height as u64;
-            return Err(VulkanError::InvalidStride);
-        }
-        let size = stride as u64 * tex.height as u64;
         if size != dst.len() as u64 {
             return Err(VulkanError::InvalidBufferSize);
         }
         let region = BufferImageCopy::default()
-            .buffer_row_length(stride / shm_info.bpp)
+            .buffer_row_length(tex.stride / shm_info.bpp)
             .buffer_image_height(tex.height)
             .image_subresource(ImageSubresourceLayers {
                 aspect_mask: ImageAspectFlags::COLOR,

src/it/test_gfx_api.rs

@@ -214,44 +214,17 @@ struct TestDmaBufGfxImage {
 }
 
 impl TestGfxImage {
-    fn read_pixels(
+    fn read_pixels(&self, shm: &[Cell<u8>]) -> Result<(), GfxError> {
-        &self,
+        let copy = |height: i32, stride: i32, src: *const u8, dst: *mut u8| unsafe {
-        x: i32,
+            let size = (height * stride) as usize;
-        y: i32,
+            assert!(shm.len() >= size);
-        width: i32,
+            ptr::copy_nonoverlapping(src, dst, size);
-        height: i32,
-        stride: i32,
-        format: &'static Format,
-        shm: &[Cell<u8>],
-    ) -> Result<(), GfxError> {
-        assert!(x >= 0);
-        assert!(y >= 0);
-        assert!(width >= 0);
-        assert!(height >= 0);
-        assert!(stride >= 0);
-        assert!(x + width <= self.width());
-        assert!(y + height <= self.height());
-        assert!(stride >= width * 4);
-        let size = (stride * height) as usize;
-        assert!(shm.len() >= size);
-        let copy = |src_stride: i32, src_format: &Format, mut src: *const u8, mut dst: *mut u8| unsafe {
-            src = src.add((y * src_stride + x * 4) as usize);
-            for _ in 0..height {
-                ptr::copy_nonoverlapping(src, dst, (width * 4) as usize);
-                if !src_format.has_alpha && format.has_alpha {
-                    for i in 0..width {
-                        *dst.add((i * 4 + 3) as usize) = 255;
-                    }
-                }
-                src = src.add(src_stride as usize);
-                dst = dst.add(stride as usize);
-            }
         };
         match self {
             TestGfxImage::Shm(s) => {
                 copy(
+                    s.height,
                     s.stride,
-                    s.format,
                     s.data.borrow().as_ptr(),
                     shm.as_ptr() as _,
                 );
@@ -260,8 +233,8 @@ impl TestGfxImage {
                 let map = d.bo.clone().map_read().map_err(TestGfxError::MapDmaBuf)?;
                 unsafe {
                     copy(
+                        d.buf.height,
                         map.stride(),
-                        d.buf.format,
                         map.data().as_ptr(),
                         shm.as_ptr() as _,
                     );
@@ -575,19 +548,8 @@ impl GfxFramebuffer for TestGfxFb {
         Ok(None)
     }
 
-    fn copy_to_shm(
+    fn copy_to_shm(self: Rc<Self>, shm: &[Cell<u8>]) -> Result<(), GfxError> {
-        self: Rc<Self>,
+        self.img.deref().read_pixels(shm)
-        x: i32,
-        y: i32,
-        width: i32,
-        height: i32,
-        stride: i32,
-        format: &'static Format,
-        shm: &[Cell<u8>],
-    ) -> Result<(), GfxError> {
-        self.img
-            .deref()
-            .read_pixels(x, y, width, height, stride, format, shm)
     }
 
     fn format(&self) -> &'static Format {

src/state.rs

@@ -1045,17 +1045,7 @@ impl State {
             scale,
         )
         .map_err(ShmScreencopyError::CopyToTemporary)?;
-        let acc = mem.access(|mem| {
+        let acc = mem.access(|mem| fb.copy_to_shm(mem));
-            fb.copy_to_shm(
-                0,
-                0,
-                capture.rect.width(),
-                capture.rect.height(),
-                stride,
-                format,
-                mem,
-            )
-        });
         match acc {
             Ok(res) => res.map_err(ShmScreencopyError::ReadPixels),
             Err(e) => {