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

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

This reverts commit 7c24b6b2ff.

Reason for revert: We can reland this CL now as the fix in shaderc has
been merged.

Note that the declaration whether a storage texture is multisampled or not
cannot be extracted correctly in SPVC. The fix in Dawn will be added after
it is fixed in shaderc.

Original change's description:
> 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>

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

Bug: dawn:267, chromium:1063570
Change-Id: If762cbb206e738f4e54e75c88d506fdf3a44f280
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/17461
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
This commit is contained in:
Jiawei Shao 2020-03-24 00:56:53 +00:00 committed by Commit Bot service account
parent 80a1868f33
commit 4a858a0cbd
4 changed files with 288 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/dawn_native/BindGroupLayout.cpp
View File

@ -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:

1
src/dawn_native/BindGroupLayout.h
View File

@ -57,6 +57,7 @@ namespace dawn_native {
std::array<wgpu::BindingType, kMaxBindingsPerGroup> types;
std::array<wgpu::TextureComponentType, kMaxBindingsPerGroup> textureComponentTypes;
std::array<wgpu::TextureViewDimension, kMaxBindingsPerGroup> textureDimensions;
std::array<wgpu::TextureFormat, kMaxBindingsPerGroup> storageTextureFormats;
std::bitset<kMaxBindingsPerGroup> hasDynamicOffset;
std::bitset<kMaxBindingsPerGroup> multisampled;
BindingIndex bindingCount;

63
src/dawn_native/ShaderModule.cpp
View File

@ -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;
}
}
}

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

@ -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<wgpu::BindingType, 2> 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<wgpu::BindingType, 2> 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<wgpu::BindingType, 2> 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<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));
}
}
}
}
}