layers: Fix warning for Storage Image

layers/drawdispatch/descriptor_validator.cpp

@@ -17,7 +17,9 @@
 
 #include "descriptor_validator.h"
 #include <vulkan/vulkan_core.h>
+#include <sstream>
 #include "generated/spirv_grammar_helper.h"
+#include "generated/spirv_validation_helper.h"
 #include "state_tracker/shader_stage_state.h"
 #include "error_message/error_strings.h"
 #include "state_tracker/descriptor_sets.h"
@@ -387,15 +389,27 @@ bool DescriptorValidator::ValidateDescriptor(const spirv::ResourceInterfaceVaria
 
         if (image_view_ci.format != VK_FORMAT_UNDEFINED && resource_variable.info.image_format != VK_FORMAT_UNDEFINED &&
             image_view_ci.format != resource_variable.info.image_format) {
+            // This warning was added after being discussed in https://gitlab.khronos.org/vulkan/vulkan/-/issues/4128
             auto set = descriptor_set.Handle();
             const LogObjectList objlist(set, image_view);
-            skip |= dev_state.LogUndefinedValue(
-                "Undefined-Value-StorageImage-FormatMismatch", objlist, loc,
-                "the %s is accessed by a OpTypeImage that has a format operand of %s which doesn't match the %s format (%s). Any "
-                "loads or stores with the variable will produce undefined values.",
-                DescribeDescriptor(resource_variable, index, descriptor_type).c_str(),
-                string_VkFormat(resource_variable.info.image_format), dev_state.FormatHandle(image_view).c_str(),
-                string_VkFormat(image_view_ci.format));
+            std::stringstream msg;
+            msg << "the " << DescribeDescriptor(resource_variable, index, descriptor_type)
+                << " is accessed by a OpTypeImage that has a Format operand "
+                << string_SpirvImageFormat(resource_variable.info.image_format) << " (equivalent to "
+                << string_VkFormat(resource_variable.info.image_format) << ") which doesn't match the "
+                << dev_state.FormatHandle(image_view) << " format (" << string_VkFormat(image_view_ci.format)
+                << "). Any loads or stores with the variable will produce undefined values.";
+            if (vkuFormatCompatibilityClass(image_view_ci.format) ==
+                vkuFormatCompatibilityClass(resource_variable.info.image_format)) {
+                msg << " While the formats are compatible, Storage Images must exactly match. Two ways to resolve this are\n";
+                msg << "1. Set your ImageView to " << string_VkFormat(resource_variable.info.image_format)
+                    << " and swizzle the values in the shader to match the desired results.\n";
+                msg << "2. Use the Unknown format in your shader (will need the widely supported "
+                       "shaderStorageImageWriteWithoutFormat feature)";
+            }
+            msg << "\nSpec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation";
+            skip |= dev_state.LogUndefinedValue("Undefined-Value-StorageImage-FormatMismatch-ImageView", objlist, loc, "%s",
+                                                msg.str().c_str());
         }
 
         const bool image_format_width_64 = vkuFormatHasComponentSize(image_view_ci.format, 64);
@@ -941,15 +955,25 @@ bool DescriptorValidator::ValidateDescriptor(const spirv::ResourceInterfaceVaria
 
     if (buffer_view_format != VK_FORMAT_UNDEFINED && resource_variable.info.image_format != VK_FORMAT_UNDEFINED &&
         buffer_view_format != resource_variable.info.image_format) {
+        // This warning was added after being discussed in https://gitlab.khronos.org/vulkan/vulkan/-/issues/4128
         auto set = descriptor_set.Handle();
         const LogObjectList objlist(set, buffer_view);
-        skip |= dev_state.LogUndefinedValue(
-            "Undefined-Value-StorageImage-FormatMismatch", objlist, loc,
-            "the %s is accessed by a OpTypeImage that has a format operand of %s which doesn't match the %s format (%s). Any loads "
-            "or stores with the variable will produce undefined values.",
-            DescribeDescriptor(resource_variable, index, descriptor_type).c_str(),
-            string_VkFormat(resource_variable.info.image_format), dev_state.FormatHandle(buffer_view).c_str(),
-            string_VkFormat(buffer_view_format));
+        std::stringstream msg;
+        msg << "the " << DescribeDescriptor(resource_variable, index, descriptor_type)
+            << " is accessed by a OpTypeImage that has a Format operand "
+            << string_SpirvImageFormat(resource_variable.info.image_format) << " (equivalent to "
+            << string_VkFormat(resource_variable.info.image_format) << ") which doesn't match the "
+            << dev_state.FormatHandle(buffer_view) << " format (" << string_VkFormat(buffer_view_format)
+            << "). Any loads or stores with the variable will produce undefined values.";
+        if (vkuFormatCompatibilityClass(buffer_view_format) == vkuFormatCompatibilityClass(resource_variable.info.image_format)) {
+            msg << " While the formats are compatible, Texel Buffers must exactly match. Two ways to resolve this are\n";
+            msg << "1. Set your ImageView to " << string_VkFormat(resource_variable.info.image_format)
+                << " and swizzle the values in the shader to match the desired results.\n";
+            msg << "2. Use the Unknown format in your shader";
+        }
+        msg << "\nSpec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation";
+        skip |= dev_state.LogUndefinedValue("Undefined-Value-StorageImage-FormatMismatch-BufferView", objlist, loc, "%s",
+                                            msg.str().c_str());
     }
 
     const bool buffer_format_width_64 = vkuFormatHasComponentSize(buffer_view_format, 64);

layers/vulkan/generated/spirv_validation_helper.cpp

@@ -882,6 +882,95 @@ VkFormat CompatibleSpirvImageFormat(uint32_t spirv_image_format) {
     };
 }
 
+const char *string_SpirvImageFormat(VkFormat format) {
+    switch (format) {
+        case VK_FORMAT_R8_UNORM:
+            return "R8";
+        case VK_FORMAT_R8_SNORM:
+            return "R8Snorm";
+        case VK_FORMAT_R8_UINT:
+            return "R8ui";
+        case VK_FORMAT_R8_SINT:
+            return "R8i";
+        case VK_FORMAT_R8G8_UNORM:
+            return "Rg8";
+        case VK_FORMAT_R8G8_SNORM:
+            return "Rg8Snorm";
+        case VK_FORMAT_R8G8_UINT:
+            return "Rg8ui";
+        case VK_FORMAT_R8G8_SINT:
+            return "Rg8i";
+        case VK_FORMAT_R8G8B8A8_UNORM:
+            return "Rgba8";
+        case VK_FORMAT_R8G8B8A8_SNORM:
+            return "Rgba8Snorm";
+        case VK_FORMAT_R8G8B8A8_UINT:
+            return "Rgba8ui";
+        case VK_FORMAT_R8G8B8A8_SINT:
+            return "Rgba8i";
+        case VK_FORMAT_A2B10G10R10_UNORM_PACK32:
+            return "Rgb10A2";
+        case VK_FORMAT_A2B10G10R10_UINT_PACK32:
+            return "Rgb10a2ui";
+        case VK_FORMAT_R16_UNORM:
+            return "R16";
+        case VK_FORMAT_R16_SNORM:
+            return "R16Snorm";
+        case VK_FORMAT_R16_UINT:
+            return "R16ui";
+        case VK_FORMAT_R16_SINT:
+            return "R16i";
+        case VK_FORMAT_R16_SFLOAT:
+            return "R16f";
+        case VK_FORMAT_R16G16_UNORM:
+            return "Rg16";
+        case VK_FORMAT_R16G16_SNORM:
+            return "Rg16Snorm";
+        case VK_FORMAT_R16G16_UINT:
+            return "Rg16ui";
+        case VK_FORMAT_R16G16_SINT:
+            return "Rg16i";
+        case VK_FORMAT_R16G16_SFLOAT:
+            return "Rg16f";
+        case VK_FORMAT_R16G16B16A16_UNORM:
+            return "Rgba16";
+        case VK_FORMAT_R16G16B16A16_SNORM:
+            return "Rgba16Snorm";
+        case VK_FORMAT_R16G16B16A16_UINT:
+            return "Rgba16ui";
+        case VK_FORMAT_R16G16B16A16_SINT:
+            return "Rgba16i";
+        case VK_FORMAT_R16G16B16A16_SFLOAT:
+            return "Rgba16f";
+        case VK_FORMAT_R32_UINT:
+            return "R32ui";
+        case VK_FORMAT_R32_SINT:
+            return "R32i";
+        case VK_FORMAT_R32_SFLOAT:
+            return "R32f";
+        case VK_FORMAT_R32G32_UINT:
+            return "Rg32ui";
+        case VK_FORMAT_R32G32_SINT:
+            return "Rg32i";
+        case VK_FORMAT_R32G32_SFLOAT:
+            return "Rg32f";
+        case VK_FORMAT_R32G32B32A32_UINT:
+            return "Rgba32ui";
+        case VK_FORMAT_R32G32B32A32_SINT:
+            return "Rgba32i";
+        case VK_FORMAT_R32G32B32A32_SFLOAT:
+            return "Rgba32f";
+        case VK_FORMAT_R64_UINT:
+            return "R64ui";
+        case VK_FORMAT_R64_SINT:
+            return "R64i";
+        case VK_FORMAT_B10G11R11_UFLOAT_PACK32:
+            return "R11fG11fB10f";
+        default:
+            return "Unknown SPIR-V Format";
+    };
+}
+
 // clang-format off
 static inline const char* SpvCapabilityRequirements(uint32_t capability) {
     static const vvl::unordered_map<uint32_t, std::string_view> table {

layers/vulkan/generated/spirv_validation_helper.h

@@ -29,5 +29,8 @@
 
 // This is the one function that requires mapping SPIR-V enums to Vulkan enums
 VkFormat CompatibleSpirvImageFormat(uint32_t spirv_image_format);
+// Since we keep things in VkFormat for checking, we need a way to get the original SPIR-V
+// Format name for any error message
+const char* string_SpirvImageFormat(VkFormat format);
 
 // NOLINTEND

scripts/generators/spirv_validation_generator.py

@@ -158,6 +158,9 @@ def generateHeader(self):
 
             // This is the one function that requires mapping SPIR-V enums to Vulkan enums
             VkFormat CompatibleSpirvImageFormat(uint32_t spirv_image_format);
+            // Since we keep things in VkFormat for checking, we need a way to get the original SPIR-V
+            // Format name for any error message
+            const char* string_SpirvImageFormat(VkFormat format);
         ''')
 
         self.write("".join(out))
@@ -270,6 +273,17 @@ def generateSource(self):
         out.append('    };\n')
         out.append('}\n')
 
+        out.append('''
+            const char* string_SpirvImageFormat(VkFormat format) {
+                switch (format) {
+            ''')
+        for format in [x for x in self.vk.formats.values() if x.spirvImageFormat]:
+            out.append(f'        case {format.name}:\n')
+            out.append(f'            return \"{format.spirvImageFormat}\";\n')
+        out.append('        default:\n')
+        out.append('            return "Unknown SPIR-V Format";\n')
+        out.append('    };\n')
+        out.append('}\n')
 
         out.append('''
 // clang-format off

tests/unit/shader_storage_image.cpp

@@ -911,7 +911,7 @@ void NegativeShaderStorageImage::FormatComponentMismatchTest(std::string spirv_f
     vk::CmdBindDescriptorSets(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout.handle(), 0, 1,
                               &descriptor_set.set_, 0, nullptr);
 
-    m_errorMonitor->SetDesiredWarning("Undefined-Value-StorageImage-FormatMismatch");
+    m_errorMonitor->SetDesiredWarning("Undefined-Value-StorageImage-FormatMismatch-ImageView");
     vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
     m_errorMonitor->VerifyFound();
     m_command_buffer.End();
@@ -923,6 +923,8 @@ TEST_F(NegativeShaderStorageImage, FormatComponentTypeMismatch) {
 
 TEST_F(NegativeShaderStorageImage, FormatComponentSizeMismatch) { FormatComponentMismatchTest("Rgba8", VK_FORMAT_R8_UNORM); }
 
-TEST_F(NegativeShaderStorageImage, FormatComponentNumericMismatch) {
-    FormatComponentMismatchTest("Rgba8", VK_FORMAT_R8G8B8A8_SNORM);
+TEST_F(NegativeShaderStorageImage, FormatComponentWidthMismatch) {
+    FormatComponentMismatchTest("Rgba8", VK_FORMAT_R32G32B32A32_SFLOAT);
 }
+
+TEST_F(NegativeShaderStorageImage, FormatCompatibleMismatch) { FormatComponentMismatchTest("Rgba8", VK_FORMAT_B8G8R8A8_UNORM); }

tests/unit/shader_storage_image_positive.cpp

@@ -1,8 +1,8 @@
 /*
- * Copyright (c) 2015-2024 The Khronos Group Inc.
- * Copyright (c) 2015-2024 Valve Corporation
- * Copyright (c) 2015-2024 LunarG, Inc.
- * Copyright (c) 2015-2024 Google, Inc.
+ * Copyright (c) 2015-2025 The Khronos Group Inc.
+ * Copyright (c) 2015-2025 Valve Corporation
+ * Copyright (c) 2015-2025 LunarG, Inc.
+ * Copyright (c) 2015-2025 Google, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,6 +14,7 @@
 #include "../framework/layer_validation_tests.h"
 #include "../framework/pipeline_helper.h"
 #include "../framework/descriptor_helper.h"
+#include "error_message/log_message_type.h"
 
 class PositiveShaderStorageImage : public VkLayerTest {};
 
@@ -365,3 +366,40 @@ TEST_F(PositiveShaderStorageImage, UnknownWriteLessComponentMultiEntrypoint) {
     m_command_buffer.EndRenderPass();
     m_command_buffer.End();
 }
+
+TEST_F(PositiveShaderStorageImage, FormatTypeMatch) {
+    TEST_DESCRIPTION("Will not produce warning as formats are the same");
+    RETURN_IF_SKIP(Init());
+    m_errorMonitor->ExpectSuccess(kErrorBit | kWarningBit);
+    std::string cs_source = R"glsl(
+        #version 450
+        layout(set = 0, binding = 0, Rgba8) uniform image2D si0;
+        void main() {
+            imageStore(si0, ivec2(0), vec4(0));
+        }
+    )glsl";
+
+    vkt::Image image(*m_device, 4, 4, 1, VK_FORMAT_R8G8B8A8_UNORM, VK_IMAGE_USAGE_STORAGE_BIT);
+    image.SetLayout(VK_IMAGE_LAYOUT_GENERAL);
+    vkt::ImageView image_view = image.CreateView();
+
+    OneOffDescriptorSet descriptor_set(m_device, {
+                                                     {0, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, 1, VK_SHADER_STAGE_ALL, nullptr},
+                                                 });
+    vkt::PipelineLayout pipeline_layout(*m_device, {&descriptor_set.layout_});
+    descriptor_set.WriteDescriptorImageInfo(0, image_view, VK_NULL_HANDLE, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE,
+                                            VK_IMAGE_LAYOUT_GENERAL);
+    descriptor_set.UpdateDescriptorSets();
+
+    CreateComputePipelineHelper pipe(*this);
+    pipe.cs_ = std::make_unique<VkShaderObj>(this, cs_source.c_str(), VK_SHADER_STAGE_COMPUTE_BIT);
+    pipe.cp_ci_.layout = pipeline_layout.handle();
+    pipe.CreateComputePipeline();
+
+    m_command_buffer.Begin();
+    vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe.Handle());
+    vk::CmdBindDescriptorSets(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout.handle(), 0, 1,
+                              &descriptor_set.set_, 0, nullptr);
+    vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
+    m_errorMonitor->VerifyFound();
+}

tests/unit/shader_storage_texel.cpp

@@ -235,7 +235,7 @@ TEST_F(NegativeShaderStorageTexel, FormatComponentTypeMismatch) {
     vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe.Handle());
     vk::CmdBindDescriptorSets(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout, 0, 1,
                               &descriptor_set.set_, 0, nullptr);
-    m_errorMonitor->SetDesiredWarning("Undefined-Value-StorageImage-FormatMismatch");
+    m_errorMonitor->SetDesiredWarning("Undefined-Value-StorageImage-FormatMismatch-BufferView");
     vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
     m_errorMonitor->VerifyFound();
     m_command_buffer.End();