commit 9ed9e760ff1d61943e9ba8fb4c34bc34225c9d40
author Jiawei Shao <jiawei.shao@intel.com> Wed Apr 24 01:07:35 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Apr 24 01:07:35 2024 +0000
tree 41509bdb05c9f231de328a3de339cfd64523bc8c
parent 71926f1b61d3d450f02d4bb38004efea2bafd22a

Show `undefined` when the value is `wgpu::kCopyStrideUndefined`

This patch replaces the integer value of `wgpu::kCopyStrideUndefined`
(0xFFFFFFFF) with "undefined" in the validation error message to
make it more readable.

Bug: dawn:563
Change-Id: I9d4755541b9b9b63e810cde615338eac2446924c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/185040
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn/native/CommandValidation.cpp

diff --git a/src/dawn/native/CommandValidation.cpp b/src/dawn/native/CommandValidation.cpp
index ea5d05b..fddcdb9 100644
--- a/src/dawn/native/CommandValidation.cpp
+++ b/src/dawn/native/CommandValidation.cpp
@@ -47,6 +47,7 @@
 #include "dawn/native/RenderBundle.h"
 #include "dawn/native/RenderPipeline.h"
 #include "dawn/native/ValidationUtils_autogen.h"
+#include "dawn/native/webgpu_absl_format.h"
 
 namespace dawn::native {
 
@@ -276,13 +277,13 @@
     DAWN_ASSERT(copyExtent.height % blockInfo.height == 0);
     uint32_t heightInBlocks = copyExtent.height / blockInfo.height;
 
-    // TODO(dawn:563): Right now kCopyStrideUndefined will be formatted as a large value in the
-    // validation message. Investigate ways to make it print as a more readable symbol.
     DAWN_INVALID_IF(
         copyExtent.depthOrArrayLayers > 1 && (layout.bytesPerRow == wgpu::kCopyStrideUndefined ||
                                               layout.rowsPerImage == wgpu::kCopyStrideUndefined),
         "Copy depth (%u) is > 1, but bytesPerRow (%u) or rowsPerImage (%u) are not specified.",
-        copyExtent.depthOrArrayLayers, layout.bytesPerRow, layout.rowsPerImage);
+        copyExtent.depthOrArrayLayers,
+        WrapUndefined(layout.bytesPerRow, wgpu::kCopyStrideUndefined),
+        WrapUndefined(layout.rowsPerImage, wgpu::kCopyStrideUndefined));
 
     DAWN_INVALID_IF(heightInBlocks > 1 && layout.bytesPerRow == wgpu::kCopyStrideUndefined,
                     "HeightInBlocks (%u) is > 1, but bytesPerRow is not specified.",

src/dawn/native/webgpu_absl_format.h

diff --git a/src/dawn/native/webgpu_absl_format.h b/src/dawn/native/webgpu_absl_format.h
index b23f969..619bdfd 100644
--- a/src/dawn/native/webgpu_absl_format.h
+++ b/src/dawn/native/webgpu_absl_format.h
@@ -28,6 +28,8 @@
 #ifndef SRC_DAWN_NATIVE_WEBGPU_ABSL_FORMAT_H_
 #define SRC_DAWN_NATIVE_WEBGPU_ABSL_FORMAT_H_
 
+#include <optional>
+
 #include "absl/strings/str_format.h"
 #include "dawn/native/dawn_platform.h"
 #include "dawn/native/webgpu_absl_format_autogen.h"
@@ -264,6 +266,29 @@
     return {true};
 }
 
+template <typename T>
+struct UndefinedWrapper {
+    std::optional<T> value;
+};
+
+template <typename T>
+UndefinedWrapper<T> WrapUndefined(T value, T undefinedValue) {
+    return value == undefinedValue ? UndefinedWrapper<T>() : UndefinedWrapper<T>{value};
+}
+
+template <typename T>
+absl::FormatConvertResult<absl::FormatConversionCharSet::kNumeric> AbslFormatConvert(
+    const UndefinedWrapper<T>& value,
+    const absl::FormatConversionSpec& spec,
+    absl::FormatSink* s) {
+    if (!value.value) {
+        s->Append("undefined");
+    } else {
+        s->Append(absl::StrFormat("%u", static_cast<T>(*value.value)));
+    }
+    return {true};
+}
+
 }  // namespace dawn::native
 
 #endif  // SRC_DAWN_NATIVE_WEBGPU_ABSL_FORMAT_H_