Fix separable Geometry Shaders.

src/libANGLE/Context.cpp

@@ -9066,7 +9066,6 @@ void StateCache::setValidDrawModes(bool pointsOK,
 void StateCache::updateValidDrawModes(Context *context)
 {
     const State &state = context->getState();
-    Program *program   = state.getProgram();
 
     if (mCachedTransformFeedbackActiveUnpaused)
     {
@@ -9109,7 +9108,7 @@ void StateCache::updateValidDrawModes(Context *context)
     }
 
     ASSERT(programExecutable->hasLinkedShaderStage(ShaderType::Geometry));
-    PrimitiveMode gsMode = program->getGeometryShaderInputPrimitiveType();
+    PrimitiveMode gsMode = programExecutable->getGeometryShaderInputPrimitiveType();
 
     bool pointsOK  = gsMode == PrimitiveMode::Points;
     bool linesOK   = gsMode == PrimitiveMode::Lines;

src/libANGLE/Program.cpp

@@ -1203,11 +1203,6 @@ ProgramState::ProgramState()
       mBinaryRetrieveableHint(false),
       mSeparable(false),
       mNumViews(-1),
-      // [GL_EXT_geometry_shader] Table 20.22
-      mGeometryShaderInputPrimitiveType(PrimitiveMode::Triangles),
-      mGeometryShaderOutputPrimitiveType(PrimitiveMode::TriangleStrip),
-      mGeometryShaderInvocations(1),
-      mGeometryShaderMaxVertices(0),
       mDrawIDLocation(-1),
       mBaseVertexLocation(-1),
       mBaseInstanceLocation(-1),
@@ -1939,17 +1934,13 @@ void Program::unlink()
     mState.mYUVOutput = false;
     mState.mActiveOutputVariables.reset();
     mState.mComputeShaderLocalSize.fill(1);
-    mState.mNumViews                          = -1;
-    mState.mGeometryShaderInputPrimitiveType  = PrimitiveMode::Triangles;
-    mState.mGeometryShaderOutputPrimitiveType = PrimitiveMode::TriangleStrip;
-    mState.mGeometryShaderInvocations         = 1;
-    mState.mGeometryShaderMaxVertices         = 0;
-    mState.mDrawIDLocation                    = -1;
-    mState.mBaseVertexLocation                = -1;
-    mState.mBaseInstanceLocation              = -1;
-    mState.mCachedBaseVertex                  = 0;
-    mState.mCachedBaseInstance                = 0;
-    mState.mEarlyFramentTestsOptimization     = false;
+    mState.mNumViews                      = -1;
+    mState.mDrawIDLocation                = -1;
+    mState.mBaseVertexLocation            = -1;
+    mState.mBaseInstanceLocation          = -1;
+    mState.mCachedBaseVertex              = 0;
+    mState.mCachedBaseInstance            = 0;
+    mState.mEarlyFramentTestsOptimization = false;
     mState.mSpecConstUsageBits.reset();
 
     mValidated = false;
@@ -2241,23 +2232,23 @@ const sh::WorkGroupSize &Program::getComputeShaderLocalSize() const
 
 PrimitiveMode Program::getGeometryShaderInputPrimitiveType() const
 {
-    ASSERT(!mLinkingState);
-    return mState.mGeometryShaderInputPrimitiveType;
+    ASSERT(!mLinkingState && mState.mExecutable);
+    return mState.mExecutable->getGeometryShaderInputPrimitiveType();
 }
 PrimitiveMode Program::getGeometryShaderOutputPrimitiveType() const
 {
-    ASSERT(!mLinkingState);
-    return mState.mGeometryShaderOutputPrimitiveType;
+    ASSERT(!mLinkingState && mState.mExecutable);
+    return mState.mExecutable->getGeometryShaderOutputPrimitiveType();
 }
 GLint Program::getGeometryShaderInvocations() const
 {
-    ASSERT(!mLinkingState);
-    return mState.mGeometryShaderInvocations;
+    ASSERT(!mLinkingState && mState.mExecutable);
+    return mState.mExecutable->getGeometryShaderInvocations();
 }
 GLint Program::getGeometryShaderMaxVertices() const
 {
-    ASSERT(!mLinkingState);
-    return mState.mGeometryShaderMaxVertices;
+    ASSERT(!mLinkingState && mState.mExecutable);
+    return mState.mExecutable->getGeometryShaderMaxVertices();
 }
 
 const sh::ShaderVariable &Program::getInputResource(size_t index) const
@@ -3502,10 +3493,11 @@ bool Program::linkValidateShaders(InfoLog &infoLog)
                 return false;
             }
 
-            mState.mGeometryShaderInputPrimitiveType  = inputPrimitive.value();
-            mState.mGeometryShaderOutputPrimitiveType = outputPrimitive.value();
-            mState.mGeometryShaderMaxVertices         = maxVertices.value();
-            mState.mGeometryShaderInvocations = geometryShader->getGeometryShaderInvocations();
+            mState.mExecutable->mGeometryShaderInputPrimitiveType  = inputPrimitive.value();
+            mState.mExecutable->mGeometryShaderOutputPrimitiveType = outputPrimitive.value();
+            mState.mExecutable->mGeometryShaderMaxVertices         = maxVertices.value();
+            mState.mExecutable->mGeometryShaderInvocations =
+                geometryShader->getGeometryShaderInvocations();
         }
     }
 
