access: this is quite complicated actually..

crates/renderer/src/images.rs

@@ -449,7 +449,11 @@ impl Image {
             });
         }
 
-        if !desc.mip_range.fits_in(self.desc.mip_levels) {
+        // update imageview desc to make sure the mip range and layer ranges don't contain `vk::REMAINING_MIP_LEVELS`.
+        desc.mip_range.set_max_end(self.desc.mip_levels);
+        desc.layer_range.set_max_end(self.desc.array_layers);
+
+        if !MipRange::from(..self.desc.mip_levels).contains(&desc.mip_range) {
             tracing::error!(
                 "image view mip range {:?} exceeds image mip levels {}",
                 desc.mip_range,
@@ -785,6 +789,7 @@ impl From<(i32, i32, i32)> for Offset {
     }
 }
 
+/// `start..end` range of mip levels or array layers.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct MipRange {
     start: u32,
@@ -801,6 +806,18 @@ impl Default for MipRange {
 }
 
 impl MipRange {
+    pub fn new(start: u32, end: u32) -> Self {
+        Self { start, end }
+    }
+    pub fn with_max_end(mut self, end: u32) -> Self {
+        self.set_max_end(end);
+        self
+    }
+    pub fn set_max_end(&mut self, end: u32) {
+        if self.end == vk::REMAINING_MIP_LEVELS {
+            self.end = end;
+        }
+    }
     pub fn len(&self) -> u32 {
         self.end - self.start
     }
@@ -813,6 +830,8 @@ impl MipRange {
         self.end
     }
 
+    /// Returns the intersection of this range with another range.
+    /// If the ranges do not intersect, returns an empty range.
     pub fn range(&self, other: &Self) -> Self {
         Self {
             start: self.start.max(other.start).min(self.end),
@@ -828,10 +847,19 @@ impl MipRange {
         self.start < total_mips && (self.end == vk::REMAINING_MIP_LEVELS || self.end <= total_mips)
     }
 
+    pub fn contains(&self, other: &Self) -> bool {
+        self.start <= other.start && self.end >= other.end
+    }
+
     pub fn intersects(&self, other: &Self) -> bool {
         self.start < other.end && self.end > other.start
     }
 
+    /// Returns the union of this range with another range.
+    /// The union of two ranges is the smallest range that contains both ranges.
+    /// Note that since the union of two ranges may contain values that are not
+    /// in either range, this function does not satisfy the properties of a
+    /// mathematical union operation.
     pub fn union(&self, other: &Self) -> Self {
         Self {
             start: self.start.min(other.start),
@@ -840,6 +868,59 @@ impl MipRange {
     }
 }
 
+use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign};
+
+macro_rules! impl_op {
+    (impl $trait:ident {$fn:ident} for $ty:ident { fn: $method:ident }) => {
+        impl $trait for $ty {
+            type Output = $ty;
+
+            fn $fn(self, rhs: $ty) -> Self::Output {
+                self.$method(&rhs)
+            }
+        }
+        impl $trait<&$ty> for $ty {
+            type Output = $ty;
+
+            fn $fn(self, rhs: &$ty) -> Self::Output {
+                self.$method(rhs)
+            }
+        }
+        impl $trait<&$ty> for &$ty {
+            type Output = $ty;
+
+            fn $fn(self, rhs: &$ty) -> Self::Output {
+                self.$method(rhs)
+            }
+        }
+        impl $trait<$ty> for &$ty {
+            type Output = $ty;
+
+            fn $fn(self, rhs: $ty) -> Self::Output {
+                self.$method(&rhs)
+            }
+        }
+    };
+
+    (impl $trait:ident {$fn:ident} for $ty:ident { λ: $method:expr }) => {
+        impl $trait for $ty {
+            fn $fn(&mut self, rhs: $ty) {
+                $method(self, &rhs)
+            }
+        }
+        impl $trait<&$ty> for $ty {
+            fn $fn(&mut self, rhs: &$ty) {
+                $method(self, rhs)
+            }
+        }
+    };
+}
+
+impl_op!(impl BitOr { bitor } for MipRange { fn: union });
+impl_op!(impl BitOrAssign { bitor_assign } for MipRange { λ: |this: &mut MipRange, other: &MipRange| {*this = this.union(other)} });
+impl_op!(impl BitAnd { bitand } for MipRange { fn: range });
+impl_op!(impl BitAndAssign { bitand_assign } for MipRange { λ: |this: &mut MipRange, other: &MipRange| {*this = this.range(other)} });
+
 impl IntoIterator for MipRange {
     type Item = u32;
 

crates/renderer/src/render_graph/resources.rs

@@ -160,9 +160,8 @@ impl ResourceAccess {
     /// Attempts to fold another ResourceAccess into this one. Returns true if
     /// successful, false if they are incompatible.
     pub fn try_fold(&mut self, other: &Self) -> bool {
-        // if `other` has access flags that aren't in `self`, we can't fold
+        // can only fold if both accesses are reads or both are writes
-        // them.
+        if access_flag_is_read(self.access) != access_flag_is_read(other.access) {
-        if !self.access.contains(other.access) {
             return false;
         }
 
@@ -182,6 +181,7 @@ impl ResourceAccess {
                 // merge stages: we need the barrier to scope all stages that
                 // touch the resource.
                 self.stages |= other.stages;
+                self.access |= other.access;
 
                 true
             }
@@ -201,23 +201,49 @@ impl ResourceAccess {
                     ..
                 },
             ) => {
+                if layout != layout_b {
+                    // layout transition required even on reads, so can't fold if layouts differ.
+                    // TODO: consider if we can still merge `other` into `self`
+                    // if they overlap, and reduce the second barrier to only a
+                    // layout transition.
+                    return false;
+                }
+
                 // two accesses conflict if they overlap;
                 // they overlap if they share mip levels or array layers with
                 // the same aspects.
                 // If one access contains the other, we can merge them (i
                 // think), however: if a previous access requires a less general
                 // barrier, then is it better to split?
-                if !aspect.contains(*aspect_b)
+
-                    || layout != layout_b
+                let overlaps = aspect.intersects(*aspect_b)
-                    || !mip_level.intersects(mip_level_b)
+                    && mip_level.intersects(mip_level_b)
-                    || !array_layers.intersects(array_layers_b)
+                    && array_layers.intersects(array_layers_b);
-                {
+
+                // non-overlapping sub-resources shouldn't be folded.
+                if !overlaps {
                     return false;
                 }
 
-                *aspect = *aspect | *aspect_b;
+                // we know they overlap, but does `other` lie within `self`?
-                *mip_level = mip_level.union(mip_level_b);
+                let b_is_subresource = aspect.contains(*aspect_b)
-                *array_layers = array_layers.union(array_layers_b);
+                    && mip_level.contains(mip_level_b)
+                    && array_layers.contains(array_layers_b);
+
+                // only fold if `other` is a subresource of `self`, otherwise we
+                // might need a more general barrier than `self` requires, and
+                // it's better to split.
+                if !b_is_subresource {
+                    return false;
+                }
+
+                // this should be a no-op?
+
+                // *aspect |= *aspect_b;
+                // *mip_level |= mip_level_b;
+                // *array_layers |= array_layers_b;
+
+                // origin and extent don't matter for barriers.
                 // let lower_left = Offset::from(*origin).min(Offset::from(*origin_b));
                 // let upper_right = (Offset::from(*origin) + Extent::from(*extent).as_offset())
                 //     .max(Offset::from(*origin_b) + Extent::from(*extent_b).as_offset());
@@ -227,6 +253,7 @@ impl ResourceAccess {
                 // merge stages: we need the barrier to scope all stages that
                 // touch the resource.
                 self.stages |= other.stages;
+                self.access |= other.access;
 
                 true
             }