Improving OpenGL backend validation messages.
Improves validation messages in various OpenGL backend files:
- BackendGL.cpp
- BindGroupGL.cpp
- CommandBufferGL.cpp
- DeviceGL.cpp
- PipelineGL.cpp
- QueueGL.cpp
- ShaderModuleGL.cpp
Bug: dawn:563
Change-Id: Idd5751b6f68ea435e5f3c045dcbfd0e5c049fce6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/67144
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Brandon Jones <bajones@google.com>
diff --git a/src/dawn_native/opengl/BackendGL.cpp b/src/dawn_native/opengl/BackendGL.cpp
index 9a66779..1907162 100644
--- a/src/dawn_native/opengl/BackendGL.cpp
+++ b/src/dawn_native/opengl/BackendGL.cpp
@@ -278,17 +278,14 @@
const AdapterDiscoveryOptionsBase* optionsBase) {
// TODO(cwallez@chromium.org): For now only create a single OpenGL adapter because don't
// know how to handle MakeCurrent.
- if (mCreatedAdapter) {
- return DAWN_VALIDATION_ERROR("The OpenGL backend can only create a single adapter");
- }
+ DAWN_INVALID_IF(mCreatedAdapter, "The OpenGL backend can only create a single adapter.");
ASSERT(static_cast<wgpu::BackendType>(optionsBase->backendType) == GetType());
const AdapterDiscoveryOptions* options =
static_cast<const AdapterDiscoveryOptions*>(optionsBase);
- if (options->getProc == nullptr) {
- return DAWN_VALIDATION_ERROR("AdapterDiscoveryOptions::getProc must be set");
- }
+ DAWN_INVALID_IF(options->getProc == nullptr,
+ "AdapterDiscoveryOptions::getProc must be set");
std::unique_ptr<Adapter> adapter = std::make_unique<Adapter>(
GetInstance(), static_cast<wgpu::BackendType>(optionsBase->backendType));
diff --git a/src/dawn_native/opengl/BindGroupGL.cpp b/src/dawn_native/opengl/BindGroupGL.cpp
index 721189b..c726f29 100644
--- a/src/dawn_native/opengl/BindGroupGL.cpp
+++ b/src/dawn_native/opengl/BindGroupGL.cpp
@@ -33,12 +33,13 @@
if (bindingInfo.bindingType == BindingInfoType::StorageTexture) {
ASSERT(entry.textureView != nullptr);
const uint32_t textureViewLayerCount = entry.textureView->GetLayerCount();
- if (textureViewLayerCount != 1 &&
- textureViewLayerCount != entry.textureView->GetTexture()->GetArrayLayers()) {
- return DAWN_VALIDATION_ERROR(
- "Currently the OpenGL backend only supports either binding a layer or "
- "the entire texture as storage texture.");
- }
+ DAWN_INVALID_IF(
+ textureViewLayerCount != 1 &&
+ textureViewLayerCount != entry.textureView->GetTexture()->GetArrayLayers(),
+ "%s binds %u layers. Currently the OpenGL backend only supports either binding "
+ "1 layer or the all layers (%u) for storage texture.",
+ entry.textureView, textureViewLayerCount,
+ entry.textureView->GetTexture()->GetArrayLayers());
}
}
diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp
index 788eb9e..b34935a 100644
--- a/src/dawn_native/opengl/CommandBufferGL.cpp
+++ b/src/dawn_native/opengl/CommandBufferGL.cpp
@@ -645,10 +645,10 @@
auto& dst = copy->destination;
Buffer* buffer = ToBackend(src.buffer.Get());
- if (dst.aspect == Aspect::Stencil) {
- return DAWN_VALIDATION_ERROR(
- "Copies to stencil textures unsupported on OpenGL");
- }
+ DAWN_INVALID_IF(
+ dst.aspect == Aspect::Stencil,
+ "Copies to stencil textures are unsupported on the OpenGL backend.");
+
ASSERT(dst.aspect == Aspect::Color);
buffer->EnsureDataInitialized();
diff --git a/src/dawn_native/opengl/DeviceGL.cpp b/src/dawn_native/opengl/DeviceGL.cpp
index 08e544c..db5e06e 100644
--- a/src/dawn_native/opengl/DeviceGL.cpp
+++ b/src/dawn_native/opengl/DeviceGL.cpp
@@ -162,7 +162,7 @@
Surface* surface,
NewSwapChainBase* previousSwapChain,
const SwapChainDescriptor* descriptor) {
- return DAWN_VALIDATION_ERROR("New swapchains not implemented.");
+ return DAWN_FORMAT_VALIDATION_ERROR("New swapchains not implemented.");
}
ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
return AcquireRef(new Texture(this, descriptor));
@@ -181,26 +181,23 @@
MaybeError Device::ValidateEGLImageCanBeWrapped(const TextureDescriptor* descriptor,
::EGLImage image) {
- if (descriptor->dimension != wgpu::TextureDimension::e2D) {
- return DAWN_VALIDATION_ERROR("EGLImage texture must be 2D");
- }
+ DAWN_INVALID_IF(descriptor->dimension != wgpu::TextureDimension::e2D,
+ "Texture dimension (%s) is not %s.", descriptor->dimension,
+ wgpu::TextureDimension::e2D);
- if (descriptor->usage &
- (wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::StorageBinding)) {
- return DAWN_VALIDATION_ERROR("EGLImage texture cannot have sampled or storage usage");
- }
+ DAWN_INVALID_IF(descriptor->mipLevelCount != 1, "Mip level count (%u) is not 1.",
+ descriptor->mipLevelCount);
- if (descriptor->mipLevelCount != 1) {
- return DAWN_VALIDATION_ERROR("EGLImage mip level count must be 1");
- }
+ DAWN_INVALID_IF(descriptor->size.depthOrArrayLayers != 1,
+ "Array layer count (%u) is not 1.", descriptor->size.depthOrArrayLayers);
- if (descriptor->size.depthOrArrayLayers != 1) {
- return DAWN_VALIDATION_ERROR("EGLImage array layer count must be 1");
- }
+ DAWN_INVALID_IF(descriptor->sampleCount != 1, "Sample count (%u) is not 1.",
+ descriptor->sampleCount);
- if (descriptor->sampleCount != 1) {
- return DAWN_VALIDATION_ERROR("EGLImage sample count must be 1");
- }
+ DAWN_INVALID_IF(descriptor->usage & (wgpu::TextureUsage::TextureBinding |
+ wgpu::TextureUsage::StorageBinding),
+ "Texture usage (%s) cannot have %s or %s.", descriptor->usage,
+ wgpu::TextureUsage::TextureBinding, wgpu::TextureUsage::StorageBinding);
return {};
}
@@ -229,7 +226,9 @@
if (textureDescriptor->size.width != static_cast<uint32_t>(width) ||
textureDescriptor->size.height != static_cast<uint32_t>(height) ||
textureDescriptor->size.depthOrArrayLayers != 1) {
- ConsumedError(DAWN_VALIDATION_ERROR("EGLImage size doesn't match descriptor"));
+ ConsumedError(DAWN_FORMAT_VALIDATION_ERROR(
+ "EGLImage size (width: %u, height: %u, depth: 1) doesn't match descriptor size %s.",
+ width, height, &textureDescriptor->size));
gl.DeleteTextures(1, &tex);
return nullptr;
}
diff --git a/src/dawn_native/opengl/PipelineGL.cpp b/src/dawn_native/opengl/PipelineGL.cpp
index c2fcbe6..614a125 100644
--- a/src/dawn_native/opengl/PipelineGL.cpp
+++ b/src/dawn_native/opengl/PipelineGL.cpp
@@ -68,9 +68,8 @@
if (infoLogLength > 1) {
std::vector<char> buffer(infoLogLength);
gl.GetShaderInfoLog(shader, infoLogLength, nullptr, &buffer[0]);
- std::stringstream ss;
- ss << source << "\nProgram compilation failed:\n" << buffer.data();
- return DAWN_VALIDATION_ERROR(ss.str().c_str());
+ return DAWN_FORMAT_VALIDATION_ERROR("%s\nProgram compilation failed:\n%s",
+ source, buffer.data());
}
}
return shader;
@@ -123,9 +122,7 @@
if (infoLogLength > 1) {
std::vector<char> buffer(infoLogLength);
gl.GetProgramInfoLog(mProgram, infoLogLength, nullptr, &buffer[0]);
- std::stringstream ss;
- ss << "Program link failed:\n" << buffer.data();
- return DAWN_VALIDATION_ERROR(ss.str().c_str());
+ return DAWN_FORMAT_VALIDATION_ERROR("Program link failed:\n%s", buffer.data());
}
}
diff --git a/src/dawn_native/opengl/QueueGL.cpp b/src/dawn_native/opengl/QueueGL.cpp
index 977f8bf..ec98b64 100644
--- a/src/dawn_native/opengl/QueueGL.cpp
+++ b/src/dawn_native/opengl/QueueGL.cpp
@@ -56,9 +56,8 @@
const void* data,
const TextureDataLayout& dataLayout,
const Extent3D& writeSizePixel) {
- if (destination.aspect == wgpu::TextureAspect::StencilOnly) {
- return DAWN_VALIDATION_ERROR("Writes to stencil textures unsupported on OpenGL");
- }
+ DAWN_INVALID_IF(destination.aspect == wgpu::TextureAspect::StencilOnly,
+ "Writes to stencil textures unsupported on the OpenGL backend.");
TextureCopy textureCopy;
textureCopy.texture = destination.texture;
diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp
index 5019593..414fb79 100644
--- a/src/dawn_native/opengl/ShaderModuleGL.cpp
+++ b/src/dawn_native/opengl/ShaderModuleGL.cpp
@@ -80,29 +80,25 @@
const spirv_cross::Compiler& compiler, BindingInfoType bindingType,
BindingInfoArray* bindings, bool isStorageBuffer = false) -> MaybeError {
for (const auto& resource : resources) {
- if (!compiler.get_decoration_bitset(resource.id).get(spv::DecorationBinding)) {
- return DAWN_VALIDATION_ERROR("No Binding decoration set for resource");
- }
+ DAWN_INVALID_IF(
+ !compiler.get_decoration_bitset(resource.id).get(spv::DecorationBinding),
+ "No Binding decoration set for resource");
- if (!compiler.get_decoration_bitset(resource.id)
- .get(spv::DecorationDescriptorSet)) {
- return DAWN_VALIDATION_ERROR("No Descriptor Decoration set for resource");
- }
+ DAWN_INVALID_IF(
+ !compiler.get_decoration_bitset(resource.id).get(spv::DecorationDescriptorSet),
+ "No Descriptor Decoration set for resource");
BindingNumber bindingNumber(
compiler.get_decoration(resource.id, spv::DecorationBinding));
BindGroupIndex bindGroupIndex(
compiler.get_decoration(resource.id, spv::DecorationDescriptorSet));
- if (bindGroupIndex >= kMaxBindGroupsTyped) {
- return DAWN_VALIDATION_ERROR("Bind group index over limits in the SPIRV");
- }
+ DAWN_INVALID_IF(bindGroupIndex >= kMaxBindGroupsTyped,
+ "Bind group index over limits in the SPIRV");
const auto& it =
(*bindings)[bindGroupIndex].emplace(bindingNumber, ShaderBindingInfo{});
- if (!it.second) {
- return DAWN_VALIDATION_ERROR("Shader has duplicate bindings");
- }
+ DAWN_INVALID_IF(!it.second, "Shader has duplicate bindings");
ShaderBindingInfo* info = &it.first->second;
info->id = resource.id;
@@ -123,17 +119,14 @@
SpirvBaseTypeToSampleTypeBit(textureComponentType);
if (imageType.depth) {
- if ((info->texture.compatibleSampleTypes & SampleTypeBit::Float) == 0) {
- return DAWN_VALIDATION_ERROR(
- "Depth textures must have a float type");
- }
+ DAWN_INVALID_IF(
+ (info->texture.compatibleSampleTypes & SampleTypeBit::Float) == 0,
+ "Depth textures must have a float type");
info->texture.compatibleSampleTypes = SampleTypeBit::Depth;
}
- if (imageType.ms && imageType.arrayed) {
- return DAWN_VALIDATION_ERROR(
- "Multisampled array textures aren't supported");
- }
+ DAWN_INVALID_IF(imageType.ms && imageType.arrayed,
+ "Multisampled array textures aren't supported");
break;
}
case BindingInfoType::Buffer: {
@@ -162,33 +155,28 @@
}
case BindingInfoType::StorageTexture: {
spirv_cross::Bitset flags = compiler.get_decoration_bitset(resource.id);
- if (flags.get(spv::DecorationNonReadable)) {
- info->storageTexture.access = wgpu::StorageTextureAccess::WriteOnly;
- } else {
- return DAWN_VALIDATION_ERROR(
- "Read-write storage textures are not supported");
- }
+ DAWN_INVALID_IF(!flags.get(spv::DecorationNonReadable),
+ "Read-write storage textures are not supported.");
+ info->storageTexture.access = wgpu::StorageTextureAccess::WriteOnly;
spirv_cross::SPIRType::ImageType imageType =
compiler.get_type(info->base_type_id).image;
wgpu::TextureFormat storageTextureFormat =
SpirvImageFormatToTextureFormat(imageType.format);
- if (storageTextureFormat == wgpu::TextureFormat::Undefined) {
- return DAWN_VALIDATION_ERROR(
- "Invalid image format declaration on storage image");
- }
+ DAWN_INVALID_IF(storageTextureFormat == wgpu::TextureFormat::Undefined,
+ "Invalid image format declaration on storage image.");
+
const Format& format = device->GetValidInternalFormat(storageTextureFormat);
- if (!format.supportsStorageUsage) {
- return DAWN_VALIDATION_ERROR(
- "The storage texture format is not supported");
- }
- if (imageType.ms) {
- return DAWN_VALIDATION_ERROR(
- "Multisampled storage textures aren't supported");
- }
- if (imageType.depth) {
- return DAWN_VALIDATION_ERROR("Depth storage textures aren't supported");
- }
+ DAWN_INVALID_IF(!format.supportsStorageUsage,
+ "The storage texture format (%s) is not supported.",
+ storageTextureFormat);
+
+ DAWN_INVALID_IF(imageType.ms,
+ "Multisampled storage textures aren't supported.");
+
+ DAWN_INVALID_IF(imageType.depth,
+ "Depth storage textures aren't supported.");
+
info->storageTexture.format = storageTextureFormat;
info->storageTexture.viewDimension =
SpirvDimToTextureViewDimension(imageType.dim, imageType.arrayed);
@@ -199,7 +187,7 @@
break;
}
case BindingInfoType::ExternalTexture: {
- return DAWN_VALIDATION_ERROR("External textures are not supported.");
+ return DAWN_FORMAT_VALIDATION_ERROR("External textures are not supported.");
}
}
}
@@ -264,11 +252,8 @@
tint::writer::spirv::Options options;
options.disable_workgroup_init = GetDevice()->IsToggleEnabled(Toggle::DisableWorkgroupInit);
auto result = tint::writer::spirv::Generate(GetTintProgram(), options);
- if (!result.success) {
- std::ostringstream errorStream;
- errorStream << "Generator: " << result.error << std::endl;
- return DAWN_VALIDATION_ERROR(errorStream.str().c_str());
- }
+ DAWN_INVALID_IF(!result.success, "An error occured while generating SPIR-V: %s.",
+ result.error);
DAWN_TRY_ASSIGN(mGLBindings, ReflectShaderUsingSPIRVCross(GetDevice(), result.spirv));
@@ -293,11 +278,8 @@
tintOptions.disable_workgroup_init =
GetDevice()->IsToggleEnabled(Toggle::DisableWorkgroupInit);
auto result = tint::writer::spirv::Generate(&program, tintOptions);
- if (!result.success) {
- std::ostringstream errorStream;
- errorStream << "Generator: " << result.error << std::endl;
- return DAWN_VALIDATION_ERROR(errorStream.str().c_str());
- }
+ DAWN_INVALID_IF(!result.success, "An error occured while generating SPIR-V: %s.",
+ result.error);
std::vector<uint32_t> spirv = std::move(result.spirv);
DAWN_TRY(
