encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert "Check bind group layout with storage texture in pipeline descriptors" 

This reverts commit 63f2666ee7.

Reason for revert: causes failures in dawn_unittests after Dawn uses SPVC by default. We need a fix in both SPVC and this CL before re-landing.  

Original change's description:
> Check bind group layout with storage texture in pipeline descriptors
> 
> This patch adds all the validations on the use of bind group layout with
> read-only storage texture and write-only storage texture in the creation
> of pipeline objects.
> 
> 1. GPUBindGroupLayout.bindingType must match the type of the storage
>    texture variable (read-only or write-only) in shader.
> 2. GPUBindGroupLayout.storageTextureFormat must be a valid texture
>    format that supports STORAGE usage.
> 3. GPUBindGroupLayout.storageTextureFormat must match the storage
>    texture format declaration in shader.
> 4. GPUBindGroupLayout.textureDimension must match the storage texture
>    dimension declared in shader.
> 
> BUG=dawn:267
> TEST=dawn_unittests
> 
> Change-Id: Ifa3c2194dc76de14f790a0a73868e69bbb31c814
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/17167
> Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
> Reviewed-by: Kai Ninomiya <kainino@chromium.org>

TBR=cwallez@chromium.org,kainino@chromium.org,yunchao.he@intel.com,jiawei.shao@intel.com,shaobo.yan@intel.com,hao.x.li@intel.com,enga@chromium.org,jiajie.hu@intel.com

Change-Id: Idb4083b11f22fa7e4c5c8477bc4b65b58900746e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: dawn:267
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/17380
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
This commit is contained in:
Jiawei Shao 2020-03-20 01:44:29 +00:00 committed by Commit Bot service account
parent 63f2666ee7
commit 7c24b6b2ff
4 changed files with 26 additions and 275 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/dawn_native/BindGroupLayout.cpp
View File

