From bdc029ee3918873893a02295bc0b83506d66449e Mon Sep 17 00:00:00 2001 From: shrekshao Date: Mon, 19 Jul 2021 23:27:27 +0000 Subject: [PATCH] Validate format is blendable when blending is enabled Treat color target format with "float" capabilities as blendable format and validate when blending is enabled. Add helpers for checking float16 texture values. Bug: dawn:726 Change-Id: Icf8c0182e5e9a13523970c84b5af91f395a089af Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/57744 Commit-Queue: Shrek Shao Reviewed-by: Austin Eng --- src/common/Math.cpp | 6 ++ src/common/Math.h | 1 + src/dawn_native/RenderPipeline.cpp | 5 ++ .../ShaderVisibleDescriptorAllocatorD3D12.cpp | 5 +- src/tests/DawnTest.cpp | 37 ++++++++--- src/tests/DawnTest.h | 28 +++++--- .../RenderPipelineValidationTests.cpp | 66 +++++++++++++++++++ .../white_box/D3D12DescriptorHeapTests.cpp | 6 +- 8 files changed, 131 insertions(+), 23 deletions(-) diff --git a/src/common/Math.cpp b/src/common/Math.cpp index 80b8438d65..053fa3e4c1 100644 --- a/src/common/Math.cpp +++ b/src/common/Math.cpp @@ -126,6 +126,12 @@ uint16_t Float32ToFloat16(float fp32) { } } +float Float16ToFloat32(uint16_t fp16) { + uint32_t tmp = (fp16 & 0x7fff) << 13 | (fp16 & 0x8000) << 16; + float tmp2 = *reinterpret_cast(&tmp); + return pow(2, 127 - 15) * tmp2; +} + bool IsFloat16NaN(uint16_t fp16) { return (fp16 & 0x7FFF) > 0x7C00; } diff --git a/src/common/Math.h b/src/common/Math.h index 56ac92b094..69cab24dea 100644 --- a/src/common/Math.h +++ b/src/common/Math.h @@ -86,6 +86,7 @@ destType BitCast(const sourceType& source) { } uint16_t Float32ToFloat16(float fp32); +float Float16ToFloat32(uint16_t fp16); bool IsFloat16NaN(uint16_t fp16); float SRGBToLinear(float srgb); diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp index d77a4ebb8e..709fafb73a 100644 --- a/src/dawn_native/RenderPipeline.cpp +++ b/src/dawn_native/RenderPipeline.cpp @@ -261,6 +261,11 @@ namespace dawn_native { if (!format->IsColor() || !format->isRenderable) { return DAWN_VALIDATION_ERROR("Color format must be color renderable"); } + if (descriptor->blend && !(format->GetAspectInfo(Aspect::Color).supportedSampleTypes & + SampleTypeBit::Float)) { + return DAWN_VALIDATION_ERROR( + "Color format must be blendable when blending is enabled"); + } if (fragmentWritten && fragmentOutputBaseType != format->GetAspectInfo(Aspect::Color).baseType) { return DAWN_VALIDATION_ERROR( diff --git a/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp b/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp index df32ae91bf..916a371cb4 100644 --- a/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp +++ b/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp @@ -22,7 +22,10 @@ namespace dawn_native { namespace d3d12 { // Limits the min/max heap size to always be some known value for testing. // Thresholds should be adjusted (lower == faster) to avoid tests taking too long to complete. - static constexpr const uint32_t kShaderVisibleSmallHeapSizes[] = {1024, 512}; + // We change the value from {1024, 512} to {32, 16} because we use blending + // for D3D12DescriptorHeapTests.EncodeManyUBO and R16Float has limited range + // and low precision at big integer. + static constexpr const uint32_t kShaderVisibleSmallHeapSizes[] = {32, 16}; uint32_t GetD3D12ShaderVisibleHeapMinSize(D3D12_DESCRIPTOR_HEAP_TYPE heapType, bool useSmallSize) { diff --git a/src/tests/DawnTest.cpp b/src/tests/DawnTest.cpp index a5fed42dbb..936bb12c05 100644 --- a/src/tests/DawnTest.cpp +++ b/src/tests/DawnTest.cpp @@ -1514,21 +1514,21 @@ namespace detail { // Helper classes to set expectations - template - ExpectEq::ExpectEq(T singleValue, T tolerance) : mTolerance(tolerance) { + template + ExpectEq::ExpectEq(T singleValue, T tolerance) : mTolerance(tolerance) { mExpected.push_back(singleValue); } - template - ExpectEq::ExpectEq(const T* values, const unsigned int count, T tolerance) + template + ExpectEq::ExpectEq(const T* values, const unsigned int count, T tolerance) : mTolerance(tolerance) { mExpected.assign(values, values + count); } namespace { - template - testing::AssertionResult CheckImpl(const T& expected, const T& actual, const T& tolerance) { + template + testing::AssertionResult CheckImpl(const T& expected, const U& actual, const T& tolerance) { ASSERT(tolerance == T{}); if (expected != actual) { return testing::AssertionFailure() << expected << ", actual " << actual; @@ -1549,12 +1549,28 @@ namespace detail { return testing::AssertionSuccess(); } + // Interpret uint16_t as float16 + // This is mostly for reading float16 output from textures + template <> + testing::AssertionResult CheckImpl(const float& expected, + const uint16_t& actual, + const float& tolerance) { + float actualF32 = Float16ToFloat32(actual); + if (abs(expected - actualF32) > tolerance) { + return tolerance == 0.0 + ? testing::AssertionFailure() << expected << ", actual " << actualF32 + : testing::AssertionFailure() << "within " << tolerance << " of " + << expected << ", actual " << actualF32; + } + return testing::AssertionSuccess(); + } + } // namespace - template - testing::AssertionResult ExpectEq::Check(const void* data, size_t size) { - DAWN_ASSERT(size == sizeof(T) * mExpected.size()); - const T* actual = static_cast(data); + template + testing::AssertionResult ExpectEq::Check(const void* data, size_t size) { + DAWN_ASSERT(size == sizeof(U) * mExpected.size()); + const U* actual = static_cast(data); for (size_t i = 0; i < mExpected.size(); ++i) { testing::AssertionResult check = CheckImpl(mExpected[i], actual[i], mTolerance); @@ -1583,6 +1599,7 @@ namespace detail { template class ExpectEq; template class ExpectEq; template class ExpectEq; + template class ExpectEq; template ExpectBetweenColors::ExpectBetweenColors(T value0, T value1) { diff --git a/src/tests/DawnTest.h b/src/tests/DawnTest.h index aa15e33056..bdf1f384ae 100644 --- a/src/tests/DawnTest.h +++ b/src/tests/DawnTest.h @@ -82,6 +82,9 @@ #define EXPECT_PIXEL_FLOAT_EQ(expected, texture, x, y) \ AddTextureExpectation(__FILE__, __LINE__, expected, texture, {x, y}) +#define EXPECT_PIXEL_FLOAT16_EQ(expected, texture, x, y) \ + AddTextureExpectation(__FILE__, __LINE__, expected, texture, {x, y}) + #define EXPECT_PIXEL_RGBA8_BETWEEN(color0, color1, texture, x, y) \ AddTextureBetweenColorsExpectation(__FILE__, __LINE__, color0, color1, texture, x, y) @@ -183,7 +186,7 @@ namespace detail { class Expectation; class CustomTextureExpectation; - template + template class ExpectEq; template class ExpectBetweenColors; @@ -341,7 +344,9 @@ class DawnTestBase { uint64_t size, detail::Expectation* expectation); - template + // T - expected value Type + // U - actual value Type (defaults = T) + template std::ostringstream& AddTextureExpectation(const char* file, int line, const T* expectedData, @@ -353,12 +358,12 @@ class DawnTestBase { uint32_t bytesPerRow = 0) { return AddTextureExpectationImpl( file, line, - new detail::ExpectEq(expectedData, - extent.width * extent.height * extent.depthOrArrayLayers), - texture, origin, extent, level, aspect, sizeof(T), bytesPerRow); + new detail::ExpectEq(expectedData, + extent.width * extent.height * extent.depthOrArrayLayers), + texture, origin, extent, level, aspect, sizeof(U), bytesPerRow); } - template + template std::ostringstream& AddTextureExpectation(const char* file, int line, const T& expectedData, @@ -367,8 +372,9 @@ class DawnTestBase { uint32_t level = 0, wgpu::TextureAspect aspect = wgpu::TextureAspect::All, uint32_t bytesPerRow = 0) { - return AddTextureExpectationImpl(file, line, new detail::ExpectEq(expectedData), texture, - origin, {1, 1}, level, aspect, sizeof(T), bytesPerRow); + return AddTextureExpectationImpl(file, line, new detail::ExpectEq(expectedData), + texture, origin, {1, 1}, level, aspect, sizeof(U), + bytesPerRow); } template + // T - expected value Type + // U - actual value Type (defaults = T) + // This is expanded for float16 mostly where T=float, U=uint16_t + template class ExpectEq : public Expectation { public: ExpectEq(T singleValue, T tolerance = {}); @@ -710,6 +719,7 @@ namespace detail { extern template class ExpectEq; extern template class ExpectEq; extern template class ExpectEq; + extern template class ExpectEq; template class ExpectBetweenColors : public Expectation { diff --git a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp index ab0e327dc5..bd78d8847e 100644 --- a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp +++ b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp @@ -35,10 +35,16 @@ class RenderPipelineValidationTest : public ValidationTest { [[stage(fragment)]] fn main() -> [[location(0)]] vec4 { return vec4(0.0, 1.0, 0.0, 1.0); })"); + + fsModuleUint = utils::CreateShaderModule(device, R"( + [[stage(fragment)]] fn main() -> [[location(0)]] vec4 { + return vec4(0u, 255u, 0u, 255u); + })"); } wgpu::ShaderModule vsModule; wgpu::ShaderModule fsModule; + wgpu::ShaderModule fsModuleUint; }; // Test cases where creation should succeed @@ -148,6 +154,66 @@ TEST_F(RenderPipelineValidationTest, NonRenderableFormat) { } } +// Tests that the color formats must be blendable when blending is enabled. +// Those are renderable color formats with "float" capabilities in +// https://gpuweb.github.io/gpuweb/#plain-color-formats +TEST_F(RenderPipelineValidationTest, NonBlendableFormat) { + { + // Succeeds because RGBA8Unorm is blendable + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = vsModule; + descriptor.cFragment.module = fsModule; + descriptor.cTargets[0].blend = &descriptor.cBlends[0]; + descriptor.cTargets[0].format = wgpu::TextureFormat::RGBA8Unorm; + + device.CreateRenderPipeline(&descriptor); + } + + { + // Fails because RGBA32Float is not blendable + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = vsModule; + descriptor.cFragment.module = fsModule; + descriptor.cTargets[0].blend = &descriptor.cBlends[0]; + descriptor.cTargets[0].format = wgpu::TextureFormat::RGBA32Float; + + ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor)); + } + + { + // Succeeds because RGBA32Float is not blendable but blending is disabled + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = vsModule; + descriptor.cFragment.module = fsModule; + descriptor.cTargets[0].blend = nullptr; + descriptor.cTargets[0].format = wgpu::TextureFormat::RGBA32Float; + + device.CreateRenderPipeline(&descriptor); + } + + { + // Fails because RGBA8Uint is not blendable + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = vsModule; + descriptor.cFragment.module = fsModuleUint; + descriptor.cTargets[0].blend = &descriptor.cBlends[0]; + descriptor.cTargets[0].format = wgpu::TextureFormat::RGBA8Uint; + + ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor)); + } + + { + // Succeeds because RGBA8Uint is not blendable but blending is disabled + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = vsModule; + descriptor.cFragment.module = fsModuleUint; + descriptor.cTargets[0].blend = nullptr; + descriptor.cTargets[0].format = wgpu::TextureFormat::RGBA8Uint; + + device.CreateRenderPipeline(&descriptor); + } +} + // Tests that the format of the color state descriptor must match the output of the fragment shader. TEST_F(RenderPipelineValidationTest, FragmentOutputFormatCompatibility) { constexpr uint32_t kNumTextureFormatBaseType = 3u; diff --git a/src/tests/white_box/D3D12DescriptorHeapTests.cpp b/src/tests/white_box/D3D12DescriptorHeapTests.cpp index c55cda9f3f..fec3550db6 100644 --- a/src/tests/white_box/D3D12DescriptorHeapTests.cpp +++ b/src/tests/white_box/D3D12DescriptorHeapTests.cpp @@ -438,7 +438,7 @@ TEST_P(D3D12DescriptorHeapTests, EncodeManyUBO) { dawn_native::Toggle::UseD3D12SmallShaderVisibleHeapForTesting)); utils::BasicRenderPass renderPass = - MakeRenderPass(kRTSize, kRTSize, wgpu::TextureFormat::R32Float); + MakeRenderPass(kRTSize, kRTSize, wgpu::TextureFormat::R16Float); utils::ComboRenderPipelineDescriptor pipelineDescriptor; pipelineDescriptor.vertex.module = mSimpleVSModule; @@ -461,7 +461,7 @@ TEST_P(D3D12DescriptorHeapTests, EncodeManyUBO) { blend.alpha.srcFactor = wgpu::BlendFactor::One; blend.alpha.dstFactor = wgpu::BlendFactor::One; - pipelineDescriptor.cTargets[0].format = wgpu::TextureFormat::R32Float; + pipelineDescriptor.cTargets[0].format = wgpu::TextureFormat::R16Float; pipelineDescriptor.cTargets[0].blend = &blend; wgpu::RenderPipeline renderPipeline = device.CreateRenderPipeline(&pipelineDescriptor); @@ -500,7 +500,7 @@ TEST_P(D3D12DescriptorHeapTests, EncodeManyUBO) { queue.Submit(1, &commands); float colorSum = numOfEncodedBindGroups * (numOfEncodedBindGroups + 1) / 2; - EXPECT_PIXEL_FLOAT_EQ(colorSum, renderPass.color, 0, 0); + EXPECT_PIXEL_FLOAT16_EQ(colorSum, renderPass.color, 0, 0); } // Verify encoding one bindgroup then a heaps worth in different submits.