Link interface blocks in ProgramImpl::link.

src/libANGLE/Program.cpp

@@ -871,7 +871,7 @@ Error Program::link(const gl::Context *context)
     }
 
     gatherAtomicCounterBuffers();
-    gatherInterfaceBlockInfo(context);
+    initInterfaceBlockBindings();
 
     setUniformValuesFromBindingQualifiers();
 
@@ -2874,55 +2874,8 @@ void Program::gatherAtomicCounterBuffers()
     // TODO(jie.a.chen@intel.com): Get the actual BUFFER_DATA_SIZE from backend for each buffer.
 }
 
-void Program::gatherUniformBlockInfo(const gl::Context *context)
+void Program::initInterfaceBlockBindings()
 {
-    UniformBlockLinker blockLinker(&mState.mUniformBlocks, &mState.mUniforms);
-    InitUniformBlockLinker(context, mState, &blockLinker);
-
-    auto getImplBlockSize = [this](const std::string &name, const std::string &mappedName,
-                                   size_t *sizeOut) {
-        return this->mProgram->getUniformBlockSize(name, mappedName, sizeOut);
-    };
-
-    auto getImplMemberInfo = [this](const std::string &name, const std::string &mappedName,
-                                    sh::BlockMemberInfo *infoOut) {
-        return this->mProgram->getUniformBlockMemberInfo(name, mappedName, infoOut);
-    };
-
-    blockLinker.linkBlocks(getImplBlockSize, getImplMemberInfo);
-}
-
-void Program::gatherShaderStorageBlockInfo(const gl::Context *context)
-{
-    ShaderStorageBlockLinker blockLinker(&mState.mShaderStorageBlocks);
-    InitShaderStorageBlockLinker(context, mState, &blockLinker);
-
-    // We don't have a way of determining block info for shader storage blocks yet.
-    // TODO(jiajia.qin@intel.com): Determine correct block size and layout.
-    auto getImplBlockSize = [this](const std::string &name, const std::string &mappedName,
-                                   size_t *sizeOut) {
-        return this->mProgram->getUniformBlockSize(name, mappedName, sizeOut);
-    };
-
-    auto getImplMemberInfo = [this](const std::string &name, const std::string &mappedName,
-                                    sh::BlockMemberInfo *infoOut) {
-        return this->mProgram->getUniformBlockMemberInfo(name, mappedName, infoOut);
-    };
-
-    blockLinker.linkBlocks(getImplBlockSize, getImplMemberInfo);
-}
-
-void Program::gatherInterfaceBlockInfo(const Context *context)
-{
-    ASSERT(mState.mUniformBlocks.empty());
-    ASSERT(mState.mShaderStorageBlocks.empty());
-
-    gatherUniformBlockInfo(context);
-    if (context->getClientVersion() >= Version(3, 1))
-    {
-        gatherShaderStorageBlockInfo(context);
-    }
-
     // Set initial bindings from shader.
     for (unsigned int blockIndex = 0; blockIndex < mState.mUniformBlocks.size(); blockIndex++)
     {

src/libANGLE/Program.h

@@ -640,9 +640,7 @@ class Program final : angle::NonCopyable, public LabeledObject
     void setUniformValuesFromBindingQualifiers();
 
     void gatherAtomicCounterBuffers();
-    void gatherUniformBlockInfo(const gl::Context *context);
-    void gatherShaderStorageBlockInfo(const gl::Context *context);
-    void gatherInterfaceBlockInfo(const Context *context);
+    void initInterfaceBlockBindings();
 
     // Both these function update the cached uniform values and return a modified "count"
     // so that the uniform update doesn't overflow the uniform.

src/libANGLE/ProgramLinkedResources.cpp

@@ -637,6 +637,8 @@ void InterfaceBlockLinker::addShaderBlocks(GLenum shader,
 void InterfaceBlockLinker::linkBlocks(const GetBlockSize &getBlockSize,
                                       const GetBlockMemberInfo &getMemberInfo) const
 {
+    ASSERT(mBlocksOut->empty());
+
     std::set<std::string> visitedList;
 
     for (const auto &shaderBlocks : mShaderBlocks)

src/libANGLE/renderer/ProgramImpl.h

@@ -83,17 +83,6 @@ class ProgramImpl : angle::NonCopyable
     // TODO: synchronize in syncState when dirty bits exist.
     virtual void setUniformBlockBinding(GLuint uniformBlockIndex, GLuint uniformBlockBinding) = 0;
 
-    // May only be called after a successful link operation.
-    // Return false for inactive blocks.
-    virtual bool getUniformBlockSize(const std::string &blockName,
-                                     const std::string &blockMappedName,
-                                     size_t *sizeOut) const = 0;
-
-    // May only be called after a successful link operation.
-    // Returns false for inactive members.
-    virtual bool getUniformBlockMemberInfo(const std::string &memberUniformName,
-                                           const std::string &memberUniformMappedName,
-                                           sh::BlockMemberInfo *memberInfoOut) const = 0;
     // CHROMIUM_path_rendering
     // Set parameters to control fragment shader input variable interpolation
     virtual void setPathFragmentInputGen(const std::string &inputName,

src/libANGLE/renderer/ProgramImpl_mock.h

@@ -63,10 +63,6 @@ class MockProgramImpl : public rx::ProgramImpl
     MOCK_CONST_METHOD3(getUniformuiv, void(const gl::Context *, GLint, GLuint *));
 
     MOCK_METHOD2(setUniformBlockBinding, void(GLuint, GLuint));
-    MOCK_CONST_METHOD3(getUniformBlockSize,
-                       bool(const std::string &, const std::string &, size_t *));
-    MOCK_CONST_METHOD3(getUniformBlockMemberInfo,
-                       bool(const std::string &, const std::string &, sh::BlockMemberInfo *));
     MOCK_METHOD4(setPathFragmentInputGen,
                  void(const std::string &, GLenum, GLint, const GLfloat *));
 

src/libANGLE/renderer/d3d/ProgramD3D.h

@@ -196,12 +196,6 @@ class ProgramD3D : public ProgramImpl
                         gl::InfoLog &infoLog) override;
     GLboolean validate(const gl::Caps &caps, gl::InfoLog *infoLog) override;
 
-    bool getUniformBlockSize(const std::string &blockName,
-                             const std::string &blockMappedName,
-                             size_t *sizeOut) const override;
-    bool getUniformBlockMemberInfo(const std::string &memberUniformName,
-                                   const std::string &memberUniformMappedName,
-                                   sh::BlockMemberInfo *memberInfoOut) const override;
     void setPathFragmentInputGen(const std::string &inputName,
                                  GLenum genMode,
                                  GLint components,
@@ -428,16 +422,15 @@ class ProgramD3D : public ProgramImpl
     void initAttribLocationsToD3DSemantic(const gl::Context *context);
 
     void reset();
-    void ensureUniformBlocksInitialized();
-
-    void initUniformBlockInfo(const gl::Context *context, gl::Shader *shader);
-    size_t getUniformBlockInfo(const sh::InterfaceBlock &interfaceBlock);
+    void initializeUniformBlocks();
 
     void updateCachedInputLayoutFromShader(const gl::Context *context);
     void updateCachedOutputLayoutFromShader();
     void updateCachedVertexExecutableIndex();
     void updateCachedPixelExecutableIndex();
 
+    void linkResources(const gl::Context *context, const gl::ProgramLinkedResources &resources);
+
     RendererD3D *mRenderer;
     DynamicHLSL *mDynamicHLSL;
 
@@ -498,9 +491,6 @@ class ProgramD3D : public ProgramImpl
     bool mFragmentUniformsDirty;
     bool mComputeUniformsDirty;
 
-    std::map<std::string, sh::BlockMemberInfo> mBlockInfo;
-    std::map<std::string, size_t> mBlockDataSizes;
-
     static unsigned int issueSerial();
     static unsigned int mCurrentSerial;
 

src/libANGLE/renderer/gl/ProgramGL.cpp

@@ -14,6 +14,7 @@
 #include "common/string_utils.h"
 #include "common/utilities.h"
 #include "libANGLE/Context.h"
+#include "libANGLE/ProgramLinkedResources.h"
 #include "libANGLE/Uniform.h"
 #include "libANGLE/renderer/gl/ContextGL.h"
 #include "libANGLE/renderer/gl/FunctionsGL.h"
@@ -212,6 +213,7 @@ gl::LinkResult ProgramGL::link(const gl::Context *context,
         mStateManager->forceUseProgram(mProgramID);
     }
 
+    linkResources(resources);
     postLink();
 
     return true;
@@ -805,4 +807,35 @@ void ProgramGL::markUnusedUniformLocations(std::vector<gl::VariableLocation> *un
     }
 }
 
+void ProgramGL::linkResources(const gl::ProgramLinkedResources &resources)
+{
+    // Gather interface block info.
+    auto getUniformBlockSize = [this](const std::string &name, const std::string &mappedName,
+                                      size_t *sizeOut) {
+        return this->getUniformBlockSize(name, mappedName, sizeOut);
+    };
+
+    auto getUniformBlockMemberInfo = [this](const std::string &name, const std::string &mappedName,
+                                            sh::BlockMemberInfo *infoOut) {
+        return this->getUniformBlockMemberInfo(name, mappedName, infoOut);
+    };
+
+    resources.uniformBlockLinker.linkBlocks(getUniformBlockSize, getUniformBlockMemberInfo);
+
+    // TODO(jiajia.qin@intel.com): Determine correct shader storage block info.
+    auto getShaderStorageBlockSize = [](const std::string &name, const std::string &mappedName,
+                                        size_t *sizeOut) {
+        *sizeOut = 0;
+        return true;
+    };
+
+    auto getShaderStorageBlockMemberInfo =
+        [](const std::string &name, const std::string &mappedName, sh::BlockMemberInfo *infoOut) {
+            *infoOut = sh::BlockMemberInfo::getDefaultBlockInfo();
+            return true;
+        };
+    resources.shaderStorageBlockLinker.linkBlocks(getShaderStorageBlockSize,
+                                                  getShaderStorageBlockMemberInfo);
+}
+
 }  // namespace rx

src/libANGLE/renderer/gl/ProgramGL.h

@@ -71,13 +71,6 @@ class ProgramGL : public ProgramImpl
 
     void setUniformBlockBinding(GLuint uniformBlockIndex, GLuint uniformBlockBinding) override;
 
-    bool getUniformBlockSize(const std::string &blockName,
-                             const std::string &blockMappedName,
-                             size_t *sizeOut) const override;
-    bool getUniformBlockMemberInfo(const std::string &memberUniformName,
-                                   const std::string &memberUniformMappedName,
-                                   sh::BlockMemberInfo *memberInfoOut) const override;
-
     void setPathFragmentInputGen(const std::string &inputName,
                                  GLenum genMode,
                                  GLint components,
@@ -97,6 +90,14 @@ class ProgramGL : public ProgramImpl
     void postLink();
     void reapplyUBOBindingsIfNeeded(const gl::Context *context);
 
+    bool getUniformBlockSize(const std::string &blockName,
+                             const std::string &blockMappedName,
+                             size_t *sizeOut) const;
+    bool getUniformBlockMemberInfo(const std::string &memberUniformName,
+                                   const std::string &memberUniformMappedName,
+                                   sh::BlockMemberInfo *memberInfoOut) const;
+    void linkResources(const gl::ProgramLinkedResources &resources);
+
     // Helper function, makes it simpler to type.
     GLint uniLoc(GLint glLocation) const { return mUniformRealLocationMap[glLocation]; }
 

src/libANGLE/renderer/null/ProgramNULL.cpp

@@ -183,23 +183,6 @@ void ProgramNULL::setUniformBlockBinding(GLuint uniformBlockIndex, GLuint unifor
 {
 }
 
-bool ProgramNULL::getUniformBlockSize(const std::string &blockName,
-                                      const std::string &blockMappedName,
-                                      size_t *sizeOut) const
-{
-    // TODO(geofflang): Compute reasonable sizes?
-    *sizeOut = 0;
-    return true;
-}
-
-bool ProgramNULL::getUniformBlockMemberInfo(const std::string &memberUniformName,
-                                            const std::string &memberUniformMappedName,
-                                            sh::BlockMemberInfo *memberInfoOut) const
-{
-    // TODO(geofflang): Compute reasonable values?
-    return true;
-}
-
 void ProgramNULL::setPathFragmentInputGen(const std::string &inputName,
                                           GLenum genMode,
                                           GLint components,

src/libANGLE/renderer/null/ProgramNULL.h

@@ -89,17 +89,6 @@ class ProgramNULL : public ProgramImpl
     // TODO: synchronize in syncState when dirty bits exist.
     void setUniformBlockBinding(GLuint uniformBlockIndex, GLuint uniformBlockBinding) override;
 
-    // May only be called after a successful link operation.
-    // Return false for inactive blocks.
-    bool getUniformBlockSize(const std::string &blockName,
-                             const std::string &blockMappedName,
-                             size_t *sizeOut) const override;
-
-    // May only be called after a successful link operation.
-    // Returns false for inactive members.
-    bool getUniformBlockMemberInfo(const std::string &memberUniformName,
-                                   const std::string &memberUniformMappedName,
-                                   sh::BlockMemberInfo *memberInfoOut) const override;
     // CHROMIUM_path_rendering
     // Set parameters to control fragment shader input variable interpolation
     void setPathFragmentInputGen(const std::string &inputName,

src/libANGLE/renderer/vulkan/ProgramVk.cpp

@@ -528,22 +528,6 @@ void ProgramVk::setUniformBlockBinding(GLuint uniformBlockIndex, GLuint uniformB
     UNIMPLEMENTED();
 }
 
-bool ProgramVk::getUniformBlockSize(const std::string &blockName,
-                                    const std::string &blockMappedName,
-                                    size_t *sizeOut) const
-{
-    UNIMPLEMENTED();
-    return bool();
-}
-
-bool ProgramVk::getUniformBlockMemberInfo(const std::string &memberUniformName,
-                                          const std::string &memberUniformMappedName,
-                                          sh::BlockMemberInfo *memberInfoOut) const
-{
-    UNIMPLEMENTED();
-    return bool();
-}
-
 void ProgramVk::setPathFragmentInputGen(const std::string &inputName,
                                         GLenum genMode,
                                         GLint components,

src/libANGLE/renderer/vulkan/ProgramVk.h

@@ -94,18 +94,6 @@ class ProgramVk : public ProgramImpl
     // TODO: synchronize in syncState when dirty bits exist.
     void setUniformBlockBinding(GLuint uniformBlockIndex, GLuint uniformBlockBinding) override;
 
-    // May only be called after a successful link operation.
-    // Return false for inactive blocks.
-    bool getUniformBlockSize(const std::string &blockName,
-                             const std::string &blockMappedName,
-                             size_t *sizeOut) const override;
-
-    // May only be called after a successful link operation.
-    // Returns false for inactive members.
-    bool getUniformBlockMemberInfo(const std::string &memberUniformName,
-                                   const std::string &memberUniformMappedName,
-                                   sh::BlockMemberInfo *memberInfoOut) const override;
-
     void setPathFragmentInputGen(const std::string &inputName,
                                  GLenum genMode,
                                  GLint components,

src/tests/gl_tests/UniformBufferTest.cpp

@@ -1156,6 +1156,46 @@ TEST_P(UniformBufferTest, Std140UniformBlockInstanceWithNestedStructsContainingV
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Tests the detaching shaders from the program and using uniform blocks works.
+// This covers a bug in ANGLE's D3D back-end.
+TEST_P(UniformBufferTest, DetachShaders)
+{
+    GLuint vertexShader = CompileShader(GL_VERTEX_SHADER, mVertexShaderSource);
+    ASSERT_NE(0u, vertexShader);
+    GLuint fragmentShader = CompileShader(GL_FRAGMENT_SHADER, mFragmentShaderSource);
+    ASSERT_NE(0u, fragmentShader);
+
+    GLuint program = glCreateProgram();
+    glAttachShader(program, vertexShader);
+    glAttachShader(program, fragmentShader);
+
+    ASSERT_TRUE(LinkAttachedProgram(program));
+
+    glDetachShader(program, vertexShader);
+    glDetachShader(program, fragmentShader);
+    glDeleteShader(vertexShader);
+    glDeleteShader(fragmentShader);
+
+    glClear(GL_COLOR_BUFFER_BIT);
+    float floatData[4] = {0.5f, 0.75f, 0.25f, 1.0f};
+
+    glBindBuffer(GL_UNIFORM_BUFFER, mUniformBuffer);
+    glBufferData(GL_UNIFORM_BUFFER, sizeof(float) * 4, floatData, GL_STATIC_DRAW);
+
+    glBindBufferBase(GL_UNIFORM_BUFFER, 0, mUniformBuffer);
+
+    GLint uniformBufferIndex = glGetUniformBlockIndex(mProgram, "uni");
+    ASSERT_NE(uniformBufferIndex, -1);
+
+    glUniformBlockBinding(program, uniformBufferIndex, 0);
+    drawQuad(program, "position", 0.5f);
+
+    ASSERT_GL_NO_ERROR();
+    EXPECT_PIXEL_NEAR(0, 0, 128, 191, 64, 255, 1);
+
+    glDeleteProgram(program);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 ANGLE_INSTANTIATE_TEST(UniformBufferTest,
                        ES3_D3D11(),