[spv-in] Improve detection of ImageClass

Author: cwfitzgerald

naga/src/front/spv/error.rs

@@ -107,8 +107,10 @@ pub enum Error {
     InvalidGlobalVar(crate::Expression),
     #[error("invalid image/sampler expression {0:?}")]
     InvalidImageExpression(crate::Expression),
-    #[error("image write without format is not currently supported. See https://github.com/gfx-rs/wgpu/issues/6797")]
-    InvalidImageWriteType,
+    #[error("cannot create a OpTypeImage as both a depth and storage image")]
+    InvalidImageDepthStorage,
+    #[error("image read/write without format is not currently supported. See https://github.com/gfx-rs/wgpu/issues/6797")]
+    InvalidStorageImageWithoutFormat,
     #[error("invalid image base type {0:?}")]
     InvalidImageBaseType(Handle<crate::Type>),
     #[error("invalid image {0:?}")]

naga/src/front/spv/mod.rs

@@ -2597,15 +2597,31 @@ impl<I: Iterator<Item = u32>> Frontend<I> {
 
         let inner = crate::TypeInner::Image {
             class: if is_depth == 1 {
+                if is_sampled == 2 {
+                    return Err(Error::InvalidImageDepthStorage);
+                }
+
                 crate::ImageClass::Depth { multi: is_msaa }
-            } else if format != 0 {
+            }
+            // If we have an unknown format and storage texture, this is
+            // StorageRead/WriteWithoutFormat. We don't currently support
+            // this.
+            else if is_sampled == 2 && format == 0 {
+                return Err(Error::InvalidStorageImageWithoutFormat);
+            }
+            // If we have explicit class information (is_sampled = 2 = Storage), use it.
+            //
+            // If we have unknown class information (is_sampled = 0 = Unknown), infer the
+            // class from the presence of an explicit format.
+            else if format != 0 && (is_sampled == 0 || is_sampled == 2) {
                 crate::ImageClass::Storage {
                     format: map_image_format(format)?,
                     access: crate::StorageAccess::default(),
                 }
-            } else if is_sampled == 2 {
-                return Err(Error::InvalidImageWriteType);
-            } else {
+            }
+            // We will hit this case either when sampled is 1, or if we have unknown
+            // sampling information or when sampled is 0 and we have no explicit format.
+            else {
                 crate::ImageClass::Sampled {
                     kind,
                     multi: is_msaa,

naga/src/valid/expression.rs

@@ -87,8 +87,10 @@ pub enum ExpressionError {
     InvalidDerivative,
     #[error("Image array index parameter is misplaced")]
     InvalidImageArrayIndex,
-    #[error("Inappropriate sample or level-of-detail index for texel access")]
-    InvalidImageOtherIndex,
+    #[error("Cannot textureLoad from a specific multisample sample on a non-multisampled image.")]
+    InvalidImageSampleSelector,
+    #[error("Cannot textureLoad from a multisampled image without specifying a sample.")]
+    MissingImageSampleSelector,
     #[error("Image array index type of {0:?} is not an integer scalar")]
     InvalidImageArrayIndexType(Handle<crate::Expression>),
     #[error("Image sample or level-of-detail index's type of {0:?} is not an integer scalar")]
@@ -715,8 +717,11 @@ impl super::Validator {
                             return Err(ExpressionError::InvalidImageOtherIndexType(sample));
                         }
                     }
-                    _ => {
-                        return Err(ExpressionError::InvalidImageOtherIndex);
+                    (Some(_), false) => {
+                        return Err(ExpressionError::InvalidImageSampleSelector);
+                    }
+                    (None, true) => {
+                        return Err(ExpressionError::MissingImageSampleSelector);
                     }
                 }
 
@@ -729,7 +734,8 @@ impl super::Validator {
                         }) => {}
                         _ => return Err(ExpressionError::InvalidImageArrayIndexType(level)),
                     },
-                    _ => {
+                    e => {
+                        std::dbg!(e, class);
                         return Err(ExpressionError::InvalidImageOtherIndex);
                     }
                 }