diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp index 3900d07b75..004916381f 100644 --- a/src/dawn/native/CommandEncoder.cpp +++ b/src/dawn/native/CommandEncoder.cpp @@ -277,7 +277,16 @@ MaybeError ValidateRenderPassDepthStencilAttachment( DAWN_TRY(ValidateCanUseAs(attachment->GetTexture(), wgpu::TextureUsage::RenderAttachment, usageValidationMode)); - const Format& format = attachment->GetFormat(); + // DS attachments must encompass all aspects of the texture, so we first check that this is + // true, which means that in the rest of the function we can assume that the view's format is + // the same as the texture's format. + const Format& format = attachment->GetTexture()->GetFormat(); + DAWN_INVALID_IF( + attachment->GetAspects() != format.aspects, + "The depth stencil attachment %s must encompass all aspects of it's texture's format (%s).", + attachment, format.format); + ASSERT(attachment->GetFormat().format == format.format); + DAWN_INVALID_IF(!format.HasDepthOrStencil(), "The depth stencil attachment %s format (%s) is not a depth stencil format.", attachment, format.format); @@ -286,9 +295,6 @@ MaybeError ValidateRenderPassDepthStencilAttachment( "The depth stencil attachment %s format (%s) is not renderable.", attachment, format.format); - DAWN_INVALID_IF(attachment->GetAspects() != format.aspects, - "The depth stencil attachment %s must encompass all aspects.", attachment); - DAWN_INVALID_IF( attachment->GetAspects() == (Aspect::Depth | Aspect::Stencil) && depthStencilAttachment->depthReadOnly != depthStencilAttachment->stencilReadOnly, diff --git a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp index 50f7c516a9..f9a99ff6fa 100644 --- a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp @@ -1051,6 +1051,7 @@ TEST_F(RenderPassDescriptorValidationTest, ValidateDepthStencilAllAspects) { // Using all aspects of a depth+stencil texture is allowed. { texDesc.format = wgpu::TextureFormat::Depth24PlusStencil8; + viewDesc.format = wgpu::TextureFormat::Undefined; viewDesc.aspect = wgpu::TextureAspect::All; wgpu::TextureView view = device.CreateTexture(&texDesc).CreateView(&viewDesc); @@ -1058,29 +1059,68 @@ TEST_F(RenderPassDescriptorValidationTest, ValidateDepthStencilAllAspects) { AssertBeginRenderPassSuccess(&renderPass); } - // Using only depth of a depth+stencil texture is an error. + // Using only depth of a depth+stencil texture is an error, case without format + // reinterpretation. { texDesc.format = wgpu::TextureFormat::Depth24PlusStencil8; + viewDesc.format = wgpu::TextureFormat::Undefined; viewDesc.aspect = wgpu::TextureAspect::DepthOnly; wgpu::TextureView view = device.CreateTexture(&texDesc).CreateView(&viewDesc); utils::ComboRenderPassDescriptor renderPass({}, view); + renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Undefined; + renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Undefined; + AssertBeginRenderPassError(&renderPass); } - // Using only stencil of a depth+stencil texture is an error. + // Using only depth of a depth+stencil texture is an error, case with format reinterpretation. { texDesc.format = wgpu::TextureFormat::Depth24PlusStencil8; + viewDesc.format = wgpu::TextureFormat::Depth24Plus; + viewDesc.aspect = wgpu::TextureAspect::DepthOnly; + + wgpu::TextureView view = device.CreateTexture(&texDesc).CreateView(&viewDesc); + utils::ComboRenderPassDescriptor renderPass({}, view); + renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Undefined; + renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Undefined; + + AssertBeginRenderPassError(&renderPass); + } + + // Using only stencil of a depth+stencil texture is an error, case without format + // reinterpration. + { + texDesc.format = wgpu::TextureFormat::Depth24PlusStencil8; + viewDesc.format = wgpu::TextureFormat::Undefined; viewDesc.aspect = wgpu::TextureAspect::StencilOnly; wgpu::TextureView view = device.CreateTexture(&texDesc).CreateView(&viewDesc); utils::ComboRenderPassDescriptor renderPass({}, view); + renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Undefined; + renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Undefined; + + AssertBeginRenderPassError(&renderPass); + } + + // Using only stencil of a depth+stencil texture is an error, case with format reinterpretation. + { + texDesc.format = wgpu::TextureFormat::Depth24PlusStencil8; + viewDesc.format = wgpu::TextureFormat::Stencil8; + viewDesc.aspect = wgpu::TextureAspect::StencilOnly; + + wgpu::TextureView view = device.CreateTexture(&texDesc).CreateView(&viewDesc); + utils::ComboRenderPassDescriptor renderPass({}, view); + renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Undefined; + renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Undefined; + AssertBeginRenderPassError(&renderPass); } // Using DepthOnly of a depth only texture is allowed. { texDesc.format = wgpu::TextureFormat::Depth24Plus; + viewDesc.format = wgpu::TextureFormat::Undefined; viewDesc.aspect = wgpu::TextureAspect::DepthOnly; wgpu::TextureView view = device.CreateTexture(&texDesc).CreateView(&viewDesc); @@ -1091,8 +1131,19 @@ TEST_F(RenderPassDescriptorValidationTest, ValidateDepthStencilAllAspects) { AssertBeginRenderPassSuccess(&renderPass); } - // TODO(https://crbug.com/dawn/666): Add a test case for stencil-only on stencil8 once this - // format is supported. + // Using StencilOnly of a stencil only texture is allowed. + { + texDesc.format = wgpu::TextureFormat::Stencil8; + viewDesc.format = wgpu::TextureFormat::Undefined; + viewDesc.aspect = wgpu::TextureAspect::StencilOnly; + + wgpu::TextureView view = device.CreateTexture(&texDesc).CreateView(&viewDesc); + utils::ComboRenderPassDescriptor renderPass({}, view); + renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Undefined; + renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Undefined; + + AssertBeginRenderPassSuccess(&renderPass); + } } // TODO(cwallez@chromium.org): Constraints on attachment aliasing?