Add two missing checks on render pass descriptor

This patch adds two missing checks on the render pass descriptor:
1. NaN is not allowed to be a value of clearColor and clearDepth.
2. Ensure only valid values can be used as loadOp and storeOp.

This patch also adds the unit tests to ensure INFINITY is a valid
value for both clearColor and clearDepth.

BUG=dawn:299

Change-Id: Ia5500701ccd99abf488a80c87adb809521d7873f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/14460
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
This commit is contained in:
Jiawei Shao 2019-12-12 01:29:01 +00:00 committed by Commit Bot service account
parent af094e6a88
commit 51c347a231
2 changed files with 99 additions and 0 deletions

View File

@ -26,6 +26,7 @@
#include "dawn_native/ErrorData.h" #include "dawn_native/ErrorData.h"
#include "dawn_native/RenderPassEncoder.h" #include "dawn_native/RenderPassEncoder.h"
#include "dawn_native/RenderPipeline.h" #include "dawn_native/RenderPipeline.h"
#include "dawn_native/ValidationUtils_autogen.h"
#include "dawn_platform/DawnPlatform.h" #include "dawn_platform/DawnPlatform.h"
#include "dawn_platform/tracing/TraceEvent.h" #include "dawn_platform/tracing/TraceEvent.h"
@ -376,6 +377,18 @@ namespace dawn_native {
"renderable"); "renderable");
} }
DAWN_TRY(ValidateLoadOp(colorAttachment.loadOp));
DAWN_TRY(ValidateStoreOp(colorAttachment.storeOp));
if (colorAttachment.loadOp == wgpu::LoadOp::Clear) {
if (std::isnan(colorAttachment.clearColor.r) ||
std::isnan(colorAttachment.clearColor.g) ||
std::isnan(colorAttachment.clearColor.b) ||
std::isnan(colorAttachment.clearColor.a)) {
return DAWN_VALIDATION_ERROR("Color clear value cannot contain NaN");
}
}
DAWN_TRY(ValidateOrSetColorAttachmentSampleCount(attachment, sampleCount)); DAWN_TRY(ValidateOrSetColorAttachmentSampleCount(attachment, sampleCount));
DAWN_TRY(ValidateResolveTarget(device, colorAttachment)); DAWN_TRY(ValidateResolveTarget(device, colorAttachment));
@ -404,6 +417,16 @@ namespace dawn_native {
"depth stencil format"); "depth stencil format");
} }
DAWN_TRY(ValidateLoadOp(depthStencilAttachment->depthLoadOp));
DAWN_TRY(ValidateLoadOp(depthStencilAttachment->stencilLoadOp));
DAWN_TRY(ValidateStoreOp(depthStencilAttachment->depthStoreOp));
DAWN_TRY(ValidateStoreOp(depthStencilAttachment->stencilStoreOp));
if (depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Clear &&
std::isnan(depthStencilAttachment->clearDepth)) {
return DAWN_VALIDATION_ERROR("Depth clear value cannot be NaN");
}
// This validates that the depth storeOp and stencil storeOps are the same // This validates that the depth storeOp and stencil storeOps are the same
if (depthStencilAttachment->depthStoreOp != depthStencilAttachment->stencilStoreOp) { if (depthStencilAttachment->depthStoreOp != depthStencilAttachment->stencilStoreOp) {
return DAWN_VALIDATION_ERROR( return DAWN_VALIDATION_ERROR(

View File

@ -18,6 +18,8 @@
#include "utils/WGPUHelpers.h" #include "utils/WGPUHelpers.h"
#include <cmath>
namespace { namespace {
class RenderPassDescriptorValidationTest : public ValidationTest { class RenderPassDescriptorValidationTest : public ValidationTest {
@ -661,6 +663,80 @@ TEST_F(MultisampledRenderPassDescriptorValidationTest, DepthStencilAttachmentSam
} }
} }
// Tests that NaN cannot be accepted as a valid color or depth clear value and INFINITY is valid in
// both color and depth clear values.
TEST_F(RenderPassDescriptorValidationTest, UseNaNOrINFINITYAsColorOrDepthClearValue) {
wgpu::TextureView color = Create2DAttachment(device, 1, 1, wgpu::TextureFormat::RGBA8Unorm);
// Tests that NaN cannot be used in clearColor.
{
utils::ComboRenderPassDescriptor renderPass({color}, nullptr);
renderPass.cColorAttachments[0].clearColor.r = NAN;
AssertBeginRenderPassError(&renderPass);
}
{
utils::ComboRenderPassDescriptor renderPass({color}, nullptr);
renderPass.cColorAttachments[0].clearColor.g = NAN;
AssertBeginRenderPassError(&renderPass);
}
{
utils::ComboRenderPassDescriptor renderPass({color}, nullptr);
renderPass.cColorAttachments[0].clearColor.b = NAN;
AssertBeginRenderPassError(&renderPass);
}
{
utils::ComboRenderPassDescriptor renderPass({color}, nullptr);
renderPass.cColorAttachments[0].clearColor.a = NAN;
AssertBeginRenderPassError(&renderPass);
}
// Tests that INFINITY can be used in clearColor.
{
utils::ComboRenderPassDescriptor renderPass({color}, nullptr);
renderPass.cColorAttachments[0].clearColor.r = INFINITY;
AssertBeginRenderPassSuccess(&renderPass);
}
{
utils::ComboRenderPassDescriptor renderPass({color}, nullptr);
renderPass.cColorAttachments[0].clearColor.g = INFINITY;
AssertBeginRenderPassSuccess(&renderPass);
}
{
utils::ComboRenderPassDescriptor renderPass({color}, nullptr);
renderPass.cColorAttachments[0].clearColor.b = INFINITY;
AssertBeginRenderPassSuccess(&renderPass);
}
{
utils::ComboRenderPassDescriptor renderPass({color}, nullptr);
renderPass.cColorAttachments[0].clearColor.a = INFINITY;
AssertBeginRenderPassSuccess(&renderPass);
}
// Tests that NaN cannot be used in clearDepth.
{
wgpu::TextureView depth =
Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24Plus);
utils::ComboRenderPassDescriptor renderPass({color}, depth);
renderPass.cDepthStencilAttachmentInfo.clearDepth = NAN;
AssertBeginRenderPassError(&renderPass);
}
// Tests that INFINITY can be used in clearDepth.
{
wgpu::TextureView depth =
Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24Plus);
utils::ComboRenderPassDescriptor renderPass({color}, depth);
renderPass.cDepthStencilAttachmentInfo.clearDepth = INFINITY;
AssertBeginRenderPassSuccess(&renderPass);
}
}
// TODO(cwallez@chromium.org): Constraints on attachment aliasing? // TODO(cwallez@chromium.org): Constraints on attachment aliasing?
} // anonymous namespace } // anonymous namespace