From 3bfa9d2f36f65a8f4076518e346259f2c75fd1b3 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 28 Nov 2024 16:38:53 +0100 Subject: [PATCH 01/24] drm/compositor: Refactor exporter out --- src/backend/drm/compositor/mod.rs | 103 +---------------- .../drm/{compositor => exporter}/dumb.rs | 1 + .../drm/{compositor => exporter}/gbm.rs | 9 ++ src/backend/drm/exporter/mod.rs | 106 ++++++++++++++++++ src/backend/drm/mod.rs | 2 + 5 files changed, 123 insertions(+), 98 deletions(-) rename src/backend/drm/{compositor => exporter}/dumb.rs (97%) rename src/backend/drm/{compositor => exporter}/gbm.rs (87%) create mode 100644 src/backend/drm/exporter/mod.rs diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index 81b3fb69c795..0ea05634a64d 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -120,14 +120,13 @@ //! } //! ``` use std::{ - cell::RefCell, collections::{HashMap, HashSet}, fmt::Debug, io::ErrorKind, os::unix::io::{AsFd, OwnedFd}, rc::Rc, str::FromStr, - sync::{Arc, Mutex}, + sync::Arc, }; use drm::{ @@ -173,13 +172,13 @@ use crate::{ }; use super::{ - error::AccessError, surface::VrrSupport, DrmDeviceFd, DrmSurface, Framebuffer, PlaneClaim, PlaneInfo, - Planes, + error::AccessError, + exporter::{ExportBuffer, ExportFramebuffer}, + surface::VrrSupport, + DrmSurface, Framebuffer, PlaneClaim, PlaneInfo, Planes, }; -pub mod dumb; mod elements; -pub mod gbm; use elements::*; @@ -844,98 +843,6 @@ impl FrameState { } } -/// Possible buffers to export as a framebuffer using [`ExportFramebuffer`] -#[derive(Debug)] -pub enum ExportBuffer<'a, B: Buffer> { - /// A wayland buffer - Wayland(&'a WlBuffer), - /// A [`Allocator`] buffer - Allocator(&'a B), -} - -impl<'a, B: Buffer> ExportBuffer<'a, B> { - #[inline] - fn from_underlying_storage(storage: &'a UnderlyingStorage<'_>) -> Option { - match storage { - UnderlyingStorage::Wayland(buffer) => Some(Self::Wayland(buffer)), - UnderlyingStorage::Memory { .. } => None, - } - } -} - -/// Export a [`ExportBuffer`] as a framebuffer -pub trait ExportFramebuffer -where - B: Buffer, -{ - /// Type of the framebuffer - type Framebuffer: Framebuffer; - - /// Type of the error - type Error: std::error::Error; - - /// Add a framebuffer for the specified buffer - fn add_framebuffer( - &self, - drm: &DrmDeviceFd, - buffer: ExportBuffer<'_, B>, - use_opaque: bool, - ) -> Result, Self::Error>; - - /// Test if the provided buffer is eligible for adding a framebuffer - fn can_add_framebuffer(&self, buffer: &ExportBuffer<'_, B>) -> bool; -} - -impl ExportFramebuffer for Arc> -where - F: ExportFramebuffer, - B: Buffer, -{ - type Framebuffer = >::Framebuffer; - type Error = >::Error; - - #[inline] - fn add_framebuffer( - &self, - drm: &DrmDeviceFd, - buffer: ExportBuffer<'_, B>, - use_opaque: bool, - ) -> Result, Self::Error> { - let guard = self.lock().unwrap(); - guard.add_framebuffer(drm, buffer, use_opaque) - } - - #[inline] - fn can_add_framebuffer(&self, buffer: &ExportBuffer<'_, B>) -> bool { - let guard = self.lock().unwrap(); - guard.can_add_framebuffer(buffer) - } -} - -impl ExportFramebuffer for Rc> -where - F: ExportFramebuffer, - B: Buffer, -{ - type Framebuffer = >::Framebuffer; - type Error = >::Error; - - #[inline] - fn add_framebuffer( - &self, - drm: &DrmDeviceFd, - buffer: ExportBuffer<'_, B>, - use_opaque: bool, - ) -> Result, Self::Error> { - self.borrow().add_framebuffer(drm, buffer, use_opaque) - } - - #[inline] - fn can_add_framebuffer(&self, buffer: &ExportBuffer<'_, B>) -> bool { - self.borrow().can_add_framebuffer(buffer) - } -} - type Frame = FrameState< DrmScanoutBuffer< ::Buffer, diff --git a/src/backend/drm/compositor/dumb.rs b/src/backend/drm/exporter/dumb.rs similarity index 97% rename from src/backend/drm/compositor/dumb.rs rename to src/backend/drm/exporter/dumb.rs index 470ef771abf3..7543bfeb3277 100644 --- a/src/backend/drm/compositor/dumb.rs +++ b/src/backend/drm/exporter/dumb.rs @@ -46,6 +46,7 @@ impl ExportFramebuffer for DrmDeviceFd { #[inline] fn can_add_framebuffer(&self, buffer: &ExportBuffer<'_, DumbBuffer>) -> bool { match buffer { + #[cfg(feature = "wayland_frontend")] ExportBuffer::Wayland(_) => false, ExportBuffer::Allocator(_) => true, } diff --git a/src/backend/drm/compositor/gbm.rs b/src/backend/drm/exporter/gbm.rs similarity index 87% rename from src/backend/drm/compositor/gbm.rs rename to src/backend/drm/exporter/gbm.rs index 9ad6fc4d05a3..f10082318e35 100644 --- a/src/backend/drm/compositor/gbm.rs +++ b/src/backend/drm/exporter/gbm.rs @@ -34,6 +34,7 @@ impl ExportFramebuffer for gbm::Device { } #[inline] + #[cfg(feature = "wayland_frontend")] fn can_add_framebuffer(&self, buffer: &ExportBuffer<'_, GbmBuffer>) -> bool { match buffer { #[cfg(not(all(feature = "backend_egl", feature = "use_system_lib")))] @@ -50,4 +51,12 @@ impl ExportFramebuffer for gbm::Device { ExportBuffer::Allocator(_) => true, } } + + #[inline] + #[cfg(not(feature = "wayland_frontend"))] + fn can_add_framebuffer(&self, buffer: &ExportBuffer<'_, GbmBuffer>) -> bool { + match buffer { + ExportBuffer::Allocator(_) => true, + } + } } diff --git a/src/backend/drm/exporter/mod.rs b/src/backend/drm/exporter/mod.rs new file mode 100644 index 000000000000..eac377a13c91 --- /dev/null +++ b/src/backend/drm/exporter/mod.rs @@ -0,0 +1,106 @@ +use std::{cell::RefCell, rc::Rc, sync::{Arc, Mutex}}; + +use wayland_server::protocol::wl_buffer::WlBuffer; + +use crate::backend::{allocator::Buffer, renderer::element::UnderlyingStorage}; + +use super::{DrmDeviceFd, Framebuffer}; + +#[cfg(feature = "backend_drm")] +pub mod dumb; +#[cfg(feature = "backend_gbm")] +pub mod gbm; + +/// Possible buffers to export as a framebuffer using [`ExportFramebuffer`] +#[derive(Debug)] +pub enum ExportBuffer<'a, B: Buffer> { + /// A wayland buffer + Wayland(&'a WlBuffer), + /// A [`Allocator`] buffer + Allocator(&'a B), +} + +impl<'a, B: Buffer> ExportBuffer<'a, B> { + /// Create the export buffer from an [`UnderlyingStorage`] + #[inline] + pub fn from_underlying_storage(storage: &'a UnderlyingStorage<'_>) -> Option { + match storage { + #[cfg(feature = "wayland_frontend")] + UnderlyingStorage::Wayland(buffer) => Some(Self::Wayland(buffer)), + UnderlyingStorage::Memory { .. } => None, + } + } +} + +/// Export a [`ExportBuffer`] as a framebuffer +pub trait ExportFramebuffer +where + B: Buffer, +{ + /// Type of the framebuffer + type Framebuffer: Framebuffer; + + /// Type of the error + type Error: std::error::Error; + + /// Add a framebuffer for the specified buffer + fn add_framebuffer( + &self, + drm: &DrmDeviceFd, + buffer: ExportBuffer<'_, B>, + use_opaque: bool, + ) -> Result, Self::Error>; + + /// Test if the provided buffer is eligible for adding a framebuffer + fn can_add_framebuffer(&self, buffer: &ExportBuffer<'_, B>) -> bool; +} + +impl ExportFramebuffer for Arc> +where + F: ExportFramebuffer, + B: Buffer, +{ + type Framebuffer = >::Framebuffer; + type Error = >::Error; + + #[inline] + fn add_framebuffer( + &self, + drm: &DrmDeviceFd, + buffer: ExportBuffer<'_, B>, + use_opaque: bool, + ) -> Result, Self::Error> { + let guard = self.lock().unwrap(); + guard.add_framebuffer(drm, buffer, use_opaque) + } + + #[inline] + fn can_add_framebuffer(&self, buffer: &ExportBuffer<'_, B>) -> bool { + let guard = self.lock().unwrap(); + guard.can_add_framebuffer(buffer) + } +} + +impl ExportFramebuffer for Rc> +where + F: ExportFramebuffer, + B: Buffer, +{ + type Framebuffer = >::Framebuffer; + type Error = >::Error; + + #[inline] + fn add_framebuffer( + &self, + drm: &DrmDeviceFd, + buffer: ExportBuffer<'_, B>, + use_opaque: bool, + ) -> Result, Self::Error> { + self.borrow().add_framebuffer(drm, buffer, use_opaque) + } + + #[inline] + fn can_add_framebuffer(&self, buffer: &ExportBuffer<'_, B>) -> bool { + self.borrow().can_add_framebuffer(buffer) + } +} \ No newline at end of file diff --git a/src/backend/drm/mod.rs b/src/backend/drm/mod.rs index 53e657903773..6c2def70aaeb 100644 --- a/src/backend/drm/mod.rs +++ b/src/backend/drm/mod.rs @@ -76,8 +76,10 @@ pub(crate) mod device; #[cfg(feature = "backend_drm")] pub mod dumb; mod error; +pub mod exporter; #[cfg(feature = "backend_gbm")] pub mod gbm; +pub mod output; mod surface; From e9cfc3563577bc0be55eb7f256f8ad52ab563797 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 28 Nov 2024 16:38:53 +0100 Subject: [PATCH 02/24] drm/compositor: Refactor buffer handling --- src/backend/drm/compositor/frame_result.rs | 452 +++++++++++++ src/backend/drm/compositor/mod.rs | 733 ++++----------------- 2 files changed, 584 insertions(+), 601 deletions(-) create mode 100644 src/backend/drm/compositor/frame_result.rs diff --git a/src/backend/drm/compositor/frame_result.rs b/src/backend/drm/compositor/frame_result.rs new file mode 100644 index 000000000000..9816258b0b3f --- /dev/null +++ b/src/backend/drm/compositor/frame_result.rs @@ -0,0 +1,452 @@ +use std::collections::HashSet; + +use crate::{ + backend::{ + allocator::{ + dmabuf::{AsDmabuf, Dmabuf}, + Buffer, Slot, + }, + drm::Framebuffer, + renderer::{ + damage::OutputDamageTracker, + element::{Element, Id, RenderElement, RenderElementStates}, + sync::SyncPoint, + utils::{CommitCounter, DamageSet, DamageSnapshot, OpaqueRegions}, + Blit, Color32F, Frame, Renderer, + }, + }, + output::OutputNoMode, + utils::{Buffer as BufferCoords, Physical, Point, Rectangle, Scale, Size, Transform}, +}; + +use super::{DrmScanoutBuffer, ScanoutBuffer}; + +/// Result for [`DrmCompositor::render_frame`] +/// +/// **Note**: This struct may contain a reference to the composited buffer +/// of the primary display plane. Dropping it will remove said reference and +/// allows the buffer to be reused. +/// +/// Keeping the buffer longer may cause the following issues: +/// - **Too much damage** - until the buffer is marked free it is not considered +/// submitted by the swapchain, causing the age value of newly queried buffers +/// to be lower than necessary, potentially resulting in more rendering than necessary. +/// To avoid this make sure the buffer is dropped before starting the next render. +/// - **Exhaustion of swapchain images** - Continuing rendering while holding on +/// to too many buffers may cause the swapchain to run out of images, returning errors +/// on rendering until buffers are freed again. The exact amount of images in a +/// swapchain is an implementation detail, but should generally be expect to be +/// large enough to hold onto at least one `RenderFrameResult`. +pub struct RenderFrameResult<'a, B: Buffer, F: Framebuffer, E> { + /// If this frame contains any changes and should be submitted + pub is_empty: bool, + /// The render element states of this frame + pub states: RenderElementStates, + /// Element for the primary plane + pub primary_element: PrimaryPlaneElement<'a, B, F, E>, + /// Overlay elements in front to back order + pub overlay_elements: Vec<&'a E>, + /// Optional cursor plane element + /// + /// If set always above all other elements + pub cursor_element: Option<&'a E>, + + pub(super) primary_plane_element_id: Id, + pub(super) supports_fencing: bool, +} + +impl<'a, B: Buffer, F: Framebuffer, E> RenderFrameResult<'a, B, F, E> { + /// Returns if synchronization with kms submission can't be guaranteed through the available apis. + pub fn needs_sync(&self) -> bool { + if let PrimaryPlaneElement::Swapchain(ref element) = self.primary_element { + !self.supports_fencing || !element.sync.is_exportable() + } else { + false + } + } +} + +struct SwapchainElement<'a, 'b, B: Buffer> { + id: Id, + slot: &'a Slot, + transform: Transform, + damage: &'b DamageSnapshot, +} + +impl<'a, 'b, B: Buffer> Element for SwapchainElement<'a, 'b, B> { + fn id(&self) -> &Id { + &self.id + } + + fn current_commit(&self) -> CommitCounter { + self.damage.current_commit() + } + + fn src(&self) -> Rectangle { + Rectangle::from_loc_and_size((0, 0), self.slot.size()).to_f64() + } + + fn geometry(&self, _scale: Scale) -> Rectangle { + Rectangle::from_loc_and_size( + (0, 0), + self.slot.size().to_logical(1, self.transform).to_physical(1), + ) + } + + fn transform(&self) -> Transform { + self.transform + } + + fn damage_since(&self, scale: Scale, commit: Option) -> DamageSet { + self.damage + .damage_since(commit) + .map(|d| { + d.into_iter() + .map(|d| d.to_logical(1, self.transform, &self.slot.size()).to_physical(1)) + .collect() + }) + .unwrap_or_else(|| DamageSet::from_slice(&[self.geometry(scale)])) + } + + fn opaque_regions(&self, scale: Scale) -> OpaqueRegions { + OpaqueRegions::from_slice(&[self.geometry(scale)]) + } +} + +enum FrameResultDamageElement<'a, 'b, E, B: Buffer> { + Element(&'a E), + Swapchain(SwapchainElement<'a, 'b, B>), +} + +impl<'a, 'b, E, B> Element for FrameResultDamageElement<'a, 'b, E, B> +where + E: Element, + B: Buffer, +{ + fn id(&self) -> &Id { + match self { + FrameResultDamageElement::Element(e) => e.id(), + FrameResultDamageElement::Swapchain(e) => e.id(), + } + } + + fn current_commit(&self) -> CommitCounter { + match self { + FrameResultDamageElement::Element(e) => e.current_commit(), + FrameResultDamageElement::Swapchain(e) => e.current_commit(), + } + } + + fn src(&self) -> Rectangle { + match self { + FrameResultDamageElement::Element(e) => e.src(), + FrameResultDamageElement::Swapchain(e) => e.src(), + } + } + + fn geometry(&self, scale: Scale) -> Rectangle { + match self { + FrameResultDamageElement::Element(e) => e.geometry(scale), + FrameResultDamageElement::Swapchain(e) => e.geometry(scale), + } + } + + fn location(&self, scale: Scale) -> Point { + match self { + FrameResultDamageElement::Element(e) => e.location(scale), + FrameResultDamageElement::Swapchain(e) => e.location(scale), + } + } + + fn transform(&self) -> Transform { + match self { + FrameResultDamageElement::Element(e) => e.transform(), + FrameResultDamageElement::Swapchain(e) => e.transform(), + } + } + + fn damage_since(&self, scale: Scale, commit: Option) -> DamageSet { + match self { + FrameResultDamageElement::Element(e) => e.damage_since(scale, commit), + FrameResultDamageElement::Swapchain(e) => e.damage_since(scale, commit), + } + } + + fn opaque_regions(&self, scale: Scale) -> OpaqueRegions { + match self { + FrameResultDamageElement::Element(e) => e.opaque_regions(scale), + FrameResultDamageElement::Swapchain(e) => e.opaque_regions(scale), + } + } +} + +#[derive(Debug)] +/// Defines the element for the primary plane +pub enum PrimaryPlaneElement<'a, B: Buffer, F: Framebuffer, E> { + /// A slot from the swapchain was used for rendering + /// the primary plane + Swapchain(PrimarySwapchainElement), + /// An element has been assigned for direct scan-out + Element(&'a E), +} + +/// Error for [`RenderFrameResult::blit_frame_result`] +#[derive(Debug, thiserror::Error)] +pub enum BlitFrameResultError { + /// A render error occurred + #[error(transparent)] + Rendering(R), + /// A error occurred during exporting the buffer + #[error(transparent)] + Export(E), +} + +impl<'a, B, F, E> RenderFrameResult<'a, B, F, E> +where + B: Buffer, + F: Framebuffer, +{ + /// Get the damage of this frame for the specified dtr and age + pub fn damage_from_age<'d>( + &self, + damage_tracker: &'d mut OutputDamageTracker, + age: usize, + filter: impl IntoIterator, + ) -> Result<(Option<&'d Vec>>, RenderElementStates), OutputNoMode> + where + E: Element, + { + #[allow(clippy::mutable_key_type)] + let filter_ids: HashSet = filter.into_iter().collect(); + + let mut elements: Vec> = + Vec::with_capacity(usize::from(self.cursor_element.is_some()) + self.overlay_elements.len() + 1); + if let Some(cursor) = self.cursor_element { + if !filter_ids.contains(cursor.id()) { + elements.push(FrameResultDamageElement::Element(cursor)); + } + } + + elements.extend( + self.overlay_elements + .iter() + .filter(|e| !filter_ids.contains(e.id())) + .map(|e| FrameResultDamageElement::Element(*e)), + ); + + let primary_render_element = match &self.primary_element { + PrimaryPlaneElement::Swapchain(PrimarySwapchainElement { + slot, + transform, + damage, + .. + }) => FrameResultDamageElement::Swapchain(SwapchainElement { + id: self.primary_plane_element_id.clone(), + transform: *transform, + slot: match &slot.buffer { + ScanoutBuffer::Swapchain(slot) => slot, + _ => unreachable!(), + }, + damage, + }), + PrimaryPlaneElement::Element(e) => FrameResultDamageElement::Element(*e), + }; + + elements.push(primary_render_element); + + damage_tracker.damage_output(age, &elements) + } +} + +impl<'a, B, F, E> RenderFrameResult<'a, B, F, E> +where + B: Buffer + AsDmabuf, + ::Error: std::fmt::Debug, + F: Framebuffer, +{ + /// Blit the frame result into a currently bound buffer + #[allow(clippy::too_many_arguments)] + pub fn blit_frame_result( + &self, + size: impl Into>, + transform: Transform, + scale: impl Into>, + renderer: &mut R, + damage: impl IntoIterator>, + filter: impl IntoIterator, + ) -> Result::Error, ::Error>> + where + R: Renderer + Blit, + ::TextureId: 'static, + E: Element + RenderElement, + { + let size = size.into(); + let scale = scale.into(); + #[allow(clippy::mutable_key_type)] + let filter_ids: HashSet = filter.into_iter().collect(); + let damage = damage.into_iter().collect::>(); + + // If we have no damage we can exit early + if damage.is_empty() { + return Ok(SyncPoint::signaled()); + } + + let mut opaque_regions: Vec> = Vec::new(); + + let mut elements_to_render: Vec<&'a E> = + Vec::with_capacity(usize::from(self.cursor_element.is_some()) + self.overlay_elements.len() + 1); + + if let Some(cursor_element) = self.cursor_element.as_ref() { + if !filter_ids.contains(cursor_element.id()) { + elements_to_render.push(*cursor_element); + opaque_regions.extend(cursor_element.opaque_regions(scale)); + } + } + + for element in self + .overlay_elements + .iter() + .filter(|e| !filter_ids.contains(e.id())) + { + elements_to_render.push(element); + opaque_regions.extend(element.opaque_regions(scale)); + } + + let primary_dmabuf = match &self.primary_element { + PrimaryPlaneElement::Swapchain(PrimarySwapchainElement { slot, sync, .. }) => { + let dmabuf = match &slot.buffer { + ScanoutBuffer::Swapchain(slot) => slot.export().map_err(BlitFrameResultError::Export)?, + _ => unreachable!(), + }; + let size = dmabuf.size(); + let geometry = Rectangle::from_loc_and_size( + (0, 0), + size.to_logical(1, Transform::Normal).to_physical(1), + ); + opaque_regions.push(geometry); + Some((sync.clone(), dmabuf, geometry)) + } + PrimaryPlaneElement::Element(e) => { + elements_to_render.push(*e); + opaque_regions.extend(e.opaque_regions(scale)); + None + } + }; + + let clear_damage = + Rectangle::subtract_rects_many_in_place(damage.clone(), opaque_regions.iter().copied()); + + let mut sync: Option = None; + if !clear_damage.is_empty() { + tracing::trace!("clearing frame damage {:#?}", clear_damage); + + let mut frame = renderer + .render(size, transform) + .map_err(BlitFrameResultError::Rendering)?; + + frame + .clear(Color32F::BLACK, &clear_damage) + .map_err(BlitFrameResultError::Rendering)?; + + sync = Some(frame.finish().map_err(BlitFrameResultError::Rendering)?); + } + + // first do the potential blit + if let Some((sync, dmabuf, geometry)) = primary_dmabuf { + let blit_damage = damage + .iter() + .filter_map(|d| d.intersection(geometry)) + .collect::>(); + + tracing::trace!("blitting frame with damage: {:#?}", blit_damage); + + renderer.wait(&sync).map_err(BlitFrameResultError::Rendering)?; + for rect in blit_damage { + renderer + .blit_from( + dmabuf.clone(), + rect, + rect, + crate::backend::renderer::TextureFilter::Linear, + ) + .map_err(BlitFrameResultError::Rendering)?; + } + } + + // then render the remaining elements if any + if !elements_to_render.is_empty() { + tracing::trace!("drawing {} frame element(s)", elements_to_render.len()); + + let mut frame = renderer + .render(size, transform) + .map_err(BlitFrameResultError::Rendering)?; + + for element in elements_to_render.iter().rev() { + let src = element.src(); + let dst = element.geometry(scale); + let element_damage = damage + .iter() + .filter_map(|d| { + d.intersection(dst).map(|mut d| { + d.loc -= dst.loc; + d + }) + }) + .collect::>(); + + // no need to render without damage + if element_damage.is_empty() { + continue; + } + + tracing::trace!("drawing frame element with damage: {:#?}", element_damage); + + element + .draw(&mut frame, src, dst, &element_damage, &[]) + .map_err(BlitFrameResultError::Rendering)?; + } + + Ok(frame.finish().map_err(BlitFrameResultError::Rendering)?) + } else { + Ok(sync.unwrap_or_default()) + } + } +} + +impl<'a, B: Buffer + std::fmt::Debug, F: Framebuffer + std::fmt::Debug, E: std::fmt::Debug> std::fmt::Debug + for RenderFrameResult<'a, B, F, E> +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RenderFrameResult") + .field("is_empty", &self.is_empty) + .field("states", &self.states) + .field("primary_element", &self.primary_element) + .field("overlay_elements", &self.overlay_elements) + .field("cursor_element", &self.cursor_element) + .finish() + } +} + +#[derive(Debug)] +/// Defines the element for the primary plane in cases where a composited buffer was used. +pub struct PrimarySwapchainElement { + /// The slot from the swapchain + pub(super) slot: DrmScanoutBuffer, + /// Sync point + pub sync: SyncPoint, + /// The transform applied during rendering + pub transform: Transform, + /// The damage on the primary plane + pub damage: DamageSnapshot, +} + +impl PrimarySwapchainElement { + /// Access the underlying swapchain buffer + #[inline] + pub fn buffer(&self) -> &B { + match &self.slot.buffer { + ScanoutBuffer::Swapchain(slot) => slot, + _ => unreachable!(), + } + } +} diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index 0ea05634a64d..ac8ea1f0673a 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -120,11 +120,10 @@ //! } //! ``` use std::{ - collections::{HashMap, HashSet}, + collections::HashMap, fmt::Debug, io::ErrorKind, os::unix::io::{AsFd, OwnedFd}, - rc::Rc, str::FromStr, sync::Arc, }; @@ -161,12 +160,12 @@ use crate::{ RenderElementStates, RenderingReason, UnderlyingStorage, }, sync::SyncPoint, - utils::{CommitCounter, DamageBag, DamageSet, DamageSnapshot, OpaqueRegions}, - Bind, Blit, Color32F, DebugFlags, Frame as RendererFrame, Renderer, Texture, + utils::{CommitCounter, DamageBag}, + Bind, Color32F, DebugFlags, Frame as RendererFrame, Renderer, Texture, }, SwapBuffersError, }, - output::{OutputModeSource, OutputNoMode}, + output::OutputModeSource, utils::{Buffer as BufferCoords, DevPath, Physical, Point, Rectangle, Scale, Size, Transform}, wayland::{shm, single_pixel_buffer}, }; @@ -179,8 +178,10 @@ use super::{ }; mod elements; +mod frame_result; use elements::*; +pub use frame_result::*; impl RenderElementState { pub(crate) fn zero_copy(visible_area: usize) -> Self { @@ -202,8 +203,18 @@ impl RenderElementState { #[derive(Debug)] enum ScanoutBuffer { Wayland(crate::backend::renderer::utils::Buffer), - Swapchain(Slot), - Cursor(GbmBuffer), + Swapchain(Arc>), + Cursor(Arc), +} + +impl Clone for ScanoutBuffer { + fn clone(&self) -> Self { + match self { + Self::Wayland(arg0) => Self::Wayland(arg0.clone()), + Self::Swapchain(arg0) => Self::Swapchain(arg0.clone()), + Self::Cursor(arg0) => Self::Cursor(arg0.clone()), + } + } } impl ScanoutBuffer { @@ -277,7 +288,16 @@ where struct DrmScanoutBuffer { buffer: ScanoutBuffer, - fb: OwnedFramebuffer>, + fb: CachedDrmFramebuffer, +} + +impl Clone for DrmScanoutBuffer { + fn clone(&self) -> Self { + DrmScanoutBuffer { + buffer: self.buffer.clone(), + fb: self.fb.clone(), + } + } } impl std::fmt::Debug for DrmScanoutBuffer @@ -377,7 +397,7 @@ where fb_cache: SmallVec< [( ElementFramebufferCacheKey, - Result, ExportBufferError>, + Result, ExportBufferError>, ); 4], >, } @@ -390,7 +410,7 @@ where fn get( &self, cache_key: &ElementFramebufferCacheKey, - ) -> Option, ExportBufferError>> { + ) -> Option, ExportBufferError>> { self.fb_cache.iter().find_map(|(k, r)| { if k == cache_key { Some(r.as_ref().map_err(|err| *err)) @@ -404,7 +424,7 @@ where fn insert( &mut self, cache_key: ElementFramebufferCacheKey, - fb: Result, ExportBufferError>, + fb: Result, ExportBufferError>, ) { self.fb_cache.push((cache_key, fb)); } @@ -446,31 +466,31 @@ impl PlaneProperties { } } -struct ElementPlaneConfig<'a, B> { +struct ElementPlaneConfig<'a, B: Buffer, F: Framebuffer> { z_index: usize, geometry: Rectangle, properties: PlaneProperties, - buffer: Owned, + buffer: DrmScanoutBuffer, failed_planes: &'a mut PlanesSnapshot, } #[derive(Debug)] -struct PlaneConfig { +struct PlaneConfig { pub properties: PlaneProperties, - pub buffer: Owned, + pub buffer: DrmScanoutBuffer, pub damage_clips: Option, pub plane_claim: PlaneClaim, pub sync: Option<(SyncPoint, Option>)>, } -impl PlaneConfig { +impl PlaneConfig { #[inline] - pub fn is_compatible(&self, other: &PlaneConfig) -> bool { + pub fn is_compatible(&self, other: &PlaneConfig) -> bool { self.properties.is_compatible(&other.properties) } } -impl Clone for PlaneConfig { +impl Clone for PlaneConfig { #[inline] fn clone(&self) -> Self { Self { @@ -491,14 +511,14 @@ struct PlaneElementState { } #[derive(Debug)] -struct PlaneState { +struct PlaneState { skip: bool, needs_test: bool, element_state: Option, - config: Option>, + config: Option>, } -impl Default for PlaneState { +impl Default for PlaneState { #[inline] fn default() -> Self { Self { @@ -510,10 +530,10 @@ impl Default for PlaneState { } } -impl PlaneState { +impl PlaneState { #[inline] - fn buffer(&self) -> Option<&B> { - self.config.as_ref().map(|config| &*config.buffer) + fn buffer(&self) -> Option<&DrmScanoutBuffer> { + self.config.as_ref().map(|config| &config.buffer) } #[inline] @@ -526,7 +546,7 @@ impl PlaneState { } } -impl Clone for PlaneState { +impl Clone for PlaneState { #[inline] fn clone(&self) -> Self { Self { @@ -539,11 +559,11 @@ impl Clone for PlaneState { } #[derive(Debug)] -struct FrameState> { - planes: SmallVec<[(plane::Handle, PlaneState); 10]>, +struct FrameState { + planes: SmallVec<[(plane::Handle, PlaneState); 10]>, } -impl> FrameState { +impl FrameState { #[inline] fn is_assigned(&self, handle: plane::Handle) -> bool { self.planes @@ -573,14 +593,14 @@ impl> FrameState { } #[inline] - fn plane_state(&self, handle: plane::Handle) -> Option<&PlaneState> { + fn plane_state(&self, handle: plane::Handle) -> Option<&PlaneState> { self.planes .iter() .find_map(|(p, state)| if *p == handle { Some(state) } else { None }) } #[inline] - fn plane_state_mut(&mut self, handle: plane::Handle) -> Option<&mut PlaneState> { + fn plane_state_mut(&mut self, handle: plane::Handle) -> Option<&mut PlaneState> { self.planes .iter_mut() .find_map(|(p, state)| if *p == handle { Some(state) } else { None }) @@ -594,59 +614,13 @@ impl> FrameState { } #[inline] - fn plane_buffer(&self, handle: plane::Handle) -> Option<&B> { + fn plane_buffer(&self, handle: plane::Handle) -> Option<&DrmScanoutBuffer> { self.plane_state(handle) - .and_then(|state| state.config.as_ref().map(|config| &*config.buffer)) - } -} - -#[derive(Debug)] -struct Owned(Rc); - -impl std::ops::Deref for Owned { - type Target = B; - - #[inline] - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl From for Owned { - #[inline] - fn from(outer: B) -> Self { - Self(Rc::new(outer)) + .and_then(|state| state.config.as_ref().map(|config| &config.buffer)) } } -impl AsRef for Owned -where - B: Framebuffer, -{ - #[inline] - fn as_ref(&self) -> &framebuffer::Handle { - (*self.0).as_ref() - } -} - -impl Framebuffer for Owned -where - B: Framebuffer, -{ - #[inline] - fn format(&self) -> drm_fourcc::DrmFormat { - (*self.0).format() - } -} - -impl Clone for Owned { - #[inline] - fn clone(&self) -> Self { - Self(self.0.clone()) - } -} - -impl FrameState { +impl FrameState { fn from_planes(primary_plane: plane::Handle, planes: &Planes) -> Self { let mut tmp = SmallVec::with_capacity(planes.overlay.len() + planes.cursor.len() + 1); tmp.push((primary_plane, PlaneState::default())); @@ -667,10 +641,10 @@ impl FrameState { } } -impl FrameState { +impl FrameState { #[profiling::function] #[inline] - fn set_state(&mut self, plane: plane::Handle, state: PlaneState) { + fn set_state(&mut self, plane: plane::Handle, state: PlaneState) { let current_config = match self.plane_state_mut(plane) { Some(config) => config, None => return, @@ -684,7 +658,7 @@ impl FrameState { surface: &DrmSurface, supports_fencing: bool, plane: plane::Handle, - state: PlaneState, + state: PlaneState, allow_modeset: bool, ) -> Result<(), DrmError> { let current_config = match self.plane_state_mut(plane) { @@ -843,12 +817,8 @@ impl FrameState { } } -type Frame = FrameState< - DrmScanoutBuffer< - ::Buffer, - ::Buffer>>::Framebuffer, - >, ->; +type CompositorFrameState = + FrameState<::Buffer, ::Buffer>>::Framebuffer>; type FrameErrorType = FrameError< ::Error, @@ -856,445 +826,15 @@ type FrameErrorType = FrameError< ::Buffer>>::Error, >; -type FrameResult = Result>; +pub(crate) type FrameResult = Result>; -type RenderFrameErrorType = RenderFrameError< +pub(crate) type RenderFrameErrorType = RenderFrameError< ::Error, <::Buffer as AsDmabuf>::Error, ::Buffer>>::Error, R, >; -#[derive(Debug)] -/// Defines the element for the primary plane in cases where a composited buffer was used. -pub struct PrimarySwapchainElement { - /// The slot from the swapchain - slot: Owned>, - /// Sync point - pub sync: SyncPoint, - /// The transform applied during rendering - pub transform: Transform, - /// The damage on the primary plane - pub damage: DamageSnapshot, -} - -impl PrimarySwapchainElement { - /// Access the underlying swapchain buffer - #[inline] - pub fn buffer(&self) -> &B { - match &self.slot.0.buffer { - ScanoutBuffer::Swapchain(slot) => slot, - _ => unreachable!(), - } - } -} - -#[derive(Debug)] -/// Defines the element for the primary plane -pub enum PrimaryPlaneElement<'a, B: Buffer, F: Framebuffer, E> { - /// A slot from the swapchain was used for rendering - /// the primary plane - Swapchain(PrimarySwapchainElement), - /// An element has been assigned for direct scan-out - Element(&'a E), -} - -/// Result for [`DrmCompositor::render_frame`] -/// -/// **Note**: This struct may contain a reference to the composited buffer -/// of the primary display plane. Dropping it will remove said reference and -/// allows the buffer to be reused. -/// -/// Keeping the buffer longer may cause the following issues: -/// - **Too much damage** - until the buffer is marked free it is not considered -/// submitted by the swapchain, causing the age value of newly queried buffers -/// to be lower than necessary, potentially resulting in more rendering than necessary. -/// To avoid this make sure the buffer is dropped before starting the next render. -/// - **Exhaustion of swapchain images** - Continuing rendering while holding on -/// to too many buffers may cause the swapchain to run out of images, returning errors -/// on rendering until buffers are freed again. The exact amount of images in a -/// swapchain is an implementation detail, but should generally be expect to be -/// large enough to hold onto at least one `RenderFrameResult`. -pub struct RenderFrameResult<'a, B: Buffer, F: Framebuffer, E> { - /// If this frame contains any changes and should be submitted - pub is_empty: bool, - /// The render element states of this frame - pub states: RenderElementStates, - /// Element for the primary plane - pub primary_element: PrimaryPlaneElement<'a, B, F, E>, - /// Overlay elements in front to back order - pub overlay_elements: Vec<&'a E>, - /// Optional cursor plane element - /// - /// If set always above all other elements - pub cursor_element: Option<&'a E>, - - primary_plane_element_id: Id, - supports_fencing: bool, -} - -impl RenderFrameResult<'_, B, F, E> { - /// Returns if synchronization with kms submission can't be guaranteed through the available apis. - pub fn needs_sync(&self) -> bool { - if let PrimaryPlaneElement::Swapchain(ref element) = self.primary_element { - !self.supports_fencing || !element.sync.is_exportable() - } else { - false - } - } -} - -struct SwapchainElement<'a, 'b, B: Buffer> { - id: Id, - slot: &'a Slot, - transform: Transform, - damage: &'b DamageSnapshot, -} - -impl Element for SwapchainElement<'_, '_, B> { - fn id(&self) -> &Id { - &self.id - } - - fn current_commit(&self) -> CommitCounter { - self.damage.current_commit() - } - - fn src(&self) -> Rectangle { - Rectangle::from_loc_and_size((0, 0), self.slot.size()).to_f64() - } - - fn geometry(&self, _scale: Scale) -> Rectangle { - Rectangle::from_loc_and_size( - (0, 0), - self.slot.size().to_logical(1, self.transform).to_physical(1), - ) - } - - fn transform(&self) -> Transform { - self.transform - } - - fn damage_since(&self, scale: Scale, commit: Option) -> DamageSet { - self.damage - .damage_since(commit) - .map(|d| { - d.into_iter() - .map(|d| d.to_logical(1, self.transform, &self.slot.size()).to_physical(1)) - .collect() - }) - .unwrap_or_else(|| DamageSet::from_slice(&[self.geometry(scale)])) - } - - fn opaque_regions(&self, scale: Scale) -> OpaqueRegions { - OpaqueRegions::from_slice(&[self.geometry(scale)]) - } -} - -enum FrameResultDamageElement<'a, 'b, E, B: Buffer> { - Element(&'a E), - Swapchain(SwapchainElement<'a, 'b, B>), -} - -impl Element for FrameResultDamageElement<'_, '_, E, B> -where - E: Element, - B: Buffer, -{ - fn id(&self) -> &Id { - match self { - FrameResultDamageElement::Element(e) => e.id(), - FrameResultDamageElement::Swapchain(e) => e.id(), - } - } - - fn current_commit(&self) -> CommitCounter { - match self { - FrameResultDamageElement::Element(e) => e.current_commit(), - FrameResultDamageElement::Swapchain(e) => e.current_commit(), - } - } - - fn src(&self) -> Rectangle { - match self { - FrameResultDamageElement::Element(e) => e.src(), - FrameResultDamageElement::Swapchain(e) => e.src(), - } - } - - fn geometry(&self, scale: Scale) -> Rectangle { - match self { - FrameResultDamageElement::Element(e) => e.geometry(scale), - FrameResultDamageElement::Swapchain(e) => e.geometry(scale), - } - } - - fn location(&self, scale: Scale) -> Point { - match self { - FrameResultDamageElement::Element(e) => e.location(scale), - FrameResultDamageElement::Swapchain(e) => e.location(scale), - } - } - - fn transform(&self) -> Transform { - match self { - FrameResultDamageElement::Element(e) => e.transform(), - FrameResultDamageElement::Swapchain(e) => e.transform(), - } - } - - fn damage_since(&self, scale: Scale, commit: Option) -> DamageSet { - match self { - FrameResultDamageElement::Element(e) => e.damage_since(scale, commit), - FrameResultDamageElement::Swapchain(e) => e.damage_since(scale, commit), - } - } - - fn opaque_regions(&self, scale: Scale) -> OpaqueRegions { - match self { - FrameResultDamageElement::Element(e) => e.opaque_regions(scale), - FrameResultDamageElement::Swapchain(e) => e.opaque_regions(scale), - } - } -} - -/// Error for [`RenderFrameResult::blit_frame_result`] -#[derive(Debug, thiserror::Error)] -pub enum BlitFrameResultError { - /// A render error occurred - #[error(transparent)] - Rendering(R), - /// A error occurred during exporting the buffer - #[error(transparent)] - Export(E), -} - -impl RenderFrameResult<'_, B, F, E> -where - B: Buffer, - F: Framebuffer, -{ - /// Get the damage of this frame for the specified dtr and age - pub fn damage_from_age<'d>( - &self, - damage_tracker: &'d mut OutputDamageTracker, - age: usize, - filter: impl IntoIterator, - ) -> Result<(Option<&'d Vec>>, RenderElementStates), OutputNoMode> - where - E: Element, - { - #[allow(clippy::mutable_key_type)] - let filter_ids: HashSet = filter.into_iter().collect(); - - let mut elements: Vec> = - Vec::with_capacity(usize::from(self.cursor_element.is_some()) + self.overlay_elements.len() + 1); - if let Some(cursor) = self.cursor_element { - if !filter_ids.contains(cursor.id()) { - elements.push(FrameResultDamageElement::Element(cursor)); - } - } - - elements.extend( - self.overlay_elements - .iter() - .filter(|e| !filter_ids.contains(e.id())) - .map(|e| FrameResultDamageElement::Element(*e)), - ); - - let primary_render_element = match &self.primary_element { - PrimaryPlaneElement::Swapchain(PrimarySwapchainElement { - slot, - transform, - damage, - .. - }) => FrameResultDamageElement::Swapchain(SwapchainElement { - id: self.primary_plane_element_id.clone(), - transform: *transform, - slot: match &slot.0.buffer { - ScanoutBuffer::Swapchain(slot) => slot, - _ => unreachable!(), - }, - damage, - }), - PrimaryPlaneElement::Element(e) => FrameResultDamageElement::Element(*e), - }; - - elements.push(primary_render_element); - - damage_tracker.damage_output(age, &elements) - } -} - -impl<'a, B, F, E> RenderFrameResult<'a, B, F, E> -where - B: Buffer + AsDmabuf, - ::Error: std::fmt::Debug, - F: Framebuffer, -{ - /// Blit the frame result into a currently bound buffer - #[allow(clippy::too_many_arguments)] - pub fn blit_frame_result( - &self, - size: impl Into>, - transform: Transform, - scale: impl Into>, - renderer: &mut R, - damage: impl IntoIterator>, - filter: impl IntoIterator, - ) -> Result::Error, ::Error>> - where - R: Renderer + Blit, - ::TextureId: 'static, - E: Element + RenderElement, - { - let size = size.into(); - let scale = scale.into(); - #[allow(clippy::mutable_key_type)] - let filter_ids: HashSet = filter.into_iter().collect(); - let damage = damage.into_iter().collect::>(); - - // If we have no damage we can exit early - if damage.is_empty() { - return Ok(SyncPoint::signaled()); - } - - let mut opaque_regions: Vec> = Vec::new(); - - let mut elements_to_render: Vec<&'a E> = - Vec::with_capacity(usize::from(self.cursor_element.is_some()) + self.overlay_elements.len() + 1); - - if let Some(cursor_element) = self.cursor_element.as_ref() { - if !filter_ids.contains(cursor_element.id()) { - elements_to_render.push(*cursor_element); - opaque_regions.extend(cursor_element.opaque_regions(scale)); - } - } - - for element in self - .overlay_elements - .iter() - .filter(|e| !filter_ids.contains(e.id())) - { - elements_to_render.push(element); - opaque_regions.extend(element.opaque_regions(scale)); - } - - let primary_dmabuf = match &self.primary_element { - PrimaryPlaneElement::Swapchain(PrimarySwapchainElement { slot, sync, .. }) => { - let dmabuf = match &slot.0.buffer { - ScanoutBuffer::Swapchain(slot) => slot.export().map_err(BlitFrameResultError::Export)?, - _ => unreachable!(), - }; - let size = dmabuf.size(); - let geometry = Rectangle::from_loc_and_size( - (0, 0), - size.to_logical(1, Transform::Normal).to_physical(1), - ); - opaque_regions.push(geometry); - Some((sync.clone(), dmabuf, geometry)) - } - PrimaryPlaneElement::Element(e) => { - elements_to_render.push(*e); - opaque_regions.extend(e.opaque_regions(scale)); - None - } - }; - - let clear_damage = - Rectangle::subtract_rects_many_in_place(damage.clone(), opaque_regions.iter().copied()); - - let mut sync: Option = None; - if !clear_damage.is_empty() { - trace!("clearing frame damage {:#?}", clear_damage); - - let mut frame = renderer - .render(size, transform) - .map_err(BlitFrameResultError::Rendering)?; - - frame - .clear(Color32F::BLACK, &clear_damage) - .map_err(BlitFrameResultError::Rendering)?; - - sync = Some(frame.finish().map_err(BlitFrameResultError::Rendering)?); - } - - // first do the potential blit - if let Some((sync, dmabuf, geometry)) = primary_dmabuf { - let blit_damage = damage - .iter() - .filter_map(|d| d.intersection(geometry)) - .collect::>(); - - trace!("blitting frame with damage: {:#?}", blit_damage); - - renderer.wait(&sync).map_err(BlitFrameResultError::Rendering)?; - for rect in blit_damage { - renderer - .blit_from( - dmabuf.clone(), - rect, - rect, - crate::backend::renderer::TextureFilter::Linear, - ) - .map_err(BlitFrameResultError::Rendering)?; - } - } - - // then render the remaining elements if any - if !elements_to_render.is_empty() { - trace!("drawing {} frame element(s)", elements_to_render.len()); - - let mut frame = renderer - .render(size, transform) - .map_err(BlitFrameResultError::Rendering)?; - - for element in elements_to_render.iter().rev() { - let src = element.src(); - let dst = element.geometry(scale); - let element_damage = damage - .iter() - .filter_map(|d| { - d.intersection(dst).map(|mut d| { - d.loc -= dst.loc; - d - }) - }) - .collect::>(); - - // no need to render without damage - if element_damage.is_empty() { - continue; - } - - trace!("drawing frame element with damage: {:#?}", element_damage); - - element - .draw(&mut frame, src, dst, &element_damage, &[]) - .map_err(BlitFrameResultError::Rendering)?; - } - - Ok(frame.finish().map_err(BlitFrameResultError::Rendering)?) - } else { - Ok(sync.unwrap_or_default()) - } - } -} - -impl std::fmt::Debug - for RenderFrameResult<'_, B, F, E> -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("RenderFrameResult") - .field("is_empty", &self.is_empty) - .field("states", &self.states) - .field("primary_element", &self.primary_element) - .field("overlay_elements", &self.overlay_elements) - .field("cursor_element", &self.cursor_element) - .finish() - } -} - #[derive(Debug)] struct CursorState { allocator: GbmAllocator, @@ -1395,7 +935,7 @@ impl From<&PlaneInfo> for PlaneAssignment { } struct PendingFrame::Buffer>, U> { - frame: Frame, + frame: CompositorFrameState, user_data: U, } @@ -1443,7 +983,7 @@ enum PreparedFrameKind { } struct PreparedFrame::Buffer>> { - frame: Frame, + frame: CompositorFrameState, kind: PreparedFrameKind, } @@ -1497,7 +1037,7 @@ where framebuffer_exporter: F, - current_frame: Frame, + current_frame: CompositorFrameState, pending_frame: Option>, queued_frame: Option>, next_frame: Option>, @@ -1507,10 +1047,8 @@ where cursor_size: Size, cursor_state: Option>, - element_states: - IndexMap>::Framebuffer>>>, - previous_element_states: - IndexMap>::Framebuffer>>>, + element_states: IndexMap>::Framebuffer>>, + previous_element_states: IndexMap>::Framebuffer>>, opaque_regions: Vec>, element_opaque_regions_workhouse: Vec>, @@ -1730,7 +1268,7 @@ where framebuffer_exporter: &F, mut renderer_formats: Vec, code: DrmFourcc, - ) -> Result<(Swapchain, Frame, bool), (A, FrameErrorType)> { + ) -> Result<(Swapchain, FrameState, bool), (A, FrameErrorType)> { // select a format let mut plane_formats = drm.plane_info().formats.iter().copied().collect::>(); @@ -1832,14 +1370,15 @@ where Ok(None) => return Err((swapchain.allocator, FrameError::NoFramebuffer)), Err(err) => return Err((swapchain.allocator, FrameError::FramebufferExport(err))), }; - buffer - .userdata() - .insert_if_missing(|| OwnedFramebuffer::new(DrmFramebuffer::Exporter(fb_buffer))); + buffer.userdata().insert_if_missing(|| { + let init = CachedDrmFramebuffer::new(DrmFramebuffer::Exporter(fb_buffer)); + init + }); let mode = drm.pending_mode(); let handle = buffer .userdata() - .get::>::Framebuffer>>>() + .get::>::Framebuffer>>() .unwrap() .clone(); @@ -1866,10 +1405,10 @@ where alpha: 1.0, format: buffer.format(), }, - buffer: Owned::from(DrmScanoutBuffer { - buffer: ScanoutBuffer::Swapchain(buffer), + buffer: DrmScanoutBuffer { + buffer: ScanoutBuffer::Swapchain(Arc::new(buffer)), fb: handle, - }), + }, damage_clips: None, plane_claim, sync: None, @@ -1957,7 +1496,7 @@ where // it and use the Slot userdata to cache it let maybe_buffer = primary_plane_buffer .userdata() - .get::>::Framebuffer>>>(); + .get::>::Framebuffer>>(); if maybe_buffer.is_none() { let fb_buffer = self .framebuffer_exporter @@ -1970,13 +1509,13 @@ where .ok_or(FrameError::NoFramebuffer)?; primary_plane_buffer .userdata() - .insert_if_missing(|| OwnedFramebuffer::new(DrmFramebuffer::Exporter(fb_buffer))); + .insert_if_missing(|| CachedDrmFramebuffer::new(DrmFramebuffer::Exporter(fb_buffer))); } // This unwrap is safe as we error out above if we were unable to export a framebuffer let fb = primary_plane_buffer .userdata() - .get::>::Framebuffer>>>() + .get::>::Framebuffer>>() .unwrap() .clone(); @@ -1990,7 +1529,10 @@ where // So first we want to create a clean state, for that we have to reset all overlay and cursor planes // to nothing. We only want to test if the primary plane alone can be used for scan-out. - let mut next_frame_state = { + let mut next_frame_state: FrameState< + ::Buffer, + ::Buffer>>::Framebuffer, + > = { let previous_state = self .pending_frame .as_ref() @@ -2022,7 +1564,10 @@ where error!("failed to claim primary plane"); FrameError::PrimaryPlaneClaimFailed })?; - let primary_plane_state = PlaneState { + let primary_plane_state: PlaneState< + ::Buffer, + ::Buffer>>::Framebuffer, + > = PlaneState { skip: false, needs_test: false, element_state: None, @@ -2035,10 +1580,10 @@ where alpha: 1.0, format: primary_plane_buffer.format(), }, - buffer: Owned::from(DrmScanoutBuffer { - buffer: ScanoutBuffer::Swapchain(primary_plane_buffer), + buffer: DrmScanoutBuffer { + buffer: ScanoutBuffer::Swapchain(Arc::new(primary_plane_buffer)), fb, - }), + }, damage_clips: None, plane_claim, sync: None, @@ -2370,7 +1915,7 @@ where let render = next_frame_state .plane_buffer(self.surface.plane()) - .map(|config| matches!(&config.buffer, ScanoutBuffer::Swapchain(_))) + .map(|config| matches!(config.buffer, ScanoutBuffer::Swapchain(_))) .unwrap_or(false); if render { @@ -2926,13 +2471,10 @@ where element_zindex: usize, element_geometry: Rectangle, element_is_opaque: bool, - element_states: &mut IndexMap< - Id, - ElementState>::Framebuffer>>, - >, + element_states: &mut IndexMap>::Framebuffer>>, primary_plane_elements: &[&'a E], scale: Scale, - frame_state: &mut Frame, + frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, try_assign_primary_plane: bool, @@ -3026,12 +2568,9 @@ where element: &'a E, element_zindex: usize, element_geometry: Rectangle, - element_states: &mut IndexMap< - Id, - ElementState>::Framebuffer>>, - >, + element_states: &mut IndexMap>::Framebuffer>>, scale: Scale, - frame_state: &mut Frame, + frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, ) -> Result> @@ -3106,7 +2645,7 @@ where element_zindex: usize, element_geometry: Rectangle, scale: Scale, - frame_state: &mut Frame, + frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, ) -> Option @@ -3459,10 +2998,10 @@ where transform: Transform::Normal, format: framebuffer.format(), }, - buffer: Owned::from(DrmScanoutBuffer { - buffer: ScanoutBuffer::Cursor(cursor_buffer), - fb: OwnedFramebuffer::new(DrmFramebuffer::Gbm(framebuffer)), - }), + buffer: DrmScanoutBuffer { + buffer: ScanoutBuffer::Cursor(Arc::new(cursor_buffer)), + fb: CachedDrmFramebuffer::new(DrmFramebuffer::Gbm(framebuffer)), + }, damage_clips: None, plane_claim, sync: None, @@ -3536,21 +3075,16 @@ where element: &E, element_zindex: usize, element_geometry: Rectangle, - element_states: &'a mut IndexMap< - Id, - ElementState>::Framebuffer>>, - >, - frame_state: &mut Frame, + element_states: &'a mut IndexMap>::Framebuffer>>, + frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, allow_opaque_fallback: bool, ) -> Result< ElementPlaneConfig< 'a, - DrmScanoutBuffer< - ::Buffer, - ::Buffer>>::Framebuffer, - >, + ::Buffer, + ::Buffer>>::Framebuffer, >, ExportBufferError, > @@ -3591,7 +3125,9 @@ where }, ); } - let element_fb_cache = element_states + let element_fb_cache: &mut ElementFramebufferCache< + ::Buffer>>::Framebuffer, + > = element_states .get_mut(element_id) .map(|state| &mut state.fb_cache) .unwrap(); @@ -3616,7 +3152,7 @@ where ExportBufferError::ExportFailed }) .and_then(|fb| { - fb.map(|fb| OwnedFramebuffer::new(DrmFramebuffer::Exporter(fb))) + fb.map(|fb| CachedDrmFramebuffer::new(DrmFramebuffer::Exporter(fb))) .ok_or(ExportBufferError::Unsupported) }); @@ -3637,7 +3173,8 @@ where ); } - let fb = element_fb_cache.get(&element_cache_key).unwrap()?; + let fb: &CachedDrmFramebuffer<::Buffer>>::Framebuffer> = + element_fb_cache.get(&element_cache_key).unwrap()?; let src = element.src(); let dst = output_transform.transform_rect_in(element_geometry, &output_geometry.size); @@ -3656,12 +3193,13 @@ where transform, format: fb.format(), }; - let buffer = ScanoutBuffer::from_underlying_storage(underlying_storage) - .map(|buffer| { - Owned::from(DrmScanoutBuffer { - fb: fb.clone(), - buffer, - }) + let buffer: DrmScanoutBuffer< + ::Buffer, + ::Buffer>>::Framebuffer, + > = ScanoutBuffer::from_underlying_storage(underlying_storage) + .map(|buffer| DrmScanoutBuffer { + fb: fb.clone(), + buffer, }) .ok_or(ExportBufferError::Unsupported)?; @@ -3795,13 +3333,10 @@ where element_zindex: usize, element_geometry: Rectangle, element_is_opaque: bool, - element_states: &mut IndexMap< - Id, - ElementState>::Framebuffer>>, - >, + element_states: &mut IndexMap>::Framebuffer>>, primary_plane_elements: &[&'a E], scale: Scale, - frame_state: &mut Frame, + frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, ) -> Result> @@ -3880,10 +3415,8 @@ where let mut test_overlay_plane = |plane: &PlaneInfo, element_config: &ElementPlaneConfig< '_, - DrmScanoutBuffer< - ::Buffer, - ::Buffer>>::Framebuffer, - >, + ::Buffer, + ::Buffer>>::Framebuffer, >| { // something is already assigned to our overlay plane if frame_state.is_assigned(plane.handle) { @@ -3996,14 +3529,12 @@ where element: &E, element_config: &ElementPlaneConfig< '_, - DrmScanoutBuffer< - ::Buffer, - ::Buffer>>::Framebuffer, - >, + ::Buffer, + ::Buffer>>::Framebuffer, >, plane: &PlaneInfo, scale: Scale, - frame_state: &mut Frame, + frame_state: &mut CompositorFrameState, ) -> Result> where R: Renderer, @@ -4359,43 +3890,43 @@ where } } -struct OwnedFramebuffer(Rc); +struct CachedDrmFramebuffer(Arc>); -impl PartialEq for OwnedFramebuffer { +impl PartialEq for CachedDrmFramebuffer { #[inline] fn eq(&self, other: &Self) -> bool { AsRef::::as_ref(&self) == AsRef::::as_ref(&other) } } -impl std::fmt::Debug for OwnedFramebuffer { +impl std::fmt::Debug for CachedDrmFramebuffer { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("OwnedFramebuffer").field(&self.0).finish() + f.debug_tuple("CachedDrmFramebuffer").field(&self.0).finish() } } -impl OwnedFramebuffer { +impl CachedDrmFramebuffer { #[inline] - fn new(buffer: B) -> Self { - OwnedFramebuffer(Rc::new(buffer)) + fn new(buffer: DrmFramebuffer) -> Self { + CachedDrmFramebuffer(Arc::new(buffer)) } } -impl Clone for OwnedFramebuffer { +impl Clone for CachedDrmFramebuffer { #[inline] fn clone(&self) -> Self { Self(self.0.clone()) } } -impl AsRef for OwnedFramebuffer { +impl AsRef for CachedDrmFramebuffer { #[inline] fn as_ref(&self) -> &framebuffer::Handle { (*self.0).as_ref() } } -impl Framebuffer for OwnedFramebuffer { +impl Framebuffer for CachedDrmFramebuffer { #[inline] fn format(&self) -> drm_fourcc::DrmFormat { (*self.0).format() From 6ba2e40dd721852f08993bf7111500c1a5d0a92a Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 28 Nov 2024 17:26:59 +0100 Subject: [PATCH 03/24] drm/compositor: Add FrameMode --- src/backend/drm/compositor/mod.rs | 52 ++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index ac8ea1f0673a..ec493b3fa25d 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -58,7 +58,7 @@ //! # use std::{collections::HashSet, mem::MaybeUninit}; //! # //! use smithay::{ -//! backend::drm::{compositor::DrmCompositor, DrmSurface}, +//! backend::drm::{compositor::{DrmCompositor, FrameMode}, DrmSurface}, //! output::{Output, PhysicalProperties, Subpixel}, //! utils::Size, //! }; @@ -104,7 +104,7 @@ //! //! # let elements: Vec> = Vec::new(); //! let render_frame_result = compositor -//! .render_frame::<_, _>(&mut renderer, &elements, CLEAR_COLOR) +//! .render_frame::<_, _>(&mut renderer, &elements, CLEAR_COLOR, FrameMode::ALL) //! .expect("failed to render frame"); //! //! if !render_frame_result.is_empty { @@ -1011,6 +1011,22 @@ where } } +bitflags::bitflags! { + /// Possible flags for a DMA buffer + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] + pub struct FrameMode: u32 { + /// Allow to realize the frame by compositing elements on the primary plane + const COMPOSITE = 1; + const PRIMARY_PLANE_SCANOUT = 2; + const OVERLAY_PLANE_SCANOUT = 4; + const CURSOR_PLANE_SCANOUT = 8; + /// Allow to realize the frame by assigning elements on planes + const SCANOUT = Self::PRIMARY_PLANE_SCANOUT.bits() | Self::OVERLAY_PLANE_SCANOUT.bits() | Self::CURSOR_PLANE_SCANOUT.bits(); + + const ALL = Self::COMPOSITE.bits() | Self::SCANOUT.bits(); + } +} + /// Composite an output using a combination of planes and rendering /// /// see the [`module docs`](crate::backend::drm::compositor) for more information @@ -1031,7 +1047,6 @@ where primary_plane_element_id: Id, primary_plane_damage_bag: DamageBag, supports_fencing: bool, - direct_scanout: bool, reset_pending: bool, signaled_fence: Option>, @@ -1216,7 +1231,6 @@ where primary_plane_element_id: Id::new(), primary_plane_damage_bag: DamageBag::new(4), primary_is_opaque: is_opaque, - direct_scanout: true, reset_pending: true, signaled_fence, current_frame, @@ -1253,13 +1267,6 @@ where Err(error.unwrap()) } - /// Enable or disable direct scanout. - /// - /// This is mostly useful for debugging purposes. - pub fn use_direct_scanout(&mut self, enabled: bool) { - self.direct_scanout = enabled; - } - fn find_supported_format( drm: Arc, supports_fencing: bool, @@ -1441,6 +1448,7 @@ where renderer: &mut R, elements: &'a [E], clear_color: impl Into, + frame_mode: FrameMode, ) -> Result, RenderFrameErrorType> where E: RenderElement, @@ -1777,6 +1785,7 @@ where output_transform, output_geometry, try_assign_primary_plane, + frame_mode, ) { Ok(direct_scan_out_plane) => { match direct_scan_out_plane.type_ { @@ -2478,13 +2487,14 @@ where output_transform: Transform, output_geometry: Rectangle, try_assign_primary_plane: bool, + frame_mode: FrameMode, ) -> Result> where R: Renderer + Bind, E: RenderElement, { // Check if we have a free plane, otherwise we can exit early - if !self.direct_scanout { + if !frame_mode.intersects(FrameMode::SCANOUT) { trace!( "skipping direct scan-out for element {:?}, no free planes", element.id() @@ -2505,6 +2515,7 @@ where frame_state, output_transform, output_geometry, + frame_mode, ) { Ok(plane) => { trace!( @@ -2527,6 +2538,7 @@ where frame_state, output_transform, output_geometry, + frame_mode, ) { trace!("assigned element {:?} to cursor {:?}", element.id(), plane.handle); return Ok(plane); @@ -2544,6 +2556,7 @@ where frame_state, output_transform, output_geometry, + frame_mode, ) { Ok(plane) => { trace!( @@ -2573,11 +2586,16 @@ where frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, + frame_mode: FrameMode, ) -> Result> where R: Renderer, E: RenderElement, { + if !frame_mode.contains(FrameMode::PRIMARY_PLANE_SCANOUT) { + return Err(None); + } + if frame_state .plane_state(self.surface.plane()) .map(|state| state.element_state.is_some()) @@ -2648,11 +2666,16 @@ where frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, + frame_mode: FrameMode, ) -> Option where R: Renderer, E: RenderElement, { + if !frame_mode.contains(FrameMode::CURSOR_PLANE_SCANOUT) { + return None; + } + let Some(cursor_state) = self.cursor_state.as_mut() else { trace!("no cursor state, skipping cursor rendering"); return None; @@ -3339,11 +3362,16 @@ where frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, + frame_mode: FrameMode, ) -> Result> where R: Renderer, E: RenderElement, { + if !frame_mode.contains(FrameMode::OVERLAY_PLANE_SCANOUT) { + return Err(None); + } + let element_id = element.id(); // Check if we have a free plane, otherwise we can exit early From 91493afa393ebf86df5d393f0a2be760fb817507 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 28 Nov 2024 17:29:29 +0100 Subject: [PATCH 04/24] drm: Don't define combound error types via `Renderer` --- src/backend/drm/compositor/mod.rs | 7 ++++--- src/backend/renderer/damage/mod.rs | 12 ++++++------ src/desktop/space/mod.rs | 2 +- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index ec493b3fa25d..438dc907fe14 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -832,7 +832,7 @@ pub(crate) type RenderFrameErrorType = RenderFrameError< ::Error, <::Buffer as AsDmabuf>::Error, ::Buffer>>::Error, - R, + ::Error, >; #[derive(Debug)] @@ -1454,6 +1454,7 @@ where E: RenderElement, R: Renderer + Bind, ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, { let mut clear_color = clear_color.into(); @@ -4009,7 +4010,7 @@ pub enum RenderFrameError< A: std::error::Error + Send + Sync + 'static, B: std::error::Error + Send + Sync + 'static, F: std::error::Error + Send + Sync + 'static, - R: Renderer, + R: std::error::Error + Send + Sync + 'static, > { /// Preparing the frame encountered an error #[error(transparent)] @@ -4024,7 +4025,7 @@ where A: std::error::Error + Send + Sync + 'static, B: std::error::Error + Send + Sync + 'static, F: std::error::Error + Send + Sync + 'static, - R: Renderer, + R: std::error::Error + Send + Sync + 'static, { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/src/backend/renderer/damage/mod.rs b/src/backend/renderer/damage/mod.rs index 1245f429fbe0..9790fb3b3fd1 100644 --- a/src/backend/renderer/damage/mod.rs +++ b/src/backend/renderer/damage/mod.rs @@ -302,10 +302,10 @@ pub struct OutputDamageTracker { /// Errors thrown by [`OutputDamageTracker::render_output`] #[derive(thiserror::Error)] -pub enum Error { +pub enum Error { /// The provided [`Renderer`] returned an error #[error(transparent)] - Rendering(R::Error), + Rendering(E), /// The given [`Output`] has no mode set #[error(transparent)] OutputNoMode(#[from] OutputNoMode), @@ -332,7 +332,7 @@ impl RenderOutputResult<'_> { } } -impl std::fmt::Debug for Error { +impl std::fmt::Debug for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Error::Rendering(err) => std::fmt::Debug::fmt(err, f), @@ -423,7 +423,7 @@ impl OutputDamageTracker { age: usize, elements: &[E], clear_color: Color32F, - ) -> Result, Error> + ) -> Result, Error> where E: RenderElement, R: Renderer + Bind, @@ -443,7 +443,7 @@ impl OutputDamageTracker { age: usize, elements: &[E], clear_color: impl Into, - ) -> Result, Error> + ) -> Result, Error> where E: RenderElement, R: Renderer, @@ -790,7 +790,7 @@ impl OutputDamageTracker { elements: &'e [E], clear_color: Color32F, pre_render: F, - ) -> Result, Error> + ) -> Result, Error> where E: RenderElement, R: Renderer, diff --git a/src/desktop/space/mod.rs b/src/desktop/space/mod.rs index 2f5c2e3a9858..2e893a54b320 100644 --- a/src/desktop/space/mod.rs +++ b/src/desktop/space/mod.rs @@ -683,7 +683,7 @@ pub fn render_output< custom_elements: &'a [C], damage_tracker: &'d mut OutputDamageTracker, clear_color: impl Into, -) -> Result, OutputDamageTrackerError> +) -> Result, OutputDamageTrackerError> where ::TextureId: Clone + Texture + 'static, >::RenderElement: 'a, From 07700f2712af9e9baadc26e5633e18bc5e1bba4f Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 28 Nov 2024 17:31:27 +0100 Subject: [PATCH 05/24] drm/compositor: Refactor format selection --- src/backend/drm/compositor/mod.rs | 351 ++++++++++++++++++++++++------ 1 file changed, 290 insertions(+), 61 deletions(-) diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index 438dc907fe14..6edaf0272987 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -1108,7 +1108,7 @@ where planes: Option, mut allocator: A, framebuffer_exporter: F, - color_formats: &[DrmFourcc], + color_formats: impl IntoIterator, renderer_formats: impl IntoIterator, cursor_size: Size, gbm: Option>, @@ -1200,9 +1200,9 @@ where allocator, &framebuffer_exporter, renderer_formats.clone(), - *format, + format, ) { - Ok((swapchain, current_frame, is_opaque)) => { + Ok((swapchain, is_opaque)) => { let cursor_state = gbm.map(|gbm| { #[cfg(feature = "renderer_pixman")] let pixman_renderer = match PixmanRenderer::new() { @@ -1226,6 +1226,7 @@ where }); let overlay_plane_element_ids = OverlayPlaneElementIds::from_planes(&planes); + let current_frame = FrameState::from_planes(surface.plane(), &planes); let drm_renderer = DrmCompositor { primary_plane_element_id: Id::new(), @@ -1267,16 +1268,170 @@ where Err(error.unwrap()) } - fn find_supported_format( - drm: Arc, + pub fn with_format( + output_mode_source: impl Into + Debug, + surface: DrmSurface, + planes: Option, + allocator: A, + framebuffer_exporter: F, + code: DrmFourcc, + modifiers: impl IntoIterator, + cursor_size: Size, + gbm: Option>, + ) -> FrameResult { + let signaled_fence = match surface.create_syncobj(true) { + Ok(signaled_syncobj) => match surface.syncobj_to_fd(signaled_syncobj, true) { + Ok(signaled_fence) => { + let _ = surface.destroy_syncobj(signaled_syncobj); + Some(Arc::new(signaled_fence)) + } + Err(err) => { + tracing::warn!(?err, "failed to export signaled syncobj"); + let _ = surface.destroy_syncobj(signaled_syncobj); + None + } + }, + Err(err) => { + tracing::warn!(?err, "failed to create signaled syncobj"); + None + } + }; + + let span = info_span!( + parent: None, + "drm_compositor", + device = ?surface.dev_path(), + crtc = ?surface.crtc(), + ); + + let output_mode_source = output_mode_source.into(); + + let surface = Arc::new(surface); + let mut planes = match planes { + Some(planes) => planes, + None => surface.planes().clone(), + }; + + // We do not support direct scan-out on legacy + if surface.is_legacy() { + planes.cursor.clear(); + planes.overlay.clear(); + } + + // The selection algorithm expects the planes to be ordered form front to back + planes + .overlay + .sort_by_key(|p| std::cmp::Reverse(p.zpos.unwrap_or_default())); + + let driver = surface.get_driver().map_err(|err| { + FrameError::DrmError(DrmError::Access(AccessError { + errmsg: "Failed to query drm driver", + dev: surface.dev_path(), + source: err, + })) + })?; + // `IN_FENCE_FD` makes commit fail on Nvidia driver + // https://github.com/NVIDIA/open-gpu-kernel-modules/issues/622 + let is_nvidia = driver.name().to_string_lossy().to_lowercase().contains("nvidia") + || driver + .description() + .to_string_lossy() + .to_lowercase() + .contains("nvidia"); + + let cursor_size = Size::from((cursor_size.w as i32, cursor_size.h as i32)); + let damage_tracker = OutputDamageTracker::from_mode_source(output_mode_source.clone()); + let supports_fencing = !surface.is_legacy() + && surface + .get_driver_capability(DriverCapability::SyncObj) + .map(|val| val != 0) + .map_err(|err| { + FrameError::DrmError(DrmError::Access(AccessError { + errmsg: "Failed to query driver capability", + dev: surface.dev_path(), + source: err, + })) + })? + && plane_has_property(&*surface, surface.plane(), "IN_FENCE_FD")? + && !(is_nvidia && nvidia_drm_version().unwrap_or((0, 0, 0)) < (560, 35, 3)); + + let (swapchain, is_opaque) = Self::test_format( + &surface, + supports_fencing, + &planes, + allocator, + &framebuffer_exporter, + code, + modifiers, + ) + .map_err(|(_, err)| err)?; + + let cursor_state = gbm.map(|gbm| { + #[cfg(feature = "renderer_pixman")] + let pixman_renderer = match PixmanRenderer::new() { + Ok(pixman_renderer) => Some(pixman_renderer), + Err(err) => { + tracing::warn!(?err, "failed to initialize pixman renderer for cursor plane"); + None + } + }; + + let cursor_allocator = + GbmAllocator::new(gbm.clone(), GbmBufferFlags::CURSOR | GbmBufferFlags::WRITE); + CursorState { + allocator: cursor_allocator, + framebuffer_exporter: gbm, + previous_output_scale: None, + previous_output_transform: None, + #[cfg(feature = "renderer_pixman")] + pixman_renderer, + } + }); + + let overlay_plane_element_ids = OverlayPlaneElementIds::from_planes(&planes); + let current_frame = FrameState::from_planes(surface.plane(), &planes); + + let drm_renderer = DrmCompositor { + primary_plane_element_id: Id::new(), + primary_plane_damage_bag: DamageBag::new(4), + primary_is_opaque: is_opaque, + reset_pending: true, + signaled_fence, + current_frame, + pending_frame: None, + queued_frame: None, + next_frame: None, + swapchain, + framebuffer_exporter, + cursor_size, + cursor_state, + surface, + damage_tracker, + output_mode_source, + planes, + overlay_plane_element_ids, + element_states: IndexMap::new(), + previous_element_states: IndexMap::new(), + opaque_regions: Vec::new(), + element_opaque_regions_workhouse: Vec::new(), + supports_fencing, + debug_flags: DebugFlags::empty(), + span, + }; + + Ok(drm_renderer) + } + + fn test_format( + drm: &DrmSurface, supports_fencing: bool, planes: &Planes, allocator: A, framebuffer_exporter: &F, - mut renderer_formats: Vec, code: DrmFourcc, - ) -> Result<(Swapchain, FrameState, bool), (A, FrameErrorType)> { - // select a format + modifiers: impl IntoIterator, + ) -> Result<(Swapchain, bool), (A, FrameErrorType)> { + let modifiers = modifiers.into_iter().collect::>(); let mut plane_formats = drm.plane_info().formats.iter().copied().collect::>(); let opaque_code = get_opaque(code).unwrap_or(code); @@ -1287,63 +1442,25 @@ where return Err((allocator, FrameError::NoSupportedPlaneFormat)); } plane_formats.retain(|fmt| fmt.code == code || fmt.code == opaque_code); - renderer_formats.retain(|fmt| fmt.code == code); - trace!("Plane formats: {:?}", plane_formats); - trace!("Renderer formats: {:?}", renderer_formats); + if plane_formats.is_empty() { + return Err((allocator, FrameError::NoSupportedPlaneFormat)); + } let plane_modifiers = plane_formats .iter() .map(|fmt| fmt.modifier) .collect::>(); - let renderer_modifiers = renderer_formats - .iter() - .map(|fmt| fmt.modifier) - .collect::>(); - debug!( - "Remaining intersected modifiers: {:?}", - plane_modifiers - .intersection(&renderer_modifiers) - .collect::>() - ); - if plane_formats.is_empty() { + let swapchain_modifiers = plane_modifiers + .intersection(&modifiers) + .copied() + .collect::>(); + + if swapchain_modifiers.is_empty() { return Err((allocator, FrameError::NoSupportedPlaneFormat)); - } else if renderer_formats.is_empty() { - return Err((allocator, FrameError::NoSupportedRendererFormat)); } - let formats = { - // Special case: if a format supports explicit LINEAR (but no implicit Modifiers) - // and the other doesn't support any modifier, force Implicit. - // This should at least result in a working pipeline possibly with a linear buffer, - // but we cannot be sure. - if (plane_formats.len() == 1 - && plane_formats.iter().next().unwrap().modifier == DrmModifier::Invalid - && renderer_formats - .iter() - .all(|x| x.modifier != DrmModifier::Invalid) - && renderer_formats.iter().any(|x| x.modifier == DrmModifier::Linear)) - || (renderer_formats.len() == 1 - && renderer_formats.first().unwrap().modifier == DrmModifier::Invalid - && plane_formats.iter().all(|x| x.modifier != DrmModifier::Invalid) - && plane_formats.iter().any(|x| x.modifier == DrmModifier::Linear)) - { - vec![DrmFormat { - code, - modifier: DrmModifier::Invalid, - }] - } else { - plane_modifiers - .intersection(&renderer_modifiers) - .cloned() - .map(|modifier| DrmFormat { code, modifier }) - .collect::>() - } - }; - debug!("Testing Formats: {:?}", formats); - - let modifiers = formats.iter().map(|x| x.modifier).collect::>(); let mode = drm.pending_mode(); let mut swapchain: Swapchain = Swapchain::new( @@ -1351,7 +1468,7 @@ where mode.size().0 as u32, mode.size().1 as u32, code, - modifiers, + swapchain_modifiers, ); // Test format @@ -1423,13 +1540,10 @@ where }; match current_frame_state.test_state(&drm, supports_fencing, drm.plane(), plane_state, true) { - Ok(_) => { - debug!("Chosen format: {:?}", dmabuf.format()); - Ok((swapchain, current_frame_state, use_opaque)) - } + Ok(_) => Ok((swapchain, use_opaque)), Err(err) => { warn!( - "Mode-setting failed with automatically selected buffer format {:?}: {}", + "Mode-setting failed with buffer format {:?}: {}", dmabuf.format(), err ); @@ -1438,6 +1552,98 @@ where } } + fn find_supported_format( + drm: Arc, + supports_fencing: bool, + planes: &Planes, + allocator: A, + framebuffer_exporter: &F, + mut renderer_formats: Vec, + code: DrmFourcc, + ) -> Result<(Swapchain, bool), (A, FrameErrorType)> { + // select a format + let mut plane_formats = drm.plane_info().formats.iter().copied().collect::>(); + + let opaque_code = get_opaque(code).unwrap_or(code); + if !plane_formats + .iter() + .any(|fmt| fmt.code == code || fmt.code == opaque_code) + { + return Err((allocator, FrameError::NoSupportedPlaneFormat)); + } + plane_formats.retain(|fmt| fmt.code == code || fmt.code == opaque_code); + renderer_formats.retain(|fmt| fmt.code == code); + + trace!("Plane formats: {:?}", plane_formats); + trace!("Renderer formats: {:?}", renderer_formats); + + let plane_modifiers = plane_formats + .iter() + .map(|fmt| fmt.modifier) + .collect::>(); + let renderer_modifiers = renderer_formats + .iter() + .map(|fmt| fmt.modifier) + .collect::>(); + debug!( + "Remaining intersected modifiers: {:?}", + plane_modifiers + .intersection(&renderer_modifiers) + .collect::>() + ); + + if plane_formats.is_empty() { + return Err((allocator, FrameError::NoSupportedPlaneFormat)); + } else if renderer_formats.is_empty() { + return Err((allocator, FrameError::NoSupportedRendererFormat)); + } + + let formats = { + // Special case: if a format supports explicit LINEAR (but no implicit Modifiers) + // and the other doesn't support any modifier, force Implicit. + // This should at least result in a working pipeline possibly with a linear buffer, + // but we cannot be sure. + if (plane_formats.len() == 1 + && plane_formats.iter().next().unwrap().modifier == DrmModifier::Invalid + && renderer_formats + .iter() + .all(|x| x.modifier != DrmModifier::Invalid) + && renderer_formats.iter().any(|x| x.modifier == DrmModifier::Linear)) + || (renderer_formats.len() == 1 + && renderer_formats.first().unwrap().modifier == DrmModifier::Invalid + && plane_formats.iter().all(|x| x.modifier != DrmModifier::Invalid) + && plane_formats.iter().any(|x| x.modifier == DrmModifier::Linear)) + { + vec![DrmFormat { + code, + modifier: DrmModifier::Invalid, + }] + } else { + plane_modifiers + .intersection(&renderer_modifiers) + .cloned() + .map(|modifier| DrmFormat { code, modifier }) + .collect::>() + } + }; + + debug!("Testing Formats: {:?}", formats); + + let modifiers = formats.iter().map(|x| x.modifier).collect::>(); + + let (swapchain, use_opaque) = Self::test_format( + &*drm, + supports_fencing, + planes, + allocator, + framebuffer_exporter, + code, + modifiers, + )?; + + Ok((swapchain, use_opaque)) + } + /// Render the next frame /// /// - `elements` for this frame in front-to-back order @@ -2460,6 +2666,29 @@ where self.swapchain.format() } + pub fn set_format( + &mut self, + allocator: A, + code: DrmFourcc, + modifiers: impl IntoIterator, + ) -> Result<(), FrameErrorType> { + let (swapchain, is_oapque) = Self::test_format( + &self.surface, + self.supports_fencing, + &self.planes, + allocator, + &self.framebuffer_exporter, + code, + modifiers, + ) + .map_err(|(_, err)| err)?; + + self.swapchain = swapchain; + self.primary_is_opaque = is_oapque; + + Ok(()) + } + /// Change the output mode source. pub fn set_output_mode_source(&mut self, output_mode_source: OutputModeSource) { // Avoid clearing damage if mode source did not change. From e7362bcbace0d04300d3805a5b4b91b5d1286a7b Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 28 Nov 2024 17:31:59 +0100 Subject: [PATCH 06/24] drm/compositor: Add `commit_frame` --- src/backend/drm/compositor/mod.rs | 59 ++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index 6edaf0272987..fca335be6015 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -2429,6 +2429,41 @@ where Ok(()) } + pub fn commit_frame(&mut self) -> FrameResult<(), A, F> { + if !self.surface.is_active() { + return Err(FrameErrorType::::DrmError(DrmError::DeviceInactive)); + } + + let mut prepared_frame = self.next_frame.take().ok_or(FrameErrorType::::EmptyFrame)?; + if prepared_frame.is_empty() { + return Err(FrameErrorType::::EmptyFrame); + } + + if let Some(plane_state) = prepared_frame.frame.plane_state(self.surface.plane()) { + if !plane_state.skip { + let slot = plane_state.buffer().and_then(|config| match &config.buffer { + ScanoutBuffer::Swapchain(slot) => Some(slot), + _ => None, + }); + + if let Some(slot) = slot { + self.swapchain.submitted(slot); + } + } + } + + let flip = prepared_frame + .frame + .commit(&self.surface, self.supports_fencing, false, false); + + if flip.is_ok() { + self.queued_frame = None; + self.pending_frame = None; + } + + self.handle_flip(prepared_frame, None, flip) + } + /// Re-evaluates the current state of the crtc and forces calls to [`render_frame`](DrmCompositor::render_frame) /// to return `false` for [`RenderFrameResult::is_empty`] until a frame is queued with [`queue_frame`](DrmCompositor::queue_frame). /// @@ -2462,13 +2497,22 @@ where .page_flip(&self.surface, self.supports_fencing, allow_partial_update, true) }; + self.handle_flip(prepared_frame, Some(user_data), flip) + } + + fn handle_flip( + &mut self, + prepared_frame: PreparedFrame, + user_data: Option, + flip: Result<(), crate::backend::drm::error::Error>, + ) -> FrameResult<(), A, F> { match flip { Ok(_) => { if prepared_frame.kind == PreparedFrameKind::Full { self.reset_pending = false; } - self.pending_frame = Some(PendingFrame { + self.pending_frame = user_data.map(|user_data| PendingFrame { frame: prepared_frame.frame, user_data, }); @@ -4296,3 +4340,16 @@ fn nvidia_drm_version() -> Option<(u32, u32, u32)> { let patch = u32::from_str(components.next()?).ok()?; Some((major, minor, patch)) } + +#[test] +fn drm_compositor_is_send() { + use std::marker::PhantomData; + + use crate::backend::drm::DrmDeviceFd; + + fn is_send() { + let _ = PhantomData::; + } + + is_send::, GbmDevice, (), DrmDeviceFd>>(); +} From 379e02e5fe76d669aedbbfd70f2e3863e11d969f Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 28 Nov 2024 17:32:57 +0100 Subject: [PATCH 07/24] drm/compositor: Introduce `DrmOutput`/`Manager` --- src/backend/drm/output.rs | 581 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 581 insertions(+) create mode 100644 src/backend/drm/output.rs diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs new file mode 100644 index 000000000000..d3b633f945c0 --- /dev/null +++ b/src/backend/drm/output.rs @@ -0,0 +1,581 @@ +use std::{ + collections::HashMap, + fmt, + marker::PhantomData, + os::fd::AsFd, + sync::{Arc, Mutex, RwLock, TryLockError}, +}; + +use drm::control::{self, connector, crtc, Mode}; +use drm_fourcc::{DrmFormat, DrmFourcc, DrmModifier}; + +use crate::{ + backend::{ + allocator::{ + dmabuf::{AsDmabuf, Dmabuf}, + gbm::GbmDevice, + Allocator, + }, + renderer::{element::RenderElement, Bind, Color32F, DebugFlags, Renderer, Texture}, + }, + output::OutputModeSource, +}; + +use super::{ + compositor::{ + DrmCompositor, FrameError, FrameMode, FrameResult, RenderFrameError, RenderFrameErrorType, + RenderFrameResult, + }, + exporter::ExportFramebuffer, + DrmDevice, DrmError, Planes, +}; + +type CompositorList = Arc>>>>; + +pub struct DrmOutputManager +where + A: Allocator, + F: ExportFramebuffer<::Buffer>, + ::Buffer>>::Framebuffer: fmt::Debug + 'static, + G: AsFd + 'static, +{ + device: DrmDevice, + allocator: A, + exporter: F, + gbm: Option>, + compositor: CompositorList, + color_formats: Vec, + renderer_formats: Vec, +} + +impl fmt::Debug for DrmOutputManager +where + A: Allocator + fmt::Debug, + ::Buffer: fmt::Debug, + F: ExportFramebuffer<::Buffer> + fmt::Debug, + U: fmt::Debug, + ::Buffer>>::Framebuffer: fmt::Debug + 'static, + G: AsFd + fmt::Debug + 'static, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("DrmOutputManager") + .field("device", &self.device) + .field("allocator", &self.allocator) + .field("exporter", &self.exporter) + .field("gbm", &self.gbm) + .field("compositor", &self.compositor) + .field("color_formats", &self.color_formats) + .field("renderer_formats", &self.renderer_formats) + .finish() + } +} + +impl DrmOutputManager +where + A: Allocator, + F: ExportFramebuffer<::Buffer>, + ::Buffer>>::Framebuffer: fmt::Debug + 'static, + G: AsFd + 'static, +{ + pub fn device(&self) -> &DrmDevice { + &self.device + } + + pub fn device_mut(&mut self) -> &mut DrmDevice { + &mut self.device + } +} + +impl DrmOutputManager +where + A: Allocator, + F: ExportFramebuffer<::Buffer>, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + G: AsFd + 'static, +{ + pub fn pause(&mut self) { + self.device.pause(); + } +} + +#[derive(thiserror::Error, Debug)] +pub enum DrmOutputManagerError +where + A: std::error::Error + Send + Sync + 'static, + B: std::error::Error + Send + Sync + 'static, + F: std::error::Error + Send + Sync + 'static, + R: std::error::Error + Send + Sync + 'static, +{ + #[error("The specified CRTC {0:?} is already in use.")] + DuplicateCrtc(crtc::Handle), + #[error(transparent)] + Drm(#[from] DrmError), + #[error(transparent)] + Frame(FrameError), + #[error(transparent)] + RenderFrame(RenderFrameError), +} + +impl DrmOutputManager +where + A: Allocator + std::clone::Clone + std::fmt::Debug, + ::Buffer: AsDmabuf, + ::Error: Send + Sync + 'static, + <::Buffer as AsDmabuf>::Error: + std::marker::Send + std::marker::Sync + 'static, + F: ExportFramebuffer<::Buffer> + std::clone::Clone, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + ::Buffer>>::Error: + std::marker::Send + std::marker::Sync + 'static, + G: AsFd + std::clone::Clone + 'static, + U: 'static, +{ + pub fn new( + device: DrmDevice, + allocator: A, + exporter: F, + gbm: Option>, + color_formats: impl IntoIterator, + renderer_formats: impl IntoIterator, + ) -> Self { + Self { + device, + allocator: allocator, + exporter, + gbm, + compositor: Default::default(), + color_formats: color_formats.into_iter().collect(), + renderer_formats: renderer_formats.into_iter().collect(), + } + } + + pub fn initialize_output<'a, R, E>( + &'a mut self, + crtc: crtc::Handle, + ) -> DrmOutputBuilder<'a, A, F, U, G, R, E> + where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, + { + DrmOutputBuilder { + manager: self, + crtc, + render_elements: HashMap::new(), + _renderer: PhantomData, + } + } + + pub fn reset_format( + &mut self, + renderer: &mut R, + render_elements: RE, + ) -> Result< + (), + DrmOutputManagerError< + ::Error, + <::Buffer as AsDmabuf>::Error, + ::Buffer>>::Error, + R::Error, + >, + > + where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, + RE: for<'r> Fn(&'r crtc::Handle) -> (&'r [E], Color32F), + { + // TODO: re-evaluate format and re-render if changed... + Ok(()) + } + + pub fn activate(&mut self, disable_connectors: bool) -> Result<(), DrmError> { + self.device.activate(disable_connectors)?; + + // We request a write guard here to guarantee unique access + let write_guard = self.compositor.write().unwrap(); + for compositor in write_guard.values() { + if let Err(err) = compositor.lock().unwrap().reset_state() { + tracing::warn!("Failed to reset drm surface state: {}", err); + } + } + + Ok(()) + } +} + +pub struct DrmOutputBuilder<'a, A, F, U, G, R, E> +where + A: Allocator + std::clone::Clone + std::fmt::Debug, + ::Buffer: AsDmabuf, + ::Error: Send + Sync + 'static, + <::Buffer as AsDmabuf>::Error: + std::marker::Send + std::marker::Sync + 'static, + F: ExportFramebuffer<::Buffer> + std::clone::Clone, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + ::Buffer>>::Error: + std::marker::Send + std::marker::Sync + 'static, + G: AsFd + std::clone::Clone + 'static, + U: 'static, + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, +{ + manager: &'a mut DrmOutputManager, + crtc: crtc::Handle, + render_elements: HashMap, Color32F)>, + _renderer: PhantomData, +} + +impl<'a, A, F, U, G, R, E> DrmOutputBuilder<'a, A, F, U, G, R, E> +where + A: Allocator + std::clone::Clone + std::fmt::Debug, + ::Buffer: AsDmabuf, + ::Error: Send + Sync + 'static, + <::Buffer as AsDmabuf>::Error: + std::marker::Send + std::marker::Sync + 'static, + F: ExportFramebuffer<::Buffer> + std::clone::Clone, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + ::Buffer>>::Error: + std::marker::Send + std::marker::Sync + 'static, + G: AsFd + std::clone::Clone + 'static, + U: 'static, + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, +{ + pub fn add_render_contents( + &mut self, + crtc: &crtc::Handle, + clear_color: Color32F, + elements: impl IntoIterator, + ) { + self.render_elements + .insert(*crtc, (elements.into_iter().collect(), clear_color)); + } + + pub fn build( + self, + renderer: &mut R, + mode: control::Mode, + connectors: &[connector::Handle], + output_mode_source: impl Into + std::fmt::Debug, + planes: Option, + ) -> Result< + DrmOutput, + DrmOutputManagerError< + ::Error, + <::Buffer as AsDmabuf>::Error, + ::Buffer>>::Error, + R::Error, + >, + > { + let output_mode_source = output_mode_source.into(); + + let mut write_guard = self.manager.compositor.write().unwrap(); + if write_guard.contains_key(&self.crtc) { + return Err(DrmOutputManagerError::DuplicateCrtc(self.crtc)); + } + + let surface = self + .manager + .device + .create_surface(self.crtc, mode, connectors) + .map_err(DrmOutputManagerError::Drm)?; + + let compositor = DrmCompositor::::new( + output_mode_source.clone(), + surface, + planes.clone(), + self.manager.allocator.clone(), + self.manager.exporter.clone(), + self.manager.color_formats.iter().copied(), + self.manager.renderer_formats.iter().copied(), + self.manager.device.cursor_size(), + self.manager.gbm.clone(), + ); + + match compositor { + Ok(compositor) => { + write_guard.insert(self.crtc, Mutex::new(compositor)); + } + Err(err) => { + tracing::warn!(crtc=?self.crtc, ?err, "failed to initialize crtc"); + // Okay, so this can fail for various reasons... + // + // Enabling an additional CRTC might fail because the bandwidth + // requirement is higher then supported with the current configuration. + // + // * Bandwidth limitation caused by overlay plane usage: + // Each overlay plane requires some certain bandwidth and we only + // test that during plane assignment implicitly through an atomic test. + // When trying to enable an additional CRTC we might hit some limit and the + // only way to resolve this might be to disable all overlay planes and + // retry enabling the CRTC. + // + // * Bandwidth limitation caused by the primary plane format: + // Different formats (might) require a higher memory bandwidth than others. + // This also applies to the same fourcc with different modifiers. For example + // the Intel CCS Formats use an additional plane to transport meta-data. + // So if we fail to enable an additional CRTC we might be able to resolve + // the issue by using a different format. Again the only way to know is by + // trying out what works. + // + // So the easiest option is to evaluate a new format and re-create all DrmCompositor + // without disabling the CRTCs. + + // TODO: Find out the *best* working combination of formats per active crtc + for (handle, compositor) in write_guard.iter_mut() { + let mut compositor = compositor.lock().unwrap(); + + let (elements, clear_color) = self + .render_elements + .get(handle) + .map(|(ref elements, ref color)| (&**elements, color)) + .unwrap_or((&[], &Color32F::BLACK)); + compositor.reset_buffer_ages(); + compositor + .render_frame(renderer, &elements, *clear_color, FrameMode::COMPOSITE) + .map_err(DrmOutputManagerError::RenderFrame)?; + compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; + + let current_format = compositor.format(); + if let Err(err) = compositor.set_format( + self.manager.allocator.clone(), + current_format, + [DrmModifier::Invalid], + ) { + tracing::warn!(?err, "failed to set new format"); + } + + compositor + .render_frame(renderer, &elements, *clear_color, FrameMode::COMPOSITE) + .map_err(DrmOutputManagerError::RenderFrame)?; + compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; + } + + // :) Use the selected format instead of replicating DrmCompositor format selection... + let mut init_err = None; + for format in self.manager.color_formats.iter() { + let surface = self + .manager + .device + .create_surface(self.crtc, mode, connectors) + .map_err(DrmOutputManagerError::Drm)?; + + let compositor = DrmCompositor::::with_format( + output_mode_source.clone(), + surface, + planes.clone(), + self.manager.allocator.clone(), + self.manager.exporter.clone(), + *format, + [DrmModifier::Invalid], + self.manager.device.cursor_size(), + self.manager.gbm.clone(), + ); + + match compositor { + Ok(compositor) => { + write_guard.insert(self.crtc, Mutex::new(compositor)); + break; + } + Err(err) => init_err = Some(err), + } + } + + if let Some(err) = init_err { + return Err(DrmOutputManagerError::Frame(err)); + } + } + }; + + let compositor = write_guard.get_mut(&self.crtc).unwrap(); + let mut compositor = compositor.lock().unwrap(); + let (elements, clear_color) = self + .render_elements + .get(&self.crtc) + .map(|(ref elements, ref color)| (&**elements, color)) + .unwrap_or((&[], &Color32F::BLACK)); + compositor + .render_frame(renderer, elements, *clear_color, FrameMode::COMPOSITE) + .map_err(DrmOutputManagerError::RenderFrame)?; + compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; + + Ok(DrmOutput { + crtc: self.crtc, + compositor: self.manager.compositor.clone(), + }) + } +} + +pub struct DrmOutput +where + A: Allocator, + F: ExportFramebuffer<::Buffer>, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + G: AsFd + 'static, +{ + crtc: crtc::Handle, + compositor: CompositorList, +} + +impl fmt::Debug for DrmOutput +where + A: Allocator + fmt::Debug, + ::Buffer: fmt::Debug, + F: ExportFramebuffer<::Buffer> + fmt::Debug, + U: fmt::Debug, + ::Buffer>>::Framebuffer: fmt::Debug + 'static, + G: AsFd + fmt::Debug + 'static, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut d = f.debug_struct("DrmOutput"); + match self.compositor.try_read() { + Ok(guard) => d.field("compositor", &guard.get(&self.crtc)), + Err(TryLockError::Poisoned(err)) => d.field("compositor", &&**err.get_ref()), + Err(TryLockError::WouldBlock) => d.field("compositor", &""), + }; + d.finish() + } +} + +impl DrmOutput +where + A: Allocator + std::clone::Clone, + ::Buffer: AsDmabuf, + ::Error: Send + Sync + 'static, + <::Buffer as AsDmabuf>::Error: std::marker::Send + std::marker::Sync + 'static, + F: ExportFramebuffer<::Buffer> + std::clone::Clone, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + ::Buffer>>::Error: + std::marker::Send + std::marker::Sync + 'static, + G: AsFd + std::clone::Clone + 'static, +{ + /// Set the [`DebugFlags`] to use + /// + /// Note: This will reset the primary plane swapchain if + /// the flags differ from the current flags + pub fn set_debug_flags(&self, flags: DebugFlags) { + self.with_compositor(|compositor| compositor.set_debug_flags(flags)); + } + + /// Reset the underlying buffers + pub fn reset_buffers(&self) { + self.with_compositor(|compositor| compositor.reset_buffers()); + } + + pub fn frame_submitted(&self) -> FrameResult, A, F> { + self.with_compositor(|compositor| compositor.frame_submitted()) + } + + pub fn format(&self) -> DrmFourcc { + self.with_compositor(|compositor| compositor.format()) + } + + pub fn render_frame<'a, R, E>( + &mut self, + renderer: &mut R, + elements: &'a [E], + clear_color: impl Into, + frame_mode: FrameMode, + ) -> Result, RenderFrameErrorType> + where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, + { + self.with_compositor(|compositor| { + compositor.render_frame(renderer, elements, clear_color, frame_mode) + }) + } + + pub fn queue_frame(&mut self, user_data: U) -> FrameResult<(), A, F> { + self.with_compositor(|compositor| compositor.queue_frame(user_data)) + } + + pub fn commit_frame(&mut self) -> FrameResult<(), A, F> { + self.with_compositor(|compositor| compositor.commit_frame()) + } + + pub fn use_mode( + &mut self, + renderer: &mut R, + mode: Mode, + render_elements: RE, + ) -> Result< + (), + DrmOutputManagerError< + ::Error, + <::Buffer as AsDmabuf>::Error, + ::Buffer>>::Error, + R::Error, + >, + > + where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, + RE: for<'r> Fn(&'r crtc::Handle) -> (&'r [E], Color32F), + { + let mut res = self.with_compositor(|compositor| compositor.use_mode(mode)); + if res.is_err() { + let mut write_guard = self.compositor.write().unwrap(); + + for (handle, compositor) in write_guard.iter_mut() { + let mut compositor = compositor.lock().unwrap(); + + let (elements, clear_color) = render_elements(handle); + compositor.reset_buffer_ages(); + compositor + .render_frame(renderer, elements, clear_color, FrameMode::COMPOSITE) + .map_err(DrmOutputManagerError::RenderFrame)?; + compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; + } + let compositor = write_guard.get_mut(&self.crtc).unwrap(); + let mut compositor = compositor.lock().unwrap(); + res = compositor.use_mode(mode); + } + res.map_err(DrmOutputManagerError::Frame) + } +} + +impl DrmOutput +where + A: Allocator, + F: ExportFramebuffer<::Buffer>, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + G: AsFd + 'static, +{ + pub fn crtc(&self) -> crtc::Handle { + self.crtc + } + + pub fn with_compositor(&self, f: T) -> R + where + T: FnOnce(&mut DrmCompositor) -> R, + { + let read_guard = self.compositor.read().unwrap(); + let mut compositor_guard = read_guard.get(&self.crtc).unwrap().lock().unwrap(); + f(&mut compositor_guard) + } +} + +impl Drop for DrmOutput +where + A: Allocator, + F: ExportFramebuffer<::Buffer>, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + G: AsFd + 'static, +{ + fn drop(&mut self) { + let mut write_guard = self.compositor.write().unwrap(); + write_guard.remove(&self.crtc); + } +} From b8c3def3431a86bbec03e39baebe1c78b92c20ab Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 28 Nov 2024 17:33:11 +0100 Subject: [PATCH 08/24] anvil: Adopt drm changes --- anvil/src/drawing.rs | 7 +- anvil/src/render.rs | 6 +- anvil/src/udev.rs | 528 +++++++++++++++---------------------------- 3 files changed, 182 insertions(+), 359 deletions(-) diff --git a/anvil/src/drawing.rs b/anvil/src/drawing.rs index 0fa62201ed02..0dfe1edc725b 100644 --- a/anvil/src/drawing.rs +++ b/anvil/src/drawing.rs @@ -6,8 +6,7 @@ use smithay::{ memory::{MemoryRenderBuffer, MemoryRenderBufferRenderElement}, surface::WaylandSurfaceRenderElement, AsRenderElements, Kind, - }, - ImportAll, ImportMem, Renderer, Texture, + }, Color32F, ImportAll, ImportMem, Renderer, Texture }, input::pointer::CursorImageStatus, render_elements, @@ -23,8 +22,8 @@ use smithay::{ utils::{Buffer, Logical, Rectangle, Size, Transform}, }; -pub static CLEAR_COLOR: [f32; 4] = [0.8, 0.8, 0.9, 1.0]; -pub static CLEAR_COLOR_FULLSCREEN: [f32; 4] = [0.0, 0.0, 0.0, 0.0]; +pub static CLEAR_COLOR: Color32F = Color32F::new(0.8, 0.8, 0.9, 1.0); +pub static CLEAR_COLOR_FULLSCREEN: Color32F = Color32F::new(0.0, 0.0, 0.0, 0.0); pub struct PointerElement { buffer: Option, diff --git a/anvil/src/render.rs b/anvil/src/render.rs index 6346799a71be..e1a6f5fa89c3 100644 --- a/anvil/src/render.rs +++ b/anvil/src/render.rs @@ -9,7 +9,7 @@ use smithay::{ }, AsRenderElements, RenderElement, Wrap, }, - ImportAll, ImportMem, Renderer, + Color32F, ImportAll, ImportMem, Renderer, }, desktop::space::{ constrain_space_element, ConstrainBehavior, ConstrainReference, Space, SpaceRenderElements, @@ -142,7 +142,7 @@ pub fn output_elements( custom_elements: impl IntoIterator>, renderer: &mut R, show_window_preview: bool, -) -> (Vec>>, [f32; 4]) +) -> (Vec>>, Color32F) where R: Renderer + ImportAll + ImportMem, R::TextureId: Clone + 'static, @@ -198,7 +198,7 @@ pub fn render_output<'a, 'd, R>( damage_tracker: &'d mut OutputDamageTracker, age: usize, show_window_preview: bool, -) -> Result, OutputDamageTrackerError> +) -> Result, OutputDamageTrackerError> where R: Renderer + ImportAll + ImportMem, R::TextureId: Clone + 'static, diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index 58e6e9f9da99..1ea916e97c32 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -6,13 +6,16 @@ use std::{ time::{Duration, Instant}, }; -use crate::state::{DndIcon, SurfaceDmabufFeedback}; use crate::{ drawing::*, render::*, shell::WindowElement, state::{take_presentation_feedback, AnvilState, Backend}, }; +use crate::{ + shell::WindowRenderElement, + state::{DndIcon, SurfaceDmabufFeedback}, +}; #[cfg(feature = "renderer_sync")] use smithay::backend::drm::compositor::PrimaryPlaneElement; #[cfg(feature = "egl")] @@ -28,19 +31,20 @@ use smithay::{ Fourcc, }, drm::{ - compositor::DrmCompositor, CreateDrmNodeError, DrmAccessError, DrmDevice, DrmDeviceFd, DrmError, - DrmEvent, DrmEventMetadata, DrmNode, DrmSurface, GbmBufferedSurface, NodeType, + compositor::{DrmCompositor, FrameMode}, + output::{DrmOutput, DrmOutputManager}, + CreateDrmNodeError, DrmAccessError, DrmDevice, DrmDeviceFd, DrmError, DrmEvent, DrmEventMetadata, + DrmNode, DrmSurface, GbmBufferedSurface, NodeType, }, egl::{self, context::ContextPriority, EGLDevice, EGLDisplay}, input::InputEvent, libinput::{LibinputInputBackend, LibinputSessionInterface}, renderer::{ - damage::{Error as OutputDamageTrackerError, OutputDamageTracker}, - element::{memory::MemoryRenderBuffer, AsRenderElements, RenderElement, RenderElementStates}, - gles::{GlesRenderer, GlesTexture}, + damage::Error as OutputDamageTrackerError, + element::{memory::MemoryRenderBuffer, AsRenderElements, RenderElementStates}, + gles::GlesRenderer, multigpu::{gbm::GbmGlesBackend, GpuManager, MultiRenderer}, - sync::SyncPoint, - Bind, DebugFlags, ExportMem, ImportDma, ImportMemWl, Offscreen, Renderer, + Color32F, DebugFlags, ImportDma, ImportMemWl, }, session::{ libseat::{self, LibSeatSession}, @@ -62,7 +66,7 @@ use smithay::{ reexports::{ calloop::{ timer::{TimeoutAction, Timer}, - EventLoop, LoopHandle, RegistrationToken, + EventLoop, RegistrationToken, }, drm::{ control::{connector, crtc, Device, ModeTypeFlags}, @@ -74,15 +78,11 @@ use smithay::{ linux_dmabuf::zv1::server::zwp_linux_dmabuf_feedback_v1, presentation_time::server::wp_presentation_feedback, }, - wayland_server::{ - backend::GlobalId, - protocol::wl_surface::{self}, - Display, DisplayHandle, - }, + wayland_server::{backend::GlobalId, protocol::wl_surface, Display, DisplayHandle}, }, - utils::{DeviceFd, IsAlive, Logical, Monotonic, Physical, Point, Rectangle, Scale, Time, Transform}, + utils::{DeviceFd, IsAlive, Logical, Monotonic, Point, Scale, Time, Transform}, wayland::{ - compositor::{self}, + compositor, dmabuf::{DmabufFeedbackBuilder, DmabufGlobal, DmabufHandler, DmabufState, ImportNotifier}, drm_lease::{ DrmLease, DrmLeaseBuilder, DrmLeaseHandler, DrmLeaseRequest, DrmLeaseState, LeaseRejected, @@ -148,7 +148,7 @@ impl UdevData { for (_, backend) in self.backends.iter_mut() { for (_, surface) in backend.surfaces.iter_mut() { - surface.compositor.set_debug_flags(flags); + surface.drm_output.set_debug_flags(flags); } } } @@ -193,7 +193,7 @@ impl Backend for UdevData { if let Some(id) = output.user_data().get::() { if let Some(gpu) = self.backends.get_mut(&id.device_id) { if let Some(surface) = gpu.surfaces.get_mut(&id.crtc) { - surface.compositor.reset_buffers(); + surface.drm_output.reset_buffers(); } } } @@ -319,7 +319,7 @@ pub fn run_udev() { info!("pausing session"); for backend in data.backend_data.backends.values_mut() { - backend.drm.pause(); + backend.drm_output_manager.pause(); backend.active_leases.clear(); if let Some(lease_global) = backend.leasing_global.as_mut() { lease_global.suspend(); @@ -345,17 +345,12 @@ pub fn run_udev() { // state as is and assume it will just work. If this assumption fails // we will try to reset the state when trying to queue a frame. backend - .drm + .drm_output_manager .activate(false) .expect("failed to activate drm backend"); if let Some(lease_global) = backend.leasing_global.as_mut() { lease_global.resume::>(); } - for surface in backend.surfaces.values_mut() { - if let Err(err) = surface.compositor.reset_state() { - warn!("Failed to reset drm surface state: {}", err); - } - } data.handle .insert_idle(move |data| data.render(node, None, data.clock.now())); } @@ -430,12 +425,14 @@ pub fn run_udev() { // Update the per drm surface dmabuf feedback backend_data.surfaces.values_mut().for_each(|surface_data| { surface_data.dmabuf_feedback = surface_data.dmabuf_feedback.take().or_else(|| { - get_surface_dmabuf_feedback( - primary_gpu, - surface_data.render_node, - gpus, - &surface_data.compositor, - ) + surface_data.drm_output.with_compositor(|compositor| { + get_surface_dmabuf_feedback( + primary_gpu, + surface_data.render_node, + gpus, + &compositor.surface(), + ) + }) }); }); }); @@ -448,7 +445,7 @@ pub fn run_udev() { .and_then(|x| x.ok()) { if let Some(backend) = state.backend_data.backends.get(&primary_node) { - let import_device = backend.drm.device_fd().clone(); + let import_device = backend.drm_output_manager.device().device_fd().clone(); if supports_syncobj_eventfd(&import_device) { let syncobj_state = DrmSyncobjState::new::>(&display_handle, import_device); @@ -525,7 +522,8 @@ impl DrmLeaseHandler for AnvilState { .get(&node) .ok_or(LeaseRejected::default())?; - let mut builder = DrmLeaseBuilder::new(&backend.drm); + let drm_device = backend.drm_output_manager.device(); + let mut builder = DrmLeaseBuilder::new(drm_device); for conn in request.connectors { if let Some((_, crtc)) = backend .non_desktop_connectors @@ -534,21 +532,19 @@ impl DrmLeaseHandler for AnvilState { { builder.add_connector(conn); builder.add_crtc(*crtc); - let planes = backend.drm.planes(crtc).map_err(LeaseRejected::with_cause)?; + let planes = drm_device.planes(crtc).map_err(LeaseRejected::with_cause)?; let (primary_plane, primary_plane_claim) = planes .primary .iter() .find_map(|plane| { - backend - .drm + drm_device .claim_plane(plane.handle, *crtc) .map(|claim| (plane, claim)) }) .ok_or_else(LeaseRejected::default)?; builder.add_plane(primary_plane.handle, primary_plane_claim); if let Some((cursor, claim)) = planes.cursor.iter().find_map(|plane| { - backend - .drm + drm_device .claim_plane(plane.handle, *crtc) .map(|claim| (plane, claim)) }) { @@ -592,167 +588,18 @@ pub type GbmDrmCompositor = DrmCompositor< DrmDeviceFd, >; -enum SurfaceComposition { - Surface { - surface: RenderSurface, - damage_tracker: OutputDamageTracker, - debug_flags: DebugFlags, - }, - Compositor(GbmDrmCompositor), -} - -struct SurfaceCompositorRenderResult<'a> { - rendered: bool, - states: RenderElementStates, - sync: Option, - damage: Option<&'a Vec>>, -} - -impl SurfaceComposition { - #[profiling::function] - fn frame_submitted(&mut self) -> Result>, SwapBuffersError> { - match self { - SurfaceComposition::Compositor(c) => c.frame_submitted().map_err(Into::::into), - SurfaceComposition::Surface { surface, .. } => { - surface.frame_submitted().map_err(Into::::into) - } - } - } - - fn format(&self) -> smithay::reexports::gbm::Format { - match self { - SurfaceComposition::Compositor(c) => c.format(), - SurfaceComposition::Surface { surface, .. } => surface.format(), - } - } - - fn surface(&self) -> &DrmSurface { - match self { - SurfaceComposition::Compositor(c) => c.surface(), - SurfaceComposition::Surface { surface, .. } => surface.surface(), - } - } - - fn reset_buffers(&mut self) { - match self { - SurfaceComposition::Compositor(c) => c.reset_buffers(), - SurfaceComposition::Surface { surface, .. } => surface.reset_buffers(), - } - } - - fn reset_state(&mut self) -> Result<(), SwapBuffersError> { - match self { - SurfaceComposition::Compositor(c) => c.reset_state().map_err(Into::::into), - SurfaceComposition::Surface { surface, .. } => surface - .surface() - .reset_state() - .map_err(Into::::into), - } - } - - #[profiling::function] - fn queue_frame( - &mut self, - sync: Option, - damage: Option>>, - user_data: Option, - ) -> Result<(), SwapBuffersError> { - match self { - SurfaceComposition::Surface { surface, .. } => surface - .queue_buffer(sync, damage, user_data) - .map_err(Into::::into), - SurfaceComposition::Compositor(c) => { - c.queue_frame(user_data).map_err(Into::::into) - } - } - } - - #[profiling::function] - fn render_frame( - &mut self, - renderer: &mut R, - elements: &[E], - clear_color: [f32; 4], - ) -> Result, SwapBuffersError> - where - R: Renderer + Bind + Bind + Offscreen + ExportMem, - ::TextureId: 'static, - ::Error: Into, - E: RenderElement, - { - match self { - SurfaceComposition::Surface { - surface, - damage_tracker, - debug_flags, - } => { - let (dmabuf, age) = surface.next_buffer().map_err(Into::::into)?; - renderer.bind(dmabuf).map_err(Into::::into)?; - let current_debug_flags = renderer.debug_flags(); - renderer.set_debug_flags(*debug_flags); - let res = damage_tracker - .render_output(renderer, age.into(), elements, clear_color) - .map(|res| { - #[cfg(feature = "renderer_sync")] - res.sync.wait(); - let rendered = res.damage.is_some(); - SurfaceCompositorRenderResult { - rendered, - damage: res.damage, - states: res.states, - sync: rendered.then_some(res.sync), - } - }) - .map_err(|err| match err { - OutputDamageTrackerError::Rendering(err) => err.into(), - _ => unreachable!(), - }); - renderer.set_debug_flags(current_debug_flags); - res - } - SurfaceComposition::Compositor(compositor) => compositor - .render_frame(renderer, elements, clear_color) - .map(|render_frame_result| { - #[cfg(feature = "renderer_sync")] - if let PrimaryPlaneElement::Swapchain(element) = render_frame_result.primary_element { - element.sync.wait(); - } - SurfaceCompositorRenderResult { - rendered: !render_frame_result.is_empty, - damage: None, - states: render_frame_result.states, - sync: None, - } - }) - .map_err(|err| match err { - smithay::backend::drm::compositor::RenderFrameError::PrepareFrame(err) => err.into(), - smithay::backend::drm::compositor::RenderFrameError::RenderFrame( - OutputDamageTrackerError::Rendering(err), - ) => err.into(), - _ => unreachable!(), - }), - } - } - - fn set_debug_flags(&mut self, flags: DebugFlags) { - match self { - SurfaceComposition::Surface { - surface, debug_flags, .. - } => { - *debug_flags = flags; - surface.reset_buffers(); - } - SurfaceComposition::Compositor(c) => c.set_debug_flags(flags), - } - } -} - struct SurfaceData { dh: DisplayHandle, device_id: DrmNode, render_node: DrmNode, global: Option, - compositor: SurfaceComposition, + drm_output: DrmOutput< + GbmAllocator, + GbmDevice, + Option, + DrmDeviceFd, + >, + disable_direct_scanout: bool, #[cfg(feature = "debug")] fps: fps_ticker::Fps, #[cfg(feature = "debug")] @@ -773,8 +620,12 @@ struct BackendData { non_desktop_connectors: Vec<(connector::Handle, crtc::Handle)>, leasing_global: Option, active_leases: Vec, - gbm: GbmDevice, - drm: DrmDevice, + drm_output_manager: DrmOutputManager< + GbmAllocator, + GbmDevice, + Option, + DrmDeviceFd, + >, drm_scanner: DrmScanner, render_node: DrmNode, registration_token: RegistrationToken, @@ -798,7 +649,7 @@ fn get_surface_dmabuf_feedback( primary_gpu: DrmNode, render_node: DrmNode, gpus: &mut GpuManager>, - composition: &SurfaceComposition, + surface: &DrmSurface, ) -> Option { let primary_formats = gpus.single_renderer(&primary_gpu).ok()?.dmabuf_formats(); let render_formats = gpus.single_renderer(&render_node).ok()?.dmabuf_formats(); @@ -809,7 +660,6 @@ fn get_surface_dmabuf_feedback( .copied() .collect::(); - let surface = composition.surface(); let planes = surface.planes().clone(); // We limit the scan-out tranche to formats we can also render from @@ -893,12 +743,29 @@ impl AnvilState { .add_node(render_node, gbm.clone()) .map_err(DeviceAddError::AddNode)?; + let allocator = GbmAllocator::new(gbm.clone(), GbmBufferFlags::RENDERING | GbmBufferFlags::SCANOUT); + let color_formats = if std::env::var("ANVIL_DISABLE_10BIT").is_ok() { + SUPPORTED_FORMATS_8BIT_ONLY + } else { + SUPPORTED_FORMATS + }; + let mut renderer = self.backend_data.gpus.single_renderer(&render_node).unwrap(); + let render_formats = renderer.as_mut().egl_context().dmabuf_render_formats().clone(); + + let drm_device_manager = DrmOutputManager::new( + drm, + allocator, + gbm.clone(), + Some(gbm), + color_formats.iter().copied(), + render_formats, + ); + self.backend_data.backends.insert( node, BackendData { registration_token, - gbm, - drm, + drm_output_manager: drm_device_manager, drm_scanner: DrmScanner::new(), non_desktop_connectors: Vec::new(), render_node, @@ -931,20 +798,20 @@ impl AnvilState { .gpus .single_renderer(&device.render_node) .unwrap(); - let render_formats = renderer.as_mut().egl_context().dmabuf_render_formats().clone(); let output_name = format!("{}-{}", connector.interface().as_str(), connector.interface_id()); info!(?crtc, "Trying to setup connector {}", output_name,); - let non_desktop = device - .drm + let drm_device = device.drm_output_manager.device(); + + let non_desktop = drm_device .get_properties(connector.handle()) .ok() .and_then(|props| { let (info, value) = props .into_iter() .filter_map(|(handle, value)| { - let info = device.drm.get_property(handle).ok()?; + let info = drm_device.get_property(handle).ok()?; Some((info, value)) }) @@ -954,7 +821,7 @@ impl AnvilState { }) .unwrap_or(false); - let display_info = display_info::for_connector(&device.drm, connector.handle()); + let display_info = display_info::for_connector(drm_device, connector.handle()); let make = display_info .as_ref() @@ -986,14 +853,6 @@ impl AnvilState { let drm_mode = connector.modes()[mode_id]; let wl_mode = WlMode::from(drm_mode); - let surface = match device.drm.create_surface(crtc, drm_mode, &[connector.handle()]) { - Ok(surface) => surface, - Err(err) => { - warn!("Failed to create drm surface: {}", err); - return; - } - }; - let (phys_w, phys_h) = connector.size().unwrap_or((0, 0)); let output = Output::new( output_name, @@ -1024,90 +883,71 @@ impl AnvilState { #[cfg(feature = "debug")] let fps_element = self.backend_data.fps_texture.clone().map(FpsElement::new); - let allocator = GbmAllocator::new( - device.gbm.clone(), - GbmBufferFlags::RENDERING | GbmBufferFlags::SCANOUT, - ); - - let color_formats = if std::env::var("ANVIL_DISABLE_10BIT").is_ok() { - SUPPORTED_FORMATS_8BIT_ONLY - } else { - SUPPORTED_FORMATS + let driver = match drm_device.get_driver() { + Ok(driver) => driver, + Err(err) => { + warn!("Failed to query drm driver: {}", err); + return; + } }; - let compositor = if std::env::var("ANVIL_DISABLE_DRM_COMPOSITOR").is_ok() { - let gbm_surface = - match GbmBufferedSurface::new(surface, allocator, color_formats, render_formats) { - Ok(renderer) => renderer, - Err(err) => { - warn!("Failed to create rendering surface: {}", err); - return; - } - }; - SurfaceComposition::Surface { - surface: gbm_surface, - damage_tracker: OutputDamageTracker::from_output(&output), - debug_flags: self.backend_data.debug_flags, + let mut planes = match drm_device.planes(&crtc) { + Ok(planes) => planes, + Err(err) => { + warn!("Failed to query crtc planes: {}", err); + return; } - } else { - let driver = match device.drm.get_driver() { - Ok(driver) => driver, - Err(err) => { - warn!("Failed to query drm driver: {}", err); - return; - } - }; - - let mut planes = surface.planes().clone(); + }; - // Using an overlay plane on a nvidia card breaks - if driver.name().to_string_lossy().to_lowercase().contains("nvidia") - || driver - .description() - .to_string_lossy() - .to_lowercase() - .contains("nvidia") - { - planes.overlay = vec![]; - } + // Using an overlay plane on a nvidia card breaks + if driver.name().to_string_lossy().to_lowercase().contains("nvidia") + || driver + .description() + .to_string_lossy() + .to_lowercase() + .contains("nvidia") + { + planes.overlay = vec![]; + } - let mut compositor = match DrmCompositor::new( + let drm_output = match device + .drm_output_manager + .initialize_output::<_, OutputRenderElements, WindowRenderElement>>>( + crtc + ).build( + &mut renderer, + drm_mode, + &[connector.handle()], &output, - surface, Some(planes), - allocator, - device.gbm.clone(), - color_formats, - render_formats, - device.drm.cursor_size(), - Some(device.gbm.clone()), ) { - Ok(compositor) => compositor, - Err(err) => { - warn!("Failed to create drm compositor: {}", err); - return; - } - }; - compositor.set_debug_flags(self.backend_data.debug_flags); - - let disable_direct_scanout = std::env::var("ANVIL_DISABLE_DIRECT_SCANOUT").is_ok(); - compositor.use_direct_scanout(!disable_direct_scanout); - SurfaceComposition::Compositor(compositor) + Ok(drm_output) => drm_output, + Err(err) => { + warn!("Failed to initialize drm output: {}", err); + return; + } }; - let dmabuf_feedback = get_surface_dmabuf_feedback( - self.backend_data.primary_gpu, - device.render_node, - &mut self.backend_data.gpus, - &compositor, - ); + let disable_direct_scanout = std::env::var("ANVIL_DISABLE_DIRECT_SCANOUT").is_ok(); + + let dmabuf_feedback = drm_output.with_compositor(|compositor| { + compositor.set_debug_flags(self.backend_data.debug_flags); + + get_surface_dmabuf_feedback( + self.backend_data.primary_gpu, + device.render_node, + &mut self.backend_data.gpus, + compositor.surface(), + ) + }); let surface = SurfaceData { dh: self.display_handle.clone(), device_id: node, render_node: device.render_node, global: Some(global), - compositor, + drm_output, + disable_direct_scanout, #[cfg(feature = "debug")] fps: fps_ticker::Fps::default(), #[cfg(feature = "debug")] @@ -1116,8 +956,6 @@ impl AnvilState { }; device.surfaces.insert(crtc, surface); - - self.schedule_initial_render(node, crtc, self.handle.clone()); } } @@ -1155,6 +993,20 @@ impl AnvilState { self.space.unmap_output(&output); } } + + let mut renderer = self + .backend_data + .gpus + .single_renderer(&device.render_node) + .unwrap(); + let _ = device.drm_output_manager.reset_format::<_, OutputRenderElements< + UdevRenderer<'_>, + WindowRenderElement>, + >, _>(&mut renderer, |_| { + // FIXME: For a flicker free operation we should return the actual elements for this output.. + // Instead we just use black to "simulate" a modeset :) + (&[], Color32F::BLACK) + }); } fn device_changed(&mut self, node: DrmNode) { @@ -1164,7 +1016,10 @@ impl AnvilState { return; }; - let scan_result = match device.drm_scanner.scan_connectors(&device.drm) { + let scan_result = match device + .drm_scanner + .scan_connectors(device.drm_output_manager.device()) + { Ok(scan_result) => scan_result, Err(err) => { tracing::warn!(?err, "Failed to scan connectors"); @@ -1282,11 +1137,12 @@ impl AnvilState { (self.clock.now(), wp_presentation_feedback::Kind::Vsync) }; - let schedule_render = match surface - .compositor + let submit_result = surface + .drm_output .frame_submitted() - .map_err(Into::::into) - { + .map_err(Into::::into); + + let schedule_render = match submit_result { Ok(user_data) => { if let Some(mut feedback) = user_data.flatten() { feedback.presented( @@ -1453,7 +1309,7 @@ impl AnvilState { let mut renderer = if primary_gpu == render_node { self.backend_data.gpus.single_renderer(&render_node) } else { - let format = surface.compositor.format(); + let format = surface.drm_output.format(); self.backend_data .gpus .renderer(&primary_gpu, &render_node, format) @@ -1517,7 +1373,11 @@ impl AnvilState { // reset the complete state, disabling all connectors and planes in case we hit a test failed // most likely we hit this after a tty switch when a foreign master changed CRTC <-> connector bindings // and we run in a mismatch - device.drm.reset_state().expect("failed to reset drm device"); + device + .drm_output_manager + .device_mut() + .reset_state() + .expect("failed to reset drm device"); true } _ => panic!("Rendering loop lost: {}", err), @@ -1557,44 +1417,6 @@ impl AnvilState { profiling::finish_frame!(); } - - fn schedule_initial_render( - &mut self, - node: DrmNode, - crtc: crtc::Handle, - evt_handle: LoopHandle<'static, AnvilState>, - ) { - let device = if let Some(device) = self.backend_data.backends.get_mut(&node) { - device - } else { - return; - }; - - let surface = if let Some(surface) = device.surfaces.get_mut(&crtc) { - surface - } else { - return; - }; - - let node = surface.render_node; - let result = { - let mut renderer = self.backend_data.gpus.single_renderer(&node).unwrap(); - initial_render(surface, &mut renderer) - }; - - if let Err(err) = result { - match err { - SwapBuffersError::AlreadySwapped => {} - SwapBuffersError::TemporaryFailure(err) => { - // TODO dont reschedule after 3(?) retries - warn!("Failed to submit page_flip: {}", err); - let handle = evt_handle.clone(); - evt_handle.insert_idle(move |data| data.schedule_initial_render(node, crtc, handle)); - } - SwapBuffersError::ContextLost(err) => panic!("Rendering loop lost: {}", err), - } - } - } } #[allow(clippy::too_many_arguments)] @@ -1688,36 +1510,38 @@ fn render_surface<'a>( let (elements, clear_color) = output_elements(output, space, custom_elements, renderer, show_window_preview); - let SurfaceCompositorRenderResult { - rendered, - states, - sync, - damage, - } = surface - .compositor - .render_frame::<_, _, GlesTexture>(renderer, &elements, clear_color)?; + + let frame_mode = surface + .disable_direct_scanout + .then_some(FrameMode::COMPOSITE) + .unwrap_or(FrameMode::ALL); + let (rendered, states) = surface + .drm_output + .render_frame(renderer, &elements, clear_color, frame_mode) + .map(|render_frame_result| { + #[cfg(feature = "renderer_sync")] + if let PrimaryPlaneElement::Swapchain(element) = render_frame_result.primary_element { + element.sync.wait(); + } + (!render_frame_result.is_empty, render_frame_result.states) + }) + .map_err(|err| match err { + smithay::backend::drm::compositor::RenderFrameError::PrepareFrame(err) => { + SwapBuffersError::from(err) + } + smithay::backend::drm::compositor::RenderFrameError::RenderFrame( + OutputDamageTrackerError::Rendering(err), + ) => SwapBuffersError::from(err), + _ => unreachable!(), + })?; if rendered { let output_presentation_feedback = take_presentation_feedback(output, space, &states); - let damage = damage.cloned(); surface - .compositor - .queue_frame(sync, damage, Some(output_presentation_feedback)) + .drm_output + .queue_frame(Some(output_presentation_feedback)) .map_err(Into::::into)?; } Ok((rendered, states)) } - -fn initial_render( - surface: &mut SurfaceData, - renderer: &mut UdevRenderer<'_>, -) -> Result<(), SwapBuffersError> { - surface - .compositor - .render_frame::<_, CustomRenderElements<_>, GlesTexture>(renderer, &[], CLEAR_COLOR)?; - surface.compositor.queue_frame(None, None, None)?; - surface.compositor.reset_buffers(); - - Ok(()) -} From fc12f5985e4464a4da7c4ac2bb7b8bf2c91eacf7 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Wed, 4 Dec 2024 17:11:07 +0100 Subject: [PATCH 09/24] drm/output: Use builder pattern for `use_mode` as well --- src/backend/drm/output.rs | 93 +++++++++++++++++++++++++++++++++------ 1 file changed, 80 insertions(+), 13 deletions(-) diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs index d3b633f945c0..52b82b56207a 100644 --- a/src/backend/drm/output.rs +++ b/src/backend/drm/output.rs @@ -503,11 +503,74 @@ where self.with_compositor(|compositor| compositor.commit_frame()) } - pub fn use_mode( + pub fn use_mode(&mut self) -> DrmModeSwitcher + where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, + { + DrmModeSwitcher { + compositor: self.compositor.clone(), + crtc: self.crtc, + render_elements: HashMap::new(), + _renderer: PhantomData, + } + } +} + +pub struct DrmModeSwitcher +where + A: Allocator + std::clone::Clone, + ::Buffer: AsDmabuf, + ::Error: Send + Sync + 'static, + <::Buffer as AsDmabuf>::Error: std::marker::Send + std::marker::Sync + 'static, + F: ExportFramebuffer<::Buffer> + std::clone::Clone, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + ::Buffer>>::Error: + std::marker::Send + std::marker::Sync + 'static, + G: AsFd + std::clone::Clone + 'static, + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, +{ + compositor: CompositorList, + crtc: crtc::Handle, + render_elements: HashMap, Color32F)>, + _renderer: PhantomData, +} + +impl DrmModeSwitcher +where + A: Allocator + std::clone::Clone, + ::Buffer: AsDmabuf, + ::Error: Send + Sync + 'static, + <::Buffer as AsDmabuf>::Error: std::marker::Send + std::marker::Sync + 'static, + F: ExportFramebuffer<::Buffer> + std::clone::Clone, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + ::Buffer>>::Error: + std::marker::Send + std::marker::Sync + 'static, + G: AsFd + std::clone::Clone + 'static, + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, +{ + pub fn add_render_contents( &mut self, + crtc: &crtc::Handle, + clear_color: Color32F, + elements: impl IntoIterator, + ) { + self.render_elements + .insert(*crtc, (elements.into_iter().collect(), clear_color)); + } + + pub fn commit( + self, renderer: &mut R, mode: Mode, - render_elements: RE, ) -> Result< (), DrmOutputManagerError< @@ -516,32 +579,36 @@ where ::Buffer>>::Error, R::Error, >, - > - where - E: RenderElement, - R: Renderer + Bind, - ::TextureId: Texture + 'static, - ::Error: Send + Sync + 'static, - RE: for<'r> Fn(&'r crtc::Handle) -> (&'r [E], Color32F), - { - let mut res = self.with_compositor(|compositor| compositor.use_mode(mode)); + > { + let mut res = { + let read_guard = self.compositor.read().unwrap(); + let mut compositor_guard = read_guard.get(&self.crtc).unwrap().lock().unwrap(); + compositor_guard.use_mode(mode) + }; + if res.is_err() { let mut write_guard = self.compositor.write().unwrap(); for (handle, compositor) in write_guard.iter_mut() { let mut compositor = compositor.lock().unwrap(); - let (elements, clear_color) = render_elements(handle); + let (elements, clear_color) = self + .render_elements + .get(handle) + .map(|(ref elements, ref color)| (&**elements, color)) + .unwrap_or((&[], &Color32F::BLACK)); compositor.reset_buffer_ages(); compositor - .render_frame(renderer, elements, clear_color, FrameMode::COMPOSITE) + .render_frame(renderer, elements, *clear_color, FrameMode::COMPOSITE) .map_err(DrmOutputManagerError::RenderFrame)?; compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; } + let compositor = write_guard.get_mut(&self.crtc).unwrap(); let mut compositor = compositor.lock().unwrap(); res = compositor.use_mode(mode); } + res.map_err(DrmOutputManagerError::Frame) } } From 10e8019d6e966b734eb4a72d845a57ba552bf7fd Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Wed, 4 Dec 2024 17:11:43 +0100 Subject: [PATCH 10/24] drm/output: Provide a way to lock the device for custom needs --- src/backend/drm/output.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs index 52b82b56207a..5a3c3bfe2ef8 100644 --- a/src/backend/drm/output.rs +++ b/src/backend/drm/output.rs @@ -167,6 +167,14 @@ where } } + pub fn with_compositors( + &mut self, + f: impl FnOnce(&HashMap>>) -> R, + ) -> R { + let write_guard = self.compositor.write().unwrap(); + f(&*write_guard) + } + pub fn reset_format( &mut self, renderer: &mut R, From 1fc5ddaff6d4ead00190ca835727976430535130 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Wed, 4 Dec 2024 17:26:25 +0100 Subject: [PATCH 11/24] drm/output: Also allow to set the mode from the manager --- src/backend/drm/output.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs index 5a3c3bfe2ef8..4d6384f58d95 100644 --- a/src/backend/drm/output.rs +++ b/src/backend/drm/output.rs @@ -175,6 +175,21 @@ where f(&*write_guard) } + pub fn use_mode(&mut self, crtc: crtc::Handle) -> DrmModeSwitcher + where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, + { + DrmModeSwitcher { + compositor: self.compositor.clone(), + crtc: crtc, + render_elements: HashMap::new(), + _renderer: PhantomData, + } + } + pub fn reset_format( &mut self, renderer: &mut R, From 211d5b5950033b434d9c3e1666ca26345181869a Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Mon, 9 Dec 2024 17:11:54 +0100 Subject: [PATCH 12/24] drm/output: Add a way to restore modifiers --- src/backend/allocator/swapchain.rs | 5 ++ src/backend/drm/compositor/mod.rs | 5 ++ src/backend/drm/output.rs | 110 +++++++++++++++++++++++++++-- 3 files changed, 116 insertions(+), 4 deletions(-) diff --git a/src/backend/allocator/swapchain.rs b/src/backend/allocator/swapchain.rs index f80df7b03c30..364a64b282aa 100644 --- a/src/backend/allocator/swapchain.rs +++ b/src/backend/allocator/swapchain.rs @@ -258,4 +258,9 @@ where pub fn format(&self) -> Fourcc { self.fourcc } + + /// Get allowed modifiers + pub fn modifiers(&self) -> &[Modifier] { + &self.modifiers + } } diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index fca335be6015..dd96c28b96b3 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -2710,6 +2710,11 @@ where self.swapchain.format() } + /// Get the allowed modifiers of the underlying swapchain + pub fn modifiers(&self) -> &[DrmModifier] { + self.swapchain.modifiers() + } + pub fn set_format( &mut self, allocator: A, diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs index 4d6384f58d95..c1ce07211117 100644 --- a/src/backend/drm/output.rs +++ b/src/backend/drm/output.rs @@ -190,10 +190,10 @@ where } } - pub fn reset_format( + pub fn try_to_restore_modifiers( &mut self, renderer: &mut R, - render_elements: RE, + render_elements: DrmOutputRenderElements, ) -> Result< (), DrmOutputManagerError< @@ -208,9 +208,44 @@ where R: Renderer + Bind, ::TextureId: Texture + 'static, ::Error: Send + Sync + 'static, - RE: for<'r> Fn(&'r crtc::Handle) -> (&'r [E], Color32F), { - // TODO: re-evaluate format and re-render if changed... + let mut write_guard = self.compositor.write().unwrap(); + + // check if implicit modifiers are in use + if write_guard + .values_mut() + .any(|c| c.lock().unwrap().modifiers() == &[DrmModifier::Invalid]) + { + // if so, first lower the bandwidth by disabling planes on all compositors + for compositor in write_guard.values_mut() { + let mut compositor = compositor.lock().unwrap(); + compositor.reset_buffer_ages(); + render_elements.submit_composited_frame(&mut *compositor, renderer)?; + } + + for compositor in write_guard.values_mut() { + let mut compositor = compositor.lock().unwrap(); + if compositor.modifiers() != &[DrmModifier::Invalid] { + continue; + } + + let current_format = compositor.format(); + if let Err(err) = compositor.set_format( + self.allocator.clone(), + current_format, + self.renderer_formats + .iter() + .filter(|f| f.code == current_format) + .map(|f| f.modifier), + ) { + tracing::warn!(?err, "failed to reset format"); + continue; + } + + render_elements.submit_composited_frame(&mut *compositor, renderer)?; + } + } + Ok(()) } @@ -669,3 +704,70 @@ where write_guard.remove(&self.crtc); } } + +pub struct DrmOutputRenderElements +where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, +{ + render_elements: HashMap, Color32F)>, + _renderer: PhantomData, +} + +impl DrmOutputRenderElements +where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, +{ + pub fn add_output( + &mut self, + crtc: &crtc::Handle, + clear_color: Color32F, + elements: impl IntoIterator, + ) { + self.render_elements + .insert(*crtc, (elements.into_iter().collect(), clear_color)); + } + + fn submit_composited_frame( + &self, + compositor: &mut DrmCompositor, + renderer: &mut R, + ) -> Result< + (), + DrmOutputManagerError< + ::Error, + <::Buffer as AsDmabuf>::Error, + ::Buffer>>::Error, + R::Error, + >, + > + where + A: Allocator + std::clone::Clone + std::fmt::Debug, + ::Buffer: AsDmabuf, + ::Error: Send + Sync + 'static, + <::Buffer as AsDmabuf>::Error: + std::marker::Send + std::marker::Sync + 'static, + F: ExportFramebuffer<::Buffer> + std::clone::Clone, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + ::Buffer>>::Error: + std::marker::Send + std::marker::Sync + 'static, + G: AsFd + std::clone::Clone + 'static, + U: 'static, + { + let (elements, clear_color) = self + .render_elements + .get(&compositor.crtc()) + .map(|(ref elements, ref color)| (&**elements, color)) + .unwrap_or((&[], &Color32F::BLACK)); + compositor + .render_frame(renderer, &elements, *clear_color, FrameMode::COMPOSITE) + .map_err(DrmOutputManagerError::RenderFrame)?; + compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; + Ok(()) + } +} From 91332f314b3f6713624b409bc54637b757929882 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Mon, 9 Dec 2024 17:12:47 +0100 Subject: [PATCH 13/24] drm/output: Refactor everything to use `DrmOutputRenderElements` --- anvil/src/drawing.rs | 3 +- anvil/src/udev.rs | 19 +- src/backend/drm/output.rs | 591 ++++++++++++++++++-------------------- 3 files changed, 284 insertions(+), 329 deletions(-) diff --git a/anvil/src/drawing.rs b/anvil/src/drawing.rs index 0dfe1edc725b..863c7624f087 100644 --- a/anvil/src/drawing.rs +++ b/anvil/src/drawing.rs @@ -6,7 +6,8 @@ use smithay::{ memory::{MemoryRenderBuffer, MemoryRenderBufferRenderElement}, surface::WaylandSurfaceRenderElement, AsRenderElements, Kind, - }, Color32F, ImportAll, ImportMem, Renderer, Texture + }, + Color32F, ImportAll, ImportMem, Renderer, Texture, }, input::pointer::CursorImageStatus, render_elements, diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index 1ea916e97c32..1ff452dd5709 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -32,7 +32,7 @@ use smithay::{ }, drm::{ compositor::{DrmCompositor, FrameMode}, - output::{DrmOutput, DrmOutputManager}, + output::{DrmOutput, DrmOutputManager, DrmOutputRenderElements}, CreateDrmNodeError, DrmAccessError, DrmDevice, DrmDeviceFd, DrmError, DrmEvent, DrmEventMetadata, DrmNode, DrmSurface, GbmBufferedSurface, NodeType, }, @@ -44,7 +44,7 @@ use smithay::{ element::{memory::MemoryRenderBuffer, AsRenderElements, RenderElementStates}, gles::GlesRenderer, multigpu::{gbm::GbmGlesBackend, GpuManager, MultiRenderer}, - Color32F, DebugFlags, ImportDma, ImportMemWl, + DebugFlags, ImportDma, ImportMemWl, }, session::{ libseat::{self, LibSeatSession}, @@ -913,13 +913,13 @@ impl AnvilState { let drm_output = match device .drm_output_manager .initialize_output::<_, OutputRenderElements, WindowRenderElement>>>( - crtc - ).build( - &mut renderer, + crtc, drm_mode, &[connector.handle()], &output, Some(planes), + &mut renderer, + &DrmOutputRenderElements::default(), ) { Ok(drm_output) => drm_output, Err(err) => { @@ -999,14 +999,15 @@ impl AnvilState { .gpus .single_renderer(&device.render_node) .unwrap(); - let _ = device.drm_output_manager.reset_format::<_, OutputRenderElements< + let _ = device.drm_output_manager.try_to_restore_modifiers::<_, OutputRenderElements< UdevRenderer<'_>, WindowRenderElement>, - >, _>(&mut renderer, |_| { + >>( + &mut renderer, // FIXME: For a flicker free operation we should return the actual elements for this output.. // Instead we just use black to "simulate" a modeset :) - (&[], Color32F::BLACK) - }); + &DrmOutputRenderElements::default(), + ); } fn device_changed(&mut self, node: DrmNode) { diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs index c1ce07211117..b4855e0cdb5e 100644 --- a/src/backend/drm/output.rs +++ b/src/backend/drm/output.rs @@ -152,19 +152,186 @@ where pub fn initialize_output<'a, R, E>( &'a mut self, crtc: crtc::Handle, - ) -> DrmOutputBuilder<'a, A, F, U, G, R, E> + mode: control::Mode, + connectors: &[connector::Handle], + output_mode_source: impl Into + std::fmt::Debug, + planes: Option, + renderer: &mut R, + render_elements: &DrmOutputRenderElements, + ) -> Result< + DrmOutput, + DrmOutputManagerError< + ::Error, + <::Buffer as AsDmabuf>::Error, + ::Buffer>>::Error, + R::Error, + >, + > where E: RenderElement, R: Renderer + Bind, ::TextureId: Texture + 'static, ::Error: Send + Sync + 'static, { - DrmOutputBuilder { - manager: self, - crtc, - render_elements: HashMap::new(), - _renderer: PhantomData, + let output_mode_source = output_mode_source.into(); + + let mut write_guard = self.compositor.write().unwrap(); + if write_guard.contains_key(&crtc) { + return Err(DrmOutputManagerError::DuplicateCrtc(crtc)); } + + let mut create_compositor = + |implicit_modifiers: bool| -> FrameResult, A, F> { + let surface = self.device.create_surface(crtc, mode, connectors)?; + + if implicit_modifiers { + DrmCompositor::::new( + output_mode_source.clone(), + surface, + planes.clone(), + self.allocator.clone(), + self.exporter.clone(), + self.color_formats.iter().copied(), + self.renderer_formats + .iter() + .filter(|f| f.modifier == DrmModifier::Invalid) + .copied(), + self.device.cursor_size(), + self.gbm.clone(), + ) + } else { + DrmCompositor::::new( + output_mode_source.clone(), + surface, + planes.clone(), + self.allocator.clone(), + self.exporter.clone(), + self.color_formats.iter().copied(), + self.renderer_formats.iter().copied(), + self.device.cursor_size(), + self.gbm.clone(), + ) + } + }; + + let compositor = create_compositor(false); + + // Okay, so this can fail for various reasons... + // + // Enabling an additional CRTC might fail because the bandwidth + // requirement is higher then supported with the current configuration. + // + // * Bandwidth limitation caused by overlay plane usage: + // Each overlay plane requires some certain bandwidth and we only + // test that during plane assignment implicitly through an atomic test. + // When trying to enable an additional CRTC we might hit some limit and the + // only way to resolve this might be to disable all overlay planes and + // retry enabling the CRTC. + // + // * Bandwidth limitation caused by the primary plane format: + // Different formats (might) require a higher memory bandwidth than others. + // This also applies to the same fourcc with different modifiers. For example + // the Intel CCS Formats use an additional plane to transport meta-data. + // So if we fail to enable an additional CRTC we might be able to resolve + // the issue by using a different format. Again the only way to know is by + // trying out what works. + // + // So for now we try to disable overlay planes first. If that doesn't work, + // we set the modifiers to `Invalid` (for now) to give the driver the opportunity + // to re-allocate in the background. (TODO: Do this ourselves). + + match compositor { + Ok(compositor) => { + write_guard.insert(crtc, Mutex::new(compositor)); + } + Err(err) => { + tracing::warn!( + ?crtc, + ?err, + "failed to initialize crtc, trying to lower bandwidth" + ); + + for compositor in write_guard.values_mut() { + let mut compositor = compositor.lock().unwrap(); + compositor.reset_buffer_ages(); + render_elements.submit_composited_frame(&mut *compositor, renderer)?; + } + + let compositor = create_compositor(false); + + match compositor { + Ok(compositor) => { + write_guard.insert(crtc, Mutex::new(compositor)); + } + Err(err) => { + tracing::warn!( + ?crtc, + ?err, + "failed to initialize crtc, trying implicit modifiers" + ); + + for compositor in write_guard.values_mut() { + let mut compositor = compositor.lock().unwrap(); + + let current_format = compositor.format(); + if let Err(err) = compositor.set_format( + self.allocator.clone(), + current_format, + [DrmModifier::Invalid], + ) { + tracing::warn!(?err, "failed to set new format"); + continue; + } + + render_elements.submit_composited_frame(&mut *compositor, renderer)?; + } + + let compositor = create_compositor(true); + + match compositor { + Ok(compositor) => { + write_guard.insert(crtc, Mutex::new(compositor)); + } + Err(err) => { + // try to reset formats + for compositor in write_guard.values_mut() { + let mut compositor = compositor.lock().unwrap(); + + let current_format = compositor.format(); + if let Err(err) = compositor.set_format( + self.allocator.clone(), + current_format, + self.renderer_formats + .iter() + .filter(|f| f.code == current_format) + .map(|f| f.modifier), + ) { + tracing::warn!(?err, "failed to reset format"); + continue; + } + + render_elements.submit_composited_frame(&mut *compositor, renderer)?; + } + + return Err(DrmOutputManagerError::Frame(err)); + } + } + } + } + } + }; + + // We need to render the new output once to lock in the primary plane as used with the new format, so we don't hit the bandwidth issue, + // when downstream potentially uses `FrameMode::ALL` immediately after this. + + let compositor = write_guard.get_mut(&crtc).unwrap(); + let mut compositor = compositor.lock().unwrap(); + render_elements.submit_composited_frame(&mut *compositor, renderer)?; + + Ok(DrmOutput { + crtc, + compositor: self.compositor.clone(), + }) } pub fn with_compositors( @@ -175,25 +342,34 @@ where f(&*write_guard) } - pub fn use_mode(&mut self, crtc: crtc::Handle) -> DrmModeSwitcher + pub fn use_mode( + &mut self, + crtc: &crtc::Handle, + mode: Mode, + renderer: &mut R, + render_elements: &DrmOutputRenderElements, + ) -> Result< + (), + DrmOutputManagerError< + ::Error, + <::Buffer as AsDmabuf>::Error, + ::Buffer>>::Error, + R::Error, + >, + > where E: RenderElement, R: Renderer + Bind, ::TextureId: Texture + 'static, ::Error: Send + Sync + 'static, { - DrmModeSwitcher { - compositor: self.compositor.clone(), - crtc: crtc, - render_elements: HashMap::new(), - _renderer: PhantomData, - } + use_mode_internal(&self.compositor, crtc, mode, renderer, render_elements) } pub fn try_to_restore_modifiers( &mut self, renderer: &mut R, - render_elements: DrmOutputRenderElements, + render_elements: &DrmOutputRenderElements, ) -> Result< (), DrmOutputManagerError< @@ -264,213 +440,6 @@ where } } -pub struct DrmOutputBuilder<'a, A, F, U, G, R, E> -where - A: Allocator + std::clone::Clone + std::fmt::Debug, - ::Buffer: AsDmabuf, - ::Error: Send + Sync + 'static, - <::Buffer as AsDmabuf>::Error: - std::marker::Send + std::marker::Sync + 'static, - F: ExportFramebuffer<::Buffer> + std::clone::Clone, - ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, - ::Buffer>>::Error: - std::marker::Send + std::marker::Sync + 'static, - G: AsFd + std::clone::Clone + 'static, - U: 'static, - E: RenderElement, - R: Renderer + Bind, - ::TextureId: Texture + 'static, - ::Error: Send + Sync + 'static, -{ - manager: &'a mut DrmOutputManager, - crtc: crtc::Handle, - render_elements: HashMap, Color32F)>, - _renderer: PhantomData, -} - -impl<'a, A, F, U, G, R, E> DrmOutputBuilder<'a, A, F, U, G, R, E> -where - A: Allocator + std::clone::Clone + std::fmt::Debug, - ::Buffer: AsDmabuf, - ::Error: Send + Sync + 'static, - <::Buffer as AsDmabuf>::Error: - std::marker::Send + std::marker::Sync + 'static, - F: ExportFramebuffer<::Buffer> + std::clone::Clone, - ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, - ::Buffer>>::Error: - std::marker::Send + std::marker::Sync + 'static, - G: AsFd + std::clone::Clone + 'static, - U: 'static, - E: RenderElement, - R: Renderer + Bind, - ::TextureId: Texture + 'static, - ::Error: Send + Sync + 'static, -{ - pub fn add_render_contents( - &mut self, - crtc: &crtc::Handle, - clear_color: Color32F, - elements: impl IntoIterator, - ) { - self.render_elements - .insert(*crtc, (elements.into_iter().collect(), clear_color)); - } - - pub fn build( - self, - renderer: &mut R, - mode: control::Mode, - connectors: &[connector::Handle], - output_mode_source: impl Into + std::fmt::Debug, - planes: Option, - ) -> Result< - DrmOutput, - DrmOutputManagerError< - ::Error, - <::Buffer as AsDmabuf>::Error, - ::Buffer>>::Error, - R::Error, - >, - > { - let output_mode_source = output_mode_source.into(); - - let mut write_guard = self.manager.compositor.write().unwrap(); - if write_guard.contains_key(&self.crtc) { - return Err(DrmOutputManagerError::DuplicateCrtc(self.crtc)); - } - - let surface = self - .manager - .device - .create_surface(self.crtc, mode, connectors) - .map_err(DrmOutputManagerError::Drm)?; - - let compositor = DrmCompositor::::new( - output_mode_source.clone(), - surface, - planes.clone(), - self.manager.allocator.clone(), - self.manager.exporter.clone(), - self.manager.color_formats.iter().copied(), - self.manager.renderer_formats.iter().copied(), - self.manager.device.cursor_size(), - self.manager.gbm.clone(), - ); - - match compositor { - Ok(compositor) => { - write_guard.insert(self.crtc, Mutex::new(compositor)); - } - Err(err) => { - tracing::warn!(crtc=?self.crtc, ?err, "failed to initialize crtc"); - // Okay, so this can fail for various reasons... - // - // Enabling an additional CRTC might fail because the bandwidth - // requirement is higher then supported with the current configuration. - // - // * Bandwidth limitation caused by overlay plane usage: - // Each overlay plane requires some certain bandwidth and we only - // test that during plane assignment implicitly through an atomic test. - // When trying to enable an additional CRTC we might hit some limit and the - // only way to resolve this might be to disable all overlay planes and - // retry enabling the CRTC. - // - // * Bandwidth limitation caused by the primary plane format: - // Different formats (might) require a higher memory bandwidth than others. - // This also applies to the same fourcc with different modifiers. For example - // the Intel CCS Formats use an additional plane to transport meta-data. - // So if we fail to enable an additional CRTC we might be able to resolve - // the issue by using a different format. Again the only way to know is by - // trying out what works. - // - // So the easiest option is to evaluate a new format and re-create all DrmCompositor - // without disabling the CRTCs. - - // TODO: Find out the *best* working combination of formats per active crtc - for (handle, compositor) in write_guard.iter_mut() { - let mut compositor = compositor.lock().unwrap(); - - let (elements, clear_color) = self - .render_elements - .get(handle) - .map(|(ref elements, ref color)| (&**elements, color)) - .unwrap_or((&[], &Color32F::BLACK)); - compositor.reset_buffer_ages(); - compositor - .render_frame(renderer, &elements, *clear_color, FrameMode::COMPOSITE) - .map_err(DrmOutputManagerError::RenderFrame)?; - compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; - - let current_format = compositor.format(); - if let Err(err) = compositor.set_format( - self.manager.allocator.clone(), - current_format, - [DrmModifier::Invalid], - ) { - tracing::warn!(?err, "failed to set new format"); - } - - compositor - .render_frame(renderer, &elements, *clear_color, FrameMode::COMPOSITE) - .map_err(DrmOutputManagerError::RenderFrame)?; - compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; - } - - // :) Use the selected format instead of replicating DrmCompositor format selection... - let mut init_err = None; - for format in self.manager.color_formats.iter() { - let surface = self - .manager - .device - .create_surface(self.crtc, mode, connectors) - .map_err(DrmOutputManagerError::Drm)?; - - let compositor = DrmCompositor::::with_format( - output_mode_source.clone(), - surface, - planes.clone(), - self.manager.allocator.clone(), - self.manager.exporter.clone(), - *format, - [DrmModifier::Invalid], - self.manager.device.cursor_size(), - self.manager.gbm.clone(), - ); - - match compositor { - Ok(compositor) => { - write_guard.insert(self.crtc, Mutex::new(compositor)); - break; - } - Err(err) => init_err = Some(err), - } - } - - if let Some(err) = init_err { - return Err(DrmOutputManagerError::Frame(err)); - } - } - }; - - let compositor = write_guard.get_mut(&self.crtc).unwrap(); - let mut compositor = compositor.lock().unwrap(); - let (elements, clear_color) = self - .render_elements - .get(&self.crtc) - .map(|(ref elements, ref color)| (&**elements, color)) - .unwrap_or((&[], &Color32F::BLACK)); - compositor - .render_frame(renderer, elements, *clear_color, FrameMode::COMPOSITE) - .map_err(DrmOutputManagerError::RenderFrame)?; - compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; - - Ok(DrmOutput { - crtc: self.crtc, - compositor: self.manager.compositor.clone(), - }) - } -} - pub struct DrmOutput where A: Allocator, @@ -504,7 +473,7 @@ where impl DrmOutput where - A: Allocator + std::clone::Clone, + A: Allocator + std::clone::Clone + fmt::Debug, ::Buffer: AsDmabuf, ::Error: Send + Sync + 'static, <::Buffer as AsDmabuf>::Error: std::marker::Send + std::marker::Sync + 'static, @@ -513,6 +482,7 @@ where ::Buffer>>::Error: std::marker::Send + std::marker::Sync + 'static, G: AsFd + std::clone::Clone + 'static, + U: 'static, { /// Set the [`DebugFlags`] to use /// @@ -561,74 +531,11 @@ where self.with_compositor(|compositor| compositor.commit_frame()) } - pub fn use_mode(&mut self) -> DrmModeSwitcher - where - E: RenderElement, - R: Renderer + Bind, - ::TextureId: Texture + 'static, - ::Error: Send + Sync + 'static, - { - DrmModeSwitcher { - compositor: self.compositor.clone(), - crtc: self.crtc, - render_elements: HashMap::new(), - _renderer: PhantomData, - } - } -} - -pub struct DrmModeSwitcher -where - A: Allocator + std::clone::Clone, - ::Buffer: AsDmabuf, - ::Error: Send + Sync + 'static, - <::Buffer as AsDmabuf>::Error: std::marker::Send + std::marker::Sync + 'static, - F: ExportFramebuffer<::Buffer> + std::clone::Clone, - ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, - ::Buffer>>::Error: - std::marker::Send + std::marker::Sync + 'static, - G: AsFd + std::clone::Clone + 'static, - E: RenderElement, - R: Renderer + Bind, - ::TextureId: Texture + 'static, - ::Error: Send + Sync + 'static, -{ - compositor: CompositorList, - crtc: crtc::Handle, - render_elements: HashMap, Color32F)>, - _renderer: PhantomData, -} - -impl DrmModeSwitcher -where - A: Allocator + std::clone::Clone, - ::Buffer: AsDmabuf, - ::Error: Send + Sync + 'static, - <::Buffer as AsDmabuf>::Error: std::marker::Send + std::marker::Sync + 'static, - F: ExportFramebuffer<::Buffer> + std::clone::Clone, - ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, - ::Buffer>>::Error: - std::marker::Send + std::marker::Sync + 'static, - G: AsFd + std::clone::Clone + 'static, - E: RenderElement, - R: Renderer + Bind, - ::TextureId: Texture + 'static, - ::Error: Send + Sync + 'static, -{ - pub fn add_render_contents( + pub fn use_mode( &mut self, - crtc: &crtc::Handle, - clear_color: Color32F, - elements: impl IntoIterator, - ) { - self.render_elements - .insert(*crtc, (elements.into_iter().collect(), clear_color)); - } - - pub fn commit( - self, - renderer: &mut R, mode: Mode, + renderer: &mut R, + render_elements: &DrmOutputRenderElements, ) -> Result< (), DrmOutputManagerError< @@ -637,37 +544,14 @@ where ::Buffer>>::Error, R::Error, >, - > { - let mut res = { - let read_guard = self.compositor.read().unwrap(); - let mut compositor_guard = read_guard.get(&self.crtc).unwrap().lock().unwrap(); - compositor_guard.use_mode(mode) - }; - - if res.is_err() { - let mut write_guard = self.compositor.write().unwrap(); - - for (handle, compositor) in write_guard.iter_mut() { - let mut compositor = compositor.lock().unwrap(); - - let (elements, clear_color) = self - .render_elements - .get(handle) - .map(|(ref elements, ref color)| (&**elements, color)) - .unwrap_or((&[], &Color32F::BLACK)); - compositor.reset_buffer_ages(); - compositor - .render_frame(renderer, elements, *clear_color, FrameMode::COMPOSITE) - .map_err(DrmOutputManagerError::RenderFrame)?; - compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; - } - - let compositor = write_guard.get_mut(&self.crtc).unwrap(); - let mut compositor = compositor.lock().unwrap(); - res = compositor.use_mode(mode); - } - - res.map_err(DrmOutputManagerError::Frame) + > + where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, + { + use_mode_internal(&self.compositor, &self.crtc, mode, renderer, render_elements) } } @@ -705,6 +589,60 @@ where } } +fn use_mode_internal( + compositor: &CompositorList, + crtc: &crtc::Handle, + mode: Mode, + renderer: &mut R, + render_elements: &DrmOutputRenderElements, +) -> Result< + (), + DrmOutputManagerError< + ::Error, + <::Buffer as AsDmabuf>::Error, + ::Buffer>>::Error, + R::Error, + >, +> +where + A: Allocator + std::clone::Clone + fmt::Debug, + ::Buffer: AsDmabuf, + ::Error: Send + Sync + 'static, + <::Buffer as AsDmabuf>::Error: + std::marker::Send + std::marker::Sync + 'static, + F: ExportFramebuffer<::Buffer> + std::clone::Clone, + ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, + ::Buffer>>::Error: + std::marker::Send + std::marker::Sync + 'static, + G: AsFd + std::clone::Clone + 'static, + U: 'static, + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, +{ + let mut write_guard = compositor.write().unwrap(); + + let mut res = { + let mut compositor_guard = write_guard.get(crtc).unwrap().lock().unwrap(); + compositor_guard.use_mode(mode) + }; + + if res.is_err() { + for compositor in write_guard.values_mut() { + let mut compositor = compositor.lock().unwrap(); + render_elements.submit_composited_frame(&mut compositor, renderer)?; + } + + let compositor = write_guard.get_mut(crtc).unwrap(); + let mut compositor = compositor.lock().unwrap(); + res = compositor.use_mode(mode); + } + + res.map_err(DrmOutputManagerError::Frame) +} + +#[derive(Debug)] pub struct DrmOutputRenderElements where E: RenderElement, @@ -716,6 +654,21 @@ where _renderer: PhantomData, } +impl Default for DrmOutputRenderElements +where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, +{ + fn default() -> Self { + DrmOutputRenderElements { + render_elements: HashMap::new(), + _renderer: PhantomData, + } + } +} + impl DrmOutputRenderElements where E: RenderElement, From 6522b680d293b45ccedee76c2eba89724880c041 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Tue, 10 Dec 2024 17:43:17 +0100 Subject: [PATCH 14/24] drm/compositor: Don't scanout format other than swapchain on primary plane --- src/backend/drm/compositor/mod.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index dd96c28b96b3..3f4473e10003 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -2895,6 +2895,21 @@ where true, )?; + if let ScanoutBuffer::Swapchain(slot) = &frame_state + .plane_buffer(self.surface.plane()) + .expect("We have a buffer for the primary plane") + .buffer + { + if slot.format() != element_config.properties.format { + trace!( + "failed to assign element {:?} to primary {:?}, format doesn't match", + element.id(), + self.surface.plane() + ); + return Err(None); + } + } + let has_underlay = self .planes .overlay From 0d5fe0e2600388caaa439157653f3552f01a7f88 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Tue, 10 Dec 2024 17:43:28 +0100 Subject: [PATCH 15/24] drm/output: Add documentation --- src/backend/drm/compositor/mod.rs | 39 ++++++- src/backend/drm/exporter/mod.rs | 10 +- src/backend/drm/output.rs | 162 +++++++++++++++++++++++++++++- 3 files changed, 202 insertions(+), 9 deletions(-) diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index 3f4473e10003..1d14a1ad967f 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -1017,12 +1017,15 @@ bitflags::bitflags! { pub struct FrameMode: u32 { /// Allow to realize the frame by compositing elements on the primary plane const COMPOSITE = 1; + /// Allow to realize the frame by scanning out elements on the primary plane const PRIMARY_PLANE_SCANOUT = 2; + /// Allow to realize the frame by scanning out elements on overlay planes const OVERLAY_PLANE_SCANOUT = 4; + /// Allow to realize the frame by scanning out elements on cursor planes const CURSOR_PLANE_SCANOUT = 8; - /// Allow to realize the frame by assigning elements on planes + /// Allow to realize the frame by assigning elements on any plane const SCANOUT = Self::PRIMARY_PLANE_SCANOUT.bits() | Self::OVERLAY_PLANE_SCANOUT.bits() | Self::CURSOR_PLANE_SCANOUT.bits(); - + /// Allow all available operations to realize the frame const ALL = Self::COMPOSITE.bits() | Self::SCANOUT.bits(); } } @@ -1268,6 +1271,23 @@ where Err(error.unwrap()) } + /// Initialize a new [`DrmCompositor`] with a pre-selected format. + /// + /// The [`OutputModeSource`] can be created from an [`Output`](crate::output::Output), which will automatically track + /// the output's mode changes. An [`OutputModeSource::Static`] variant should only be used when + /// manually updating modes using [`DrmCompositor::set_output_mode_source`]. + /// + /// - `output_mode_source` is used to determine the current mode, scale and transform + /// - `surface` for the compositor to use + /// - `planes` defines which planes the compositor is allowed to use for direct scan-out. + /// `None` will result in the compositor to use all planes as specified by [`DrmSurface::planes`] + /// - `allocator` used for the primary plane swapchain + /// - `framebuffer_exporter` is used to create drm framebuffers for the swapchain buffers (and if possible + /// for element buffers) for scan-out + /// - `code` is the fixed format to initialize the framebuffer with + /// - `modifiers` is the set of modifiers allowed, when allocating buffers with the specified color format + /// - `cursor_size` as reported by the drm device, used for creating buffer for the cursor plane + /// - `gbm` device used for creating buffers for the cursor plane, `None` will disable the cursor plane pub fn with_format( output_mode_source: impl Into + Debug, surface: DrmSurface, @@ -1647,6 +1667,7 @@ where /// Render the next frame /// /// - `elements` for this frame in front-to-back order + /// - `frame_mode` specifies techniques allowed to realize the frame #[instrument(level = "trace", parent = &self.span, skip_all)] #[profiling::function] pub fn render_frame<'a, R, E>( @@ -2429,6 +2450,19 @@ where Ok(()) } + /// Commits the current frame for scan-out. + /// + /// If `render_frame` has not been called prior to this function or returned no damage + /// this function will return [`FrameError::EmptyFrame`]. Instead of calling `commit_frame` it + /// is the callers responsibility to re-schedule the frame. A simple strategy for frame + /// re-scheduling is to queue a one-shot timer that will trigger after approximately one + /// retrace duration. + /// + /// *Note*: It is your responsibility to synchronize rendering if the [`RenderFrameResult`] + /// returned by the previous [`render_frame`](DrmCompositor::render_frame) call returns `true` on [`RenderFrameResult::needs_sync`]. + /// + /// *Note*: This function should not be followed up with [`DrmCompositor::frame_submitted`] + /// and will not generate a vblank event on the underlying device. pub fn commit_frame(&mut self) -> FrameResult<(), A, F> { if !self.surface.is_active() { return Err(FrameErrorType::::DrmError(DrmError::DeviceInactive)); @@ -2715,6 +2749,7 @@ where self.swapchain.modifiers() } + /// Reset the underlying swapchain and assign a new color format. pub fn set_format( &mut self, allocator: A, diff --git a/src/backend/drm/exporter/mod.rs b/src/backend/drm/exporter/mod.rs index eac377a13c91..303e13799c04 100644 --- a/src/backend/drm/exporter/mod.rs +++ b/src/backend/drm/exporter/mod.rs @@ -1,4 +1,10 @@ -use std::{cell::RefCell, rc::Rc, sync::{Arc, Mutex}}; +//! Trait and data structures to describe types, that can be exported to a drm framebuffer + +use std::{ + cell::RefCell, + rc::Rc, + sync::{Arc, Mutex}, +}; use wayland_server::protocol::wl_buffer::WlBuffer; @@ -103,4 +109,4 @@ where fn can_add_framebuffer(&self, buffer: &ExportBuffer<'_, B>) -> bool { self.borrow().can_add_framebuffer(buffer) } -} \ No newline at end of file +} diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs index b4855e0cdb5e..f5ea23744ff8 100644 --- a/src/backend/drm/output.rs +++ b/src/backend/drm/output.rs @@ -1,3 +1,5 @@ +//! Device-wide synchronization helpers + use std::{ collections::HashMap, fmt, @@ -32,6 +34,15 @@ use super::{ type CompositorList = Arc>>>>; +/// Provides synchronization between a [`DrmDevice`] and derived [`DrmOutput`]s. +/// +/// When working with a single [`DrmDevice`] with multiple outputs and using the [`DrmCompositor`] +/// for hardware scanout one can quickly run into a set of bandwidth issues, where plane usage on +/// on [`DrmCompositor`] can cause commits on another to fail. Especially in multi-threaded contexts +/// these scenarios are difficult to handle as your need information over the state of all outputs at once. +/// +/// The `DrmOutputManager` provides a way to handle operation commonly affected by this by locking the +/// whole device, while still providing [`DrmOutput`]-handles to drive individual surfaces. pub struct DrmOutputManager where A: Allocator, @@ -77,10 +88,12 @@ where ::Buffer>>::Framebuffer: fmt::Debug + 'static, G: AsFd + 'static, { + /// Access the underlying managed device pub fn device(&self) -> &DrmDevice { &self.device } + /// Mutably access the underlying managed device pub fn device_mut(&mut self) -> &mut DrmDevice { &mut self.device } @@ -93,11 +106,13 @@ where ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, G: AsFd + 'static, { + /// Pause the underlying device. See [`DrmDevice::pause`]. pub fn pause(&mut self) { self.device.pause(); } } +/// Errors returned by `DrmOutputManager`'s methods #[derive(thiserror::Error, Debug)] pub enum DrmOutputManagerError where @@ -106,12 +121,16 @@ where F: std::error::Error + Send + Sync + 'static, R: std::error::Error + Send + Sync + 'static, { + /// The specified CRTC is already in use #[error("The specified CRTC {0:?} is already in use.")] DuplicateCrtc(crtc::Handle), + /// The underlying drm device returned an error #[error(transparent)] Drm(#[from] DrmError), + /// The underlying [`DrmCompositor`] returned an error #[error(transparent)] Frame(FrameError), + /// The underlying [`DrmCompositor`] returned an error upon rendering a frame #[error(transparent)] RenderFrame(RenderFrameError), } @@ -130,6 +149,19 @@ where G: AsFd + std::clone::Clone + 'static, U: 'static, { + /// Create a new [`DrmOutputManager`] from a [`DrmDevice`]. + /// + /// - `device` the underlying [`DrmDevice`] + /// - `allocator` used by created [`DrmOutput`]s for primary plane swapchains. + /// - `exporter` used by created [`DrmOutput`]s to create drm framebuffers + /// for the swapchain buffers (and if possible for element buffers) + /// for scan-out. + /// - `gbm` device used by created [`DrmOutput`]s for creating buffers for the + /// cursor plane, `None` will disable the cursor plane. + /// - `color_formats` as tested in order when creating a new [`DrmOutput`] + /// - `renderer_formats` as reported by the used renderer, used to build the + /// intersection between the possible scan-out formats of the + /// primary plane of created [`DrmOutput`]s and the renderer pub fn new( device: DrmDevice, allocator: A, @@ -149,6 +181,24 @@ where } } + /// Create a new [`DrmOutput`] for the provided crtc of the underlying device. + /// + /// The [`OutputModeSource`] can be created from an [`Output`](crate::output::Output), which will automatically track + /// the output's mode changes. An [`OutputModeSource::Static`] variant should only be used when + /// manually updating modes using [`DrmCompositor::set_output_mode_source`]. + /// + /// This might cause commits on other surfaces to meet the bandwidth + /// requirements of the output by temporarily disabling additional planes, + /// forcing composition or falling back to implicit modifiers. + /// + /// - `crtc` - the crtc the underlying surface should drive + /// - `mode` - the mode the underlying surface should be initialized with + /// - `connectors` - the set of connectors the underlying surface should be initialized with + /// - `output_mode_source` used to to determine the size, scale and transform + /// - `planes` defines which planes the compositor is allowed to use for direct scan-out. + /// `None` will result in the compositor to use all planes as specified by [`DrmSurface::planes`] + /// - `renderer` used for compositing, when commits are necessarily to realize bandwidth constraints + /// - `render_elements` used for rendering, when commits are necessarily to realize bandwidth constraints pub fn initialize_output<'a, R, E>( &'a mut self, crtc: crtc::Handle, @@ -253,8 +303,11 @@ where for compositor in write_guard.values_mut() { let mut compositor = compositor.lock().unwrap(); - compositor.reset_buffer_ages(); - render_elements.submit_composited_frame(&mut *compositor, renderer)?; + if let Err(err) = render_elements.submit_composited_frame(&mut *compositor, renderer) { + if !matches!(err, DrmOutputManagerError::Frame(FrameError::EmptyFrame)) { + return Err(err); + } + } } let compositor = create_compositor(false); @@ -334,6 +387,7 @@ where }) } + /// Grants exclusive access to all underlying [`DrmCompositor`]s. pub fn with_compositors( &mut self, f: impl FnOnce(&HashMap>>) -> R, @@ -342,6 +396,14 @@ where f(&*write_guard) } + /// Tries to apply a new [`Mode`] for the provided `crtc`. + /// + /// Fails if the mode is not compatible with the underlying + /// [`crtc`] or any of the pending [`connector`]s. + /// + /// This might cause commits on other surfaces to meet the bandwidth + /// requirements of the new mode by temporarily disabling additional planes + /// and forcing composition. pub fn use_mode( &mut self, crtc: &crtc::Handle, @@ -366,6 +428,13 @@ where use_mode_internal(&self.compositor, crtc, mode, renderer, render_elements) } + /// Tries to restore explicit modifiers on all surfaces. + /// + /// Adding new outputs (via [`DrmOutputManager::initialize_output`]) might cause + /// surfaces to fall back to implicit modifiers to satisfy bandwidth requirements + /// of the new output. These should generally be avoided, so it is recommended + /// to call this method after destroying an individual output to try and re-allocate + /// implicit buffers used for the remaining outputs. pub fn try_to_restore_modifiers( &mut self, renderer: &mut R, @@ -395,8 +464,11 @@ where // if so, first lower the bandwidth by disabling planes on all compositors for compositor in write_guard.values_mut() { let mut compositor = compositor.lock().unwrap(); - compositor.reset_buffer_ages(); - render_elements.submit_composited_frame(&mut *compositor, renderer)?; + if let Err(err) = render_elements.submit_composited_frame(&mut *compositor, renderer) { + if !matches!(err, DrmOutputManagerError::Frame(FrameError::EmptyFrame)) { + return Err(err); + } + } } for compositor in write_guard.values_mut() { @@ -425,6 +497,12 @@ where Ok(()) } + /// Activates a previously paused device. + /// + /// Specifying `true` for `disable_connectors` will call [`DrmDevice::reset_state`] if + /// the device was not active before. Otherwise you need to make sure there are no + /// conflicting requirements when enabling or creating surfaces or you are prepared + /// to handle errors caused by those. pub fn activate(&mut self, disable_connectors: bool) -> Result<(), DrmError> { self.device.activate(disable_connectors)?; @@ -440,6 +518,7 @@ where } } +/// A handle to an underlying [`DrmCompositor`] handled by an [`DrmOutputManager`]. pub struct DrmOutput where A: Allocator, @@ -497,14 +576,24 @@ where self.with_compositor(|compositor| compositor.reset_buffers()); } + /// Marks the current frame as submitted. + /// + /// *Note*: Needs to be called, after the vblank event of the matching [`DrmDevice`] + /// was received after calling [`DrmOutput::queue_frame`] on this surface. + /// Otherwise the underlying swapchain will run out of buffers eventually. pub fn frame_submitted(&self) -> FrameResult, A, F> { self.with_compositor(|compositor| compositor.frame_submitted()) } + /// Get the format of the underlying swapchain pub fn format(&self) -> DrmFourcc { self.with_compositor(|compositor| compositor.format()) } + /// Render the next frame + /// + /// - `elements` for this frame in front-to-back order + /// - `frame_mode` specifies techniques allowed to realize the frame pub fn render_frame<'a, R, E>( &mut self, renderer: &mut R, @@ -523,14 +612,52 @@ where }) } + /// Queues the current frame for scan-out. + /// + /// If `render_frame` has not been called prior to this function or returned no damage + /// this function will return [`FrameError::EmptyFrame`]. Instead of calling `queue_frame` it + /// is the callers responsibility to re-schedule the frame. A simple strategy for frame + /// re-scheduling is to queue a one-shot timer that will trigger after approximately one + /// retrace duration. + /// + /// *Note*: It is your responsibility to synchronize rendering if the [`RenderFrameResult`] + /// returned by the previous [`render_frame`](DrmOutput::render_frame) call returns `true` on [`RenderFrameResult::needs_sync`]. + /// + /// *Note*: This function needs to be followed up with [`DrmOutput::frame_submitted`] + /// when a vblank event is received, that denotes successful scan-out of the frame. + /// Otherwise the underlying swapchain will eventually run out of buffers. + /// + /// `user_data` can be used to attach some data to a specific buffer and later retrieved with [`DrmCompositor::frame_submitted`] pub fn queue_frame(&mut self, user_data: U) -> FrameResult<(), A, F> { self.with_compositor(|compositor| compositor.queue_frame(user_data)) } + /// Commits the current frame for scan-out. + /// + /// If `render_frame` has not been called prior to this function or returned no damage + /// this function will return [`FrameError::EmptyFrame`]. Instead of calling `commit_frame` it + /// is the callers responsibility to re-schedule the frame. A simple strategy for frame + /// re-scheduling is to queue a one-shot timer that will trigger after approximately one + /// retrace duration. + /// + /// *Note*: It is your responsibility to synchronize rendering if the [`RenderFrameResult`] + /// returned by the previous [`render_frame`](DrmOutput::render_frame) call returns `true` on [`RenderFrameResult::needs_sync`]. + /// + /// *Note*: This function should not be followed up with [`DrmOutput::frame_submitted`] + /// and will not generate a vblank event on the underlying device. + pub fn commit_frame(&mut self) -> FrameResult<(), A, F> { self.with_compositor(|compositor| compositor.commit_frame()) } + /// Tries to apply a new [`Mode`] for this `DrmOutput`. + /// + /// Fails if the mode is not compatible with the underlying + /// [`crtc`] or any of the pending [`connector`]s. + /// + /// This might cause commits on other surfaces to meet the bandwidth + /// requirements of the new mode by temporarily disabling additional planes + /// and forcing composition. pub fn use_mode( &mut self, mode: Mode, @@ -562,10 +689,12 @@ where ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, G: AsFd + 'static, { + /// Returns the underlying [`crtc`] of this surface pub fn crtc(&self) -> crtc::Handle { self.crtc } + /// Provides exclusive access to the underlying [`DrmCompositor`] pub fn with_compositor(&self, f: T) -> R where T: FnOnce(&mut DrmCompositor) -> R, @@ -631,17 +760,36 @@ where if res.is_err() { for compositor in write_guard.values_mut() { let mut compositor = compositor.lock().unwrap(); - render_elements.submit_composited_frame(&mut compositor, renderer)?; + if let Err(err) = render_elements.submit_composited_frame(&mut *compositor, renderer) { + if !matches!(err, DrmOutputManagerError::Frame(FrameError::EmptyFrame)) { + return Err(err); + } + } } let compositor = write_guard.get_mut(crtc).unwrap(); let mut compositor = compositor.lock().unwrap(); res = compositor.use_mode(mode); + + if res.is_ok() { + if let Err(err) = render_elements.submit_composited_frame(&mut *compositor, renderer) { + if !matches!(err, DrmOutputManagerError::Frame(FrameError::EmptyFrame)) { + return Err(err); + } + } + } } res.map_err(DrmOutputManagerError::Frame) } +/// Set of render elements for a set of outputs managed by an [`DrmOutputManager`]. +/// +/// A few methods of the [`DrmOutputManager`] and [`DrmOutput`] might need to do +/// commits to multiple surfaces to satisfy bandwidth constraints. To not render +/// a black screen this struct can be populated with screen contents to be used +/// when such an operation is required. Outputs not provided via +/// [`DrmOutputRenderElements::add_output`] will fallback to black. #[derive(Debug)] pub struct DrmOutputRenderElements where @@ -676,6 +824,10 @@ where ::TextureId: Texture + 'static, ::Error: Send + Sync + 'static, { + /// Adds elements to be used when rendering for a given `crtc`. + /// + /// Outputs not provided via this will fallback to black when + /// this struct is passed to any consuming function. pub fn add_output( &mut self, crtc: &crtc::Handle, From 86e359a8380e2f615d5301587003447f830a6ba2 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Tue, 10 Dec 2024 18:46:06 +0100 Subject: [PATCH 16/24] chore: Address feature requirement mismatches --- src/backend/drm/compositor/mod.rs | 4 ++-- src/backend/drm/exporter/gbm.rs | 4 +++- src/backend/drm/exporter/mod.rs | 2 ++ src/backend/drm/mod.rs | 1 + src/backend/renderer/test.rs | 12 ++++++++++-- 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index 1d14a1ad967f..aef093b102bd 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -141,7 +141,7 @@ use wayland_server::{protocol::wl_buffer::WlBuffer, Resource}; #[cfg(feature = "renderer_pixman")] use crate::backend::renderer::{ pixman::{PixmanError, PixmanRenderBuffer, PixmanRenderer, PixmanTexture}, - ImportAll, Unbind, + Frame as _, ImportAll, Unbind, }; use crate::{ backend::{ @@ -161,7 +161,7 @@ use crate::{ }, sync::SyncPoint, utils::{CommitCounter, DamageBag}, - Bind, Color32F, DebugFlags, Frame as RendererFrame, Renderer, Texture, + Bind, Color32F, DebugFlags, Renderer, Texture, }, SwapBuffersError, }, diff --git a/src/backend/drm/exporter/gbm.rs b/src/backend/drm/exporter/gbm.rs index f10082318e35..d1a9c386c207 100644 --- a/src/backend/drm/exporter/gbm.rs +++ b/src/backend/drm/exporter/gbm.rs @@ -3,10 +3,12 @@ use std::os::unix::io::AsFd; use super::{ExportBuffer, ExportFramebuffer}; +#[cfg(feature = "wayland_frontend")] +use crate::backend::drm::gbm::framebuffer_from_wayland_buffer; use crate::backend::{ allocator::gbm::GbmBuffer, drm::{ - gbm::{framebuffer_from_bo, framebuffer_from_wayland_buffer, Error, GbmFramebuffer}, + gbm::{framebuffer_from_bo, Error, GbmFramebuffer}, DrmDeviceFd, }, }; diff --git a/src/backend/drm/exporter/mod.rs b/src/backend/drm/exporter/mod.rs index 303e13799c04..1616b68c423d 100644 --- a/src/backend/drm/exporter/mod.rs +++ b/src/backend/drm/exporter/mod.rs @@ -6,6 +6,7 @@ use std::{ sync::{Arc, Mutex}, }; +#[cfg(feature = "wayland_frontend")] use wayland_server::protocol::wl_buffer::WlBuffer; use crate::backend::{allocator::Buffer, renderer::element::UnderlyingStorage}; @@ -21,6 +22,7 @@ pub mod gbm; #[derive(Debug)] pub enum ExportBuffer<'a, B: Buffer> { /// A wayland buffer + #[cfg(feature = "wayland_frontend")] Wayland(&'a WlBuffer), /// A [`Allocator`] buffer Allocator(&'a B), diff --git a/src/backend/drm/mod.rs b/src/backend/drm/mod.rs index 6c2def70aaeb..8dbbacb16681 100644 --- a/src/backend/drm/mod.rs +++ b/src/backend/drm/mod.rs @@ -79,6 +79,7 @@ mod error; pub mod exporter; #[cfg(feature = "backend_gbm")] pub mod gbm; +#[cfg(all(feature = "wayland_frontend", feature = "backend_gbm"))] pub mod output; mod surface; diff --git a/src/backend/renderer/test.rs b/src/backend/renderer/test.rs index f603eb5977c3..c0a28471b743 100644 --- a/src/backend/renderer/test.rs +++ b/src/backend/renderer/test.rs @@ -2,7 +2,11 @@ #[cfg(feature = "wayland_frontend")] use std::cell::Cell; -#[cfg(all(feature = "wayland_frontend", feature = "use_system_lib"))] +#[cfg(all( + feature = "wayland_frontend", + feature = "use_system_lib", + feature = "backend_egl" +))] use crate::backend::renderer::ImportEgl; #[cfg(feature = "wayland_frontend")] use crate::{ @@ -168,7 +172,11 @@ impl ImportDma for DummyRenderer { } } -#[cfg(all(feature = "wayland_frontend", feature = "use_system_lib"))] +#[cfg(all( + feature = "wayland_frontend", + feature = "use_system_lib", + feature = "backend_egl" +))] impl ImportEgl for DummyRenderer { fn bind_wl_display( &mut self, From d9ce00439de7f4f77b1c16e84bf76dc91774445d Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Tue, 10 Dec 2024 18:46:16 +0100 Subject: [PATCH 17/24] chore: Fix clippy issues --- anvil/src/udev.rs | 11 +- src/backend/drm/compositor/frame_result.rs | 12 +-- src/backend/drm/compositor/mod.rs | 12 +-- src/backend/drm/output.rs | 112 +++++++-------------- 4 files changed, 54 insertions(+), 93 deletions(-) diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index 1ff452dd5709..cc74e09cbadb 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -430,7 +430,7 @@ pub fn run_udev() { primary_gpu, surface_data.render_node, gpus, - &compositor.surface(), + compositor.surface(), ) }) }); @@ -1512,10 +1512,11 @@ fn render_surface<'a>( let (elements, clear_color) = output_elements(output, space, custom_elements, renderer, show_window_preview); - let frame_mode = surface - .disable_direct_scanout - .then_some(FrameMode::COMPOSITE) - .unwrap_or(FrameMode::ALL); + let frame_mode = if surface.disable_direct_scanout { + FrameMode::COMPOSITE + } else { + FrameMode::ALL + }; let (rendered, states) = surface .drm_output .render_frame(renderer, &elements, clear_color, frame_mode) diff --git a/src/backend/drm/compositor/frame_result.rs b/src/backend/drm/compositor/frame_result.rs index 9816258b0b3f..2edfa46df19d 100644 --- a/src/backend/drm/compositor/frame_result.rs +++ b/src/backend/drm/compositor/frame_result.rs @@ -55,7 +55,7 @@ pub struct RenderFrameResult<'a, B: Buffer, F: Framebuffer, E> { pub(super) supports_fencing: bool, } -impl<'a, B: Buffer, F: Framebuffer, E> RenderFrameResult<'a, B, F, E> { +impl RenderFrameResult<'_, B, F, E> { /// Returns if synchronization with kms submission can't be guaranteed through the available apis. pub fn needs_sync(&self) -> bool { if let PrimaryPlaneElement::Swapchain(ref element) = self.primary_element { @@ -73,7 +73,7 @@ struct SwapchainElement<'a, 'b, B: Buffer> { damage: &'b DamageSnapshot, } -impl<'a, 'b, B: Buffer> Element for SwapchainElement<'a, 'b, B> { +impl Element for SwapchainElement<'_, '_, B> { fn id(&self) -> &Id { &self.id } @@ -118,7 +118,7 @@ enum FrameResultDamageElement<'a, 'b, E, B: Buffer> { Swapchain(SwapchainElement<'a, 'b, B>), } -impl<'a, 'b, E, B> Element for FrameResultDamageElement<'a, 'b, E, B> +impl Element for FrameResultDamageElement<'_, '_, E, B> where E: Element, B: Buffer, @@ -201,7 +201,7 @@ pub enum BlitFrameResultError { Export(E), } -impl<'a, B, F, E> RenderFrameResult<'a, B, F, E> +impl RenderFrameResult<'_, B, F, E> where B: Buffer, F: Framebuffer, @@ -413,8 +413,8 @@ where } } -impl<'a, B: Buffer + std::fmt::Debug, F: Framebuffer + std::fmt::Debug, E: std::fmt::Debug> std::fmt::Debug - for RenderFrameResult<'a, B, F, E> +impl std::fmt::Debug + for RenderFrameResult<'_, B, F, E> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("RenderFrameResult") diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index aef093b102bd..a4744a02f987 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -1288,6 +1288,7 @@ where /// - `modifiers` is the set of modifiers allowed, when allocating buffers with the specified color format /// - `cursor_size` as reported by the drm device, used for creating buffer for the cursor plane /// - `gbm` device used for creating buffers for the cursor plane, `None` will disable the cursor plane + #[allow(clippy::too_many_arguments)] pub fn with_format( output_mode_source: impl Into + Debug, surface: DrmSurface, @@ -1514,10 +1515,9 @@ where Ok(None) => return Err((swapchain.allocator, FrameError::NoFramebuffer)), Err(err) => return Err((swapchain.allocator, FrameError::FramebufferExport(err))), }; - buffer.userdata().insert_if_missing(|| { - let init = CachedDrmFramebuffer::new(DrmFramebuffer::Exporter(fb_buffer)); - init - }); + buffer + .userdata() + .insert_if_missing(|| CachedDrmFramebuffer::new(DrmFramebuffer::Exporter(fb_buffer))); let mode = drm.pending_mode(); let handle = buffer @@ -1559,7 +1559,7 @@ where }), }; - match current_frame_state.test_state(&drm, supports_fencing, drm.plane(), plane_state, true) { + match current_frame_state.test_state(drm, supports_fencing, drm.plane(), plane_state, true) { Ok(_) => Ok((swapchain, use_opaque)), Err(err) => { warn!( @@ -1652,7 +1652,7 @@ where let modifiers = formats.iter().map(|x| x.modifier).collect::>(); let (swapchain, use_opaque) = Self::test_format( - &*drm, + &drm, supports_fencing, planes, allocator, diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs index f5ea23744ff8..1ebc11b87ff2 100644 --- a/src/backend/drm/output.rs +++ b/src/backend/drm/output.rs @@ -135,6 +135,17 @@ where RenderFrame(RenderFrameError), } +/// Result returned by `DrmOutputManager`'s methods +pub type DrmOutputManagerResult = Result< + U, + DrmOutputManagerError< + ::Error, + <::Buffer as AsDmabuf>::Error, + ::Buffer>>::Error, + ::Error, + >, +>; + impl DrmOutputManager where A: Allocator + std::clone::Clone + std::fmt::Debug, @@ -172,7 +183,7 @@ where ) -> Self { Self { device, - allocator: allocator, + allocator, exporter, gbm, compositor: Default::default(), @@ -199,8 +210,9 @@ where /// `None` will result in the compositor to use all planes as specified by [`DrmSurface::planes`] /// - `renderer` used for compositing, when commits are necessarily to realize bandwidth constraints /// - `render_elements` used for rendering, when commits are necessarily to realize bandwidth constraints - pub fn initialize_output<'a, R, E>( - &'a mut self, + #[allow(clippy::too_many_arguments)] + pub fn initialize_output( + &mut self, crtc: crtc::Handle, mode: control::Mode, connectors: &[connector::Handle], @@ -208,15 +220,7 @@ where planes: Option, renderer: &mut R, render_elements: &DrmOutputRenderElements, - ) -> Result< - DrmOutput, - DrmOutputManagerError< - ::Error, - <::Buffer as AsDmabuf>::Error, - ::Buffer>>::Error, - R::Error, - >, - > + ) -> DrmOutputManagerResult, A, F, R> where E: RenderElement, R: Renderer + Bind, @@ -302,7 +306,7 @@ where ); for compositor in write_guard.values_mut() { - let mut compositor = compositor.lock().unwrap(); + let compositor = compositor.get_mut().unwrap(); if let Err(err) = render_elements.submit_composited_frame(&mut *compositor, renderer) { if !matches!(err, DrmOutputManagerError::Frame(FrameError::EmptyFrame)) { return Err(err); @@ -324,7 +328,7 @@ where ); for compositor in write_guard.values_mut() { - let mut compositor = compositor.lock().unwrap(); + let compositor = compositor.get_mut().unwrap(); let current_format = compositor.format(); if let Err(err) = compositor.set_format( @@ -348,7 +352,7 @@ where Err(err) => { // try to reset formats for compositor in write_guard.values_mut() { - let mut compositor = compositor.lock().unwrap(); + let compositor = compositor.get_mut().unwrap(); let current_format = compositor.format(); if let Err(err) = compositor.set_format( @@ -378,7 +382,7 @@ where // when downstream potentially uses `FrameMode::ALL` immediately after this. let compositor = write_guard.get_mut(&crtc).unwrap(); - let mut compositor = compositor.lock().unwrap(); + let compositor = compositor.get_mut().unwrap(); render_elements.submit_composited_frame(&mut *compositor, renderer)?; Ok(DrmOutput { @@ -410,15 +414,7 @@ where mode: Mode, renderer: &mut R, render_elements: &DrmOutputRenderElements, - ) -> Result< - (), - DrmOutputManagerError< - ::Error, - <::Buffer as AsDmabuf>::Error, - ::Buffer>>::Error, - R::Error, - >, - > + ) -> DrmOutputManagerResult<(), A, F, R> where E: RenderElement, R: Renderer + Bind, @@ -439,15 +435,7 @@ where &mut self, renderer: &mut R, render_elements: &DrmOutputRenderElements, - ) -> Result< - (), - DrmOutputManagerError< - ::Error, - <::Buffer as AsDmabuf>::Error, - ::Buffer>>::Error, - R::Error, - >, - > + ) -> DrmOutputManagerResult<(), A, F, R> where E: RenderElement, R: Renderer + Bind, @@ -459,11 +447,11 @@ where // check if implicit modifiers are in use if write_guard .values_mut() - .any(|c| c.lock().unwrap().modifiers() == &[DrmModifier::Invalid]) + .any(|c| c.get_mut().unwrap().modifiers() == [DrmModifier::Invalid]) { // if so, first lower the bandwidth by disabling planes on all compositors for compositor in write_guard.values_mut() { - let mut compositor = compositor.lock().unwrap(); + let compositor = compositor.get_mut().unwrap(); if let Err(err) = render_elements.submit_composited_frame(&mut *compositor, renderer) { if !matches!(err, DrmOutputManagerError::Frame(FrameError::EmptyFrame)) { return Err(err); @@ -472,8 +460,8 @@ where } for compositor in write_guard.values_mut() { - let mut compositor = compositor.lock().unwrap(); - if compositor.modifiers() != &[DrmModifier::Invalid] { + let compositor = compositor.get_mut().unwrap(); + if compositor.modifiers() != [DrmModifier::Invalid] { continue; } @@ -507,9 +495,9 @@ where self.device.activate(disable_connectors)?; // We request a write guard here to guarantee unique access - let write_guard = self.compositor.write().unwrap(); - for compositor in write_guard.values() { - if let Err(err) = compositor.lock().unwrap().reset_state() { + let mut write_guard = self.compositor.write().unwrap(); + for compositor in write_guard.values_mut() { + if let Err(err) = compositor.get_mut().unwrap().reset_state() { tracing::warn!("Failed to reset drm surface state: {}", err); } } @@ -645,7 +633,6 @@ where /// /// *Note*: This function should not be followed up with [`DrmOutput::frame_submitted`] /// and will not generate a vblank event on the underlying device. - pub fn commit_frame(&mut self) -> FrameResult<(), A, F> { self.with_compositor(|compositor| compositor.commit_frame()) } @@ -663,15 +650,7 @@ where mode: Mode, renderer: &mut R, render_elements: &DrmOutputRenderElements, - ) -> Result< - (), - DrmOutputManagerError< - ::Error, - <::Buffer as AsDmabuf>::Error, - ::Buffer>>::Error, - R::Error, - >, - > + ) -> DrmOutputManagerResult<(), A, F, R> where E: RenderElement, R: Renderer + Bind, @@ -724,15 +703,7 @@ fn use_mode_internal( mode: Mode, renderer: &mut R, render_elements: &DrmOutputRenderElements, -) -> Result< - (), - DrmOutputManagerError< - ::Error, - <::Buffer as AsDmabuf>::Error, - ::Buffer>>::Error, - R::Error, - >, -> +) -> DrmOutputManagerResult<(), A, F, R> where A: Allocator + std::clone::Clone + fmt::Debug, ::Buffer: AsDmabuf, @@ -752,14 +723,11 @@ where { let mut write_guard = compositor.write().unwrap(); - let mut res = { - let mut compositor_guard = write_guard.get(crtc).unwrap().lock().unwrap(); - compositor_guard.use_mode(mode) - }; + let mut res = write_guard.get(crtc).unwrap().lock().unwrap().use_mode(mode); if res.is_err() { for compositor in write_guard.values_mut() { - let mut compositor = compositor.lock().unwrap(); + let compositor = compositor.get_mut().unwrap(); if let Err(err) = render_elements.submit_composited_frame(&mut *compositor, renderer) { if !matches!(err, DrmOutputManagerError::Frame(FrameError::EmptyFrame)) { return Err(err); @@ -768,7 +736,7 @@ where } let compositor = write_guard.get_mut(crtc).unwrap(); - let mut compositor = compositor.lock().unwrap(); + let compositor = compositor.get_mut().unwrap(); res = compositor.use_mode(mode); if res.is_ok() { @@ -842,15 +810,7 @@ where &self, compositor: &mut DrmCompositor, renderer: &mut R, - ) -> Result< - (), - DrmOutputManagerError< - ::Error, - <::Buffer as AsDmabuf>::Error, - ::Buffer>>::Error, - R::Error, - >, - > + ) -> DrmOutputManagerResult<(), A, F, R> where A: Allocator + std::clone::Clone + std::fmt::Debug, ::Buffer: AsDmabuf, @@ -870,7 +830,7 @@ where .map(|(ref elements, ref color)| (&**elements, color)) .unwrap_or((&[], &Color32F::BLACK)); compositor - .render_frame(renderer, &elements, *clear_color, FrameMode::COMPOSITE) + .render_frame(renderer, elements, *clear_color, FrameMode::COMPOSITE) .map_err(DrmOutputManagerError::RenderFrame)?; compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; Ok(()) From 9be0da4764e24670e6cbe751831aacc27aea061d Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Tue, 10 Dec 2024 19:02:32 +0100 Subject: [PATCH 18/24] drm/compositor: Fix test --- src/backend/drm/compositor/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index a4744a02f987..6dfb3296e3fc 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -81,7 +81,7 @@ //! # let surface: DrmSurface = todo!(); //! # let allocator: GbmAllocator = todo!(); //! # let exporter: GbmDevice = todo!(); -//! # let color_formats = &[DrmFourcc::Argb8888]; +//! # let color_formats = [DrmFourcc::Argb8888]; //! # let renderer_formats = HashSet::from([DrmFormat { //! # code: DrmFourcc::Argb8888, //! # modifier: DrmModifier::Linear, From ceaa97ee74f6727cae5f8f2a3bc2a555c17871bb Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Tue, 10 Dec 2024 19:30:21 +0100 Subject: [PATCH 19/24] msrv: Bump to 1.80.1 --- .github/workflows/ci.yml | 2 +- Cargo.toml | 2 +- anvil/src/udev.rs | 4 +--- clippy.toml | 2 +- examples/buffer_test.rs | 8 ++------ src/backend/renderer/gles/mod.rs | 14 +++++++------- src/backend/renderer/gles/shaders/mod.rs | 22 +++++++++++----------- src/backend/vulkan/mod.rs | 5 ++--- src/desktop/mod.rs | 2 +- src/input/keyboard/keymap_file.rs | 6 +++--- src/wayland/dmabuf/mod.rs | 3 +-- src/wayland/pointer_constraints.rs | 5 ++--- src/wayland/shm/pool.rs | 8 +++----- 13 files changed, 36 insertions(+), 47 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d11a862d1e12..0c21b535ef7f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,7 +83,7 @@ jobs: - name: Checkout sources uses: actions/checkout@v3 - name: Rust toolchain - uses: dtolnay/rust-toolchain@1.72.0 + uses: dtolnay/rust-toolchain@1.80.1 - name: Get date for registry cache id: date run: echo "::set-output name=date::$(date +'%Y-%m-%d')" diff --git a/Cargo.toml b/Cargo.toml index 4c6da0e4d75e..09842a62fee3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ readme = "README.md" homepage = "https://smithay.github.io/" keywords = ["wayland", "compositor", "graphics", "server"] categories = ["gui"] -rust-version = "1.72.0" +rust-version = "1.80.1" [package.metadata.docs.rs] features = ["test_all_features"] diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index cc74e09cbadb..509fae823b7b 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -771,10 +771,8 @@ impl AnvilState { render_node, surfaces: HashMap::new(), leasing_global: DrmLeaseState::new::>(&self.display_handle, &node) - .map_err(|err| { - // TODO replace with inspect_err, once stable + .inspect_err(|err| { warn!(?err, "Failed to initialize drm lease global for: {}", node); - err }) .ok(), active_leases: Vec::new(), diff --git a/clippy.toml b/clippy.toml index 7904b9c642d3..d5b3a37f2da2 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1,4 +1,4 @@ -msrv = "1.72.0" +msrv = "1.80.1" type-complexity-threshold = 400 disallowed-macros = [ diff --git a/examples/buffer_test.rs b/examples/buffer_test.rs index 32f7896e186a..671d3f8d2c4e 100644 --- a/examples/buffer_test.rs +++ b/examples/buffer_test.rs @@ -1,4 +1,4 @@ -use std::{ffi::CStr, fmt, fs::File, os::unix::io::OwnedFd, path::PathBuf}; +use std::{fmt, fs::File, os::unix::io::OwnedFd, path::PathBuf}; use clap::{Args, Parser, Subcommand, ValueEnum}; use smithay::{ @@ -189,11 +189,7 @@ fn buffer_test(args: TestArgs) { Instance::new(Version::VERSION_1_2, None).expect("Unable to create vulkan instance"); let physical_device = PhysicalDevice::enumerate(&instance) .expect("Failed to enumerate physical devices") - .filter(|phd| { - phd.has_device_extension(unsafe { - CStr::from_bytes_with_nul_unchecked(b"VK_EXT_physical_device_drm\0") - }) - }) + .filter(|phd| phd.has_device_extension(c"VK_EXT_physical_device_drm")) .find(|phd| { phd.primary_node().unwrap() == Some(node) || phd.render_node().unwrap() == Some(node) }) diff --git a/src/backend/renderer/gles/mod.rs b/src/backend/renderer/gles/mod.rs index a59dd3cd1525..7e7cb38eefc5 100644 --- a/src/backend/renderer/gles/mod.rs +++ b/src/backend/renderer/gles/mod.rs @@ -1847,13 +1847,13 @@ impl GlesRenderer { let debug_shader = format!("#version 100\n#define {}\n{}", shaders::DEBUG_FLAGS, src.as_ref()); let debug_program = unsafe { link_program(&self.gl, shaders::VERTEX_SHADER, &debug_shader)? }; - let vert = CStr::from_bytes_with_nul(b"vert\0").expect("NULL terminated"); - let vert_position = CStr::from_bytes_with_nul(b"vert_position\0").expect("NULL terminated"); - let matrix = CStr::from_bytes_with_nul(b"matrix\0").expect("NULL terminated"); - let tex_matrix = CStr::from_bytes_with_nul(b"tex_matrix\0").expect("NULL terminated"); - let size = CStr::from_bytes_with_nul(b"size\0").expect("NULL terminated"); - let alpha = CStr::from_bytes_with_nul(b"alpha\0").expect("NULL terminated"); - let tint = CStr::from_bytes_with_nul(b"tint\0").expect("NULL terminated"); + let vert = c"vert"; + let vert_position = c"vert_position"; + let matrix = c"matrix"; + let tex_matrix = c"tex_matrix"; + let size = c"size"; + let alpha = c"alpha"; + let tint = c"tint"; unsafe { Ok(GlesPixelProgram(Rc::new(GlesPixelProgramInner { diff --git a/src/backend/renderer/gles/shaders/mod.rs b/src/backend/renderer/gles/shaders/mod.rs index 148b6e16eacb..6125f730dc23 100644 --- a/src/backend/renderer/gles/shaders/mod.rs +++ b/src/backend/renderer/gles/shaders/mod.rs @@ -142,13 +142,13 @@ pub(super) unsafe fn texture_program( let program = unsafe { link_program(gl, shaders::VERTEX_SHADER, &shader)? }; let debug_program = unsafe { link_program(gl, shaders::VERTEX_SHADER, debug_shader.as_ref())? }; - let vert = CStr::from_bytes_with_nul(b"vert\0").expect("NULL terminated"); - let vert_position = CStr::from_bytes_with_nul(b"vert_position\0").expect("NULL terminated"); - let tex = CStr::from_bytes_with_nul(b"tex\0").expect("NULL terminated"); - let matrix = CStr::from_bytes_with_nul(b"matrix\0").expect("NULL terminated"); - let tex_matrix = CStr::from_bytes_with_nul(b"tex_matrix\0").expect("NULL terminated"); - let alpha = CStr::from_bytes_with_nul(b"alpha\0").expect("NULL terminated"); - let tint = CStr::from_bytes_with_nul(b"tint\0").expect("NULL terminated"); + let vert = c"vert"; + let vert_position = c"vert_position"; + let tex = c"tex"; + let matrix = c"matrix"; + let tex_matrix = c"tex_matrix"; + let alpha = c"alpha"; + let tint = c"tint"; Ok(GlesTexProgramVariant { normal: GlesTexProgramInternal { @@ -223,10 +223,10 @@ pub(super) unsafe fn texture_program( pub(super) unsafe fn solid_program(gl: &ffi::Gles2) -> Result { let program = link_program(gl, shaders::VERTEX_SHADER_SOLID, shaders::FRAGMENT_SHADER_SOLID)?; - let matrix = CStr::from_bytes_with_nul(b"matrix\0").expect("NULL terminated"); - let color = CStr::from_bytes_with_nul(b"color\0").expect("NULL terminated"); - let vert = CStr::from_bytes_with_nul(b"vert\0").expect("NULL terminated"); - let position = CStr::from_bytes_with_nul(b"position\0").expect("NULL terminated"); + let matrix = c"matrix"; + let color = c"color"; + let vert = c"vert"; + let position = c"position"; Ok(GlesSolidProgram { program, diff --git a/src/backend/vulkan/mod.rs b/src/backend/vulkan/mod.rs index 889f4a5e6000..0298df62f6ca 100644 --- a/src/backend/vulkan/mod.rs +++ b/src/backend/vulkan/mod.rs @@ -219,8 +219,7 @@ impl Instance { // Enable debug layers if present and debug assertions are enabled. if cfg!(debug_assertions) { - const VALIDATION: &CStr = - unsafe { CStr::from_bytes_with_nul_unchecked(b"VK_LAYER_KHRONOS_validation\0") }; + const VALIDATION: &CStr = c"VK_LAYER_KHRONOS_validation"; if available_layers .iter() @@ -257,7 +256,7 @@ impl Instance { let mut app_info = vk::ApplicationInfo::default() .api_version(api_version.to_raw()) // SAFETY: null terminated with no interior null bytes. - .engine_name(unsafe { CStr::from_bytes_with_nul_unchecked(b"Smithay\0") }) + .engine_name(c"Smithay") .engine_version(Version::SMITHAY.to_raw()); if let Some(app_version) = app_version { diff --git a/src/desktop/mod.rs b/src/desktop/mod.rs index 3c66bbdcf62a..efd73c219539 100644 --- a/src/desktop/mod.rs +++ b/src/desktop/mod.rs @@ -14,7 +14,7 @@ //! //! A window represents what is typically understood by the end-user as a single application window. //! -//! Currently it abstracts over xdg-shell toplevels and Xwayland surfaces (TODO). +//! Currently it abstracts over xdg-shell toplevels and Xwayland surfaces. //! It provides a bunch of methods to calculate and retrieve its size, manage itself, attach additional user_data //! as well as a [drawing function](`crate::backend::renderer::element::AsRenderElements::render_elements`) to ease rendering it's related surfaces. //! diff --git a/src/input/keyboard/keymap_file.rs b/src/input/keyboard/keymap_file.rs index 596175149656..6568c7c47636 100644 --- a/src/input/keyboard/keymap_file.rs +++ b/src/input/keyboard/keymap_file.rs @@ -1,4 +1,4 @@ -use std::ffi::{CStr, CString}; +use std::ffi::CString; use std::os::unix::io::{AsFd, BorrowedFd}; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -21,7 +21,7 @@ pub struct KeymapFile { impl KeymapFile { /// Turn the keymap into a string using KEYMAP_FORMAT_TEXT_V1, create a sealed file for it, and store the string pub fn new(keymap: &Keymap) -> Self { - let name = CStr::from_bytes_with_nul(b"smithay-keymap\0").unwrap(); + let name = c"smithay-keymap"; let keymap = keymap.get_as_string(KEYMAP_FORMAT_TEXT_V1); let sealed = SealedFile::with_content(name, &CString::new(keymap.as_str()).unwrap()); @@ -42,7 +42,7 @@ impl KeymapFile { pub(crate) fn change_keymap(&mut self, keymap: &Keymap) { let keymap = keymap.get_as_string(xkb::KEYMAP_FORMAT_TEXT_V1); - let name = CStr::from_bytes_with_nul(b"smithay-keymap-file\0").unwrap(); + let name = c"smithay-keymap-file"; let sealed = SealedFile::with_content(name, &CString::new(keymap.clone()).unwrap()); if let Err(err) = sealed.as_ref() { diff --git a/src/wayland/dmabuf/mod.rs b/src/wayland/dmabuf/mod.rs index 9270c96e3299..2a159621e3b8 100644 --- a/src/wayland/dmabuf/mod.rs +++ b/src/wayland/dmabuf/mod.rs @@ -189,7 +189,6 @@ mod dispatch; use std::{ collections::HashMap, - ffi::CStr, ops::Sub, os::unix::io::AsFd, sync::{ @@ -397,7 +396,7 @@ impl DmabufFeedbackBuilder { .flat_map(DmabufFeedbackFormat::to_ne_bytes) .collect::>(); - let name = CStr::from_bytes_with_nul(b"smithay-dmabuffeedback-format-table\0").unwrap(); + let name = c"smithay-dmabuffeedback-format-table"; let format_table_file = SealedFile::with_data(name, &formats)?; // remove all formats from the main tranche that are already covered diff --git a/src/wayland/pointer_constraints.rs b/src/wayland/pointer_constraints.rs index 51c6b1176526..59c1bab4cf84 100644 --- a/src/wayland/pointer_constraints.rs +++ b/src/wayland/pointer_constraints.rs @@ -192,9 +192,8 @@ impl PointerConstraint { } Self::Locked(locked) => { locked.region.clone_from(&locked.pending_region); - locked.pending_cursor_position_hint.take().map(|hint| { - locked.cursor_position_hint = Some(hint); - hint + locked.pending_cursor_position_hint.take().inspect(|hint| { + locked.cursor_position_hint = Some(*hint); }) } } diff --git a/src/wayland/shm/pool.rs b/src/wayland/shm/pool.rs index eed8059d24e1..d3ca7524afe2 100644 --- a/src/wayland/shm/pool.rs +++ b/src/wayland/shm/pool.rs @@ -7,7 +7,7 @@ use std::{ os::unix::io::{AsFd, BorrowedFd, OwnedFd}, ptr, sync::{ - mpsc::{sync_channel, SyncSender}, + mpsc::{channel, Sender}, OnceLock, RwLock, }, thread, @@ -28,10 +28,8 @@ use tracing::{debug, instrument, trace}; // // To work around this problem, we spawn a separate thread whose sole purpose is dropping stuff we // send it through a channel. Conveniently, Pool is already Send, so there's no problem doing this. -// -// We use SyncSender because the regular Sender only got Sync in 1.72 which is above our MSRV. -static DROP_THIS: Lazy> = Lazy::new(|| { - let (tx, rx) = sync_channel(16); +static DROP_THIS: Lazy> = Lazy::new(|| { + let (tx, rx) = channel(); thread::Builder::new() .name("Shm dropping thread".to_owned()) .spawn(move || { From afa1117c3e7668a3894cb9539c6c7e03e5418266 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Tue, 10 Dec 2024 19:30:51 +0100 Subject: [PATCH 20/24] anvil: Don't use type alias for min-ver of `image` --- anvil/Cargo.toml | 2 +- anvil/src/udev.rs | 3 ++- anvil/src/winit.rs | 3 ++- anvil/src/x11.rs | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/anvil/Cargo.toml b/anvil/Cargo.toml index 249fb547d739..528e7a034391 100644 --- a/anvil/Cargo.toml +++ b/anvil/Cargo.toml @@ -9,7 +9,7 @@ version = "0.0.1" [dependencies] bitflags = "2.2.1" fps_ticker = {version = "1.0.0", optional = true} -image = {version = "0.25.1", default-features = false, optional = true} +image = {version = "0.25.1", default-features = false, optional = true, features = ["png"]} rand = "0.8" tracing = { version = "0.1.37", features = ["max_level_trace", "release_max_level_debug"] } tracing-subscriber = { version = "0.3.16", features = ["env-filter"] } diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index 509fae823b7b..0891dac8455a 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -380,8 +380,9 @@ pub fn run_udev() { #[cfg(feature = "debug")] { + #[allow(deprecated)] let fps_image = - image::ImageReader::with_format(std::io::Cursor::new(FPS_NUMBERS_PNG), image::ImageFormat::Png) + image::io::Reader::with_format(std::io::Cursor::new(FPS_NUMBERS_PNG), image::ImageFormat::Png) .decode() .unwrap(); let fps_texture = renderer diff --git a/anvil/src/winit.rs b/anvil/src/winit.rs index f2ca111966ed..66fb0caaf628 100644 --- a/anvil/src/winit.rs +++ b/anvil/src/winit.rs @@ -126,8 +126,9 @@ pub fn run_winit() { output.set_preferred(mode); #[cfg(feature = "debug")] + #[allow(deprecated)] let fps_image = - image::ImageReader::with_format(std::io::Cursor::new(FPS_NUMBERS_PNG), image::ImageFormat::Png) + image::io::Reader::with_format(std::io::Cursor::new(FPS_NUMBERS_PNG), image::ImageFormat::Png) .decode() .unwrap(); #[cfg(feature = "debug")] diff --git a/anvil/src/x11.rs b/anvil/src/x11.rs index 50e3ce1c92aa..88803754f820 100644 --- a/anvil/src/x11.rs +++ b/anvil/src/x11.rs @@ -205,8 +205,9 @@ pub fn run_x11() { }; #[cfg(feature = "debug")] + #[allow(deprecated)] let fps_image = - image::ImageReader::with_format(std::io::Cursor::new(FPS_NUMBERS_PNG), image::ImageFormat::Png) + image::io::Reader::with_format(std::io::Cursor::new(FPS_NUMBERS_PNG), image::ImageFormat::Png) .decode() .unwrap(); #[cfg(feature = "debug")] From 243fe8818952821da948e17904ceeb547ea758c0 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Fri, 13 Dec 2024 15:21:52 +0100 Subject: [PATCH 21/24] drm/output: Also allow modifier fallback for `use_mode` --- src/backend/drm/output.rs | 100 ++++++++++++++++++++++++++++++++------ 1 file changed, 85 insertions(+), 15 deletions(-) diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs index 1ebc11b87ff2..790836f1b592 100644 --- a/src/backend/drm/output.rs +++ b/src/backend/drm/output.rs @@ -386,8 +386,10 @@ where render_elements.submit_composited_frame(&mut *compositor, renderer)?; Ok(DrmOutput { - crtc, compositor: self.compositor.clone(), + crtc, + allocator: self.allocator.clone(), + renderer_formats: self.renderer_formats.clone(), }) } @@ -421,7 +423,15 @@ where ::TextureId: Texture + 'static, ::Error: Send + Sync + 'static, { - use_mode_internal(&self.compositor, crtc, mode, renderer, render_elements) + use_mode_internal( + &self.compositor, + crtc, + mode, + &self.allocator, + &self.renderer_formats, + renderer, + render_elements, + ) } /// Tries to restore explicit modifiers on all surfaces. @@ -514,8 +524,10 @@ where ::Buffer>>::Framebuffer: std::fmt::Debug + 'static, G: AsFd + 'static, { - crtc: crtc::Handle, compositor: CompositorList, + crtc: crtc::Handle, + allocator: A, + renderer_formats: Vec, } impl fmt::Debug for DrmOutput @@ -657,7 +669,15 @@ where ::TextureId: Texture + 'static, ::Error: Send + Sync + 'static, { - use_mode_internal(&self.compositor, &self.crtc, mode, renderer, render_elements) + use_mode_internal( + &self.compositor, + &self.crtc, + mode, + &self.allocator, + &self.renderer_formats, + renderer, + render_elements, + ) } } @@ -701,6 +721,8 @@ fn use_mode_internal( compositor: &CompositorList, crtc: &crtc::Handle, mode: Mode, + allocator: &A, + renderer_formats: &[DrmFormat], renderer: &mut R, render_elements: &DrmOutputRenderElements, ) -> DrmOutputManagerResult<(), A, F, R> @@ -723,9 +745,11 @@ where { let mut write_guard = compositor.write().unwrap(); - let mut res = write_guard.get(crtc).unwrap().lock().unwrap().use_mode(mode); + let res = write_guard.get(crtc).unwrap().lock().unwrap().use_mode(mode); + + if let Err(err @ FrameError::DrmError(DrmError::TestFailed(_))) = res.as_ref() { + tracing::warn!(?crtc, ?err, "failed to set mode, trying to lower bandwidth usage"); - if res.is_err() { for compositor in write_guard.values_mut() { let compositor = compositor.get_mut().unwrap(); if let Err(err) = render_elements.submit_composited_frame(&mut *compositor, renderer) { @@ -735,20 +759,66 @@ where } } - let compositor = write_guard.get_mut(crtc).unwrap(); - let compositor = compositor.get_mut().unwrap(); - res = compositor.use_mode(mode); + let compositor = write_guard.get_mut(crtc).unwrap().get_mut().unwrap(); + match compositor.use_mode(mode) { + Ok(_) => { + if let Err(err) = render_elements.submit_composited_frame(&mut *compositor, renderer) { + if !matches!(err, DrmOutputManagerError::Frame(FrameError::EmptyFrame)) { + return Err(err); + } + } + } + Err(err @ FrameError::DrmError(DrmError::TestFailed(_))) => { + tracing::warn!(?crtc, ?err, "failed to set mode, trying implicit modifiers"); - if res.is_ok() { - if let Err(err) = render_elements.submit_composited_frame(&mut *compositor, renderer) { - if !matches!(err, DrmOutputManagerError::Frame(FrameError::EmptyFrame)) { - return Err(err); + for compositor in write_guard.values_mut() { + let compositor = compositor.get_mut().unwrap(); + + let current_format = compositor.format(); + if let Err(err) = + compositor.set_format(allocator.clone(), current_format, [DrmModifier::Invalid]) + { + tracing::warn!(?err, "failed to set new format"); + continue; + } + + render_elements.submit_composited_frame(&mut *compositor, renderer)?; + } + + let compositor = write_guard.get_mut(crtc).unwrap().get_mut().unwrap(); + match compositor.use_mode(mode) { + Ok(_) => render_elements.submit_composited_frame(&mut *compositor, renderer)?, + Err(err) => { + // try to reset format + + for compositor in write_guard.values_mut() { + let compositor = compositor.get_mut().unwrap(); + + let current_format = compositor.format(); + if let Err(err) = compositor.set_format( + allocator.clone(), + current_format, + renderer_formats + .iter() + .filter(|f| f.code == current_format) + .map(|f| f.modifier), + ) { + tracing::warn!(?err, "failed to reset format"); + continue; + } + + render_elements.submit_composited_frame(&mut *compositor, renderer)?; + } + + return Err(DrmOutputManagerError::Frame(err)); + } } } - } + Err(err) => return Err(DrmOutputManagerError::Frame(err)), + }; } - res.map_err(DrmOutputManagerError::Frame) + Ok(()) } /// Set of render elements for a set of outputs managed by an [`DrmOutputManager`]. From 0f2e89f4be81047a914c38381d34e70c75431b7c Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Mon, 16 Dec 2024 18:39:02 +0100 Subject: [PATCH 22/24] drm/output: Add constructors for `DrmOutputRenderElements` --- src/backend/drm/output.rs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs index 790836f1b592..345bbdb3f3aa 100644 --- a/src/backend/drm/output.rs +++ b/src/backend/drm/output.rs @@ -840,19 +840,41 @@ where _renderer: PhantomData, } -impl Default for DrmOutputRenderElements +impl DrmOutputRenderElements where E: RenderElement, R: Renderer + Bind, ::TextureId: Texture + 'static, ::Error: Send + Sync + 'static, { - fn default() -> Self { + /// Construct a new empty set of render elements + pub fn new() -> Self { DrmOutputRenderElements { render_elements: HashMap::new(), _renderer: PhantomData, } } + + /// Construct a new empty set of render elements with pre-allocated capacity + /// for a number of crtcs. + pub fn with_capacity(cap: usize) -> Self { + DrmOutputRenderElements { + render_elements: HashMap::with_capacity(cap), + _renderer: PhantomData, + } + } +} + +impl Default for DrmOutputRenderElements +where + E: RenderElement, + R: Renderer + Bind, + ::TextureId: Texture + 'static, + ::Error: Send + Sync + 'static, +{ + fn default() -> Self { + Self::new() + } } impl DrmOutputRenderElements From 0e56726761a1f608d943ea1075dc78e102356e47 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Mon, 16 Dec 2024 18:41:34 +0100 Subject: [PATCH 23/24] deps: Update pixman to 0.2.1 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 09842a62fee3..fbc40ea5554b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,7 +69,7 @@ xkbcommon = { version = "0.8.0", features = ["wayland"]} encoding_rs = { version = "0.8.33", optional = true } profiling = "1.0.13" smallvec = "1.11" -pixman = { version = "0.2.0", features = ["drm-fourcc", "sync"], optional = true } +pixman = { version = "0.2.1", features = ["drm-fourcc", "sync"], optional = true } [dev-dependencies] From 06c52e62dfd2894d3e31fcff27fd115ca0ef56fe Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Tue, 17 Dec 2024 13:13:08 +0100 Subject: [PATCH 24/24] drm/compositor: Introduce additional `FrameFlags` (instead of `FrameMode`) --- anvil/src/udev.rs | 6 +-- src/backend/drm/compositor/mod.rs | 62 +++++++++++++++++-------------- src/backend/drm/output.rs | 8 ++-- 3 files changed, 42 insertions(+), 34 deletions(-) diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index 0891dac8455a..d138c53f4330 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -31,7 +31,7 @@ use smithay::{ Fourcc, }, drm::{ - compositor::{DrmCompositor, FrameMode}, + compositor::{DrmCompositor, FrameFlags}, output::{DrmOutput, DrmOutputManager, DrmOutputRenderElements}, CreateDrmNodeError, DrmAccessError, DrmDevice, DrmDeviceFd, DrmError, DrmEvent, DrmEventMetadata, DrmNode, DrmSurface, GbmBufferedSurface, NodeType, @@ -1512,9 +1512,9 @@ fn render_surface<'a>( output_elements(output, space, custom_elements, renderer, show_window_preview); let frame_mode = if surface.disable_direct_scanout { - FrameMode::COMPOSITE + FrameFlags::empty() } else { - FrameMode::ALL + FrameFlags::DEFAULT }; let (rendered, states) = surface .drm_output diff --git a/src/backend/drm/compositor/mod.rs b/src/backend/drm/compositor/mod.rs index 6dfb3296e3fc..5e04059094db 100644 --- a/src/backend/drm/compositor/mod.rs +++ b/src/backend/drm/compositor/mod.rs @@ -58,7 +58,7 @@ //! # use std::{collections::HashSet, mem::MaybeUninit}; //! # //! use smithay::{ -//! backend::drm::{compositor::{DrmCompositor, FrameMode}, DrmSurface}, +//! backend::drm::{compositor::{DrmCompositor, FrameFlags}, DrmSurface}, //! output::{Output, PhysicalProperties, Subpixel}, //! utils::Size, //! }; @@ -104,7 +104,7 @@ //! //! # let elements: Vec> = Vec::new(); //! let render_frame_result = compositor -//! .render_frame::<_, _>(&mut renderer, &elements, CLEAR_COLOR, FrameMode::ALL) +//! .render_frame::<_, _>(&mut renderer, &elements, CLEAR_COLOR, FrameFlags::DEFAULT) //! .expect("failed to render frame"); //! //! if !render_frame_result.is_empty { @@ -1014,19 +1014,23 @@ where bitflags::bitflags! { /// Possible flags for a DMA buffer #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] - pub struct FrameMode: u32 { - /// Allow to realize the frame by compositing elements on the primary plane - const COMPOSITE = 1; + pub struct FrameFlags: u32 { /// Allow to realize the frame by scanning out elements on the primary plane - const PRIMARY_PLANE_SCANOUT = 2; + /// with the same pixel format as the main swapchain + const ALLOW_PRIMARY_PLANE_SCANOUT = 1; + /// Allow to realize the frame by scanning out elements on the primary plane + /// regardless of their format + const ALLOW_PRIMARY_PLANE_SCANOUT_ANY = 2; /// Allow to realize the frame by scanning out elements on overlay planes - const OVERLAY_PLANE_SCANOUT = 4; + const ALLOW_OVERLAY_PLANE_SCANOUT = 4; /// Allow to realize the frame by scanning out elements on cursor planes - const CURSOR_PLANE_SCANOUT = 8; + const ALLOW_CURSOR_PLANE_SCANOUT = 8; + /// Return `EmptyFrame`, if only the cursor plane would have been updated + const SKIP_CURSOR_ONLY_UPDATES = 16; /// Allow to realize the frame by assigning elements on any plane - const SCANOUT = Self::PRIMARY_PLANE_SCANOUT.bits() | Self::OVERLAY_PLANE_SCANOUT.bits() | Self::CURSOR_PLANE_SCANOUT.bits(); - /// Allow all available operations to realize the frame - const ALL = Self::COMPOSITE.bits() | Self::SCANOUT.bits(); + const ALLOW_SCANOUT = Self::ALLOW_PRIMARY_PLANE_SCANOUT.bits() | Self::ALLOW_OVERLAY_PLANE_SCANOUT.bits() | Self::ALLOW_CURSOR_PLANE_SCANOUT.bits(); + /// Safe default set of flags + const DEFAULT = Self::ALLOW_SCANOUT.bits(); } } @@ -1667,7 +1671,7 @@ where /// Render the next frame /// /// - `elements` for this frame in front-to-back order - /// - `frame_mode` specifies techniques allowed to realize the frame + /// - `frame_flags` specifies techniques allowed to realize the frame #[instrument(level = "trace", parent = &self.span, skip_all)] #[profiling::function] pub fn render_frame<'a, R, E>( @@ -1675,7 +1679,7 @@ where renderer: &mut R, elements: &'a [E], clear_color: impl Into, - frame_mode: FrameMode, + frame_flags: FrameFlags, ) -> Result, RenderFrameErrorType> where E: RenderElement, @@ -2013,7 +2017,7 @@ where output_transform, output_geometry, try_assign_primary_plane, - frame_mode, + frame_flags, ) { Ok(direct_scan_out_plane) => { match direct_scan_out_plane.type_ { @@ -2355,7 +2359,7 @@ where }; // if the update only contains a cursor position update, skip it for vrr - if self.vrr_enabled() + if frame_flags.contains(FrameFlags::SKIP_CURSOR_ONLY_UPDATES) && allow_partial_update && next_frame_state.planes.iter().all(|(plane, state)| { state.skip @@ -2801,14 +2805,14 @@ where output_transform: Transform, output_geometry: Rectangle, try_assign_primary_plane: bool, - frame_mode: FrameMode, + frame_flags: FrameFlags, ) -> Result> where R: Renderer + Bind, E: RenderElement, { // Check if we have a free plane, otherwise we can exit early - if !frame_mode.intersects(FrameMode::SCANOUT) { + if !frame_flags.intersects(FrameFlags::ALLOW_SCANOUT) { trace!( "skipping direct scan-out for element {:?}, no free planes", element.id() @@ -2829,7 +2833,7 @@ where frame_state, output_transform, output_geometry, - frame_mode, + frame_flags, ) { Ok(plane) => { trace!( @@ -2852,7 +2856,7 @@ where frame_state, output_transform, output_geometry, - frame_mode, + frame_flags, ) { trace!("assigned element {:?} to cursor {:?}", element.id(), plane.handle); return Ok(plane); @@ -2870,7 +2874,7 @@ where frame_state, output_transform, output_geometry, - frame_mode, + frame_flags, ) { Ok(plane) => { trace!( @@ -2900,13 +2904,15 @@ where frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, - frame_mode: FrameMode, + frame_flags: FrameFlags, ) -> Result> where R: Renderer, E: RenderElement, { - if !frame_mode.contains(FrameMode::PRIMARY_PLANE_SCANOUT) { + if !frame_flags + .intersects(FrameFlags::ALLOW_PRIMARY_PLANE_SCANOUT | FrameFlags::ALLOW_PRIMARY_PLANE_SCANOUT_ANY) + { return Err(None); } @@ -2935,7 +2941,9 @@ where .expect("We have a buffer for the primary plane") .buffer { - if slot.format() != element_config.properties.format { + if !frame_flags.contains(FrameFlags::ALLOW_PRIMARY_PLANE_SCANOUT_ANY) + && slot.format() != element_config.properties.format + { trace!( "failed to assign element {:?} to primary {:?}, format doesn't match", element.id(), @@ -2995,13 +3003,13 @@ where frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, - frame_mode: FrameMode, + frame_flags: FrameFlags, ) -> Option where R: Renderer, E: RenderElement, { - if !frame_mode.contains(FrameMode::CURSOR_PLANE_SCANOUT) { + if !frame_flags.contains(FrameFlags::ALLOW_CURSOR_PLANE_SCANOUT) { return None; } @@ -3691,13 +3699,13 @@ where frame_state: &mut CompositorFrameState, output_transform: Transform, output_geometry: Rectangle, - frame_mode: FrameMode, + frame_flags: FrameFlags, ) -> Result> where R: Renderer, E: RenderElement, { - if !frame_mode.contains(FrameMode::OVERLAY_PLANE_SCANOUT) { + if !frame_flags.contains(FrameFlags::ALLOW_OVERLAY_PLANE_SCANOUT) { return Err(None); } diff --git a/src/backend/drm/output.rs b/src/backend/drm/output.rs index 345bbdb3f3aa..e4e3900cf429 100644 --- a/src/backend/drm/output.rs +++ b/src/backend/drm/output.rs @@ -25,7 +25,7 @@ use crate::{ use super::{ compositor::{ - DrmCompositor, FrameError, FrameMode, FrameResult, RenderFrameError, RenderFrameErrorType, + DrmCompositor, FrameError, FrameFlags, FrameResult, RenderFrameError, RenderFrameErrorType, RenderFrameResult, }, exporter::ExportFramebuffer, @@ -379,7 +379,7 @@ where }; // We need to render the new output once to lock in the primary plane as used with the new format, so we don't hit the bandwidth issue, - // when downstream potentially uses `FrameMode::ALL` immediately after this. + // when downstream potentially uses `FrameFlags::DEFAULT` immediately after this. let compositor = write_guard.get_mut(&crtc).unwrap(); let compositor = compositor.get_mut().unwrap(); @@ -599,7 +599,7 @@ where renderer: &mut R, elements: &'a [E], clear_color: impl Into, - frame_mode: FrameMode, + frame_mode: FrameFlags, ) -> Result, RenderFrameErrorType> where E: RenderElement, @@ -922,7 +922,7 @@ where .map(|(ref elements, ref color)| (&**elements, color)) .unwrap_or((&[], &Color32F::BLACK)); compositor - .render_frame(renderer, elements, *clear_color, FrameMode::COMPOSITE) + .render_frame(renderer, elements, *clear_color, FrameFlags::empty()) .map_err(DrmOutputManagerError::RenderFrame)?; compositor.commit_frame().map_err(DrmOutputManagerError::Frame)?; Ok(())