Skip to content

Commit

Permalink
layers: Fix warning for Storage Image
Browse files Browse the repository at this point in the history
  • Loading branch information
spencer-lunarg committed Feb 26, 2025
1 parent 68535ea commit 11c5a1f
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 22 deletions.
49 changes: 35 additions & 14 deletions layers/drawdispatch/descriptor_validator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "descriptor_validator.h"
#include <vulkan/vulkan_core.h>
#include <sstream>
#include "generated/spirv_grammar_helper.h"
#include "state_tracker/shader_stage_state.h"
#include "error_message/error_strings.h"
Expand Down Expand Up @@ -387,15 +388,26 @@ 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 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);
Expand Down Expand Up @@ -941,15 +953,24 @@ 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 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);
Expand Down
8 changes: 5 additions & 3 deletions tests/unit/shader_storage_image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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); }
46 changes: 42 additions & 4 deletions tests/unit/shader_storage_image_positive.cpp
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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 {};

Expand Down Expand Up @@ -365,3 +366,40 @@ TEST_F(PositiveShaderStorageImage, UnknownWriteLessComponentMultiEntrypoint) {
m_command_buffer.EndRenderPass();
m_command_buffer.End();
}

TEST_F(PositiveShaderStorageImage, FormatComponentTypeMismatch) {
TEST_DESCRIPTION("It is valid if the formats are not exact as long as they are compatibile");
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();
}
2 changes: 1 addition & 1 deletion tests/unit/shader_storage_texel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 11c5a1f

Please sign in to comment.