Fix depth bias calculations
We were simply adding the constant depth bias to the z coordinate,
instead of interpreting it as relative to the minimum resolvable
difference. In the legacy OpenGL ES code stack we're multiplying it by
2^-23, but this assumes a 32-bit floating-point depth format, while
Vulkan must also support 16-bit integer depth, and doesn't take the
exponent of the z values of the polygon into account.
This change fixes the issue for both integer depth formats, which have a
constant minimum resolvable difference, and floating-point depth
formats, where the maximum exponent of the z values is used to determine
the minimum resolvable difference per polygon.
The viewport's depth range transform was corrected to be performed
before the depth bias offset.
Also the depth bias clamp operation was corrected to be performed only
on the bias value, not the polygon's z value for vertex 0.
We were also converting scalars to vectors a bit earlier than necessary,
so this was refactored to happen later to simplify the calculations.
The bias is added per fragment, after z interpolation, instead of being
added to the 'C' factor of the plane equation. That's because we compute
'y * B + C' per row, and then add 'x * A' per fragment. When the bias is
included in 'C', the additions cause loss of precision greater than the
minimum resolvable difference, and dEQP-GLES3.functional.polygon_offset
fails because it uses a 'units' depth bias of 1.
Note this change is intentionally unoptimized, to serve as a spec-
compliant reference implementation. dEQP has poor test coverage for this
functionality so we need to start off with something that is easy to
reference against the spec.
Bug: b/139341727
Fixes: b/160463658
Change-Id: Ief1dd609d8ac974e76b1b785a924b09b448297a2
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/48629
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/Device/Context.hpp b/src/Device/Context.hpp
index e4648dc..60c36b5 100644
--- a/src/Device/Context.hpp
+++ b/src/Device/Context.hpp
@@ -100,7 +100,7 @@
VkPolygonMode polygonMode;
VkLineRasterizationModeEXT lineRasterizationMode;
- float depthBias;
+ float constantDepthBias;
float slopeDepthBias;
float depthBiasClamp;
bool depthRangeUnrestricted;
diff --git a/src/Device/PixelProcessor.cpp b/src/Device/PixelProcessor.cpp
index 6665777..4691a01 100644
--- a/src/Device/PixelProcessor.cpp
+++ b/src/Device/PixelProcessor.cpp
@@ -116,6 +116,8 @@
state.depthCompareMode = context->depthCompareMode;
state.depthFormat = context->depthBuffer->getFormat();
+ state.depthBias = (context->constantDepthBias != 0.0f) || (context->slopeDepthBias != 0.0f);
+
// "For fixed-point depth buffers, fragment depth values are always limited to the range [0,1] by clamping after depth bias addition is performed.
// Unless the VK_EXT_depth_range_unrestricted extension is enabled, fragment depth values are clamped even when the depth buffer uses a floating-point representation."
state.depthClamp = !state.depthFormat.isFloatFormat() || !context->depthRangeUnrestricted;
diff --git a/src/Device/PixelProcessor.hpp b/src/Device/PixelProcessor.hpp
index b2656aa..16b879c 100644
--- a/src/Device/PixelProcessor.hpp
+++ b/src/Device/PixelProcessor.hpp
@@ -93,6 +93,7 @@
bool centroid;
VkFrontFace frontFace;
vk::Format depthFormat;
+ bool depthBias;
bool depthClamp;
};
diff --git a/src/Device/Primitive.hpp b/src/Device/Primitive.hpp
index fcacdb8..595a2f2 100644
--- a/src/Device/Primitive.hpp
+++ b/src/Device/Primitive.hpp
@@ -69,6 +69,7 @@
float pointSizeInv;
PlaneEquation z;
+ float4 zBias;
PlaneEquation w;
PlaneEquation V[MAX_INTERFACE_COMPONENTS];
diff --git a/src/Device/Renderer.cpp b/src/Device/Renderer.cpp
index d7b3b3f..262446a 100644
--- a/src/Device/Renderer.cpp
+++ b/src/Device/Renderer.cpp
@@ -330,11 +330,6 @@
float Z = F - N;
constexpr float subPixF = vk::SUBPIXEL_PRECISION_FACTOR;
- if(context->isDrawTriangle(false))
- {
- N += context->depthBias;
- }
-
data->WxF = float4(W * subPixF);
data->HxF = float4(H * subPixF);
data->X0xF = float4(X0 * subPixF - subPixF / 2);
@@ -342,10 +337,27 @@
data->halfPixelX = float4(0.5f / W);
data->halfPixelY = float4(0.5f / H);
data->viewportHeight = abs(viewport.height);
- data->slopeDepthBias = context->slopeDepthBias;
- data->depthBiasClamp = context->depthBiasClamp;
data->depthRange = Z;
data->depthNear = N;
+ data->constantDepthBias = context->constantDepthBias;
+ data->slopeDepthBias = context->slopeDepthBias;
+ data->depthBiasClamp = context->depthBiasClamp;
+
+ if(context->depthBuffer)
+ {
+ switch(context->depthBuffer->getFormat(VK_IMAGE_ASPECT_DEPTH_BIT))
+ {
+ case VK_FORMAT_D16_UNORM:
+ data->minimumResolvableDepthDifference = 1.0f / 0xFFFF;
+ break;
+ case VK_FORMAT_D32_SFLOAT:
+ // The minimum resolvable depth difference is determined per-polygon for floating-point depth
+ // buffers. DrawData::minimumResolvableDepthDifference is unused.
+ break;
+ default:
+ UNSUPPORTED("Depth format: %d", int(context->depthBuffer->getFormat(VK_IMAGE_ASPECT_DEPTH_BIT)));
+ }
+ }
}
// Target
diff --git a/src/Device/Renderer.hpp b/src/Device/Renderer.hpp
index 3685b29..ab19a8b 100644
--- a/src/Device/Renderer.hpp
+++ b/src/Device/Renderer.hpp
@@ -87,9 +87,11 @@
float4 halfPixelX;
float4 halfPixelY;
float viewportHeight;
- float slopeDepthBias;
float depthRange;
float depthNear;
+ float minimumResolvableDepthDifference;
+ float constantDepthBias;
+ float slopeDepthBias;
float depthBiasClamp;
unsigned int *colorBuffer[RENDERTARGETS];
diff --git a/src/Device/SetupProcessor.cpp b/src/Device/SetupProcessor.cpp
index ad588b7..b5ff120 100644
--- a/src/Device/SetupProcessor.cpp
+++ b/src/Device/SetupProcessor.cpp
@@ -22,6 +22,7 @@
#include "Pipeline/SetupRoutine.hpp"
#include "Pipeline/SpirvShader.hpp"
#include "System/Debug.hpp"
+#include "Vulkan/VkImageView.hpp"
#include <cstring>
@@ -64,6 +65,8 @@
state.isDrawPoint = context->isDrawPoint(true);
state.isDrawLine = context->isDrawLine(true);
state.isDrawTriangle = context->isDrawTriangle(true);
+ state.fixedPointDepthBuffer = context->depthBuffer && !context->depthBuffer->getFormat(VK_IMAGE_ASPECT_DEPTH_BIT).isFloatFormat();
+ state.applyConstantDepthBias = context->isDrawTriangle(false) && (context->constantDepthBias != 0.0f);
state.applySlopeDepthBias = context->isDrawTriangle(false) && (context->slopeDepthBias != 0.0f);
state.applyDepthBiasClamp = context->isDrawTriangle(false) && (context->depthBiasClamp != 0.0f);
state.interpolateZ = context->depthBufferActive() || vPosZW;
diff --git a/src/Device/SetupProcessor.hpp b/src/Device/SetupProcessor.hpp
index 50290b6..de3f886 100644
--- a/src/Device/SetupProcessor.hpp
+++ b/src/Device/SetupProcessor.hpp
@@ -48,6 +48,8 @@
bool isDrawPoint : 1;
bool isDrawLine : 1;
bool isDrawTriangle : 1;
+ bool fixedPointDepthBuffer : 1;
+ bool applyConstantDepthBias : 1;
bool applySlopeDepthBias : 1;
bool applyDepthBiasClamp : 1;
bool interpolateZ : 1;
diff --git a/src/Pipeline/PixelRoutine.cpp b/src/Pipeline/PixelRoutine.cpp
index dc27e90..9fbfe80 100644
--- a/src/Pipeline/PixelRoutine.cpp
+++ b/src/Pipeline/PixelRoutine.cpp
@@ -92,6 +92,11 @@
z[q] = interpolate(x, Dz[q], z[q], primitive + OFFSET(Primitive, z), false, false);
+ if(state.depthBias)
+ {
+ z[q] += *Pointer<Float4>(primitive + OFFSET(Primitive, zBias), 16);
+ }
+
if(state.depthClamp)
{
z[q] = Min(Max(z[q], Float4(0.0f)), Float4(1.0f));
diff --git a/src/Pipeline/SetupRoutine.cpp b/src/Pipeline/SetupRoutine.cpp
index ddfdef3..5ed9b9c 100644
--- a/src/Pipeline/SetupRoutine.cpp
+++ b/src/Pipeline/SetupRoutine.cpp
@@ -403,9 +403,9 @@
z1 -= z0;
z2 -= z0;
- Float4 A;
- Float4 B;
- Float4 C;
+ Float A;
+ Float B;
+ Float C;
if(!point)
{
@@ -416,41 +416,80 @@
Float D = *Pointer<Float>(data + OFFSET(DrawData, depthRange)) / (x1 * y2 - x2 * y1);
- Float a = (y2 * z1 - y1 * z2) * D;
- Float b = (x1 * z2 - x2 * z1) * D;
-
- A = Float4(a);
- B = Float4(b);
+ A = (y2 * z1 - y1 * z2) * D;
+ B = (x1 * z2 - x2 * z1) * D;
}
else
{
- A = Float4(0, 0, 0, 0);
- B = Float4(0, 0, 0, 0);
+ A = 0.0f;
+ B = 0.0f;
}
- *Pointer<Float4>(primitive + OFFSET(Primitive, z.A), 16) = A;
- *Pointer<Float4>(primitive + OFFSET(Primitive, z.B), 16) = B;
+ C = z0 * *Pointer<Float>(data + OFFSET(DrawData, depthRange)) + *Pointer<Float>(data + OFFSET(DrawData, depthNear));
- Float c = z0;
+ *Pointer<Float4>(primitive + OFFSET(Primitive, z.A), 16) = Float4(A);
+ *Pointer<Float4>(primitive + OFFSET(Primitive, z.B), 16) = Float4(B);
+ *Pointer<Float4>(primitive + OFFSET(Primitive, z.C), 16) = Float4(C);
+
+ Float bias = 0.0f;
+
+ if(state.applyConstantDepthBias)
+ {
+ Float r; // Minimum resolvable difference
+
+ if(state.fixedPointDepthBuffer)
+ {
+ // TODO(b/139341727): Pre-multiply the constant depth bias factor by the minimum
+ // resolvable difference.
+
+ // TODO(b/139341727): When there's a fixed-point depth buffer and no depth bias clamp,
+ // the constant depth bias factor could be added to 'depthNear', eliminating the per-
+ // polygon addition.
+
+ r = *Pointer<Float>(data + OFFSET(DrawData, minimumResolvableDepthDifference));
+ }
+ else // Floating-point depth buffer
+ {
+ // "For floating-point depth buffers, there is no single minimum resolvable difference.
+ // In this case, the minimum resolvable difference for a given polygon is dependent on
+ // the maximum exponent, e, in the range of z values spanned by the primitive. If n is
+ // the number of bits in the floating-point mantissa, the minimum resolvable difference,
+ // r, for the given primitive is defined as r = 2^(e-n)."
+
+ Float Z0 = C;
+ Float Z1 = z1 * *Pointer<Float>(data + OFFSET(DrawData, depthRange)) + *Pointer<Float>(data + OFFSET(DrawData, depthNear));
+ Float Z2 = z2 * *Pointer<Float>(data + OFFSET(DrawData, depthRange)) + *Pointer<Float>(data + OFFSET(DrawData, depthNear));
+
+ Int e0 = As<Int>(Z0) & 0x7F800000;
+ Int e1 = As<Int>(Z1) & 0x7F800000;
+ Int e2 = As<Int>(Z2) & 0x7F800000;
+
+ Int e = Max(Max(e0, e1), e2);
+
+ r = As<Float>(e - (23 << 23));
+ }
+
+ bias = r * *Pointer<Float>(data + OFFSET(DrawData, constantDepthBias));
+ }
if(state.applySlopeDepthBias)
{
- Float bias = Max(Abs(Float(A.x)), Abs(Float(B.x)));
- bias *= *Pointer<Float>(data + OFFSET(DrawData, slopeDepthBias));
+ Float m = Max(Abs(A), Abs(B));
- c += bias;
+ bias += m * *Pointer<Float>(data + OFFSET(DrawData, slopeDepthBias)); // TODO(b/155302798): Optimize 0 += x;
}
- C = Float4(c * *Pointer<Float>(data + OFFSET(DrawData, depthRange)) + *Pointer<Float>(data + OFFSET(DrawData, depthNear)));
-
- if(state.applyDepthBiasClamp)
+ if(state.applyConstantDepthBias || state.applySlopeDepthBias)
{
- Float clamp = *Pointer<Float>(data + OFFSET(DrawData, depthBiasClamp));
- Float4 clamp4 = Float4(clamp);
- C = IfThenElse(clamp > 0.0f, Min(clamp4, C), Max(clamp4, C));
- }
+ if(state.applyDepthBiasClamp)
+ {
+ Float clamp = *Pointer<Float>(data + OFFSET(DrawData, depthBiasClamp));
- *Pointer<Float4>(primitive + OFFSET(Primitive, z.C), 16) = C;
+ bias = IfThenElse(clamp > 0.0f, Min(bias, clamp), Max(bias, clamp));
+ }
+
+ *Pointer<Float4>(primitive + OFFSET(Primitive, zBias), 16) = Float4(bias);
+ }
}
for(int interpolant = 0; interpolant < MAX_INTERFACE_COMPONENTS; interpolant++)
diff --git a/src/Pipeline/ShaderCore.cpp b/src/Pipeline/ShaderCore.cpp
index f0d6ee4..b010131 100644
--- a/src/Pipeline/ShaderCore.cpp
+++ b/src/Pipeline/ShaderCore.cpp
@@ -224,6 +224,8 @@
Float4 reciprocal(RValue<Float4> x, bool pp, bool finite, bool exactAtPow2)
{
+ //return Float4(1.0f) / x;
+
Float4 rcp = Rcp_pp(x, exactAtPow2);
if(!pp)
diff --git a/src/Reactor/Reactor.cpp b/src/Reactor/Reactor.cpp
index d09ddfe..97053dc 100644
--- a/src/Reactor/Reactor.cpp
+++ b/src/Reactor/Reactor.cpp
@@ -3835,6 +3835,11 @@
store(argument.rvalue());
}
+RValue<Float> Float::operator=(float rhs)
+{
+ return RValue<Float>(storeValue(Nucleus::createConstantFloat(rhs)));
+}
+
RValue<Float> Float::operator=(RValue<Float> rhs)
{
return store(rhs);
diff --git a/src/Reactor/Reactor.hpp b/src/Reactor/Reactor.hpp
index 28730b4..b943920 100644
--- a/src/Reactor/Reactor.hpp
+++ b/src/Reactor/Reactor.hpp
@@ -2096,7 +2096,7 @@
template<int T>
Float(const SwizzleMask1<Float4, T> &rhs);
- // RValue<Float> operator=(float rhs); // FIXME: Implement
+ RValue<Float> operator=(float rhs);
RValue<Float> operator=(RValue<Float> rhs);
RValue<Float> operator=(const Float &rhs);
RValue<Float> operator=(const Reference<Float> &rhs);
diff --git a/src/Vulkan/VkCommandBuffer.cpp b/src/Vulkan/VkCommandBuffer.cpp
index e140825..cff9139 100644
--- a/src/Vulkan/VkCommandBuffer.cpp
+++ b/src/Vulkan/VkCommandBuffer.cpp
@@ -547,7 +547,7 @@
if(pipeline->hasDynamicState(VK_DYNAMIC_STATE_DEPTH_BIAS))
{
- context.depthBias = executionState.dynamicState.depthBiasConstantFactor;
+ context.constantDepthBias = executionState.dynamicState.depthBiasConstantFactor;
context.slopeDepthBias = executionState.dynamicState.depthBiasSlopeFactor;
context.depthBiasClamp = executionState.dynamicState.depthBiasClamp;
}
diff --git a/src/Vulkan/VkPipeline.cpp b/src/Vulkan/VkPipeline.cpp
index 7cb2a2e..86913ec 100644
--- a/src/Vulkan/VkPipeline.cpp
+++ b/src/Vulkan/VkPipeline.cpp
@@ -250,7 +250,7 @@
context.cullMode = rasterizationState->cullMode;
context.frontFace = rasterizationState->frontFace;
context.polygonMode = rasterizationState->polygonMode;
- context.depthBias = (rasterizationState->depthBiasEnable != VK_FALSE) ? rasterizationState->depthBiasConstantFactor : 0.0f;
+ context.constantDepthBias = (rasterizationState->depthBiasEnable != VK_FALSE) ? rasterizationState->depthBiasConstantFactor : 0.0f;
context.slopeDepthBias = (rasterizationState->depthBiasEnable != VK_FALSE) ? rasterizationState->depthBiasSlopeFactor : 0.0f;
context.depthBiasClamp = (rasterizationState->depthBiasEnable != VK_FALSE) ? rasterizationState->depthBiasClamp : 0.0f;
context.depthRangeUnrestricted = device->hasExtension(VK_EXT_DEPTH_RANGE_UNRESTRICTED_EXTENSION_NAME);
