Format texture sample types and aspects in errors

Should help make a couple of previously ambiguous error messages clearer
for developers.

Bug: dawn:1259
Change-Id: I09a4598153d3340407c5a318dcf6be6d03efd2bd
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/84780
Commit-Queue: Brandon Jones <bajones@chromium.org>
Auto-Submit: Brandon Jones <bajones@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>

src/dawn/native/BindGroup.cpp

@@ -131,8 +131,8 @@ namespace dawn::native {
             TextureViewBase* view = entry.textureView;
 
             Aspect aspect = view->GetAspects();
-            // TODO(dawn:563): Format Aspects
+            DAWN_INVALID_IF(!HasOneBit(aspect), "Multiple aspects (%s) selected in %s.", aspect,
-            DAWN_INVALID_IF(!HasOneBit(aspect), "Multiple aspects selected in %s.", view);
+                            view);
 
             TextureBase* texture = view->GetTexture();
             switch (bindingInfo.bindingType) {
@@ -152,9 +152,11 @@ namespace dawn::native {
                         "Sample count (%u) of %s doesn't match expectation (multisampled: %d).",
                         texture->GetSampleCount(), texture, bindingInfo.texture.multisampled);
 
-                    // TODO(dawn:563): Improve error message.
+                    DAWN_INVALID_IF(
-                    DAWN_INVALID_IF((supportedTypes & requiredType) == 0,
+                        (supportedTypes & requiredType) == 0,
-                                    "Texture component type usage mismatch.");
+                        "None of the supported sample types (%s) of %s match the expected sample "
+                        "types (%s).",
+                        supportedTypes, texture, requiredType);
 
                     DAWN_INVALID_IF(
                         entry.textureView->GetDimension() != bindingInfo.texture.viewDimension,

src/dawn/native/BindingInfo.cpp

@@ -18,62 +18,6 @@
 
 namespace dawn::native {
 
-    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
-        BindingInfoType value,
-        const absl::FormatConversionSpec& spec,
-        absl::FormatSink* s) {
-        switch (value) {
-            case BindingInfoType::Buffer:
-                s->Append("buffer");
-                break;
-            case BindingInfoType::Sampler:
-                s->Append("sampler");
-                break;
-            case BindingInfoType::Texture:
-                s->Append("texture");
-                break;
-            case BindingInfoType::StorageTexture:
-                s->Append("storageTexture");
-                break;
-            case BindingInfoType::ExternalTexture:
-                s->Append("externalTexture");
-                break;
-            default:
-                UNREACHABLE();
-        }
-        return {true};
-    }
-
-    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
-        const BindingInfo& value,
-        const absl::FormatConversionSpec& spec,
-        absl::FormatSink* s) {
-        static const auto* const fmt =
-            new absl::ParsedFormat<'u', 's', 's', 's'>("{ binding: %u, visibility: %s, %s: %s }");
-        switch (value.bindingType) {
-            case BindingInfoType::Buffer:
-                s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding),
-                                          value.visibility, value.bindingType, value.buffer));
-                break;
-            case BindingInfoType::Sampler:
-                s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding),
-                                          value.visibility, value.bindingType, value.sampler));
-                break;
-            case BindingInfoType::Texture:
-                s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding),
-                                          value.visibility, value.bindingType, value.texture));
-                break;
-            case BindingInfoType::StorageTexture:
-                s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding),
-                                          value.visibility, value.bindingType,
-                                          value.storageTexture));
-                break;
-            case BindingInfoType::ExternalTexture:
-                break;
-        }
-        return {true};
-    }
-
     void IncrementBindingCounts(BindingCounts* bindingCounts, const BindGroupLayoutEntry& entry) {
         bindingCounts->totalCount += 1;
 

src/dawn/native/BindingInfo.h

@@ -50,11 +50,6 @@ namespace dawn::native {
 
     enum class BindingInfoType { Buffer, Sampler, Texture, StorageTexture, ExternalTexture };
 
-    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
-        BindingInfoType value,
-        const absl::FormatConversionSpec& spec,
-        absl::FormatSink* s);
-
     struct BindingInfo {
         BindingNumber binding;
         wgpu::ShaderStage visibility;
@@ -68,11 +63,6 @@ namespace dawn::native {
         StorageTextureBindingLayout storageTexture;
     };
 
-    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
-        const BindingInfo& value,
-        const absl::FormatConversionSpec& spec,
-        absl::FormatSink* s);
-
     struct BindingSlot {
         BindGroupIndex group;
         BindingNumber binding;

src/dawn/native/PerStage.cpp

@@ -16,26 +16,6 @@
 
 namespace dawn::native {
 
-    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
-        SingleShaderStage value,
-        const absl::FormatConversionSpec& spec,
-        absl::FormatSink* s) {
-        switch (value) {
-            case SingleShaderStage::Compute:
-                s->Append("Compute");
-                break;
-            case SingleShaderStage::Vertex:
-                s->Append("Vertex");
-                break;
-            case SingleShaderStage::Fragment:
-                s->Append("Fragment");
-                break;
-            default:
-                UNREACHABLE();
-        }
-        return {true};
-    }
-
     BitSetIterator<kNumStages, SingleShaderStage> IterateStages(wgpu::ShaderStage stages) {
         std::bitset<kNumStages> bits(static_cast<uint32_t>(stages));
         return BitSetIterator<kNumStages, SingleShaderStage>(bits);

src/dawn/native/PerStage.h

@@ -28,11 +28,6 @@ namespace dawn::native {
 
     enum class SingleShaderStage { Vertex, Fragment, Compute };
 
-    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
-        SingleShaderStage value,
-        const absl::FormatConversionSpec& spec,
-        absl::FormatSink* s);
-
     static_assert(static_cast<uint32_t>(SingleShaderStage::Vertex) < kNumStages);
     static_assert(static_cast<uint32_t>(SingleShaderStage::Fragment) < kNumStages);
     static_assert(static_cast<uint32_t>(SingleShaderStage::Compute) < kNumStages);

src/dawn/native/RenderPipeline.cpp

@@ -28,88 +28,6 @@
 #include <sstream>
 
 namespace dawn::native {
-    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
-        VertexFormatBaseType value,
-        const absl::FormatConversionSpec& spec,
-        absl::FormatSink* s) {
-        switch (value) {
-            case VertexFormatBaseType::Float:
-                s->Append("Float");
-                break;
-            case VertexFormatBaseType::Uint:
-                s->Append("Uint");
-                break;
-            case VertexFormatBaseType::Sint:
-                s->Append("Sint");
-                break;
-            default:
-                UNREACHABLE();
-        }
-        return {true};
-    }
-
-    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
-        InterStageComponentType value,
-        const absl::FormatConversionSpec& spec,
-        absl::FormatSink* s) {
-        switch (value) {
-            case InterStageComponentType::Float:
-                s->Append("Float");
-                break;
-            case InterStageComponentType::Uint:
-                s->Append("Uint");
-                break;
-            case InterStageComponentType::Sint:
-                s->Append("Sint");
-                break;
-            default:
-                UNREACHABLE();
-        }
-        return {true};
-    }
-
-    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
-        InterpolationType value,
-        const absl::FormatConversionSpec& spec,
-        absl::FormatSink* s) {
-        switch (value) {
-            case InterpolationType::Perspective:
-                s->Append("Perspective");
-                break;
-            case InterpolationType::Linear:
-                s->Append("Linear");
-                break;
-            case InterpolationType::Flat:
-                s->Append("Flat");
-                break;
-            default:
-                UNREACHABLE();
-        }
-        return {true};
-    }
-
-    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
-        InterpolationSampling value,
-        const absl::FormatConversionSpec& spec,
-        absl::FormatSink* s) {
-        switch (value) {
-            case InterpolationSampling::None:
-                s->Append("None");
-                break;
-            case InterpolationSampling::Center:
-                s->Append("Center");
-                break;
-            case InterpolationSampling::Centroid:
-                s->Append("Centroid");
-                break;
-            case InterpolationSampling::Sample:
-                s->Append("Sample");
-                break;
-            default:
-                UNREACHABLE();
-        }
-        return {true};
-    }
 
     // Helper functions
     namespace {

src/dawn/native/Surface.h

@@ -113,6 +113,7 @@ namespace dawn::native {
         uint32_t mXWindow = 0;
     };
 
+    // Not defined in webgpu_absl_format.h/cpp because you can't forward-declare a nested type.
     absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
         Surface::Type value,
         const absl::FormatConversionSpec& spec,

src/dawn/native/webgpu_absl_format.cpp

@@ -14,9 +14,16 @@
 
 #include "dawn/native/webgpu_absl_format.h"
 
+#include "dawn/native/BindingInfo.h"
 #include "dawn/native/Device.h"
+#include "dawn/native/Format.h"
 #include "dawn/native/ObjectBase.h"
+#include "dawn/native/PerStage.h"
+#include "dawn/native/ShaderModule.h"
+#include "dawn/native/Subresource.h"
+#include "dawn/native/Surface.h"
 #include "dawn/native/Texture.h"
+#include "dawn/native/VertexFormat.h"
 
 namespace dawn::native {
 
@@ -62,6 +69,36 @@ namespace dawn::native {
         return {true};
     }
 
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        const BindingInfo& value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s) {
+        static const auto* const fmt =
+            new absl::ParsedFormat<'u', 's', 's', 's'>("{ binding: %u, visibility: %s, %s: %s }");
+        switch (value.bindingType) {
+            case BindingInfoType::Buffer:
+                s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding),
+                                          value.visibility, value.bindingType, value.buffer));
+                break;
+            case BindingInfoType::Sampler:
+                s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding),
+                                          value.visibility, value.bindingType, value.sampler));
+                break;
+            case BindingInfoType::Texture:
+                s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding),
+                                          value.visibility, value.bindingType, value.texture));
+                break;
+            case BindingInfoType::StorageTexture:
+                s->Append(absl::StrFormat(*fmt, static_cast<uint32_t>(value.binding),
+                                          value.visibility, value.bindingType,
+                                          value.storageTexture));
+                break;
+            case BindingInfoType::ExternalTexture:
+                break;
+        }
+        return {true};
+    }
+
     //
     // Objects
     //
