ES31: Clean up Program::link

src/libANGLE/Program.cpp

@@ -827,42 +827,13 @@ Error Program::link(const gl::Context *context)
     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)
-    {
-        mInfoLog << "Both a compute and non-compute shaders are attached to the same program.";
-        return NoError();
-    }
-
-    if (computeShader)
-    {
-        if (!computeShader->isCompiled(context))
+    if (!linkValidateShaders(context, mInfoLog))
     {
-            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())
+    if (mState.mAttachedComputeShader)
     {
-            mInfoLog << "Work group size is not specified.";
-            return NoError();
-        }
-
         if (!linkUniforms(context, mInfoLog, mUniformLocationBindings))
         {
             return NoError();
@@ -890,24 +861,6 @@ Error Program::link(const gl::Context *context)
     }
     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 @@ Error Program::link(const gl::Context *context)
 
         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 @@ Error Program::link(const gl::Context *context)
         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 @@ GLenum Program::getTransformFeedbackBufferMode() const
     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 @@ bool Program::linkVaryings(const Context *context, InfoLog &infoLog) const
             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 @@ bool Program::linkAtomicCounterBuffers()
     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 @@ bool Program::linkAttributes(const Context *context, InfoLog &infoLog)
     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 @@ bool Program::validateVertexAndFragmentInterfaceBlocks(
         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 Program::linkInterfaceBlocks(const Context *context, InfoLog &infoLog)
     }
 
     bool webglCompatibility = context->getExtensions().webglCompatibility;
-    if (!validateVertexAndFragmentInterfaceBlocks(vertexUniformBlocks, fragmentUniformBlocks,
-                                                  infoLog, webglCompatibility))
+    if (!ValidateGraphicsInterfaceBlocks(vertexUniformBlocks, fragmentUniformBlocks, infoLog,
+                                         webglCompatibility))
     {
         return false;
     }
@@ -2446,9 +2462,8 @@ bool Program::linkInterfaceBlocks(const Context *context, InfoLog &infoLog)
             return false;
         }
 
-        if (!validateVertexAndFragmentInterfaceBlocks(vertexShaderStorageBlocks,
-                                                      fragmentShaderStorageBlocks, infoLog,
-                                                      webglCompatibility))
+        if (!ValidateGraphicsInterfaceBlocks(vertexShaderStorageBlocks, fragmentShaderStorageBlocks,
+                                             infoLog, webglCompatibility))
         {
             return false;
         }
