From e70563ac03f52960cb921bbc2b5ec838ac96620d Mon Sep 17 00:00:00 2001 From: Loko Kung Date: Mon, 12 Dec 2022 20:55:05 +0000 Subject: [PATCH] Implements maxColorAttachmentBytesPerSample - Adds the limit - Adds relevant format-specific data into format table - Adds deprecation validations regarding the limit - Adds deprecation validation unit tests and helpful utils - Moves deprecated api tests from end2end to unittests, allowing tests to be cross-files for ease after deprecation. - Updates some validation messages to include helpful contexts. Bug: dawn:1522 Change-Id: Ib05f9adb60808ff4d68061d9646e76c729a23643 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/113543 Kokoro: Kokoro Reviewed-by: Austin Eng Commit-Queue: Loko Kung Reviewed-by: Corentin Wallez --- dawn.json | 1 + src/dawn/native/CommandEncoder.cpp | 7 +- src/dawn/native/CommandValidation.cpp | 32 ++++++ src/dawn/native/CommandValidation.h | 6 ++ src/dawn/native/Device.cpp | 3 +- src/dawn/native/Error.h | 8 ++ src/dawn/native/Format.cpp | 73 +++++++------ src/dawn/native/Format.h | 6 +- src/dawn/native/Limits.cpp | 1 + src/dawn/native/RenderBundleEncoder.cpp | 7 +- src/dawn/native/RenderBundleEncoder.h | 2 +- src/dawn/native/RenderPipeline.cpp | 5 +- src/dawn/tests/BUILD.gn | 3 +- .../tests/end2end/RenderPassLoadOpTests.cpp | 20 +++- src/dawn/tests/end2end/TextureViewTests.cpp | 4 +- .../validation}/DeprecatedAPITests.cpp | 52 +++------ .../unittests/validation/DeprecatedAPITests.h | 45 ++++++++ .../RenderBundleValidationTests.cpp | 70 +++++++++++- .../RenderPassDescriptorValidationTests.cpp | 73 ++++++++++++- .../RenderPipelineValidationTests.cpp | 100 +++++++++++++++++- src/dawn/utils/WGPUHelpers.cpp | 4 +- src/dawn/utils/WGPUHelpers.h | 2 +- 22 files changed, 438 insertions(+), 86 deletions(-) rename src/dawn/tests/{end2end => unittests/validation}/DeprecatedAPITests.cpp (79%) create mode 100644 src/dawn/tests/unittests/validation/DeprecatedAPITests.h diff --git a/dawn.json b/dawn.json index 115ccc5a18..2b83e89e48 100644 --- a/dawn.json +++ b/dawn.json @@ -1271,6 +1271,7 @@ {"name": "max inter stage shader components", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "max inter stage shader variables", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "max color attachments", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, + {"name": "max color attachment bytes per sample", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "max compute workgroup storage size", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "max compute invocations per workgroup", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "max compute workgroup size x", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp index 95157cad47..f08204759a 100644 --- a/src/dawn/native/CommandEncoder.cpp +++ b/src/dawn/native/CommandEncoder.cpp @@ -458,6 +458,7 @@ MaybeError ValidateRenderPassDescriptor(DeviceBase* device, descriptor->colorAttachmentCount, maxColorAttachments); bool isAllColorAttachmentNull = true; + ColorAttachmentFormats colorAttachmentFormats; for (uint32_t i = 0; i < descriptor->colorAttachmentCount; ++i) { DAWN_TRY_CONTEXT( ValidateRenderPassColorAttachment(device, descriptor->colorAttachments[i], width, @@ -465,8 +466,11 @@ MaybeError ValidateRenderPassDescriptor(DeviceBase* device, "validating colorAttachments[%u].", i); if (descriptor->colorAttachments[i].view) { isAllColorAttachmentNull = false; + colorAttachmentFormats->push_back(&descriptor->colorAttachments[i].view->GetFormat()); } } + DAWN_TRY_CONTEXT(ValidateColorAttachmentBytesPerSample(device, colorAttachmentFormats), + "validating color attachment bytes per sample."); if (descriptor->depthStencilAttachment != nullptr) { DAWN_TRY_CONTEXT(ValidateRenderPassDepthStencilAttachment( @@ -854,7 +858,8 @@ Ref CommandEncoder::BeginRenderPass(const RenderPassDescripto // If descriptor is invalid, make pass invalid and stop immediately if (device->ConsumedError(ValidateRenderPassDescriptor(device, descriptor, &width, &height, - &sampleCount, mUsageValidationMode))) { + &sampleCount, mUsageValidationMode), + "validating render pass descriptor.")) { return RenderPassEncoder::MakeError(device, this, &mEncodingContext); } diff --git a/src/dawn/native/CommandValidation.cpp b/src/dawn/native/CommandValidation.cpp index 836c811ac6..e7874e6635 100644 --- a/src/dawn/native/CommandValidation.cpp +++ b/src/dawn/native/CommandValidation.cpp @@ -16,6 +16,8 @@ #include #include +#include +#include #include #include "dawn/common/BitSetIterator.h" @@ -491,4 +493,34 @@ MaybeError ValidateCanUseAs(const BufferBase* buffer, wgpu::BufferUsage usage) { return {}; } +namespace { +std::string TextureFormatsToString(const ColorAttachmentFormats& formats) { + std::ostringstream ss; + ss << "[ "; + for (const Format* format : formats) { + ss << absl::StrFormat("%s", format->format) << " "; + } + ss << "]"; + return ss.str(); +} +} // anonymous namespace + +MaybeError ValidateColorAttachmentBytesPerSample(DeviceBase* device, + const ColorAttachmentFormats& formats) { + uint32_t totalByteSize = 0; + for (const Format* format : formats) { + totalByteSize = Align(totalByteSize, format->renderTargetComponentAlignment); + totalByteSize += format->renderTargetPixelByteCost; + } + uint32_t maxColorAttachmentBytesPerSample = + device->GetLimits().v1.maxColorAttachmentBytesPerSample; + // TODO(dawn:1522) Promote to DAWN_INVALID_IF after deprecation period. + DAWN_DEPRECATED_IF( + device, totalByteSize > maxColorAttachmentBytesPerSample, + "Total color attachment bytes per sample (%u) exceeds maximum (%u) with formats (%s).", + totalByteSize, maxColorAttachmentBytesPerSample, TextureFormatsToString(formats)); + + return {}; +} + } // namespace dawn::native diff --git a/src/dawn/native/CommandValidation.h b/src/dawn/native/CommandValidation.h index d4f66d70a3..53ecfc24e8 100644 --- a/src/dawn/native/CommandValidation.h +++ b/src/dawn/native/CommandValidation.h @@ -17,6 +17,8 @@ #include +#include "dawn/common/Constants.h" +#include "dawn/common/StackContainer.h" #include "dawn/native/CommandAllocator.h" #include "dawn/native/Error.h" #include "dawn/native/Features.h" @@ -90,6 +92,10 @@ MaybeError ValidateCanUseAs(const TextureBase* texture, UsageValidationMode mode); MaybeError ValidateCanUseAs(const BufferBase* buffer, wgpu::BufferUsage usage); +using ColorAttachmentFormats = StackVector; +MaybeError ValidateColorAttachmentBytesPerSample(DeviceBase* device, + const ColorAttachmentFormats& formats); + } // namespace dawn::native #endif // SRC_DAWN_NATIVE_COMMANDVALIDATION_H_ diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index c713c26f7d..23607a16b3 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp @@ -1585,7 +1585,8 @@ ResultOrError> DeviceBase::CreateRenderBundleEncoder( const RenderBundleEncoderDescriptor* descriptor) { DAWN_TRY(ValidateIsAlive()); if (IsValidationEnabled()) { - DAWN_TRY(ValidateRenderBundleEncoderDescriptor(this, descriptor)); + DAWN_TRY_CONTEXT(ValidateRenderBundleEncoderDescriptor(this, descriptor), + "validating render bundle encoder descriptor."); } return RenderBundleEncoder::Create(this, descriptor); } diff --git a/src/dawn/native/Error.h b/src/dawn/native/Error.h index 6ab93c788c..4523a547f4 100644 --- a/src/dawn/native/Error.h +++ b/src/dawn/native/Error.h @@ -90,6 +90,14 @@ using ResultOrError = Result; ? MaybeError(DAWN_VALIDATION_ERROR(__VA_ARGS__)) \ : (device->EmitDeprecationWarning(absl::StrFormat(__VA_ARGS__)), MaybeError{}) +// DAWN_DEPRECATED_IF is used analogous to DAWN_INVALID_IF at deprecation paths. +#define DAWN_DEPRECATED_IF(device, EXPR, ...) \ + if (DAWN_UNLIKELY(EXPR)) { \ + return DAWN_MAKE_DEPRECATION_ERROR(device, __VA_ARGS__); \ + } \ + for (;;) \ + break + // DAWN_DEVICE_LOST_ERROR means that there was a real unrecoverable native device lost error. // We can't even do a graceful shutdown because the Device is gone. #define DAWN_DEVICE_LOST_ERROR(MESSAGE) DAWN_MAKE_ERROR(InternalErrorType::DeviceLost, MESSAGE) diff --git a/src/dawn/native/Format.cpp b/src/dawn/native/Format.cpp index fdfe477945..7969515315 100644 --- a/src/dawn/native/Format.cpp +++ b/src/dawn/native/Format.cpp @@ -160,6 +160,8 @@ FormatTable BuildFormatTable(const DeviceBase* device) { [&AddFormat](wgpu::TextureFormat format, bool renderable, bool supportsStorageUsage, bool supportsMultisample, bool supportsResolveTarget, uint32_t byteSize, SampleTypeBit sampleTypes, uint8_t componentCount, + uint8_t renderTargetPixelByteCost = 0, + uint8_t renderTargetComponentAlignment = 0, wgpu::TextureFormat baseFormat = wgpu::TextureFormat::Undefined) { Format internalFormat; internalFormat.format = format; @@ -175,6 +177,13 @@ FormatTable BuildFormatTable(const DeviceBase* device) { internalFormat.supportsResolveTarget = supportsResolveTarget; internalFormat.aspects = Aspect::Color; internalFormat.componentCount = componentCount; + if (renderable) { + // If the color format is renderable, it must have a pixel byte size and component + // alignment specified. + ASSERT(renderTargetPixelByteCost != 0 && renderTargetComponentAlignment != 0); + internalFormat.renderTargetPixelByteCost = renderTargetPixelByteCost; + internalFormat.renderTargetComponentAlignment = renderTargetComponentAlignment; + } // Default baseFormat of each color formats should be themselves. if (baseFormat == wgpu::TextureFormat::Undefined) { @@ -333,52 +342,52 @@ FormatTable BuildFormatTable(const DeviceBase* device) { // clang-format off // 1 byte color formats - AddColorFormat(wgpu::TextureFormat::R8Unorm, true, false, true, true, 1, kAnyFloat, 1); + AddColorFormat(wgpu::TextureFormat::R8Unorm, true, false, true, true, 1, kAnyFloat, 1, 1, 1); AddColorFormat(wgpu::TextureFormat::R8Snorm, false, false, false, false, 1, kAnyFloat, 1); - AddColorFormat(wgpu::TextureFormat::R8Uint, true, false, true, false, 1, SampleTypeBit::Uint, 1); - AddColorFormat(wgpu::TextureFormat::R8Sint, true, false, true, false, 1, SampleTypeBit::Sint, 1); + AddColorFormat(wgpu::TextureFormat::R8Uint, true, false, true, false, 1, SampleTypeBit::Uint, 1, 1, 1); + AddColorFormat(wgpu::TextureFormat::R8Sint, true, false, true, false, 1, SampleTypeBit::Sint, 1, 1, 1); // 2 bytes color formats - AddColorFormat(wgpu::TextureFormat::R16Uint, true, false, true, false, 2, SampleTypeBit::Uint, 1); - AddColorFormat(wgpu::TextureFormat::R16Sint, true, false, true, false, 2, SampleTypeBit::Sint, 1); - AddColorFormat(wgpu::TextureFormat::R16Float, true, false, true, true, 2, kAnyFloat, 1); - AddColorFormat(wgpu::TextureFormat::RG8Unorm, true, false, true, true, 2, kAnyFloat, 2); + AddColorFormat(wgpu::TextureFormat::R16Uint, true, false, true, false, 2, SampleTypeBit::Uint, 1, 2, 2); + AddColorFormat(wgpu::TextureFormat::R16Sint, true, false, true, false, 2, SampleTypeBit::Sint, 1, 2, 2); + AddColorFormat(wgpu::TextureFormat::R16Float, true, false, true, true, 2, kAnyFloat, 1, 2, 2); + AddColorFormat(wgpu::TextureFormat::RG8Unorm, true, false, true, true, 2, kAnyFloat, 2, 2, 1); AddColorFormat(wgpu::TextureFormat::RG8Snorm, false, false, false, false, 2, kAnyFloat, 2); - AddColorFormat(wgpu::TextureFormat::RG8Uint, true, false, true, false, 2, SampleTypeBit::Uint, 2); - AddColorFormat(wgpu::TextureFormat::RG8Sint, true, false, true, false, 2, SampleTypeBit::Sint, 2); + AddColorFormat(wgpu::TextureFormat::RG8Uint, true, false, true, false, 2, SampleTypeBit::Uint, 2, 2, 1); + AddColorFormat(wgpu::TextureFormat::RG8Sint, true, false, true, false, 2, SampleTypeBit::Sint, 2, 2, 1); // 4 bytes color formats - AddColorFormat(wgpu::TextureFormat::R32Uint, true, true, false, false, 4, SampleTypeBit::Uint, 1); - AddColorFormat(wgpu::TextureFormat::R32Sint, true, true, false, false, 4, SampleTypeBit::Sint, 1); - AddColorFormat(wgpu::TextureFormat::R32Float, true, true, true, false, 4, SampleTypeBit::UnfilterableFloat, 1); - AddColorFormat(wgpu::TextureFormat::RG16Uint, true, false, true, false, 4, SampleTypeBit::Uint, 2); - AddColorFormat(wgpu::TextureFormat::RG16Sint, true, false, true, false, 4, SampleTypeBit::Sint, 2); - AddColorFormat(wgpu::TextureFormat::RG16Float, true, false, true, true, 4, kAnyFloat, 2); - AddColorFormat(wgpu::TextureFormat::RGBA8Unorm, true, true, true, true, 4, kAnyFloat, 4); - AddColorFormat(wgpu::TextureFormat::RGBA8UnormSrgb, true, false, true, true, 4, kAnyFloat, 4, wgpu::TextureFormat::RGBA8Unorm); + AddColorFormat(wgpu::TextureFormat::R32Uint, true, true, false, false, 4, SampleTypeBit::Uint, 1, 4, 4); + AddColorFormat(wgpu::TextureFormat::R32Sint, true, true, false, false, 4, SampleTypeBit::Sint, 1, 4, 4); + AddColorFormat(wgpu::TextureFormat::R32Float, true, true, true, false, 4, SampleTypeBit::UnfilterableFloat, 1, 4, 4); + AddColorFormat(wgpu::TextureFormat::RG16Uint, true, false, true, false, 4, SampleTypeBit::Uint, 2, 4, 2); + AddColorFormat(wgpu::TextureFormat::RG16Sint, true, false, true, false, 4, SampleTypeBit::Sint, 2, 4, 2); + AddColorFormat(wgpu::TextureFormat::RG16Float, true, false, true, true, 4, kAnyFloat, 2, 4, 2); + AddColorFormat(wgpu::TextureFormat::RGBA8Unorm, true, true, true, true, 4, kAnyFloat, 4, 8, 1); + AddColorFormat(wgpu::TextureFormat::RGBA8UnormSrgb, true, false, true, true, 4, kAnyFloat, 4, 8, 1, wgpu::TextureFormat::RGBA8Unorm); AddColorFormat(wgpu::TextureFormat::RGBA8Snorm, false, true, false, false, 4, kAnyFloat, 4); - AddColorFormat(wgpu::TextureFormat::RGBA8Uint, true, true, true, false, 4, SampleTypeBit::Uint, 4); - AddColorFormat(wgpu::TextureFormat::RGBA8Sint, true, true, true, false, 4, SampleTypeBit::Sint, 4); - AddColorFormat(wgpu::TextureFormat::BGRA8Unorm, true, false, true, true, 4, kAnyFloat, 4); - AddColorFormat(wgpu::TextureFormat::BGRA8UnormSrgb, true, false, true, true, 4, kAnyFloat, 4, wgpu::TextureFormat::BGRA8Unorm); - AddColorFormat(wgpu::TextureFormat::RGB10A2Unorm, true, false, true, true, 4, kAnyFloat, 4); + AddColorFormat(wgpu::TextureFormat::RGBA8Uint, true, true, true, false, 4, SampleTypeBit::Uint, 4, 4, 1); + AddColorFormat(wgpu::TextureFormat::RGBA8Sint, true, true, true, false, 4, SampleTypeBit::Sint, 4, 4, 1); + AddColorFormat(wgpu::TextureFormat::BGRA8Unorm, true, false, true, true, 4, kAnyFloat, 4, 8, 1); + AddColorFormat(wgpu::TextureFormat::BGRA8UnormSrgb, true, false, true, true, 4, kAnyFloat, 4, 8, 1, wgpu::TextureFormat::BGRA8Unorm); + AddColorFormat(wgpu::TextureFormat::RGB10A2Unorm, true, false, true, true, 4, kAnyFloat, 4, 8, 4); bool isRG11B10UfloatRenderable = device->HasFeature(Feature::RG11B10UfloatRenderable); - AddColorFormat(wgpu::TextureFormat::RG11B10Ufloat, isRG11B10UfloatRenderable, false, isRG11B10UfloatRenderable, false, 4, kAnyFloat, 3); + AddColorFormat(wgpu::TextureFormat::RG11B10Ufloat, isRG11B10UfloatRenderable, false, isRG11B10UfloatRenderable, false, 4, kAnyFloat, 3, 8, 4); AddColorFormat(wgpu::TextureFormat::RGB9E5Ufloat, false, false, false, false, 4, kAnyFloat, 3); // 8 bytes color formats - AddColorFormat(wgpu::TextureFormat::RG32Uint, true, true, false, false, 8, SampleTypeBit::Uint, 2); - AddColorFormat(wgpu::TextureFormat::RG32Sint, true, true, false, false, 8, SampleTypeBit::Sint, 2); - AddColorFormat(wgpu::TextureFormat::RG32Float, true, true, false, false, 8, SampleTypeBit::UnfilterableFloat, 2); - AddColorFormat(wgpu::TextureFormat::RGBA16Uint, true, true, true, false, 8, SampleTypeBit::Uint, 4); - AddColorFormat(wgpu::TextureFormat::RGBA16Sint, true, true, true, false, 8, SampleTypeBit::Sint, 4); - AddColorFormat(wgpu::TextureFormat::RGBA16Float, true, true, true, true, 8, kAnyFloat, 4); + AddColorFormat(wgpu::TextureFormat::RG32Uint, true, true, false, false, 8, SampleTypeBit::Uint, 2, 8, 4); + AddColorFormat(wgpu::TextureFormat::RG32Sint, true, true, false, false, 8, SampleTypeBit::Sint, 2, 8, 4); + AddColorFormat(wgpu::TextureFormat::RG32Float, true, true, false, false, 8, SampleTypeBit::UnfilterableFloat, 2, 8, 4); + AddColorFormat(wgpu::TextureFormat::RGBA16Uint, true, true, true, false, 8, SampleTypeBit::Uint, 4, 8, 2); + AddColorFormat(wgpu::TextureFormat::RGBA16Sint, true, true, true, false, 8, SampleTypeBit::Sint, 4, 8, 2); + AddColorFormat(wgpu::TextureFormat::RGBA16Float, true, true, true, true, 8, kAnyFloat, 4, 8, 2); // 16 bytes color formats - AddColorFormat(wgpu::TextureFormat::RGBA32Uint, true, true, false, false, 16, SampleTypeBit::Uint, 4); - AddColorFormat(wgpu::TextureFormat::RGBA32Sint, true, true, false, false, 16, SampleTypeBit::Sint, 4); - AddColorFormat(wgpu::TextureFormat::RGBA32Float, true, true, false, false, 16, SampleTypeBit::UnfilterableFloat, 4); + AddColorFormat(wgpu::TextureFormat::RGBA32Uint, true, true, false, false, 16, SampleTypeBit::Uint, 4, 16, 4); + AddColorFormat(wgpu::TextureFormat::RGBA32Sint, true, true, false, false, 16, SampleTypeBit::Sint, 4, 16, 4); + AddColorFormat(wgpu::TextureFormat::RGBA32Float, true, true, false, false, 16, SampleTypeBit::UnfilterableFloat, 4, 16, 4); // Depth-stencil formats AddStencilFormat(wgpu::TextureFormat::Stencil8, true); diff --git a/src/dawn/native/Format.h b/src/dawn/native/Format.h index 0a5f8f2158..8a895c7c5a 100644 --- a/src/dawn/native/Format.h +++ b/src/dawn/native/Format.h @@ -97,8 +97,10 @@ struct Format { bool supportsMultisample = false; bool supportsResolveTarget = false; Aspect aspects{}; - // Only used for renderable color formats, number of color channels. - uint8_t componentCount = 0; + // Only used for renderable color formats: + uint8_t componentCount = 0; // number of color channels + uint8_t renderTargetPixelByteCost = 0; // byte cost of pixel in render targets + uint8_t renderTargetComponentAlignment = 0; // byte alignment for components in render targets bool IsColor() const; bool HasDepth() const; diff --git a/src/dawn/native/Limits.cpp b/src/dawn/native/Limits.cpp index dfa06509e8..433ab3c6e4 100644 --- a/src/dawn/native/Limits.cpp +++ b/src/dawn/native/Limits.cpp @@ -59,6 +59,7 @@ X(Maximum, maxInterStageShaderComponents, 60, 60) \ X(Maximum, maxInterStageShaderVariables, 16, 16) \ X(Maximum, maxColorAttachments, 8, 8) \ + X(Maximum, maxColorAttachmentBytesPerSample, 32, 32) \ X(Maximum, maxComputeInvocationsPerWorkgroup, 256, 256) \ X(Maximum, maxComputeWorkgroupSizeX, 256, 256) \ X(Maximum, maxComputeWorkgroupSizeY, 256, 256) \ diff --git a/src/dawn/native/RenderBundleEncoder.cpp b/src/dawn/native/RenderBundleEncoder.cpp index 01e48fc9e9..7649a3b09f 100644 --- a/src/dawn/native/RenderBundleEncoder.cpp +++ b/src/dawn/native/RenderBundleEncoder.cpp @@ -16,6 +16,7 @@ #include +#include "dawn/common/StackContainer.h" #include "dawn/native/CommandValidation.h" #include "dawn/native/Commands.h" #include "dawn/native/Device.h" @@ -57,7 +58,7 @@ MaybeError ValidateDepthStencilAttachmentFormat(const DeviceBase* device, return {}; } -MaybeError ValidateRenderBundleEncoderDescriptor(const DeviceBase* device, +MaybeError ValidateRenderBundleEncoderDescriptor(DeviceBase* device, const RenderBundleEncoderDescriptor* descriptor) { DAWN_INVALID_IF(!IsValidSampleCount(descriptor->sampleCount), "Sample count (%u) is not supported.", descriptor->sampleCount); @@ -68,14 +69,18 @@ MaybeError ValidateRenderBundleEncoderDescriptor(const DeviceBase* device, descriptor->colorFormatsCount, maxColorAttachments); bool allColorFormatsUndefined = true; + ColorAttachmentFormats colorAttachmentFormats; for (uint32_t i = 0; i < descriptor->colorFormatsCount; ++i) { wgpu::TextureFormat format = descriptor->colorFormats[i]; if (format != wgpu::TextureFormat::Undefined) { DAWN_TRY_CONTEXT(ValidateColorAttachmentFormat(device, format), "validating colorFormats[%u]", i); + colorAttachmentFormats->push_back(&device->GetValidInternalFormat(format)); allColorFormatsUndefined = false; } } + DAWN_TRY_CONTEXT(ValidateColorAttachmentBytesPerSample(device, colorAttachmentFormats), + "validating color attachment bytes per sample."); if (descriptor->depthStencilFormat != wgpu::TextureFormat::Undefined) { DAWN_TRY_CONTEXT(ValidateDepthStencilAttachmentFormat( diff --git a/src/dawn/native/RenderBundleEncoder.h b/src/dawn/native/RenderBundleEncoder.h index 24ee19ef01..b8a9d61502 100644 --- a/src/dawn/native/RenderBundleEncoder.h +++ b/src/dawn/native/RenderBundleEncoder.h @@ -23,7 +23,7 @@ namespace dawn::native { -MaybeError ValidateRenderBundleEncoderDescriptor(const DeviceBase* device, +MaybeError ValidateRenderBundleEncoderDescriptor(DeviceBase* device, const RenderBundleEncoderDescriptor* descriptor); class RenderBundleEncoder final : public RenderEncoderBase { diff --git a/src/dawn/native/RenderPipeline.cpp b/src/dawn/native/RenderPipeline.cpp index 9b0ff96e45..60e8125773 100644 --- a/src/dawn/native/RenderPipeline.cpp +++ b/src/dawn/native/RenderPipeline.cpp @@ -16,10 +16,10 @@ #include #include -#include #include "dawn/common/BitSetIterator.h" #include "dawn/native/ChainUtils_autogen.h" +#include "dawn/native/CommandValidation.h" #include "dawn/native/Commands.h" #include "dawn/native/Device.h" #include "dawn/native/InternalPipelineStore.h" @@ -351,6 +351,7 @@ MaybeError ValidateFragmentState(DeviceBase* device, const EntryPointMetadata& fragmentMetadata = descriptor->module->GetEntryPoint(descriptor->entryPoint); + ColorAttachmentFormats colorAttachmentFormats; for (ColorAttachmentIndex i(uint8_t(0)); i < ColorAttachmentIndex(static_cast(descriptor->targetCount)); ++i) { const ColorTargetState* target = &descriptor->targets[static_cast(i)]; @@ -359,12 +360,14 @@ MaybeError ValidateFragmentState(DeviceBase* device, ValidateColorTargetState(device, target, fragmentMetadata.fragmentOutputsWritten[i], fragmentMetadata.fragmentOutputVariables[i]), "validating targets[%u].", static_cast(i)); + colorAttachmentFormats->push_back(&device->GetValidInternalFormat(target->format)); } else { DAWN_INVALID_IF(target->blend, "Color target[%u] blend state is set when the format is undefined.", static_cast(i)); } } + DAWN_TRY(ValidateColorAttachmentBytesPerSample(device, colorAttachmentFormats)); DAWN_INVALID_IF(fragmentMetadata.usesSampleMaskOutput && alphaToCoverageEnabled, "alphaToCoverageEnabled is true when the sample_mask builtin is a " diff --git a/src/dawn/tests/BUILD.gn b/src/dawn/tests/BUILD.gn index 80bfe3fcd5..21a1b3684e 100644 --- a/src/dawn/tests/BUILD.gn +++ b/src/dawn/tests/BUILD.gn @@ -317,6 +317,8 @@ dawn_test("dawn_unittests") { "unittests/validation/CopyCommandsValidationTests.cpp", "unittests/validation/CopyTextureForBrowserTests.cpp", "unittests/validation/DebugMarkerValidationTests.cpp", + "unittests/validation/DeprecatedAPITests.cpp", + "unittests/validation/DeprecatedAPITests.h", "unittests/validation/DeviceValidationTests.cpp", "unittests/validation/DrawIndirectValidationTests.cpp", "unittests/validation/DrawVertexAndIndexBufferOOBValidationTests.cpp", @@ -486,7 +488,6 @@ source_set("end2end_tests_sources") { "end2end/CreatePipelineAsyncTests.cpp", "end2end/CullingTests.cpp", "end2end/DebugMarkerTests.cpp", - "end2end/DeprecatedAPITests.cpp", "end2end/DepthBiasTests.cpp", "end2end/DepthStencilCopyTests.cpp", "end2end/DepthStencilLoadOpTests.cpp", diff --git a/src/dawn/tests/end2end/RenderPassLoadOpTests.cpp b/src/dawn/tests/end2end/RenderPassLoadOpTests.cpp index ca70dbb23c..2b16c34f07 100644 --- a/src/dawn/tests/end2end/RenderPassLoadOpTests.cpp +++ b/src/dawn/tests/end2end/RenderPassLoadOpTests.cpp @@ -465,6 +465,7 @@ TEST_P(RenderPassLoadOpTests, LoadOpClearNormalizedFormatsOutOfBound) { } // Test clearing multiple color attachments with different big integers can still work correctly. +// TODO(dawn:1522) Refactor and fix this test to avoid deprecation warnings. TEST_P(RenderPassLoadOpTests, LoadOpClearWithBigInt32ValuesOnMultipleColorAttachments) { constexpr int32_t kMaxInt32RepresentableInFloat = 1 << std::numeric_limits::digits; constexpr int32_t kMinInt32RepresentableInFloat = -kMaxInt32RepresentableInFloat; @@ -523,7 +524,14 @@ TEST_P(RenderPassLoadOpTests, LoadOpClearWithBigInt32ValuesOnMultipleColorAttach wgpu::RenderPassDescriptor renderPassDescriptor = {}; renderPassDescriptor.colorAttachmentCount = kMaxColorAttachments; renderPassDescriptor.colorAttachments = colorAttachmentsInfo.data(); - wgpu::RenderPassEncoder renderPass = encoder.BeginRenderPass(&renderPassDescriptor); + wgpu::RenderPassEncoder renderPass; + if (HasToggleEnabled("apply_clear_big_integer_color_value_with_draw")) { + // When the toggle is enabled, an extra internal pipeline is created which will hit the same + // deprecation issue again, hence we need to check for 2 warnings instead of 1. + EXPECT_DEPRECATION_WARNINGS(renderPass = encoder.BeginRenderPass(&renderPassDescriptor), 2); + } else { + EXPECT_DEPRECATION_WARNING(renderPass = encoder.BeginRenderPass(&renderPassDescriptor)); + } renderPass.End(); std::array outputBuffers; @@ -553,6 +561,7 @@ TEST_P(RenderPassLoadOpTests, LoadOpClearWithBigInt32ValuesOnMultipleColorAttach // Test clearing multiple color attachments with different big unsigned integers can still work // correctly. +// TODO(dawn:1522) Refactor and fix this test to avoid deprecation warnings. TEST_P(RenderPassLoadOpTests, LoadOpClearWithBigUInt32ValuesOnMultipleColorAttachments) { constexpr int32_t kMaxUInt32RepresentableInFloat = 1 << std::numeric_limits::digits; @@ -628,7 +637,14 @@ TEST_P(RenderPassLoadOpTests, LoadOpClearWithBigUInt32ValuesOnMultipleColorAttac wgpu::RenderPassDescriptor renderPassDescriptor = {}; renderPassDescriptor.colorAttachmentCount = kMaxColorAttachments; renderPassDescriptor.colorAttachments = colorAttachmentsInfo.data(); - wgpu::RenderPassEncoder renderPass = encoder.BeginRenderPass(&renderPassDescriptor); + wgpu::RenderPassEncoder renderPass; + if (HasToggleEnabled("apply_clear_big_integer_color_value_with_draw")) { + // When the toggle is enabled, an extra internal pipeline is created which will hit the same + // deprecation issue again, hence we need to check for 2 warnings instead of 1. + EXPECT_DEPRECATION_WARNINGS(renderPass = encoder.BeginRenderPass(&renderPassDescriptor), 2); + } else { + EXPECT_DEPRECATION_WARNING(renderPass = encoder.BeginRenderPass(&renderPassDescriptor)); + } renderPass.End(); std::array outputBuffers; diff --git a/src/dawn/tests/end2end/TextureViewTests.cpp b/src/dawn/tests/end2end/TextureViewTests.cpp index e00f08fecb..2955338e0b 100644 --- a/src/dawn/tests/end2end/TextureViewTests.cpp +++ b/src/dawn/tests/end2end/TextureViewTests.cpp @@ -832,7 +832,7 @@ TEST_P(TextureViewRenderingTest, SRGBReinterpretationRenderAttachment) { wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0), {{0, sampledTexture.CreateView()}}); - utils::ComboRenderPassDescriptor renderPassInfo{textureView}; + utils::ComboRenderPassDescriptor renderPassInfo({textureView}); wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassInfo); pass.SetPipeline(pipeline); @@ -947,7 +947,7 @@ TEST_P(TextureViewRenderingTest, SRGBReinterpretionResolveAttachment) { wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0), {{0, sampledTexture.CreateView()}}); - utils::ComboRenderPassDescriptor renderPassInfo{multisampledTextureView}; + utils::ComboRenderPassDescriptor renderPassInfo({multisampledTextureView}); renderPassInfo.cColorAttachments[0].resolveTarget = resolveView; wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassInfo); diff --git a/src/dawn/tests/end2end/DeprecatedAPITests.cpp b/src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp similarity index 79% rename from src/dawn/tests/end2end/DeprecatedAPITests.cpp rename to src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp index 20d3fdd342..152be1b3f3 100644 --- a/src/dawn/tests/end2end/DeprecatedAPITests.cpp +++ b/src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp @@ -12,38 +12,27 @@ // See the License for the specific language governing permissions and // limitations under the License. -// This file contains test for deprecated parts of Dawn's API while following WebGPU's evolution. -// It contains test for the "old" behavior that will be deleted once users are migrated, tests that -// a deprecation warning is emitted when the "old" behavior is used, and tests that an error is -// emitted when both the old and the new behavior are used (when applicable). - #include -#include "dawn/tests/DawnTest.h" +#include "dawn/tests/unittests/validation/DeprecatedAPITests.h" #include "dawn/common/Constants.h" #include "dawn/utils/ComboRenderPipelineDescriptor.h" #include "dawn/utils/WGPUHelpers.h" -constexpr char kDisallowDeprecatedAPIsToggleName[] = "disallow_deprecated_apis"; +WGPUDevice DeprecationTests::CreateTestDevice(dawn::native::Adapter dawnAdapter) { + wgpu::DeviceDescriptor descriptor = {}; -#define EXPECT_DEPRECATION_ERROR_OR_WARNING(statement) \ - if (HasToggleEnabled(kDisallowDeprecatedAPIsToggleName)) { \ - ASSERT_DEVICE_ERROR(statement); \ - } else { \ - EXPECT_DEPRECATION_WARNING(statement); \ - } \ - for (;;) \ - break + wgpu::DawnTogglesDeviceDescriptor togglesDesc = {}; + const char* forceEnabledToggles[1] = {kDisallowDeprecatedAPIsToggleName}; + togglesDesc.forceEnabledToggles = forceEnabledToggles; + togglesDesc.forceEnabledTogglesCount = 1; -class DeprecationTests : public DawnTest { - protected: - void SetUp() override { - DawnTest::SetUp(); - // Skip when validation is off because warnings might be emitted during validation calls - DAWN_TEST_UNSUPPORTED_IF(HasToggleEnabled("skip_validation")); + if (GetParam()) { + descriptor.nextInChain = &togglesDesc; } -}; + return dawnAdapter.CreateDevice(&descriptor); +} // Test that setting attachment rather than view for render pass color and depth/stencil attachments // is deprecated. @@ -208,16 +197,9 @@ TEST_P(DeprecationTests, MultisampledTextureSampleType) { })); } -DAWN_INSTANTIATE_TEST(DeprecationTests, - D3D12Backend(), - MetalBackend(), - NullBackend(), - OpenGLBackend(), - OpenGLESBackend(), - VulkanBackend(), - D3D12Backend({kDisallowDeprecatedAPIsToggleName}), - MetalBackend({kDisallowDeprecatedAPIsToggleName}), - NullBackend({kDisallowDeprecatedAPIsToggleName}), - OpenGLBackend({kDisallowDeprecatedAPIsToggleName}), - OpenGLESBackend({kDisallowDeprecatedAPIsToggleName}), - VulkanBackend({kDisallowDeprecatedAPIsToggleName})); +INSTANTIATE_TEST_SUITE_P(DeprecatedAPITest, + DeprecationTests, + testing::Values(true, false), + [](const testing::TestParamInfo& info) { + return info.param ? "Disallowed" : "Allowed"; + }); diff --git a/src/dawn/tests/unittests/validation/DeprecatedAPITests.h b/src/dawn/tests/unittests/validation/DeprecatedAPITests.h new file mode 100644 index 0000000000..e00f217dbb --- /dev/null +++ b/src/dawn/tests/unittests/validation/DeprecatedAPITests.h @@ -0,0 +1,45 @@ +// Copyright 2022 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef SRC_DAWN_TESTS_UNITTESTS_VALIDATION_DEPRECATEDAPITESTS_H_ +#define SRC_DAWN_TESTS_UNITTESTS_VALIDATION_DEPRECATEDAPITESTS_H_ + +#include "dawn/tests/unittests/validation/ValidationTest.h" + +// This test header should be included when testing for deprecated parts of Dawn's API while +// following WebGPU's evolution. Tests in this suite test that a deprecation warning is emitted when +// the "old" behavior is used, and tests that an error is emitted when both the old and the new +// behavior are used (when applicable). Note that implementations of tests in this suite may be +// scattered in other files as well for organizational purposes so that similar tests can live +// together. + +static constexpr char kDisallowDeprecatedAPIsToggleName[] = "disallow_deprecated_apis"; + +#define EXPECT_DEPRECATION_ERROR_OR_WARNING(statement) \ + if (HasToggleEnabled(kDisallowDeprecatedAPIsToggleName)) { \ + ASSERT_DEVICE_ERROR(statement); \ + } else { \ + EXPECT_DEPRECATION_WARNING(statement); \ + } \ + for (;;) \ + break + +// Parameter is a single bool. When true, deprecated APIs are strictly disallowed (i.e. generate +// errors). Otherwise, deprecated APIs only generate a warning message. +class DeprecationTests : public ValidationTest, public testing::WithParamInterface { + protected: + WGPUDevice CreateTestDevice(dawn::native::Adapter dawnAdapter) override; +}; + +#endif // SRC_DAWN_TESTS_UNITTESTS_VALIDATION_DEPRECATEDAPITESTS_H_ diff --git a/src/dawn/tests/unittests/validation/RenderBundleValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderBundleValidationTests.cpp index aebdc0e88a..f00a0bbb81 100644 --- a/src/dawn/tests/unittests/validation/RenderBundleValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/RenderBundleValidationTests.cpp @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include + +#include "dawn/tests/unittests/validation/DeprecatedAPITests.h" #include "dawn/tests/unittests/validation/ValidationTest.h" #include "dawn/common/Constants.h" @@ -615,7 +618,7 @@ TEST_F(RenderBundleValidationTest, RequiresAtLeastOneTextureFormat) { TEST_F(RenderBundleValidationTest, ColorFormatsCountOutOfBounds) { std::array colorFormats; for (uint32_t i = 0; i < colorFormats.size(); ++i) { - colorFormats[i] = wgpu::TextureFormat::RGBA8Unorm; + colorFormats[i] = wgpu::TextureFormat::R8Unorm; } // colorFormatsCount <= kMaxColorAttachments is valid. @@ -1139,3 +1142,68 @@ TEST_F(RenderBundleValidationTest, TextureFormats) { // Don't test non-renerable depth/stencil formats because we don't have any. } + +// Tests validation for per-pixel accounting for render targets. The tests currently assume that the +// default maxColorAttachmentBytesPerSample limit of 32 is used. +TEST_P(DeprecationTests, RenderBundleColorFormatsBytesPerSample) { + struct TestCase { + std::vector formats; + bool success; + }; + static std::vector kTestCases = { + // Simple 1 format cases. + + // R8Unorm take 1 byte and are aligned to 1 byte so we can have 8 (max). + {{wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, + wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, + wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm}, + true}, + // RGBA8Uint takes 4 bytes and are aligned to 1 byte so we can have 8 (max). + {{wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint, + wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint, + wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint, + wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint}, + true}, + // RGBA8Unorm takes 8 bytes (special case) and are aligned to 1 byte so only 4 allowed. + {{wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm}, + true}, + {{wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA8Unorm}, + false}, + // RGBA32Float takes 16 bytes and are aligned to 4 bytes so only 2 are allowed. + {{wgpu::TextureFormat::RGBA32Float, wgpu::TextureFormat::RGBA32Float}, true}, + {{wgpu::TextureFormat::RGBA32Float, wgpu::TextureFormat::RGBA32Float, + wgpu::TextureFormat::RGBA32Float}, + false}, + + // Different format alignment cases. + + // Alignment causes the first 1 byte R8Unorm to become 4 bytes. So even though 1+4+8+16+1 < + // 32, the 4 byte alignment requirement of R32Float makes the first R8Unorm become 4 and + // 4+4+8+16+1 > 32. Re-ordering this so the R8Unorm's are at the end, however is allowed: + // 4+8+16+1+1 < 32. + {{wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R32Float, + wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA32Float, + wgpu::TextureFormat::R8Unorm}, + false}, + {{wgpu::TextureFormat::R32Float, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA32Float, wgpu::TextureFormat::R8Unorm, + wgpu::TextureFormat::R8Unorm}, + true}, + }; + + for (const TestCase& testCase : kTestCases) { + utils::ComboRenderBundleEncoderDescriptor descriptor; + descriptor.colorFormatsCount = testCase.formats.size(); + for (size_t i = 0; i < testCase.formats.size(); i++) { + descriptor.cColorFormats[i] = testCase.formats.at(i); + } + if (testCase.success) { + device.CreateRenderBundleEncoder(&descriptor); + } else { + EXPECT_DEPRECATION_ERROR_OR_WARNING(device.CreateRenderBundleEncoder(&descriptor)); + } + } +} diff --git a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp index 87b292f325..ef401daf3e 100644 --- a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp @@ -13,7 +13,9 @@ // limitations under the License. #include +#include +#include "dawn/tests/unittests/validation/DeprecatedAPITests.h" #include "dawn/tests/unittests/validation/ValidationTest.h" #include "dawn/common/Constants.h" @@ -98,8 +100,7 @@ TEST_F(RenderPassDescriptorValidationTest, OneAttachment) { TEST_F(RenderPassDescriptorValidationTest, ColorAttachmentOutOfBounds) { std::array colorAttachments; for (uint32_t i = 0; i < colorAttachments.size(); i++) { - colorAttachments[i].view = - Create2DAttachment(device, 1, 1, wgpu::TextureFormat::RGBA8Unorm); + colorAttachments[i].view = Create2DAttachment(device, 1, 1, wgpu::TextureFormat::R8Unorm); colorAttachments[i].resolveTarget = nullptr; colorAttachments[i].clearValue = {0.0f, 0.0f, 0.0f, 0.0f}; colorAttachments[i].loadOp = wgpu::LoadOp::Clear; @@ -1400,6 +1401,74 @@ TEST_F(RenderPassDescriptorValidationTest, ValidateDepthStencilAllAspects) { } } +// Tests validation for per-pixel accounting for render targets. The tests currently assume that the +// default maxColorAttachmentBytesPerSample limit of 32 is used. +TEST_P(DeprecationTests, RenderPassColorAttachmentBytesPerSample) { + struct TestCase { + std::vector formats; + bool success; + }; + static std::vector kTestCases = { + // Simple 1 format cases. + + // R8Unorm take 1 byte and are aligned to 1 byte so we can have 8 (max). + {{wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, + wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, + wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm}, + true}, + // RGBA8Uint takes 4 bytes and are aligned to 1 byte so we can have 8 (max). + {{wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint, + wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint, + wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint, + wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint}, + true}, + // RGBA8Unorm takes 8 bytes (special case) and are aligned to 1 byte so only 4 allowed. + {{wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm}, + true}, + {{wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA8Unorm}, + false}, + // RGBA32Float takes 16 bytes and are aligned to 4 bytes so only 2 are allowed. + {{wgpu::TextureFormat::RGBA32Float, wgpu::TextureFormat::RGBA32Float}, true}, + {{wgpu::TextureFormat::RGBA32Float, wgpu::TextureFormat::RGBA32Float, + wgpu::TextureFormat::RGBA32Float}, + false}, + + // Different format alignment cases. + + // Alignment causes the first 1 byte R8Unorm to become 4 bytes. So even though 1+4+8+16+1 < + // 32, the 4 byte alignment requirement of R32Float makes the first R8Unorm become 4 and + // 4+4+8+16+1 > 32. Re-ordering this so the R8Unorm's are at the end, however is allowed: + // 4+8+16+1+1 < 32. + {{wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R32Float, + wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA32Float, + wgpu::TextureFormat::R8Unorm}, + false}, + {{wgpu::TextureFormat::R32Float, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA32Float, wgpu::TextureFormat::R8Unorm, + wgpu::TextureFormat::R8Unorm}, + true}, + }; + + for (const TestCase& testCase : kTestCases) { + std::vector colorAttachmentInfo; + for (size_t i = 0; i < testCase.formats.size(); i++) { + colorAttachmentInfo.push_back(Create2DAttachment(device, 1, 1, testCase.formats.at(i))); + } + utils::ComboRenderPassDescriptor descriptor(colorAttachmentInfo); + wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder(); + if (testCase.success) { + wgpu::RenderPassEncoder renderPassEncoder = commandEncoder.BeginRenderPass(&descriptor); + renderPassEncoder.End(); + commandEncoder.Finish(); + } else { + EXPECT_DEPRECATION_ERROR_OR_WARNING(commandEncoder.BeginRenderPass(&descriptor)); + } + } +} + // TODO(cwallez@chromium.org): Constraints on attachment aliasing? } // anonymous namespace diff --git a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp index 7e8a19f4e8..66fe44aefd 100644 --- a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp @@ -18,6 +18,7 @@ #include #include "dawn/common/Constants.h" +#include "dawn/tests/unittests/validation/DeprecatedAPITests.h" #include "dawn/tests/unittests/validation/ValidationTest.h" #include "dawn/utils/ComboRenderPipelineDescriptor.h" #include "dawn/utils/WGPUHelpers.h" @@ -750,7 +751,7 @@ TEST_F(RenderPipelineValidationTest, VertexOnlyPipelineRequireDepthStencilAttach // Vertex-only render pipeline must have a depth stencil attachment { - utils::ComboRenderPassDescriptor renderPassDescriptor({}); + utils::ComboRenderPassDescriptor renderPassDescriptor; wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); ASSERT_DEVICE_ERROR(encoder.BeginRenderPass(&renderPassDescriptor)); @@ -1380,6 +1381,103 @@ TEST_F(RenderPipelineValidationTest, BindingsFromCorrectEntryPoint) { ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor)); } +// Tests validation for per-pixel accounting for render targets. The tests currently assume that the +// default maxColorAttachmentBytesPerSample limit of 32 is used. +TEST_P(DeprecationTests, RenderPipelineColorAttachmentBytesPerSample) { + // Creates a fragment shader with maximum number of color attachments to enable testing. + auto CreateShader = [&](const std::vector& formats) -> wgpu::ShaderModule { + // Default type to use when formats.size() < kMaxColorAttachments. + static constexpr std::string_view kDefaultWgslType = "vec4"; + + std::ostringstream bindings; + std::ostringstream outputs; + for (size_t i = 0; i < kMaxColorAttachments; i++) { + if (i < formats.size()) { + std::ostringstream type; + type << "vec4<" << utils::GetWGSLColorTextureComponentType(formats.at(i)) << ">"; + bindings << "@location(" << i << ") o" << i << " : " << type.str() << ", "; + outputs << type.str() << "(1), "; + } else { + bindings << "@location(" << i << ") o" << i << " : " << kDefaultWgslType << ", "; + outputs << kDefaultWgslType << "(1), "; + } + } + + std::ostringstream fsShader; + fsShader << "struct Outputs { " << bindings.str() << "}\n"; + fsShader << "@fragment fn main() -> Outputs {\n"; + fsShader << " return Outputs(" << outputs.str() << ");\n"; + fsShader << "}"; + return utils::CreateShaderModule(device, fsShader.str().c_str()); + }; + + struct TestCase { + std::vector formats; + bool success; + }; + static std::vector kTestCases = { + // Simple 1 format cases. + + // R8Unorm take 1 byte and are aligned to 1 byte so we can have 8 (max). + {{wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, + wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm, + wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R8Unorm}, + true}, + // RGBA8Uint takes 4 bytes and are aligned to 1 byte so we can have 8 (max). + {{wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint, + wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint, + wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint, + wgpu::TextureFormat::RGBA8Uint, wgpu::TextureFormat::RGBA8Uint}, + true}, + // RGBA8Unorm takes 8 bytes (special case) and are aligned to 1 byte so only 4 allowed. + {{wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm}, + true}, + {{wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA8Unorm}, + false}, + // RGBA32Float takes 16 bytes and are aligned to 4 bytes so only 2 are allowed. + {{wgpu::TextureFormat::RGBA32Float, wgpu::TextureFormat::RGBA32Float}, true}, + {{wgpu::TextureFormat::RGBA32Float, wgpu::TextureFormat::RGBA32Float, + wgpu::TextureFormat::RGBA32Float}, + false}, + + // Different format alignment cases. + + // Alignment causes the first 1 byte R8Unorm to become 4 bytes. So even though 1+4+8+16+1 < + // 32, the 4 byte alignment requirement of R32Float makes the first R8Unorm become 4 and + // 4+4+8+16+1 > 32. Re-ordering this so the R8Unorm's are at the end, however is allowed: + // 4+8+16+1+1 < 32. + {{wgpu::TextureFormat::R8Unorm, wgpu::TextureFormat::R32Float, + wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureFormat::RGBA32Float, + wgpu::TextureFormat::R8Unorm}, + false}, + {{wgpu::TextureFormat::R32Float, wgpu::TextureFormat::RGBA8Unorm, + wgpu::TextureFormat::RGBA32Float, wgpu::TextureFormat::R8Unorm, + wgpu::TextureFormat::R8Unorm}, + true}, + }; + + for (const TestCase& testCase : kTestCases) { + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = utils::CreateShaderModule(device, R"( + @vertex fn main() -> @builtin(position) vec4 { + return vec4(0.0, 0.0, 0.0, 1.0); + })"); + descriptor.cFragment.module = CreateShader(testCase.formats); + descriptor.cFragment.targetCount = testCase.formats.size(); + for (size_t i = 0; i < testCase.formats.size(); i++) { + descriptor.cTargets[i].format = testCase.formats.at(i); + } + if (testCase.success) { + device.CreateRenderPipeline(&descriptor); + } else { + EXPECT_DEPRECATION_ERROR_OR_WARNING(device.CreateRenderPipeline(&descriptor)); + } + } +} + class DepthClipControlValidationTest : public RenderPipelineValidationTest { protected: WGPUDevice CreateTestDevice(dawn::native::Adapter dawnAdapter) override { diff --git a/src/dawn/utils/WGPUHelpers.cpp b/src/dawn/utils/WGPUHelpers.cpp index d016620d1b..8cdf718959 100644 --- a/src/dawn/utils/WGPUHelpers.cpp +++ b/src/dawn/utils/WGPUHelpers.cpp @@ -96,7 +96,7 @@ wgpu::Buffer CreateBufferFromData(const wgpu::Device& device, } ComboRenderPassDescriptor::ComboRenderPassDescriptor( - std::initializer_list colorAttachmentInfo, + const std::vector& colorAttachmentInfo, wgpu::TextureView depthStencil) { for (uint32_t i = 0; i < kMaxColorAttachments; ++i) { cColorAttachments[i].loadOp = wgpu::LoadOp::Clear; @@ -174,7 +174,7 @@ BasicRenderPass::BasicRenderPass() height(0), color(nullptr), colorFormat(wgpu::TextureFormat::RGBA8Unorm), - renderPassInfo({}) {} + renderPassInfo() {} BasicRenderPass::BasicRenderPass(uint32_t texWidth, uint32_t texHeight, diff --git a/src/dawn/utils/WGPUHelpers.h b/src/dawn/utils/WGPUHelpers.h index 9217ca9216..b33f9d7cd0 100644 --- a/src/dawn/utils/WGPUHelpers.h +++ b/src/dawn/utils/WGPUHelpers.h @@ -57,7 +57,7 @@ wgpu::TextureDataLayout CreateTextureDataLayout(uint64_t offset, struct ComboRenderPassDescriptor : public wgpu::RenderPassDescriptor { public: - ComboRenderPassDescriptor(std::initializer_list colorAttachmentInfo, + ComboRenderPassDescriptor(const std::vector& colorAttachmentInfo = {}, wgpu::TextureView depthStencil = wgpu::TextureView()); ~ComboRenderPassDescriptor();