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>

src/dawn_native/opengl/BackendGL.cpp

@@ -278,17 +278,14 @@ namespace dawn_native { namespace opengl {
         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) {
+        DAWN_INVALID_IF(mCreatedAdapter, "The OpenGL backend can only create a single adapter.");
-            return DAWN_VALIDATION_ERROR("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) {
+        DAWN_INVALID_IF(options->getProc == nullptr,
-            return DAWN_VALIDATION_ERROR("AdapterDiscoveryOptions::getProc must be set");
+                        "AdapterDiscoveryOptions::getProc must be set");
-        }
 
         std::unique_ptr<Adapter> adapter = std::make_unique<Adapter>(
             GetInstance(), static_cast<wgpu::BackendType>(optionsBase->backendType));

src/dawn_native/opengl/BindGroupGL.cpp

@@ -33,12 +33,13 @@ namespace dawn_native { namespace opengl {
             if (bindingInfo.bindingType == BindingInfoType::StorageTexture) {
                 ASSERT(entry.textureView != nullptr);
                 const uint32_t textureViewLayerCount = entry.textureView->GetLayerCount();
-                if (textureViewLayerCount != 1 &&
+                DAWN_INVALID_IF(
-                    textureViewLayerCount != entry.textureView->GetTexture()->GetArrayLayers()) {
+                    textureViewLayerCount != 1 &&
-                    return DAWN_VALIDATION_ERROR(
+                        textureViewLayerCount != entry.textureView->GetTexture()->GetArrayLayers(),
-                        "Currently the OpenGL backend only supports either binding a layer or "
+                    "%s binds %u layers. Currently the OpenGL backend only supports either binding "
-                        "the entire texture as storage texture.");
+                    "1 layer or the all layers (%u) for storage texture.",
-                }
+                    entry.textureView, textureViewLayerCount,
+                    entry.textureView->GetTexture()->GetArrayLayers());
             }
         }
 

src/dawn_native/opengl/CommandBufferGL.cpp

@@ -645,10 +645,10 @@ namespace dawn_native { namespace opengl {
                     auto& dst = copy->destination;
                     Buffer* buffer = ToBackend(src.buffer.Get());
 
-                    if (dst.aspect == Aspect::Stencil) {
+                    DAWN_INVALID_IF(
-                        return DAWN_VALIDATION_ERROR(
+                        dst.aspect == Aspect::Stencil,
-                            "Copies to stencil textures unsupported on OpenGL");
+                        "Copies to stencil textures are unsupported on the OpenGL backend.");
-                    }
+
                     ASSERT(dst.aspect == Aspect::Color);
 
                     buffer->EnsureDataInitialized();

src/dawn_native/opengl/DeviceGL.cpp

@@ -162,7 +162,7 @@ namespace dawn_native { namespace opengl {
         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 @@ namespace dawn_native { namespace opengl {
 
     MaybeError Device::ValidateEGLImageCanBeWrapped(const TextureDescriptor* descriptor,
                                                     ::EGLImage image) {
-        if (descriptor->dimension != wgpu::TextureDimension::e2D) {
+        DAWN_INVALID_IF(descriptor->dimension != wgpu::TextureDimension::e2D,
-            return DAWN_VALIDATION_ERROR("EGLImage texture must be 2D");
+                        "Texture dimension (%s) is not %s.", descriptor->dimension,
-        }
+                        wgpu::TextureDimension::e2D);
 
-        if (descriptor->usage &
+        DAWN_INVALID_IF(descriptor->mipLevelCount != 1, "Mip level count (%u) is not 1.",
-            (wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::StorageBinding)) {
+                        descriptor->mipLevelCount);
-            return DAWN_VALIDATION_ERROR("EGLImage texture cannot have sampled or storage usage");
-        }
 
-        if (descriptor->mipLevelCount != 1) {
+        DAWN_INVALID_IF(descriptor->size.depthOrArrayLayers != 1,
-            return DAWN_VALIDATION_ERROR("EGLImage mip level count must be 1");
+                        "Array layer count (%u) is not 1.", descriptor->size.depthOrArrayLayers);
-        }
 
-        if (descriptor->size.depthOrArrayLayers != 1) {
+        DAWN_INVALID_IF(descriptor->sampleCount != 1, "Sample count (%u) is not 1.",
-            return DAWN_VALIDATION_ERROR("EGLImage array layer count must be 1");
+                        descriptor->sampleCount);
-        }
 
-        if (descriptor->sampleCount != 1) {
+        DAWN_INVALID_IF(descriptor->usage & (wgpu::TextureUsage::TextureBinding |
-            return DAWN_VALIDATION_ERROR("EGLImage sample count must be 1");
+                                             wgpu::TextureUsage::StorageBinding),
-        }
+                        "Texture usage (%s) cannot have %s or %s.", descriptor->usage,
+                        wgpu::TextureUsage::TextureBinding, wgpu::TextureUsage::StorageBinding);
 
         return {};
     }
@@ -229,7 +226,9 @@ namespace dawn_native { namespace opengl {
         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;
         }

src/dawn_native/opengl/PipelineGL.cpp

@@ -68,9 +68,8 @@ namespace dawn_native { namespace opengl {
                 if (infoLogLength > 1) {
                     std::vector<char> buffer(infoLogLength);
                     gl.GetShaderInfoLog(shader, infoLogLength, nullptr, &buffer[0]);
-                    std::stringstream ss;
+                    return DAWN_FORMAT_VALIDATION_ERROR("%s\nProgram compilation failed:\n%s",
-                    ss << source << "\nProgram compilation failed:\n" << buffer.data();
+                                                        source, buffer.data());
-                    return DAWN_VALIDATION_ERROR(ss.str().c_str());
                 }
             }
             return shader;
@@ -123,9 +122,7 @@ namespace dawn_native { namespace opengl {
             if (infoLogLength > 1) {
                 std::vector<char> buffer(infoLogLength);
                 gl.GetProgramInfoLog(mProgram, infoLogLength, nullptr, &buffer[0]);
-                std::stringstream ss;
+                return DAWN_FORMAT_VALIDATION_ERROR("Program link failed:\n%s", buffer.data());
-                ss << "Program link failed:\n" << buffer.data();
-                return DAWN_VALIDATION_ERROR(ss.str().c_str());
             }
         }
 

src/dawn_native/opengl/QueueGL.cpp

@@ -56,9 +56,8 @@ namespace dawn_native { namespace opengl {
                                        const void* data,
                                        const TextureDataLayout& dataLayout,
                                        const Extent3D& writeSizePixel) {
-        if (destination.aspect == wgpu::TextureAspect::StencilOnly) {
+        DAWN_INVALID_IF(destination.aspect == wgpu::TextureAspect::StencilOnly,
-            return DAWN_VALIDATION_ERROR("Writes to stencil textures unsupported on OpenGL");
+                        "Writes to stencil textures unsupported on the OpenGL backend.");
-        }
 
         TextureCopy textureCopy;
         textureCopy.texture = destination.texture;

src/dawn_native/opengl/ShaderModuleGL.cpp

@@ -80,29 +80,25 @@ namespace dawn_native { namespace opengl {
                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)) {
+                DAWN_INVALID_IF(
-                    return DAWN_VALIDATION_ERROR("No Binding decoration set for resource");
+                    !compiler.get_decoration_bitset(resource.id).get(spv::DecorationBinding),
-                }
+                    "No Binding decoration set for resource");
 
-                if (!compiler.get_decoration_bitset(resource.id)
+                DAWN_INVALID_IF(
-                         .get(spv::DecorationDescriptorSet)) {
+                    !compiler.get_decoration_bitset(resource.id).get(spv::DecorationDescriptorSet),
-                    return DAWN_VALIDATION_ERROR("No Descriptor Decoration set for resource");
+                    "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) {
+                DAWN_INVALID_IF(bindGroupIndex >= kMaxBindGroupsTyped,
-                    return DAWN_VALIDATION_ERROR("Bind group index over limits in the SPIRV");
+                                "Bind group index over limits in the SPIRV");
-                }
 
                 const auto& it =
                     (*bindings)[bindGroupIndex].emplace(bindingNumber, ShaderBindingInfo{});
-                if (!it.second) {
+                DAWN_INVALID_IF(!it.second, "Shader has duplicate bindings");
-                    return DAWN_VALIDATION_ERROR("Shader has duplicate bindings");
-                }
 
                 ShaderBindingInfo* info = &it.first->second;
                 info->id = resource.id;
@@ -123,17 +119,14 @@ namespace dawn_native { namespace opengl {
                             SpirvBaseTypeToSampleTypeBit(textureComponentType);
 
                         if (imageType.depth) {
-                            if ((info->texture.compatibleSampleTypes & SampleTypeBit::Float) == 0) {
+                            DAWN_INVALID_IF(
-                                return DAWN_VALIDATION_ERROR(
+                                (info->texture.compatibleSampleTypes & SampleTypeBit::Float) == 0,
-                                    "Depth textures must have a float type");
+                                "Depth textures must have a float type");
-                            }
                             info->texture.compatibleSampleTypes = SampleTypeBit::Depth;
                         }
 
-                        if (imageType.ms && imageType.arrayed) {
+                        DAWN_INVALID_IF(imageType.ms && imageType.arrayed,
-                            return DAWN_VALIDATION_ERROR(
+                                        "Multisampled array textures aren't supported");
-                                "Multisampled array textures aren't supported");
-                        }
                         break;
                     }
                     case BindingInfoType::Buffer: {
@@ -162,33 +155,28 @@ namespace dawn_native { namespace opengl {
                     }
                     case BindingInfoType::StorageTexture: {
                         spirv_cross::Bitset flags = compiler.get_decoration_bitset(resource.id);
-                        if (flags.get(spv::DecorationNonReadable)) {
+                        DAWN_INVALID_IF(!flags.get(spv::DecorationNonReadable),
-                            info->storageTexture.access = wgpu::StorageTextureAccess::WriteOnly;
+                                        "Read-write storage textures are not supported.");
-                        } else {
+                        info->storageTexture.access = wgpu::StorageTextureAccess::WriteOnly;
-                            return DAWN_VALIDATION_ERROR(
-                                "Read-write storage textures are not supported");
-                        }
 
                         spirv_cross::SPIRType::ImageType imageType =
                             compiler.get_type(info->base_type_id).image;
                         wgpu::TextureFormat storageTextureFormat =
                             SpirvImageFormatToTextureFormat(imageType.format);
-                        if (storageTextureFormat == wgpu::TextureFormat::Undefined) {
+                        DAWN_INVALID_IF(storageTextureFormat == wgpu::TextureFormat::Undefined,
-                            return DAWN_VALIDATION_ERROR(
+                                        "Invalid image format declaration on storage image.");
-                                "Invalid image format declaration on storage image");
+
-                        }
                         const Format& format = device->GetValidInternalFormat(storageTextureFormat);
-                        if (!format.supportsStorageUsage) {
+                        DAWN_INVALID_IF(!format.supportsStorageUsage,
-                            return DAWN_VALIDATION_ERROR(
+                                        "The storage texture format (%s) is not supported.",
-                                "The storage texture format is not supported");
+                                        storageTextureFormat);
-                        }
+
-                        if (imageType.ms) {
+                        DAWN_INVALID_IF(imageType.ms,
-                            return DAWN_VALIDATION_ERROR(
+                                        "Multisampled storage textures aren't supported.");
-                                "Multisampled storage textures aren't supported");
+
-                        }
+                        DAWN_INVALID_IF(imageType.depth,
-                        if (imageType.depth) {
+                                        "Depth storage textures aren't supported.");
-                            return DAWN_VALIDATION_ERROR("Depth storage textures aren't supported");
+
-                        }
                         info->storageTexture.format = storageTextureFormat;
                         info->storageTexture.viewDimension =
                             SpirvDimToTextureViewDimension(imageType.dim, imageType.arrayed);
@@ -199,7 +187,7 @@ namespace dawn_native { namespace opengl {
                         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 @@ namespace dawn_native { namespace opengl {
         tint::writer::spirv::Options options;
         options.disable_workgroup_init = GetDevice()->IsToggleEnabled(Toggle::DisableWorkgroupInit);
         auto result = tint::writer::spirv::Generate(GetTintProgram(), options);
-        if (!result.success) {
+        DAWN_INVALID_IF(!result.success, "An error occured while generating SPIR-V: %s.",
-            std::ostringstream errorStream;
+                        result.error);
-            errorStream << "Generator: " << result.error << std::endl;
-            return DAWN_VALIDATION_ERROR(errorStream.str().c_str());
-        }
 
         DAWN_TRY_ASSIGN(mGLBindings, ReflectShaderUsingSPIRVCross(GetDevice(), result.spirv));
 
@@ -293,11 +278,8 @@ namespace dawn_native { namespace opengl {
         tintOptions.disable_workgroup_init =
             GetDevice()->IsToggleEnabled(Toggle::DisableWorkgroupInit);
         auto result = tint::writer::spirv::Generate(&program, tintOptions);
-        if (!result.success) {
+        DAWN_INVALID_IF(!result.success, "An error occured while generating SPIR-V: %s.",
-            std::ostringstream errorStream;
+                        result.error);
-            errorStream << "Generator: " << result.error << std::endl;
-            return DAWN_VALIDATION_ERROR(errorStream.str().c_str());
-        }
 
         std::vector<uint32_t> spirv = std::move(result.spirv);
         DAWN_TRY(