Merge #191

191: changes for amethyst shadow mapping r=omni-viral a=Frizi

This is a commit with changes I had to make while working on shadow mapping. The set includes:
- fix for broken serde feature after conversion to u32 spirv slices (`serde_bytes_u32`)
- removal of unnecessary assertion on spirv buffer length
- allow adding dynamic nodes to graph
- Allow creating `DescBuilder` with public `new`
- always pick first layer for bound attachments, comment explains why
- allow llvm inlining to better optimize texture conversion common cases

The rest is all pretty much just rustfmt. We probably need to add a CI step that verifies that, as we have quite a lot of noise due to not formatting the code immediately.

Co-authored-by: Frizi <frizi09@gmail.com>

chain/src/collect.rs

@@ -1,6 +1,6 @@
 use std::cmp::max;
-use std::collections::HashMap;
 use std::collections::hash_map::RandomState;
+use std::collections::HashMap;
 use std::hash::Hash;
 use std::ops::Range;
 

chain/src/schedule/submission.rs

@@ -1,6 +1,6 @@
-use std::collections::HashMap;
 use super::queue::QueueId;
 use crate::Id;
+use std::collections::HashMap;
 
 /// Submission id.
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]

command/src/buffer/encoder.rs

@@ -229,12 +229,7 @@ where
         dependencies: gfx_hal::memory::Dependencies,
         barriers: impl IntoIterator<Item = gfx_hal::memory::Barrier<'b, B>>,
     ) {
-        gfx_hal::command::CommandBuffer::pipeline_barrier(
-            self.raw,
-            stages,
-            dependencies,
-            barriers,
-        )
+        gfx_hal::command::CommandBuffer::pipeline_barrier(self.raw, stages, dependencies, barriers)
     }
 
     /// Push graphics constants.

command/src/family/mod.rs

@@ -79,7 +79,8 @@ where
                 let pos = queue_groups.iter().position(|qg| qg.family.0 == id.index);
                 let group = queue_groups.swap_remove(pos.unwrap());
                 assert_eq!(group.queues.len(), count);
-                group.queues
+                group
+                    .queues
                     .into_iter()
                     .enumerate()
                     .map(|(index, queue)| Queue::new(queue, QueueId { family: id, index }))
@@ -222,6 +223,19 @@ where
     pub fn indices(&self) -> &[usize] {
         &self.families_indices
     }
+
+    /// Find family id matching predicate
+    pub fn find<F>(&self, predicate: F) -> Option<FamilyId>
+    where
+        F: FnMut(&&Family<B>) -> bool,
+    {
+        self.families.iter().find(predicate).map(Family::id)
+    }
+
+    /// Get first matching family id with specified capability
+    pub fn with_capability<C: Capability>(&self) -> Option<FamilyId> {
+        self.find(|family| Supports::<C>::supports(&family.capability()).is_some())
+    }
 }
 
 /// Query queue families from device.

command/src/fence.rs

@@ -3,7 +3,7 @@ use {
         family::QueueId,
         util::{device_owned, Device, DeviceId},
     },
-    gfx_hal::{Backend, device::Device as _},
+    gfx_hal::{device::Device as _, Backend},
 };
 
 /// Queue epoch is the point in particluar queue timeline when fence is submitted.

command/src/pool.rs

@@ -2,7 +2,7 @@
 
 use {
     crate::{buffer::*, capability::*, family::FamilyId, util::Device},
-    gfx_hal::{Backend, device::Device as _},
+    gfx_hal::{device::Device as _, Backend},
 };
 
 /// Simple pool wrapper.

descriptor/src/ranges.rs

