From 1b46fd0cebe72338d6760e925b130d05ec29ee2d Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Fri, 21 Feb 2025 10:50:27 +0100 Subject: [PATCH 1/4] vulkan: fix RenderingAttachmentInfo layout --- src/gfx_apis/vulkan/renderer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gfx_apis/vulkan/renderer.rs b/src/gfx_apis/vulkan/renderer.rs index 1726d2e1..eb9ad378 100644 --- a/src/gfx_apis/vulkan/renderer.rs +++ b/src/gfx_apis/vulkan/renderer.rs @@ -519,7 +519,7 @@ impl VulkanRenderer { let rendering_attachment_info = { let mut rai = RenderingAttachmentInfo::default() .image_view(fb.render_view.unwrap_or(fb.texture_view)) - .image_layout(ImageLayout::GENERAL) + .image_layout(ImageLayout::COLOR_ATTACHMENT_OPTIMAL) .load_op(AttachmentLoadOp::LOAD) .store_op(AttachmentStoreOp::STORE); if let Some(clear) = load_clear { From cf6016f61f9fb8318c12e2c7ccf659423873054d Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Fri, 21 Feb 2025 11:14:01 +0100 Subject: [PATCH 2/4] vulkan: preserve framebuffer in pending frame --- src/gfx_api.rs | 51 +++++++++++++------------ src/gfx_apis/gl/renderer/framebuffer.rs | 6 ++- src/gfx_apis/vulkan/image.rs | 4 +- src/gfx_apis/vulkan/renderer.rs | 10 +++-- src/it/test_gfx_api.rs | 2 +- 5 files changed, 39 insertions(+), 34 deletions(-) diff --git a/src/gfx_api.rs b/src/gfx_api.rs index 3bb4acba..ca7cadac 100644 --- a/src/gfx_api.rs +++ b/src/gfx_api.rs @@ -257,23 +257,8 @@ pub enum ResetStatus { pub trait GfxFramebuffer: Debug { fn physical_size(&self) -> (i32, i32); - fn full_region(&self) -> Region { - let (width, height) = self.physical_size(); - Region::new2(Rect::new_sized_unchecked(0, 0, width, height)) - } - - fn render( - &self, - acquire_sync: AcquireSync, - release_sync: ReleaseSync, - ops: &[GfxApiOpt], - clear: Option<&Color>, - ) -> Result, GfxError> { - self.render_with_region(acquire_sync, release_sync, ops, clear, &self.full_region()) - } - fn render_with_region( - &self, + self: Rc, acquire_sync: AcquireSync, release_sync: ReleaseSync, ops: &[GfxApiOpt], @@ -300,8 +285,24 @@ pub trait GfxInternalFramebuffer: GfxFramebuffer { } impl dyn GfxFramebuffer { + pub fn render( + self: &Rc, + acquire_sync: AcquireSync, + release_sync: ReleaseSync, + ops: &[GfxApiOpt], + clear: Option<&Color>, + ) -> Result, GfxError> { + self.clone() + .render_with_region(acquire_sync, release_sync, ops, clear, &self.full_region()) + } + + fn full_region(&self) -> Region { + let (width, height) = self.physical_size(); + Region::new2(Rect::new_sized_unchecked(0, 0, width, height)) + } + pub fn clear( - &self, + self: &Rc, acquire_sync: AcquireSync, release_sync: ReleaseSync, ) -> Result, GfxError> { @@ -309,7 +310,7 @@ impl dyn GfxFramebuffer { } pub fn clear_with( - &self, + self: &Rc, acquire_sync: AcquireSync, release_sync: ReleaseSync, r: f32, @@ -334,7 +335,7 @@ impl dyn GfxFramebuffer { } pub fn copy_texture( - &self, + self: &Rc, fb_acquire_sync: AcquireSync, fb_release_sync: ReleaseSync, texture: &Rc, @@ -365,7 +366,7 @@ impl dyn GfxFramebuffer { } pub fn render_custom( - &self, + self: &Rc, acquire_sync: AcquireSync, release_sync: ReleaseSync, scale: Scale, @@ -407,13 +408,13 @@ impl dyn GfxFramebuffer { } pub fn perform_render_pass( - &self, + self: &Rc, acquire_sync: AcquireSync, release_sync: ReleaseSync, pass: &GfxRenderPass, region: &Region, ) -> Result, GfxError> { - self.render_with_region( + self.clone().render_with_region( acquire_sync, release_sync, &pass.ops, @@ -423,7 +424,7 @@ impl dyn GfxFramebuffer { } pub fn render_output( - &self, + self: &Rc, acquire_sync: AcquireSync, release_sync: ReleaseSync, node: &OutputNode, @@ -449,7 +450,7 @@ impl dyn GfxFramebuffer { } pub fn render_node( - &self, + self: &Rc, acquire_sync: AcquireSync, release_sync: ReleaseSync, node: &dyn Node, @@ -478,7 +479,7 @@ impl dyn GfxFramebuffer { } pub fn render_hardware_cursor( - &self, + self: &Rc, acquire_sync: AcquireSync, release_sync: ReleaseSync, cursor: &dyn Cursor, diff --git a/src/gfx_apis/gl/renderer/framebuffer.rs b/src/gfx_apis/gl/renderer/framebuffer.rs index 8da5ccbe..edd03187 100644 --- a/src/gfx_apis/gl/renderer/framebuffer.rs +++ b/src/gfx_apis/gl/renderer/framebuffer.rs @@ -100,14 +100,16 @@ impl GfxFramebuffer for Framebuffer { } fn render_with_region( - &self, + self: Rc, acquire_sync: AcquireSync, _release_sync: ReleaseSync, ops: &[GfxApiOpt], clear: Option<&Color>, _region: &Region, ) -> Result, GfxError> { - self.render(acquire_sync, ops, clear).map_err(|e| e.into()) + (*self) + .render(acquire_sync, ops, clear) + .map_err(|e| e.into()) } fn format(&self) -> &'static Format { diff --git a/src/gfx_apis/vulkan/image.rs b/src/gfx_apis/vulkan/image.rs index 79292272..197577f2 100644 --- a/src/gfx_apis/vulkan/image.rs +++ b/src/gfx_apis/vulkan/image.rs @@ -531,7 +531,7 @@ impl GfxFramebuffer for VulkanImage { } fn render_with_region( - &self, + self: Rc, acquire_sync: AcquireSync, release_sync: ReleaseSync, ops: &[GfxApiOpt], @@ -539,7 +539,7 @@ impl GfxFramebuffer for VulkanImage { region: &Region, ) -> Result, GfxError> { self.renderer - .execute(self, acquire_sync, release_sync, ops, clear, region) + .execute(&self, acquire_sync, release_sync, ops, clear, region) .map_err(|e| e.into()) } diff --git a/src/gfx_apis/vulkan/renderer.rs b/src/gfx_apis/vulkan/renderer.rs index eb9ad378..f36c64f5 100644 --- a/src/gfx_apis/vulkan/renderer.rs +++ b/src/gfx_apis/vulkan/renderer.rs @@ -158,6 +158,7 @@ pub(super) struct PendingFrame { point: u64, renderer: Rc, cmd: Cell>>, + _fb: Rc, _textures: Vec, wait_semaphores: Cell>>, waiter: Cell>>, @@ -1008,7 +1009,7 @@ impl VulkanRenderer { } } - fn create_pending_frame(self: &Rc, buf: Rc) { + fn create_pending_frame(self: &Rc, buf: Rc, fb: &Rc) { zone!("create_pending_frame"); let point = self.allocate_point(); let mut memory = self.memory.borrow_mut(); @@ -1016,6 +1017,7 @@ impl VulkanRenderer { point, renderer: self.clone(), cmd: Cell::new(Some(buf)), + _fb: fb.clone(), _textures: mem::take(&mut memory.textures), wait_semaphores: Cell::new(mem::take(&mut memory.wait_semaphores)), waiter: Cell::new(None), @@ -1037,7 +1039,7 @@ impl VulkanRenderer { pub fn execute( self: &Rc, - fb: &VulkanImage, + fb: &Rc, fb_acquire_sync: AcquireSync, fb_release_sync: ReleaseSync, opts: &[GfxApiOpt], @@ -1102,7 +1104,7 @@ impl VulkanRenderer { fn try_execute( self: &Rc, - fb: &VulkanImage, + fb: &Rc, fb_acquire_sync: AcquireSync, fb_release_sync: ReleaseSync, opts: &[GfxApiOpt], @@ -1127,7 +1129,7 @@ impl VulkanRenderer { self.submit(buf.buffer)?; self.import_release_semaphore(fb, fb_release_sync); self.store_layouts(fb); - self.create_pending_frame(buf); + self.create_pending_frame(buf, fb); Ok(()) } diff --git a/src/it/test_gfx_api.rs b/src/it/test_gfx_api.rs index 0ec94b1d..1cd262af 100644 --- a/src/it/test_gfx_api.rs +++ b/src/it/test_gfx_api.rs @@ -377,7 +377,7 @@ impl GfxFramebuffer for TestGfxFb { } fn render_with_region( - &self, + self: Rc, _acquire_sync: AcquireSync, _release_sync: ReleaseSync, ops: &[GfxApiOpt], From 8a3a377f61285cac97632f77927e2b41c4068b0b Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Fri, 21 Feb 2025 11:49:39 +0100 Subject: [PATCH 3/4] vulkan: de-duplicate used textures --- src/gfx_apis/vulkan/image.rs | 2 ++ src/gfx_apis/vulkan/renderer.rs | 9 ++++++--- src/gfx_apis/vulkan/shm_image.rs | 1 + 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/gfx_apis/vulkan/image.rs b/src/gfx_apis/vulkan/image.rs index 197577f2..c0746361 100644 --- a/src/gfx_apis/vulkan/image.rs +++ b/src/gfx_apis/vulkan/image.rs @@ -66,6 +66,7 @@ pub struct VulkanImage { pub(super) shader_read_only_optimal_descriptor: Box<[u8]>, pub(super) descriptor_buffer_version: Cell, pub(super) descriptor_buffer_offset: Cell, + pub(super) execution_version: Cell, } #[derive(Copy, Clone, Eq, PartialEq, Debug)] @@ -452,6 +453,7 @@ impl VulkanDmaBufImageTemplate { .sampler_read_only_descriptor(texture_view), descriptor_buffer_version: Cell::new(0), descriptor_buffer_offset: Cell::new(0), + execution_version: Cell::new(0), })) } diff --git a/src/gfx_apis/vulkan/renderer.rs b/src/gfx_apis/vulkan/renderer.rs index f36c64f5..80fc21af 100644 --- a/src/gfx_apis/vulkan/renderer.rs +++ b/src/gfx_apis/vulkan/renderer.rs @@ -87,7 +87,6 @@ pub struct VulkanRenderer { pub(super) shm_allocator: Rc, pub(super) sampler: Rc, pub(super) tex_sampler_descriptor_buffer_cache: Rc, - pub(super) descriptor_buffer_version: NumCell, pub(super) tex_descriptor_buffer_writer: RefCell, } @@ -254,7 +253,6 @@ impl VulkanDevice { shm_allocator, sampler, tex_sampler_descriptor_buffer_cache: tex_descriptor_buffer_cache, - descriptor_buffer_version: Default::default(), tex_descriptor_buffer_writer, }); render.get_or_create_pipelines(XRGB8888.vk_format)?; @@ -327,7 +325,7 @@ impl VulkanRenderer { return Ok(()); }; zone!("create_descriptor_buffer"); - let version = self.descriptor_buffer_version.add_fetch(1); + let version = self.allocate_point(); let memory = &mut *self.memory.borrow_mut(); let writer = &mut *self.tex_descriptor_buffer_writer.borrow_mut(); writer.clear(); @@ -368,12 +366,16 @@ impl VulkanRenderer { let mut memory = self.memory.borrow_mut(); memory.dmabuf_sample.clear(); memory.queue_transfer.clear(); + let execution = self.allocate_point(); for cmd in opts { if let GfxApiOpt::CopyTexture(c) = cmd { let tex = c.tex.clone().into_vk(&self.device.device); if tex.contents_are_undefined.get() { continue; } + if tex.execution_version.replace(execution) == execution { + continue; + } match tex.queue_state.get().acquire(QueueFamily::Gfx) { QueueTransfer::Unnecessary => {} QueueTransfer::Possible => memory.queue_transfer.push(tex.clone()), @@ -1055,6 +1057,7 @@ impl VulkanRenderer { memory.queue_transfer.clear(); memory.wait_semaphores.clear(); memory.release_fence.take(); + memory.descriptor_buffer.take(); memory.release_sync_file.take() }; res.map(|_| sync_file) diff --git a/src/gfx_apis/vulkan/shm_image.rs b/src/gfx_apis/vulkan/shm_image.rs index a03cef16..b405a9a0 100644 --- a/src/gfx_apis/vulkan/shm_image.rs +++ b/src/gfx_apis/vulkan/shm_image.rs @@ -462,6 +462,7 @@ impl VulkanRenderer { shader_read_only_optimal_descriptor: self.sampler_read_only_descriptor(view), descriptor_buffer_version: Cell::new(0), descriptor_buffer_offset: Cell::new(0), + execution_version: Cell::new(0), }); let shm = match &img.ty { VulkanImageMemory::DmaBuf(_) => unreachable!(), From bbe8fdecf86a6a72e69cddb2cadb0e0eabe56779 Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Fri, 21 Feb 2025 11:53:48 +0100 Subject: [PATCH 4/4] vulkan: don't call vkCmdClearAttachments if damage is empty --- src/gfx_apis/vulkan/renderer.rs | 42 +++++++++++++++++---------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/gfx_apis/vulkan/renderer.rs b/src/gfx_apis/vulkan/renderer.rs index 80fc21af..15e9a8bd 100644 --- a/src/gfx_apis/vulkan/renderer.rs +++ b/src/gfx_apis/vulkan/renderer.rs @@ -50,7 +50,7 @@ use { SubmitInfo2, Viewport, WriteDescriptorSet, }, }, - isnt::std_1::collections::IsntHashMapExt, + isnt::std_1::{collections::IsntHashMapExt, primitive::IsntSliceExt}, linearize::{Linearize, StaticMap, static_map}, std::{ cell::{Cell, RefCell}, @@ -543,25 +543,27 @@ impl VulkanRenderer { unsafe { self.device.device.cmd_begin_rendering(buf, &rendering_info); } - if let Some(clear) = manual_clear { - let clear_attachment = ClearAttachment::default() - .color_attachment(0) - .clear_value(clear) - .aspect_mask(ImageAspectFlags::COLOR); - memory.clear_rects.clear(); - for region in &memory.paint_regions { - memory.clear_rects.push(ClearRect { - rect: region.rect, - base_array_layer: 0, - layer_count: 1, - }); - } - unsafe { - self.device.device.cmd_clear_attachments( - buf, - &[clear_attachment], - &memory.clear_rects, - ); + if memory.paint_regions.is_not_empty() { + if let Some(clear) = manual_clear { + let clear_attachment = ClearAttachment::default() + .color_attachment(0) + .clear_value(clear) + .aspect_mask(ImageAspectFlags::COLOR); + memory.clear_rects.clear(); + for region in &memory.paint_regions { + memory.clear_rects.push(ClearRect { + rect: region.rect, + base_array_layer: 0, + layer_count: 1, + }); + } + unsafe { + self.device.device.cmd_clear_attachments( + buf, + &[clear_attachment], + &memory.clear_rects, + ); + } } } }