commit 7e69c975fa0967ee295334a22d47a9c093be36cf
author Nicolas Capens <capn@google.com> Sat Sep 11 00:30:57 2021 -0400
committer Nicolas Capens <nicolascapens@google.com> Thu Sep 16 13:18:06 2021 +0000
tree 45d4a59cf1d9a3838a9506f86f265c8bf9d6607b
parent 9e3dca1e5848f2826473467a4e1242c263f3a7fa

Clamp depth before the depth test

Previously we clamped fragment depth right after interpolation, and
after shader execution for shader-provided depth. This means logically
it could be clamped twice, although only one of them would actually be
used. It was also confusing to understand where it was subsequently
consumed, and didn't closely follow the Vulkan spec which considers
depth clamp a part of the depth test (but, importantly, takes place
regardless of the value of depthTestEnable).

This change performs depth clamping right before the actual depth test.

Bug: b/185227903
Change-Id: I4307b9beef2e1a10606bf9accac5c6b86daa6d67
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/57008
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Sean Risser <srisser@google.com>

src/Pipeline/PixelProgram.cpp

diff --git a/src/Pipeline/PixelProgram.cpp b/src/Pipeline/PixelProgram.cpp
index 42ce095..9b321f9 100644
--- a/src/Pipeline/PixelProgram.cpp
+++ b/src/Pipeline/PixelProgram.cpp
@@ -245,11 +245,6 @@
 		for(unsigned int q : samples)
 		{
 			z[q] = routine.getVariable(it->second.Id)[it->second.FirstComponent];
-
-			if(state.depthClamp)
-			{
-				z[q] = Min(Max(z[q], Float4(state.minDepthClamp)), Float4(state.maxDepthClamp));
-			}
 		}
 	}
 }

src/Pipeline/PixelRoutine.cpp

diff --git a/src/Pipeline/PixelRoutine.cpp b/src/Pipeline/PixelRoutine.cpp
index 4d25a63..c24ca31 100644
--- a/src/Pipeline/PixelRoutine.cpp
+++ b/src/Pipeline/PixelRoutine.cpp
@@ -128,10 +128,6 @@
 				}
 
 				unclampedZ[q] = z[q];
-				if(state.depthClamp)
-				{
-					z[q] = Min(Max(z[q], Float4(state.minDepthClamp)), Float4(state.maxDepthClamp));
-				}
 			}
 		}
 
@@ -141,6 +137,7 @@
 		{
 			for(unsigned int q : samples)
 			{
+				z[q] = clampDepth(z[q]);
 				depthPass = depthPass || depthTest(zBuffer, q, x, z[q], sMask[q], zMask[q], cMask[q]);
 				depthBoundsTest(zBuffer, q, x, zMask[q], cMask[q]);
 			}
@@ -310,6 +307,7 @@
 				{
 					for(unsigned int q : samples)
 					{
+						z[q] = clampDepth(z[q]);
 						depthPass = depthPass || depthTest(zBuffer, q, x, z[q], sMask[q], zMask[q], cMask[q]);
 						depthBoundsTest(zBuffer, q, x, zMask[q], cMask[q]);
 					}
@@ -573,6 +571,16 @@
 	return zMask != 0;
 }
 
+Float4 PixelRoutine::clampDepth(const Float4 &z)
+{
+	if(!state.depthClamp)
+	{
+		return z;
+	}
+
+	return Min(Max(z, Float4(state.minDepthClamp)), Float4(state.maxDepthClamp));
+}
+
 Bool PixelRoutine::depthTest(const Pointer<Byte> &zBuffer, int q, const Int &x, const Float4 &z, const Int &sMask, Int &zMask, const Int &cMask)
 {
 	if(!state.depthTestActive)

src/Pipeline/PixelRoutine.hpp

diff --git a/src/Pipeline/PixelRoutine.hpp b/src/Pipeline/PixelRoutine.hpp
index c07689f..f1d618d 100644
--- a/src/Pipeline/PixelRoutine.hpp
+++ b/src/Pipeline/PixelRoutine.hpp
@@ -73,6 +73,7 @@
 	void stencilTest(Byte8 &value, VkCompareOp stencilCompareMode, bool isBack);
 	void stencilOperation(Byte8 &newValue, const Byte8 &bufferValue, const PixelProcessor::States::StencilOpState &ops, bool isBack, const Int &zMask, const Int &sMask);
 	void stencilOperation(Byte8 &output, const Byte8 &bufferValue, VkStencilOp operation, bool isBack);
+	Float4 clampDepth(const Float4 &z);
 	Bool depthTest(const Pointer<Byte> &zBuffer, int q, const Int &x, const Float4 &z, const Int &sMask, Int &zMask, const Int &cMask);
 	void depthBoundsTest(const Pointer<Byte> &zBuffer, int q, const Int &x, Int &zMask, Int &cMask);