Skip to content

Commit

Permalink
Improve errors in BindGroupLayout, BindingInfo
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Reviewed-by: Austin Eng <[email protected]>
Commit-Queue: Brandon Jones <[email protected]>
  • Loading branch information
@toji
toji authored and Dawn LUCI CQ committed Oct 19, 2021
1 parent 26ae0ea commit 538f795
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 142 deletions.
106 changes: 59 additions & 47 deletions src/dawn_native/BindGroupLayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ namespace dawn_native {
DAWN_TRY_ASSIGN(format, device->GetInternalFormat(storageTextureFormat));

ASSERT(format != nullptr);
if (!format->supportsStorageUsage) {
return DAWN_VALIDATION_ERROR("Texture format does not support storage textures");
}
DAWN_INVALID_IF(!format->supportsStorageUsage,
"Texture format (%s) does not support storage textures.",
storageTextureFormat);

return {};
}
Expand All @@ -48,8 +48,8 @@ namespace dawn_native {
switch (dimension) {
case wgpu::TextureViewDimension::Cube:
case wgpu::TextureViewDimension::CubeArray:
return DAWN_VALIDATION_ERROR(
"Cube map and cube map texture views cannot be used as storage textures");
return DAWN_FORMAT_VALIDATION_ERROR(
"%s texture views cannot be used as storage textures.", dimension);

case wgpu::TextureViewDimension::e1D:
case wgpu::TextureViewDimension::e2D:
Expand All @@ -62,40 +62,25 @@ namespace dawn_native {
}
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));

int bindingMemberCount = 0;
BindingInfoType bindingType;
wgpu::ShaderStage allowedStages = kAllStages;

