Validate texture aspect on TextureView creation
Bug: dawn:439
Change-Id: Iba8c283e2f4551d9600410ff958d5a304a49ae2c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/30724
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp
index 5e98cef..a246297 100644
--- a/src/dawn_native/CommandValidation.cpp
+++ b/src/dawn_native/CommandValidation.cpp
@@ -501,24 +501,8 @@
return DAWN_VALIDATION_ERROR("mipLevel out of range");
}
- switch (textureCopy.aspect) {
- case wgpu::TextureAspect::All:
- break;
- case wgpu::TextureAspect::DepthOnly:
- if ((texture->GetFormat().aspects & Aspect::Depth) == 0) {
- return DAWN_VALIDATION_ERROR(
- "Texture does not have depth aspect for texture copy");
- }
- break;
- case wgpu::TextureAspect::StencilOnly:
- if ((texture->GetFormat().aspects & Aspect::Stencil) == 0) {
- return DAWN_VALIDATION_ERROR(
- "Texture does not have stencil aspect for texture copy");
- }
- break;
- default:
- UNREACHABLE();
- break;
+ if (TryConvertAspect(texture->GetFormat(), textureCopy.aspect) == Aspect::None) {
+ return DAWN_VALIDATION_ERROR("Texture does not have selected aspect for texture copy.");
}
if (texture->GetSampleCount() > 1 || texture->GetFormat().HasDepthOrStencil()) {
diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp
index c3ad9bf..c46ae88 100644
--- a/src/dawn_native/Texture.cpp
+++ b/src/dawn_native/Texture.cpp
@@ -271,8 +271,8 @@
DAWN_TRY(ValidateTextureFormat(descriptor->format));
DAWN_TRY(ValidateTextureAspect(descriptor->aspect));
- if (descriptor->aspect != wgpu::TextureAspect::All) {
- return DAWN_VALIDATION_ERROR("Texture aspect must be 'all'");
+ if (TryConvertAspect(texture->GetFormat(), descriptor->aspect) == Aspect::None) {
+ return DAWN_VALIDATION_ERROR("Texture does not have selected aspect for texture view.");
}
// TODO(jiawei.shao@intel.com): check stuff based on resource limits
@@ -358,15 +358,19 @@
}
Aspect ConvertAspect(const Format& format, wgpu::TextureAspect aspect) {
+ Aspect aspectMask = TryConvertAspect(format, aspect);
+ ASSERT(aspectMask != Aspect::None);
+ return aspectMask;
+ }
+
+ Aspect TryConvertAspect(const Format& format, wgpu::TextureAspect aspect) {
switch (aspect) {
case wgpu::TextureAspect::All:
return format.aspects;
case wgpu::TextureAspect::DepthOnly:
- ASSERT(format.aspects & Aspect::Depth);
- return Aspect::Depth;
+ return format.aspects & Aspect::Depth;
case wgpu::TextureAspect::StencilOnly:
- ASSERT(format.aspects & Aspect::Stencil);
- return Aspect::Stencil;
+ return format.aspects & Aspect::Stencil;
}
}
diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h
index 11dd965..001f5d5 100644
--- a/src/dawn_native/Texture.h
+++ b/src/dawn_native/Texture.h
@@ -73,9 +73,19 @@
wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::Storage |
wgpu::TextureUsage::OutputAttachment;
+ // Convert the TextureAspect to an Aspect mask for the format. ASSERTs if the aspect
+ // does not exist in the format.
+ // Also ASSERTs if "All" is selected and results in more than one aspect.
Aspect ConvertSingleAspect(const Format& format, wgpu::TextureAspect aspect);
+
+ // Convert the TextureAspect to an Aspect mask for the format. ASSERTs if the aspect
+ // does not exist in the format.
Aspect ConvertAspect(const Format& format, wgpu::TextureAspect aspect);
+ // Try to convert the TextureAspect to an Aspect mask for the format. May return
+ // Aspect::None.
+ Aspect TryConvertAspect(const Format& format, wgpu::TextureAspect aspect);
+
struct SubresourceRange {
uint32_t baseMipLevel;
uint32_t levelCount;
diff --git a/src/tests/unittests/validation/TextureViewValidationTests.cpp b/src/tests/unittests/validation/TextureViewValidationTests.cpp
index c5c3242..b31439a 100644
--- a/src/tests/unittests/validation/TextureViewValidationTests.cpp
+++ b/src/tests/unittests/validation/TextureViewValidationTests.cpp
@@ -344,20 +344,43 @@
ASSERT_DEVICE_ERROR(texture.CreateView(&descriptor));
}
- // Test that only TextureAspect::All is supported
- TEST_F(TextureViewValidationTest, AspectMustBeAll) {
+ // Test that the selected TextureAspects must exist in the texture format
+ TEST_F(TextureViewValidationTest, AspectMustExist) {
wgpu::TextureDescriptor descriptor = {};
descriptor.size = {1, 1, 1};
- descriptor.format = wgpu::TextureFormat::Depth32Float;
descriptor.usage = wgpu::TextureUsage::Sampled | wgpu::TextureUsage::OutputAttachment;
- wgpu::Texture texture = device.CreateTexture(&descriptor);
- wgpu::TextureViewDescriptor viewDescriptor = {};
- viewDescriptor.aspect = wgpu::TextureAspect::All;
- texture.CreateView(&viewDescriptor);
+ // Can select: All and DepthOnly from Depth32Float, but not StencilOnly
+ {
+ descriptor.format = wgpu::TextureFormat::Depth32Float;
+ wgpu::Texture texture = device.CreateTexture(&descriptor);
- viewDescriptor.aspect = wgpu::TextureAspect::DepthOnly;
- ASSERT_DEVICE_ERROR(texture.CreateView(&viewDescriptor));
+ wgpu::TextureViewDescriptor viewDescriptor = {};
+ viewDescriptor.aspect = wgpu::TextureAspect::All;
+ texture.CreateView(&viewDescriptor);
+
+ viewDescriptor.aspect = wgpu::TextureAspect::DepthOnly;
+ texture.CreateView(&viewDescriptor);
+
+ viewDescriptor.aspect = wgpu::TextureAspect::StencilOnly;
+ ASSERT_DEVICE_ERROR(texture.CreateView(&viewDescriptor));
+ }
+
+ // Can select: All, DepthOnly, and StencilOnly from Depth24PlusStencil8
+ {
+ descriptor.format = wgpu::TextureFormat::Depth24PlusStencil8;
+ wgpu::Texture texture = device.CreateTexture(&descriptor);
+
+ wgpu::TextureViewDescriptor viewDescriptor = {};
+ viewDescriptor.aspect = wgpu::TextureAspect::All;
+ texture.CreateView(&viewDescriptor);
+
+ viewDescriptor.aspect = wgpu::TextureAspect::DepthOnly;
+ texture.CreateView(&viewDescriptor);
+
+ viewDescriptor.aspect = wgpu::TextureAspect::StencilOnly;
+ texture.CreateView(&viewDescriptor);
+ }
}
} // anonymous namespace