@@ -5189,12 +5181,6 @@ angle::Result Program::serialize(const Context *context, angle::MemoryBuffer *bi
     stream.writeInt(computeLocalSize[1]);
     stream.writeInt(computeLocalSize[2]);
 
-    ASSERT(mState.mGeometryShaderInvocations >= 1 && mState.mGeometryShaderMaxVertices >= 0);
-    stream.writeEnum(mState.mGeometryShaderInputPrimitiveType);
-    stream.writeEnum(mState.mGeometryShaderOutputPrimitiveType);
-    stream.writeInt(mState.mGeometryShaderInvocations);
-    stream.writeInt(mState.mGeometryShaderMaxVertices);
-
     stream.writeInt(mState.mNumViews);
     stream.writeBool(mState.mEarlyFramentTestsOptimization);
     stream.writeInt(mState.mSpecConstUsageBits.bits());
@@ -5389,11 +5375,6 @@ angle::Result Program::deserialize(const Context *context,
     mState.mComputeShaderLocalSize[1] = stream.readInt<int>();
     mState.mComputeShaderLocalSize[2] = stream.readInt<int>();
 
-    mState.mGeometryShaderInputPrimitiveType  = stream.readEnum<PrimitiveMode>();
-    mState.mGeometryShaderOutputPrimitiveType = stream.readEnum<PrimitiveMode>();
-    mState.mGeometryShaderInvocations         = stream.readInt<int>();
-    mState.mGeometryShaderMaxVertices         = stream.readInt<int>();
-
     mState.mNumViews                      = stream.readInt<int>();
     mState.mEarlyFramentTestsOptimization = stream.readBool();
     mState.mSpecConstUsageBits            = rx::SpecConstUsageBits(stream.readInt<uint32_t>());

src/libANGLE/Program.h

@@ -356,20 +356,6 @@ class ProgramState final : angle::NonCopyable
 
     bool isSeparable() const { return mSeparable; }
 
-    PrimitiveMode getGeometryShaderInputPrimitiveType() const
-    {
-        return mGeometryShaderInputPrimitiveType;
-    }
-
-    PrimitiveMode getGeometryShaderOutputPrimitiveType() const
-    {
-        return mGeometryShaderOutputPrimitiveType;
-    }
-
-    int getGeometryShaderInvocations() const { return mGeometryShaderInvocations; }
-
-    int getGeometryShaderMaxVertices() const { return mGeometryShaderMaxVertices; }
-
     int getDrawIDLocation() const { return mDrawIDLocation; }
 
     int getBaseVertexLocation() const { return mBaseVertexLocation; }
@@ -428,12 +414,6 @@ class ProgramState final : angle::NonCopyable
     // ANGLE_multiview.
     int mNumViews;
 
-    // GL_EXT_geometry_shader.
-    PrimitiveMode mGeometryShaderInputPrimitiveType;
-    PrimitiveMode mGeometryShaderOutputPrimitiveType;
-    int mGeometryShaderInvocations;
-    int mGeometryShaderMaxVertices;
-
     // GL_ANGLE_multi_draw
     int mDrawIDLocation;
 

src/libANGLE/ProgramExecutable.cpp

@@ -41,7 +41,12 @@ ProgramExecutable::ProgramExecutable()
       mPipelineHasComputeTextures(false),
       mPipelineHasGraphicsImages(false),
       mPipelineHasComputeImages(false),
-      mIsCompute(false)
+      mIsCompute(false),
+      // [GL_EXT_geometry_shader] Table 20.22
+      mGeometryShaderInputPrimitiveType(PrimitiveMode::Triangles),
+      mGeometryShaderOutputPrimitiveType(PrimitiveMode::TriangleStrip),
+      mGeometryShaderInvocations(1),
+      mGeometryShaderMaxVertices(0)
 {
     reset();
 }
@@ -136,6 +141,11 @@ void ProgramExecutable::reset()
     mPipelineHasComputeDefaultUniforms       = false;
     mPipelineHasGraphicsTextures             = false;
     mPipelineHasComputeTextures              = false;
+
+    mGeometryShaderInputPrimitiveType  = PrimitiveMode::Triangles;
+    mGeometryShaderOutputPrimitiveType = PrimitiveMode::TriangleStrip;
+    mGeometryShaderInvocations         = 1;
+    mGeometryShaderMaxVertices         = 0;
 }
 
 void ProgramExecutable::load(gl::BinaryInputStream *stream)
@@ -162,6 +172,11 @@ void ProgramExecutable::load(gl::BinaryInputStream *stream)
     mPipelineHasComputeDefaultUniforms       = stream->readBool();
     mPipelineHasGraphicsTextures             = stream->readBool();
     mPipelineHasComputeTextures              = stream->readBool();
+
+    mGeometryShaderInputPrimitiveType  = stream->readEnum<PrimitiveMode>();
+    mGeometryShaderOutputPrimitiveType = stream->readEnum<PrimitiveMode>();
+    mGeometryShaderInvocations         = stream->readInt<int>();
+    mGeometryShaderMaxVertices         = stream->readInt<int>();
 }
 
 void ProgramExecutable::save(gl::BinaryOutputStream *stream) const
