Commit 0aca3ca9 by Nicolas Capens Committed by Nicolas Capens

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: 's avatarNicolas Capens <nicolascapens@google.com> Kokoro-Result: kokoro <noreply+kokoro@google.com> Reviewed-by: 's avatarAntonio Maiorano <amaiorano@google.com>
parent e8ce1f83
......@@ -100,7 +100,7 @@ public:
VkPolygonMode polygonMode;
VkLineRasterizationModeEXT lineRasterizationMode;
float depthBias;
float constantDepthBias;
float slopeDepthBias;
float depthBiasClamp;
bool depthRangeUnrestricted;
......
......@@ -116,6 +116,8 @@ const PixelProcessor::State PixelProcessor::update(const Context *context) const
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;
......
......@@ -93,6 +93,7 @@ public:
bool centroid;
VkFrontFace frontFace;
vk::Format depthFormat;
bool depthBias;
bool depthClamp;
};
......
......@@ -69,6 +69,7 @@ struct Primitive MEMORY_SANITIZER_ONLY(
float pointSizeInv;
PlaneEquation z;
float4 zBias;
PlaneEquation w;
PlaneEquation V[MAX_INTERFACE_COMPONENTS];
......
......@@ -330,11 +330,6 @@ void Renderer::draw(const sw::Context *context, VkIndexType indexType, unsigned
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 @@ void Renderer::draw(const sw::Context *context, VkIndexType indexType, unsigned
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
......
......@@ -87,9 +87,11 @@ struct DrawData
float4 halfPixelX;
float4 halfPixelY;
float viewportHeight;
float slopeDepthBias;
float depthRange;
float depthNear;
float minimumResolvableDepthDifference;
float constantDepthBias;
float slopeDepthBias;
float depthBiasClamp;
unsigned int *colorBuffer[RENDERTARGETS];
......
......@@ -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 @@ SetupProcessor::State SetupProcessor::update(const sw::Context *context) const
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;
......
......@@ -48,6 +48,8 @@ public:
bool isDrawPoint : 1;
bool isDrawLine : 1;
bool isDrawTriangle : 1;
bool fixedPointDepthBuffer : 1;
bool applyConstantDepthBias : 1;
bool applySlopeDepthBias : 1;
bool applyDepthBiasClamp : 1;
bool interpolateZ : 1;
......
......@@ -92,6 +92,11 @@ void PixelRoutine::quad(Pointer<Byte> cBuffer[RENDERTARGETS], Pointer<Byte> &zBu
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));
......
......@@ -403,9 +403,9 @@ void SetupRoutine::generate()
z1 -= z0;
z2 -= z0;
Float4 A;
Float4 B;
Float4 C;
Float A;
Float B;
Float C;
if(!point)
{
......@@ -416,41 +416,80 @@ void SetupRoutine::generate()
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);
if(state.applySlopeDepthBias)
Float bias = 0.0f;
if(state.applyConstantDepthBias)
{
Float bias = Max(Abs(Float(A.x)), Abs(Float(B.x)));
bias *= *Pointer<Float>(data + OFFSET(DrawData, slopeDepthBias));
Float r; // Minimum resolvable difference
c += bias;
}
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.
C = Float4(c * *Pointer<Float>(data + OFFSET(DrawData, depthRange)) + *Pointer<Float>(data + OFFSET(DrawData, depthNear)));
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));
if(state.applyDepthBiasClamp)
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 clamp = *Pointer<Float>(data + OFFSET(DrawData, depthBiasClamp));
Float4 clamp4 = Float4(clamp);
C = IfThenElse(clamp > 0.0f, Min(clamp4, C), Max(clamp4, C));
Float m = Max(Abs(A), Abs(B));
bias += m * *Pointer<Float>(data + OFFSET(DrawData, slopeDepthBias)); // TODO(b/155302798): Optimize 0 += x;
}
*Pointer<Float4>(primitive + OFFSET(Primitive, z.C), 16) = C;
if(state.applyConstantDepthBias || state.applySlopeDepthBias)
{
if(state.applyDepthBiasClamp)
{
Float clamp = *Pointer<Float>(data + OFFSET(DrawData, depthBiasClamp));
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++)
......
......@@ -224,6 +224,8 @@ Float4 power(RValue<Float4> x, RValue<Float4> y, bool pp)
Float4 reciprocal(RValue<Float4> x, bool pp, bool finite, bool exactAtPow2)
{
//return Float4(1.0f) / x;
Float4 rcp = Rcp_pp(x, exactAtPow2);
if(!pp)
......
......@@ -3835,6 +3835,11 @@ Float::Float(Argument<Float> argument)
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);
......
......@@ -2096,7 +2096,7 @@ public:
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);
......
......@@ -547,7 +547,7 @@ public:
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;
}
......
......@@ -250,7 +250,7 @@ GraphicsPipeline::GraphicsPipeline(const VkGraphicsPipelineCreateInfo *pCreateIn
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);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment