From d896a207de7072bc66caf7f0e59020218995ae74 Mon Sep 17 00:00:00 2001 From: janis Date: Sun, 25 Oct 2026 22:27:01 +0100 Subject: [PATCH] refactor: simplify barrier building logic and remove unused structures --- crates/renderer_rendergraph/src/graph.rs | 665 +++---------------- crates/renderer_rendergraph/src/recorder.rs | 4 +- crates/renderer_rendergraph/src/resources.rs | 88 +-- crates/renderer_resources/src/images.rs | 4 +- 4 files changed, 90 insertions(+), 671 deletions(-) diff --git a/crates/renderer_rendergraph/src/graph.rs b/crates/renderer_rendergraph/src/graph.rs index 92944a2..34a9549 100644 --- a/crates/renderer_rendergraph/src/graph.rs +++ b/crates/renderer_rendergraph/src/graph.rs @@ -19,8 +19,7 @@ use crate::{ recorder::{CommandIndex, CommandList, CommandRecorder, RecordError, scope_range}, resources::{ Buffer, GraphBufferDesc, GraphTextureDesc, ResourceDesc, ResourceId, Subresource, - SubresourceScope, SubresourceScopeInner, Texture, access_flag_is_read, - access_flag_is_write, + SubresourceScope, SubresourceScopeInner, Texture, access_flag_is_write, }, }; @@ -55,40 +54,6 @@ pub struct PipelineBarrier { pub resources: Vec, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -struct AccessRef { - ordered_index: usize, - access_index: usize, -} - -#[derive(Debug, Clone, Copy)] -struct BarrierResourceRef { - barrier_index: usize, - resource_index: usize, -} - -impl BarrierResourceRef { - fn get(self, barriers: &[PipelineBarrier]) -> &BarrierResource { - &barriers[self.barrier_index].resources[self.resource_index] - } - - fn widen( - self, - barriers: &mut [PipelineBarrier], - stage: vk::PipelineStageFlags2, - access: vk::AccessFlags2, - ) { - let resource = &mut barriers[self.barrier_index].resources[self.resource_index]; - resource.dst_stage |= stage; - resource.dst_access |= access; - } -} - -struct EmittedBarrierEntry { - access: AccessRef, - resource: BarrierResourceRef, -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct ResourceLifetime { pub first_command: CommandIndex, @@ -289,8 +254,7 @@ impl RenderGraph { let compiler = GraphCompiler::new(&self.resources, accesses, command_count); let batches = compiler.topo_batches()?; compiler.validate_resources()?; - let flat_order = batches.iter().flatten().copied().collect::>(); - let barriers = compiler.build_barriers(&flat_order, &batches); + let barriers = compiler.build_barriers(&batches); let (lifetimes, interference) = compiler.bind_lifetimes(); Ok(CompiledGraph { @@ -298,7 +262,6 @@ impl RenderGraph { resolved: HashMap::new(), commands: self.commands, batches, - flat_order, barriers, lifetimes, interference, @@ -372,22 +335,17 @@ impl<'a> GraphCompiler<'a> { Ok(()) } - fn build_barriers( - &self, - order: &[CommandIndex], - batches: &[Vec], - ) -> Vec { + fn build_barriers(&self, batches: &[Vec]) -> Vec { let initial_states = self .resources .iter() .map(ResourceDesc::initial_state) .collect::>(); - self.build_barriers_with_initial_states(order, batches, &initial_states) + self.build_barriers_with_initial_states(batches, &initial_states) } fn build_barriers_with_initial_states( &self, - order: &[CommandIndex], batches: &[Vec], initial_states: &[crate::resources::ResourceState], ) -> Vec { @@ -400,136 +358,27 @@ impl<'a> GraphCompiler<'a> { let mut result: Vec = Vec::new(); - // Associates each access that produced a barrier with its - // BarrierResource, so a later contained read can widen its dst masks. - // This empirically-driven optimization reduces barrier calls, but is - // not guaranteed to be cost-free: wider stage/access masks can increase - // cache-visibility or stage-synchronization costs on some vendors, and - // the later read waits at the earlier barrier point. - let mut barrier_entries: HashMap = HashMap::new(); - - for (ordered_index, &command_index) in order.iter().enumerate() { + for (ordered_index, &command_index) in batches.iter().flatten().enumerate() { let mut barrier_resources = Vec::new(); - let mut emitted_entries = Vec::new(); - for (current_access_index, current) in - self.accesses[command_index.index()].iter().enumerate() - { - let current_is_pure_read = access_flag_is_read(current.scope.access) - && !access_flag_is_write(current.scope.access); - - // Only reads can fold: an overlapping write breaks the read - // chain for the affected subresources, while disjoint writes - // leave it intact. - if current_is_pure_read { - let mut folded = false; - - 'prior_accesses: for prior_order_index in (0..ordered_index).rev() { - let prior_command = order[prior_order_index]; - for (prior_access_index, prior) in self.accesses[prior_command.index()] + for current in &self.accesses[command_index.index()] { + // The most recent conflicting access is the complete source scope + // for this barrier; prior barrier masks are not carried forward. + let previous = batches + .iter() + .flatten() + .take(ordered_index) + .filter_map(|&prior_command| { + self.accesses[prior_command.index()] .iter() - .enumerate() .rev() - .filter(|(_, prior)| prior.resource == current.resource) - { - if !access_flag_is_write(prior.scope.access) { - if access_flag_is_read(prior.scope.access) - && prior.scope.has_same_access_polarity(¤t.scope) - && prior.scope.contains_subresource_range(¤t.scope) - { - let prior_access = AccessRef { - ordered_index: prior_order_index, - access_index: prior_access_index, - }; - if let Some(barrier_resource) = - barrier_entries.get(&prior_access).copied() - { - let prior_barrier_resource = barrier_resource.get(&result); - // Folding is valid only if the earlier barrier covers - // this entire read; widening masks cannot extend its range - // or change the layout established at that barrier. - if Self::can_fold_read( - prior.scope, - current.scope, - prior_barrier_resource, - ) { - barrier_resource.widen( - &mut result, - current.scope.stage, - current.scope.access, - ); - folded = true; - } - } - - if folded { - break 'prior_accesses; - } - - // This is the nearest containing read. If it - // cannot fold, normal hazard/initial-state - // handling must decide what barrier is needed. - break 'prior_accesses; - } - } else if prior.scope.overlaps_subresource(¤t.scope) { - // Only a write overlapping this read's range breaks - // its chain; disjoint writes do not affect containment. - break 'prior_accesses; - } - } - } - - if folded { - continue; - } - } - - let mut previous = None; - let mut src_stage = vk::PipelineStageFlags2::NONE; - let mut src_access = vk::AccessFlags2::NONE; - - // Accumulate every outstanding conflicting source, newest first. - // A prior barrier is a chain point only for the subresources its - // range covers. For partial coverage, conservatively keep scanning - // and include every older conflicting access; source masks do not - // encode which part of the current range they made available. - 'prior_barriers: for prior_order_index in (0..ordered_index).rev() { - let prior_command = order[prior_order_index]; - for (prior_access_index, prior) in self.accesses[prior_command.index()] - .iter() - .enumerate() - .rev() - .filter(|(_, prior)| { - prior.resource == current.resource - && prior.scope.conflicts(¤t.scope) - }) - { - if previous.is_none() { - previous = Some((prior_command, prior)); - } - - src_stage |= prior.scope.stage; - src_access |= prior.scope.access; - - let prior_access = AccessRef { - ordered_index: prior_order_index, - access_index: prior_access_index, - }; - - if let Some(barrier_resource) = barrier_entries.get(&prior_access).copied() - { - // The prior barrier's source masks carry the older - // availability chain; include them in this source scope. - let prior_barrier = barrier_resource.get(&result); - src_stage |= prior_barrier.src_stage; - src_access |= prior_barrier.src_access; - - if prior_barrier.range.contains_scope(¤t.scope) { - break 'prior_barriers; - } - } - } - } + .find(|prior| { + prior.resource == current.resource + && prior.scope.conflicts(¤t.scope) + }) + .map(|prior| (prior_command, prior)) + }) + .last(); if let Some((previous_command, previous)) = previous { debug_assert!( @@ -539,15 +388,13 @@ impl<'a> GraphCompiler<'a> { // Layouts follow the latest conflicting access, which is // the state the resource actually reached most recently. - // TODO: Define a range-aware policy if accumulated sources - // with different layouts can overlap only partially. let (old_layout, new_layout) = Self::layouts(previous.scope, current.scope); barrier_resources.push(BarrierResource { id: current.resource, range: scope_range(current.scope), - src_stage, - src_access, + src_stage: previous.scope.stage, + src_access: previous.scope.access, dst_stage: current.scope.stage, dst_access: current.scope.access, old_layout, @@ -555,17 +402,6 @@ impl<'a> GraphCompiler<'a> { queue_transfer: None, }); - emitted_entries.push(EmittedBarrierEntry { - access: AccessRef { - ordered_index, - access_index: current_access_index, - }, - resource: BarrierResourceRef { - barrier_index: result.len(), - resource_index: barrier_resources.len() - 1, - }, - }); - continue; } @@ -595,24 +431,10 @@ impl<'a> GraphCompiler<'a> { new_layout, queue_transfer: None, }); - - emitted_entries.push(EmittedBarrierEntry { - access: AccessRef { - ordered_index, - access_index: current_access_index, - }, - resource: BarrierResourceRef { - barrier_index: result.len(), - resource_index: barrier_resources.len() - 1, - }, - }); } } if !barrier_resources.is_empty() { - for entry in emitted_entries { - barrier_entries.insert(entry.access, entry.resource); - } result.push(PipelineBarrier { before_command: command_index, resources: barrier_resources, @@ -647,18 +469,6 @@ impl<'a> GraphCompiler<'a> { (initial.layout, current_layout) } - fn can_fold_read( - prior: SubresourceScope, - current: SubresourceScope, - barrier: &BarrierResource, - ) -> bool { - let (_, prior_layout) = Self::layouts(prior, prior); - let (_, current_layout) = Self::layouts(prior, current); - prior_layout == current_layout - && barrier.new_layout == current_layout - && barrier.range.contains_scope(¤t) - } - fn bind_lifetimes(&self) -> (HashMap, ResourceInterference) { let mut lifetimes = HashMap::new(); for (command, command_accesses) in self.accesses.iter().enumerate() { @@ -732,7 +542,6 @@ pub struct CompiledGraph { resolved: HashMap, commands: CommandList, batches: Vec>, - flat_order: Vec, barriers: Vec, lifetimes: HashMap, interference: ResourceInterference, @@ -748,10 +557,6 @@ impl CompiledGraph { &self.batches } - pub fn command_order(&self) -> &[CommandIndex] { - &self.flat_order - } - pub fn barriers(&self) -> &[PipelineBarrier] { &self.barriers } @@ -1260,7 +1065,15 @@ mod tests { }); let compiled = graph.compile().expect("graph compiles"); - assert_eq!(compiled.command_order(), &[CommandIndex(0)]); + assert_eq!( + compiled + .batches() + .iter() + .flatten() + .copied() + .collect::>(), + vec![CommandIndex(0)] + ); assert!(compiled.barriers().is_empty()); assert_eq!(compiled.resource_lifetimes().len(), 2); assert!(compiled.interference().interferes(src.id(), dst.id())); @@ -1308,8 +1121,13 @@ mod tests { let compiled = graph.compile().expect("graph compiles"); assert_eq!( - compiled.command_order(), - &[CommandIndex(0), CommandIndex(1)] + compiled + .batches() + .iter() + .flatten() + .copied() + .collect::>(), + vec![CommandIndex(0), CommandIndex(1)] ); assert_eq!(compiled.barriers().len(), 1); assert_eq!(compiled.barriers()[0].before_command, CommandIndex(1)); @@ -1317,7 +1135,7 @@ mod tests { } #[test] - fn write_read_write_barrier_accumulates_outstanding_write_and_read_sources() { + fn write_read_write_emits_barriers_from_each_most_recent_conflict() { let resource = ResourceId(0); let resources = [ResourceDesc::CreatedBuffer(GraphBufferDesc { size: 64, @@ -1348,8 +1166,26 @@ mod tests { ]; let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - let barriers = compiler.build_barriers(&order, &batches); + let barriers = compiler.build_barriers(&batches); + + assert_eq!(barriers.len(), 2); + assert_eq!(barriers[0].before_command, CommandIndex(1)); + assert_eq!( + barriers[0].resources[0].src_stage, + vk::PipelineStageFlags2::COPY + ); + assert_eq!( + barriers[0].resources[0].src_access, + vk::AccessFlags2::TRANSFER_WRITE + ); + assert_eq!( + barriers[0].resources[0].dst_stage, + vk::PipelineStageFlags2::VERTEX_SHADER + ); + assert_eq!( + barriers[0].resources[0].dst_access, + vk::AccessFlags2::SHADER_READ + ); let second_write_barrier = barriers .iter() @@ -1357,114 +1193,11 @@ mod tests { .expect("second write has a barrier"); assert_eq!( second_write_barrier.resources[0].src_stage, - vk::PipelineStageFlags2::COPY | vk::PipelineStageFlags2::VERTEX_SHADER + vk::PipelineStageFlags2::VERTEX_SHADER ); assert_eq!( second_write_barrier.resources[0].src_access, - vk::AccessFlags2::TRANSFER_WRITE | vk::AccessFlags2::SHADER_READ - ); - } - - #[test] - fn partial_barrier_coverage_keeps_accumulating_older_write_sources() { - let resource = ResourceId(0); - let resources = [ResourceDesc::CreatedBuffer(GraphBufferDesc { - size: 64, - ..Default::default() - })]; - let accesses = vec![ - vec![scoped_access( - resource, - 0, - 64, - vk::PipelineStageFlags2::COPY, - vk::AccessFlags2::TRANSFER_WRITE, - )], - vec![scoped_access( - resource, - 0, - 16, - vk::PipelineStageFlags2::COMPUTE_SHADER, - vk::AccessFlags2::SHADER_WRITE, - )], - vec![scoped_access( - resource, - 0, - 64, - vk::PipelineStageFlags2::FRAGMENT_SHADER, - vk::AccessFlags2::COLOR_ATTACHMENT_WRITE, - )], - ]; - let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); - let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - let barriers = compiler.build_barriers(&order, &batches); - let third_write_barrier = barriers - .iter() - .find(|barrier| barrier.before_command == CommandIndex(2)) - .expect("third write has a barrier"); - - assert_eq!( - third_write_barrier.resources[0].src_stage, - vk::PipelineStageFlags2::COPY | vk::PipelineStageFlags2::COMPUTE_SHADER - ); - assert_eq!( - third_write_barrier.resources[0].src_access, - vk::AccessFlags2::TRANSFER_WRITE | vk::AccessFlags2::SHADER_WRITE - ); - } - - #[test] - fn disjoint_write_does_not_break_contained_read_barrier_folding() { - let resource = ResourceId(0); - let resources = [ResourceDesc::CreatedBuffer(GraphBufferDesc { - size: 128, - ..Default::default() - })]; - let accesses = vec![ - vec![scoped_access( - resource, - 0, - 32, - vk::PipelineStageFlags2::COPY, - vk::AccessFlags2::TRANSFER_WRITE, - )], - vec![scoped_access( - resource, - 0, - 32, - vk::PipelineStageFlags2::VERTEX_SHADER, - vk::AccessFlags2::SHADER_READ, - )], - vec![scoped_access( - resource, - 64, - 32, - vk::PipelineStageFlags2::COMPUTE_SHADER, - vk::AccessFlags2::SHADER_WRITE, - )], - vec![scoped_access( - resource, - 8, - 8, - vk::PipelineStageFlags2::FRAGMENT_SHADER, - vk::AccessFlags2::UNIFORM_READ, - )], - ]; - let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); - let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - let barriers = compiler.build_barriers(&order, &batches); - - assert_eq!(barriers.len(), 1); - assert_eq!(barriers[0].before_command, CommandIndex(1)); - assert_eq!( - barriers[0].resources[0].dst_stage, - vk::PipelineStageFlags2::VERTEX_SHADER | vk::PipelineStageFlags2::FRAGMENT_SHADER - ); - assert_eq!( - barriers[0].resources[0].dst_access, - vk::AccessFlags2::SHADER_READ | vk::AccessFlags2::UNIFORM_READ + vk::AccessFlags2::SHADER_READ ); } @@ -1502,9 +1235,7 @@ mod tests { ]; let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - let barriers = - compiler.build_barriers_with_initial_states(&order, &batches, &initial_states); + let barriers = compiler.build_barriers_with_initial_states(&batches, &initial_states); assert_eq!(barriers.len(), 1); assert_eq!(barriers[0].before_command, CommandIndex(1)); @@ -1523,255 +1254,7 @@ mod tests { } #[test] - fn contained_read_folds_into_prior_read_barrier_destination_masks() { - let resource = ResourceId(0); - let resources = [ResourceDesc::CreatedBuffer(GraphBufferDesc { - size: 64, - ..Default::default() - })]; - let accesses = vec![ - vec![scoped_access( - resource, - 0, - 64, - vk::PipelineStageFlags2::COPY, - vk::AccessFlags2::TRANSFER_WRITE, - )], - vec![scoped_access( - resource, - 0, - 64, - vk::PipelineStageFlags2::VERTEX_SHADER, - vk::AccessFlags2::SHADER_READ, - )], - vec![scoped_access( - resource, - 16, - 16, - vk::PipelineStageFlags2::FRAGMENT_SHADER, - vk::AccessFlags2::UNIFORM_READ, - )], - ]; - let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); - let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - let barriers = compiler.build_barriers(&order, &batches); - - assert_eq!(barriers.len(), 1); - assert_eq!(barriers[0].before_command, CommandIndex(1)); - assert_eq!(barriers[0].resources.len(), 1); - assert_eq!( - barriers[0].resources[0].dst_stage, - vk::PipelineStageFlags2::VERTEX_SHADER | vk::PipelineStageFlags2::FRAGMENT_SHADER - ); - assert_eq!( - barriers[0].resources[0].dst_access, - vk::AccessFlags2::SHADER_READ | vk::AccessFlags2::UNIFORM_READ - ); - } - - #[test] - fn narrow_prior_barrier_does_not_cover_a_later_larger_read() { - let resource = ResourceId(0); - let prior = SubresourceScope { - access: vk::AccessFlags2::SHADER_READ, - stage: vk::PipelineStageFlags2::VERTEX_SHADER, - subresource: SubresourceScopeInner::Buffer(BufferRange { - offset: 0, - size: 64, - }), - }; - let current = SubresourceScope { - access: vk::AccessFlags2::UNIFORM_READ, - stage: vk::PipelineStageFlags2::FRAGMENT_SHADER, - subresource: SubresourceScopeInner::Buffer(BufferRange { - offset: 0, - size: 32, - }), - }; - let narrow_barrier = BarrierResource { - id: resource, - range: Subresource::Buffer(BufferRange { - offset: 0, - size: 16, - }), - src_stage: vk::PipelineStageFlags2::COPY, - src_access: vk::AccessFlags2::TRANSFER_WRITE, - dst_stage: prior.stage, - dst_access: prior.access, - old_layout: None, - new_layout: None, - queue_transfer: None, - }; - - assert!(prior.contains_subresource_range(¤t)); - assert!(!GraphCompiler::can_fold_read( - prior, - current, - &narrow_barrier - )); - - // The prior scope contains the later read and the layouts match; only - // the synthetic barrier's narrower range invalidates folding. - let resources = [ResourceDesc::CreatedBuffer(GraphBufferDesc { - size: 64, - ..Default::default() - })]; - let accesses = vec![ - vec![scoped_access( - resource, - 0, - 64, - vk::PipelineStageFlags2::COPY, - vk::AccessFlags2::TRANSFER_WRITE, - )], - vec![scoped_access( - resource, - 0, - 16, - vk::PipelineStageFlags2::VERTEX_SHADER, - vk::AccessFlags2::SHADER_READ, - )], - vec![scoped_access( - resource, - 0, - 64, - vk::PipelineStageFlags2::FRAGMENT_SHADER, - vk::AccessFlags2::UNIFORM_READ, - )], - ]; - let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); - let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - let barriers = compiler.build_barriers(&order, &batches); - - let later_read_barrier = barriers - .iter() - .find(|barrier| barrier.before_command == CommandIndex(2)) - .expect("larger later read gets its own barrier"); - assert_eq!(later_read_barrier.resources[0].dst_stage, current.stage); - } - - #[test] - fn intervening_write_prevents_contained_read_barrier_folding() { - let resource = ResourceId(0); - let resources = [ResourceDesc::CreatedBuffer(GraphBufferDesc { - size: 64, - ..Default::default() - })]; - let accesses = vec![ - vec![scoped_access( - resource, - 0, - 64, - vk::PipelineStageFlags2::COPY, - vk::AccessFlags2::TRANSFER_WRITE, - )], - vec![scoped_access( - resource, - 0, - 64, - vk::PipelineStageFlags2::VERTEX_SHADER, - vk::AccessFlags2::SHADER_READ, - )], - vec![scoped_access( - resource, - 0, - 64, - vk::PipelineStageFlags2::COMPUTE_SHADER, - vk::AccessFlags2::SHADER_WRITE, - )], - vec![scoped_access( - resource, - 16, - 16, - vk::PipelineStageFlags2::FRAGMENT_SHADER, - vk::AccessFlags2::SHADER_READ, - )], - ]; - let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); - let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - let barriers = compiler.build_barriers(&order, &batches); - - assert_eq!(barriers.len(), 3); - assert_eq!(barriers[0].before_command, CommandIndex(1)); - assert_eq!(barriers[1].before_command, CommandIndex(2)); - assert_eq!(barriers[2].before_command, CommandIndex(3)); - assert_eq!( - barriers[2].resources[0].dst_stage, - vk::PipelineStageFlags2::FRAGMENT_SHADER - ); - } - - #[test] - fn contained_texture_read_folds_aspect_mip_and_layer_ranges() { - let resource = ResourceId(0); - let resources = [ResourceDesc::CreatedTexture(GraphTextureDesc { - format: vk::Format::R8G8B8A8_UNORM, - mip_levels: 4, - array_layers: 8, - ..Default::default() - })]; - let accesses = vec![ - vec![texture_scoped_access( - resource, - texture_range( - vk::ImageAspectFlags::COLOR | vk::ImageAspectFlags::DEPTH, - 0, - 4, - 0, - 8, - ), - vk::ImageLayout::GENERAL, - vk::PipelineStageFlags2::COPY, - vk::AccessFlags2::TRANSFER_WRITE, - )], - vec![texture_scoped_access( - resource, - texture_range( - vk::ImageAspectFlags::COLOR | vk::ImageAspectFlags::DEPTH, - 0, - 4, - 0, - 8, - ), - vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, - vk::PipelineStageFlags2::VERTEX_SHADER, - vk::AccessFlags2::SHADER_READ, - )], - vec![texture_scoped_access( - resource, - texture_range(vk::ImageAspectFlags::COLOR, 1, 2, 2, 3), - vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, - vk::PipelineStageFlags2::FRAGMENT_SHADER, - vk::AccessFlags2::SHADER_SAMPLED_READ, - )], - ]; - let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); - let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - let barriers = compiler.build_barriers(&order, &batches); - - assert_eq!(barriers.len(), 2); - assert_eq!(barriers[0].before_command, CommandIndex(0)); - assert_eq!(barriers[1].before_command, CommandIndex(1)); - let barrier = &barriers[1].resources[0]; - assert_eq!( - barrier.dst_stage, - vk::PipelineStageFlags2::VERTEX_SHADER | vk::PipelineStageFlags2::FRAGMENT_SHADER - ); - assert_eq!( - barrier.dst_access, - vk::AccessFlags2::SHADER_READ | vk::AccessFlags2::SHADER_SAMPLED_READ - ); - assert!( - matches!(barrier.range, Subresource::Texture(range) if range.aspect_mask == vk::ImageAspectFlags::COLOR | vk::ImageAspectFlags::DEPTH && range.base_mip_level == 0 && range.level_count == 4 && range.base_array_layer == 0 && range.layer_count == 8) - ); - } - - #[test] - fn overlapping_texture_reads_with_different_layouts_are_not_folded() { + fn overlapping_texture_reads_with_different_layouts_get_layout_transition() { let resource = ResourceId(0); let resources = [ResourceDesc::CreatedTexture(GraphTextureDesc { format: vk::Format::R8G8B8A8_UNORM, @@ -1803,8 +1286,7 @@ mod tests { ]; let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - let barriers = compiler.build_barriers(&order, &batches); + let barriers = compiler.build_barriers(&batches); assert_eq!(barriers.len(), 3); let current_read_barrier = &barriers[2]; @@ -1820,7 +1302,7 @@ mod tests { } #[test] - fn pure_read_without_prior_barrier_entry_needs_no_barrier() { + fn overlapping_pure_reads_without_layout_change_need_no_barrier() { let resource = ResourceId(0); let resources = [ResourceDesc::CreatedBuffer(GraphBufferDesc { size: 64, @@ -1844,9 +1326,8 @@ mod tests { ]; let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - assert!(compiler.build_barriers(&order, &batches).is_empty()); + assert!(compiler.build_barriers(&batches).is_empty()); } #[test] @@ -1874,8 +1355,7 @@ mod tests { ]; let compiler = GraphCompiler::new(&resources, &accesses, accesses.len()); let batches = compiler.topo_batches().expect("graph is acyclic"); - let order = batches.iter().flatten().copied().collect::>(); - let barriers = compiler.build_barriers(&order, &batches); + let barriers = compiler.build_barriers(&batches); assert_eq!(batches, [vec![CommandIndex(0)], vec![CommandIndex(1)]]); assert_eq!(barriers.len(), 1); @@ -1946,8 +1426,13 @@ mod tests { ] ); assert_eq!( - compiled.command_order(), - [CommandIndex(0), CommandIndex(2), CommandIndex(1)] + compiled + .batches() + .iter() + .flatten() + .copied() + .collect::>(), + vec![CommandIndex(0), CommandIndex(2), CommandIndex(1)] ); assert_eq!(compiled.barriers().len(), 1); assert_eq!(compiled.barriers()[0].before_command, CommandIndex(1)); diff --git a/crates/renderer_rendergraph/src/recorder.rs b/crates/renderer_rendergraph/src/recorder.rs index 9f617a6..16912b1 100644 --- a/crates/renderer_rendergraph/src/recorder.rs +++ b/crates/renderer_rendergraph/src/recorder.rs @@ -50,8 +50,8 @@ impl SideEffectMap { /// Record an access scope for a command and resource. /// /// Each scope must describe the command's complete use for that access, - /// not merely an enclosing bound. In particular, the reverse containment - /// fold is sound only when the larger scope represents the complete use. + /// not merely an enclosing bound, because `SideEffectMap::insert` may merge + /// contained scopes for the same command and resource. pub fn insert( &mut self, command_index: CommandIndex, diff --git a/crates/renderer_rendergraph/src/resources.rs b/crates/renderer_rendergraph/src/resources.rs index 15aff07..6308b03 100644 --- a/crates/renderer_rendergraph/src/resources.rs +++ b/crates/renderer_rendergraph/src/resources.rs @@ -136,50 +136,6 @@ pub enum Subresource { Texture(vk::ImageSubresourceRange), } -impl Subresource { - /// Whether this barrier range fully covers the scope's subresource range. - pub fn contains_scope(self, scope: &SubresourceScope) -> bool { - match (self, scope.subresource) { - (Self::Buffer(range), SubresourceScopeInner::Buffer(other)) => range.contains(&other), - (Self::Texture(range), SubresourceScopeInner::Texture { range: other, .. }) => { - range.aspect_mask.contains(other.aspect_mask) - && range_contains( - range.base_mip_level, - range.level_count, - other.base_mip_level, - other.level_count, - vk::REMAINING_MIP_LEVELS, - ) - && range_contains( - range.base_array_layer, - range.layer_count, - other.base_array_layer, - other.layer_count, - vk::REMAINING_ARRAY_LAYERS, - ) - } - _ => false, - } - } -} - -fn range_contains( - base: u32, - count: u32, - other_base: u32, - other_count: u32, - remaining: u32, -) -> bool { - let end = |base: u32, count: u32| { - if count == remaining { - u64::MAX - } else { - u64::from(base) + u64::from(count) - } - }; - base <= other_base && end(base, count) >= end(other_base, other_count) -} - #[derive(Debug, Clone, Copy)] pub enum SubresourceScopeInner { Buffer(BufferRange), @@ -197,43 +153,21 @@ pub struct SubresourceScope { } impl SubresourceScope { - /// Whether the scopes refer to any common buffer bytes or texture - /// subresources, regardless of access mode or image layout. - pub fn overlaps_subresource(&self, other: &Self) -> bool { - match (self.subresource, other.subresource) { - (SubresourceScopeInner::Buffer(a), SubresourceScopeInner::Buffer(b)) => a.overlaps(b), - ( - SubresourceScopeInner::Texture { range: a, .. }, - SubresourceScopeInner::Texture { range: b, .. }, - ) => a.intersects(&b), - _ => false, - } - } - - /// Whether this scope's subresource contains the other's subresource. + /// Whether this scope contains the other's subresource range. /// Texture ranges are compatible only when their layouts match. pub fn contains_subresource(&self, other: &Self) -> bool { - self.contains_subresource_range(other) - && match (self.subresource, other.subresource) { - ( - SubresourceScopeInner::Texture { layout: a, .. }, - SubresourceScopeInner::Texture { layout: b, .. }, - ) => a == b, - (SubresourceScopeInner::Buffer(_), SubresourceScopeInner::Buffer(_)) => true, - _ => false, - } - } - - /// Whether this scope's range contains the other's, without considering - /// texture layout. Use this to find candidates whose layout must be checked - /// separately (for example, when deciding whether a read can fold). - pub fn contains_subresource_range(&self, other: &Self) -> bool { match (self.subresource, other.subresource) { (SubresourceScopeInner::Buffer(a), SubresourceScopeInner::Buffer(b)) => a.contains(&b), ( - SubresourceScopeInner::Texture { range: a, .. }, - SubresourceScopeInner::Texture { range: b, .. }, - ) => a.contains(&b), + SubresourceScopeInner::Texture { + range: a, + layout: layout_a, + }, + SubresourceScopeInner::Texture { + range: b, + layout: layout_b, + }, + ) => layout_a == layout_b && a.contains(&b), _ => false, } } @@ -244,7 +178,7 @@ impl SubresourceScope { && access_flag_is_write(self.access) == access_flag_is_write(other.access) } - /// Fold sub-ranges with compatible access polarity into one scope. + /// Merge sub-ranges with compatible access polarity into one scope. pub fn try_merge(&mut self, other: &Self) -> bool { if access_flag_is_read(self.access) != access_flag_is_read(other.access) || access_flag_is_write(self.access) != access_flag_is_write(other.access) diff --git a/crates/renderer_resources/src/images.rs b/crates/renderer_resources/src/images.rs index 7bad7b4..8acb9de 100644 --- a/crates/renderer_resources/src/images.rs +++ b/crates/renderer_resources/src/images.rs @@ -1127,10 +1127,10 @@ impl ImageSubresourceExt for vk::ImageSubresource { self.aspect_mask } fn layers(&self) -> MipRange { - MipRange::single(self.mip_level) + MipRange::single(self.array_layer) } fn mip_levels(&self) -> MipRange { - MipRange::single(self.array_layer) + MipRange::single(self.mip_level) } }