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>
diff --git a/src/dawn_native/Adapter.cpp b/src/dawn_native/Adapter.cpp
index d83871b..bdbfc2c 100644
--- a/src/dawn_native/Adapter.cpp
+++ b/src/dawn_native/Adapter.cpp
@@ -144,8 +144,8 @@
if (err.IsError()) {
std::unique_ptr<ErrorData> errorData = err.AcquireError();
- callback(WGPURequestDeviceStatus_Error, device, errorData->GetMessage().c_str(),
- userdata);
+ callback(WGPURequestDeviceStatus_Error, device,
+ errorData->GetFormattedMessage().c_str(), userdata);
return;
}
WGPURequestDeviceStatus status =
@@ -166,9 +166,11 @@
}
if (descriptor != nullptr && descriptor->requiredLimits != nullptr) {
- DAWN_TRY(ValidateLimits(
- mUseTieredLimits ? ApplyLimitTiers(mLimits.v1) : mLimits.v1,
- reinterpret_cast<const RequiredLimits*>(descriptor->requiredLimits)->limits));
+ DAWN_TRY_CONTEXT(
+ ValidateLimits(
+ mUseTieredLimits ? ApplyLimitTiers(mLimits.v1) : mLimits.v1,
+ reinterpret_cast<const RequiredLimits*>(descriptor->requiredLimits)->limits),
+ "validating required limits");
DAWN_INVALID_IF(descriptor->requiredLimits->nextInChain != nullptr,
"nextInChain is not nullptr.");
diff --git a/src/dawn_native/BackendConnection.cpp b/src/dawn_native/BackendConnection.cpp
index 09ef4ef..89379cb 100644
--- a/src/dawn_native/BackendConnection.cpp
+++ b/src/dawn_native/BackendConnection.cpp
@@ -30,7 +30,7 @@
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
diff --git a/src/dawn_native/BindGroup.cpp b/src/dawn_native/BindGroup.cpp
index 9d248ea..361ca82 100644
--- a/src/dawn_native/BindGroup.cpp
+++ b/src/dawn_native/BindGroup.cpp
@@ -36,10 +36,13 @@
MaybeError ValidateBufferBinding(const DeviceBase* device,
const BindGroupEntry& entry,
const BindingInfo& bindingInfo) {
- if (entry.buffer == nullptr || entry.sampler != nullptr ||
- entry.textureView != nullptr || entry.nextInChain != nullptr) {
- return DAWN_VALIDATION_ERROR("Expected buffer binding");
- }
+ DAWN_INVALID_IF(entry.buffer == nullptr, "Binding entry buffer not set.");
+
+ 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 @@
MaybeError ValidateTextureBinding(DeviceBase* device,
const BindGroupEntry& entry,
const BindingInfo& bindingInfo) {
- if (entry.textureView == nullptr || entry.sampler != nullptr ||
- entry.buffer != nullptr || entry.nextInChain != nullptr) {
- return DAWN_VALIDATION_ERROR("Expected texture binding");
- }
+ DAWN_INVALID_IF(entry.textureView == nullptr, "Binding entry textureView not set.");
+
+ 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 @@
MaybeError ValidateSamplerBinding(const DeviceBase* device,
const BindGroupEntry& entry,
const BindingInfo& bindingInfo) {
- if (entry.sampler == nullptr || entry.textureView != nullptr ||
- entry.buffer != nullptr || entry.nextInChain != nullptr) {
- return DAWN_VALIDATION_ERROR("Expected sampler binding");
- }
+ DAWN_INVALID_IF(entry.sampler == nullptr, "Binding entry sampler not set.");
+
+ 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 @@
const ExternalTextureBindingEntry* externalTextureBindingEntry = nullptr;
FindInChain(entry.nextInChain, &externalTextureBindingEntry);
- if (entry.sampler != nullptr || entry.textureView != nullptr ||
- entry.buffer != nullptr || externalTextureBindingEntry == nullptr) {
- return DAWN_VALIDATION_ERROR("Expected external texture binding");
- }
+ DAWN_INVALID_IF(externalTextureBindingEntry == nullptr,
+ "Binding entry external texture not set.");
+
+ 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 @@
MaybeError ValidateBindGroupDescriptor(DeviceBase* device,
const BindGroupDescriptor* descriptor) {
- if (descriptor->nextInChain != nullptr) {
- return DAWN_VALIDATION_ERROR("nextInChain must be nullptr");
- }
+ DAWN_INVALID_IF(descriptor->nextInChain != nullptr, "nextInChain must be nullptr.");
DAWN_TRY(device->ValidateObject(descriptor->layout));
diff --git a/src/dawn_native/CommandBuffer.cpp b/src/dawn_native/CommandBuffer.cpp
index 5b07bb6..18fef0d 100644
--- a/src/dawn_native/CommandBuffer.cpp
+++ b/src/dawn_native/CommandBuffer.cpp
@@ -59,9 +59,7 @@
MaybeError CommandBufferBase::ValidateCanUseInSubmitNow() const {
ASSERT(!IsError());
- if (mDestroyed) {
- return DAWN_VALIDATION_ERROR("Command buffer reused in submit");
- }
+ DAWN_INVALID_IF(mDestroyed, "%s cannot be submitted more than once.", this);
return {};
}
diff --git a/src/dawn_native/CommandBufferStateTracker.cpp b/src/dawn_native/CommandBufferStateTracker.cpp
index cff63f6..5210936 100644
--- a/src/dawn_native/CommandBufferStateTracker.cpp
+++ b/src/dawn_native/CommandBufferStateTracker.cpp
@@ -246,7 +246,7 @@
// 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 @@
// 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.");
diff --git a/src/dawn_native/ComputePipeline.cpp b/src/dawn_native/ComputePipeline.cpp
index d79d4a0..72addc4 100644
--- a/src/dawn_native/ComputePipeline.cpp
+++ b/src/dawn_native/ComputePipeline.cpp
@@ -23,7 +23,7 @@
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) {
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index d5ae480..84ad2d4 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -507,10 +507,8 @@
}
MaybeError DeviceBase::ValidateIsAlive() const {
- if (DAWN_LIKELY(mState == State::Alive)) {
- return {};
- }
- return DAWN_VALIDATION_ERROR("Device is lost");
+ DAWN_INVALID_IF(mState != State::Alive, "%s is lost.", this);
+ return {};
}
void DeviceBase::APILoseForTesting() {
@@ -615,14 +613,10 @@
ResultOrError<const Format*> DeviceBase::GetInternalFormat(wgpu::TextureFormat format) const {
size_t index = ComputeFormatIndex(format);
- if (index >= mFormatTable.size()) {
- return DAWN_VALIDATION_ERROR("Unknown texture format");
- }
+ DAWN_INVALID_IF(index >= mFormatTable.size(), "Unknown texture format %s.", format);
const Format* internalFormat = &mFormatTable[index];
- if (!internalFormat->isSupported) {
- return DAWN_VALIDATION_ERROR("Unsupported texture format");
- }
+ DAWN_INVALID_IF(!internalFormat->isSupported, "Unsupported texture format %s.", format);
return internalFormat;
}
diff --git a/src/dawn_native/Instance.cpp b/src/dawn_native/Instance.cpp
index 936471e..f2b08c7 100644
--- a/src/dawn_native/Instance.cpp
+++ b/src/dawn_native/Instance.cpp
@@ -185,9 +185,7 @@
}
}
- if (!foundBackend) {
- return DAWN_VALIDATION_ERROR("Backend isn't present.");
- }
+ DAWN_INVALID_IF(!foundBackend, "No matching backend found.");
return {};
}
diff --git a/src/dawn_native/Limits.cpp b/src/dawn_native/Limits.cpp
index 5c55a55..949ce83 100644
--- a/src/dawn_native/Limits.cpp
+++ b/src/dawn_native/Limits.cpp
@@ -98,9 +98,9 @@
template <typename T>
static MaybeError Validate(T supported, T required) {
- if (IsBetter(required, supported)) {
- return DAWN_VALIDATION_ERROR("requiredLimit lower than supported limit");
- }
+ DAWN_INVALID_IF(IsBetter(required, supported),
+ "Required limit (%u) is lower than the supported limit (%u).",
+ required, supported);
return {};
}
};
@@ -114,9 +114,9 @@
template <typename T>
static MaybeError Validate(T supported, T required) {
- if (IsBetter(required, supported)) {
- return DAWN_VALIDATION_ERROR("requiredLimit greater than supported limit");
- }
+ DAWN_INVALID_IF(IsBetter(required, supported),
+ "Required limit (%u) is greater than the supported limit (%u).",
+ required, supported);
return {};
}
};
@@ -163,10 +163,11 @@
}
MaybeError ValidateLimits(const Limits& supportedLimits, const Limits& requiredLimits) {
-#define X(Better, limitName, ...) \
- if (!IsLimitUndefined(requiredLimits.limitName)) { \
- DAWN_TRY(CheckLimit<LimitBetterDirection::Better>::Validate(supportedLimits.limitName, \
- requiredLimits.limitName)); \
+#define X(Better, limitName, ...) \
+ if (!IsLimitUndefined(requiredLimits.limitName)) { \
+ DAWN_TRY_CONTEXT(CheckLimit<LimitBetterDirection::Better>::Validate( \
+ supportedLimits.limitName, requiredLimits.limitName), \
+ "validating " #limitName); \
}
LIMITS(X)
#undef X
diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp
index e6af98b..644ce17 100644
--- a/src/dawn_native/Queue.cpp
+++ b/src/dawn_native/Queue.cpp
@@ -434,9 +434,7 @@
*status = WGPUQueueWorkDoneStatus_Error;
DAWN_TRY(GetDevice()->ValidateObject(this));
- if (signalValue != 0) {
- return DAWN_VALIDATION_ERROR("SignalValue must currently be 0.");
- }
+ DAWN_INVALID_IF(signalValue != 0, "SignalValue (%u) is not 0.", signalValue);
return {};
}
@@ -451,17 +449,17 @@
DAWN_TRY(ValidateImageCopyTexture(GetDevice(), *destination, *writeSize));
- if (dataLayout.offset > dataSize) {
- return DAWN_VALIDATION_ERROR("Queue::WriteTexture out of range");
- }
+ DAWN_INVALID_IF(dataLayout.offset > dataSize,
+ "Data offset (%u) is greater than the data size (%u).", dataLayout.offset,
+ dataSize);
- if (!(destination->texture->GetUsage() & wgpu::TextureUsage::CopyDst)) {
- return DAWN_VALIDATION_ERROR("Texture needs the CopyDst usage bit");
- }
+ DAWN_INVALID_IF(!(destination->texture->GetUsage() & wgpu::TextureUsage::CopyDst),
+ "Usage (%s) of %s does not include %s.", destination->texture->GetUsage(),
+ destination->texture, wgpu::TextureUsage::CopyDst);
- if (destination->texture->GetSampleCount() > 1) {
- return DAWN_VALIDATION_ERROR("The sample count of textures must be 1");
- }
+ DAWN_INVALID_IF(destination->texture->GetSampleCount() > 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,
