Improve validation errors for encoders
Improves the validation messages in ComputePassEncoder.cpp,
ProgrammablePassEncoder.cpp, RenderBundleEncoder.cpp, and
EncodingContext.cpp/h to give them more contextual information.
Bug: dawn:563
Change-Id: I87c46c4bfda1375809fae93239029ea4e3b9c0a2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/67000
Commit-Queue: Brandon Jones <bajones@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp
index ab0857f..c9510d0 100644
--- a/src/dawn_native/CommandEncoder.cpp
+++ b/src/dawn_native/CommandEncoder.cpp
@@ -891,8 +891,7 @@
if (GetDevice()->IsValidationEnabled()) {
DAWN_INVALID_IF(
mDebugGroupStackSize == 0,
- "Every call to PopDebugGroup must be balanced by a corresponding call to "
- "PushDebugGroup.");
+ "PopDebugGroup called when no debug groups are currently pushed.");
}
allocator->Allocate<PopDebugGroupCmd>(Command::PopDebugGroup);
mDebugGroupStackSize--;
diff --git a/src/dawn_native/ComputePassEncoder.cpp b/src/dawn_native/ComputePassEncoder.cpp
index 46277b1..1aa4845 100644
--- a/src/dawn_native/ComputePassEncoder.cpp
+++ b/src/dawn_native/ComputePassEncoder.cpp
@@ -26,18 +26,6 @@
namespace dawn_native {
- namespace {
-
- MaybeError ValidatePerDimensionDispatchSizeLimit(const DeviceBase* device, uint32_t size) {
- if (size > device->GetLimits().v1.maxComputeWorkgroupsPerDimension) {
- return DAWN_VALIDATION_ERROR("Dispatch size exceeds defined limits");
- }
-
- return {};
- }
-
- } // namespace
-
ComputePassEncoder::ComputePassEncoder(DeviceBase* device,
CommandEncoder* commandEncoder,
EncodingContext* encodingContext)
@@ -85,9 +73,24 @@
[&](CommandAllocator* allocator) -> MaybeError {
if (IsValidationEnabled()) {
DAWN_TRY(mCommandBufferState.ValidateCanDispatch());
- DAWN_TRY(ValidatePerDimensionDispatchSizeLimit(GetDevice(), x));
- DAWN_TRY(ValidatePerDimensionDispatchSizeLimit(GetDevice(), y));
- DAWN_TRY(ValidatePerDimensionDispatchSizeLimit(GetDevice(), z));
+
+ uint32_t workgroupsPerDimension =
+ GetDevice()->GetLimits().v1.maxComputeWorkgroupsPerDimension;
+
+ DAWN_INVALID_IF(
+ x > workgroupsPerDimension,
+ "Dispatch size X (%u) exceeds max compute workgroups per dimension (%u).",
+ x, workgroupsPerDimension);
+
+ DAWN_INVALID_IF(
+ y > workgroupsPerDimension,
+ "Dispatch size Y (%u) exceeds max compute workgroups per dimension (%u).",
+ y, workgroupsPerDimension);
+
+ DAWN_INVALID_IF(
+ z > workgroupsPerDimension,
+ "Dispatch size Z (%u) exceeds max compute workgroups per dimension (%u).",
+ z, workgroupsPerDimension);
}
// Record the synchronization scope for Dispatch, which is just the current
@@ -117,21 +120,20 @@
// Indexed dispatches need a compute-shader based validation to check that the
// dispatch sizes aren't too big. Disallow them as unsafe until the validation
// is implemented.
- if (GetDevice()->IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) {
- return DAWN_VALIDATION_ERROR(
- "DispatchIndirect is disallowed because it doesn't validate that the "
- "dispatch "
- "size is valid yet.");
- }
+ DAWN_INVALID_IF(
+ GetDevice()->IsToggleEnabled(Toggle::DisallowUnsafeAPIs),
+ "DispatchIndirect is disallowed because it doesn't validate that the "
+ "dispatch size is valid yet.");
- if (indirectOffset % 4 != 0) {
- return DAWN_VALIDATION_ERROR("Indirect offset must be a multiple of 4");
- }
+ DAWN_INVALID_IF(indirectOffset % 4 != 0,
+ "Indirect offset (%u) is not a multiple of 4.", indirectOffset);
- if (indirectOffset >= indirectBuffer->GetSize() ||
- indirectOffset + kDispatchIndirectSize > indirectBuffer->GetSize()) {
- return DAWN_VALIDATION_ERROR("Indirect offset out of bounds");
- }
+ DAWN_INVALID_IF(
+ indirectOffset >= indirectBuffer->GetSize() ||
+ indirectOffset + kDispatchIndirectSize > indirectBuffer->GetSize(),
+ "Indirect offset (%u) and dispatch size (%u) exceeds the indirect buffer "
+ "size (%u).",
+ indirectOffset, kDispatchIndirectSize, indirectBuffer->GetSize());
}
// Record the synchronization scope for Dispatch, both the bindgroups and the
diff --git a/src/dawn_native/EncodingContext.cpp b/src/dawn_native/EncodingContext.cpp
index 2724291..7b14f5e 100644
--- a/src/dawn_native/EncodingContext.cpp
+++ b/src/dawn_native/EncodingContext.cpp
@@ -24,7 +24,7 @@
namespace dawn_native {
- EncodingContext::EncodingContext(DeviceBase* device, const ObjectBase* initialEncoder)
+ EncodingContext::EncodingContext(DeviceBase* device, const ApiObjectBase* initialEncoder)
: mDevice(device), mTopLevelEncoder(initialEncoder), mCurrentEncoder(initialEncoder) {
}
@@ -87,7 +87,7 @@
}
}
- void EncodingContext::EnterPass(const ObjectBase* passEncoder) {
+ void EncodingContext::EnterPass(const ApiObjectBase* passEncoder) {
// Assert we're at the top level.
ASSERT(mCurrentEncoder == mTopLevelEncoder);
ASSERT(passEncoder != nullptr);
@@ -95,7 +95,7 @@
mCurrentEncoder = passEncoder;
}
- MaybeError EncodingContext::ExitRenderPass(const ObjectBase* passEncoder,
+ MaybeError EncodingContext::ExitRenderPass(const ApiObjectBase* passEncoder,
RenderPassResourceUsageTracker usageTracker,
CommandEncoder* commandEncoder,
IndirectDrawMetadata indirectDrawMetadata) {
@@ -121,7 +121,7 @@
return {};
}
- void EncodingContext::ExitComputePass(const ObjectBase* passEncoder,
+ void EncodingContext::ExitComputePass(const ApiObjectBase* passEncoder,
ComputePassResourceUsage usages) {
ASSERT(mCurrentEncoder != mTopLevelEncoder);
ASSERT(mCurrentEncoder == passEncoder);
@@ -161,12 +161,10 @@
}
MaybeError EncodingContext::Finish() {
- if (IsFinished()) {
- return DAWN_VALIDATION_ERROR("Command encoding already finished");
- }
+ DAWN_INVALID_IF(IsFinished(), "Command encoding already finished.");
- const void* currentEncoder = mCurrentEncoder;
- const void* topLevelEncoder = mTopLevelEncoder;
+ const ApiObjectBase* currentEncoder = mCurrentEncoder;
+ const ApiObjectBase* topLevelEncoder = mTopLevelEncoder;
// Even if finish validation fails, it is now invalid to call any encoding commands,
// so we clear the encoders. Note: mTopLevelEncoder == nullptr is used as a flag for
@@ -178,9 +176,8 @@
if (mError != nullptr) {
return std::move(mError);
}
- if (currentEncoder != topLevelEncoder) {
- return DAWN_VALIDATION_ERROR("Command buffer recording ended mid-pass");
- }
+ DAWN_INVALID_IF(currentEncoder != topLevelEncoder,
+ "Command buffer recording ended before %s was ended.", currentEncoder);
return {};
}
diff --git a/src/dawn_native/EncodingContext.h b/src/dawn_native/EncodingContext.h
index b058e2b..c395cf2 100644
--- a/src/dawn_native/EncodingContext.h
+++ b/src/dawn_native/EncodingContext.h
@@ -28,13 +28,13 @@
class CommandEncoder;
class DeviceBase;
- class ObjectBase;
+ class ApiObjectBase;
// Base class for allocating/iterating commands.
// It performs error tracking as well as encoding state for render/compute passes.
class EncodingContext {
public:
- EncodingContext(DeviceBase* device, const ObjectBase* initialEncoder);
+ EncodingContext(DeviceBase* device, const ApiObjectBase* initialEncoder);
~EncodingContext();
CommandIterator AcquireCommands();
@@ -70,14 +70,15 @@
return false;
}
- inline bool CheckCurrentEncoder(const ObjectBase* encoder) {
+ inline bool CheckCurrentEncoder(const ApiObjectBase* encoder) {
if (DAWN_UNLIKELY(encoder != mCurrentEncoder)) {
if (mCurrentEncoder != mTopLevelEncoder) {
// The top level encoder was used when a pass encoder was current.
- HandleError(DAWN_VALIDATION_ERROR("Command cannot be recorded inside a pass"));
+ HandleError(DAWN_FORMAT_VALIDATION_ERROR(
+ "Command cannot be recorded while %s is active.", mCurrentEncoder));
} else {
- HandleError(DAWN_VALIDATION_ERROR(
- "Recording in an error or already ended pass encoder"));
+ HandleError(DAWN_FORMAT_VALIDATION_ERROR(
+ "Recording in an error or already ended %s.", encoder));
}
return false;
}
@@ -85,7 +86,7 @@
}
template <typename EncodeFunction>
- inline bool TryEncode(const ObjectBase* encoder, EncodeFunction&& encodeFunction) {
+ inline bool TryEncode(const ApiObjectBase* encoder, EncodeFunction&& encodeFunction) {
if (!CheckCurrentEncoder(encoder)) {
return false;
}
@@ -94,7 +95,7 @@
}
template <typename EncodeFunction, typename... Args>
- inline bool TryEncode(const ObjectBase* encoder,
+ inline bool TryEncode(const ApiObjectBase* encoder,
EncodeFunction&& encodeFunction,
const char* formatStr,
const Args&... args) {
@@ -111,12 +112,12 @@
void WillBeginRenderPass();
// Functions to set current encoder state
- void EnterPass(const ObjectBase* passEncoder);
- MaybeError ExitRenderPass(const ObjectBase* passEncoder,
+ void EnterPass(const ApiObjectBase* passEncoder);
+ MaybeError ExitRenderPass(const ApiObjectBase* passEncoder,
RenderPassResourceUsageTracker usageTracker,
CommandEncoder* commandEncoder,
IndirectDrawMetadata indirectDrawMetadata);
- void ExitComputePass(const ObjectBase* passEncoder, ComputePassResourceUsage usages);
+ void ExitComputePass(const ApiObjectBase* passEncoder, ComputePassResourceUsage usages);
MaybeError Finish();
const RenderPassUsages& GetRenderPassUsages() const;
@@ -138,12 +139,12 @@
// There can only be two levels of encoders. Top-level and render/compute pass.
// The top level encoder is the encoder the EncodingContext is created with.
// It doubles as flag to check if encoding has been Finished.
- const ObjectBase* mTopLevelEncoder;
+ const ApiObjectBase* mTopLevelEncoder;
// The current encoder must be the same as the encoder provided to TryEncode,
// otherwise an error is produced. It may be nullptr if the EncodingContext is an error.
// The current encoder changes with Enter/ExitPass which should be called by
// CommandEncoder::Begin/EndPass.
- const ObjectBase* mCurrentEncoder;
+ const ApiObjectBase* mCurrentEncoder;
RenderPassUsages mRenderPassUsages;
bool mWereRenderPassUsagesAcquired = false;
diff --git a/src/dawn_native/ProgrammablePassEncoder.cpp b/src/dawn_native/ProgrammablePassEncoder.cpp
index c6e2e28..8cf1b82 100644
--- a/src/dawn_native/ProgrammablePassEncoder.cpp
+++ b/src/dawn_native/ProgrammablePassEncoder.cpp
@@ -48,9 +48,9 @@
}
MaybeError ProgrammablePassEncoder::ValidateProgrammableEncoderEnd() const {
- if (mDebugGroupStackSize != 0) {
- return DAWN_VALIDATION_ERROR("Each Push must be balanced by a corresponding Pop.");
- }
+ DAWN_INVALID_IF(mDebugGroupStackSize != 0,
+ "PushDebugGroup called %u time(s) without a corresponding PopDebugGroup.",
+ mDebugGroupStackSize);
return {};
}
@@ -75,10 +75,9 @@
this,
[&](CommandAllocator* allocator) -> MaybeError {
if (IsValidationEnabled()) {
- if (mDebugGroupStackSize == 0) {
- return DAWN_VALIDATION_ERROR(
- "Pop must be balanced by a corresponding Push.");
- }
+ DAWN_INVALID_IF(
+ mDebugGroupStackSize == 0,
+ "PopDebugGroup called when no debug groups are currently pushed.");
}
allocator->Allocate<PopDebugGroupCmd>(Command::PopDebugGroup);
mDebugGroupStackSize--;
@@ -115,18 +114,21 @@
const uint32_t* dynamicOffsetsIn) const {
DAWN_TRY(GetDevice()->ValidateObject(group));
- if (index >= kMaxBindGroupsTyped) {
- return DAWN_VALIDATION_ERROR("Setting bind group over the max");
- }
+ DAWN_INVALID_IF(index >= kMaxBindGroupsTyped,
+ "Bind group index (%u) exceeds the maximum (%u).",
+ static_cast<uint32_t>(index), kMaxBindGroups);
ityp::span<BindingIndex, const uint32_t> dynamicOffsets(dynamicOffsetsIn,
BindingIndex(dynamicOffsetCountIn));
// Dynamic offsets count must match the number required by the layout perfectly.
const BindGroupLayoutBase* layout = group->GetLayout();
- if (layout->GetDynamicBufferCount() != dynamicOffsets.size()) {
- return DAWN_VALIDATION_ERROR("dynamicOffset count mismatch");
- }
+ DAWN_INVALID_IF(
+ layout->GetDynamicBufferCount() != dynamicOffsets.size(),
+ "The number of dynamic offsets (%u) does not match the number of dynamic buffers (%u) "
+ "in %s.",
+ static_cast<uint32_t>(dynamicOffsets.size()),
+ static_cast<uint32_t>(layout->GetDynamicBufferCount()), layout);
for (BindingIndex i{0}; i < dynamicOffsets.size(); ++i) {
const BindingInfo& bindingInfo = layout->GetBindingInfo(i);
@@ -150,9 +152,9 @@
UNREACHABLE();
}
- if (!IsAligned(dynamicOffsets[i], requiredAlignment)) {
- return DAWN_VALIDATION_ERROR("Dynamic Buffer Offset need to be aligned");
- }
+ DAWN_INVALID_IF(!IsAligned(dynamicOffsets[i], requiredAlignment),
+ "Dynamic Offset[%u] (%u) is not %u byte aligned.",
+ static_cast<uint32_t>(i), dynamicOffsets[i], requiredAlignment);
BufferBinding bufferBinding = group->GetBindingAsBufferBinding(i);
@@ -163,15 +165,20 @@
if ((dynamicOffsets[i] >
bufferBinding.buffer->GetSize() - bufferBinding.offset - bufferBinding.size)) {
- if ((bufferBinding.buffer->GetSize() - bufferBinding.offset) ==
- bufferBinding.size) {
- return DAWN_VALIDATION_ERROR(
- "Dynamic offset out of bounds. The binding goes to the end of the "
- "buffer even with a dynamic offset of 0. Did you forget to specify "
- "the binding's size?");
- } else {
- return DAWN_VALIDATION_ERROR("Dynamic offset out of bounds");
- }
+ DAWN_INVALID_IF(
+ (bufferBinding.buffer->GetSize() - bufferBinding.offset) == bufferBinding.size,
+ "Dynamic Offset[%u] (%u) is out of bounds of %s with a size of %u and a bound "
+ "range of (offset: %u, size: %u). The binding goes to the end of the buffer "
+ "even with a dynamic offset of 0. Did you forget to specify "
+ "the binding's size?",
+ static_cast<uint32_t>(i), dynamicOffsets[i], bufferBinding.buffer,
+ bufferBinding.buffer->GetSize(), bufferBinding.offset, bufferBinding.size);
+
+ return DAWN_FORMAT_VALIDATION_ERROR(
+ "Dynamic Offset[%u] (%u) is out of bounds of "
+ "%s with a size of %u and a bound range of (offset: %u, size: %u).",
+ static_cast<uint32_t>(i), dynamicOffsets[i], bufferBinding.buffer,
+ bufferBinding.buffer->GetSize(), bufferBinding.offset, bufferBinding.size);
}
}
diff --git a/src/dawn_native/RenderBundleEncoder.cpp b/src/dawn_native/RenderBundleEncoder.cpp
index 19f8c64..b21a095 100644
--- a/src/dawn_native/RenderBundleEncoder.cpp
+++ b/src/dawn_native/RenderBundleEncoder.cpp
@@ -123,7 +123,8 @@
RenderBundleBase* RenderBundleEncoder::APIFinish(const RenderBundleDescriptor* descriptor) {
RenderBundleBase* result = nullptr;
- if (GetDevice()->ConsumedError(FinishImpl(descriptor), &result)) {
+ if (GetDevice()->ConsumedError(FinishImpl(descriptor), &result, "calling Finish(%s).",
+ descriptor)) {
return RenderBundleBase::MakeError(GetDevice());
}
