Skip depth bounds test when there is no depth attachment
The Vulkan spec states that "If the depth bounds test is disabled, or if
there is no depth attachment, the coverage mask is unmodified by this
operation."
Previously the depth bounds test was performed even when there was no
depth attachment. We were also not obtaining the depth buffer format
unless the regular depth test was enabled.
This was discovered while refactoring to assert that the depth buffer
had one of the expected formats, but it was VK_FORMAT_UNDEFINED in tests
like e.g. dEQP-VK.dynamic_state.ds_state.depth_bounds_1.
Bug: b/181656417
Bug: b/185227903
Change-Id: Ic0a06b668c85eee7565a9007f7efb77621f3f7ac
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/57069
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Sean Risser <srisser@google.com>
diff --git a/src/Device/Context.cpp b/src/Device/Context.cpp
index 28ef706..355e3a4 100644
--- a/src/Device/Context.cpp
+++ b/src/Device/Context.cpp
@@ -160,6 +160,18 @@
}
}
+VkFormat Attachments::depthFormat() const
+{
+ if(depthBuffer)
+ {
+ return depthBuffer->getFormat();
+ }
+ else
+ {
+ return VK_FORMAT_UNDEFINED;
+ }
+}
+
Inputs::Inputs(const VkPipelineVertexInputStateCreateInfo *vertexInputState)
{
if(vertexInputState->flags != 0)
@@ -467,7 +479,7 @@
minDepthBounds = depthStencilState->minDepthBounds;
maxDepthBounds = depthStencilState->maxDepthBounds;
- depthBufferEnable = (depthStencilState->depthTestEnable != VK_FALSE);
+ depthTestEnable = (depthStencilState->depthTestEnable != VK_FALSE);
depthWriteEnable = (depthStencilState->depthWriteEnable != VK_FALSE);
depthCompareMode = depthStencilState->depthCompareOp;
@@ -582,14 +594,13 @@
bool GraphicsState::depthWriteActive(const Attachments &attachments) const
{
- if(!depthBufferActive(attachments)) return false;
-
- return depthWriteEnable;
+ // "Depth writes are always disabled when depthTestEnable is VK_FALSE."
+ return depthTestActive(attachments) && depthWriteEnable;
}
-bool GraphicsState::depthBufferActive(const Attachments &attachments) const
+bool GraphicsState::depthTestActive(const Attachments &attachments) const
{
- return attachments.depthBuffer && depthBufferEnable;
+ return attachments.depthBuffer && depthTestEnable;
}
bool GraphicsState::stencilActive(const Attachments &attachments) const
@@ -597,9 +608,9 @@
return attachments.stencilBuffer && stencilEnable;
}
-bool GraphicsState::depthBoundsTestActive() const
+bool GraphicsState::depthBoundsTestActive(const Attachments &attachments) const
{
- return depthBoundsTestEnable;
+ return attachments.depthBuffer && depthBoundsTestEnable;
}
const GraphicsState GraphicsState::combineStates(const DynamicState &dynamicState) const
diff --git a/src/Device/Context.hpp b/src/Device/Context.hpp
index c85475b..3bdf670 100644
--- a/src/Device/Context.hpp
+++ b/src/Device/Context.hpp
@@ -57,6 +57,7 @@
bool isColorClamped(int index) const;
VkFormat colorFormat(int index) const;
+ VkFormat depthFormat() const;
};
struct Inputs
@@ -186,9 +187,9 @@
int colorWriteActive(int index, const Attachments &attachments) const;
bool depthWriteActive(const Attachments &attachments) const;
- bool depthBufferActive(const Attachments &attachments) const;
+ bool depthTestActive(const Attachments &attachments) const;
bool stencilActive(const Attachments &attachments) const;
- bool depthBoundsTestActive() const;
+ bool depthBoundsTestActive(const Attachments &attachments) const;
private:
inline bool hasDynamicState(VkDynamicState dynamicState) const { return (dynamicStateFlags & (1 << dynamicState)) != 0; }
@@ -231,7 +232,7 @@
// Pixel processor states
bool rasterizerDiscard = false;
bool depthBoundsTestEnable = false;
- bool depthBufferEnable = false;
+ bool depthTestEnable = false;
VkCompareOp depthCompareMode = VK_COMPARE_OP_NEVER;
bool depthWriteEnable = false;
bool depthClampEnable = false;
diff --git a/src/Device/PixelProcessor.cpp b/src/Device/PixelProcessor.cpp
index 6137722..617c0ad 100644
--- a/src/Device/PixelProcessor.cpp
+++ b/src/Device/PixelProcessor.cpp
@@ -110,11 +110,15 @@
state.backStencil = pipelineState.getBackStencil();
}
- if(pipelineState.depthBufferActive(attachments))
+ state.depthFormat = attachments.depthFormat();
+ state.depthBoundsTestActive = pipelineState.depthBoundsTestActive(attachments);
+ state.minDepthBounds = pipelineState.getMinDepthBounds();
+ state.maxDepthBounds = pipelineState.getMaxDepthBounds();
+
+ if(pipelineState.depthTestActive(attachments))
{
state.depthTestActive = true;
state.depthCompareMode = pipelineState.getDepthCompareMode();
- state.depthFormat = attachments.depthBuffer->getFormat();
state.depthBias = (pipelineState.getConstantDepthBias() != 0.0f) || (pipelineState.getSlopeDepthBias() != 0.0f);
@@ -136,10 +140,6 @@
}
}
- state.depthBoundsTestActive = pipelineState.depthBoundsTestActive();
- state.minDepthBounds = pipelineState.getMinDepthBounds();
- state.maxDepthBounds = pipelineState.getMaxDepthBounds();
-
state.occlusionEnabled = occlusionEnabled;
bool fragmentContainsKill = (fragmentShader && fragmentShader->getAnalysis().ContainsKill);
diff --git a/src/Device/SetupProcessor.cpp b/src/Device/SetupProcessor.cpp
index de13fb4..e9aa3c8 100644
--- a/src/Device/SetupProcessor.cpp
+++ b/src/Device/SetupProcessor.cpp
@@ -68,7 +68,7 @@
state.applyConstantDepthBias = pipelineState.isDrawTriangle(false) && (pipelineState.getConstantDepthBias() != 0.0f);
state.applySlopeDepthBias = pipelineState.isDrawTriangle(false) && (pipelineState.getSlopeDepthBias() != 0.0f);
state.applyDepthBiasClamp = pipelineState.isDrawTriangle(false) && (pipelineState.getDepthBiasClamp() != 0.0f);
- state.interpolateZ = pipelineState.depthBufferActive(attachments) || vPosZW;
+ state.interpolateZ = pipelineState.depthTestActive(attachments) || vPosZW;
state.interpolateW = fragmentShader != nullptr;
state.frontFace = pipelineState.getFrontFace();
state.cullMode = pipelineState.getCullMode();
diff --git a/src/Pipeline/PixelRoutine.cpp b/src/Pipeline/PixelRoutine.cpp
index ef9bd1c..4d25a63 100644
--- a/src/Pipeline/PixelRoutine.cpp
+++ b/src/Pipeline/PixelRoutine.cpp
@@ -580,13 +580,16 @@
return true;
}
- if(state.depthFormat == VK_FORMAT_D16_UNORM)
+ switch(state.depthFormat)
{
+ case VK_FORMAT_D16_UNORM:
return depthTest16(zBuffer, q, x, z, sMask, zMask, cMask);
- }
- else
- {
+ case VK_FORMAT_D32_SFLOAT:
+ case VK_FORMAT_D32_SFLOAT_S8_UINT:
return depthTest32F(zBuffer, q, x, z, sMask, zMask, cMask);
+ default:
+ UNSUPPORTED("Depth format: %d", int(state.depthFormat));
+ return false;
}
}
@@ -627,18 +630,33 @@
void PixelRoutine::depthBoundsTest(const Pointer<Byte> &zBuffer, int q, const Int &x, Int &zMask, Int &cMask)
{
- if(state.depthBoundsTestActive)
+ if(!state.depthBoundsTestActive)
{
- Int4 zTest = (state.depthFormat == VK_FORMAT_D16_UNORM) ? depthBoundsTest16(zBuffer, q, x) : depthBoundsTest32F(zBuffer, q, x);
+ return;
+ }
- if(!state.depthTestActive)
- {
- cMask &= zMask & SignMask(zTest);
- }
- else
- {
- zMask &= cMask & SignMask(zTest);
- }
+ Int4 zTest;
+ switch(state.depthFormat)
+ {
+ case VK_FORMAT_D16_UNORM:
+ zTest = depthBoundsTest16(zBuffer, q, x);
+ break;
+ case VK_FORMAT_D32_SFLOAT:
+ case VK_FORMAT_D32_SFLOAT_S8_UINT:
+ zTest = depthBoundsTest32F(zBuffer, q, x);
+ break;
+ default:
+ UNSUPPORTED("Depth format: %d", int(state.depthFormat));
+ break;
+ }
+
+ if(!state.depthTestActive)
+ {
+ cMask &= zMask & SignMask(zTest);
+ }
+ else
+ {
+ zMask &= cMask & SignMask(zTest);
}
}
@@ -723,17 +741,19 @@
for(unsigned int q : samples)
{
- if(state.depthFormat == VK_FORMAT_D16_UNORM)
+ switch(state.depthFormat)
{
+ case VK_FORMAT_D16_UNORM:
writeDepth16(zBuffer, q, x, z[q], zMask[q]);
- }
- else if(state.depthFormat == VK_FORMAT_D32_SFLOAT ||
- state.depthFormat == VK_FORMAT_D32_SFLOAT_S8_UINT)
- {
+ break;
+ case VK_FORMAT_D32_SFLOAT:
+ case VK_FORMAT_D32_SFLOAT_S8_UINT:
writeDepth32F(zBuffer, q, x, z[q], zMask[q]);
- }
- else
+ break;
+ default:
UNSUPPORTED("Depth format: %d", int(state.depthFormat));
+ break;
+ }
}
}
