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 <bajones@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn_native/metal/SamplerMTL.mm b/src/dawn_native/metal/SamplerMTL.mm
index dae8d75..608c6bb 100644
--- a/src/dawn_native/metal/SamplerMTL.mm
+++ b/src/dawn_native/metal/SamplerMTL.mm
@@ -53,10 +53,12 @@
// static
ResultOrError<Ref<Sampler>> 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> 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 c994cee..4c0aaba 100644
--- a/src/dawn_native/metal/ShaderModuleMTL.mm
+++ b/src/dawn_native/metal/ShaderModuleMTL.mm
@@ -143,14 +143,13 @@
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 @@
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 @@
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 84eb649..17851ab 100644
--- a/src/dawn_native/metal/SwapChainMTL.mm
+++ b/src/dawn_native/metal/SwapChainMTL.mm
@@ -84,9 +84,9 @@
// 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 0a8a607..68fca21 100644
--- a/src/dawn_native/metal/TextureMTL.mm
+++ b/src/dawn_native/metal/TextureMTL.mm
@@ -124,7 +124,8 @@
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 @@
// 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 {};
}
