diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index 5e98cef65b..a2462977d0 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -501,24 +501,8 @@ namespace dawn_native { 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 c3ad9bf039..c46ae88626 100644 --- a/src/dawn_native/Texture.cpp +++ b/src/dawn_native/Texture.cpp @@ -271,8 +271,8 @@ namespace dawn_native { 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 @@ namespace dawn_native { } 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 11dd965554..001f5d545d 100644 --- a/src/dawn_native/Texture.h +++ b/src/dawn_native/Texture.h @@ -73,9 +73,19 @@ namespace dawn_native { 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 c5c324220c..b31439a511 100644 --- a/src/tests/unittests/validation/TextureViewValidationTests.cpp +++ b/src/tests/unittests/validation/TextureViewValidationTests.cpp @@ -344,20 +344,43 @@ namespace { 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