Commit a43d7306 by Olli Etuaho Committed by Commit Bot

Clean up varying linkage in D3D

D3D10+ has stricter rules on linking shader inputs with outputs than the previous comments in the code indicated. Add comments and checks related to this and structure the code in a way that makes it a bit less prone to errors. This page is intended to document the rules though it is somewhat vague: https://docs.microsoft.com/en-us/windows/desktop/direct3dhlsl/dx-graphics-hlsl-signatures BUG=angleproject:2767 TEST=angle_end2end_tests Change-Id: I236ec4cd5cbf3889fd2c97947ee81a6c5ae6a787 Reviewed-on: https://chromium-review.googlesource.com/1169818Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent 7ae70d8f
...@@ -439,8 +439,9 @@ void DynamicHLSL::generateVaryingLinkHLSL(const VaryingPacking &varyingPacking, ...@@ -439,8 +439,9 @@ void DynamicHLSL::generateVaryingLinkHLSL(const VaryingPacking &varyingPacking,
hlslStream << " v" << registerIndex << " : " << varyingSemantic << registerIndex << ";\n"; hlslStream << " v" << registerIndex << " : " << varyingSemantic << registerIndex << ";\n";
} }
// Note that the following outputs need to be declared after the others to make multiview // Note that the following outputs need to be declared after the others. They are not included
// shaders with a flat varying to work correctly. This is a workaround for a D3D bug. // in pixel shader inputs even when they are in vertex/geometry shader outputs, and the pixel
// shader input struct must be a prefix of the vertex/geometry shader output struct.
if (builtins.glViewportIndex.enabled) if (builtins.glViewportIndex.enabled)
{ {
...@@ -1272,10 +1273,23 @@ BuiltinVaryingsD3D::BuiltinVaryingsD3D(const ProgramD3DMetadata &metadata, ...@@ -1272,10 +1273,23 @@ BuiltinVaryingsD3D::BuiltinVaryingsD3D(const ProgramD3DMetadata &metadata,
{ {
updateBuiltins(gl::ShaderType::Vertex, metadata, packing); updateBuiltins(gl::ShaderType::Vertex, metadata, packing);
updateBuiltins(gl::ShaderType::Fragment, metadata, packing); updateBuiltins(gl::ShaderType::Fragment, metadata, packing);
if (metadata.getRendererMajorShaderModel() >= 4) int shaderModel = metadata.getRendererMajorShaderModel();
if (shaderModel >= 4)
{ {
updateBuiltins(gl::ShaderType::Geometry, metadata, packing); updateBuiltins(gl::ShaderType::Geometry, metadata, packing);
} }
// In shader model >= 4, some builtins need to be the same in vertex and pixel shaders - input
// struct needs to be a prefix of output struct.
ASSERT(shaderModel < 4 || mBuiltinInfo[gl::ShaderType::Vertex].glPosition.enabled ==
mBuiltinInfo[gl::ShaderType::Fragment].glPosition.enabled);
ASSERT(shaderModel < 4 || mBuiltinInfo[gl::ShaderType::Vertex].glFragCoord.enabled ==
mBuiltinInfo[gl::ShaderType::Fragment].glFragCoord.enabled);
ASSERT(shaderModel < 4 || mBuiltinInfo[gl::ShaderType::Vertex].glPointCoord.enabled ==
mBuiltinInfo[gl::ShaderType::Fragment].glPointCoord.enabled);
ASSERT(shaderModel < 4 || mBuiltinInfo[gl::ShaderType::Vertex].glPointSize.enabled ==
mBuiltinInfo[gl::ShaderType::Fragment].glPointSize.enabled);
ASSERT(shaderModel < 4 || mBuiltinInfo[gl::ShaderType::Vertex].glViewIDOVR.enabled ==
mBuiltinInfo[gl::ShaderType::Fragment].glViewIDOVR.enabled);
} }
BuiltinVaryingsD3D::~BuiltinVaryingsD3D() = default; BuiltinVaryingsD3D::~BuiltinVaryingsD3D() = default;
...@@ -1287,6 +1301,10 @@ void BuiltinVaryingsD3D::updateBuiltins(gl::ShaderType shaderType, ...@@ -1287,6 +1301,10 @@ void BuiltinVaryingsD3D::updateBuiltins(gl::ShaderType shaderType,
const std::string &userSemantic = GetVaryingSemantic(metadata.getRendererMajorShaderModel(), const std::string &userSemantic = GetVaryingSemantic(metadata.getRendererMajorShaderModel(),
metadata.usesSystemValuePointSize()); metadata.usesSystemValuePointSize());
// Note that when enabling builtins only for specific shader stages in shader model >= 4, the
// code needs to ensure that the input struct of the shader stage is a prefix of the output
// struct of the previous stage.
unsigned int reservedSemanticIndex = packing.getMaxSemanticIndex(); unsigned int reservedSemanticIndex = packing.getMaxSemanticIndex();
BuiltinInfo *builtins = &mBuiltinInfo[shaderType]; BuiltinInfo *builtins = &mBuiltinInfo[shaderType];
...@@ -1329,34 +1347,31 @@ void BuiltinVaryingsD3D::updateBuiltins(gl::ShaderType shaderType, ...@@ -1329,34 +1347,31 @@ void BuiltinVaryingsD3D::updateBuiltins(gl::ShaderType shaderType,
} }
} }
if (shaderType == gl::ShaderType::Vertex && metadata.hasANGLEMultiviewEnabled()) if (metadata.hasANGLEMultiviewEnabled())
{ {
// Although it is possible to compute gl_ViewID_OVR from the value of
// SV_ViewportArrayIndex or SV_RenderTargetArrayIndex and the multi-view state in the
// driver constant buffer, it is easier and cleaner to always pass it as a varying.
builtins->glViewIDOVR.enable(userSemantic, reservedSemanticIndex++); builtins->glViewIDOVR.enable(userSemantic, reservedSemanticIndex++);
if (metadata.canSelectViewInVertexShader())
if (shaderType == gl::ShaderType::Vertex)
{ {
if (metadata.canSelectViewInVertexShader())
{
builtins->glViewportIndex.enableSystem("SV_ViewportArrayIndex");
builtins->glLayer.enableSystem("SV_RenderTargetArrayIndex");
}
}
if (shaderType == gl::ShaderType::Geometry)
{
// gl_Layer and gl_ViewportIndex are necessary so that we can write to either based on
// the multiview state in the driver constant buffer.
builtins->glViewportIndex.enableSystem("SV_ViewportArrayIndex"); builtins->glViewportIndex.enableSystem("SV_ViewportArrayIndex");
builtins->glLayer.enableSystem("SV_RenderTargetArrayIndex"); builtins->glLayer.enableSystem("SV_RenderTargetArrayIndex");
} }
} }
if (shaderType == gl::ShaderType::Fragment && metadata.hasANGLEMultiviewEnabled())
{
builtins->glViewIDOVR.enable(userSemantic, reservedSemanticIndex++);
}
if (shaderType == gl::ShaderType::Geometry && metadata.hasANGLEMultiviewEnabled())
{
// Although it is possible to retrieve gl_ViewID_OVR from the value of
// SV_ViewportArrayIndex or SV_RenderTargetArrayIndex based on the multi-view state in the
// driver constant buffer, it is easier and cleaner to pass it as a varying.
builtins->glViewIDOVR.enable(userSemantic, reservedSemanticIndex++);
// gl_Layer and gl_ViewportIndex are necessary so that we can write to either based on the
// multiview state in the driver constant buffer.
builtins->glViewportIndex.enableSystem("SV_ViewportArrayIndex");
builtins->glLayer.enableSystem("SV_RenderTargetArrayIndex");
}
// Special case: do not include PSIZE semantic in HLSL 3 pixel shaders // Special case: do not include PSIZE semantic in HLSL 3 pixel shaders
if (metadata.usesSystemValuePointSize() && if (metadata.usesSystemValuePointSize() &&
(shaderType != gl::ShaderType::Fragment || metadata.getRendererMajorShaderModel() >= 4)) (shaderType != gl::ShaderType::Fragment || metadata.getRendererMajorShaderModel() >= 4))
......
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