Identify unlabeled objects in error messages
Also will report a texture's format if unlabeled to try and
provide more context.
Additionally changes the logic for unlabeled
texture views slightly to show the parent texture info only if the
view has no label. (Previously only showed the parent texture
info if the texture was labeled.)
Omits "unlabeled" tag from queue objects, since there's only one
and it feels a bit silly to demand a label for it.
Bug: dawn:2271, dawn:2436
Change-Id: I99d29bdd6c9b57e671292032f3eab49106988b2c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/177680
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Brandon Jones <bajones@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
diff --git a/src/dawn/native/ObjectBase.cpp b/src/dawn/native/ObjectBase.cpp
index fb7ccd3..8d89660 100644
--- a/src/dawn/native/ObjectBase.cpp
+++ b/src/dawn/native/ObjectBase.cpp
@@ -124,6 +124,8 @@
s->Append(ObjectTypeAsString(GetType()));
if (!mLabel.empty()) {
s->Append(absl::StrFormat(" \"%s\"", mLabel));
+ } else {
+ s->Append(" (unlabeled)");
}
}
diff --git a/src/dawn/native/Queue.cpp b/src/dawn/native/Queue.cpp
index 2e392f7..98c046d 100644
--- a/src/dawn/native/Queue.cpp
+++ b/src/dawn/native/Queue.cpp
@@ -29,6 +29,7 @@
#include <algorithm>
#include <cstring>
+#include <string>
#include <utility>
#include <vector>
@@ -270,6 +271,16 @@
return ObjectType::Queue;
}
+// It doesn't make much sense right now to mark the default queue as "unlabeled", so this override
+// prevents that. Consider removing when multiqueue is implemented.
+void QueueBase::FormatLabel(absl::FormatSink* s) const {
+ s->Append(ObjectTypeAsString(GetType()));
+ const std::string& label = GetLabel();
+ if (!label.empty()) {
+ s->Append(absl::StrFormat(" \"%s\"", label));
+ }
+}
+
void QueueBase::APISubmit(uint32_t commandCount, CommandBufferBase* const* commands) {
MaybeError result = SubmitInternal(commandCount, commands);
diff --git a/src/dawn/native/Queue.h b/src/dawn/native/Queue.h
index 4bd344c..a8d40b6 100644
--- a/src/dawn/native/Queue.h
+++ b/src/dawn/native/Queue.h
@@ -69,6 +69,7 @@
static Ref<QueueBase> MakeError(DeviceBase* device, const char* label);
ObjectType GetType() const override;
+ void FormatLabel(absl::FormatSink* s) const override;
// Dawn API
void APISubmit(uint32_t commandCount, CommandBufferBase* const* commands);
diff --git a/src/dawn/native/Texture.cpp b/src/dawn/native/Texture.cpp
index 017526c2..2950c78 100644
--- a/src/dawn/native/Texture.cpp
+++ b/src/dawn/native/Texture.cpp
@@ -28,7 +28,6 @@
#include "dawn/native/Texture.h"
#include <algorithm>
-#include <string>
#include <utility>
#include "dawn/common/Assert.h"
@@ -832,6 +831,32 @@
return ObjectType::Texture;
}
+void TextureBase::FormatLabel(absl::FormatSink* s) const {
+ s->Append(ObjectTypeAsString(GetType()));
+
+ const std::string& label = GetLabel();
+ if (!label.empty()) {
+ s->Append(absl::StrFormat(" \"%s\"", label));
+ } else if (!IsError()) {
+ s->Append(absl::StrFormat(" (unlabeled %s, %s)", GetSizeLabel(), mFormat->format));
+ }
+}
+
+std::string TextureBase::GetSizeLabel() const {
+ if (mDimension == wgpu::TextureDimension::e1D) {
+ return absl::StrFormat("%d px", mBaseSize.width);
+ } else if (mDimension == wgpu::TextureDimension::e3D) {
+ return absl::StrFormat("%dx%dx%d px", mBaseSize.width, mBaseSize.height,
+ mBaseSize.depthOrArrayLayers);
+ }
+
+ if (mBaseSize.depthOrArrayLayers > 1) {
+ return absl::StrFormat("%dx%d px, %d layer", mBaseSize.width, mBaseSize.height,
+ mBaseSize.depthOrArrayLayers);
+ }
+ return absl::StrFormat("%dx%d px", mBaseSize.width, mBaseSize.height);
+}
+
wgpu::TextureDimension TextureBase::GetDimension() const {
DAWN_ASSERT(!IsError());
return mDimension;
@@ -1225,8 +1250,7 @@
return;
}
- const std::string& textureLabel = mTexture->GetLabel();
- if (!textureLabel.empty()) {
+ if (label.empty()) {
s->Append(" of ");
GetTexture()->FormatLabel(s);
}
diff --git a/src/dawn/native/Texture.h b/src/dawn/native/Texture.h
index e4881af..4ca39f6 100644
--- a/src/dawn/native/Texture.h
+++ b/src/dawn/native/Texture.h
@@ -28,6 +28,7 @@
#ifndef SRC_DAWN_NATIVE_TEXTURE_H_
#define SRC_DAWN_NATIVE_TEXTURE_H_
+#include <string>
#include <vector>
#include "dawn/common/WeakRef.h"
@@ -84,6 +85,7 @@
static Ref<TextureBase> MakeError(DeviceBase* device, const TextureDescriptor* descriptor);
ObjectType GetType() const override;
+ void FormatLabel(absl::FormatSink* s) const override;
wgpu::TextureDimension GetDimension() const;
wgpu::TextureViewDimension GetCompatibilityTextureBindingViewDimension() const;
@@ -184,6 +186,8 @@
TextureBase(DeviceBase* device, const TextureDescriptor* descriptor, ObjectBase::ErrorTag tag);
+ std::string GetSizeLabel() const;
+
wgpu::TextureDimension mDimension;
wgpu::TextureViewDimension
mCompatibilityTextureBindingViewDimension; // only used for compatibility mode
diff --git a/src/dawn/tests/unittests/validation/CommandBufferValidationTests.cpp b/src/dawn/tests/unittests/validation/CommandBufferValidationTests.cpp
index 4dd342b..72324a5 100644
--- a/src/dawn/tests/unittests/validation/CommandBufferValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/CommandBufferValidationTests.cpp
@@ -61,9 +61,9 @@
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&placeholderRenderPass);
- ASSERT_DEVICE_ERROR(
- encoder.Finish(),
- HasSubstr("Command buffer recording ended before [RenderPassEncoder] was ended."));
+ ASSERT_DEVICE_ERROR(encoder.Finish(),
+ HasSubstr("Command buffer recording ended before [RenderPassEncoder "
+ "(unlabeled)] was ended."));
}
// Error case, command buffer ended mid-pass. Trying to use encoders after Finish
@@ -71,11 +71,12 @@
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&placeholderRenderPass);
+ ASSERT_DEVICE_ERROR(encoder.Finish(),
+ HasSubstr("Command buffer recording ended before [RenderPassEncoder "
+ "(unlabeled)] was ended."));
ASSERT_DEVICE_ERROR(
- encoder.Finish(),
- HasSubstr("Command buffer recording ended before [RenderPassEncoder] was ended."));
- ASSERT_DEVICE_ERROR(
- pass.End(), HasSubstr("Parent encoder of [RenderPassEncoder] is already finished."));
+ pass.End(),
+ HasSubstr("Parent encoder of [RenderPassEncoder (unlabeled)] is already finished."));
}
}
@@ -93,9 +94,9 @@
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
- ASSERT_DEVICE_ERROR(
- encoder.Finish(),
- HasSubstr("Command buffer recording ended before [ComputePassEncoder] was ended."));
+ ASSERT_DEVICE_ERROR(encoder.Finish(),
+ HasSubstr("Command buffer recording ended before [ComputePassEncoder "
+ "(unlabeled)] was ended."));
}
// Error case, command buffer ended mid-pass. Trying to use encoders after Finish
@@ -103,11 +104,12 @@
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
+ ASSERT_DEVICE_ERROR(encoder.Finish(),
+ HasSubstr("Command buffer recording ended before [ComputePassEncoder "
+ "(unlabeled)] was ended."));
ASSERT_DEVICE_ERROR(
- encoder.Finish(),
- HasSubstr("Command buffer recording ended before [ComputePassEncoder] was ended."));
- ASSERT_DEVICE_ERROR(
- pass.End(), HasSubstr("Parent encoder of [ComputePassEncoder] is already finished."));
+ pass.End(),
+ HasSubstr("Parent encoder of [ComputePassEncoder (unlabeled)] is already finished."));
}
}
@@ -128,7 +130,8 @@
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&placeholderRenderPass);
pass.End();
- ASSERT_DEVICE_ERROR(pass.End(), HasSubstr("[RenderPassEncoder] was already ended."));
+ ASSERT_DEVICE_ERROR(pass.End(),
+ HasSubstr("[RenderPassEncoder (unlabeled)] was already ended."));
encoder.Finish();
}
}
@@ -148,7 +151,8 @@
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
pass.End();
- ASSERT_DEVICE_ERROR(pass.End(), HasSubstr("[ComputePassEncoder] was already ended."));
+ ASSERT_DEVICE_ERROR(pass.End(),
+ HasSubstr("[ComputePassEncoder (unlabeled)] was already ended."));
encoder.Finish();
}
}
@@ -163,7 +167,8 @@
wgpu::RenderPassEncoder pass0 = encoder.BeginRenderPass(&placeholderRenderPass);
pass0.End();
wgpu::RenderPassEncoder pass1 = encoder.BeginRenderPass(&placeholderRenderPass);
- ASSERT_DEVICE_ERROR(pass0.End(), HasSubstr("[RenderPassEncoder] was already ended."));
+ ASSERT_DEVICE_ERROR(pass0.End(),
+ HasSubstr("[RenderPassEncoder (unlabeled)] was already ended."));
pass1.End();
encoder.Finish();
}
@@ -174,7 +179,8 @@
wgpu::ComputePassEncoder pass0 = encoder.BeginComputePass();
pass0.End();
wgpu::ComputePassEncoder pass1 = encoder.BeginComputePass();
- ASSERT_DEVICE_ERROR(pass0.End(), HasSubstr("[ComputePassEncoder] was already ended."));
+ ASSERT_DEVICE_ERROR(pass0.End(),
+ HasSubstr("[ComputePassEncoder (unlabeled)] was already ended."));
pass1.End();
encoder.Finish();
}
@@ -242,7 +248,7 @@
encoder.Finish();
ASSERT_DEVICE_ERROR(encoder.CopyBufferToBuffer(copyBuffer, 0, copyBuffer, 0, 0),
- HasSubstr("[CommandEncoder] is already finished."));
+ HasSubstr("[CommandEncoder (unlabeled)] is already finished."));
}
// Test that encoding command after a failed finish produces an error
@@ -264,7 +270,7 @@
ASSERT_DEVICE_ERROR(encoder.Finish());
ASSERT_DEVICE_ERROR(encoder.CopyBufferToBuffer(copyBuffer, 0, copyBuffer, 0, 0),
- HasSubstr("[CommandEncoder] is already finished."));
+ HasSubstr("[CommandEncoder (unlabeled)] is already finished."));
}
// Test that passes which are de-referenced prior to ending still allow the correct errors to be
@@ -284,18 +290,18 @@
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
encoder.BeginRenderPass(&placeholderRenderPass);
- ASSERT_DEVICE_ERROR(
- encoder.Finish(),
- HasSubstr("Command buffer recording ended before [RenderPassEncoder] was ended."));
+ ASSERT_DEVICE_ERROR(encoder.Finish(),
+ HasSubstr("Command buffer recording ended before [RenderPassEncoder "
+ "(unlabeled)] was ended."));
}
// Error case, no reference is kept to a compute pass.
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
encoder.BeginComputePass();
- ASSERT_DEVICE_ERROR(
- encoder.Finish(),
- HasSubstr("Command buffer recording ended before [ComputePassEncoder] was ended."));
+ ASSERT_DEVICE_ERROR(encoder.Finish(),
+ HasSubstr("Command buffer recording ended before [ComputePassEncoder "
+ "(unlabeled)] was ended."));
}
// Error case, beginning a new pass after failing to end a de-referenced pass.
@@ -304,9 +310,9 @@
encoder.BeginRenderPass(&placeholderRenderPass);
wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
pass.End();
- ASSERT_DEVICE_ERROR(
- encoder.Finish(),
- HasSubstr("Command buffer recording ended before [RenderPassEncoder] was ended."));
+ ASSERT_DEVICE_ERROR(encoder.Finish(),
+ HasSubstr("Command buffer recording ended before [RenderPassEncoder "
+ "(unlabeled)] was ended."));
}
// Error case, deleting the pass after finishing the command encoder shouldn't generate an
@@ -314,9 +320,9 @@
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
- ASSERT_DEVICE_ERROR(
- encoder.Finish(),
- HasSubstr("Command buffer recording ended before [ComputePassEncoder] was ended."));
+ ASSERT_DEVICE_ERROR(encoder.Finish(),
+ HasSubstr("Command buffer recording ended before [ComputePassEncoder "
+ "(unlabeled)] was ended."));
pass = nullptr;
}
@@ -436,7 +442,8 @@
// encoding.
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&placeholderRenderPass);
pass.End();
- ASSERT_DEVICE_ERROR(encoder.Finish(), HasSubstr("[Invalid CommandEncoder] is invalid."));
+ ASSERT_DEVICE_ERROR(encoder.Finish(),
+ HasSubstr("[Invalid CommandEncoder (unlabeled)] is invalid."));
}
}
diff --git a/src/dawn/tests/unittests/validation/InternalUsageValidationTests.cpp b/src/dawn/tests/unittests/validation/InternalUsageValidationTests.cpp
index 980bff8..c1e3764 100644
--- a/src/dawn/tests/unittests/validation/InternalUsageValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/InternalUsageValidationTests.cpp
@@ -76,7 +76,7 @@
// Check that the encoder records that it is invalid, and not any other errors.
encoder.InjectValidationError("injected error");
ASSERT_DEVICE_ERROR(encoder.Finish(),
- testing::HasSubstr("[Invalid CommandEncoder] is invalid"));
+ testing::HasSubstr("[Invalid CommandEncoder (unlabeled)] is invalid"));
}
class TextureInternalUsageValidationTest : public ValidationTest {
