ES31: Clean up Program::link
This patch intends to solve some structure and coding style issues
in Program::link to make it easier to support linking program with
geometry shader.
1. Move all the shader specific validations to linkValidateShaders.
Geometry shader related link validations can also be added here.
2. Rename functions with "VertexAndFragment" to "Graphics" because
these functions will also be responsible for the validations on
geometry shader.
3. Refer uniforms by pointer when validating uniforms.
4. Re-declare functions to 'static' if we can and capitialize the
first letter of all static functions in Program.h.
BUG=angleproject:1941
Change-Id: I46608e86bddc12d95cbbbf9a85803d07ccf843d8
Reviewed-on: https://chromium-review.googlesource.com/836149
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libANGLE/Program.cpp b/src/libANGLE/Program.cpp
index 4582b51..a58d4df 100644
--- a/src/libANGLE/Program.cpp
+++ b/src/libANGLE/Program.cpp
@@ -827,42 +827,13 @@
unlink();
mInfoLog.reset();
- const Caps &caps = data.getCaps();
-
- Shader *vertexShader = mState.mAttachedVertexShader;
- Shader *fragmentShader = mState.mAttachedFragmentShader;
- Shader *computeShader = mState.mAttachedComputeShader;
-
- bool isComputeShaderAttached = (computeShader != nullptr);
- bool nonComputeShadersAttached = (vertexShader != nullptr || fragmentShader != nullptr);
- // Check whether we both have a compute and non-compute shaders attached.
- // If there are of both types attached, then linking should fail.
- // OpenGL ES 3.10, 7.3 Program Objects, under LinkProgram
- if (isComputeShaderAttached == true && nonComputeShadersAttached == true)
+ if (!linkValidateShaders(context, mInfoLog))
{
- mInfoLog << "Both a compute and non-compute shaders are attached to the same program.";
return NoError();
}
- if (computeShader)
+ if (mState.mAttachedComputeShader)
{
- if (!computeShader->isCompiled(context))
- {
- mInfoLog << "Attached compute shader is not compiled.";
- return NoError();
- }
- ASSERT(computeShader->getType() == GL_COMPUTE_SHADER);
-
- mState.mComputeShaderLocalSize = computeShader->getWorkGroupSize(context);
-
- // GLSL ES 3.10, 4.4.1.1 Compute Shader Inputs
- // If the work group size is not specified, a link time error should occur.
- if (!mState.mComputeShaderLocalSize.isDeclared())
- {
- mInfoLog << "Work group size is not specified.";
- return NoError();
- }
-
if (!linkUniforms(context, mInfoLog, mUniformLocationBindings))
{
return NoError();
@@ -890,24 +861,6 @@
}
else
{
- if (!fragmentShader || !fragmentShader->isCompiled(context))
- {
- return NoError();
- }
- ASSERT(fragmentShader->getType() == GL_FRAGMENT_SHADER);
-
- if (!vertexShader || !vertexShader->isCompiled(context))
- {
- return NoError();
- }
- ASSERT(vertexShader->getType() == GL_VERTEX_SHADER);
-
- if (fragmentShader->getShaderVersion(context) != vertexShader->getShaderVersion(context))
- {
- mInfoLog << "Fragment shader version does not match vertex shader version.";
- return NoError();
- }
-
if (!linkAttributes(context, mInfoLog))
{
return NoError();
@@ -935,7 +888,8 @@
const auto &mergedVaryings = getMergedVaryings(context);
- mState.mNumViews = vertexShader->getNumViews(context);
+ ASSERT(mState.mAttachedVertexShader);
+ mState.mNumViews = mState.mAttachedVertexShader->getNumViews(context);
linkOutputVariables(context);
@@ -953,7 +907,7 @@
InitUniformBlockLinker(context, mState, &resources.uniformBlockLinker);
InitShaderStorageBlockLinker(context, mState, &resources.shaderStorageBlockLinker);
- if (!linkValidateTransformFeedback(context, mInfoLog, mergedVaryings, caps))
+ if (!linkValidateTransformFeedback(context, mInfoLog, mergedVaryings, context->getCaps()))
{
return NoError();
}
@@ -1998,6 +1952,68 @@
return mState.mTransformFeedbackBufferMode;
}
+bool Program::linkValidateShaders(const Context *context, InfoLog &infoLog)
+{
+ Shader *vertexShader = mState.mAttachedVertexShader;
+ Shader *fragmentShader = mState.mAttachedFragmentShader;
+ Shader *computeShader = mState.mAttachedComputeShader;
+
+ bool isComputeShaderAttached = (computeShader != nullptr);
+ bool isGraphicsShaderAttached = (vertexShader != nullptr || fragmentShader != nullptr);
+ // Check whether we both have a compute and non-compute shaders attached.
+ // If there are of both types attached, then linking should fail.
+ // OpenGL ES 3.10, 7.3 Program Objects, under LinkProgram
+ if (isComputeShaderAttached == true && isGraphicsShaderAttached == true)
+ {
+ infoLog << "Both compute and graphics shaders are attached to the same program.";
+ return false;
+ }
+
+ if (computeShader)
+ {
+ if (!computeShader->isCompiled(context))
+ {
+ infoLog << "Attached compute shader is not compiled.";
+ return false;
+ }
+ ASSERT(computeShader->getType() == GL_COMPUTE_SHADER);
+
+ mState.mComputeShaderLocalSize = computeShader->getWorkGroupSize(context);
+
+ // GLSL ES 3.10, 4.4.1.1 Compute Shader Inputs
+ // If the work group size is not specified, a link time error should occur.
+ if (!mState.mComputeShaderLocalSize.isDeclared())
+ {
+ infoLog << "Work group size is not specified.";
+ return false;
+ }
+ }
+ else
+ {
+ if (!fragmentShader || !fragmentShader->isCompiled(context))
+ {
+ infoLog << "No compiled fragment shader when at least one graphics shader is attached.";
+ return false;
+ }
+ ASSERT(fragmentShader->getType() == GL_FRAGMENT_SHADER);
+
+ if (!vertexShader || !vertexShader->isCompiled(context))
+ {
+ infoLog << "No compiled vertex shader when at least one graphics shader is attached.";
+ return false;
+ }
+ ASSERT(vertexShader->getType() == GL_VERTEX_SHADER);
+
+ if (fragmentShader->getShaderVersion(context) != vertexShader->getShaderVersion(context))
+ {
+ infoLog << "Fragment shader version does not match vertex shader version.";
+ return false;
+ }
+ }
+
+ return true;
+}
+
bool Program::linkVaryings(const Context *context, InfoLog &infoLog) const
{
Shader *vertexShader = mState.mAttachedVertexShader;
@@ -2025,7 +2041,7 @@
if (output.name == input.name)
{
ASSERT(!input.isBuiltIn());
- if (!linkValidateVaryings(infoLog, output.name, input, output,
+ if (!LinkValidateVaryings(infoLog, output.name, input, output,
vertexShader->getShaderVersion(context)))
{
return false;
@@ -2202,14 +2218,14 @@
return true;
}
-bool Program::linkValidateInterfaceBlockFields(InfoLog &infoLog,
+bool Program::LinkValidateInterfaceBlockFields(InfoLog &infoLog,
const std::string &uniformName,
const sh::InterfaceBlockField &vertexUniform,
const sh::InterfaceBlockField &fragmentUniform,
bool webglCompatibility)
{
// If webgl, validate precision of UBO fields, otherwise don't. See Khronos bug 10287.
- if (!linkValidateVariablesBase(infoLog, uniformName, vertexUniform, fragmentUniform,
+ if (!LinkValidateVariablesBase(infoLog, uniformName, vertexUniform, fragmentUniform,
webglCompatibility))
{
return false;
@@ -2333,11 +2349,11 @@
return true;
}
-bool Program::validateVertexAndFragmentInterfaceBlocks(
+bool Program::ValidateGraphicsInterfaceBlocks(
const std::vector<sh::InterfaceBlock> &vertexInterfaceBlocks,
const std::vector<sh::InterfaceBlock> &fragmentInterfaceBlocks,
InfoLog &infoLog,
- bool webglCompatibility) const
+ bool webglCompatibility)
{
// Check that interface blocks defined in the vertex and fragment shaders are identical
typedef std::map<std::string, const sh::InterfaceBlock *> InterfaceBlockMap;
@@ -2354,7 +2370,7 @@
if (entry != linkedInterfaceBlocks.end())
{
const sh::InterfaceBlock &vertexInterfaceBlock = *entry->second;
- if (!areMatchingInterfaceBlocks(infoLog, vertexInterfaceBlock, fragmentInterfaceBlock,
+ if (!AreMatchingInterfaceBlocks(infoLog, vertexInterfaceBlock, fragmentInterfaceBlock,
webglCompatibility))
{
return false;
@@ -2417,8 +2433,8 @@
}
bool webglCompatibility = context->getExtensions().webglCompatibility;
- if (!validateVertexAndFragmentInterfaceBlocks(vertexUniformBlocks, fragmentUniformBlocks,
- infoLog, webglCompatibility))
+ if (!ValidateGraphicsInterfaceBlocks(vertexUniformBlocks, fragmentUniformBlocks, infoLog,
+ webglCompatibility))
{
return false;
}
@@ -2446,9 +2462,8 @@
return false;
}
- if (!validateVertexAndFragmentInterfaceBlocks(vertexShaderStorageBlocks,
- fragmentShaderStorageBlocks, infoLog,
- webglCompatibility))
+ if (!ValidateGraphicsInterfaceBlocks(vertexShaderStorageBlocks, fragmentShaderStorageBlocks,
+ infoLog, webglCompatibility))
{
return false;
}
@@ -2456,12 +2471,12 @@
return true;
}
-bool Program::areMatchingInterfaceBlocks(InfoLog &infoLog,
+bool Program::AreMatchingInterfaceBlocks(InfoLog &infoLog,
const sh::InterfaceBlock &vertexInterfaceBlock,
const sh::InterfaceBlock &fragmentInterfaceBlock,
- bool webglCompatibility) const
+ bool webglCompatibility)
{
- const char* blockName = vertexInterfaceBlock.name.c_str();
+ const char *blockName = vertexInterfaceBlock.name.c_str();
// validate blocks for the same member types
if (vertexInterfaceBlock.fields.size() != fragmentInterfaceBlock.fields.size())
{
@@ -2497,7 +2512,7 @@
return false;
}
std::string memberName = "interface block '" + vertexInterfaceBlock.name + "' member '" + vertexMember.name + "'";
- if (!linkValidateInterfaceBlockFields(infoLog, memberName, vertexMember, fragmentMember,
+ if (!LinkValidateInterfaceBlockFields(infoLog, memberName, vertexMember, fragmentMember,
webglCompatibility))
{
return false;
@@ -2506,8 +2521,11 @@
return true;
}
-bool Program::linkValidateVariablesBase(InfoLog &infoLog, const std::string &variableName, const sh::ShaderVariable &vertexVariable,
- const sh::ShaderVariable &fragmentVariable, bool validatePrecision)
+bool Program::LinkValidateVariablesBase(InfoLog &infoLog,
+ const std::string &variableName,
+ const sh::ShaderVariable &vertexVariable,
+ const sh::ShaderVariable &fragmentVariable,
+ bool validatePrecision)
{
if (vertexVariable.type != fragmentVariable.type)
{
@@ -2554,7 +2572,8 @@
const std::string memberName = variableName.substr(0, variableName.length() - 1) + "." +
vertexMember.name + "'";
- if (!linkValidateVariablesBase(infoLog, vertexMember.name, vertexMember, fragmentMember, validatePrecision))
+ if (!LinkValidateVariablesBase(infoLog, vertexMember.name, vertexMember, fragmentMember,
+ validatePrecision))
{
return false;
}
@@ -2563,13 +2582,13 @@
return true;
}
-bool Program::linkValidateVaryings(InfoLog &infoLog,
+bool Program::LinkValidateVaryings(InfoLog &infoLog,
const std::string &varyingName,
const sh::Varying &vertexVarying,
const sh::Varying &fragmentVarying,
int shaderVersion)
{
- if (!linkValidateVariablesBase(infoLog, varyingName, vertexVarying, fragmentVarying, false))
+ if (!LinkValidateVariablesBase(infoLog, varyingName, vertexVarying, fragmentVarying, false))
{
return false;
}