[spv-out] Avoid generating duplicate OpTypeImage instructions.

Fixes #1305.

Ensure that two `back::spv::LocalType::Image` values are sure to be equal (and
hash equal) whenever they would generate the same `OpTypeImage` instruction, so
that the usual duplicate removal via `Writer::lookup_type` works. Accomplish
this by changing the contents of `LocalType::Image` to more closely match the
operands of `OpTypeImage` instructions.

Previously, `LocalType::Image` included a `ImageClass::Storage::access` value,
which did not affect the `OpTypeImage` instruction generated. If two
`LocalType::Image` values differered only in their `access`, then they would get
separate entries in `Writer::lookup_type`, two identical `OpTypeImage`
instructions would be generated, and SPIR-V validation would fail.

src/back/spv/instructions.rs

@@ -219,34 +219,26 @@ impl super::Instruction {
         instruction
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub(super) fn type_image(
         id: Word,
         sampled_type_id: Word,
         dim: spirv::Dim,
+        depth: bool,
         arrayed: bool,
-        image_class: crate::ImageClass,
+        multisampled: bool,
+        sampled: bool,
+        image_format: spirv::ImageFormat,
     ) -> Self {
         let mut instruction = Self::new(Op::TypeImage);
         instruction.set_result(id);
         instruction.add_operand(sampled_type_id);
         instruction.add_operand(dim as u32);
-
-        let (depth, multi, sampled) = match image_class {
-            crate::ImageClass::Sampled { kind: _, multi } => (false, multi, true),
-            crate::ImageClass::Depth { multi } => (true, multi, true),
-            crate::ImageClass::Storage { .. } => (false, false, false),
-        };
         instruction.add_operand(depth as u32);
         instruction.add_operand(arrayed as u32);
-        instruction.add_operand(multi as u32);
+        instruction.add_operand(multisampled as u32);
         instruction.add_operand(if sampled { 1 } else { 2 });
-
-        let format = match image_class {
-            crate::ImageClass::Storage { format, .. } => format.into(),
-            _ => spirv::ImageFormat::Unknown,
-        };
-
-        instruction.add_operand(format as u32);
+        instruction.add_operand(image_format as u32);
         instruction
     }
 

src/back/spv/mod.rs

@@ -153,6 +153,70 @@ impl Function {
     }
 }
 
+fn map_dim(dim: crate::ImageDimension) -> spirv::Dim {
+    match dim {
+        crate::ImageDimension::D1 => spirv::Dim::Dim1D,
+        crate::ImageDimension::D2 => spirv::Dim::Dim2D,
+        crate::ImageDimension::D3 => spirv::Dim::Dim3D,
+        crate::ImageDimension::Cube => spirv::Dim::DimCube,
+    }
+}
+
+/// Characteristics of a SPIR-V `OpTypeImage` type.
+///
+/// SPIR-V requires non-composite types to be unique, including images. Since we
+/// use `LocalType` for this deduplication, it's essential that `LocalImageType`
+/// be equal whenever the corresponding `OpTypeImage`s would be. To reduce the
+/// likelihood of mistakes, we use fields that correspond exactly to the
+/// operands of an `OpTypeImage` instruction, using the actual SPIR-V types
+/// where practical.
+#[derive(Debug, PartialEq, Hash, Eq, Copy, Clone)]
+struct LocalImageType {
+    sampled_type: crate::ScalarKind,
+    dim: spirv::Dim,
+    depth: bool,
+    arrayed: bool,
+    multisampled: bool,
+    sampled: bool,
+    image_format: spirv::ImageFormat,
+}
+
+impl LocalImageType {
+    /// Construct a `LocalImageType` from the fields of a `TypeInner::Image`.
+    fn from_inner(dim: crate::ImageDimension, arrayed: bool, class: crate::ImageClass) -> Self {
+        let dim = map_dim(dim);
+        match class {
+            crate::ImageClass::Sampled { kind, multi } => LocalImageType {
+                sampled_type: kind,
+                dim,
+                depth: false,
+                arrayed,
+                multisampled: multi,
+                sampled: true,
+                image_format: spirv::ImageFormat::Unknown,
+            },
+            crate::ImageClass::Depth { multi } => LocalImageType {
+                sampled_type: crate::ScalarKind::Float,
+                dim,
+                depth: true,
+                arrayed,
+                multisampled: multi,
+                sampled: true,
+                image_format: spirv::ImageFormat::Unknown,
+            },
+            crate::ImageClass::Storage { format, access: _ } => LocalImageType {
+                sampled_type: crate::ScalarKind::from(format),
+                dim,
+                depth: false,
+                arrayed,
+                multisampled: false,
+                sampled: false,
+                image_format: format.into(),
+            },
+        }
+    }
+}
+
 /// A SPIR-V type constructed during code generation.
 ///
 /// In the process of writing SPIR-V, we need to synthesize various types for
@@ -189,11 +253,7 @@ enum LocalType {
         base: Handle<crate::Type>,
         class: spirv::StorageClass,
     },
-    Image {
-        dim: crate::ImageDimension,
-        arrayed: bool,
-        class: crate::ImageClass,
-    },
+    Image(LocalImageType),
     SampledImage {
         image_type_id: Word,
     },
@@ -262,11 +322,7 @@ fn make_local(inner: &crate::TypeInner) -> Option<LocalType> {
             dim,
             arrayed,
             class,
-        } => LocalType::Image {
-            dim,
-            arrayed,
-            class,
-        },
+        } => LocalType::Image(LocalImageType::from_inner(dim, arrayed, class)),
         crate::TypeInner::Sampler { comparison: _ } => LocalType::Sampler,
         _ => return None,
     })

src/back/spv/writer.rs

@@ -13,15 +13,6 @@ use crate::{
 use spirv::Word;
 use std::collections::hash_map::Entry;
 
-fn map_dim(dim: crate::ImageDimension) -> spirv::Dim {
-    match dim {
-        crate::ImageDimension::D1 => spirv::Dim::Dim1D,
-        crate::ImageDimension::D2 => spirv::Dim::Dim2D,
-        crate::ImageDimension::D3 => spirv::Dim::Dim3D,
-        crate::ImageDimension::Cube => spirv::Dim::DimCube,
-    }
-}
-
 impl Function {
     fn to_words(&self, sink: &mut impl Extend<Word>) {
         self.signature.as_ref().unwrap().to_words(sink);
@@ -688,25 +679,24 @@ impl Writer {
                 }));
                 Instruction::type_pointer(id, class, type_id)
             }
-            LocalType::Image {
-                dim,
-                arrayed,
-                class,
-            } => {
-                let kind = match class {
-                    crate::ImageClass::Sampled { kind, multi: _ } => kind,
-                    crate::ImageClass::Depth { multi: _ } => crate::ScalarKind::Float,
-                    crate::ImageClass::Storage { format, .. } => format.into(),
-                };
+            LocalType::Image(image) => {
                 let local_type = LocalType::Value {
                     vector_size: None,
-                    kind,
+                    kind: image.sampled_type,
                     width: 4,
                     pointer_class: None,
                 };
                 let type_id = self.get_type_id(LookupType::Local(local_type));
-                let dim = map_dim(dim);
-                Instruction::type_image(id, type_id, dim, arrayed, class)
+                Instruction::type_image(
+                    id,
+                    type_id,
+                    image.dim,
+                    image.depth,
+                    image.arrayed,
+                    image.multisampled,
+                    image.sampled,
+                    image.image_format,
+                )
             }
             LocalType::Sampler => Instruction::type_sampler(id),
             LocalType::SampledImage { image_type_id } => {