@@ -129,4 +166,233 @@ namespace dawn::native {
         return {true};
     }
 
+    //
+    // Enums
+    //
+
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString>
+    AbslFormatConvert(Aspect value, const absl::FormatConversionSpec& spec, absl::FormatSink* s) {
+        if (value == Aspect::None) {
+            s->Append("None");
+            return {true};
+        }
+
+        bool first = true;
+
+        if (value & Aspect::Color) {
+            first = false;
+            s->Append("Color");
+            value &= ~Aspect::Color;
+        }
+
+        if (value & Aspect::Depth) {
+            if (!first) {
+                s->Append("|");
+            }
+            first = false;
+            s->Append("Depth");
+            value &= ~Aspect::Depth;
+        }
+
+        if (value & Aspect::Stencil) {
+            if (!first) {
+                s->Append("|");
+            }
+            first = false;
+            s->Append("Stencil");
+            value &= ~Aspect::Stencil;
+        }
+
+        // Output any remaining flags as a hex value
+        if (static_cast<bool>(value)) {
+            if (!first) {
+                s->Append("|");
+            }
+            s->Append(absl::StrFormat("%x", static_cast<uint8_t>(value)));
+        }
+
+        return {true};
+    }
+
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        SampleTypeBit value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s) {
+        if (value == SampleTypeBit::None) {
+            s->Append("None");
+            return {true};
+        }
+
+        bool first = true;
+
+        if (value & SampleTypeBit::Float) {
+            first = false;
+            s->Append("Float");
+            value &= ~SampleTypeBit::Float;
+        }
+
+        if (value & SampleTypeBit::UnfilterableFloat) {
+            if (!first) {
+                s->Append("|");
+            }
+            first = false;
+            s->Append("UnfilterableFloat");
+            value &= ~SampleTypeBit::UnfilterableFloat;
+        }
+
+        if (value & SampleTypeBit::Depth) {
+            if (!first) {
+                s->Append("|");
+            }
+            first = false;
+            s->Append("Depth");
+            value &= ~SampleTypeBit::Depth;
+        }
+
+        if (value & SampleTypeBit::Sint) {
+            if (!first) {
+                s->Append("|");
+            }
+            first = false;
+            s->Append("Sint");
+            value &= ~SampleTypeBit::Sint;
+        }
+
+        if (value & SampleTypeBit::Uint) {
+            if (!first) {
+                s->Append("|");
+            }
+            first = false;
+            s->Append("Uint");
+            value &= ~SampleTypeBit::Uint;
+        }
+
+        // Output any remaining flags as a hex value
+        if (static_cast<bool>(value)) {
+            if (!first) {
+                s->Append("|");
+            }
+            s->Append(absl::StrFormat("%x", static_cast<uint8_t>(value)));
+        }
+
+        return {true};
+    }
+
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        BindingInfoType value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s) {
+        switch (value) {
+            case BindingInfoType::Buffer:
+                s->Append("buffer");
+                break;
+            case BindingInfoType::Sampler:
+                s->Append("sampler");
+                break;
+            case BindingInfoType::Texture:
+                s->Append("texture");
+                break;
+            case BindingInfoType::StorageTexture:
+                s->Append("storageTexture");
+                break;
+            case BindingInfoType::ExternalTexture:
+                s->Append("externalTexture");
+                break;
+        }
+        return {true};
+    }
+
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        SingleShaderStage value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s) {
+        switch (value) {
+            case SingleShaderStage::Compute:
+                s->Append("Compute");
+                break;
+            case SingleShaderStage::Vertex:
+                s->Append("Vertex");
+                break;
+            case SingleShaderStage::Fragment:
+                s->Append("Fragment");
+                break;
+        }
+        return {true};
+    }
+
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        VertexFormatBaseType value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s) {
+        switch (value) {
+            case VertexFormatBaseType::Float:
+                s->Append("Float");
+                break;
+            case VertexFormatBaseType::Uint:
+                s->Append("Uint");
+                break;
+            case VertexFormatBaseType::Sint:
+                s->Append("Sint");
+                break;
+        }
+        return {true};
+    }
+
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        InterStageComponentType value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s) {
+        switch (value) {
+            case InterStageComponentType::Float:
+                s->Append("Float");
+                break;
+            case InterStageComponentType::Uint:
+                s->Append("Uint");
+                break;
+            case InterStageComponentType::Sint:
+                s->Append("Sint");
+                break;
+        }
+        return {true};
+    }
+
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        InterpolationType value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s) {
+        switch (value) {
+            case InterpolationType::Perspective:
+                s->Append("Perspective");
+                break;
+            case InterpolationType::Linear:
+                s->Append("Linear");
+                break;
+            case InterpolationType::Flat:
+                s->Append("Flat");
+                break;
+        }
+        return {true};
+    }
+
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        InterpolationSampling value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s) {
+        switch (value) {
+            case InterpolationSampling::None:
+                s->Append("None");
+                break;
+            case InterpolationSampling::Center:
+                s->Append("Center");
+                break;
+            case InterpolationSampling::Centroid:
+                s->Append("Centroid");
+                break;
+            case InterpolationSampling::Sample:
+                s->Append("Sample");
+                break;
+        }
+        return {true};
+    }
+
 }  // namespace dawn::native

