Improve errors in BindGroupLayout, BindingInfo

Updates all validation messages in BindGroupLayout.cpp and
BindingInfo.cpp to give them better contextual information.

Bug: dawn:563
Change-Id: I7166dce65c93d7c8ac4dd72555fff34c9202e041
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/66841
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>
This commit is contained in:
Brandon Jones 2021-10-19 16:14:51 +00:00 committed by Dawn LUCI CQ
parent 26ae0ea4c5
commit 538f795e6c
8 changed files with 199 additions and 134 deletions

View File

@ -37,9 +37,9 @@ namespace dawn_native {
DAWN_TRY_ASSIGN(format, device->GetInternalFormat(storageTextureFormat)); DAWN_TRY_ASSIGN(format, device->GetInternalFormat(storageTextureFormat));
ASSERT(format != nullptr); ASSERT(format != nullptr);
if (!format->supportsStorageUsage) { DAWN_INVALID_IF(!format->supportsStorageUsage,
return DAWN_VALIDATION_ERROR("Texture format does not support storage textures"); "Texture format (%s) does not support storage textures.",
} storageTextureFormat);
return {}; return {};
} }
@ -48,8 +48,8 @@ namespace dawn_native {
switch (dimension) { switch (dimension) {
case wgpu::TextureViewDimension::Cube: case wgpu::TextureViewDimension::Cube:
case wgpu::TextureViewDimension::CubeArray: case wgpu::TextureViewDimension::CubeArray:
return DAWN_VALIDATION_ERROR( return DAWN_FORMAT_VALIDATION_ERROR(
"Cube map and cube map texture views cannot be used as storage textures"); "%s texture views cannot be used as storage textures.", dimension);
case wgpu::TextureViewDimension::e1D: case wgpu::TextureViewDimension::e1D:
case wgpu::TextureViewDimension::e2D: case wgpu::TextureViewDimension::e2D:
@ -62,40 +62,25 @@ namespace dawn_native {
} }
UNREACHABLE(); UNREACHABLE();
} }
} // anonymous namespace
MaybeError ValidateBindGroupLayoutDescriptor(DeviceBase* device,
const BindGroupLayoutDescriptor* descriptor,
bool allowInternalBinding) {
if (descriptor->nextInChain != nullptr) {
return DAWN_VALIDATION_ERROR("nextInChain must be nullptr");
}
std::set<BindingNumber> bindingsSet;
BindingCounts bindingCounts = {};
for (uint32_t i = 0; i < descriptor->entryCount; ++i) {
const BindGroupLayoutEntry& entry = descriptor->entries[i];
BindingNumber bindingNumber = BindingNumber(entry.binding);
if (bindingsSet.count(bindingNumber) != 0) {
return DAWN_VALIDATION_ERROR("some binding index was specified more than once");
}
MaybeError ValidateBindGroupLayoutEntry(DeviceBase* device,
const BindGroupLayoutEntry& entry,
bool allowInternalBinding) {
DAWN_TRY(ValidateShaderStage(entry.visibility)); DAWN_TRY(ValidateShaderStage(entry.visibility));
int bindingMemberCount = 0; int bindingMemberCount = 0;
BindingInfoType bindingType;
wgpu::ShaderStage allowedStages = kAllStages; wgpu::ShaderStage allowedStages = kAllStages;
if (entry.buffer.type != wgpu::BufferBindingType::Undefined) { if (entry.buffer.type != wgpu::BufferBindingType::Undefined) {
bindingMemberCount++; bindingMemberCount++;
bindingType = BindingInfoType::Buffer;
const BufferBindingLayout& buffer = entry.buffer; const BufferBindingLayout& buffer = entry.buffer;
// The kInternalStorageBufferBinding is used internally and not a value // The kInternalStorageBufferBinding is used internally and not a value
// in wgpu::BufferBindingType. // in wgpu::BufferBindingType.
if (buffer.type == kInternalStorageBufferBinding) { if (buffer.type == kInternalStorageBufferBinding) {
if (!allowInternalBinding) { DAWN_INVALID_IF(!allowInternalBinding, "Internal binding types are disallowed");
return DAWN_VALIDATION_ERROR("Internal binding types are disallowed");
}
} else { } else {
DAWN_TRY(ValidateBufferBindingType(buffer.type)); DAWN_TRY(ValidateBufferBindingType(buffer.type));
} }
@ -107,22 +92,23 @@ namespace dawn_native {
// Dynamic storage buffers aren't bounds checked properly in D3D12. Disallow them as // Dynamic storage buffers aren't bounds checked properly in D3D12. Disallow them as
// unsafe until the bounds checks are implemented. // unsafe until the bounds checks are implemented.
if (device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs) && DAWN_INVALID_IF(
buffer.hasDynamicOffset && device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs) &&
(buffer.type == wgpu::BufferBindingType::Storage || buffer.hasDynamicOffset &&
buffer.type == kInternalStorageBufferBinding || (buffer.type == wgpu::BufferBindingType::Storage ||
buffer.type == wgpu::BufferBindingType::ReadOnlyStorage)) { buffer.type == kInternalStorageBufferBinding ||
return DAWN_VALIDATION_ERROR( buffer.type == wgpu::BufferBindingType::ReadOnlyStorage),
"Dynamic storage buffers are disallowed because they aren't secure yet. " "Dynamic storage buffers are disallowed because they aren't secure yet. "
"See https://crbug.com/dawn/429"); "See https://crbug.com/dawn/429");
}
} }
if (entry.sampler.type != wgpu::SamplerBindingType::Undefined) { if (entry.sampler.type != wgpu::SamplerBindingType::Undefined) {
bindingMemberCount++; bindingMemberCount++;
bindingType = BindingInfoType::Sampler;
DAWN_TRY(ValidateSamplerBindingType(entry.sampler.type)); DAWN_TRY(ValidateSamplerBindingType(entry.sampler.type));
} }
if (entry.texture.sampleType != wgpu::TextureSampleType::Undefined) { if (entry.texture.sampleType != wgpu::TextureSampleType::Undefined) {
bindingMemberCount++; bindingMemberCount++;
bindingType = BindingInfoType::Texture;
const TextureBindingLayout& texture = entry.texture; const TextureBindingLayout& texture = entry.texture;
DAWN_TRY(ValidateTextureSampleType(texture.sampleType)); DAWN_TRY(ValidateTextureSampleType(texture.sampleType));
@ -133,12 +119,14 @@ namespace dawn_native {
viewDimension = texture.viewDimension; viewDimension = texture.viewDimension;
} }
if (texture.multisampled && viewDimension != wgpu::TextureViewDimension::e2D) { DAWN_INVALID_IF(
return DAWN_VALIDATION_ERROR("Multisampled texture bindings must be 2D."); texture.multisampled && viewDimension != wgpu::TextureViewDimension::e2D,
} "View dimension (%s) for a multisampled texture bindings was not %s.",
viewDimension, wgpu::TextureViewDimension::e2D);
} }
if (entry.storageTexture.access != wgpu::StorageTextureAccess::Undefined) { if (entry.storageTexture.access != wgpu::StorageTextureAccess::Undefined) {
bindingMemberCount++; bindingMemberCount++;
bindingType = BindingInfoType::StorageTexture;
const StorageTextureBindingLayout& storageTexture = entry.storageTexture; const StorageTextureBindingLayout& storageTexture = entry.storageTexture;
DAWN_TRY(ValidateStorageTextureAccess(storageTexture.access)); DAWN_TRY(ValidateStorageTextureAccess(storageTexture.access));
DAWN_TRY(ValidateStorageTextureFormat(device, storageTexture.format)); DAWN_TRY(ValidateStorageTextureFormat(device, storageTexture.format));
@ -158,24 +146,48 @@ namespace dawn_native {
FindInChain(entry.nextInChain, &externalTextureBindingLayout); FindInChain(entry.nextInChain, &externalTextureBindingLayout);
if (externalTextureBindingLayout != nullptr) { if (externalTextureBindingLayout != nullptr) {
bindingMemberCount++; bindingMemberCount++;
bindingType = BindingInfoType::ExternalTexture;
} }
if (bindingMemberCount != 1) { DAWN_INVALID_IF(bindingMemberCount != 1,
return DAWN_VALIDATION_ERROR( "BindGroupLayoutEntry had more than one of buffer, sampler, texture, "
"Exactly one of buffer, sampler, texture, storageTexture, or externalTexture " "storageTexture, or externalTexture set");
"must be set for each BindGroupLayoutEntry");
}
if (!IsSubset(entry.visibility, allowedStages)) { DAWN_INVALID_IF(
return DAWN_VALIDATION_ERROR("Binding type cannot be used with this visibility."); !IsSubset(entry.visibility, allowedStages),
} "%s bindings cannot be used with a visibility of %s. Only %s are allowed.",
bindingType, entry.visibility, allowedStages);
return {};
}
} // anonymous namespace
MaybeError ValidateBindGroupLayoutDescriptor(DeviceBase* device,
const BindGroupLayoutDescriptor* descriptor,
bool allowInternalBinding) {
DAWN_INVALID_IF(descriptor->nextInChain != nullptr, "nextInChain must be nullptr");
std::set<BindingNumber> bindingsSet;
BindingCounts bindingCounts = {};
for (uint32_t i = 0; i < descriptor->entryCount; ++i) {
const BindGroupLayoutEntry& entry = descriptor->entries[i];
BindingNumber bindingNumber = BindingNumber(entry.binding);
DAWN_INVALID_IF(bindingsSet.count(bindingNumber) != 0,
"On entries[%u]: binding index (%u) was specified by a previous entry.",
i, entry.binding);
DAWN_TRY_CONTEXT(ValidateBindGroupLayoutEntry(device, entry, allowInternalBinding),
"validating entries[%u]", i);
IncrementBindingCounts(&bindingCounts, entry); IncrementBindingCounts(&bindingCounts, entry);
bindingsSet.insert(bindingNumber); bindingsSet.insert(bindingNumber);
} }
DAWN_TRY(ValidateBindingCounts(bindingCounts)); DAWN_TRY_CONTEXT(ValidateBindingCounts(bindingCounts), "validating binding counts");
return {}; return {};
} }

