Refactor sample shading state determination
Previously all logic for determining the actual state for sample shading
was near the top of the PixelRoutine::quad() method, with affected
readability. This change makes us handle the case where SampleId or
SamplePosition decorations are present in the fragment shader in the
PixelProcessor class, which overrides sampleShadingEnabled and
minSampleShading, since this is implementation-independent behavior.
The implementation-dependent behavior of doing per-sample shading even
if the effective minSampleShading isn't 1.0, is now handled in the
PixelRoutine constructor. Other invariant state is also determined in
the constructor.
Bug: b/185227903
Bug: b/194521425
Change-Id: I341dde391a41a0d2b454f9dad4180067ad4dc4e3
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/56049
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>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Device/PixelProcessor.cpp b/src/Device/PixelProcessor.cpp
index 1990fff..de940fa 100644
--- a/src/Device/PixelProcessor.cpp
+++ b/src/Device/PixelProcessor.cpp
@@ -154,8 +154,26 @@
state.multiSampleMask = pipelineState.getMultiSampleMask();
state.enableMultiSampling = (state.multiSampleCount > 1) &&
!(pipelineState.isDrawLine(true) && (pipelineState.getLineRasterizationMode() == VK_LINE_RASTERIZATION_MODE_BRESENHAM_EXT));
- state.sampleShadingEnabled = pipelineState.hasSampleShadingEnabled();
- state.minSampleShading = pipelineState.getMinSampleShading();
+
+ // SampleId and SamplePosition require per-sample fragment shader invocations, so the Vulkan spec
+ // requires turning on sample shading if either of them is present in the shader:
+ // "If a fragment shader entry point's interface includes an input variable decorated with SampleId,
+ // Sample Shading is considered enabled with a minSampleShading value of 1.0."
+ // "If a fragment shader entry point's interface includes an input variable decorated with SamplePosition,
+ // Sample Shading is considered enabled with a minSampleShading value of 1.0."
+ bool shaderContainsSampleDecoration = fragmentShader && (fragmentShader->hasBuiltinInput(spv::BuiltInSampleId) ||
+ fragmentShader->hasBuiltinInput(spv::BuiltInSamplePosition));
+
+ if(shaderContainsSampleDecoration)
+ {
+ state.sampleShadingEnabled = true;
+ state.minSampleShading = 1.0f;
+ }
+ else
+ {
+ state.sampleShadingEnabled = pipelineState.hasSampleShadingEnabled();
+ state.minSampleShading = pipelineState.getMinSampleShading();
+ }
if(state.enableMultiSampling && fragmentShader)
{
diff --git a/src/Pipeline/PixelRoutine.cpp b/src/Pipeline/PixelRoutine.cpp
index 3e6ba99..543b1c9 100644
--- a/src/Pipeline/PixelRoutine.cpp
+++ b/src/Pipeline/PixelRoutine.cpp
@@ -32,6 +32,11 @@
: QuadRasterizer(state, spirvShader)
, routine(pipelineLayout)
, descriptorSets(descriptorSets)
+ , shaderContainsInterpolation(spirvShader && spirvShader->getUsedCapabilities().InterpolationFunction)
+ , shaderContainsSampleQualifier(spirvShader && spirvShader->getModes().ContainsSampleQualifier)
+ , perSampleShading((state.sampleShadingEnabled && (state.minSampleShading * state.multiSampleCount > 1.0f)) ||
+ shaderContainsSampleQualifier || shaderContainsInterpolation) // TODO(b/194714095)
+ , invocationCount(perSampleShading ? state.multiSampleCount : 1)
{
if(spirvShader)
{
@@ -63,34 +68,9 @@
Int sMask[4]; // Stencil mask
Float4 unclampedZ[4];
- bool sampleShadingEnabled = state.sampleShadingEnabled;
- float minSampleShading = state.minSampleShading;
- if(spirvShader)
+ for(int invocation = 0; invocation < invocationCount; invocation++)
{
- // SampleId and SamplePosition built-ins require the sampleRateShading feature, so the Vulkan spec
- // requires turning on per sample shading if either of them is present in the shader.
-
- // "If a fragment shader entry point's interface includes an input variable decorated with SampleId,
- // Sample Shading is considered enabled with a minSampleShading value of 1.0."
-
- // "If a fragment shader entry point's interface includes an input variable decorated with SamplePosition,
- // Sample Shading is considered enabled with a minSampleShading value of 1.0."
- if(spirvShader->hasBuiltinInput(spv::BuiltInSampleId) || spirvShader->hasBuiltinInput(spv::BuiltInSamplePosition))
- {
- sampleShadingEnabled = true;
- minSampleShading = 1.0f;
- }
- }
-
- bool shaderContainsInterpolation = spirvShader && spirvShader->getUsedCapabilities().InterpolationFunction;
- bool shaderContainsSampleQualifier = spirvShader && spirvShader->getModes().ContainsSampleQualifier;
- bool perSampleShading = (sampleShadingEnabled && (minSampleShading > 0.0f)) ||
- shaderContainsInterpolation || shaderContainsSampleQualifier;
- unsigned int numSampleRenders = perSampleShading ? state.multiSampleCount : 1;
-
- for(unsigned int i = 0; i < numSampleRenders; ++i)
- {
- int sampleId = perSampleShading ? i : -1;
+ int sampleId = perSampleShading ? invocation : -1;
unsigned int sampleLoopInit = perSampleShading ? sampleId : 0;
unsigned int sampleLoopEnd = perSampleShading ? sampleId + 1 : state.multiSampleCount;
@@ -155,7 +135,7 @@
Float4 XXXX = Float4(0.0f);
Float4 YYYY = Float4(0.0f);
- if(state.centroid || shaderContainsInterpolation)
+ if(state.centroid || shaderContainsInterpolation) // TODO(b/194714095)
{
Float4 WWWW(1.0e-9f);
@@ -179,7 +159,7 @@
w = interpolate(xxxx, Dw, rhw, primitive + OFFSET(Primitive, w), false, false);
rhw = reciprocal(w, false, false, true);
- if(state.centroid || shaderContainsInterpolation)
+ if(state.centroid || shaderContainsInterpolation) // TODO(b/194714095)
{
rhwCentroid = reciprocal(SpirvRoutine::interpolateAtXY(XXXX, YYYY, rhwCentroid, primitive + OFFSET(Primitive, w), false, false));
}
@@ -187,7 +167,7 @@
if(spirvShader)
{
- if(shaderContainsInterpolation)
+ if(shaderContainsInterpolation) // TODO(b/194714095)
{
routine.interpolationData.primitive = primitive;
diff --git a/src/Pipeline/PixelRoutine.hpp b/src/Pipeline/PixelRoutine.hpp
index 800f708..fd2f9a1 100644
--- a/src/Pipeline/PixelRoutine.hpp
+++ b/src/Pipeline/PixelRoutine.hpp
@@ -96,6 +96,12 @@
Int4 depthBoundsTest32F(const Pointer<Byte> &zBuffer, int q, const Int &x);
Int4 depthBoundsTest16(const Pointer<Byte> &zBuffer, int q, const Int &x);
+
+ // Derived state parameters
+ const bool shaderContainsInterpolation; // TODO(b/194714095)
+ const bool shaderContainsSampleQualifier;
+ const bool perSampleShading;
+ const int invocationCount;
};
} // namespace sw
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 900199b..d08925f 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -580,6 +580,9 @@
bool DepthGreater : 1;
bool DepthLess : 1;
bool DepthUnchanged : 1;
+
+ // TODO(b/177839655): These are not SPIR-V execution modes.
+ // Move to an Analysis structure.
bool ContainsKill : 1;
bool ContainsControlBarriers : 1;
bool NeedsCentroid : 1;
