diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index 57730322b0..a14e53c9fd 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -67,8 +67,10 @@ namespace dawn_native { case wgpu::BindingType::WriteonlyStorageTexture: { DAWN_TRY(ValidateTextureFormat(storageTextureFormat)); - const Format& format = device->GetValidInternalFormat(storageTextureFormat); - if (!format.supportsStorageUsage) { + const Format* format = nullptr; + DAWN_TRY_ASSIGN(format, device->GetInternalFormat(storageTextureFormat)); + ASSERT(format != nullptr); + if (!format->supportsStorageUsage) { return DAWN_VALIDATION_ERROR("The storage texture format is not supported"); } } break; @@ -170,7 +172,8 @@ namespace dawn_native { for (BindingIndex i = 0; i < info.bindingCount; ++i) { HashCombine(&hash, info.visibilities[i], info.types[i], - info.textureComponentTypes[i], info.textureDimensions[i]); + info.textureComponentTypes[i], info.textureDimensions[i], + info.storageTextureFormats[i]); } return hash; @@ -186,7 +189,8 @@ namespace dawn_native { for (BindingIndex i = 0; i < a.bindingCount; ++i) { if ((a.visibilities[i] != b.visibilities[i]) || (a.types[i] != b.types[i]) || (a.textureComponentTypes[i] != b.textureComponentTypes[i]) || - (a.textureDimensions[i] != b.textureDimensions[i])) { + (a.textureDimensions[i] != b.textureDimensions[i]) || + (a.storageTextureFormats[i] != b.storageTextureFormats[i])) { return false; } } @@ -275,6 +279,7 @@ namespace dawn_native { mBindingInfo.types[i] = binding.type; mBindingInfo.visibilities[i] = binding.visibility; mBindingInfo.textureComponentTypes[i] = binding.textureComponentType; + mBindingInfo.storageTextureFormats[i] = binding.storageTextureFormat; switch (binding.type) { case wgpu::BindingType::UniformBuffer: diff --git a/src/dawn_native/BindGroupLayout.h b/src/dawn_native/BindGroupLayout.h index ed96c27260..c110f6ac2f 100644 --- a/src/dawn_native/BindGroupLayout.h +++ b/src/dawn_native/BindGroupLayout.h @@ -57,6 +57,7 @@ namespace dawn_native { std::array types; std::array textureComponentTypes; std::array textureDimensions; + std::array storageTextureFormats; std::bitset hasDynamicOffset; std::bitset multisampled; BindingIndex bindingCount; diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index a280e56b3a..8732a51520 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -387,14 +387,16 @@ namespace dawn_native { BindingInfo* info = &it.first->second; info->id = binding.id; info->base_type_id = binding.base_type_id; - if (binding.binding_type == shaderc_spvc_binding_type_sampled_texture) { - info->multisampled = binding.multisampled; - info->textureDimension = ToWGPUTextureViewDimension(binding.texture_dimension); - info->textureComponentType = ToDawnFormatType(binding.texture_component_type); - } info->type = ToWGPUBindingType(binding.binding_type); switch (info->type) { + case wgpu::BindingType::SampledTexture: { + info->multisampled = binding.multisampled; + info->textureDimension = + ToWGPUTextureViewDimension(binding.texture_dimension); + info->textureComponentType = + ToDawnFormatType(binding.texture_component_type); + } break; case wgpu::BindingType::StorageTexture: case wgpu::BindingType::ReadonlyStorageTexture: case wgpu::BindingType::WriteonlyStorageTexture: { @@ -410,7 +412,11 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR( "The storage texture format is not supported"); } + // TODO(jiawei.shao@intel.com): extract info->multisampled when it is + // supported for storage images in SPVC. info->storageTextureFormat = storageTextureFormat; + info->textureDimension = + ToWGPUTextureViewDimension(binding.texture_dimension); } break; default: break; @@ -610,6 +616,8 @@ namespace dawn_native { "The storage texture format is not supported"); } info->storageTextureFormat = storageTextureFormat; + info->textureDimension = + SpirvDimToTextureViewDimension(imageType.dim, imageType.arrayed); } break; default: info->type = bindingType; @@ -767,15 +775,44 @@ namespace dawn_native { return false; } - if (layoutBindingType == wgpu::BindingType::SampledTexture) { - Format::Type layoutTextureComponentType = Format::TextureComponentTypeToFormatType( - layoutInfo.textureComponentTypes[bindingIndex]); - if (layoutTextureComponentType != moduleInfo.textureComponentType) { + switch (layoutBindingType) { + case wgpu::BindingType::SampledTexture: { + Format::Type layoutTextureComponentType = + Format::TextureComponentTypeToFormatType( + layoutInfo.textureComponentTypes[bindingIndex]); + if (layoutTextureComponentType != moduleInfo.textureComponentType) { + return false; + } + + if (layoutInfo.textureDimensions[bindingIndex] != moduleInfo.textureDimension) { + return false; + } + } break; + + case wgpu::BindingType::ReadonlyStorageTexture: + case wgpu::BindingType::WriteonlyStorageTexture: { + ASSERT(layoutInfo.storageTextureFormats[bindingIndex] != + wgpu::TextureFormat::Undefined); + ASSERT(moduleInfo.storageTextureFormat != wgpu::TextureFormat::Undefined); + if (layoutInfo.storageTextureFormats[bindingIndex] != + moduleInfo.storageTextureFormat) { + return false; + } + if (layoutInfo.textureDimensions[bindingIndex] != moduleInfo.textureDimension) { + return false; + } + } break; + + case wgpu::BindingType::UniformBuffer: + case wgpu::BindingType::ReadonlyStorageBuffer: + case wgpu::BindingType::StorageBuffer: + case wgpu::BindingType::Sampler: + break; + + case wgpu::BindingType::StorageTexture: + default: + UNREACHABLE(); return false; - } - if (layoutInfo.textureDimensions[bindingIndex] != moduleInfo.textureDimension) { - return false; - } } } diff --git a/src/tests/unittests/validation/StorageTextureValidationTests.cpp b/src/tests/unittests/validation/StorageTextureValidationTests.cpp index 80957b087c..3927d4dd5c 100644 --- a/src/tests/unittests/validation/StorageTextureValidationTests.cpp +++ b/src/tests/unittests/validation/StorageTextureValidationTests.cpp @@ -92,20 +92,44 @@ class StorageTextureValidationTests : public ValidationTest { } } + static const char* GetGLSLFloatImageTypeDeclaration(wgpu::TextureViewDimension dimension) { + switch (dimension) { + case wgpu::TextureViewDimension::e1D: + return "image1D"; + case wgpu::TextureViewDimension::e2D: + return "image2D"; + case wgpu::TextureViewDimension::e2DArray: + return "image2DArray"; + case wgpu::TextureViewDimension::Cube: + return "imageCube"; + case wgpu::TextureViewDimension::CubeArray: + return "imageCubeArray"; + case wgpu::TextureViewDimension::e3D: + return "image3D"; + case wgpu::TextureViewDimension::Undefined: + default: + UNREACHABLE(); + return ""; + } + } + static std::string CreateComputeShaderWithStorageTexture( wgpu::BindingType storageTextureBindingType, - wgpu::TextureFormat textureFormat) { + wgpu::TextureFormat textureFormat, + wgpu::TextureViewDimension textureViewDimension = wgpu::TextureViewDimension::e2D) { const char* glslImageFormatQualifier = GetGLSLImageFormatQualifier(textureFormat); const char* textureComponentTypePrefix = utils::GetColorTextureComponentTypePrefix(textureFormat); return CreateComputeShaderWithStorageTexture( - storageTextureBindingType, glslImageFormatQualifier, textureComponentTypePrefix); + storageTextureBindingType, glslImageFormatQualifier, textureComponentTypePrefix, + GetGLSLFloatImageTypeDeclaration(textureViewDimension)); } static std::string CreateComputeShaderWithStorageTexture( wgpu::BindingType storageTextureBindingType, const char* glslImageFormatQualifier, - const char* textureComponentTypePrefix) { + const char* textureComponentTypePrefix, + const char* glslImageTypeDeclaration = "image2D") { const char* memoryQualifier = ""; switch (storageTextureBindingType) { case wgpu::BindingType::ReadonlyStorageTexture: @@ -123,8 +147,8 @@ class StorageTextureValidationTests : public ValidationTest { ostream << "#version 450\n" "layout (set = 0, binding = 0, " << glslImageFormatQualifier << ") uniform " << memoryQualifier << " " - << textureComponentTypePrefix - << "image2D image0;\n" + << textureComponentTypePrefix << glslImageTypeDeclaration + << " image0;\n" "void main() {\n" "}\n"; @@ -144,6 +168,9 @@ class StorageTextureValidationTests : public ValidationTest { void main() { fragColor = vec4(1.f, 0.f, 0.f, 1.f); })"); + + const std::array kSupportedStorageTextureBindingTypes = { + wgpu::BindingType::ReadonlyStorageTexture, wgpu::BindingType::WriteonlyStorageTexture}; }; // Validate read-only storage textures can be declared in vertex and fragment @@ -376,10 +403,7 @@ TEST_F(StorageTextureValidationTests, StorageTextureFormatInShaders) { wgpu::TextureFormat::RG16Sint, wgpu::TextureFormat::RG16Float, wgpu::TextureFormat::RGB10A2Unorm, wgpu::TextureFormat::RG11B10Float}; - constexpr std::array kStorageTextureBindingTypes = { - wgpu::BindingType::ReadonlyStorageTexture, wgpu::BindingType::WriteonlyStorageTexture}; - - for (wgpu::BindingType storageTextureBindingType : kStorageTextureBindingTypes) { + for (wgpu::BindingType storageTextureBindingType : kSupportedStorageTextureBindingTypes) { for (wgpu::TextureFormat format : kWGPUTextureFormatSupportedAsSPIRVImageFormats) { std::string computeShader = CreateComputeShaderWithStorageTexture(storageTextureBindingType, format); @@ -410,10 +434,7 @@ TEST_F(StorageTextureValidationTests, UnsupportedSPIRVStorageTextureFormat) { {"r16_snorm", ""}, {"rgb10_a2ui", "u"}}}; - constexpr std::array kStorageTextureBindingTypes = { - wgpu::BindingType::ReadonlyStorageTexture, wgpu::BindingType::WriteonlyStorageTexture}; - - for (wgpu::BindingType bindingType : kStorageTextureBindingTypes) { + for (wgpu::BindingType bindingType : kSupportedStorageTextureBindingTypes) { for (const TextureFormatInfo& formatInfo : kUnsupportedTextureFormats) { std::string computeShader = CreateComputeShaderWithStorageTexture( bindingType, formatInfo.name, formatInfo.componentTypePrefix); @@ -422,3 +443,197 @@ TEST_F(StorageTextureValidationTests, UnsupportedSPIRVStorageTextureFormat) { } } } + +// Verify when we create and use a bind group layout with storage textures in the creation of +// render and compute pipeline, the binding type in the bind group layout must match the +// declaration in the shader. +TEST_F(StorageTextureValidationTests, BindGroupLayoutBindingTypeMatchesShaderDeclaration) { + constexpr std::array kSupportedBindingTypes = { + wgpu::BindingType::UniformBuffer, wgpu::BindingType::StorageBuffer, + wgpu::BindingType::ReadonlyStorageBuffer, wgpu::BindingType::Sampler, + wgpu::BindingType::SampledTexture, wgpu::BindingType::ReadonlyStorageTexture, + wgpu::BindingType::WriteonlyStorageTexture}; + constexpr wgpu::TextureFormat kStorageTextureFormat = wgpu::TextureFormat::R32Float; + + for (wgpu::BindingType bindingTypeInShader : kSupportedStorageTextureBindingTypes) { + // Create the compute shader with the given binding type. + std::string computeShader = + CreateComputeShaderWithStorageTexture(bindingTypeInShader, kStorageTextureFormat); + wgpu::ShaderModule csModule = utils::CreateShaderModule( + device, utils::SingleShaderStage::Compute, computeShader.c_str()); + + // Set common fields of compute pipeline descriptor. + wgpu::ComputePipelineDescriptor defaultComputePipelineDescriptor; + defaultComputePipelineDescriptor.computeStage.module = csModule; + defaultComputePipelineDescriptor.computeStage.entryPoint = "main"; + + // Set common fileds of bind group layout binding. + wgpu::BindGroupLayoutBinding defaultBindGroupLayoutBinding; + defaultBindGroupLayoutBinding.binding = 0; + defaultBindGroupLayoutBinding.visibility = wgpu::ShaderStage::Compute; + defaultBindGroupLayoutBinding.storageTextureFormat = kStorageTextureFormat; + + for (wgpu::BindingType bindingTypeInBindgroupLayout : kSupportedBindingTypes) { + wgpu::ComputePipelineDescriptor computePipelineDescriptor = + defaultComputePipelineDescriptor; + + // Create bind group layout with different binding types. + wgpu::BindGroupLayoutBinding bindGroupLayoutBinding = defaultBindGroupLayoutBinding; + bindGroupLayoutBinding.type = bindingTypeInBindgroupLayout; + wgpu::BindGroupLayout bindGroupLayout = + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); + computePipelineDescriptor.layout = + utils::MakeBasicPipelineLayout(device, &bindGroupLayout); + + // The binding type in the bind group layout must the same as the related image object + // declared in shader. + if (bindingTypeInBindgroupLayout == bindingTypeInShader) { + device.CreateComputePipeline(&computePipelineDescriptor); + } else { + ASSERT_DEVICE_ERROR(device.CreateComputePipeline(&computePipelineDescriptor)); + } + } + } +} + +// Verify it is invalid not to set a valid texture format in a bind group layout when the binding +// type is read-only or write-only storage texture. +TEST_F(StorageTextureValidationTests, UndefinedStorageTextureFormatInBindGroupLayout) { + wgpu::BindGroupLayoutBinding errorBindGroupLayoutBinding; + errorBindGroupLayoutBinding.binding = 0; + errorBindGroupLayoutBinding.visibility = wgpu::ShaderStage::Compute; + errorBindGroupLayoutBinding.storageTextureFormat = wgpu::TextureFormat::Undefined; + + for (wgpu::BindingType bindingType : kSupportedStorageTextureBindingTypes) { + errorBindGroupLayoutBinding.type = bindingType; + ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(device, {errorBindGroupLayoutBinding})); + } +} + +// Verify it is invalid to create a bind group layout with storage textures and an unsupported +// storage texture format. +TEST_F(StorageTextureValidationTests, StorageTextureFormatInBindGroupLayout) { + wgpu::BindGroupLayoutBinding defaultBindGroupLayoutBinding; + defaultBindGroupLayoutBinding.binding = 0; + defaultBindGroupLayoutBinding.visibility = wgpu::ShaderStage::Compute; + + for (wgpu::BindingType bindingType : kSupportedStorageTextureBindingTypes) { + for (wgpu::TextureFormat textureFormat : utils::kAllTextureFormats) { + wgpu::BindGroupLayoutBinding bindGroupLayoutBinding = defaultBindGroupLayoutBinding; + bindGroupLayoutBinding.type = bindingType; + bindGroupLayoutBinding.storageTextureFormat = textureFormat; + if (utils::TextureFormatSupportsStorageTexture(textureFormat)) { + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); + } else { + ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding})); + } + } + } +} + +// Verify the storage texture format in the bind group layout must match the declaration in shader. +TEST_F(StorageTextureValidationTests, BindGroupLayoutStorageTextureFormatMatchesShaderDeclaration) { + for (wgpu::BindingType bindingType : kSupportedStorageTextureBindingTypes) { + for (wgpu::TextureFormat storageTextureFormatInShader : utils::kAllTextureFormats) { + if (!utils::TextureFormatSupportsStorageTexture(storageTextureFormatInShader)) { + continue; + } + + // Create the compute shader module with the given binding type and storage texture + // format. + std::string computeShader = + CreateComputeShaderWithStorageTexture(bindingType, storageTextureFormatInShader); + wgpu::ShaderModule csModule = utils::CreateShaderModule( + device, utils::SingleShaderStage::Compute, computeShader.c_str()); + + // Set common fields of compute pipeline descriptor. + wgpu::ComputePipelineDescriptor defaultComputePipelineDescriptor; + defaultComputePipelineDescriptor.computeStage.module = csModule; + defaultComputePipelineDescriptor.computeStage.entryPoint = "main"; + + // Set common fileds of bind group layout binding. + wgpu::BindGroupLayoutBinding defaultBindGroupLayoutBinding = { + 0, wgpu::ShaderStage::Compute, bindingType}; + + for (wgpu::TextureFormat storageTextureFormatInBindGroupLayout : + utils::kAllTextureFormats) { + if (!utils::TextureFormatSupportsStorageTexture( + storageTextureFormatInBindGroupLayout)) { + continue; + } + + // Create the bind group layout with the given storage texture format. + wgpu::BindGroupLayoutBinding bindGroupLayoutBinding = defaultBindGroupLayoutBinding; + bindGroupLayoutBinding.storageTextureFormat = storageTextureFormatInBindGroupLayout; + wgpu::BindGroupLayout bindGroupLayout = + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); + + // Create the compute pipeline with the bind group layout. + wgpu::ComputePipelineDescriptor computePipelineDescriptor = + defaultComputePipelineDescriptor; + computePipelineDescriptor.layout = + utils::MakeBasicPipelineLayout(device, &bindGroupLayout); + + // The storage texture format in the bind group layout must be the same as the one + // declared in the shader. + if (storageTextureFormatInShader == storageTextureFormatInBindGroupLayout) { + device.CreateComputePipeline(&computePipelineDescriptor); + } else { + ASSERT_DEVICE_ERROR(device.CreateComputePipeline(&computePipelineDescriptor)); + } + } + } + } +} + +// Verify the dimension of the bind group layout with storage textures must match the one declared +// in shader. +TEST_F(StorageTextureValidationTests, BindGroupLayoutTextureDimensionMatchesShaderDeclaration) { + constexpr std::array kAllDimensions = { + wgpu::TextureViewDimension::e1D, wgpu::TextureViewDimension::e2D, + wgpu::TextureViewDimension::e2DArray, wgpu::TextureViewDimension::Cube, + wgpu::TextureViewDimension::CubeArray, wgpu::TextureViewDimension::e3D}; + constexpr wgpu::TextureFormat kStorageTextureFormat = wgpu::TextureFormat::R32Float; + + for (wgpu::BindingType bindingType : kSupportedStorageTextureBindingTypes) { + for (wgpu::TextureViewDimension dimensionInShader : kAllDimensions) { + // Create the compute shader with the given texture view dimension. + std::string computeShader = CreateComputeShaderWithStorageTexture( + bindingType, kStorageTextureFormat, dimensionInShader); + wgpu::ShaderModule csModule = utils::CreateShaderModule( + device, utils::SingleShaderStage::Compute, computeShader.c_str()); + + // Set common fields of compute pipeline descriptor. + wgpu::ComputePipelineDescriptor defaultComputePipelineDescriptor; + defaultComputePipelineDescriptor.computeStage.module = csModule; + defaultComputePipelineDescriptor.computeStage.entryPoint = "main"; + + // Set common fileds of bind group layout binding. + wgpu::BindGroupLayoutBinding defaultBindGroupLayoutBinding = { + 0, wgpu::ShaderStage::Compute, bindingType}; + defaultBindGroupLayoutBinding.storageTextureFormat = kStorageTextureFormat; + + for (wgpu::TextureViewDimension dimensionInBindGroupLayout : kAllDimensions) { + // Create the bind group layout with the given texture view dimension. + wgpu::BindGroupLayoutBinding bindGroupLayoutBinding = defaultBindGroupLayoutBinding; + bindGroupLayoutBinding.textureDimension = dimensionInBindGroupLayout; + wgpu::BindGroupLayout bindGroupLayout = + utils::MakeBindGroupLayout(device, {bindGroupLayoutBinding}); + + // Create the compute pipeline with the bind group layout. + wgpu::ComputePipelineDescriptor computePipelineDescriptor = + defaultComputePipelineDescriptor; + computePipelineDescriptor.layout = + utils::MakeBasicPipelineLayout(device, &bindGroupLayout); + + // The texture dimension in the bind group layout must be the same as the one + // declared in the shader. + if (dimensionInShader == dimensionInBindGroupLayout) { + device.CreateComputePipeline(&computePipelineDescriptor); + } else { + ASSERT_DEVICE_ERROR(device.CreateComputePipeline(&computePipelineDescriptor)); + } + } + } + } +}