@@ -187,6 +202,12 @@ void ProgramExecutable::save(gl::BinaryOutputStream *stream) const
     stream->writeBool(mPipelineHasComputeDefaultUniforms);
     stream->writeBool(mPipelineHasGraphicsTextures);
     stream->writeBool(mPipelineHasComputeTextures);
+
+    ASSERT(mGeometryShaderInvocations >= 1 && mGeometryShaderMaxVertices >= 0);
+    stream->writeEnum(mGeometryShaderInputPrimitiveType);
+    stream->writeEnum(mGeometryShaderOutputPrimitiveType);
+    stream->writeInt(mGeometryShaderInvocations);
+    stream->writeInt(mGeometryShaderMaxVertices);
 }
 
 int ProgramExecutable::getInfoLogLength() const

src/libANGLE/ProgramExecutable.h

@@ -310,6 +310,20 @@ class ProgramExecutable final : public angle::Subject
 
     bool isYUVOutput() const;
 
+    PrimitiveMode getGeometryShaderInputPrimitiveType() const
+    {
+        return mGeometryShaderInputPrimitiveType;
+    }
+
+    PrimitiveMode getGeometryShaderOutputPrimitiveType() const
+    {
+        return mGeometryShaderOutputPrimitiveType;
+    }
+
+    int getGeometryShaderInvocations() const { return mGeometryShaderInvocations; }
+
+    int getGeometryShaderMaxVertices() const { return mGeometryShaderMaxVertices; }
+
   private:
     // TODO(timvp): http://anglebug.com/3570: Investigate removing these friend
     // class declarations and accessing the necessary members with getters/setters.
@@ -406,6 +420,12 @@ class ProgramExecutable final : public angle::Subject
     ShaderMap<std::vector<sh::ShaderVariable>> mLinkedOutputVaryings;
     ShaderMap<std::vector<sh::ShaderVariable>> mLinkedInputVaryings;
     ShaderMap<int> mLinkedShaderVersions;
+
+    // GL_EXT_geometry_shader.
+    PrimitiveMode mGeometryShaderInputPrimitiveType;
+    PrimitiveMode mGeometryShaderOutputPrimitiveType;
+    int mGeometryShaderInvocations;
+    int mGeometryShaderMaxVertices;
 };
 
 }  // namespace gl

src/libANGLE/frame_capture_utils.cpp

