Skip to content

S
swiftshader
Commit 4d65934f by Chris Forbes
Options

Fix FP blending path to not make clamping assumptions

We had assumed in Context in various places that negative pixel values would be clamped to zero, which is incorrect for floating-point color attachments. TODO: on the assumption that there is value in these blend optimizations themselves, we should split the context blend state to be per-attachment. This would give us the `independentBlend` optional feature, and also allow us to keep the optimization for those attachments which do have clamping in a mixed MRT scenario. Bug: b/132433217 Test: dEQP-VK.pipeline.blend.format.r16* Change-Id: I5434475aba5d21a8c8342b42cccc575b517f0900 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/31050Tested-by: 's avatarChris Forbes <chrisforbes@google.com> Presubmit-Ready: Chris Forbes <chrisforbes@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com>
parent b1665554
Showing with 37 additions and 31 deletions
src/Device/Context.cpp
...@@ -324,6 +324,20 @@ namespace sw ...@@ -324,6 +324,20 @@ namespace sw
return destBlendFactorState; return destBlendFactorState;
} }
bool Context::allTargetsColorClamp() const
{
// TODO: remove all of this and support VkPhysicalDeviceFeatures::independentBlend instead
for (int i = 0; i < RENDERTARGETS; i++)
{
if (renderTarget[i] && renderTarget[i]->getFormat().isFloatFormat())
{
return false;
}
}
return true;
}
VkBlendOp Context::blendOperation() const VkBlendOp Context::blendOperation() const
{ {
if(!alphaBlendEnable) return VK_BLEND_OP_SRC_EXT; if(!alphaBlendEnable) return VK_BLEND_OP_SRC_EXT;
...@@ -365,7 +379,7 @@ namespace sw ...@@ -365,7 +379,7 @@ namespace sw
} }
} }
case VK_BLEND_OP_SUBTRACT: case VK_BLEND_OP_SUBTRACT:
if(sourceBlendFactor() == VK_BLEND_FACTOR_ZERO) if(sourceBlendFactor() == VK_BLEND_FACTOR_ZERO && allTargetsColorClamp())
{ {
return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero
} }
...@@ -405,7 +419,7 @@ namespace sw ...@@ -405,7 +419,7 @@ namespace sw
} }
else if(sourceBlendFactor() == VK_BLEND_FACTOR_ONE) else if(sourceBlendFactor() == VK_BLEND_FACTOR_ONE)
{ {
if(destBlendFactor() == VK_BLEND_FACTOR_ZERO) if(destBlendFactor() == VK_BLEND_FACTOR_ZERO && allTargetsColorClamp())
{ {
return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero
} }
...@@ -416,7 +430,7 @@ namespace sw ...@@ -416,7 +430,7 @@ namespace sw
} }
else else
{ {
if(destBlendFactor() == VK_BLEND_FACTOR_ZERO) if(destBlendFactor() == VK_BLEND_FACTOR_ZERO && allTargetsColorClamp())
{ {
return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero
} }
...@@ -533,7 +547,7 @@ namespace sw ...@@ -533,7 +547,7 @@ namespace sw
} }
} }
case VK_BLEND_OP_SUBTRACT: case VK_BLEND_OP_SUBTRACT:
if(sourceBlendFactorAlpha() == VK_BLEND_FACTOR_ZERO) if(sourceBlendFactorAlpha() == VK_BLEND_FACTOR_ZERO && allTargetsColorClamp())
{ {
return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero
} }
...@@ -573,7 +587,7 @@ namespace sw ...@@ -573,7 +587,7 @@ namespace sw
} }
else if(sourceBlendFactorAlpha() == VK_BLEND_FACTOR_ONE) else if(sourceBlendFactorAlpha() == VK_BLEND_FACTOR_ONE)
{ {
if(destBlendFactorAlpha() == VK_BLEND_FACTOR_ZERO) if(destBlendFactorAlpha() == VK_BLEND_FACTOR_ZERO && allTargetsColorClamp())
{ {
return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero
} }
...@@ -584,7 +598,7 @@ namespace sw ...@@ -584,7 +598,7 @@ namespace sw
} }
else else
{ {
if(destBlendFactorAlpha() == VK_BLEND_FACTOR_ZERO) if(destBlendFactorAlpha() == VK_BLEND_FACTOR_ZERO && allTargetsColorClamp())
{ {
return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero return VK_BLEND_OP_ZERO_EXT; // Negative, clamped to zero
} }
......
src/Device/Context.hpp
...@@ -123,6 +123,7 @@ namespace sw ...@@ -123,6 +123,7 @@ namespace sw
bool perspectiveActive() const; bool perspectiveActive() const;
bool allTargetsColorClamp() const;
bool alphaBlendActive() const; bool alphaBlendActive() const;
VkBlendFactor sourceBlendFactor() const; VkBlendFactor sourceBlendFactor() const;
VkBlendFactor destBlendFactor() const; VkBlendFactor destBlendFactor() const;
......
src/Pipeline/PixelRoutine.cpp
...@@ -1672,10 +1672,14 @@ namespace sw ...@@ -1672,10 +1672,14 @@ namespace sw
switch(blendFactorActive) switch(blendFactorActive)
{ {
case VK_BLEND_FACTOR_ZERO: case VK_BLEND_FACTOR_ZERO:
// Optimized blendFactor.x = Float4(0);
blendFactor.y = Float4(0);
blendFactor.z = Float4(0);
break; break;
case VK_BLEND_FACTOR_ONE: case VK_BLEND_FACTOR_ONE:
// Optimized blendFactor.x = Float4(1);
blendFactor.y = Float4(1);
blendFactor.z = Float4(1);
break; break;
case VK_BLEND_FACTOR_SRC_COLOR: case VK_BLEND_FACTOR_SRC_COLOR:
blendFactor.x = oC.x; blendFactor.x = oC.x;
...@@ -1754,10 +1758,10 @@ namespace sw ...@@ -1754,10 +1758,10 @@ namespace sw
switch(blendFactorAlphaActive) switch(blendFactorAlphaActive)
{ {
case VK_BLEND_FACTOR_ZERO: case VK_BLEND_FACTOR_ZERO:
// Optimized blendFactor.w = Float4(0);
break; break;
case VK_BLEND_FACTOR_ONE: case VK_BLEND_FACTOR_ONE:
// Optimized blendFactor.w = Float4(1);
break; break;
case VK_BLEND_FACTOR_SRC_COLOR: case VK_BLEND_FACTOR_SRC_COLOR:
blendFactor.w = oC.w; blendFactor.w = oC.w;
...@@ -1928,19 +1932,13 @@ namespace sw ...@@ -1928,19 +1932,13 @@ namespace sw
blendFactor(sourceFactor, oC, pixel, state.sourceBlendFactor); blendFactor(sourceFactor, oC, pixel, state.sourceBlendFactor);
blendFactor(destFactor, oC, pixel, state.destBlendFactor); blendFactor(destFactor, oC, pixel, state.destBlendFactor);
if(state.sourceBlendFactor != VK_BLEND_FACTOR_ONE && state.sourceBlendFactor != VK_BLEND_FACTOR_ZERO) oC.x *= sourceFactor.x;
{ oC.y *= sourceFactor.y;
oC.x *= sourceFactor.x; oC.z *= sourceFactor.z;
oC.y *= sourceFactor.y;
oC.z *= sourceFactor.z;
}
if(state.destBlendFactor != VK_BLEND_FACTOR_ONE && state.destBlendFactor != VK_BLEND_FACTOR_ZERO) pixel.x *= destFactor.x;
{ pixel.y *= destFactor.y;
pixel.x *= destFactor.x; pixel.z *= destFactor.z;
pixel.y *= destFactor.y;
pixel.z *= destFactor.z;
}
switch(state.blendOperation) switch(state.blendOperation)
{ {
...@@ -1989,15 +1987,8 @@ namespace sw ...@@ -1989,15 +1987,8 @@ namespace sw
blendFactorAlpha(sourceFactor, oC, pixel, state.sourceBlendFactorAlpha); blendFactorAlpha(sourceFactor, oC, pixel, state.sourceBlendFactorAlpha);
blendFactorAlpha(destFactor, oC, pixel, state.destBlendFactorAlpha); blendFactorAlpha(destFactor, oC, pixel, state.destBlendFactorAlpha);
if(state.sourceBlendFactorAlpha != VK_BLEND_FACTOR_ONE && state.sourceBlendFactorAlpha != VK_BLEND_FACTOR_ZERO) oC.w *= sourceFactor.w;
{ pixel.w *= destFactor.w;
oC.w *= sourceFactor.w;
}
if(state.destBlendFactorAlpha != VK_BLEND_FACTOR_ONE && state.destBlendFactorAlpha != VK_BLEND_FACTOR_ZERO)
{
pixel.w *= destFactor.w;
}
switch(state.blendOperationAlpha) switch(state.blendOperationAlpha)
{ {
......
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