@ -66,10 +66,8 @@ namespace dawn_native {
case wgpu::BindingType::WriteonlyStorageTexture: {
DAWN_TRY(ValidateTextureFormat(storageTextureFormat));
const Format* format = nullptr;
DAWN_TRY_ASSIGN(format, device->GetInternalFormat(storageTextureFormat));
ASSERT(format != nullptr);
if (!format->supportsStorageUsage) {
const Format& format = device->GetValidInternalFormat(storageTextureFormat);
if (!format.supportsStorageUsage) {
return DAWN_VALIDATION_ERROR("The storage texture format is not supported");
}
} break;
@ -172,8 +170,7 @@ namespace dawn_native {
for (uint32_t binding : IterateBitSet(info.mask)) {
HashCombine(&hash, info.visibilities[binding], info.types[binding],
info.textureComponentTypes[binding], info.textureDimensions[binding],
info.storageTextureFormats[binding]);
info.textureComponentTypes[binding], info.textureDimensions[binding]);
}
return hash;
@ -190,8 +187,7 @@ namespace dawn_native {
if ((a.visibilities[binding] != b.visibilities[binding]) ||
(a.types[binding] != b.types[binding]) ||
(a.textureComponentTypes[binding] != b.textureComponentTypes[binding]) ||
(a.textureDimensions[binding] != b.textureDimensions[binding]) ||
(a.storageTextureFormats[binding] != b.storageTextureFormats[binding])) {
(a.textureDimensions[binding] != b.textureDimensions[binding])) {
return false;
}
}
@ -212,7 +208,6 @@ namespace dawn_native {
mBindingInfo.visibilities[index] = binding.visibility;
mBindingInfo.types[index] = binding.type;
mBindingInfo.textureComponentTypes[index] = binding.textureComponentType;
mBindingInfo.storageTextureFormats[index] = binding.storageTextureFormat;
// TODO(enga): This is a greedy computation because there may be holes in bindings.
// Fix this when we pack bindings.

1
src/dawn_native/BindGroupLayout.h
View File

@ -55,7 +55,6 @@ namespace dawn_native {
std::bitset<kMaxBindingsPerGroup> hasDynamicOffset;
std::bitset<kMaxBindingsPerGroup> multisampled;
std::bitset<kMaxBindingsPerGroup> mask;
std::array<wgpu::TextureFormat, kMaxBindingsPerGroup> storageTextureFormats;
};
const LayoutBindingInfo& GetBindingInfo() const;

32
src/dawn_native/ShaderModule.cpp
View File

@ -606,8 +606,6 @@ 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;
@ -759,11 +757,9 @@ namespace dawn_native {
return false;
}
switch (layoutBindingType) {
case wgpu::BindingType::SampledTexture: {
if (layoutBindingType == wgpu::BindingType::SampledTexture) {
Format::Type layoutTextureComponentType =
Format::TextureComponentTypeToFormatType(
layoutInfo.textureComponentTypes[i]);
Format::TextureComponentTypeToFormatType(layoutInfo.textureComponentTypes[i]);
if (layoutTextureComponentType != moduleInfo.textureComponentType) {
return false;
}
@ -771,30 +767,6 @@ namespace dawn_native {
if (layoutInfo.textureDimensions[i] != moduleInfo.textureDimension) {
return false;
}
} break;
case wgpu::BindingType::ReadonlyStorageTexture:
case wgpu::BindingType::WriteonlyStorageTexture: {
ASSERT(layoutInfo.storageTextureFormats[i] != wgpu::TextureFormat::Undefined);
ASSERT(moduleInfo.storageTextureFormat != wgpu::TextureFormat::Undefined);
if (layoutInfo.storageTextureFormats[i] != moduleInfo.storageTextureFormat) {
return false;
}
if (layoutInfo.textureDimensions[i] != 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;
}
}

241
src/tests/unittests/validation/StorageTextureValidationTests.cpp
View File

@ -92,44 +92,20 @@ 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::TextureViewDimension textureViewDimension = wgpu::TextureViewDimension::e2D) {
wgpu::TextureFormat textureFormat) {
const char* glslImageFormatQualifier = GetGLSLImageFormatQualifier(textureFormat);
const char* textureComponentTypePrefix =
utils::GetColorTextureComponentTypePrefix(textureFormat);
return CreateComputeShaderWithStorageTexture(
storageTextureBindingType, glslImageFormatQualifier, textureComponentTypePrefix,
GetGLSLFloatImageTypeDeclaration(textureViewDimension));
storageTextureBindingType, glslImageFormatQualifier, textureComponentTypePrefix);
}
static std::string CreateComputeShaderWithStorageTexture(
wgpu::BindingType storageTextureBindingType,
const char* glslImageFormatQualifier,
const char* textureComponentTypePrefix,
const char* glslImageTypeDeclaration = "image2D") {
const char* textureComponentTypePrefix) {
const char* memoryQualifier = "";
switch (storageTextureBindingType) {
case wgpu::BindingType::ReadonlyStorageTexture:
@ -147,8 +123,8 @@ class StorageTextureValidationTests : public ValidationTest {
ostream << "#version 450\n"
"layout (set = 0, binding = 0, "
<< glslImageFormatQualifier << ") uniform " << memoryQualifier << " "
<< textureComponentTypePrefix << glslImageTypeDeclaration
<< " image0;\n"
<< textureComponentTypePrefix
<< "image2D image0;\n"
"void main() {\n"
"}\n";
@ -168,9 +144,6 @@ class StorageTextureValidationTests : public ValidationTest {
void main() {
fragColor = vec4(1.f, 0.f, 0.f, 1.f);
})");
const std::array<wgpu::BindingType, 2> kSupportedStorageTextureBindingTypes = {
wgpu::BindingType::ReadonlyStorageTexture, wgpu::BindingType::WriteonlyStorageTexture};
};
// Validate read-only storage textures can be declared in vertex and fragment
@ -403,7 +376,10 @@ TEST_F(StorageTextureValidationTests, StorageTextureFormatInShaders) {
wgpu::TextureFormat::RG16Sint, wgpu::TextureFormat::RG16Float,
wgpu::TextureFormat::RGB10A2Unorm, wgpu::TextureFormat::RG11B10Float};
for (wgpu::BindingType storageTextureBindingType : kSupportedStorageTextureBindingTypes) {
constexpr std::array<wgpu::BindingType, 2> kStorageTextureBindingTypes = {
wgpu::BindingType::ReadonlyStorageTexture, wgpu::BindingType::WriteonlyStorageTexture};
for (wgpu::BindingType storageTextureBindingType : kStorageTextureBindingTypes) {
for (wgpu::TextureFormat format : kWGPUTextureFormatSupportedAsSPIRVImageFormats) {
std::string computeShader =
CreateComputeShaderWithStorageTexture(storageTextureBindingType, format);
@ -434,7 +410,10 @@ TEST_F(StorageTextureValidationTests, UnsupportedSPIRVStorageTextureFormat) {
{"r16_snorm", ""},
{"rgb10_a2ui", "u"}}};
for (wgpu::BindingType bindingType : kSupportedStorageTextureBindingTypes) {
constexpr std::array<wgpu::BindingType, 2> kStorageTextureBindingTypes = {
wgpu::BindingType::ReadonlyStorageTexture, wgpu::BindingType::WriteonlyStorageTexture};
for (wgpu::BindingType bindingType : kStorageTextureBindingTypes) {
for (const TextureFormatInfo& formatInfo : kUnsupportedTextureFormats) {
std::string computeShader = CreateComputeShaderWithStorageTexture(
bindingType, formatInfo.name, formatInfo.componentTypePrefix);
@ -443,197 +422,3 @@ 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<wgpu::BindingType, 7> 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<wgpu::TextureViewDimension, 6> 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));
}
}
}
}
}