video: always use correct modifiers

src/backends/metal.rs

@@ -108,6 +108,14 @@ pub enum MetalError {
     DevicePauseSignalHandler(#[source] DbusError),
     #[error("Could not create device-resumed signal handler")]
     DeviceResumeSignalHandler(#[source] DbusError),
+    #[error("Device render context does not support required format {0}")]
+    MissingDevFormat(&'static str),
+    #[error("Render context does not support required format {0}")]
+    MissingRenderFormat(&'static str),
+    #[error("Device cannot scan out any buffers writable by its GFX API (format {0})")]
+    MissingDevModifier(&'static str),
+    #[error("Device GFX API cannot read any buffers writable by the render GFX API (format {0})")]
+    MissingRenderModifier(&'static str),
 }
 
 pub struct MetalBackend {

src/backends/metal/video.rs

@@ -28,11 +28,12 @@ use {
                 DRM_MODE_ATOMIC_NONBLOCK, DRM_MODE_PAGE_FLIP_EVENT,
             },
             gbm::{GbmDevice, GBM_BO_USE_LINEAR, GBM_BO_USE_RENDERING, GBM_BO_USE_SCANOUT},
-            Modifier,
+            Modifier, INVALID_MODIFIER,
         },
     },
     ahash::{AHashMap, AHashSet},
     bstr::{BString, ByteSlice},
+    indexmap::{indexset, IndexSet},
     std::{
         cell::{Cell, RefCell},
         ffi::CString,
@@ -508,7 +509,7 @@ pub enum PlaneType {
 #[derive(Debug)]
 pub struct PlaneFormat {
     _format: &'static Format,
-    modifiers: Vec<Modifier>,
+    modifiers: IndexSet<Modifier>,
 }
 
 #[derive(Debug)]
@@ -785,7 +786,7 @@ fn create_plane(plane: DrmPlane, master: &Rc<DrmMaster>) -> Result<MetalPlane, D
                     format,
                     PlaneFormat {
                         _format: f,
-                        modifiers: vec![],
+                        modifiers: indexset![INVALID_MODIFIER],
                     },
                 );
             }
@@ -1593,14 +1594,14 @@ impl MetalBackend {
         &self,
         dev: &Rc<MetalDrmDevice>,
         format: &Format,
-        modifiers: &[Modifier],
+        plane_modifiers: &IndexSet<Modifier>,
         width: i32,
         height: i32,
         ctx: &MetalRenderContext,
         cursor: bool,
     ) -> Result<[RenderBuffer; 2], MetalError> {
         let create =
-            || self.create_scanout_buffer(dev, format, modifiers, width, height, ctx, cursor);
+            || self.create_scanout_buffer(dev, format, plane_modifiers, width, height, ctx, cursor);
         Ok([create()?, create()?])
     }
 
@@ -1608,17 +1609,35 @@ impl MetalBackend {
         &self,
         dev: &Rc<MetalDrmDevice>,
         format: &Format,
-        modifiers: &[Modifier],
+        plane_modifiers: &IndexSet<Modifier>,
         width: i32,
         height: i32,
         render_ctx: &MetalRenderContext,
         cursor: bool,
     ) -> Result<RenderBuffer, MetalError> {
+        let dev_gfx_formats = dev.ctx.gfx.formats();
+        let dev_gfx_format = match dev_gfx_formats.get(&format.drm) {
+            None => return Err(MetalError::MissingDevFormat(format.name)),
+            Some(f) => f,
+        };
+        let possible_modifiers: Vec<_> = dev_gfx_format
+            .write_modifiers
+            .iter()
+            .filter(|m| plane_modifiers.contains(*m))
+            .copied()
+            .collect();
+        if possible_modifiers.is_empty() {
+            log::warn!("Scanout modifiers: {:?}", plane_modifiers);
+            log::warn!("DEV GFX modifiers: {:?}", dev_gfx_format.write_modifiers);
+            return Err(MetalError::MissingDevModifier(format.name));
+        }
         let mut usage = GBM_BO_USE_RENDERING | GBM_BO_USE_SCANOUT;
-        if cursor && modifiers.is_empty() {
+        if cursor {
             usage |= GBM_BO_USE_LINEAR;
         };
-        let dev_bo = dev.gbm.create_bo(width, height, format, modifiers, usage);
+        let dev_bo = dev
+            .gbm
+            .create_bo(width, height, format, &possible_modifiers, usage);
         let dev_bo = match dev_bo {
             Ok(b) => b,
             Err(e) => return Err(MetalError::ScanoutBuffer(e)),
@@ -1644,11 +1663,31 @@ impl MetalBackend {
             (None, render_tex, None)
         } else {
             // Create a _bridge_ BO in the render device
+            let render_gfx_formats = render_ctx.gfx.formats();
+            let render_gfx_format = match render_gfx_formats.get(&format.drm) {
+                None => return Err(MetalError::MissingRenderFormat(format.name)),
+                Some(f) => f,
+            };
+            let possible_modifiers: Vec<_> = render_gfx_format
+                .write_modifiers
+                .iter()
+                .filter(|m| dev_gfx_format.read_modifiers.contains(*m))
+                .copied()
+                .collect();
+            if possible_modifiers.is_empty() {
+                log::warn!(
+                    "Render GFX modifiers: {:?}",
+                    render_gfx_format.write_modifiers
+                );
+                log::warn!("DEV GFX modifiers: {:?}", dev_gfx_format.read_modifiers);
+                return Err(MetalError::MissingRenderModifier(format.name));
+            }
             usage = GBM_BO_USE_RENDERING | GBM_BO_USE_LINEAR;
-            let render_bo = render_ctx
+            let render_bo =
-                .gfx
+                render_ctx
-                .gbm()
+                    .gfx
-                .create_bo(width, height, format, &[], usage);
+                    .gbm()
+                    .create_bo(width, height, format, &possible_modifiers, usage);
             let render_bo = match render_bo {
                 Ok(b) => b,
                 Err(e) => return Err(MetalError::ScanoutBuffer(e)),
@@ -1765,7 +1804,7 @@ impl MetalBackend {
             false,
         )?);
         let mut cursor_plane = None;
-        let mut cursor_modifiers = &[][..];
+        let mut cursor_modifiers = &IndexSet::new();
         for plane in crtc.possible_planes.values() {
             if plane.ty == PlaneType::Cursor
                 && !plane.assigned.get()
@@ -1773,7 +1812,7 @@ impl MetalBackend {
             {
                 if let Some(format) = plane.formats.get(&ARGB8888.drm) {
                     cursor_plane = Some(plane.clone());
-                    cursor_modifiers = &format.modifiers[..];
+                    cursor_modifiers = &format.modifiers;
                     break;
                 }
             }

src/backends/x.rs

@@ -20,7 +20,8 @@ use {
         },
         video::{
             drm::{ConnectorType, Drm, DrmError, DrmVersion},
-            gbm::{GbmDevice, GbmError, GBM_BO_USE_RENDERING},
+            gbm::{GbmDevice, GbmError, GBM_BO_USE_LINEAR, GBM_BO_USE_RENDERING},
+            INVALID_MODIFIER, LINEAR_MODIFIER,
         },
         wire_xcon::{
             ChangeProperty, ChangeWindowAttributes, ConfigureNotify, CreateCursor, CreatePixmap,
@@ -117,6 +118,8 @@ pub enum XBackendError {
     QueryDevice(#[source] XconError),
     #[error("Could not fstat the drm device")]
     DrmDeviceFstat(#[source] Errno),
+    #[error("Render device does not support XRGB8888 format")]
+    XRGB8888,
 }
 
 pub async fn create(state: &Rc<State>) -> Result<Rc<XBackend>, XBackendError> {
@@ -376,10 +379,24 @@ impl XBackend {
         height: i32,
     ) -> Result<[XImage; 2], XBackendError> {
         let mut images = [None, None];
+        let formats = self.ctx.formats();
+        let format = match formats.get(&XRGB8888.drm) {
+            Some(f) => f,
+            None => return Err(XBackendError::XRGB8888),
+        };
+        let mut usage = GBM_BO_USE_RENDERING;
+        let modifier = if format.write_modifiers.contains(&LINEAR_MODIFIER) {
+            &[LINEAR_MODIFIER]
+        } else if format.write_modifiers.contains(&INVALID_MODIFIER) {
+            usage |= GBM_BO_USE_LINEAR;
+            &[INVALID_MODIFIER]
+        } else {
+            panic!("Neither linear nor invalid modifier is supported");
+        };
         for image in &mut images {
             let bo = self
                 .gbm
-                .create_bo(width, height, XRGB8888, &[], GBM_BO_USE_RENDERING)?;
+                .create_bo(width, height, XRGB8888, modifier, usage)?;
             let dma = bo.dmabuf();
             assert!(dma.planes.len() == 1);
             let plane = dma.planes.first().unwrap();

src/cli/screenshot.rs

@@ -8,7 +8,6 @@ use {
             dmabuf::{DmaBuf, DmaBufPlane, PlaneVec},
             drm::Drm,
             gbm::{GbmDevice, GBM_BO_USE_LINEAR, GBM_BO_USE_RENDERING},
-            INVALID_MODIFIER,
         },
         wire::{
             jay_compositor::TakeScreenshot,
@@ -91,7 +90,7 @@ pub fn buf_to_qoi(buf: &Dmabuf) -> Vec<u8> {
         width: buf.width as _,
         height: buf.height as _,
         format: XRGB8888,
-        modifier: INVALID_MODIFIER,
+        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) {

src/drm_feedback.rs

@@ -42,7 +42,7 @@ impl DrmFeedback {
 fn create_fd_data(ctx: &dyn GfxContext) -> Vec<u8> {
     let mut vec = vec![];
     for (format, info) in &*ctx.formats() {
-        for modifier in &info.modifiers {
+        for modifier in &info.read_modifiers {
             vec.write_u32::<NativeEndian>(*format).unwrap();
             vec.write_u32::<NativeEndian>(0).unwrap();
             vec.write_u64::<NativeEndian>(*modifier).unwrap();

src/gfx_api.rs

@@ -301,7 +301,8 @@ pub trait GfxContext: Debug {
 #[derive(Debug)]
 pub struct GfxFormat {
     pub format: &'static Format,
-    pub modifiers: IndexSet<Modifier>,
+    pub read_modifiers: IndexSet<Modifier>,
+    pub write_modifiers: IndexSet<Modifier>,
 }
 
 #[derive(Error)]

src/gfx_apis/gl/egl/display.rs

@@ -151,19 +151,24 @@ impl EglDisplay {
                 if format.implicit_external_only && !supports_external_only {
                     continue;
                 }
-                let mut modifiers = IndexSet::new();
+                let mut read_modifiers = IndexSet::new();
+                let mut write_modifiers = IndexSet::new();
                 for modifier in format.modifiers.values() {
                     if modifier.external_only && !supports_external_only {
                         continue;
                     }
-                    modifiers.insert(modifier.modifier);
+                    if !modifier.external_only {
+                        write_modifiers.insert(modifier.modifier);
+                    }
+                    read_modifiers.insert(modifier.modifier);
                 }
-                if !modifiers.is_empty() {
+                if !read_modifiers.is_empty() || !write_modifiers.is_empty() {
                     formats.insert(
                         drm,
                         GfxFormat {
                             format: format.format,
-                            modifiers,
+                            read_modifiers,
+                            write_modifiers,
                         },
                     );
                 }

src/ifs/jay_compositor.rs

@@ -143,6 +143,7 @@ impl JayCompositor {
                     dmabuf.height,
                     plane.offset,
                     plane.stride,
+                    dmabuf.modifier,
                 );
             }
             Err(e) => {

src/ifs/jay_screencast.rs

@@ -17,10 +17,13 @@ use {
         video::{
             dmabuf::DmaBuf,
             gbm::{GbmError, GBM_BO_USE_LINEAR, GBM_BO_USE_RENDERING},
+            Modifier, INVALID_MODIFIER, LINEAR_MODIFIER,
         },
         wire::{jay_screencast::*, JayScreencastId},
     },
     ahash::AHashSet,
+    indexmap::{indexset, IndexSet},
+    once_cell::sync::Lazy,
     std::{
         cell::{Cell, RefCell},
         ops::{Deref, DerefMut},
@@ -194,17 +197,37 @@ impl JayScreencast {
 
     pub fn realloc(&self, ctx: &Rc<dyn GfxContext>) -> Result<(), JayScreencastError> {
         let mut buffers = vec![];
+        let formats = ctx.formats();
+        let format = match formats.get(&XRGB8888.drm) {
+            Some(f) => f,
+            _ => return Err(JayScreencastError::XRGB8888),
+        };
         if let Some(output) = self.output.get() {
             let mode = output.global.mode.get();
             let num = 3;
             for _ in 0..num {
-                let mut flags = GBM_BO_USE_RENDERING;
+                let mut usage = GBM_BO_USE_RENDERING;
-                if self.linear.get() {
+                let modifiers = match self.linear.get() {
-                    flags |= GBM_BO_USE_LINEAR;
+                    true if format.write_modifiers.contains(&LINEAR_MODIFIER) => {
-                }
+                        static MODS: Lazy<IndexSet<Modifier>> =
-                let buffer = ctx
+                            Lazy::new(|| indexset![LINEAR_MODIFIER]);
-                    .gbm()
+                        &MODS
-                    .create_bo(mode.width, mode.height, XRGB8888, &[], flags)?;
+                    }
+                    true if format.write_modifiers.contains(&INVALID_MODIFIER) => {
+                        usage |= GBM_BO_USE_LINEAR;
+                        static MODS: Lazy<IndexSet<Modifier>> =
+                            Lazy::new(|| indexset![INVALID_MODIFIER]);
+                        &MODS
+                    }
+                    true => return Err(JayScreencastError::Modifier),
+                    false if format.write_modifiers.is_empty() => {
+                        return Err(JayScreencastError::XRGB8888Writing)
+                    }
+                    false => &format.write_modifiers,
+                };
+                let buffer =
+                    ctx.gbm()
+                        .create_bo(mode.width, mode.height, XRGB8888, modifiers, usage)?;
                 let fb = ctx.clone().dmabuf_img(buffer.dmabuf())?.to_framebuffer()?;
                 buffers.push(ScreencastBuffer {
                     dmabuf: buffer.dmabuf().clone(),
@@ -438,6 +461,12 @@ pub enum JayScreencastError {
     GbmError(#[from] GbmError),
     #[error(transparent)]
     GfxError(#[from] GfxError),
+    #[error("Render context does not support XRGB8888 format")]
+    XRGB8888,
+    #[error("Render context does not support XRGB8888 format for rendering")]
+    XRGB8888Writing,
+    #[error("Render context supports neither linear or invalid modifier")]
+    Modifier,
 }
 efrom!(JayScreencastError, MsgParserError);
 efrom!(JayScreencastError, ClientError);

src/ifs/jay_screenshot.rs

@@ -24,6 +24,7 @@ impl JayScreenshot {
         height: i32,
         offset: u32,
         stride: u32,
+        modifier: u64,
     ) {
         self.client.event(Dmabuf {
             self_id: self.id,
@@ -33,6 +34,8 @@ impl JayScreenshot {
             height: height as _,
             offset,
             stride,
+            modifier_lo: modifier as u32,
+            modifier_hi: (modifier >> 32) as u32,
         });
     }
 

src/ifs/zwp_linux_buffer_params_v1.rs

@@ -110,7 +110,7 @@ impl ZwpLinuxBufferParamsV1 {
             Some(m) => m,
             _ => return Err(ZwpLinuxBufferParamsV1Error::NoPlanes),
         };
-        if !format.modifiers.contains(&modifier) {
+        if !format.read_modifiers.contains(&modifier) {
             return Err(ZwpLinuxBufferParamsV1Error::InvalidModifier(modifier));
         }
         let mut dmabuf = DmaBuf {

src/ifs/zwp_linux_dmabuf_v1.rs

@@ -44,7 +44,7 @@ impl ZwpLinuxDmabufV1Global {
                 for format in formats.values() {
                     obj.send_format(format.format.drm);
                     if version >= MODIFIERS_SINCE_VERSION {
-                        for &modifier in &format.modifiers {
+                        for &modifier in &format.read_modifiers {
                             obj.send_modifier(format.format.drm, modifier);
                         }
                     }

src/portal/ptr_gui.rs

@@ -693,19 +693,32 @@ impl WindowData {
         self.frame_missed.set(true);
         let width = (self.width.get() as f64 * self.scale.get().to_f64()).round() as i32;
         let height = (self.height.get() as f64 * self.scale.get().to_f64()).round() as i32;
+        let formats = ctx.ctx.formats();
+        let format = match formats.get(&ARGB8888.drm) {
+            None => {
+                log::error!("Render context does not support ARGB8888 format");
+                return;
+            }
+            Some(f) => f,
+        };
+        if format.write_modifiers.is_empty() {
+            log::error!("Render context cannot render to ARGB8888 format");
+            return;
+        }
         for _ in 0..NUM_BUFFERS {
-            let bo =
+            let bo = match ctx.ctx.gbm().create_bo(
-                match ctx
+                width,
-                    .ctx
+                height,
-                    .gbm()
+                ARGB8888,
-                    .create_bo(width, height, ARGB8888, &[], GBM_BO_USE_RENDERING)
+                &format.write_modifiers,
-                {
+                GBM_BO_USE_RENDERING,
-                    Ok(b) => b,
+            ) {
-                    Err(e) => {
+                Ok(b) => b,
-                        log::error!("Could not allocate dmabuf: {}", ErrorFmt(e));
+                Err(e) => {
-                        return;
+                    log::error!("Could not allocate dmabuf: {}", ErrorFmt(e));
-                    }
+                    return;
-                };
+                }
+            };
             let img = match ctx.ctx.clone().dmabuf_img(bo.dmabuf()) {
                 Ok(b) => b,
                 Err(e) => {

src/screenshoter.rs

@@ -7,6 +7,7 @@ use {
         video::{
             drm::DrmError,
             gbm::{GbmBo, GbmError, GBM_BO_USE_LINEAR, GBM_BO_USE_RENDERING},
+            INVALID_MODIFIER, LINEAR_MODIFIER,
         },
     },
     std::{ops::Deref, rc::Rc},
@@ -26,6 +27,10 @@ pub enum ScreenshooterError {
     RenderError(#[from] GfxError),
     #[error(transparent)]
     DrmError(#[from] DrmError),
+    #[error("Render context does not support XRGB8888")]
+    XRGB8888,
+    #[error("Render context supports neither linear nor invalid modifier for XRGB8888 rendering")]
+    Linear,
 }
 
 pub struct Screenshot {
@@ -42,13 +47,24 @@ pub fn take_screenshot(state: &State) -> Result<Screenshot, ScreenshooterError>
     if extents.is_empty() {
         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),
+    };
     let gbm = ctx.gbm();
     let bo = gbm.create_bo(
         extents.width(),
         extents.height(),
         XRGB8888,
-        &[],
+        modifiers,
-        GBM_BO_USE_RENDERING | GBM_BO_USE_LINEAR,
+        usage,
     )?;
     let fb = ctx.clone().dmabuf_fb(bo.dmabuf())?;
     fb.render_node(

src/video/drm.rs

@@ -17,6 +17,7 @@ use {
     },
     ahash::AHashMap,
     bstr::{BString, ByteSlice},
+    indexmap::IndexSet,
     std::{
         cell::RefCell,
         ffi::CString,
@@ -181,7 +182,7 @@ impl Drm {
 
 pub struct InFormat {
     pub format: u32,
-    pub modifiers: Vec<Modifier>,
+    pub modifiers: IndexSet<Modifier>,
 }
 
 pub struct DrmMaster {
@@ -418,7 +419,7 @@ impl DrmMaster {
             .unwrap()
             .map(|f| InFormat {
                 format: f,
-                modifiers: vec![],
+                modifiers: IndexSet::new(),
             })
             .collect();
         let modifiers =
@@ -435,7 +436,7 @@ impl DrmMaster {
                     log::error!("Modifier offset is out of bounds");
                     return Err(DrmError::InFormats);
                 }
-                formats[idx].modifiers.push(modifier.modifier);
+                formats[idx].modifiers.insert(modifier.modifier);
             }
         }
         Ok(formats)

src/video/gbm.rs

@@ -7,7 +7,7 @@ use {
         video::{
             dmabuf::{DmaBuf, DmaBufPlane, PlaneVec},
             drm::{Drm, DrmError},
-            Modifier,
+            Modifier, INVALID_MODIFIER,
         },
     },
     std::{
@@ -34,6 +34,8 @@ pub enum GbmError {
     DrmFd,
     #[error("Could not map bo")]
     MapBo(#[source] OsError),
+    #[error("Tried to allocate a buffer with no modifier")]
+    NoModifier,
 }
 
 pub type Device = u8;
@@ -195,18 +197,23 @@ impl GbmDevice {
         self.dev
     }
 
-    pub fn create_bo(
+    pub fn create_bo<'a>(
         &self,
         width: i32,
         height: i32,
         format: &Format,
-        modifiers: &[Modifier],
+        modifiers: impl IntoIterator<Item = &'a Modifier>,
-        usage: u32,
+        mut usage: u32,
     ) -> Result<GbmBo, GbmError> {
         unsafe {
-            let (modifiers, n_modifiers) = if modifiers.is_empty() {
+            let modifiers: Vec<Modifier> = modifiers.into_iter().copied().collect();
+            if modifiers.is_empty() {
+                return Err(GbmError::NoModifier);
+            }
+            let (modifiers, n_modifiers) = if modifiers == [INVALID_MODIFIER] {
                 (ptr::null(), 0)
             } else {
+                usage &= !GBM_BO_USE_LINEAR;
                 (modifiers.as_ptr() as _, modifiers.len() as _)
             };
             let bo = gbm_bo_create_with_modifiers2(

wire/jay_screenshot.txt

@@ -7,6 +7,8 @@ msg dmabuf = 0 {
     height: u32,
     offset: u32,
     stride: u32,
+    modifier_lo: u32,
+    modifier_hi: u32,
 }
 
 msg error = 1 {