Commit d063affa by Jiawei Shao Committed by Commit Bot

ES31: Add link validation on geometry shader varyings

This patch adds the link validation on geometry shader varyings. According to SPEC, geometry shader inputs should not be treated as arrays for the purpose of interface matching. This patch also moves the checks on fragment input bindings into a single function. BUG=angleproject:1941 TEST=angle_end2end_tests dEQP-GLES31.functional.shaders.linkage.es31.geometry.varying.* Change-Id: Ib3ca64e28683e9688edc9432d43ff5a70c86117e Reviewed-on: https://chromium-review.googlesource.com/929866Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Commit-Queue: Corentin Wallez <cwallez@chromium.org>
parent 85f0b5ea
...@@ -2217,16 +2217,51 @@ const TransformFeedbackVarying &Program::getTransformFeedbackVaryingResource(GLu ...@@ -2217,16 +2217,51 @@ const TransformFeedbackVarying &Program::getTransformFeedbackVaryingResource(GLu
bool Program::linkVaryings(const Context *context, InfoLog &infoLog) const bool Program::linkVaryings(const Context *context, InfoLog &infoLog) const
{ {
Shader *generatingShader = mState.mAttachedVertexShader; std::vector<Shader *> activeShaders;
Shader *consumingShader = mState.mAttachedFragmentShader; activeShaders.push_back(mState.mAttachedVertexShader);
if (mState.mAttachedGeometryShader)
{
activeShaders.push_back(mState.mAttachedGeometryShader);
}
activeShaders.push_back(mState.mAttachedFragmentShader);
const size_t activeShaderCount = activeShaders.size();
for (size_t shaderIndex = 0; shaderIndex < activeShaderCount - 1; ++shaderIndex)
{
if (!linkValidateShaderInterfaceMatching(context, activeShaders[shaderIndex],
activeShaders[shaderIndex + 1], infoLog))
{
return false;
}
}
if (!linkValidateBuiltInVaryings(context, infoLog))
{
return false;
}
if (!linkValidateFragmentInputBindings(context, infoLog))
{
return false;
}
return true;
}
// [OpenGL ES 3.1] Chapter 7.4.1 "Shader Interface Matchining" Page 91
// TODO(jiawei.shao@intel.com): add validation on input/output blocks matching
bool Program::linkValidateShaderInterfaceMatching(const Context *context,
gl::Shader *generatingShader,
gl::Shader *consumingShader,
gl::InfoLog &infoLog) const
{
ASSERT(generatingShader->getShaderVersion(context) == ASSERT(generatingShader->getShaderVersion(context) ==
consumingShader->getShaderVersion(context)); consumingShader->getShaderVersion(context));
const std::vector<sh::Varying> &outputVaryings = generatingShader->getOutputVaryings(context); const std::vector<sh::Varying> &outputVaryings = generatingShader->getOutputVaryings(context);
const std::vector<sh::Varying> &inputVaryings = consumingShader->getInputVaryings(context); const std::vector<sh::Varying> &inputVaryings = consumingShader->getInputVaryings(context);
std::map<GLuint, std::string> staticFragmentInputLocations; bool validateGeometryShaderInputs = consumingShader->getType() == GL_GEOMETRY_SHADER_EXT;
for (const sh::Varying &input : inputVaryings) for (const sh::Varying &input : inputVaryings)
{ {
...@@ -2247,7 +2282,7 @@ bool Program::linkVaryings(const Context *context, InfoLog &infoLog) const ...@@ -2247,7 +2282,7 @@ bool Program::linkVaryings(const Context *context, InfoLog &infoLog) const
std::string mismatchedStructFieldName; std::string mismatchedStructFieldName;
LinkMismatchError linkError = LinkMismatchError linkError =
LinkValidateVaryings(output, input, generatingShader->getShaderVersion(context), LinkValidateVaryings(output, input, generatingShader->getShaderVersion(context),
&mismatchedStructFieldName); validateGeometryShaderInputs, &mismatchedStructFieldName);
if (linkError != LinkMismatchError::NO_MISMATCH) if (linkError != LinkMismatchError::NO_MISMATCH)
{ {
LogLinkMismatch(infoLog, input.name, "varying", linkError, LogLinkMismatch(infoLog, input.name, "varying", linkError,
...@@ -2269,13 +2304,27 @@ bool Program::linkVaryings(const Context *context, InfoLog &infoLog) const ...@@ -2269,13 +2304,27 @@ bool Program::linkVaryings(const Context *context, InfoLog &infoLog) const
<< " varying"; << " varying";
return false; return false;
} }
}
// Check for aliased path rendering input bindings (if any). // TODO(jmadill): verify no unmatched output varyings?
// If more than one binding refer statically to the same
// location the link must fail. return true;
}
bool Program::linkValidateFragmentInputBindings(const Context *context, gl::InfoLog &infoLog) const
{
ASSERT(mState.mAttachedFragmentShader);
if (!input.staticUse) std::map<GLuint, std::string> staticFragmentInputLocations;
const std::vector<sh::Varying> &fragmentInputVaryings =
mState.mAttachedFragmentShader->getInputVaryings(context);
for (const sh::Varying &input : fragmentInputVaryings)
{
if (input.isBuiltIn() || !input.staticUse)
{
continue; continue;
}
const auto inputBinding = mFragmentInputBindings.getBinding(input.name); const auto inputBinding = mFragmentInputBindings.getBinding(input.name);
if (inputBinding == -1) if (inputBinding == -1)
...@@ -2294,13 +2343,6 @@ bool Program::linkVaryings(const Context *context, InfoLog &infoLog) const ...@@ -2294,13 +2343,6 @@ bool Program::linkVaryings(const Context *context, InfoLog &infoLog) const
} }
} }
if (!linkValidateBuiltInVaryings(context, infoLog))
{
return false;
}
// TODO(jmadill): verify no unmatched output varyings?
return true; return true;
} }
...@@ -2442,7 +2484,7 @@ LinkMismatchError Program::LinkValidateInterfaceBlockFields( ...@@ -2442,7 +2484,7 @@ LinkMismatchError Program::LinkValidateInterfaceBlockFields(
// If webgl, validate precision of UBO fields, otherwise don't. See Khronos bug 10287. // If webgl, validate precision of UBO fields, otherwise don't. See Khronos bug 10287.
LinkMismatchError linkError = LinkValidateVariablesBase( LinkMismatchError linkError = LinkValidateVariablesBase(
blockField1, blockField2, webglCompatibility, mismatchedBlockFieldName); blockField1, blockField2, webglCompatibility, true, mismatchedBlockFieldName);
if (linkError != LinkMismatchError::NO_MISMATCH) if (linkError != LinkMismatchError::NO_MISMATCH)
{ {
AddParentPrefix(blockField1.name, mismatchedBlockFieldName); AddParentPrefix(blockField1.name, mismatchedBlockFieldName);
...@@ -2778,13 +2820,14 @@ LinkMismatchError Program::AreMatchingInterfaceBlocks(const sh::InterfaceBlock & ...@@ -2778,13 +2820,14 @@ LinkMismatchError Program::AreMatchingInterfaceBlocks(const sh::InterfaceBlock &
LinkMismatchError Program::LinkValidateVariablesBase(const sh::ShaderVariable &variable1, LinkMismatchError Program::LinkValidateVariablesBase(const sh::ShaderVariable &variable1,
const sh::ShaderVariable &variable2, const sh::ShaderVariable &variable2,
bool validatePrecision, bool validatePrecision,
bool validateArraySize,
std::string *mismatchedStructOrBlockMemberName) std::string *mismatchedStructOrBlockMemberName)
{ {
if (variable1.type != variable2.type) if (variable1.type != variable2.type)
{ {
return LinkMismatchError::TYPE_MISMATCH; return LinkMismatchError::TYPE_MISMATCH;
} }
if (variable1.arraySizes != variable2.arraySizes) if (validateArraySize && variable1.arraySizes != variable2.arraySizes)
{ {
return LinkMismatchError::ARRAY_SIZE_MISMATCH; return LinkMismatchError::ARRAY_SIZE_MISMATCH;
} }
...@@ -2813,7 +2856,7 @@ LinkMismatchError Program::LinkValidateVariablesBase(const sh::ShaderVariable &v ...@@ -2813,7 +2856,7 @@ LinkMismatchError Program::LinkValidateVariablesBase(const sh::ShaderVariable &v
} }
LinkMismatchError linkErrorOnField = LinkValidateVariablesBase( LinkMismatchError linkErrorOnField = LinkValidateVariablesBase(
member1, member2, validatePrecision, mismatchedStructOrBlockMemberName); member1, member2, validatePrecision, true, mismatchedStructOrBlockMemberName);
if (linkErrorOnField != LinkMismatchError::NO_MISMATCH) if (linkErrorOnField != LinkMismatchError::NO_MISMATCH)
{ {
AddParentPrefix(member1.name, mismatchedStructOrBlockMemberName); AddParentPrefix(member1.name, mismatchedStructOrBlockMemberName);
...@@ -2827,10 +2870,34 @@ LinkMismatchError Program::LinkValidateVariablesBase(const sh::ShaderVariable &v ...@@ -2827,10 +2870,34 @@ LinkMismatchError Program::LinkValidateVariablesBase(const sh::ShaderVariable &v
LinkMismatchError Program::LinkValidateVaryings(const sh::Varying &outputVarying, LinkMismatchError Program::LinkValidateVaryings(const sh::Varying &outputVarying,
const sh::Varying &inputVarying, const sh::Varying &inputVarying,
int shaderVersion, int shaderVersion,
bool validateGeometryShaderInputVarying,
std::string *mismatchedStructFieldName) std::string *mismatchedStructFieldName)
{ {
if (validateGeometryShaderInputVarying)
{
// [GL_EXT_geometry_shader] Section 11.1gs.4.3:
// The OpenGL ES Shading Language doesn't support multi-dimensional arrays as shader inputs
// or outputs.
ASSERT(inputVarying.arraySizes.size() == 1u);
// Geometry shader input varyings are not treated as arrays, so a vertex array output
// varying cannot match a geometry shader input varying.
// [GL_EXT_geometry_shader] Section 7.4.1:
// Geometry shader per-vertex input variables and blocks are required to be declared as
// arrays, with each element representing input or output values for a single vertex of a
// multi-vertex primitive. For the purposes of interface matching, such variables and blocks
// are treated as though they were not declared as arrays.
if (outputVarying.isArray())
{
return LinkMismatchError::ARRAY_SIZE_MISMATCH;
}
}
// Skip the validation on the array sizes between a vertex output varying and a geometry input
// varying as it has been done before.
LinkMismatchError linkError = LinkMismatchError linkError =
LinkValidateVariablesBase(outputVarying, inputVarying, false, mismatchedStructFieldName); LinkValidateVariablesBase(outputVarying, inputVarying, false,
!validateGeometryShaderInputVarying, mismatchedStructFieldName);
if (linkError != LinkMismatchError::NO_MISMATCH) if (linkError != LinkMismatchError::NO_MISMATCH)
{ {
return linkError; return linkError;
......
...@@ -666,6 +666,7 @@ class Program final : angle::NonCopyable, public LabeledObject ...@@ -666,6 +666,7 @@ class Program final : angle::NonCopyable, public LabeledObject
const sh::ShaderVariable &variable1, const sh::ShaderVariable &variable1,
const sh::ShaderVariable &variable2, const sh::ShaderVariable &variable2,
bool validatePrecision, bool validatePrecision,
bool validateArraySize,
std::string *mismatchedStructOrBlockMemberName); std::string *mismatchedStructOrBlockMemberName);
GLuint getInputResourceIndex(const GLchar *name) const; GLuint getInputResourceIndex(const GLchar *name) const;
...@@ -724,7 +725,18 @@ class Program final : angle::NonCopyable, public LabeledObject ...@@ -724,7 +725,18 @@ class Program final : angle::NonCopyable, public LabeledObject
static LinkMismatchError LinkValidateVaryings(const sh::Varying &outputVarying, static LinkMismatchError LinkValidateVaryings(const sh::Varying &outputVarying,
const sh::Varying &inputVarying, const sh::Varying &inputVarying,
int shaderVersion, int shaderVersion,
bool validateGeometryShaderInputVarying,
std::string *mismatchedStructFieldName); std::string *mismatchedStructFieldName);
bool linkValidateShaderInterfaceMatching(const Context *context,
Shader *generatingShader,
Shader *consumingShader,
InfoLog &infoLog) const;
// Check for aliased path rendering input bindings (if any).
// If more than one binding refer statically to the same location the link must fail.
bool linkValidateFragmentInputBindings(const Context *context, InfoLog &infoLog) const;
bool linkValidateBuiltInVaryings(const Context *context, InfoLog &infoLog) const; bool linkValidateBuiltInVaryings(const Context *context, InfoLog &infoLog) const;
bool linkValidateTransformFeedback(const gl::Context *context, bool linkValidateTransformFeedback(const gl::Context *context,
InfoLog &infoLog, InfoLog &infoLog,
......
...@@ -158,7 +158,7 @@ LinkMismatchError UniformLinker::LinkValidateUniforms(const sh::Uniform &uniform ...@@ -158,7 +158,7 @@ LinkMismatchError UniformLinker::LinkValidateUniforms(const sh::Uniform &uniform
#endif #endif
LinkMismatchError linkError = Program::LinkValidateVariablesBase( LinkMismatchError linkError = Program::LinkValidateVariablesBase(
uniform1, uniform2, validatePrecision, mismatchedStructFieldName); uniform1, uniform2, validatePrecision, true, mismatchedStructFieldName);
if (linkError != LinkMismatchError::NO_MISMATCH) if (linkError != LinkMismatchError::NO_MISMATCH)
{ {
return linkError; return linkError;
......
...@@ -1645,10 +1645,6 @@ ...@@ -1645,10 +1645,6 @@
1941 OPENGL D3D11 : dEQP-GLES31.functional.geometry_shading.instanced.invocation_output_* = FAIL 1941 OPENGL D3D11 : dEQP-GLES31.functional.geometry_shading.instanced.invocation_output_* = FAIL
1941 OPENGL D3D11 : dEQP-GLES31.functional.geometry_shading.instanced.draw_* = FAIL 1941 OPENGL D3D11 : dEQP-GLES31.functional.geometry_shading.instanced.draw_* = FAIL
1941 OPENGL D3D11 : dEQP-GLES31.functional.geometry_shading.negative.* = FAIL 1941 OPENGL D3D11 : dEQP-GLES31.functional.geometry_shading.negative.* = FAIL
1941 OPENGL D3D11 : dEQP-GLES31.functional.shaders.linkage.es31.geometry.varying.rules.input_* = FAIL
1941 OPENGL D3D11 : dEQP-GLES31.functional.shaders.linkage.es31.geometry.varying.rules.output_* = FAIL
1941 OPENGL D3D11 : dEQP-GLES31.functional.shaders.linkage.es31.geometry.varying.types.* = FAIL
1941 OPENGL D3D11 : dEQP-GLES31.functional.shaders.linkage.es31.geometry.varying.qualifiers.* = FAIL
1941 OPENGL D3D11 : dEQP-GLES31.functional.shaders.linkage.es31.geometry.uniform.* = FAIL 1941 OPENGL D3D11 : dEQP-GLES31.functional.shaders.linkage.es31.geometry.uniform.* = FAIL
2324 DEBUG RELEASE : dEQP-GLES31.functional.debug.negative_coverage.callbacks.compute.program_not_active = FAIL 2324 DEBUG RELEASE : dEQP-GLES31.functional.debug.negative_coverage.callbacks.compute.program_not_active = FAIL
......
...@@ -268,6 +268,54 @@ TEST_P(GeometryShaderTest, LinkValidationOnGeometryShaderLayouts) ...@@ -268,6 +268,54 @@ TEST_P(GeometryShaderTest, LinkValidationOnGeometryShaderLayouts)
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR();
} }
// Verify that an link error occurs when the vertex shader has an array output and there is a
// geometry shader in the program.
TEST_P(GeometryShaderTest, VertexShaderArrayOutput)
{
ANGLE_SKIP_TEST_IF(!extensionEnabled("GL_EXT_geometry_shader"));
const std::string &vertexShader =
R"(#version 310 es
in vec4 vertex_in;
out vec4 vertex_out[3];
void main()
{
gl_Position = vertex_in;
vertex_out[0] = vec4(1.0, 0.0, 0.0, 1.0);
vertex_out[1] = vec4(0.0, 1.0, 0.0, 1.0);
vertex_out[2] = vec4(0.0, 0.0, 1.0, 1.0);
})";
const std::string &geometryShader =
R"(#version 310 es
#extension GL_EXT_geometry_shader : require
layout (invocations = 3, triangles) in;
layout (points, max_vertices = 3) out;
in vec4 vertex_out[];
out vec4 geometry_color;
void main()
{
gl_Position = gl_in[0].gl_Position;
geometry_color = vertex_out[0];
EmitVertex();
})";
const std::string &fragmentShader =
R"(#version 310 es
precision mediump float;
in vec4 geometry_color;
layout (location = 0) out vec4 output_color;
void main()
{
output_color = geometry_color;
})";
GLuint program = CompileProgramWithGS(vertexShader, geometryShader, fragmentShader);
EXPECT_EQ(0u, program);
EXPECT_GL_NO_ERROR();
}
ANGLE_INSTANTIATE_TEST(GeometryShaderTestES3, ES3_OPENGL(), ES3_OPENGLES(), ES3_D3D11()); ANGLE_INSTANTIATE_TEST(GeometryShaderTestES3, ES3_OPENGL(), ES3_OPENGLES(), ES3_D3D11());
ANGLE_INSTANTIATE_TEST(GeometryShaderTest, ES31_OPENGL(), ES31_OPENGLES(), ES31_D3D11()); ANGLE_INSTANTIATE_TEST(GeometryShaderTest, ES31_OPENGL(), ES31_OPENGLES(), ES31_D3D11());
} }
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