src/dawn/native/webgpu_absl_format.h

@@ -43,6 +43,12 @@ namespace dawn::native {
         const absl::FormatConversionSpec& spec,
         absl::FormatSink* s);
 
+    struct BindingInfo;
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        const BindingInfo& value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s);
+
     //
     // Objects
     //
@@ -67,6 +73,56 @@ namespace dawn::native {
         const absl::FormatConversionSpec& spec,
         absl::FormatSink* s);
 
+    //
+    // Enums
+    //
+
+    enum class Aspect : uint8_t;
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString>
+    AbslFormatConvert(Aspect value, const absl::FormatConversionSpec& spec, absl::FormatSink* s);
+
+    enum class BindingInfoType;
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        BindingInfoType value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s);
+
+    enum class SampleTypeBit : uint8_t;
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        SampleTypeBit value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s);
+
+    enum class SingleShaderStage;
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        SingleShaderStage value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s);
+
+    enum class VertexFormatBaseType;
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        VertexFormatBaseType value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s);
+
+    enum class InterStageComponentType;
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        InterStageComponentType value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s);
+
+    enum class InterpolationType;
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        InterpolationType value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s);
+
+    enum class InterpolationSampling;
+    absl::FormatConvertResult<absl::FormatConversionCharSet::kString> AbslFormatConvert(
+        InterpolationSampling value,
+        const absl::FormatConversionSpec& spec,
+        absl::FormatSink* s);
+
 }  // namespace dawn::native
 
 #endif  // DAWNNATIVE_WEBGPUABSLFORMAT_H_