if (entry.buffer.type != wgpu::BufferBindingType::Undefined) {
bindingMemberCount++;
bindingType = BindingInfoType::Buffer;
const BufferBindingLayout& buffer = entry.buffer;

// The kInternalStorageBufferBinding is used internally and not a value
// in wgpu::BufferBindingType.
if (buffer.type == kInternalStorageBufferBinding) {
if (!allowInternalBinding) {
return DAWN_VALIDATION_ERROR("Internal binding types are disallowed");
}
DAWN_INVALID_IF(!allowInternalBinding, "Internal binding types are disallowed");
} else {
DAWN_TRY(ValidateBufferBindingType(buffer.type));
}
Expand All @@ -107,22 +92,23 @@ namespace dawn_native {

// Dynamic storage buffers aren't bounds checked properly in D3D12. Disallow them as
// unsafe until the bounds checks are implemented.
if (device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs) &&
buffer.hasDynamicOffset &&
(buffer.type == wgpu::BufferBindingType::Storage ||
buffer.type == kInternalStorageBufferBinding ||
buffer.type == wgpu::BufferBindingType::ReadOnlyStorage)) {
return DAWN_VALIDATION_ERROR(
"Dynamic storage buffers are disallowed because they aren't secure yet. "
"See https://crbug.com/dawn/429");
}
DAWN_INVALID_IF(
device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs) &&
buffer.hasDynamicOffset &&
(buffer.type == wgpu::BufferBindingType::Storage ||
buffer.type == kInternalStorageBufferBinding ||
buffer.type == wgpu::BufferBindingType::ReadOnlyStorage),
"Dynamic storage buffers are disallowed because they aren't secure yet. "
"See https://crbug.com/dawn/429");
}
if (entry.sampler.type != wgpu::SamplerBindingType::Undefined) {
bindingMemberCount++;
bindingType = BindingInfoType::Sampler;
DAWN_TRY(ValidateSamplerBindingType(entry.sampler.type));
}
if (entry.texture.sampleType != wgpu::TextureSampleType::Undefined) {
bindingMemberCount++;
bindingType = BindingInfoType::Texture;
const TextureBindingLayout& texture = entry.texture;
DAWN_TRY(ValidateTextureSampleType(texture.sampleType));

Expand All @@ -133,12 +119,14 @@ namespace dawn_native {
viewDimension = texture.viewDimension;
}

if (texture.multisampled && viewDimension != wgpu::TextureViewDimension::e2D) {
return DAWN_VALIDATION_ERROR("Multisampled texture bindings must be 2D.");
}
DAWN_INVALID_IF(
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) {
bindingMemberCount++;
bindingType = BindingInfoType::StorageTexture;
const StorageTextureBindingLayout& storageTexture = entry.storageTexture;
DAWN_TRY(ValidateStorageTextureAccess(storageTexture.access));
DAWN_TRY(ValidateStorageTextureFormat(device, storageTexture.format));
Expand All @@ -158,24 +146,48 @@ namespace dawn_native {
FindInChain(entry.nextInChain, &externalTextureBindingLayout);
if (externalTextureBindingLayout != nullptr) {
bindingMemberCount++;
bindingType = BindingInfoType::ExternalTexture;
}

if (bindingMemberCount != 1) {
return DAWN_VALIDATION_ERROR(
"Exactly one of buffer, sampler, texture, storageTexture, or externalTexture "
"must be set for each BindGroupLayoutEntry");
}
DAWN_INVALID_IF(bindingMemberCount != 1,
"BindGroupLayoutEntry had more than one of buffer, sampler, texture, "
"storageTexture, or externalTexture set");

if (!IsSubset(entry.visibility, allowedStages)) {
return DAWN_VALIDATION_ERROR("Binding type cannot be used with this visibility.");
}
DAWN_INVALID_IF(
!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);

bindingsSet.insert(bindingNumber);
}

DAWN_TRY(ValidateBindingCounts(bindingCounts));
DAWN_TRY_CONTEXT(ValidateBindingCounts(bindingCounts), "validating binding counts");

return {};
}
Expand Down
185 changes: 112 additions & 73 deletions src/dawn_native/BindingInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,32 @@

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) {
bindingCounts->totalCount += 1;

Expand Down Expand Up @@ -96,84 +122,97 @@ namespace dawn_native {
}

MaybeError ValidateBindingCounts(const BindingCounts& bindingCounts) {
if (bindingCounts.dynamicUniformBufferCount > kMaxDynamicUniformBuffersPerPipelineLayout) {
return DAWN_VALIDATION_ERROR(
"The number of dynamic uniform buffers exceeds the maximum per-pipeline-layout "
"limit");
}

if (bindingCounts.dynamicStorageBufferCount > kMaxDynamicStorageBuffersPerPipelineLayout) {
return DAWN_VALIDATION_ERROR(
"The number of dynamic storage buffers exceeds the maximum per-pipeline-layout "
"limit");
}
DAWN_INVALID_IF(
bindingCounts.dynamicUniformBufferCount > kMaxDynamicUniformBuffersPerPipelineLayout,
"The number of dynamic uniform buffers (%u) exceeds the maximum per-pipeline-layout "
"limit (%u).",
bindingCounts.dynamicUniformBufferCount, kMaxDynamicUniformBuffersPerPipelineLayout);

DAWN_INVALID_IF(
bindingCounts.dynamicStorageBufferCount > kMaxDynamicStorageBuffersPerPipelineLayout,
"The number of dynamic storage buffers (%u) exceeds the maximum per-pipeline-layout "
"limit (%u).",
bindingCounts.dynamicStorageBufferCount, kMaxDynamicStorageBuffersPerPipelineLayout);

for (SingleShaderStage stage : IterateStages(kAllStages)) {
if (bindingCounts.perStage[stage].sampledTextureCount >
kMaxSampledTexturesPerShaderStage) {
return DAWN_VALIDATION_ERROR(
"The number of sampled textures exceeds the maximum "
"per-stage limit.");
}
DAWN_INVALID_IF(
bindingCounts.perStage[stage].sampledTextureCount >
kMaxSampledTexturesPerShaderStage,
"The number of sampled textures (%u) in the %s stage exceeds the maximum "
"per-stage limit (%u).",
bindingCounts.perStage[stage].sampledTextureCount, stage,
kMaxSampledTexturesPerShaderStage);

// The per-stage number of external textures is bound by the maximum sampled textures
// per stage.
if (bindingCounts.perStage[stage].externalTextureCount >
kMaxSampledTexturesPerShaderStage / kSampledTexturesPerExternalTexture) {
return DAWN_VALIDATION_ERROR(
"The number of external textures exceeds the maximum "
"per-stage limit.");
}

if (bindingCounts.perStage[stage].sampledTextureCount +
(bindingCounts.perStage[stage].externalTextureCount *
kSampledTexturesPerExternalTexture) >
kMaxSampledTexturesPerShaderStage) {
return DAWN_VALIDATION_ERROR(
"The combination of sampled textures and external textures exceeds the maximum "
"per-stage limit.");
}

if (bindingCounts.perStage[stage].samplerCount > kMaxSamplersPerShaderStage) {
return DAWN_VALIDATION_ERROR(
"The number of samplers exceeds the maximum per-stage limit.");
}

if (bindingCounts.perStage[stage].samplerCount +
(bindingCounts.perStage[stage].externalTextureCount *
kSamplersPerExternalTexture) >
kMaxSamplersPerShaderStage) {
return DAWN_VALIDATION_ERROR(
"The combination of samplers and external textures exceeds the maximum "
"per-stage limit.");
}

if (bindingCounts.perStage[stage].storageBufferCount >
kMaxStorageBuffersPerShaderStage) {
return DAWN_VALIDATION_ERROR(
"The number of storage buffers exceeds the maximum per-stage limit.");
}

if (bindingCounts.perStage[stage].storageTextureCount >
kMaxStorageTexturesPerShaderStage) {
return DAWN_VALIDATION_ERROR(
"The number of storage textures exceeds the maximum per-stage limit.");
}

if (bindingCounts.perStage[stage].uniformBufferCount >
kMaxUniformBuffersPerShaderStage) {
return DAWN_VALIDATION_ERROR(
"The number of uniform buffers exceeds the maximum per-stage limit.");
}

if (bindingCounts.perStage[stage].uniformBufferCount +
(bindingCounts.perStage[stage].externalTextureCount *
kUniformsPerExternalTexture) >
kMaxUniformBuffersPerShaderStage) {
return DAWN_VALIDATION_ERROR(
"The combination of uniform buffers and external textures exceeds the maximum "
"per-stage limit.");
}
DAWN_INVALID_IF(
bindingCounts.perStage[stage].externalTextureCount >
kMaxSampledTexturesPerShaderStage / kSampledTexturesPerExternalTexture,
"The number of external textures (%u) in the %s stage exceeds the maximum "
"per-stage limit (%u).",
bindingCounts.perStage[stage].externalTextureCount, stage,
kMaxSampledTexturesPerShaderStage / kSampledTexturesPerExternalTexture);

DAWN_INVALID_IF(
bindingCounts.perStage[stage].sampledTextureCount +
(bindingCounts.perStage[stage].externalTextureCount *
kSampledTexturesPerExternalTexture) >
kMaxSampledTexturesPerShaderStage,
"The combination of sampled textures (%u) and external textures (%u) in the %s "
"stage exceeds the maximum per-stage limit (%u).",
bindingCounts.perStage[stage].sampledTextureCount,
bindingCounts.perStage[stage].externalTextureCount, stage,
kMaxSampledTexturesPerShaderStage);

DAWN_INVALID_IF(
bindingCounts.perStage[stage].samplerCount > kMaxSamplersPerShaderStage,
"The number of samplers (%u) in the %s stage exceeds the maximum per-stage limit "
"(%u).",
bindingCounts.perStage[stage].samplerCount, stage, kMaxSamplersPerShaderStage);

DAWN_INVALID_IF(
bindingCounts.perStage[stage].samplerCount +
(bindingCounts.perStage[stage].externalTextureCount *
kSamplersPerExternalTexture) >
kMaxSamplersPerShaderStage,
"The combination of samplers (%u) and external textures (%u) in the %s stage "
"exceeds the maximum per-stage limit (%u).",
bindingCounts.perStage[stage].samplerCount,
bindingCounts.perStage[stage].externalTextureCount, stage,
kMaxSamplersPerShaderStage);

DAWN_INVALID_IF(
bindingCounts.perStage[stage].storageBufferCount > kMaxStorageBuffersPerShaderStage,
"The number of storage buffers (%u) in the %s stage exceeds the maximum per-stage "
"limit (%u).",
bindingCounts.perStage[stage].storageBufferCount, stage,
kMaxStorageBuffersPerShaderStage);

DAWN_INVALID_IF(
bindingCounts.perStage[stage].storageTextureCount >
kMaxStorageTexturesPerShaderStage,
"The number of storage textures (%u) in the %s stage exceeds the maximum per-stage "
"limit (%u).",
bindingCounts.perStage[stage].storageTextureCount, stage,
kMaxStorageTexturesPerShaderStage);

DAWN_INVALID_IF(
bindingCounts.perStage[stage].uniformBufferCount > kMaxUniformBuffersPerShaderStage,
"The number of uniform buffers (%u) in the %s stage exceeds the maximum per-stage "
"limit (%u).",
bindingCounts.perStage[stage].uniformBufferCount, stage,
kMaxUniformBuffersPerShaderStage);

DAWN_INVALID_IF(
bindingCounts.perStage[stage].uniformBufferCount +
(bindingCounts.perStage[stage].externalTextureCount *
kUniformsPerExternalTexture) >
kMaxUniformBuffersPerShaderStage,
"The combination of uniform buffers (%u) and external textures (%u) in the %s "
"stage exceeds the maximum per-stage limit (%u).",
bindingCounts.perStage[stage].uniformBufferCount,
bindingCounts.perStage[stage].externalTextureCount, stage,
kMaxUniformBuffersPerShaderStage);
}

return {};
Expand Down
5 changes: 5 additions & 0 deletions src/dawn_native/BindingInfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ namespace dawn_native {

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 {
BindingNumber binding;
wgpu::ShaderStage visibility;
Expand Down
Loading

0 comments on commit 538f795

Please sign in to comment.