Improve validation messages for surface/swap chain

Updates validation messages with more contextual information in:
 - ChainUtils.h
 - Device.cpp
 - SpirvValidation.cpp
 - Surface.cpp
 - SwapChain.cpp

Bug: dawn:563
Change-Id: I486512791caaf1acf4607539aa5ad11daf1ab9be
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/67140
Commit-Queue: Brandon Jones <bajones@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

generator/templates/dawn_native/ChainUtils.h

@@ -36,9 +36,8 @@ namespace dawn_native {
 
     template <typename T>
     MaybeError ValidateSingleSTypeInner(const ChainedStruct* chain, T sType) {
-        if (chain->sType != sType) {
-            return DAWN_VALIDATION_ERROR("Unsupported sType");
-        }
+        DAWN_INVALID_IF(chain->sType != sType,
+            "Unsupported sType (%s). Expected (%s)", chain->sType, sType);
         return {};
     }
 
@@ -57,9 +56,8 @@ namespace dawn_native {
         if (chain == nullptr) {
             return {};
         }
-        if (chain->nextInChain != nullptr) {
-            return DAWN_VALIDATION_ERROR("Chain can only contain a single chained struct");
-        }
+        DAWN_INVALID_IF(chain->nextInChain != nullptr,
+            "Chain can only contain a single chained struct.");
         return ValidateSingleSTypeInner(chain, sType);
     }
 
@@ -70,9 +68,8 @@ namespace dawn_native {
         if (chain == nullptr) {
             return {};
         }
-        if (chain->nextInChain != nullptr) {
-            return DAWN_VALIDATION_ERROR("Chain can only contain a single chained struct");
-        }
+        DAWN_INVALID_IF(chain->nextInChain != nullptr,
+            "Chain can only contain a single chained struct.");
         return ValidateSingleSTypeInner(chain, sType, sTypes...);
     }
 

src/dawn_native/Device.cpp

@@ -1444,7 +1444,8 @@ namespace dawn_native {
         const SwapChainDescriptor* descriptor) {
         DAWN_TRY(ValidateIsAlive());
         if (IsValidationEnabled()) {
-            DAWN_TRY(ValidateSwapChainDescriptor(this, surface, descriptor));
+            DAWN_TRY_CONTEXT(ValidateSwapChainDescriptor(this, surface, descriptor),
+                             "validating %s", descriptor);
         }
 
         // TODO(dawn:269): Remove this code path once implementation-based swapchains are removed.

src/dawn_native/SpirvValidation.cpp

@@ -65,10 +65,8 @@ namespace dawn_native {
             device->EmitLog(WGPULoggingType_Info, dumpedMsg.str().c_str());
         }
 
-        if (!valid) {
-            return DAWN_VALIDATION_ERROR(
-                "Produced invalid SPIRV. Please file a bug at https://crbug.com/tint.");
-        }
+        DAWN_INVALID_IF(!valid,
+                        "Produced invalid SPIRV. Please file a bug at https://crbug.com/tint.");
 
         return {};
     }

src/dawn_native/Surface.cpp

@@ -36,9 +36,9 @@ namespace dawn_native {
 
     MaybeError ValidateSurfaceDescriptor(const InstanceBase* instance,
                                          const SurfaceDescriptor* descriptor) {
-        if (descriptor->nextInChain == nullptr) {
-            return DAWN_VALIDATION_ERROR("Surface cannot be created with just the base descriptor");
-        }
+        DAWN_INVALID_IF(descriptor->nextInChain == nullptr,
+                        "Surface cannot be created with %s. nextInChain is not specified.",
+                        descriptor);
 
         DAWN_TRY(ValidateSingleSType(descriptor->nextInChain,
                                      wgpu::SType::SurfaceDescriptorFromMetalLayer,
@@ -52,9 +52,8 @@ namespace dawn_native {
         FindInChain(descriptor->nextInChain, &metalDesc);
         if (metalDesc) {
             // Check that the layer is a CAMetalLayer (or a derived class).
-            if (!InheritsFromCAMetalLayer(metalDesc->layer)) {
-                return DAWN_VALIDATION_ERROR("layer must be a CAMetalLayer");
-            }
+            DAWN_INVALID_IF(!InheritsFromCAMetalLayer(metalDesc->layer),
+                            "Layer must be a CAMetalLayer");
             return {};
         }
 #endif  // defined(DAWN_ENABLE_BACKEND_METAL)
@@ -64,9 +63,7 @@ namespace dawn_native {
         const SurfaceDescriptorFromWindowsHWND* hwndDesc = nullptr;
         FindInChain(descriptor->nextInChain, &hwndDesc);
         if (hwndDesc) {
-            if (IsWindow(static_cast<HWND>(hwndDesc->hwnd)) == 0) {
-                return DAWN_VALIDATION_ERROR("Invalid HWND");
-            }
+            DAWN_INVALID_IF(IsWindow(static_cast<HWND>(hwndDesc->hwnd)) == 0, "Invalid HWND");
             return {};
         }
 #    endif  // defined(DAWN_PLATFORM_WIN32)
@@ -75,11 +72,10 @@ namespace dawn_native {
         if (coreWindowDesc) {
             // Validate the coreWindow by query for ICoreWindow interface
             ComPtr<ABI::Windows::UI::Core::ICoreWindow> coreWindow;
-            if (coreWindowDesc->coreWindow == nullptr ||
-                FAILED(static_cast<IUnknown*>(coreWindowDesc->coreWindow)
-                           ->QueryInterface(IID_PPV_ARGS(&coreWindow)))) {
-                return DAWN_VALIDATION_ERROR("Invalid CoreWindow");
-            }
+            DAWN_INVALID_IF(coreWindowDesc->coreWindow == nullptr ||
+                                FAILED(static_cast<IUnknown*>(coreWindowDesc->coreWindow)
+                                           ->QueryInterface(IID_PPV_ARGS(&coreWindow))),
+                            "Invalid CoreWindow");
             return {};
         }
         const SurfaceDescriptorFromWindowsSwapChainPanel* swapChainPanelDesc = nullptr;
@@ -87,11 +83,10 @@ namespace dawn_native {
         if (swapChainPanelDesc) {
             // Validate the swapChainPanel by querying for ISwapChainPanel interface
             ComPtr<ABI::Windows::UI::Xaml::Controls::ISwapChainPanel> swapChainPanel;
-            if (swapChainPanelDesc->swapChainPanel == nullptr ||
-                FAILED(static_cast<IUnknown*>(swapChainPanelDesc->swapChainPanel)
-                           ->QueryInterface(IID_PPV_ARGS(&swapChainPanel)))) {
-                return DAWN_VALIDATION_ERROR("Invalid SwapChainPanel");
-            }
+            DAWN_INVALID_IF(swapChainPanelDesc->swapChainPanel == nullptr ||
+                                FAILED(static_cast<IUnknown*>(swapChainPanelDesc->swapChainPanel)
+                                           ->QueryInterface(IID_PPV_ARGS(&swapChainPanel))),
+                            "Invalid SwapChainPanel");
             return {};
         }
 #endif  // defined(DAWN_PLATFORM_WINDOWS)
@@ -111,14 +106,13 @@ namespace dawn_native {
                                               xDesc->window, &attributes);
             XSetErrorHandler(oldErrorHandler);
 
-            if (status == 0) {
-                return DAWN_VALIDATION_ERROR("Invalid X Window");
-            }
+            DAWN_INVALID_IF(status == 0, "Invalid X Window");
             return {};
         }
 #endif  // defined(DAWN_USE_X11)
 
-        return DAWN_VALIDATION_ERROR("Unsupported sType");
+        return DAWN_FORMAT_VALIDATION_ERROR("Unsupported sType (%s)",
+                                            descriptor->nextInChain->sType);
     }
 
     Surface::Surface(InstanceBase* instance, const SurfaceDescriptor* descriptor)

src/dawn_native/SwapChain.cpp

@@ -36,16 +36,19 @@ namespace dawn_native {
                               wgpu::TextureUsage allowedUsage,
                               uint32_t width,
                               uint32_t height) override {
-                GetDevice()->ConsumedError(DAWN_VALIDATION_ERROR("error swapchain"));
+                GetDevice()->ConsumedError(
+                    DAWN_FORMAT_VALIDATION_ERROR("%s is an error swapchain.", this));
             }
 
             TextureViewBase* APIGetCurrentTextureView() override {
-                GetDevice()->ConsumedError(DAWN_VALIDATION_ERROR("error swapchain"));
+                GetDevice()->ConsumedError(
+                    DAWN_FORMAT_VALIDATION_ERROR("%s is an error swapchain.", this));
                 return TextureViewBase::MakeError(GetDevice());
             }
 
             void APIPresent() override {
-                GetDevice()->ConsumedError(DAWN_VALIDATION_ERROR("error swapchain"));
+                GetDevice()->ConsumedError(
+                    DAWN_FORMAT_VALIDATION_ERROR("%s is an error swapchain.", this));
             }
         };
 
@@ -55,45 +58,43 @@ namespace dawn_native {
                                            const Surface* surface,
                                            const SwapChainDescriptor* descriptor) {
         if (descriptor->implementation != 0) {
-            if (surface != nullptr) {
-                return DAWN_VALIDATION_ERROR(
-                    "Exactly one of surface or implementation must be set");
-            }
+            DAWN_INVALID_IF(surface != nullptr,
+                            "Exactly one of surface or implementation must be set");
 
             DawnSwapChainImplementation* impl =
                 reinterpret_cast<DawnSwapChainImplementation*>(descriptor->implementation);
 
-            if (!impl->Init || !impl->Destroy || !impl->Configure || !impl->GetNextTexture ||
-                !impl->Present) {
-                return DAWN_VALIDATION_ERROR("Implementation is incomplete");
-            }
+            DAWN_INVALID_IF(!impl->Init || !impl->Destroy || !impl->Configure ||
+                                !impl->GetNextTexture || !impl->Present,
+                            "Implementation is incomplete");
 
         } else {
-            if (surface == nullptr) {
-                return DAWN_VALIDATION_ERROR(
-                    "At least one of surface or implementation must be set");
-            }
+            DAWN_INVALID_IF(surface == nullptr,
+                            "At least one of surface or implementation must be set");
 
             DAWN_TRY(ValidatePresentMode(descriptor->presentMode));
 
             // TODO(crbug.com/dawn/160): Lift this restriction once
             // wgpu::Instance::GetPreferredSurfaceFormat is implemented.
-            if (descriptor->format != wgpu::TextureFormat::BGRA8Unorm) {
-                return DAWN_VALIDATION_ERROR("Format must (currently) be BGRA8Unorm");
-            }
+            DAWN_INVALID_IF(descriptor->format != wgpu::TextureFormat::BGRA8Unorm,
+                            "Format (%s) is not %s, which is (currently) the only accepted format.",
+                            descriptor->format, wgpu::TextureFormat::BGRA8Unorm);
 
-            if (descriptor->usage != wgpu::TextureUsage::RenderAttachment) {
-                return DAWN_VALIDATION_ERROR("Usage must (currently) be RenderAttachment");
-            }
+            DAWN_INVALID_IF(descriptor->usage != wgpu::TextureUsage::RenderAttachment,
+                            "Usage (%s) is not %s, which is (currently) the only accepted usage.",
+                            descriptor->usage, wgpu::TextureUsage::RenderAttachment);
 
-            if (descriptor->width == 0 || descriptor->height == 0) {
-                return DAWN_VALIDATION_ERROR("Swapchain size can't be empty");
-            }
+            DAWN_INVALID_IF(descriptor->width == 0 || descriptor->height == 0,
+                            "Swap Chain size (width: %u, height: %u) is empty.", descriptor->width,
+                            descriptor->height);
 
-            if (descriptor->width > device->GetLimits().v1.maxTextureDimension2D ||
-                descriptor->height > device->GetLimits().v1.maxTextureDimension2D) {
-                return DAWN_VALIDATION_ERROR("Swapchain size too big");
-            }
+            DAWN_INVALID_IF(
+                descriptor->width > device->GetLimits().v1.maxTextureDimension2D ||
+                    descriptor->height > device->GetLimits().v1.maxTextureDimension2D,
+                "Swap Chain size (width: %u, height: %u) is greater than the maximum 2D texture "
+                "size (width: %u, height: %u).",
+                descriptor->width, descriptor->height, device->GetLimits().v1.maxTextureDimension2D,
+                device->GetLimits().v1.maxTextureDimension2D);
         }
 
         return {};
@@ -230,9 +231,9 @@ namespace dawn_native {
         DAWN_TRY(ValidateTextureUsage(allowedUsage));
         DAWN_TRY(ValidateTextureFormat(format));
 
-        if (width == 0 || height == 0) {
-            return DAWN_VALIDATION_ERROR("Swap chain cannot be configured to zero size");
-        }
+        DAWN_INVALID_IF(width == 0 || height == 0,
+                        "Configuration size (width: %u, height: %u) for %s is empty.", width,
+                        height, this);
 
         return {};
     }
@@ -241,10 +242,9 @@ namespace dawn_native {
         DAWN_TRY(GetDevice()->ValidateIsAlive());
         DAWN_TRY(GetDevice()->ValidateObject(this));
 
-        if (mWidth == 0) {
-            // If width is 0, it implies swap chain has never been configured
-            return DAWN_VALIDATION_ERROR("Swap chain needs to be configured before GetNextTexture");
-        }
+        // If width is 0, it implies swap chain has never been configured
+        DAWN_INVALID_IF(mWidth == 0, "%s was not configured prior to calling GetNextTexture.",
+                        this);
 
         return {};
     }
@@ -253,10 +253,10 @@ namespace dawn_native {
         DAWN_TRY(GetDevice()->ValidateIsAlive());
         DAWN_TRY(GetDevice()->ValidateObject(this));
 
-        if (mCurrentTextureView == nullptr) {
-            return DAWN_VALIDATION_ERROR(
-                "Cannot call present without a GetCurrentTextureView call for this frame");
-        }
+        DAWN_INVALID_IF(
+            mCurrentTextureView == nullptr,
+            "GetCurrentTextureView was not called on %s this frame prior to calling Present.",
+            this);
 
         return {};
     }
@@ -302,7 +302,7 @@ namespace dawn_native {
                                         uint32_t width,
                                         uint32_t height) {
         GetDevice()->ConsumedError(
-            DAWN_VALIDATION_ERROR("Configure is invalid for surface-based swapchains"));
+            DAWN_FORMAT_VALIDATION_ERROR("Configure is invalid for surface-based swapchains."));
     }
 
     TextureViewBase* NewSwapChainBase::APIGetCurrentTextureView() {
@@ -386,13 +386,12 @@ namespace dawn_native {
         DAWN_TRY(GetDevice()->ValidateIsAlive());
         DAWN_TRY(GetDevice()->ValidateObject(this));
 
-        if (!mAttached) {
-            return DAWN_VALIDATION_ERROR("Presenting on detached swapchain");
-        }
+        DAWN_INVALID_IF(!mAttached, "Cannot call Present called on detached %s.", this);
 
-        if (mCurrentTextureView == nullptr) {
-            return DAWN_VALIDATION_ERROR("Presenting without prior GetCurrentTextureView");
-        }
+        DAWN_INVALID_IF(
+            mCurrentTextureView == nullptr,
+            "GetCurrentTextureView was not called on %s this frame prior to calling Present.",
+            this);
 
         return {};
     }
@@ -401,9 +400,7 @@ namespace dawn_native {
         DAWN_TRY(GetDevice()->ValidateIsAlive());
         DAWN_TRY(GetDevice()->ValidateObject(this));
 
-        if (!mAttached) {
-            return DAWN_VALIDATION_ERROR("Getting view on detached swapchain");
-        }
+        DAWN_INVALID_IF(!mAttached, "Cannot call GetCurrentTextureView on detached %s.", this);
 
         return {};
     }