Validate depthSlice for a 3D texture used in multiple color attachments
- Make depthSlice as an optional property in RenderPassColorAttachment
and must be set for 3D attachments.
- depthSlice has been landed into the spec. We can remove the dawn only
tag.
- Add validation for attachments' same subresource used multiple times
in a render pass.
Bug: dawn:1020
Change-Id: Ibb5f66188dba4096f9186e53462bb36c300b1f56
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/155884
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Hao Li <hao.x.li@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/dawn.json b/dawn.json
index 4a51c93..7a80ae8 100644
--- a/dawn.json
+++ b/dawn.json
@@ -2545,7 +2545,7 @@
"extensible": "in",
"members": [
{"name": "view", "type": "texture view", "optional": true},
- {"name": "depth slice", "type": "uint32_t", "default": "0", "tags": ["dawn"]},
+ {"name": "depth slice", "type": "uint32_t", "default": "WGPU_DEPTH_SLICE_UNDEFINED"},
{"name": "resolve target", "type": "texture view", "optional": true},
{"name": "load op", "type": "load op"},
{"name": "store op", "type": "store op"},
@@ -3675,6 +3675,11 @@
"type": "uint32_t",
"value": "UINT32_MAX"
},
+ "depth slice undefined" : {
+ "category": "constant",
+ "type": "uint32_t",
+ "value": "(0xffffffffUL)"
+ },
"query set index undefined" : {
"category": "constant",
"type": "uint32_t",
diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp
index 128bbea..0bd5859 100644
--- a/src/dawn/native/CommandEncoder.cpp
+++ b/src/dawn/native/CommandEncoder.cpp
@@ -27,6 +27,7 @@
#include "dawn/native/CommandEncoder.h"
+#include <string_view>
#include <unordered_set>
#include <utility>
#include <vector>
@@ -62,6 +63,159 @@
namespace {
+// Record the subresource range of a attachment used in render pass for checking overlaps.
+struct RecordedAttachment {
+ const TextureBase* texture;
+ uint32_t mipLevel;
+ // For 3d color attachment, it's the attachment's depthSlice.
+ uint32_t depthOrArrayLayer;
+
+ bool operator==(const RecordedAttachment& other) const {
+ return ((other.texture == texture) && (other.mipLevel == mipLevel) &&
+ (other.depthOrArrayLayer == depthOrArrayLayer));
+ }
+};
+
+enum class AttachmentType : uint8_t {
+ ColorAttachment,
+ ResolveTarget,
+ DepthStencilAttachment,
+ StorageAttachment
+};
+
+std::string_view GetAttachmentTypeStr(AttachmentType type) {
+ switch (type) {
+ case AttachmentType::ColorAttachment:
+ return "color attachment";
+ case AttachmentType::ResolveTarget:
+ return "resolve target";
+ case AttachmentType::DepthStencilAttachment:
+ return "depth stencil attachment";
+ case AttachmentType::StorageAttachment:
+ return "storage attachment";
+ }
+ DAWN_UNREACHABLE();
+}
+
+// The width, height, sample count and subresource range need to be validated between all
+// attachments in the render pass. This class records all the states and validate them for each
+// attachment.
+class RenderPassValidationState final : public NonMovable {
+ public:
+ RenderPassValidationState() = default;
+ ~RenderPassValidationState() = default;
+
+ // Record the attachment in the render pass if it passes all validations:
+ // - the attachment has same with, height and sample count with other attachments
+ // - no overlaps with other attachments
+ // TODO(dawn:1020): Improve the error messages to include the index information of the
+ // attachment in the render pass descriptor.
+ MaybeError AddAttachment(const TextureViewBase* attachment,
+ AttachmentType attachmentType,
+ uint32_t depthSlice = WGPU_DEPTH_SLICE_UNDEFINED) {
+ if (attachment == nullptr) {
+ return {};
+ }
+
+ DAWN_ASSERT(attachment->GetLevelCount() == 1);
+ DAWN_ASSERT(attachment->GetLayerCount() == 1);
+
+ const std::string_view attachmentTypeStr = GetAttachmentTypeStr(attachmentType);
+
+ std::string_view implicitPrefixStr;
+ // Not need to validate the implicit sample count for the depth stencil attachment.
+ if (mImplicitSampleCount > 1 && attachmentType != AttachmentType::DepthStencilAttachment) {
+ DAWN_INVALID_IF(attachment->GetTexture()->GetSampleCount() != 1,
+ "The %s %s sample count (%u) is not 1 when it has implicit "
+ "sample count (%u).",
+ attachmentTypeStr, attachment,
+ attachment->GetTexture()->GetSampleCount(), mImplicitSampleCount);
+
+ implicitPrefixStr = "implicit ";
+ }
+
+ Extent3D attachmentSize = attachment->GetSingleSubresourceVirtualSize();
+
+ if (HasAttachment()) {
+ DAWN_INVALID_IF(attachmentSize.width != mWidth || attachmentSize.height != mHeight,
+ "The %s %s size (width: %u, height: %u) does not match the size of the "
+ "other attachments (width: %u, height: %u).",
+ attachmentTypeStr, attachment, attachmentSize.width,
+ attachmentSize.height, mWidth, mHeight);
+
+ // Skip the sampleCount validation for resolve target
+ DAWN_INVALID_IF(attachmentType != AttachmentType::ResolveTarget &&
+ attachment->GetTexture()->GetSampleCount() != mSampleCount,
+ "The %s %s %ssample count (%u) does not match the sample count of the "
+ "other attachments (%u).",
+ attachmentTypeStr, attachment, implicitPrefixStr,
+ attachment->GetTexture()->GetSampleCount(), mSampleCount);
+ } else {
+ mWidth = attachmentSize.width;
+ mHeight = attachmentSize.height;
+ mSampleCount = mImplicitSampleCount > 1 ? mImplicitSampleCount
+ : attachment->GetTexture()->GetSampleCount();
+ DAWN_ASSERT(mWidth != 0);
+ DAWN_ASSERT(mHeight != 0);
+ DAWN_ASSERT(mSampleCount != 0);
+ }
+
+ RecordedAttachment record;
+ record.texture = attachment->GetTexture();
+ record.mipLevel = attachment->GetBaseMipLevel();
+ if (attachment->GetDimension() == wgpu::TextureViewDimension::e3D) {
+ DAWN_ASSERT(attachment->GetBaseArrayLayer() == 0);
+ record.depthOrArrayLayer = depthSlice;
+ } else {
+ // TODO(dawn:1020): Assert depthSlice should be the 'WGPU_DEPTH_SLICE_UNDEFINED' value
+ // if the attachment is a non-3d texture view after it's initialized correctly in Blink.
+ // DAWN_ASSERT(depthSlice == WGPU_DEPTH_SLICE_UNDEFINED);
+ record.depthOrArrayLayer = attachment->GetBaseArrayLayer();
+ }
+
+ for (size_t i = 0; i < mRecords->size(); i++) {
+ DAWN_INVALID_IF(
+ mRecords[i] == record,
+ "The %s %s has read-write or write-write conflict with another attachment.",
+ attachmentTypeStr, attachment);
+ }
+
+ mRecords->push_back(record);
+
+ return {};
+ }
+
+ bool HasAttachment() const { return mRecords->size() != 0; }
+
+ bool IsValidState() const {
+ return ((mWidth > 0) && (mHeight > 0) && (mSampleCount > 0) &&
+ (mImplicitSampleCount == 0 || mImplicitSampleCount == mSampleCount));
+ }
+
+ uint32_t GetWidth() const { return mWidth; }
+
+ uint32_t GetHeight() const { return mHeight; }
+
+ uint32_t GetSampleCount() const { return mSampleCount; }
+
+ uint32_t GetImplicitSampleCount() const { return mImplicitSampleCount; }
+
+ void SetImplicitSampleCount(uint32_t implicitSampleCount) {
+ mImplicitSampleCount = implicitSampleCount;
+ }
+
+ private:
+ // The attachment's width, height and sample count.
+ uint32_t mWidth = 0;
+ uint32_t mHeight = 0;
+ uint32_t mSampleCount = 0;
+ // The implicit multisample count used by MSAA render to single sampled.
+ uint32_t mImplicitSampleCount = 0;
+
+ // The records of the attachments that were validated in render pass.
+ StackVector<RecordedAttachment, kMaxColorAttachments> mRecords;
+};
+
MaybeError ValidateB2BCopyAlignment(uint64_t dataSize, uint64_t srcOffset, uint64_t dstOffset) {
// Copy size must be a multiple of 4 bytes on macOS.
DAWN_INVALID_IF(dataSize % 4 != 0, "Copy size (%u) is not a multiple of 4.", dataSize);
@@ -154,58 +308,6 @@
return {};
}
-MaybeError ValidateOrSetAttachmentSize(const TextureViewBase* attachment,
- uint32_t* width,
- uint32_t* height) {
- Extent3D attachmentSize = attachment->GetSingleSubresourceVirtualSize();
-
- if (*width == 0) {
- DAWN_ASSERT(*height == 0);
- *width = attachmentSize.width;
- *height = attachmentSize.height;
- DAWN_ASSERT(*width != 0 && *height != 0);
- } else {
- DAWN_INVALID_IF(*width != attachmentSize.width || *height != attachmentSize.height,
- "Attachment %s size (width: %u, height: %u) does not match the size of the "
- "other attachments (width: %u, height: %u).",
- attachment, attachmentSize.width, attachmentSize.height, *width, *height);
- }
-
- return {};
-}
-
-MaybeError ValidateOrSetColorAttachmentSampleCount(const TextureViewBase* colorAttachment,
- uint32_t implicitSampleCount,
- uint32_t* sampleCount) {
- uint32_t attachmentSampleCount = 0;
- std::string implicitPrefixStr;
- if (implicitSampleCount > 1) {
- DAWN_INVALID_IF(colorAttachment->GetTexture()->GetSampleCount() != 1,
- "Color attachment %s sample count (%u) is not 1 when it has implicit "
- "sample count (%u).",
- colorAttachment, colorAttachment->GetTexture()->GetSampleCount(),
- implicitSampleCount);
-
- attachmentSampleCount = implicitSampleCount;
- implicitPrefixStr = "implicit ";
- } else {
- attachmentSampleCount = colorAttachment->GetTexture()->GetSampleCount();
- }
-
- if (*sampleCount == 0) {
- *sampleCount = attachmentSampleCount;
- DAWN_ASSERT(*sampleCount != 0);
- } else {
- DAWN_INVALID_IF(
- *sampleCount != attachmentSampleCount,
- "Color attachment %s %ssample count (%u) does not match the sample count of the "
- "other attachments (%u).",
- colorAttachment, implicitPrefixStr, attachmentSampleCount, *sampleCount);
- }
-
- return {};
-}
-
MaybeError ValidateResolveTarget(const DeviceBase* device,
const RenderPassColorAttachment& colorAttachment,
UsageValidationMode usageValidationMode) {
@@ -236,15 +338,6 @@
"The resolve target %s mip level count (%u) is not 1.", resolveTarget,
resolveTarget->GetLevelCount());
- const Extent3D& colorTextureSize = attachment->GetSingleSubresourceVirtualSize();
- const Extent3D& resolveTextureSize = resolveTarget->GetSingleSubresourceVirtualSize();
- DAWN_INVALID_IF(colorTextureSize.width != resolveTextureSize.width ||
- colorTextureSize.height != resolveTextureSize.height,
- "The Resolve target %s size (width: %u, height: %u) does not match the color "
- "attachment %s size (width: %u, height: %u).",
- resolveTarget, resolveTextureSize.width, resolveTextureSize.height, attachment,
- colorTextureSize.width, colorTextureSize.height);
-
wgpu::TextureFormat resolveTargetFormat = resolveTarget->GetFormat().format;
DAWN_INVALID_IF(
resolveTargetFormat != attachment->GetFormat().format,
@@ -261,20 +354,25 @@
MaybeError ValidateColorAttachmentDepthSlice(const TextureViewBase* attachment,
uint32_t depthSlice) {
- if (attachment->GetDimension() == wgpu::TextureViewDimension::e3D) {
- const Extent3D& attachmentSize = attachment->GetSingleSubresourceVirtualSize();
-
- DAWN_INVALID_IF(depthSlice >= attachmentSize.depthOrArrayLayers,
- "The depth slice index (%u) of 3D %s used as attachment is >= the "
- "depthOrArrayLayers (%u) of its subresource at mip level (%u).",
- depthSlice, attachment, attachmentSize.depthOrArrayLayers,
- attachment->GetBaseMipLevel());
- } else {
- DAWN_INVALID_IF(depthSlice != 0,
- "The depth slice index (%u) of non-3D %s used as attachment is not 0.",
- depthSlice, attachment);
+ if (attachment->GetDimension() != wgpu::TextureViewDimension::e3D) {
+ // TODO(dawn:1020): Validate depthSlice must not set for non-3d attachments. The depthSlice
+ // from WebGPU is not the 'WGPU_DEPTH_SLICE_UNDEFINED' value if it's not defined, we need to
+ // initialize it in Blink first, otherwise it will always be validated as set.
+ return {};
}
+ DAWN_INVALID_IF(depthSlice == WGPU_DEPTH_SLICE_UNDEFINED,
+ "depthSlice (%u) must be set and must not be undefined value (%u) for a 3D "
+ "attachment (%s).",
+ depthSlice, WGPU_DEPTH_SLICE_UNDEFINED, attachment);
+
+ const Extent3D& attachmentSize = attachment->GetSingleSubresourceVirtualSize();
+ DAWN_INVALID_IF(depthSlice >= attachmentSize.depthOrArrayLayers,
+ "depthSlice (%u) of the attachment (%s) is >= the "
+ "depthOrArrayLayers (%u) of the attachment's subresource at mip level (%u).",
+ depthSlice, attachment, attachmentSize.depthOrArrayLayers,
+ attachment->GetBaseMipLevel());
+
return {};
}
@@ -317,11 +415,8 @@
MaybeError ValidateRenderPassColorAttachment(DeviceBase* device,
const RenderPassColorAttachment& colorAttachment,
- uint32_t* width,
- uint32_t* height,
- uint32_t* sampleCount,
- uint32_t* implicitSampleCount,
- UsageValidationMode usageValidationMode) {
+ UsageValidationMode usageValidationMode,
+ RenderPassValidationState* validationState) {
TextureViewBase* attachment = colorAttachment.view;
if (attachment == nullptr) {
return {};
@@ -336,7 +431,7 @@
if (msaaRenderToSingleSampledDesc) {
DAWN_TRY(ValidateColorAttachmentRenderToSingleSampled(device, colorAttachment,
msaaRenderToSingleSampledDesc));
- *implicitSampleCount = msaaRenderToSingleSampledDesc->implicitSampleCount;
+ validationState->SetImplicitSampleCount(msaaRenderToSingleSampledDesc->implicitSampleCount);
// Note: we don't need to check whether the implicit sample count of different attachments
// are the same. That already is done by indirectly comparing the sample count in
// ValidateOrSetColorAttachmentSampleCount.
@@ -376,29 +471,30 @@
"Color clear value (%s) contains a NaN.", &clearValue);
}
- DAWN_TRY(
- ValidateOrSetColorAttachmentSampleCount(attachment, *implicitSampleCount, sampleCount));
+ DAWN_TRY(ValidateColorAttachmentDepthSlice(attachment, colorAttachment.depthSlice));
+ DAWN_TRY(ValidateAttachmentArrayLayersAndLevelCount(attachment));
- if (*implicitSampleCount <= 1) {
+ DAWN_TRY(validationState->AddAttachment(attachment, AttachmentType::ColorAttachment,
+ colorAttachment.depthSlice));
+
+ if (validationState->GetImplicitSampleCount() <= 1) {
// This step is skipped if implicitSampleCount > 1, because in that case, there shoudn't be
// any explicit resolveTarget specified.
DAWN_TRY(ValidateResolveTarget(device, colorAttachment, usageValidationMode));
+ // Add resolve target after adding color attachment to make sure there is already a color
+ // attachment for the comparation of with and height.
+ DAWN_TRY(validationState->AddAttachment(colorAttachment.resolveTarget,
+ AttachmentType::ResolveTarget));
}
- DAWN_TRY(ValidateColorAttachmentDepthSlice(attachment, colorAttachment.depthSlice));
- DAWN_TRY(ValidateAttachmentArrayLayersAndLevelCount(attachment));
- DAWN_TRY(ValidateOrSetAttachmentSize(attachment, width, height));
-
return {};
}
MaybeError ValidateRenderPassDepthStencilAttachment(
DeviceBase* device,
const RenderPassDepthStencilAttachment* depthStencilAttachment,
- uint32_t* width,
- uint32_t* height,
- uint32_t* sampleCount,
- UsageValidationMode usageValidationMode) {
+ UsageValidationMode usageValidationMode,
+ RenderPassValidationState* validationState) {
DAWN_ASSERT(depthStencilAttachment != nullptr);
TextureViewBase* attachment = depthStencilAttachment->view;
@@ -489,33 +585,19 @@
depthStencilAttachment->depthClearValue, attachment);
}
- // *sampleCount == 0 must only happen when there is no color attachment. In that case we
- // do not need to validate the sample count of the depth stencil attachment.
- const uint32_t depthStencilSampleCount = attachment->GetTexture()->GetSampleCount();
- if (*sampleCount != 0) {
- DAWN_INVALID_IF(
- depthStencilSampleCount != *sampleCount,
- "The depth stencil attachment %s sample count (%u) does not match the sample "
- "count of the other attachments (%u).",
- attachment, depthStencilSampleCount, *sampleCount);
- } else {
- *sampleCount = depthStencilSampleCount;
- }
-
DAWN_TRY(ValidateAttachmentArrayLayersAndLevelCount(attachment));
- DAWN_TRY(ValidateOrSetAttachmentSize(attachment, width, height));
+
+ DAWN_TRY(validationState->AddAttachment(attachment, AttachmentType::DepthStencilAttachment));
return {};
}
MaybeError ValidateRenderPassPLS(DeviceBase* device,
const RenderPassPixelLocalStorage* pls,
- uint32_t* width,
- uint32_t* height,
- uint32_t* sampleCount,
- uint32_t implicitSampleCount,
- UsageValidationMode usageValidationMode) {
+ UsageValidationMode usageValidationMode,
+ RenderPassValidationState* validationState) {
StackVector<StorageAttachmentInfoForValidation, 4> attachments;
+
for (size_t i = 0; i < pls->storageAttachmentCount; i++) {
const RenderPassStorageAttachment& attachment = pls->storageAttachments[i];
@@ -524,9 +606,6 @@
DAWN_TRY(ValidateCanUseAs(attachment.storage->GetTexture(),
wgpu::TextureUsage::StorageAttachment, usageValidationMode));
DAWN_TRY(ValidateAttachmentArrayLayersAndLevelCount(attachment.storage));
- DAWN_TRY(ValidateOrSetColorAttachmentSampleCount(attachment.storage, implicitSampleCount,
- sampleCount));
- DAWN_TRY(ValidateOrSetAttachmentSize(attachment.storage, width, height));
// Validate the load/storeOp and the clearValue.
DAWN_TRY(ValidateLoadOp(attachment.loadOp));
@@ -544,6 +623,9 @@
&clearValue);
}
+ DAWN_TRY(
+ validationState->AddAttachment(attachment.storage, AttachmentType::StorageAttachment));
+
attachments->push_back({attachment.offset, attachment.storage->GetFormat().format});
}
@@ -553,11 +635,8 @@
MaybeError ValidateRenderPassDescriptor(DeviceBase* device,
const RenderPassDescriptor* descriptor,
- uint32_t* width,
- uint32_t* height,
- uint32_t* sampleCount,
- uint32_t* implicitSampleCount,
- UsageValidationMode usageValidationMode) {
+ UsageValidationMode usageValidationMode,
+ RenderPassValidationState* validationState) {
DAWN_TRY(
ValidateSTypes(descriptor->nextInChain, {{wgpu::SType::RenderPassDescriptorMaxDrawCount},
{wgpu::SType::RenderPassPixelLocalStorage}}));
@@ -568,15 +647,12 @@
"Color attachment count (%u) exceeds the maximum number of color attachments (%u).",
descriptor->colorAttachmentCount, maxColorAttachments);
- bool anyColorAttachment = false;
ColorAttachmentFormats colorAttachmentFormats;
for (uint32_t i = 0; i < descriptor->colorAttachmentCount; ++i) {
- DAWN_TRY_CONTEXT(ValidateRenderPassColorAttachment(
- device, descriptor->colorAttachments[i], width, height, sampleCount,
- implicitSampleCount, usageValidationMode),
+ DAWN_TRY_CONTEXT(ValidateRenderPassColorAttachment(device, descriptor->colorAttachments[i],
+ usageValidationMode, validationState),
"validating colorAttachments[%u].", i);
if (descriptor->colorAttachments[i].view) {
- anyColorAttachment = true;
colorAttachmentFormats->push_back(&descriptor->colorAttachments[i].view->GetFormat());
}
}
@@ -584,10 +660,10 @@
"validating color attachment bytes per sample.");
if (descriptor->depthStencilAttachment != nullptr) {
- DAWN_TRY_CONTEXT(ValidateRenderPassDepthStencilAttachment(
- device, descriptor->depthStencilAttachment, width, height, sampleCount,
- usageValidationMode),
- "validating depthStencilAttachment.");
+ DAWN_TRY_CONTEXT(
+ ValidateRenderPassDepthStencilAttachment(device, descriptor->depthStencilAttachment,
+ usageValidationMode, validationState),
+ "validating depthStencilAttachment.");
}
if (descriptor->occlusionQuerySet != nullptr) {
@@ -610,26 +686,21 @@
}
// Validation for any pixel local storage.
- size_t storageAttachmentCount = 0;
const RenderPassPixelLocalStorage* pls = nullptr;
FindInChain(descriptor->nextInChain, &pls);
if (pls != nullptr) {
- storageAttachmentCount = pls->storageAttachmentCount;
- DAWN_TRY(ValidateRenderPassPLS(device, pls, width, height, sampleCount,
- *implicitSampleCount, usageValidationMode));
+ DAWN_TRY(ValidateRenderPassPLS(device, pls, usageValidationMode, validationState));
}
- DAWN_INVALID_IF(!anyColorAttachment && descriptor->depthStencilAttachment == nullptr &&
- storageAttachmentCount == 0,
- "Render pass has no attachments.");
+ DAWN_INVALID_IF(!validationState->HasAttachment(), "Render pass has no attachments.");
- if (*implicitSampleCount > 1) {
+ if (validationState->GetImplicitSampleCount() > 1) {
// TODO(dawn:1710): support multiple attachments.
DAWN_INVALID_IF(
descriptor->colorAttachmentCount != 1,
"colorAttachmentCount (%u) is not supported when the render pass has implicit sample "
"count (%u). (Currently) colorAttachmentCount = 1 is supported.",
- descriptor->colorAttachmentCount, *implicitSampleCount);
+ descriptor->colorAttachmentCount, validationState->GetImplicitSampleCount());
// TODO(dawn:1704): Consider supporting MSAARenderToSingleSampled + PLS
DAWN_INVALID_IF(pls != nullptr,
"For now PLS is invalid to use with MSAARenderToSingleSampled.");
@@ -998,25 +1069,20 @@
RenderPassResourceUsageTracker usageTracker;
- uint32_t width = 0;
- uint32_t height = 0;
- uint32_t sampleCount = 0;
- // The implicit multisample count used by MSAA render to single sampled.
- uint32_t implicitSampleCount = 0;
bool depthReadOnly = false;
bool stencilReadOnly = false;
Ref<AttachmentState> attachmentState;
+ RenderPassValidationState validationState;
std::function<void()> passEndCallback = nullptr;
bool success = mEncodingContext.TryEncode(
this,
[&](CommandAllocator* allocator) -> MaybeError {
- DAWN_TRY(ValidateRenderPassDescriptor(device, descriptor, &width, &height, &sampleCount,
- &implicitSampleCount, mUsageValidationMode));
+ DAWN_TRY(ValidateRenderPassDescriptor(device, descriptor, mUsageValidationMode,
+ &validationState));
- DAWN_ASSERT(width > 0 && height > 0 && sampleCount > 0 &&
- (implicitSampleCount == 0 || implicitSampleCount == sampleCount));
+ DAWN_ASSERT(validationState.IsValidState());
mEncodingContext.WillBeginRenderPass();
BeginRenderPassCmd* cmd =
@@ -1032,7 +1098,7 @@
TextureViewBase* colorTarget;
TextureViewBase* resolveTarget;
- if (implicitSampleCount <= 1) {
+ if (validationState.GetImplicitSampleCount() <= 1) {
colorTarget = descriptor->colorAttachments[i].view;
resolveTarget = descriptor->colorAttachments[i].resolveTarget;
@@ -1048,10 +1114,11 @@
Ref<TextureViewBase> implicitMSAATargetRef;
DAWN_TRY_ASSIGN(implicitMSAATargetRef,
device->CreateImplicitMSAARenderTextureViewFor(
- resolveTarget, implicitSampleCount));
+ resolveTarget, validationState.GetImplicitSampleCount()));
colorTarget = implicitMSAATargetRef.Get();
cmd->colorAttachments[index].view = std::move(implicitMSAATargetRef);
+ // Without explicitly setting depthSlice to zero, its value would be undefined.
cmd->colorAttachments[index].depthSlice = 0;
cmd->colorAttachments[index].loadOp = wgpu::LoadOp::Clear;
cmd->colorAttachments[index].storeOp = wgpu::StoreOp::Discard;
@@ -1153,8 +1220,8 @@
}
}
- cmd->width = width;
- cmd->height = height;
+ cmd->width = validationState.GetWidth();
+ cmd->height = validationState.GetHeight();
cmd->occlusionQuerySet = descriptor->occlusionQuerySet;
@@ -1208,18 +1275,18 @@
"encoding %s.BeginRenderPass(%s).", this, descriptor);
if (success) {
- Ref<RenderPassEncoder> passEncoder =
- RenderPassEncoder::Create(device, descriptor, this, &mEncodingContext,
- std::move(usageTracker), std::move(attachmentState), width,
- height, depthReadOnly, stencilReadOnly, passEndCallback);
+ Ref<RenderPassEncoder> passEncoder = RenderPassEncoder::Create(
+ device, descriptor, this, &mEncodingContext, std::move(usageTracker),
+ std::move(attachmentState), validationState.GetWidth(), validationState.GetHeight(),
+ depthReadOnly, stencilReadOnly, passEndCallback);
mEncodingContext.EnterPass(passEncoder.Get());
MaybeError error;
- if (implicitSampleCount > 1) {
+ if (validationState.GetImplicitSampleCount() > 1) {
error = ApplyMSAARenderToSingleSampledLoadOp(device, passEncoder.Get(), descriptor,
- implicitSampleCount);
+ validationState.GetImplicitSampleCount());
} else if (ShouldApplyClearBigIntegerColorValueWithDraw(device, descriptor)) {
// This is skipped if implicitSampleCount > 1. Because implicitSampleCount > 1 is only
// supported for non-integer textures.
diff --git a/src/dawn/native/PassResourceUsageTracker.cpp b/src/dawn/native/PassResourceUsageTracker.cpp
index c5d100d..08c0c46 100644
--- a/src/dawn/native/PassResourceUsageTracker.cpp
+++ b/src/dawn/native/PassResourceUsageTracker.cpp
@@ -79,20 +79,6 @@
textureSyncInfo.Update(
range, [usage, shaderStages](const SubresourceRange&, TextureSyncInfo* storedSyncInfo) {
- // TODO(crbug.com/dawn/1001): Consider optimizing to have fewer branches.
-
- // Using the same subresource for two different attachments is a write-write or
- // read-write hazard. Add an internal kAgainAsAttachment usage to fail the later check
- // that a subresource with a writable usage has a single usage.
- constexpr wgpu::TextureUsage kAgainAsAttachment =
- kReservedTextureUsage | static_cast<wgpu::TextureUsage>(1);
- constexpr wgpu::TextureUsage kWritableAttachmentUsages =
- wgpu::TextureUsage::RenderAttachment | wgpu::TextureUsage::StorageAttachment;
- if ((usage & kWritableAttachmentUsages) &&
- (storedSyncInfo->usage & kWritableAttachmentUsages)) {
- storedSyncInfo->usage |= kAgainAsAttachment;
- }
-
storedSyncInfo->usage |= usage;
storedSyncInfo->shaderStages |= shaderStages;
});
diff --git a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
index cf5812e..8444f2d 100644
--- a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
@@ -440,15 +440,17 @@
}
}
-// Depth slice index must be within the depth range of 3D color attachment and must be 0 for non-3D
-// color attachment.
+// Check that depthSlice must be set correctly for 3D color attachments and must not be set for
+// non-3D color attachments.
TEST_F(RenderPassDescriptorValidationTest, TextureViewDepthSliceForColor) {
constexpr uint32_t kSize = 8;
constexpr uint32_t kDepthOrArrayLayers = 4;
+ constexpr uint32_t kMipLevelCounts = 4;
constexpr wgpu::TextureFormat kColorFormat = wgpu::TextureFormat::RGBA8Unorm;
- wgpu::Texture colorTexture3D = CreateTexture(device, wgpu::TextureDimension::e3D, kColorFormat,
- kSize, kSize, kDepthOrArrayLayers, 2);
+ wgpu::Texture colorTexture3D =
+ CreateTexture(device, wgpu::TextureDimension::e3D, kColorFormat, kSize, kSize,
+ kDepthOrArrayLayers, kMipLevelCounts);
wgpu::TextureView colorView2D = Create2DAttachment(device, kSize, kSize, kColorFormat);
@@ -459,7 +461,8 @@
baseDescriptor.baseMipLevel = 0;
baseDescriptor.mipLevelCount = 1;
- // Control case: Depth slice index within the depth range of 3D color attachment is valid.
+ // Control case: It's valid if depthSlice is set within the depth range of a 3D color
+ // attachment.
{
wgpu::TextureView view = colorTexture3D.CreateView(&baseDescriptor);
utils::ComboRenderPassDescriptor renderPass({view});
@@ -467,7 +470,14 @@
AssertBeginRenderPassSuccess(&renderPass);
}
- // Depth slice index out of the depth range of 3D color attachment is invalid.
+ // It's invalid if depthSlice is not set for a 3D color attachment.
+ {
+ wgpu::TextureView view = colorTexture3D.CreateView(&baseDescriptor);
+ utils::ComboRenderPassDescriptor renderPass({view});
+ AssertBeginRenderPassError(&renderPass);
+ }
+
+ // It's invalid if depthSlice is out of the depth range of a 3D color attachment.
{
wgpu::TextureView view = colorTexture3D.CreateView(&baseDescriptor);
utils::ComboRenderPassDescriptor renderPass({view});
@@ -475,28 +485,84 @@
AssertBeginRenderPassError(&renderPass);
}
- // Depth slice index out of the depth range of 3D color attachment with non-zero mip level is
- // invalid.
+ // It's invalid if depthSlice is out of the depth range of a 3D color attachment with non-zero
+ // mip level.
{
wgpu::TextureViewDescriptor descriptor = baseDescriptor;
- descriptor.baseMipLevel = 1;
+ descriptor.baseMipLevel = 2;
wgpu::TextureView view = colorTexture3D.CreateView(&descriptor);
utils::ComboRenderPassDescriptor renderPass({view});
- renderPass.cColorAttachments[0].depthSlice = kDepthOrArrayLayers >> 1;
+ renderPass.cColorAttachments[0].depthSlice = kDepthOrArrayLayers >> 2;
AssertBeginRenderPassError(&renderPass);
}
- // Control case: Depth slice must be 0 for non-3D color attachment.
+ // TODO(dawn:1020): Add tests to check depthSlice must not be set for non-3D attachments.
+}
+
+// Check that the depth slices of a 3D color attachment cannot overlap in same render pass.
+TEST_F(RenderPassDescriptorValidationTest, TextureViewDepthSliceOverlaps) {
+ constexpr uint32_t kSize = 8;
+ constexpr uint32_t kDepthOrArrayLayers = 2;
+ constexpr uint32_t kMipLevelCounts = 2;
+ constexpr wgpu::TextureFormat kColorFormat = wgpu::TextureFormat::RGBA8Unorm;
+
+ wgpu::Texture colorTexture3D =
+ CreateTexture(device, wgpu::TextureDimension::e3D, kColorFormat, kSize, kSize,
+ kDepthOrArrayLayers, kMipLevelCounts);
+
+ wgpu::TextureViewDescriptor baseDescriptor;
+ baseDescriptor.dimension = wgpu::TextureViewDimension::e3D;
+ baseDescriptor.baseArrayLayer = 0;
+ baseDescriptor.arrayLayerCount = 1;
+ baseDescriptor.baseMipLevel = 0;
+ baseDescriptor.mipLevelCount = 1;
+
+ // Control case: It's valid if different depth slices of a texture are set in a render pass.
{
- utils::ComboRenderPassDescriptor renderPass({colorView2D});
+ wgpu::TextureView view = colorTexture3D.CreateView(&baseDescriptor);
+ utils::ComboRenderPassDescriptor renderPass({view, view});
renderPass.cColorAttachments[0].depthSlice = 0;
+ renderPass.cColorAttachments[1].depthSlice = 1;
AssertBeginRenderPassSuccess(&renderPass);
}
- // Non-zero depth slice is invalid for non-3D color attachment.
+ // It's valid if same depth slice of different mip levels from a texture with size [1, 1, n] is
+ // set in a render pass.
{
- utils::ComboRenderPassDescriptor renderPass({colorView2D});
- renderPass.cColorAttachments[0].depthSlice = 1;
+ wgpu::Texture texture = CreateTexture(device, wgpu::TextureDimension::e3D, kColorFormat, 1,
+ 1, kDepthOrArrayLayers, kMipLevelCounts);
+ wgpu::TextureView view = texture.CreateView(&baseDescriptor);
+ wgpu::TextureViewDescriptor descriptor = baseDescriptor;
+ descriptor.baseMipLevel = 1;
+ wgpu::TextureView view2 = texture.CreateView(&descriptor);
+
+ utils::ComboRenderPassDescriptor renderPass({view, view2});
+ renderPass.cColorAttachments[0].depthSlice = 0;
+ renderPass.cColorAttachments[1].depthSlice = 0;
+ AssertBeginRenderPassSuccess(&renderPass);
+ }
+
+ // It's valid if same depth slice of different textures is set in a render pass.
+ {
+ wgpu::Texture otherColorTexture3D =
+ CreateTexture(device, wgpu::TextureDimension::e3D, kColorFormat, kSize, kSize,
+ kDepthOrArrayLayers, kMipLevelCounts);
+
+ wgpu::TextureView view = colorTexture3D.CreateView(&baseDescriptor);
+ wgpu::TextureView view2 = otherColorTexture3D.CreateView(&baseDescriptor);
+
+ utils::ComboRenderPassDescriptor renderPass({view, view2});
+ renderPass.cColorAttachments[0].depthSlice = 0;
+ renderPass.cColorAttachments[1].depthSlice = 0;
+ AssertBeginRenderPassSuccess(&renderPass);
+ }
+
+ // It's invalid if same depth slice of a texture is set twice in a render pass.
+ {
+ wgpu::TextureView view = colorTexture3D.CreateView(&baseDescriptor);
+ utils::ComboRenderPassDescriptor renderPass({view, view});
+ renderPass.cColorAttachments[0].depthSlice = 0;
+ renderPass.cColorAttachments[1].depthSlice = 0;
AssertBeginRenderPassError(&renderPass);
}
}
@@ -1023,6 +1089,34 @@
}
}
+// Test the overlaps of multiple resolve target.
+TEST_F(MultisampledRenderPassDescriptorValidationTest, ResolveTargetUsedTwice) {
+ wgpu::TextureView resolveTextureView = CreateNonMultisampledColorTextureView();
+ wgpu::TextureView colorTextureView1 = CreateMultisampledColorTextureView();
+ wgpu::TextureView colorTextureView2 = CreateMultisampledColorTextureView();
+
+ // It is allowed to use different resolve targets in a render pass.
+ {
+ wgpu::TextureView anotherResolveTextureView = CreateNonMultisampledColorTextureView();
+ utils::ComboRenderPassDescriptor renderPass =
+ utils::ComboRenderPassDescriptor({colorTextureView1, colorTextureView2});
+ renderPass.cColorAttachments[0].resolveTarget = resolveTextureView;
+ renderPass.cColorAttachments[1].resolveTarget = anotherResolveTextureView;
+
+ AssertBeginRenderPassSuccess(&renderPass);
+ }
+
+ // It is not allowed to use a resolve target twice in a render pass.
+ {
+ utils::ComboRenderPassDescriptor renderPass =
+ utils::ComboRenderPassDescriptor({colorTextureView1, colorTextureView2});
+ renderPass.cColorAttachments[0].resolveTarget = resolveTextureView;
+ renderPass.cColorAttachments[1].resolveTarget = resolveTextureView;
+
+ AssertBeginRenderPassError(&renderPass);
+ }
+}
+
// Tests the texture format of the resolve target must support being used as resolve target.
TEST_F(MultisampledRenderPassDescriptorValidationTest, ResolveTargetFormat) {
for (wgpu::TextureFormat format : utils::kAllTextureFormats) {