@@ -2456,12 +2471,12 @@ bool Program::linkInterfaceBlocks(const Context *context, InfoLog &infoLog)
     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 @@ bool Program::areMatchingInterfaceBlocks(InfoLog &infoLog,
             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 @@ bool Program::areMatchingInterfaceBlocks(InfoLog &infoLog,
     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 @@ bool Program::linkValidateVariablesBase(InfoLog &infoLog, const std::string &var
         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 @@ bool Program::linkValidateVariablesBase(InfoLog &infoLog, const std::string &var
     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;
     }

src/libANGLE/Program.h

@@ -579,7 +579,7 @@ class Program final : angle::NonCopyable, public LabeledObject
     GLsizei getTransformFeedbackVaryingMaxLength() const;
     GLenum getTransformFeedbackBufferMode() const;
 
-    static bool linkValidateInterfaceBlockFields(InfoLog &infoLog,
+    static bool LinkValidateInterfaceBlockFields(InfoLog &infoLog,
                                                  const std::string &uniformName,
                                                  const sh::InterfaceBlockField &vertexUniform,
                                                  const sh::InterfaceBlockField &fragmentUniform,
@@ -614,7 +614,7 @@ class Program final : angle::NonCopyable, public LabeledObject
 
     const ProgramState &getState() const { return mState; }
 
-    static bool linkValidateVariablesBase(InfoLog &infoLog,
+    static bool LinkValidateVariablesBase(InfoLog &infoLog,
                                           const std::string &variableName,
                                           const sh::ShaderVariable &vertexVariable,
                                           const sh::ShaderVariable &fragmentVariable,
@@ -646,12 +646,13 @@ class Program final : angle::NonCopyable, public LabeledObject
 
     void unlink();
 
+    bool linkValidateShaders(const Context *context, InfoLog &infoLog);
     bool linkAttributes(const Context *context, InfoLog &infoLog);
-    bool validateVertexAndFragmentInterfaceBlocks(
+    static bool ValidateGraphicsInterfaceBlocks(
         const std::vector<sh::InterfaceBlock> &vertexInterfaceBlocks,
         const std::vector<sh::InterfaceBlock> &fragmentInterfaceBlocks,
         InfoLog &infoLog,
-        bool webglCompatibility) const;
+        bool webglCompatibility);
     bool linkInterfaceBlocks(const Context *context, InfoLog &infoLog);
     bool linkVaryings(const Context *context, InfoLog &infoLog) const;
 
@@ -663,12 +664,12 @@ class Program final : angle::NonCopyable, public LabeledObject
 
     void updateLinkedShaderStages();
 
-    bool areMatchingInterfaceBlocks(InfoLog &infoLog,
+    static bool AreMatchingInterfaceBlocks(InfoLog &infoLog,
                                            const sh::InterfaceBlock &vertexInterfaceBlock,
                                            const sh::InterfaceBlock &fragmentInterfaceBlock,
-                                    bool webglCompatibility) const;
+                                           bool webglCompatibility);
 
-    static bool linkValidateVaryings(InfoLog &infoLog,
+    static bool LinkValidateVaryings(InfoLog &infoLog,
                                      const std::string &varyingName,
                                      const sh::Varying &vertexVarying,
                                      const sh::Varying &fragmentVarying,

src/libANGLE/ProgramLinkedResources.cpp

@@ -70,7 +70,7 @@ bool UniformLinker::link(const Context *context,
     if (mState.getAttachedVertexShader() && mState.getAttachedFragmentShader())
     {
         ASSERT(mState.getAttachedComputeShader() == nullptr);
-        if (!validateVertexAndFragmentUniforms(context, infoLog))
+        if (!validateGraphicsUniforms(context, infoLog))
         {
             return false;
         }
@@ -96,11 +96,10 @@ bool UniformLinker::link(const Context *context,
     return true;
 }
 
-bool UniformLinker::validateVertexAndFragmentUniforms(const Context *context,
-                                                      InfoLog &infoLog) const
+bool UniformLinker::validateGraphicsUniforms(const Context *context, InfoLog &infoLog) const
 {
     // Check that uniforms defined in the vertex and fragment shaders are identical
-    std::map<std::string, sh::Uniform> linkedUniforms;
+    std::map<std::string, const sh::Uniform *> linkedUniforms;
     const std::vector<sh::Uniform> &vertexUniforms =
         mState.getAttachedVertexShader()->getUniforms(context);
     const std::vector<sh::Uniform> &fragmentUniforms =
@@ -108,7 +107,7 @@ bool UniformLinker::validateVertexAndFragmentUniforms(const Context *context,
 
     for (const sh::Uniform &vertexUniform : vertexUniforms)
     {
-        linkedUniforms[vertexUniform.name] = vertexUniform;
+        linkedUniforms[vertexUniform.name] = &vertexUniform;
     }
 
     for (const sh::Uniform &fragmentUniform : fragmentUniforms)
@@ -116,9 +115,9 @@ bool UniformLinker::validateVertexAndFragmentUniforms(const Context *context,
         auto entry = linkedUniforms.find(fragmentUniform.name);
         if (entry != linkedUniforms.end())
         {
-            const sh::Uniform &linkedUniform = entry->second;
+            const sh::Uniform &linkedUniform = *(entry->second);
             const std::string &uniformName   = "uniform '" + linkedUniform.name + "'";
-            if (!linkValidateUniforms(infoLog, uniformName, linkedUniform, fragmentUniform))
+            if (!LinkValidateUniforms(infoLog, uniformName, linkedUniform, fragmentUniform))
             {
                 return false;
             }
@@ -128,7 +127,7 @@ bool UniformLinker::validateVertexAndFragmentUniforms(const Context *context,
 }
 
 // GLSL ES Spec 3.00.3, section 4.3.5.
-bool UniformLinker::linkValidateUniforms(InfoLog &infoLog,
+bool UniformLinker::LinkValidateUniforms(InfoLog &infoLog,
                                          const std::string &uniformName,
                                          const sh::Uniform &vertexUniform,
                                          const sh::Uniform &fragmentUniform)
@@ -139,7 +138,7 @@ bool UniformLinker::linkValidateUniforms(InfoLog &infoLog,
     const bool validatePrecision = false;
 #endif
 
-    if (!Program::linkValidateVariablesBase(infoLog, uniformName, vertexUniform, fragmentUniform,
+    if (!Program::LinkValidateVariablesBase(infoLog, uniformName, vertexUniform, fragmentUniform,
                                             validatePrecision))
     {
         return false;

src/libANGLE/ProgramLinkedResources.h

@@ -78,9 +78,9 @@ class UniformLinker final : angle::NonCopyable
         unsigned int atomicCounterCount;
     };
 
-    bool validateVertexAndFragmentUniforms(const Context *context, InfoLog &infoLog) const;
+    bool validateGraphicsUniforms(const Context *context, InfoLog &infoLog) const;
 
-    static bool linkValidateUniforms(InfoLog &infoLog,
+    static bool LinkValidateUniforms(InfoLog &infoLog,
                                      const std::string &uniformName,
                                      const sh::Uniform &vertexUniform,
                                      const sh::Uniform &fragmentUniform);