@@ -74,7 +74,7 @@ impl DescriptorRanges {
     /// Calculate ranges from bindings, specified with an iterator.
     pub fn from_binding_iter<I>(bindings: I) -> Self
     where
-        I: Iterator<Item = DescriptorSetLayoutBinding>
+        I: Iterator<Item = DescriptorSetLayoutBinding>,
     {
         let mut descs = Self::zero();
 

factory/src/blitter.rs

@@ -137,14 +137,22 @@ impl From<BlitRegion> for gfx_hal::command::ImageBlit {
 /// A region and image states for one image in a blit.
 #[derive(Debug, Clone)]
 pub struct BlitImageState {
-    subresource: gfx_hal::image::SubresourceLayers,
-    bounds: Range<gfx_hal::image::Offset>,
-    last_stage: gfx_hal::pso::PipelineStage,
-    last_access: gfx_hal::image::Access,
-    last_layout: gfx_hal::image::Layout,
-    next_stage: gfx_hal::pso::PipelineStage,
-    next_access: gfx_hal::image::Access,
-    next_layout: gfx_hal::image::Layout,
+    /// Subresource to use for blit
+    pub subresource: gfx_hal::image::SubresourceLayers,
+    /// Image offset range to use for blit
+    pub bounds: Range<gfx_hal::image::Offset>,
+    /// Last image stage before blit
+    pub last_stage: gfx_hal::pso::PipelineStage,
+    /// Last image access before blit
+    pub last_access: gfx_hal::image::Access,
+    /// Last image layout before blit
+    pub last_layout: gfx_hal::image::Layout,
+    /// Image stage after blit
+    pub next_stage: gfx_hal::pso::PipelineStage,
+    /// Image access after blit
+    pub next_access: gfx_hal::image::Access,
+    /// Image layout after blit
+    pub next_layout: gfx_hal::image::Layout,
 }
 
 impl<B> Blitter<B>
@@ -299,8 +307,7 @@ pub unsafe fn blit_image<B, C, L>(
     dst_image: &Handle<Image<B>>,
     filter: gfx_hal::image::Filter,
     regions: impl IntoIterator<Item = BlitRegion>,
-)
-where
+) where
     B: gfx_hal::Backend,
     C: Supports<Graphics>,
     L: Level,

factory/src/factory.rs

@@ -10,12 +10,13 @@ use {
         resource::*,
         upload::{BufferState, ImageState, ImageStateOrLayout, Uploader},
         util::{rendy_backend_match, rendy_with_slow_safety_checks, Device, DeviceId, Instance},
-        wsi::{SwapchainError, Surface, Target},
+        wsi::{Surface, SwapchainError, Target},
     },
     gfx_hal::{
-        buffer, format, image,
         adapter::{Adapter, Gpu, PhysicalDevice},
+        buffer,
         device::{AllocationError, Device as _, MapError, OomOrDeviceLost, OutOfMemory, WaitFor},
+        format, image,
         pso::DescriptorSetLayoutBinding,
         window::{Extent2D, Surface as GfxSurface},
         Backend, Features, Limits,
@@ -804,8 +805,11 @@ where
         }
 
         let timeout = !unsafe {
-            self.device
-                .wait_for_fences(fences.iter().map(|f| f.raw()), wait_for.clone(), timeout_ns)
+            self.device.wait_for_fences(
+                fences.iter().map(|f| f.raw()),
+                wait_for.clone(),
+                timeout_ns,
+            )
         }?;
 
         if timeout {
@@ -1122,14 +1126,22 @@ where
 
         log::debug!("Queues: {:#?}", get_queues);
 
-        let Gpu { device, mut queue_groups } = unsafe {
+        let Gpu {
+            device,
+            mut queue_groups,
+        } = unsafe {
             adapter
                 .physical_device
                 .open(&create_queues, adapter.physical_device.features())
         }?;
 
         let families = unsafe {
-            families_from_device(device_id, &mut queue_groups, get_queues, &adapter.queue_families)
+            families_from_device(
+                device_id,
+                &mut queue_groups,
+                get_queues,
+                &adapter.queue_families,
+            )
         };
         (device, families)
     };

graph/src/graph/mod.rs

@@ -5,8 +5,13 @@ use {
         factory::Factory,
         frame::{Fences, Frame, Frames},
         memory::Data,
-        node::{BufferBarrier, DynNode, ImageBarrier, NodeBuffer, NodeBuilder, NodeBuildError, NodeImage},
-        resource::{Buffer, BufferCreationError, BufferInfo, Handle, Image, ImageCreationError, ImageInfo},
+        node::{
+            BufferBarrier, DynNode, ImageBarrier, NodeBuffer, NodeBuildError, NodeBuilder,
+            NodeImage,
+        },
+        resource::{
+            Buffer, BufferCreationError, BufferInfo, Handle, Image, ImageCreationError, ImageInfo,
+        },
         util::{device_owned, DeviceId},
         BufferId, ImageId, NodeId,
     },
@@ -338,9 +343,12 @@ where
 
     /// Add node to the graph.
     pub fn add_node<N: NodeBuilder<B, T> + 'static>(&mut self, builder: N) -> NodeId {
-        profile_scope!("add_node");
+        self.add_dyn_node(Box::new(builder))
+    }
 
-        self.nodes.push(Box::new(builder));
+    /// Add boxed node to the graph.
+    pub fn add_dyn_node(&mut self, builder: Box<dyn NodeBuilder<B, T> + 'static>) -> NodeId {
+        self.nodes.push(builder);
         NodeId(self.nodes.len() - 1)
     }
 
@@ -569,7 +577,7 @@ where
     let images = builder.images();
     chain::Node {
         id,
-        family: QueueFamilyId(builder.family(factory, families.as_slice()).unwrap().index),
+        family: QueueFamilyId(builder.family(factory, families).unwrap().index),
         dependencies: builder.dependencies().into_iter().map(|id| id.0).collect(),
         buffers: buffers
             .into_iter()

graph/src/node/mod.rs

@@ -6,7 +6,7 @@ pub mod render;
 
 use {
     crate::{
-        command::{Capability, Family, FamilyId, Fence, Queue, Submission, Submittable, Supports},
+        command::{Capability, Families, Family, FamilyId, Fence, Queue, Submission, Submittable},
         factory::{Factory, UploadError},
         frame::Frames,
         graph::GraphContext,
@@ -155,26 +155,6 @@ pub trait Node<B: Backend, T: ?Sized>:
     /// Graph will execute this node on command queue that supports this capability level.
     type Capability: Capability;
 
-    /// Description type to instantiate the node.
-    type Desc: NodeDesc<B, T, Node = Self>;
-
-    /// Desc creation.
-    /// Convenient method if `Self::Desc` implements `Default`.
-    fn desc() -> Self::Desc
-    where
-        Self::Desc: Default,
-    {
-        Default::default()
-    }
-
-    /// Builder creation.
-    fn builder() -> DescBuilder<B, T, Self::Desc>
-    where
-        Self::Desc: Default,
-    {
-        Self::desc().builder()
-    }
-
     /// Record commands required by node.
     /// Returned submits are guaranteed to be submitted within specified frame.
     fn run<'a>(
@@ -202,13 +182,7 @@ pub trait NodeDesc<B: Backend, T: ?Sized>: std::fmt::Debug + Sized + 'static {
 
     /// Make node builder.
     fn builder(self) -> DescBuilder<B, T, Self> {
-        DescBuilder {
-            desc: self,
-            buffers: Vec::new(),
-            images: Vec::new(),
-            dependencies: Vec::new(),
-            marker: std::marker::PhantomData,
-        }
+        DescBuilder::new(self)
     }
 
     /// Get set or buffer resources the node uses.
@@ -321,7 +295,7 @@ pub enum NodeBuildError {
 /// Dynamic node builder that emits `DynNode`.
 pub trait NodeBuilder<B: Backend, T: ?Sized>: std::fmt::Debug {
     /// Pick family for this node to be executed onto.
-    fn family(&self, factory: &mut Factory<B>, families: &[Family<B>]) -> Option<FamilyId>;
+    fn family(&self, factory: &mut Factory<B>, families: &Families<B>) -> Option<FamilyId>;
 
     /// Get buffer accessed by the node.
     fn buffers(&self) -> Vec<(BufferId, BufferAccess)>;
@@ -361,6 +335,16 @@ where
     B: Backend,
     T: ?Sized,
 {
+    /// Create new builder out of desc
+    pub fn new(desc: N) -> Self {
+        DescBuilder {
+            desc,
+            buffers: Vec::new(),
+            images: Vec::new(),
+            dependencies: Vec::new(),
+            marker: std::marker::PhantomData,
+        }
+    }
     /// Add buffer to the node.
     /// This method must be called for each buffer node uses.
     pub fn add_buffer(&mut self, buffer: BufferId) -> &mut Self {
@@ -410,14 +394,8 @@ where
     T: ?Sized,
     N: NodeDesc<B, T>,
 {
-    fn family(&self, _factory: &mut Factory<B>, families: &[Family<B>]) -> Option<FamilyId> {
-        families
-            .iter()
-            .find(|family| {
-                Supports::<<N::Node as Node<B, T>>::Capability>::supports(&family.capability())
-                    .is_some()
-            })
-            .map(|family| family.id())
+    fn family(&self, _factory: &mut Factory<B>, families: &Families<B>) -> Option<FamilyId> {
+        families.with_capability::<<N::Node as Node<B, T>>::Capability>()
     }
 
     fn buffers(&self) -> Vec<(BufferId, BufferAccess)> {

graph/src/node/present.rs

@@ -2,15 +2,15 @@
 
 use crate::{
     command::{
-        CommandBuffer, CommandPool, ExecutableState, Family, FamilyId, Fence, MultiShot,
+        CommandBuffer, CommandPool, ExecutableState, Families, Family, FamilyId, Fence, MultiShot,
         PendingState, Queue, SimultaneousUse, Submission, Submit,
     },
     factory::Factory,
     frame::Frames,
     graph::GraphContext,
     node::{
-        gfx_acquire_barriers, gfx_release_barriers, BufferAccess, DynNode,
-        ImageAccess, NodeBuffer, NodeBuilder, NodeBuildError, NodeImage,
+        gfx_acquire_barriers, gfx_release_barriers, BufferAccess, DynNode, ImageAccess, NodeBuffer,
+        NodeBuildError, NodeBuilder, NodeImage,
     },
     wsi::{Surface, Target},
     BufferId, ImageId, NodeId,
@@ -29,7 +29,11 @@ struct ForImage<B: gfx_hal::Backend> {
 }
 
 impl<B: gfx_hal::Backend> ForImage<B> {
-    unsafe fn dispose(self, factory: &Factory<B>, pool: &mut CommandPool<B, gfx_hal::queue::QueueType>) {
+    unsafe fn dispose(
+        self,
+        factory: &Factory<B>,
+        pool: &mut CommandPool<B, gfx_hal::queue::QueueType>,
+    ) {
         drop(self.submit);
         factory.destroy_semaphore(self.acquire);
         factory.destroy_semaphore(self.release);
@@ -355,12 +359,9 @@ where
     B: gfx_hal::Backend,
     T: ?Sized,
 {
-    fn family(&self, factory: &mut Factory<B>, families: &[Family<B>]) -> Option<FamilyId> {
+    fn family(&self, factory: &mut Factory<B>, families: &Families<B>) -> Option<FamilyId> {
         // Find correct queue family.
-        families
-            .iter()
-            .find(|family| factory.surface_support(family.id(), &self.surface))
-            .map(Family::id)
+        families.find(|family| factory.surface_support(family.id(), &self.surface))
     }
 
     fn buffers(&self) -> Vec<(BufferId, BufferAccess)> {
@@ -405,7 +406,11 @@ where
             .into();
 
         if !factory.surface_support(family.id(), &self.surface) {
-            log::warn!("Surface {:?} presentation is unsupported by family {:?} bound to the node", self.surface, family);
+            log::warn!(
+                "Surface {:?} presentation is unsupported by family {:?} bound to the node",
+                self.surface,
+                family
+            );
             return Err(NodeBuildError::QueueFamily(family.id()));
         }
 
@@ -419,7 +424,8 @@ where
             )
             .map_err(NodeBuildError::Swapchain)?;
 
-        let mut pool = factory.create_command_pool(family)
+        let mut pool = factory
+            .create_command_pool(family)
             .map_err(NodeBuildError::OutOfMemory)?;
 
         let per_image = create_per_image_data(

graph/src/node/render/group/simple.rs

@@ -9,7 +9,7 @@ use {
         },
         resource::{DescriptorSetLayout, Handle},
     },
-    gfx_hal::{Backend, device::Device as _},
+    gfx_hal::{device::Device as _, Backend},
 };
 
 pub use crate::util::types::{Layout, SetLayout};