@@ -766,10 +766,6 @@ void SerializeProgramState(gl::BinaryOutputStream *bos, const gl::ProgramState &
     bos->writeBool(programState.isSeparable());
     bos->writeBool(programState.hasEarlyFragmentTestsOptimization());
     bos->writeInt(programState.getNumViews());
-    bos->writeEnum(programState.getGeometryShaderInputPrimitiveType());
-    bos->writeEnum(programState.getGeometryShaderOutputPrimitiveType());
-    bos->writeInt(programState.getGeometryShaderInvocations());
-    bos->writeInt(programState.getGeometryShaderMaxVertices());
     bos->writeInt(programState.getDrawIDLocation());
     bos->writeInt(programState.getBaseVertexLocation());
     bos->writeInt(programState.getBaseInstanceLocation());

src/libANGLE/validationES31.cpp

@@ -1697,8 +1697,14 @@ bool ValidateUseProgramStagesBase(const Context *context,
 {
     // GL_INVALID_VALUE is generated if shaders contains set bits that are not recognized, and is
     // not the reserved value GL_ALL_SHADER_BITS.
-    const GLbitfield knownShaderBits =
+    GLbitfield knownShaderBits =
         GL_VERTEX_SHADER_BIT | GL_FRAGMENT_SHADER_BIT | GL_COMPUTE_SHADER_BIT;
+
+    if (context->getClientVersion() == ES_3_2 || context->getExtensions().geometryShader)
+    {
+        knownShaderBits |= GL_GEOMETRY_SHADER_BIT;
+    }
+
     if ((stages & ~knownShaderBits) && (stages != GL_ALL_SHADER_BITS))
     {
         context->validationError(GL_INVALID_VALUE, kUnrecognizedShaderStageBit);

src/tests/gl_tests/GeometryShaderTest.cpp

@@ -1489,6 +1489,146 @@ void main()
     EXPECT_PIXEL_RECT_EQ(0, 0, w, h, GLColor::blue);
 }
 
+// Tests separating the VS from the GS/FS and then modifying the shader.
+TEST_P(GeometryShaderTest, RecompileSeparableVSWithVaryings)
+{
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_geometry_shader"));
+
+    // Errors in D3D11/GL. No plans to fix this.
+    ANGLE_SKIP_TEST_IF(!IsVulkan());
+
+    // http://anglebug.com/5506
+    ANGLE_SKIP_TEST_IF(IsVulkan());
+
+    const char *kVS = R"(#version 310 es
+precision mediump float;
+in vec4 position;
+out vec4 vgVarying;
+uniform vec4 uniVec;
+void main()
+{
+   vgVarying = uniVec;
+   gl_Position = position;
+})";
+
+    const char *kGS = R"(#version 310 es
+#extension GL_EXT_geometry_shader : require
+
+precision mediump float;
+
+layout (triangles) in;
+layout (triangle_strip, max_vertices = 4) out;
+
+in vec4 vgVarying[];
+layout(location = 5) out vec4 gfVarying;
+
+void main()
+{
+    for (int n = 0; n < gl_in.length(); n++)
+    {
+        gl_Position = gl_in[n].gl_Position;
+        gfVarying = vgVarying[n];
+        EmitVertex();
+    }
+    EndPrimitive();
+})";
+
+    const char *kFS = R"(#version 310 es
+precision mediump float;
+
+layout(location = 5) in vec4 gfVarying;
+out vec4 fOut;
+
+void main()
+{
+    fOut = gfVarying;
+})";
+
+    GLShader vertShader(GL_VERTEX_SHADER);
+    glShaderSource(vertShader, 1, &kVS, nullptr);
+    glCompileShader(vertShader);
+
+    GLProgram vertProg;
+    glProgramParameteri(vertProg, GL_PROGRAM_SEPARABLE, GL_TRUE);
+    glAttachShader(vertProg, vertShader);
+    glLinkProgram(vertProg);
+    ASSERT_GL_NO_ERROR();
+
+    GLShader geomShader(GL_GEOMETRY_SHADER);
+    glShaderSource(geomShader, 1, &kGS, nullptr);
+    glCompileShader(geomShader);
+
+    GLShader fragShader(GL_FRAGMENT_SHADER);
+    glShaderSource(fragShader, 1, &kFS, nullptr);
+    glCompileShader(fragShader);
+
+    GLProgram geomFragProg;
+    glProgramParameteri(geomFragProg, GL_PROGRAM_SEPARABLE, GL_TRUE);
+    glAttachShader(geomFragProg, geomShader);
+    glAttachShader(geomFragProg, fragShader);
+    glLinkProgram(geomFragProg);
+    ASSERT_GL_NO_ERROR();
+
+    GLProgramPipeline pipeline;
+    glUseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vertProg);
+    glUseProgramStages(pipeline, GL_GEOMETRY_SHADER_BIT, geomFragProg);
+    glUseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, geomFragProg);
+    glBindProgramPipeline(pipeline);
+
+    glActiveShaderProgram(pipeline, vertProg);
+    GLint uniLoc = glGetUniformLocation(vertProg, "uniVec");
+    ASSERT_NE(-1, uniLoc);
+    glUniform4f(uniLoc, 0, 1, 0, 1);
+    ASSERT_GL_NO_ERROR();
+
+    drawQuadPPO(vertProg, "position", 0.5f, 1.0f);
+    EXPECT_GL_NO_ERROR();
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+
+    // Do it again with deleted shaders.
+    vertProg.reset();
+    geomFragProg.reset();
+    pipeline.reset();
+
+    glProgramParameteri(vertProg, GL_PROGRAM_SEPARABLE, GL_TRUE);
+    glAttachShader(vertProg, vertShader);
+    glLinkProgram(vertProg);
+
+    // Mess up the VS.
+    const char *otherVS = essl1_shaders::vs::Texture2D();
+    glShaderSource(vertShader, 1, &otherVS, nullptr);
+    glCompileShader(vertShader);
+
+    ASSERT_GL_NO_ERROR();
+
+    glProgramParameteri(geomFragProg, GL_PROGRAM_SEPARABLE, GL_TRUE);
+    glAttachShader(geomFragProg, geomShader);
+    glAttachShader(geomFragProg, fragShader);
+    glLinkProgram(geomFragProg);
+
+    // Mess up the FS.
+    const char *otherFS = essl1_shaders::fs::Texture2D();
+    glShaderSource(fragShader, 1, &otherFS, nullptr);
+    glCompileShader(fragShader);
+
+    ASSERT_GL_NO_ERROR();
+
+    glUseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vertProg);
+    glUseProgramStages(pipeline, GL_GEOMETRY_SHADER_BIT, geomFragProg);
+    glUseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, geomFragProg);
+    glBindProgramPipeline(pipeline);
+
+    glActiveShaderProgram(pipeline, vertProg);
+    uniLoc = glGetUniformLocation(vertProg, "uniVec");
+    ASSERT_NE(-1, uniLoc);
+    glUniform4f(uniLoc, 0, 1, 0, 1);
+    ASSERT_GL_NO_ERROR();
+
+    drawQuadPPO(vertProg, "position", 0.5f, 1.0f);
+    EXPECT_GL_NO_ERROR();
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
 ANGLE_INSTANTIATE_TEST_ES3(GeometryShaderTestES3);
 ANGLE_INSTANTIATE_TEST_ES31_AND(GeometryShaderTest,
                                 WithEmulatedPrerotation(ES31_VULKAN(), 90),

src/tests/test_utils/gl_raii.h

@@ -147,6 +147,15 @@ class GLShader : angle::NonCopyable
 
     operator GLuint() { return get(); }
 
+    void reset()
+    {
+        if (mHandle)
+        {
+            glDeleteShader(mHandle);
+            mHandle = 0;
+        }
+    }
+
   private:
     GLuint mHandle;
 };
@@ -157,13 +166,7 @@ class GLProgram
   public:
     GLProgram() : mHandle(0) {}
 
-    ~GLProgram()
-    {
-        if (mHandle)
-        {
-            glDeleteProgram(mHandle);
-        }
-    }
+    ~GLProgram() { reset(); }
 
     void makeEmpty() { mHandle = glCreateProgram(); }
 
@@ -202,7 +205,23 @@ class GLProgram
 
     bool valid() const { return mHandle != 0; }
 
-    GLuint get() { return mHandle; }
+    GLuint get()
+    {
+        if (!mHandle)
+        {
+            makeEmpty();
+        }
+        return mHandle;
+    }
+
+    void reset()
+    {
+        if (mHandle)
+        {
+            glDeleteProgram(mHandle);
+            mHandle = 0;
+        }
+    }
 
     operator GLuint() { return get(); }