From eb0d90050667e41f6d28e4755ab3662de4feeb40 Mon Sep 17 00:00:00 2001 From: Brandon Jones Date: Fri, 22 Oct 2021 17:39:20 +0000 Subject: [PATCH] Improving Metal backend validation messages. Improves validation messages in various Metal backend files: - SamplerMTL.mm - ShaderModuleMTL.mm - SwapChainMTL.mm - TextureMTL.mm Bug: dawn:563 Change-Id: I076c08bb3dbecc430d5d4d7fbfeea48166070688 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/67320 Commit-Queue: Brandon Jones Reviewed-by: Corentin Wallez --- src/dawn_native/metal/SamplerMTL.mm | 10 +++-- src/dawn_native/metal/ShaderModuleMTL.mm | 24 +++++------- src/dawn_native/metal/SwapChainMTL.mm | 6 +-- src/dawn_native/metal/TextureMTL.mm | 49 ++++++++++++------------ 4 files changed, 44 insertions(+), 45 deletions(-) diff --git a/src/dawn_native/metal/SamplerMTL.mm b/src/dawn_native/metal/SamplerMTL.mm index dae8d75bc1..608c6bbebe 100644 --- a/src/dawn_native/metal/SamplerMTL.mm +++ b/src/dawn_native/metal/SamplerMTL.mm @@ -53,10 +53,12 @@ namespace dawn_native { namespace metal { // static ResultOrError> Sampler::Create(Device* device, const SamplerDescriptor* descriptor) { - if (descriptor->compare != wgpu::CompareFunction::Undefined && - device->IsToggleEnabled(Toggle::MetalDisableSamplerCompare)) { - return DAWN_VALIDATION_ERROR("Sampler compare function not supported."); - } + DAWN_INVALID_IF( + descriptor->compare != wgpu::CompareFunction::Undefined && + device->IsToggleEnabled(Toggle::MetalDisableSamplerCompare), + "Sampler compare function (%s) not supported. Compare functions are disabled with the " + "Metal backend.", + descriptor->compare); Ref sampler = AcquireRef(new Sampler(device, descriptor)); DAWN_TRY(sampler->Initialize(descriptor)); diff --git a/src/dawn_native/metal/ShaderModuleMTL.mm b/src/dawn_native/metal/ShaderModuleMTL.mm index c994ceef90..4c0aaba951 100644 --- a/src/dawn_native/metal/ShaderModuleMTL.mm +++ b/src/dawn_native/metal/ShaderModuleMTL.mm @@ -143,14 +143,13 @@ namespace dawn_native { namespace metal { if (it != data->remappings.end()) { *remappedEntryPointName = it->second; } else { - if (GetDevice()->IsToggleEnabled(Toggle::DisableSymbolRenaming)) { - *remappedEntryPointName = entryPointName; - } else { - return DAWN_VALIDATION_ERROR("Could not find remapped name for entry point."); - } + DAWN_INVALID_IF(!GetDevice()->IsToggleEnabled(Toggle::DisableSymbolRenaming), + "Could not find remapped name for entry point."); + + *remappedEntryPointName = entryPointName; } } else { - return DAWN_VALIDATION_ERROR("Transform output missing renamer data."); + return DAWN_FORMAT_VALIDATION_ERROR("Transform output missing renamer data."); } tint::writer::msl::Options options; @@ -161,10 +160,8 @@ namespace dawn_native { namespace metal { stage == SingleShaderStage::Vertex && renderPipeline->GetPrimitiveTopology() == wgpu::PrimitiveTopology::PointList; auto result = tint::writer::msl::Generate(&program, options); - if (!result.success) { - errorStream << "Generator: " << result.error << std::endl; - return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); - } + DAWN_INVALID_IF(!result.success, "An error occured while generating MSL: %s.", + result.error); *needsStorageBufferLength = result.needs_storage_buffer_sizes; *hasInvariantAttribute = result.has_invariant_attribute; @@ -226,10 +223,9 @@ namespace dawn_native { namespace metal { options:compileOptions.Get() error:&error]); if (error != nullptr) { - if (error.code != MTLLibraryErrorCompileWarning) { - return DAWN_VALIDATION_ERROR(std::string("Unable to create library object: ") + - [error.localizedDescription UTF8String]); - } + DAWN_INVALID_IF(error.code != MTLLibraryErrorCompileWarning, + "Unable to create library object: %s.", + [error.localizedDescription UTF8String]); } ASSERT(library != nil); diff --git a/src/dawn_native/metal/SwapChainMTL.mm b/src/dawn_native/metal/SwapChainMTL.mm index 84eb649bb0..17851abf33 100644 --- a/src/dawn_native/metal/SwapChainMTL.mm +++ b/src/dawn_native/metal/SwapChainMTL.mm @@ -84,9 +84,9 @@ namespace dawn_native { namespace metal { // TODO(crbug.com/dawn/269): figure out what should happen when surfaces are used by // multiple backends one after the other. It probably needs to block until the backend // and GPU are completely finished with the previous swapchain. - if (previousSwapChain->GetBackendType() != wgpu::BackendType::Metal) { - return DAWN_VALIDATION_ERROR("metal::SwapChain cannot switch between APIs"); - } + DAWN_INVALID_IF(previousSwapChain->GetBackendType() != wgpu::BackendType::Metal, + "Metal SwapChain cannot switch backend types from %s to %s.", + previousSwapChain->GetBackendType(), wgpu::BackendType::Metal); previousSwapChain->DetachFromSurface(); } diff --git a/src/dawn_native/metal/TextureMTL.mm b/src/dawn_native/metal/TextureMTL.mm index 0a8a607cfe..68fca2108c 100644 --- a/src/dawn_native/metal/TextureMTL.mm +++ b/src/dawn_native/metal/TextureMTL.mm @@ -124,7 +124,8 @@ namespace dawn_native { namespace metal { case kCVPixelFormatType_OneComponent8: return wgpu::TextureFormat::R8Unorm; default: - return DAWN_VALIDATION_ERROR("Unsupported IOSurface format"); + return DAWN_FORMAT_VALIDATION_ERROR("Unsupported IOSurface format (%x).", + format); } } @@ -327,38 +328,38 @@ namespace dawn_native { namespace metal { // IOSurfaceGetPlaneCount can return 0 for non-planar IOSurfaces but we will treat // non-planar like it is a single plane. size_t surfacePlaneCount = std::max(size_t(1), IOSurfaceGetPlaneCount(ioSurface)); - if (plane >= surfacePlaneCount) { - return DAWN_VALIDATION_ERROR("IOSurface plane doesn't exist"); - } + DAWN_INVALID_IF(plane >= surfacePlaneCount, + "IOSurface plane (%u) exceeds the surface's plane count (%u).", plane, + surfacePlaneCount); - if (descriptor->dimension != wgpu::TextureDimension::e2D) { - return DAWN_VALIDATION_ERROR("IOSurface texture must be 2D"); - } + DAWN_INVALID_IF(descriptor->dimension != wgpu::TextureDimension::e2D, + "Texture dimension (%s) is not %s.", descriptor->dimension, + wgpu::TextureDimension::e2D); - if (descriptor->mipLevelCount != 1) { - return DAWN_VALIDATION_ERROR("IOSurface mip level count must be 1"); - } + DAWN_INVALID_IF(descriptor->mipLevelCount != 1, "Mip level count (%u) is not 1.", + descriptor->mipLevelCount); - if (descriptor->size.depthOrArrayLayers != 1) { - return DAWN_VALIDATION_ERROR("IOSurface array layer count must be 1"); - } + DAWN_INVALID_IF(descriptor->size.depthOrArrayLayers != 1, + "Array layer count (%u) is not 1.", descriptor->size.depthOrArrayLayers); - if (descriptor->sampleCount != 1) { - return DAWN_VALIDATION_ERROR("IOSurface sample count must be 1"); - } + DAWN_INVALID_IF(descriptor->sampleCount != 1, "Sample count (%u) is not 1.", + descriptor->sampleCount); - if (descriptor->size.width != IOSurfaceGetWidthOfPlane(ioSurface, plane) || - descriptor->size.height != IOSurfaceGetHeightOfPlane(ioSurface, plane) || - descriptor->size.depthOrArrayLayers != 1) { - return DAWN_VALIDATION_ERROR("IOSurface size doesn't match descriptor"); - } + uint32_t surfaceWidth = IOSurfaceGetWidthOfPlane(ioSurface, plane); + uint32_t surfaceHeight = IOSurfaceGetHeightOfPlane(ioSurface, plane); + + DAWN_INVALID_IF( + descriptor->size.width != surfaceWidth || descriptor->size.height != surfaceHeight || + descriptor->size.depthOrArrayLayers != 1, + "IOSurface size (width: %u, height %u, depth: 1) doesn't match descriptor size %s.", + surfaceWidth, surfaceHeight, &descriptor->size); wgpu::TextureFormat ioSurfaceFormat; DAWN_TRY_ASSIGN(ioSurfaceFormat, GetFormatEquivalentToIOSurfaceFormat(IOSurfaceGetPixelFormat(ioSurface))); - if (descriptor->format != ioSurfaceFormat) { - return DAWN_VALIDATION_ERROR("IOSurface format doesn't match descriptor"); - } + DAWN_INVALID_IF(descriptor->format != ioSurfaceFormat, + "IOSurface format (%s) doesn't match the descriptor format (%s).", + ioSurfaceFormat, descriptor->format); return {}; }