Various validation error improvements

A grab bag of validation error message improvements in the following
files:

 - Adapter.cpp
 - BackendConnection.cpp
 - BindGroup.cpp
 - CommandBuffer.cpp
 - CommandBufferStateTracker.cpp
 - ComputePipeline.cpp
 - Device.cpp
 - Instance.cpp
 - Limits.cpp
 - Queue.cpp

Bug: dawn:563
Change-Id: Ied9f660fc22302d3fd5af4796de32efec529ca05
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/67001
Commit-Queue: Brandon Jones <bajones@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn_native/Adapter.cpp

@@ -144,8 +144,8 @@ namespace dawn_native {
 
         if (err.IsError()) {
             std::unique_ptr<ErrorData> errorData = err.AcquireError();
-            callback(WGPURequestDeviceStatus_Error, device, errorData->GetMessage().c_str(),
+            callback(WGPURequestDeviceStatus_Error, device,
-                     userdata);
+                     errorData->GetFormattedMessage().c_str(), userdata);
             return;
         }
         WGPURequestDeviceStatus status =
@@ -166,9 +166,11 @@ namespace dawn_native {
         }
 
         if (descriptor != nullptr && descriptor->requiredLimits != nullptr) {
-            DAWN_TRY(ValidateLimits(
+            DAWN_TRY_CONTEXT(
+                ValidateLimits(
                     mUseTieredLimits ? ApplyLimitTiers(mLimits.v1) : mLimits.v1,
-                reinterpret_cast<const RequiredLimits*>(descriptor->requiredLimits)->limits));
+                    reinterpret_cast<const RequiredLimits*>(descriptor->requiredLimits)->limits),
+                "validating required limits");
 
             DAWN_INVALID_IF(descriptor->requiredLimits->nextInChain != nullptr,
                             "nextInChain is not nullptr.");

src/dawn_native/BackendConnection.cpp

@@ -30,7 +30,7 @@ namespace dawn_native {
 
     ResultOrError<std::vector<std::unique_ptr<AdapterBase>>> BackendConnection::DiscoverAdapters(
         const AdapterDiscoveryOptionsBase* options) {
-        return DAWN_VALIDATION_ERROR("DiscoverAdapters not implemented for this backend.");
+        return DAWN_FORMAT_VALIDATION_ERROR("DiscoverAdapters not implemented for this backend.");
     }
 
 }  // namespace dawn_native

src/dawn_native/BindGroup.cpp

@@ -36,10 +36,13 @@ namespace dawn_native {
         MaybeError ValidateBufferBinding(const DeviceBase* device,
                                          const BindGroupEntry& entry,
                                          const BindingInfo& bindingInfo) {
-            if (entry.buffer == nullptr || entry.sampler != nullptr ||
+            DAWN_INVALID_IF(entry.buffer == nullptr, "Binding entry buffer not set.");
-                entry.textureView != nullptr || entry.nextInChain != nullptr) {
+
-                return DAWN_VALIDATION_ERROR("Expected buffer binding");
+            DAWN_INVALID_IF(entry.sampler != nullptr || entry.textureView != nullptr,
-            }
+                            "Expected only buffer to be set for binding entry.");
+
+            DAWN_INVALID_IF(entry.nextInChain != nullptr, "nextInChain must be nullptr.");
+
             DAWN_TRY(device->ValidateObject(entry.buffer));
 
             ASSERT(bindingInfo.bindingType == BindingInfoType::Buffer);
@@ -116,10 +119,13 @@ namespace dawn_native {
         MaybeError ValidateTextureBinding(DeviceBase* device,
                                           const BindGroupEntry& entry,
                                           const BindingInfo& bindingInfo) {
-            if (entry.textureView == nullptr || entry.sampler != nullptr ||
+            DAWN_INVALID_IF(entry.textureView == nullptr, "Binding entry textureView not set.");
-                entry.buffer != nullptr || entry.nextInChain != nullptr) {
+
-                return DAWN_VALIDATION_ERROR("Expected texture binding");
+            DAWN_INVALID_IF(entry.sampler != nullptr || entry.buffer != nullptr,
-            }
+                            "Expected only textureView to be set for binding entry.");
+
+            DAWN_INVALID_IF(entry.nextInChain != nullptr, "nextInChain must be nullptr.");
+
             DAWN_TRY(device->ValidateObject(entry.textureView));
 
             TextureViewBase* view = entry.textureView;
@@ -189,10 +195,13 @@ namespace dawn_native {
         MaybeError ValidateSamplerBinding(const DeviceBase* device,
                                           const BindGroupEntry& entry,
                                           const BindingInfo& bindingInfo) {
-            if (entry.sampler == nullptr || entry.textureView != nullptr ||
+            DAWN_INVALID_IF(entry.sampler == nullptr, "Binding entry sampler not set.");
-                entry.buffer != nullptr || entry.nextInChain != nullptr) {
+
-                return DAWN_VALIDATION_ERROR("Expected sampler binding");
+            DAWN_INVALID_IF(entry.textureView != nullptr || entry.buffer != nullptr,
-            }
+                            "Expected only sampler to be set for binding entry.");
+
+            DAWN_INVALID_IF(entry.nextInChain != nullptr, "nextInChain must be nullptr.");
+
             DAWN_TRY(device->ValidateObject(entry.sampler));
 
             ASSERT(bindingInfo.bindingType == BindingInfoType::Sampler);
@@ -233,10 +242,12 @@ namespace dawn_native {
             const ExternalTextureBindingEntry* externalTextureBindingEntry = nullptr;
             FindInChain(entry.nextInChain, &externalTextureBindingEntry);
 
-            if (entry.sampler != nullptr || entry.textureView != nullptr ||
+            DAWN_INVALID_IF(externalTextureBindingEntry == nullptr,
-                entry.buffer != nullptr || externalTextureBindingEntry == nullptr) {
+                            "Binding entry external texture not set.");
-                return DAWN_VALIDATION_ERROR("Expected external texture binding");
+
-            }
+            DAWN_INVALID_IF(
+                entry.sampler != nullptr || entry.textureView != nullptr || entry.buffer != nullptr,
+                "Expected only external texture to be set for binding entry.");
 
             DAWN_TRY(ValidateSingleSType(externalTextureBindingEntry->nextInChain,
                                          wgpu::SType::ExternalTextureBindingEntry));
@@ -250,9 +261,7 @@ namespace dawn_native {
 
     MaybeError ValidateBindGroupDescriptor(DeviceBase* device,
                                            const BindGroupDescriptor* descriptor) {
-        if (descriptor->nextInChain != nullptr) {
+        DAWN_INVALID_IF(descriptor->nextInChain != nullptr, "nextInChain must be nullptr.");
-            return DAWN_VALIDATION_ERROR("nextInChain must be nullptr");
-        }
 
         DAWN_TRY(device->ValidateObject(descriptor->layout));
 

src/dawn_native/CommandBuffer.cpp

@@ -59,9 +59,7 @@ namespace dawn_native {
     MaybeError CommandBufferBase::ValidateCanUseInSubmitNow() const {
         ASSERT(!IsError());
 
-        if (mDestroyed) {
+        DAWN_INVALID_IF(mDestroyed, "%s cannot be submitted more than once.", this);
-            return DAWN_VALIDATION_ERROR("Command buffer reused in submit");
-        }
         return {};
     }
 

src/dawn_native/CommandBufferStateTracker.cpp

@@ -246,7 +246,7 @@ namespace dawn_native {
             // because to have invalid aspects one of the above conditions must have failed earlier.
             // If this is reached, make sure lazy aspects and the error checks above are consistent.
             UNREACHABLE();
-            return DAWN_VALIDATION_ERROR("Index buffer invalid");
+            return DAWN_FORMAT_VALIDATION_ERROR("Index buffer is invalid.");
         }
 
         // TODO(dawn:563): Indicate which slots were not set.
@@ -277,7 +277,7 @@ namespace dawn_native {
             // because to have invalid aspects one of the above conditions must have failed earlier.
             // If this is reached, make sure lazy aspects and the error checks above are consistent.
             UNREACHABLE();
-            return DAWN_VALIDATION_ERROR("Bind groups invalid");
+            return DAWN_FORMAT_VALIDATION_ERROR("Bind groups are invalid.");
         }
 
         DAWN_INVALID_IF(aspects[VALIDATION_ASPECT_PIPELINE], "No pipeline set.");

src/dawn_native/ComputePipeline.cpp

@@ -23,7 +23,7 @@ namespace dawn_native {
     MaybeError ValidateComputePipelineDescriptor(DeviceBase* device,
                                                  const ComputePipelineDescriptor* descriptor) {
         if (descriptor->nextInChain != nullptr) {
-            return DAWN_VALIDATION_ERROR("nextInChain must be nullptr");
+            return DAWN_FORMAT_VALIDATION_ERROR("nextInChain must be nullptr.");
         }
 
         if (descriptor->layout != nullptr) {

src/dawn_native/Device.cpp

@@ -507,11 +507,9 @@ namespace dawn_native {
     }
 
     MaybeError DeviceBase::ValidateIsAlive() const {
-        if (DAWN_LIKELY(mState == State::Alive)) {
+        DAWN_INVALID_IF(mState != State::Alive, "%s is lost.", this);
         return {};
     }
-        return DAWN_VALIDATION_ERROR("Device is lost");
-    }
 
     void DeviceBase::APILoseForTesting() {
         if (mState != State::Alive) {
@@ -615,14 +613,10 @@ namespace dawn_native {
 
     ResultOrError<const Format*> DeviceBase::GetInternalFormat(wgpu::TextureFormat format) const {
         size_t index = ComputeFormatIndex(format);
-        if (index >= mFormatTable.size()) {
+        DAWN_INVALID_IF(index >= mFormatTable.size(), "Unknown texture format %s.", format);
-            return DAWN_VALIDATION_ERROR("Unknown texture format");
-        }
 
         const Format* internalFormat = &mFormatTable[index];
-        if (!internalFormat->isSupported) {
+        DAWN_INVALID_IF(!internalFormat->isSupported, "Unsupported texture format %s.", format);
-            return DAWN_VALIDATION_ERROR("Unsupported texture format");
-        }
 
         return internalFormat;
     }

src/dawn_native/Instance.cpp

@@ -185,9 +185,7 @@ namespace dawn_native {
             }
         }
 
-        if (!foundBackend) {
+        DAWN_INVALID_IF(!foundBackend, "No matching backend found.");
-            return DAWN_VALIDATION_ERROR("Backend isn't present.");
-        }
         return {};
     }
 

src/dawn_native/Limits.cpp

@@ -98,9 +98,9 @@ namespace dawn_native {
 
             template <typename T>
             static MaybeError Validate(T supported, T required) {
-                if (IsBetter(required, supported)) {
+                DAWN_INVALID_IF(IsBetter(required, supported),
-                    return DAWN_VALIDATION_ERROR("requiredLimit lower than supported limit");
+                                "Required limit (%u) is lower than the supported limit (%u).",
-                }
+                                required, supported);
                 return {};
             }
         };
@@ -114,9 +114,9 @@ namespace dawn_native {
 
             template <typename T>
             static MaybeError Validate(T supported, T required) {
-                if (IsBetter(required, supported)) {
+                DAWN_INVALID_IF(IsBetter(required, supported),
-                    return DAWN_VALIDATION_ERROR("requiredLimit greater than supported limit");
+                                "Required limit (%u) is greater than the supported limit (%u).",
-                }
+                                required, supported);
                 return {};
             }
         };
@@ -165,8 +165,9 @@ namespace dawn_native {
     MaybeError ValidateLimits(const Limits& supportedLimits, const Limits& requiredLimits) {
 #define X(Better, limitName, ...)                                                  \
     if (!IsLimitUndefined(requiredLimits.limitName)) {                             \
-        DAWN_TRY(CheckLimit<LimitBetterDirection::Better>::Validate(supportedLimits.limitName,  \
+        DAWN_TRY_CONTEXT(CheckLimit<LimitBetterDirection::Better>::Validate(       \
-                                                                    requiredLimits.limitName)); \
+                             supportedLimits.limitName, requiredLimits.limitName), \
+                         "validating " #limitName);                                \
     }
         LIMITS(X)
 #undef X

src/dawn_native/Queue.cpp

@@ -434,9 +434,7 @@ namespace dawn_native {
         *status = WGPUQueueWorkDoneStatus_Error;
         DAWN_TRY(GetDevice()->ValidateObject(this));
 
-        if (signalValue != 0) {
+        DAWN_INVALID_IF(signalValue != 0, "SignalValue (%u) is not 0.", signalValue);
-            return DAWN_VALIDATION_ERROR("SignalValue must currently be 0.");
-        }
 
         return {};
     }
@@ -451,17 +449,17 @@ namespace dawn_native {
 
         DAWN_TRY(ValidateImageCopyTexture(GetDevice(), *destination, *writeSize));
 
-        if (dataLayout.offset > dataSize) {
+        DAWN_INVALID_IF(dataLayout.offset > dataSize,
-            return DAWN_VALIDATION_ERROR("Queue::WriteTexture out of range");
+                        "Data offset (%u) is greater than the data size (%u).", dataLayout.offset,
-        }
+                        dataSize);
 
-        if (!(destination->texture->GetUsage() & wgpu::TextureUsage::CopyDst)) {
+        DAWN_INVALID_IF(!(destination->texture->GetUsage() & wgpu::TextureUsage::CopyDst),
-            return DAWN_VALIDATION_ERROR("Texture needs the CopyDst usage bit");
+                        "Usage (%s) of %s does not include %s.", destination->texture->GetUsage(),
-        }
+                        destination->texture, wgpu::TextureUsage::CopyDst);
 
-        if (destination->texture->GetSampleCount() > 1) {
+        DAWN_INVALID_IF(destination->texture->GetSampleCount() > 1,
-            return DAWN_VALIDATION_ERROR("The sample count of textures must be 1");
+                        "Sample count (%u) of %s is not 1", destination->texture->GetSampleCount(),
-        }
+                        destination->texture);
 
         DAWN_TRY(ValidateLinearToDepthStencilCopyRestrictions(*destination));
         // We validate texture copy range before validating linear texture data,