View File

@ -18,6 +18,32 @@
namespace dawn_native { namespace dawn_native {
absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
BindingInfoType value,
const absl::FormatConversionSpec& spec,
absl::FormatSink* s) {
switch (value) {
case BindingInfoType::Buffer:
s->Append("Buffer");
break;
case BindingInfoType::Sampler:
s->Append("Sampler");
break;
case BindingInfoType::Texture:
s->Append("Texture");
break;
case BindingInfoType::StorageTexture:
s->Append("StorageTexture");
break;
case BindingInfoType::ExternalTexture:
s->Append("ExternalTexture");
break;
default:
UNREACHABLE();
}
return {true};
}
void IncrementBindingCounts(BindingCounts* bindingCounts, const BindGroupLayoutEntry& entry) { void IncrementBindingCounts(BindingCounts* bindingCounts, const BindGroupLayoutEntry& entry) {
bindingCounts->totalCount += 1; bindingCounts->totalCount += 1;
@ -96,84 +122,97 @@ namespace dawn_native {
} }
MaybeError ValidateBindingCounts(const BindingCounts& bindingCounts) { MaybeError ValidateBindingCounts(const BindingCounts& bindingCounts) {
if (bindingCounts.dynamicUniformBufferCount > kMaxDynamicUniformBuffersPerPipelineLayout) { DAWN_INVALID_IF(
return DAWN_VALIDATION_ERROR( bindingCounts.dynamicUniformBufferCount > kMaxDynamicUniformBuffersPerPipelineLayout,
"The number of dynamic uniform buffers exceeds the maximum per-pipeline-layout " "The number of dynamic uniform buffers (%u) exceeds the maximum per-pipeline-layout "
"limit"); "limit (%u).",
} bindingCounts.dynamicUniformBufferCount, kMaxDynamicUniformBuffersPerPipelineLayout);
if (bindingCounts.dynamicStorageBufferCount > kMaxDynamicStorageBuffersPerPipelineLayout) { DAWN_INVALID_IF(
return DAWN_VALIDATION_ERROR( bindingCounts.dynamicStorageBufferCount > kMaxDynamicStorageBuffersPerPipelineLayout,
"The number of dynamic storage buffers exceeds the maximum per-pipeline-layout " "The number of dynamic storage buffers (%u) exceeds the maximum per-pipeline-layout "
"limit"); "limit (%u).",
} bindingCounts.dynamicStorageBufferCount, kMaxDynamicStorageBuffersPerPipelineLayout);
for (SingleShaderStage stage : IterateStages(kAllStages)) { for (SingleShaderStage stage : IterateStages(kAllStages)) {
if (bindingCounts.perStage[stage].sampledTextureCount > DAWN_INVALID_IF(
kMaxSampledTexturesPerShaderStage) { bindingCounts.perStage[stage].sampledTextureCount >
return DAWN_VALIDATION_ERROR( kMaxSampledTexturesPerShaderStage,
"The number of sampled textures exceeds the maximum " "The number of sampled textures (%u) in the %s stage exceeds the maximum "
"per-stage limit."); "per-stage limit (%u).",
} bindingCounts.perStage[stage].sampledTextureCount, stage,
kMaxSampledTexturesPerShaderStage);
// The per-stage number of external textures is bound by the maximum sampled textures // The per-stage number of external textures is bound by the maximum sampled textures
// per stage. // per stage.
if (bindingCounts.perStage[stage].externalTextureCount > DAWN_INVALID_IF(
kMaxSampledTexturesPerShaderStage / kSampledTexturesPerExternalTexture) { bindingCounts.perStage[stage].externalTextureCount >
return DAWN_VALIDATION_ERROR( kMaxSampledTexturesPerShaderStage / kSampledTexturesPerExternalTexture,
"The number of external textures exceeds the maximum " "The number of external textures (%u) in the %s stage exceeds the maximum "
"per-stage limit."); "per-stage limit (%u).",
} bindingCounts.perStage[stage].externalTextureCount, stage,
kMaxSampledTexturesPerShaderStage / kSampledTexturesPerExternalTexture);
if (bindingCounts.perStage[stage].sampledTextureCount + DAWN_INVALID_IF(
(bindingCounts.perStage[stage].externalTextureCount * bindingCounts.perStage[stage].sampledTextureCount +
kSampledTexturesPerExternalTexture) > (bindingCounts.perStage[stage].externalTextureCount *
kMaxSampledTexturesPerShaderStage) { kSampledTexturesPerExternalTexture) >
return DAWN_VALIDATION_ERROR( kMaxSampledTexturesPerShaderStage,
"The combination of sampled textures and external textures exceeds the maximum " "The combination of sampled textures (%u) and external textures (%u) in the %s "
"per-stage limit."); "stage exceeds the maximum per-stage limit (%u).",
} bindingCounts.perStage[stage].sampledTextureCount,
bindingCounts.perStage[stage].externalTextureCount, stage,
kMaxSampledTexturesPerShaderStage);
if (bindingCounts.perStage[stage].samplerCount > kMaxSamplersPerShaderStage) { DAWN_INVALID_IF(
return DAWN_VALIDATION_ERROR( bindingCounts.perStage[stage].samplerCount > kMaxSamplersPerShaderStage,
"The number of samplers exceeds the maximum per-stage limit."); "The number of samplers (%u) in the %s stage exceeds the maximum per-stage limit "
} "(%u).",
bindingCounts.perStage[stage].samplerCount, stage, kMaxSamplersPerShaderStage);
if (bindingCounts.perStage[stage].samplerCount + DAWN_INVALID_IF(
(bindingCounts.perStage[stage].externalTextureCount * bindingCounts.perStage[stage].samplerCount +
kSamplersPerExternalTexture) > (bindingCounts.perStage[stage].externalTextureCount *
kMaxSamplersPerShaderStage) { kSamplersPerExternalTexture) >
return DAWN_VALIDATION_ERROR( kMaxSamplersPerShaderStage,
"The combination of samplers and external textures exceeds the maximum " "The combination of samplers (%u) and external textures (%u) in the %s stage "
"per-stage limit."); "exceeds the maximum per-stage limit (%u).",
} bindingCounts.perStage[stage].samplerCount,
bindingCounts.perStage[stage].externalTextureCount, stage,
kMaxSamplersPerShaderStage);
if (bindingCounts.perStage[stage].storageBufferCount > DAWN_INVALID_IF(
kMaxStorageBuffersPerShaderStage) { bindingCounts.perStage[stage].storageBufferCount > kMaxStorageBuffersPerShaderStage,
return DAWN_VALIDATION_ERROR( "The number of storage buffers (%u) in the %s stage exceeds the maximum per-stage "
"The number of storage buffers exceeds the maximum per-stage limit."); "limit (%u).",
} bindingCounts.perStage[stage].storageBufferCount, stage,
kMaxStorageBuffersPerShaderStage);
if (bindingCounts.perStage[stage].storageTextureCount > DAWN_INVALID_IF(
kMaxStorageTexturesPerShaderStage) { bindingCounts.perStage[stage].storageTextureCount >
return DAWN_VALIDATION_ERROR( kMaxStorageTexturesPerShaderStage,
"The number of storage textures exceeds the maximum per-stage limit."); "The number of storage textures (%u) in the %s stage exceeds the maximum per-stage "
} "limit (%u).",
bindingCounts.perStage[stage].storageTextureCount, stage,
kMaxStorageTexturesPerShaderStage);
if (bindingCounts.perStage[stage].uniformBufferCount > DAWN_INVALID_IF(
kMaxUniformBuffersPerShaderStage) { bindingCounts.perStage[stage].uniformBufferCount > kMaxUniformBuffersPerShaderStage,
return DAWN_VALIDATION_ERROR( "The number of uniform buffers (%u) in the %s stage exceeds the maximum per-stage "
"The number of uniform buffers exceeds the maximum per-stage limit."); "limit (%u).",
} bindingCounts.perStage[stage].uniformBufferCount, stage,
kMaxUniformBuffersPerShaderStage);
if (bindingCounts.perStage[stage].uniformBufferCount + DAWN_INVALID_IF(
(bindingCounts.perStage[stage].externalTextureCount * bindingCounts.perStage[stage].uniformBufferCount +
kUniformsPerExternalTexture) > (bindingCounts.perStage[stage].externalTextureCount *
kMaxUniformBuffersPerShaderStage) { kUniformsPerExternalTexture) >
return DAWN_VALIDATION_ERROR( kMaxUniformBuffersPerShaderStage,
"The combination of uniform buffers and external textures exceeds the maximum " "The combination of uniform buffers (%u) and external textures (%u) in the %s "
"per-stage limit."); "stage exceeds the maximum per-stage limit (%u).",
} bindingCounts.perStage[stage].uniformBufferCount,
bindingCounts.perStage[stage].externalTextureCount, stage,
kMaxUniformBuffersPerShaderStage);
} }
return {}; return {};

View File

@ -50,6 +50,11 @@ namespace dawn_native {
enum class BindingInfoType { Buffer, Sampler, Texture, StorageTexture, ExternalTexture }; enum class BindingInfoType { Buffer, Sampler, Texture, StorageTexture, ExternalTexture };
absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
BindingInfoType value,
const absl::FormatConversionSpec& spec,
absl::FormatSink* s);
struct BindingInfo { struct BindingInfo {
BindingNumber binding; BindingNumber binding;
wgpu::ShaderStage visibility; wgpu::ShaderStage visibility;

View File

@ -1179,7 +1179,9 @@ namespace dawn_native {
bool allowInternalBinding) { bool allowInternalBinding) {
DAWN_TRY(ValidateIsAlive()); DAWN_TRY(ValidateIsAlive());
if (IsValidationEnabled()) { if (IsValidationEnabled()) {
DAWN_TRY(ValidateBindGroupLayoutDescriptor(this, descriptor, allowInternalBinding)); DAWN_TRY_CONTEXT(
ValidateBindGroupLayoutDescriptor(this, descriptor, allowInternalBinding),
"validating %s", descriptor);
} }
return GetOrCreateBindGroupLayout(descriptor); return GetOrCreateBindGroupLayout(descriptor);
} }

View File

@ -16,6 +16,26 @@
namespace dawn_native { namespace dawn_native {
absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
SingleShaderStage value,
const absl::FormatConversionSpec& spec,
absl::FormatSink* s) {
switch (value) {
case SingleShaderStage::Compute:
s->Append("Compute");
break;
case SingleShaderStage::Vertex:
s->Append("Vertex");
break;
case SingleShaderStage::Fragment:
s->Append("Fragment");
break;
default:
UNREACHABLE();
}
return {true};
}
BitSetIterator<kNumStages, SingleShaderStage> IterateStages(wgpu::ShaderStage stages) { BitSetIterator<kNumStages, SingleShaderStage> IterateStages(wgpu::ShaderStage stages) {
std::bitset<kNumStages> bits(static_cast<uint32_t>(stages)); std::bitset<kNumStages> bits(static_cast<uint32_t>(stages));
return BitSetIterator<kNumStages, SingleShaderStage>(bits); return BitSetIterator<kNumStages, SingleShaderStage>(bits);

View File

@ -18,6 +18,7 @@
#include "common/Assert.h" #include "common/Assert.h"
#include "common/BitSetIterator.h" #include "common/BitSetIterator.h"
#include "common/Constants.h" #include "common/Constants.h"
#include "dawn_native/Error.h"
#include "dawn_native/dawn_platform.h" #include "dawn_native/dawn_platform.h"
@ -27,6 +28,11 @@ namespace dawn_native {
enum class SingleShaderStage { Vertex, Fragment, Compute }; enum class SingleShaderStage { Vertex, Fragment, Compute };
absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
SingleShaderStage value,
const absl::FormatConversionSpec& spec,
absl::FormatSink* s);
static_assert(static_cast<uint32_t>(SingleShaderStage::Vertex) < kNumStages, ""); static_assert(static_cast<uint32_t>(SingleShaderStage::Vertex) < kNumStages, "");
static_assert(static_cast<uint32_t>(SingleShaderStage::Fragment) < kNumStages, ""); static_assert(static_cast<uint32_t>(SingleShaderStage::Fragment) < kNumStages, "");
static_assert(static_cast<uint32_t>(SingleShaderStage::Compute) < kNumStages, ""); static_assert(static_cast<uint32_t>(SingleShaderStage::Compute) < kNumStages, "");

View File

@ -22,26 +22,6 @@
#include "dawn_native/ShaderModule.h" #include "dawn_native/ShaderModule.h"
namespace dawn_native { namespace dawn_native {
absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
SingleShaderStage value,
const absl::FormatConversionSpec& spec,
absl::FormatSink* s) {
switch (value) {
case SingleShaderStage::Compute:
s->Append("Compute");
break;
case SingleShaderStage::Vertex:
s->Append("Vertex");
break;
case SingleShaderStage::Fragment:
s->Append("Fragment");
break;
default:
UNREACHABLE();
}
return {true};
}
MaybeError ValidateProgrammableStage(DeviceBase* device, MaybeError ValidateProgrammableStage(DeviceBase* device,
const ShaderModuleBase* module, const ShaderModuleBase* module,
const std::string& entryPoint, const std::string& entryPoint,

View File

@ -230,7 +230,8 @@ namespace dawn_native {
desc.entryCount = entryVec.size(); desc.entryCount = entryVec.size();
if (device->IsValidationEnabled()) { if (device->IsValidationEnabled()) {
DAWN_TRY(ValidateBindGroupLayoutDescriptor(device, &desc)); DAWN_TRY_CONTEXT(ValidateBindGroupLayoutDescriptor(device, &desc), "validating %s",
&desc);
} }
return device->GetOrCreateBindGroupLayout(&desc, pipelineCompatibilityToken); return device->GetOrCreateBindGroupLayout(&desc, pipelineCompatibilityToken);
}; };