From cf714a81c86e56827563e20c1b2750e674aec322 Mon Sep 17 00:00:00 2001 From: Jiawei Shao Date: Wed, 28 Nov 2018 02:44:17 +0000 Subject: [PATCH] Add checks on the texture view used as color or depth stencil attachment This patch adds checks when we set a texture view as color or depth stencil attachments: 1. The mipmap level of the texture view must be 1 as it is limited in D3D12, Metal and Vulkan. 2. The array layer count of the texture view must be 1 as currently we do not plan to support layered rendering in WebGPU. 3. The format of the texture view must be color renderable when it is used as a color attachment. 4. The format of the texture view must be a depth stencil format when it is used as a depth stencil attachment. BUG=dawn:16 TEST=dawn_unittests Change-Id: Ibce3bda20d49a725c26796aa627c5067532761ad Reviewed-on: https://dawn-review.googlesource.com/c/2661 Commit-Queue: Jiawei Shao Reviewed-by: Kai Ninomiya --- src/dawn_native/RenderPassDescriptor.cpp | 37 ++- src/dawn_native/RenderPassDescriptor.h | 2 + src/dawn_native/Texture.cpp | 37 ++- src/dawn_native/Texture.h | 9 + .../RenderPassDescriptorValidationTests.cpp | 233 +++++++++++++++++- 5 files changed, 307 insertions(+), 11 deletions(-) diff --git a/src/dawn_native/RenderPassDescriptor.cpp b/src/dawn_native/RenderPassDescriptor.cpp index 6147d24f89..1426095021 100644 --- a/src/dawn_native/RenderPassDescriptor.cpp +++ b/src/dawn_native/RenderPassDescriptor.cpp @@ -83,6 +83,24 @@ namespace dawn_native { RenderPassDescriptorBuilder::RenderPassDescriptorBuilder(DeviceBase* device) : Builder(device) { } + bool RenderPassDescriptorBuilder::CheckArrayLayersAndLevelCountForAttachment( + const TextureViewBase* textureView) { + // Currently we do not support layered rendering. + if (textureView->GetLayerCount() > 1) { + HandleError( + "The layer count of the texture view used as attachment cannot be greater than 1"); + return false; + } + + if (textureView->GetLevelCount() > 1) { + HandleError( + "The mipmap level count of the texture view used as attachment cannot be greater " + "than 1"); + return false; + } + return true; + } + RenderPassDescriptorBase* RenderPassDescriptorBuilder::GetResultImpl() { auto CheckOrSetSize = [this](const TextureViewBase* attachment) -> bool { if (this->mWidth == 0) { @@ -133,8 +151,13 @@ namespace dawn_native { return; } - if (TextureFormatHasDepthOrStencil(textureView->GetTexture()->GetFormat())) { - HandleError("Using depth stencil texture as color attachment"); + if (!IsColorRenderableTextureFormat(textureView->GetFormat())) { + HandleError( + "The format of the texture view used as color attachment is not color renderable"); + return; + } + + if (!CheckArrayLayersAndLevelCountForAttachment(textureView)) { return; } @@ -162,8 +185,14 @@ namespace dawn_native { void RenderPassDescriptorBuilder::SetDepthStencilAttachment(TextureViewBase* textureView, dawn::LoadOp depthLoadOp, dawn::LoadOp stencilLoadOp) { - if (!TextureFormatHasDepthOrStencil(textureView->GetTexture()->GetFormat())) { - HandleError("Using color texture as depth stencil attachment"); + if (!TextureFormatHasDepthOrStencil(textureView->GetFormat())) { + HandleError( + "The format of the texture view used as depth stencil attachment is not a depth " + "stencil format"); + return; + } + + if (!CheckArrayLayersAndLevelCountForAttachment(textureView)) { return; } diff --git a/src/dawn_native/RenderPassDescriptor.h b/src/dawn_native/RenderPassDescriptor.h index 4079a15253..9ee4cd0263 100644 --- a/src/dawn_native/RenderPassDescriptor.h +++ b/src/dawn_native/RenderPassDescriptor.h @@ -94,6 +94,8 @@ namespace dawn_native { private: friend class RenderPassDescriptorBase; + bool CheckArrayLayersAndLevelCountForAttachment(const TextureViewBase* textureView); + std::bitset mColorAttachmentsSet; std::array mColorAttachments; diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp index 0baedf2377..0923fccb79 100644 --- a/src/dawn_native/Texture.cpp +++ b/src/dawn_native/Texture.cpp @@ -230,6 +230,26 @@ namespace dawn_native { } } + bool IsColorRenderableTextureFormat(dawn::TextureFormat format) { + switch (format) { + case dawn::TextureFormat::B8G8R8A8Unorm: + case dawn::TextureFormat::R8G8B8A8Uint: + case dawn::TextureFormat::R8G8B8A8Unorm: + case dawn::TextureFormat::R8G8Uint: + case dawn::TextureFormat::R8G8Unorm: + case dawn::TextureFormat::R8Uint: + case dawn::TextureFormat::R8Unorm: + return true; + + case dawn::TextureFormat::D32FloatS8Uint: + return false; + + default: + UNREACHABLE(); + return false; + } + } + // TextureBase TextureBase::TextureBase(DeviceBase* device, const TextureDescriptor* descriptor) @@ -277,7 +297,11 @@ namespace dawn_native { // TextureViewBase TextureViewBase::TextureViewBase(TextureBase* texture, const TextureViewDescriptor* descriptor) - : ObjectBase(texture->GetDevice()), mTexture(texture) { + : ObjectBase(texture->GetDevice()), + mTexture(texture), + mFormat(descriptor->format), + mLevelCount(descriptor->levelCount), + mLayerCount(descriptor->layerCount) { } const TextureBase* TextureViewBase::GetTexture() const { @@ -288,4 +312,15 @@ namespace dawn_native { return mTexture.Get(); } + dawn::TextureFormat TextureViewBase::GetFormat() const { + return mFormat; + } + + uint32_t TextureViewBase::GetLevelCount() const { + return mLevelCount; + } + + uint32_t TextureViewBase::GetLayerCount() const { + return mLayerCount; + } } // namespace dawn_native diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h index ab34e96315..f1194f05b2 100644 --- a/src/dawn_native/Texture.h +++ b/src/dawn_native/Texture.h @@ -32,6 +32,7 @@ namespace dawn_native { bool TextureFormatHasDepth(dawn::TextureFormat format); bool TextureFormatHasStencil(dawn::TextureFormat format); bool TextureFormatHasDepthOrStencil(dawn::TextureFormat format); + bool IsColorRenderableTextureFormat(dawn::TextureFormat format); static constexpr dawn::TextureUsageBit kReadOnlyTextureUsages = dawn::TextureUsageBit::TransferSrc | dawn::TextureUsageBit::Sampled | @@ -74,8 +75,16 @@ namespace dawn_native { const TextureBase* GetTexture() const; TextureBase* GetTexture(); + dawn::TextureFormat GetFormat() const; + uint32_t GetLevelCount() const; + uint32_t GetLayerCount() const; + private: Ref mTexture; + + dawn::TextureFormat mFormat; + uint32_t mLevelCount; + uint32_t mLayerCount; }; } // namespace dawn_native diff --git a/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp b/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp index 63be562473..e98d1c025b 100644 --- a/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp +++ b/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp @@ -21,19 +21,33 @@ namespace { class RenderPassDescriptorValidationTest : public ValidationTest { }; -dawn::TextureView Create2DAttachment(dawn::Device& device, uint32_t width, uint32_t height, dawn::TextureFormat format) { +dawn::Texture CreateTexture(dawn::Device& device, + dawn::TextureDimension dimension, + dawn::TextureFormat format, + uint32_t width, + uint32_t height, + uint32_t arrayLayer, + uint32_t levelCount) { dawn::TextureDescriptor descriptor; - descriptor.dimension = dawn::TextureDimension::e2D; + descriptor.dimension = dimension; descriptor.size.width = width; descriptor.size.height = height; descriptor.size.depth = 1; - descriptor.arrayLayer = 1; + descriptor.arrayLayer = arrayLayer; descriptor.format = format; - descriptor.levelCount = 1; + descriptor.levelCount = levelCount; descriptor.usage = dawn::TextureUsageBit::OutputAttachment; - dawn::Texture attachment = device.CreateTexture(&descriptor); - return attachment.CreateDefaultTextureView(); + return device.CreateTexture(&descriptor); +} + +dawn::TextureView Create2DAttachment(dawn::Device& device, + uint32_t width, + uint32_t height, + dawn::TextureFormat format) { + dawn::Texture texture = CreateTexture( + device, dawn::TextureDimension::e2D, format, width, height, 1, 1); + return texture.CreateDefaultTextureView(); } // A render pass with no attachments isn't valid @@ -177,6 +191,213 @@ TEST_F(RenderPassDescriptorValidationTest, FormatMismatch) { } } +// Currently only texture views with layerCount == 1 are allowed to be color and depth stencil +// attachments +TEST_F(RenderPassDescriptorValidationTest, TextureViewLayerCountForColorAndDepthStencil) { + constexpr uint32_t kLevelCount = 1; + constexpr uint32_t kSize = 32; + constexpr dawn::TextureFormat kColorFormat = dawn::TextureFormat::R8G8B8A8Unorm; + constexpr dawn::TextureFormat kDepthStencilFormat = dawn::TextureFormat::D32FloatS8Uint; + + constexpr uint32_t kArrayLayers = 10; + + dawn::Texture colorTexture = CreateTexture( + device, dawn::TextureDimension::e2D, kColorFormat, kSize, kSize, kArrayLayers, kLevelCount); + dawn::Texture depthStencilTexture = CreateTexture( + device, dawn::TextureDimension::e2D, kDepthStencilFormat, kSize, kSize, kArrayLayers, + kLevelCount); + + dawn::TextureViewDescriptor baseDescriptor; + baseDescriptor.dimension = dawn::TextureViewDimension::e2DArray; + baseDescriptor.baseArrayLayer = 0; + baseDescriptor.layerCount = kArrayLayers; + baseDescriptor.baseMipLevel = 0; + baseDescriptor.levelCount = kLevelCount; + + // Using 2D array texture view with layerCount > 1 is not allowed for color + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kColorFormat; + descriptor.layerCount = 5; + + dawn::TextureView colorTextureView = colorTexture.CreateTextureView(&descriptor); + AssertWillBeError(device.CreateRenderPassDescriptorBuilder()) + .SetColorAttachment(0, colorTextureView, dawn::LoadOp::Clear) + .GetResult(); + } + + // Using 2D array texture view with layerCount > 1 is not allowed for depth stencil + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kDepthStencilFormat; + descriptor.layerCount = 5; + + dawn::TextureView depthStencilView = depthStencilTexture.CreateTextureView(&descriptor); + AssertWillBeError(device.CreateRenderPassDescriptorBuilder()) + .SetDepthStencilAttachment(depthStencilView, dawn::LoadOp::Clear, dawn::LoadOp::Clear) + .GetResult(); + } + + // Using 2D array texture view that covers the first layer of the texture is OK for color + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kColorFormat; + descriptor.baseArrayLayer = 0; + descriptor.layerCount = 1; + + dawn::TextureView colorTextureView = colorTexture.CreateTextureView(&descriptor); + AssertWillBeSuccess(device.CreateRenderPassDescriptorBuilder()) + .SetColorAttachment(0, colorTextureView, dawn::LoadOp::Clear) + .GetResult(); + } + + // Using 2D array texture view that covers the first layer is OK for depth stencil + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kDepthStencilFormat; + descriptor.baseArrayLayer = 0; + descriptor.layerCount = 1; + + dawn::TextureView depthStencilTextureView = + depthStencilTexture.CreateTextureView(&descriptor); + AssertWillBeSuccess(device.CreateRenderPassDescriptorBuilder()) + .SetDepthStencilAttachment( + depthStencilTextureView, dawn::LoadOp::Clear, dawn::LoadOp::Clear) + .GetResult(); + } + + // Using 2D array texture view that covers the last layer is OK for color + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kColorFormat; + descriptor.baseArrayLayer = kArrayLayers - 1; + descriptor.layerCount = 1; + + dawn::TextureView colorTextureView = colorTexture.CreateTextureView(&descriptor); + AssertWillBeSuccess(device.CreateRenderPassDescriptorBuilder()) + .SetColorAttachment(0, colorTextureView, dawn::LoadOp::Clear) + .GetResult(); + } + + // Using 2D array texture view that covers the last layer is OK for depth stencil + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kDepthStencilFormat; + descriptor.baseArrayLayer = kArrayLayers - 1; + descriptor.layerCount = 1; + + dawn::TextureView depthStencilTextureView = + depthStencilTexture.CreateTextureView(&descriptor); + AssertWillBeSuccess(device.CreateRenderPassDescriptorBuilder()) + .SetDepthStencilAttachment( + depthStencilTextureView, dawn::LoadOp::Clear, dawn::LoadOp::Clear) + .GetResult(); + } +} + +// Only 2D texture views with levelCount == 1 are allowed to be color attachments +TEST_F(RenderPassDescriptorValidationTest, TextureViewLevelCountForColorAndDepthStencil) { + constexpr uint32_t kArrayLayers = 1; + constexpr uint32_t kSize = 32; + constexpr dawn::TextureFormat kColorFormat = dawn::TextureFormat::R8G8B8A8Unorm; + constexpr dawn::TextureFormat kDepthStencilFormat = dawn::TextureFormat::D32FloatS8Uint; + + constexpr uint32_t kLevelCount = 4; + + dawn::Texture colorTexture = CreateTexture( + device, dawn::TextureDimension::e2D, kColorFormat, kSize, kSize, kArrayLayers, kLevelCount); + dawn::Texture depthStencilTexture = CreateTexture( + device, dawn::TextureDimension::e2D, kDepthStencilFormat, kSize, kSize, kArrayLayers, + kLevelCount); + + dawn::TextureViewDescriptor baseDescriptor; + baseDescriptor.dimension = dawn::TextureViewDimension::e2D; + baseDescriptor.baseArrayLayer = 0; + baseDescriptor.layerCount = kArrayLayers; + baseDescriptor.baseMipLevel = 0; + baseDescriptor.levelCount = kLevelCount; + + // Using 2D texture view with levelCount > 1 is not allowed for color + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kColorFormat; + descriptor.levelCount = 2; + + dawn::TextureView colorTextureView = colorTexture.CreateTextureView(&descriptor); + AssertWillBeError(device.CreateRenderPassDescriptorBuilder()) + .SetColorAttachment(0, colorTextureView, dawn::LoadOp::Clear) + .GetResult(); + } + + // Using 2D texture view with levelCount > 1 is not allowed for depth stencil + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kDepthStencilFormat; + descriptor.levelCount = 2; + + dawn::TextureView depthStencilView = depthStencilTexture.CreateTextureView(&descriptor); + AssertWillBeError(device.CreateRenderPassDescriptorBuilder()) + .SetDepthStencilAttachment(depthStencilView, dawn::LoadOp::Clear, dawn::LoadOp::Clear) + .GetResult(); + } + + // Using 2D texture view that covers the first level of the texture is OK for color + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kColorFormat; + descriptor.baseMipLevel = 0; + descriptor.levelCount = 1; + + dawn::TextureView colorTextureView = colorTexture.CreateTextureView(&descriptor); + AssertWillBeSuccess(device.CreateRenderPassDescriptorBuilder()) + .SetColorAttachment(0, colorTextureView, dawn::LoadOp::Clear) + .GetResult(); + } + + // Using 2D texture view that covers the first level is OK for depth stencil + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kDepthStencilFormat; + descriptor.baseMipLevel = 0; + descriptor.levelCount = 1; + + dawn::TextureView depthStencilTextureView = + depthStencilTexture.CreateTextureView(&descriptor); + AssertWillBeSuccess(device.CreateRenderPassDescriptorBuilder()) + .SetDepthStencilAttachment( + depthStencilTextureView, dawn::LoadOp::Clear, dawn::LoadOp::Clear) + .GetResult(); + } + + // Using 2D texture view that covers the last level is OK for color + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kColorFormat; + descriptor.baseMipLevel = kLevelCount - 1; + descriptor.levelCount = 1; + + dawn::TextureView colorTextureView = colorTexture.CreateTextureView(&descriptor); + AssertWillBeSuccess(device.CreateRenderPassDescriptorBuilder()) + .SetColorAttachment(0, colorTextureView, dawn::LoadOp::Clear) + .GetResult(); + } + + // Using 2D texture view that covers the last level is OK for depth stencil + { + dawn::TextureViewDescriptor descriptor = baseDescriptor; + descriptor.format = kDepthStencilFormat; + descriptor.baseMipLevel = kLevelCount - 1; + descriptor.levelCount = 1; + + dawn::TextureView depthStencilTextureView = + depthStencilTexture.CreateTextureView(&descriptor); + AssertWillBeSuccess(device.CreateRenderPassDescriptorBuilder()) + .SetDepthStencilAttachment( + depthStencilTextureView, dawn::LoadOp::Clear, dawn::LoadOp::Clear) + .GetResult(); + } +} + // TODO(cwallez@chromium.org): Constraints on attachment aliasing? } // anonymous namespace