Don't sync the read framebuffer on draw calls.

src/libANGLE/Context.cpp

@@ -449,6 +449,14 @@ void Context::initialize()
     }
 
     // Initialize dirty bit masks
+    mDrawDirtyObjects.set(State::DIRTY_OBJECT_DRAW_FRAMEBUFFER);
+    mDrawDirtyObjects.set(State::DIRTY_OBJECT_VERTEX_ARRAY);
+    mDrawDirtyObjects.set(State::DIRTY_OBJECT_PROGRAM_TEXTURES);
+
+    mPathOperationDirtyObjects.set(State::DIRTY_OBJECT_DRAW_FRAMEBUFFER);
+    mPathOperationDirtyObjects.set(State::DIRTY_OBJECT_VERTEX_ARRAY);
+    mPathOperationDirtyObjects.set(State::DIRTY_OBJECT_PROGRAM_TEXTURES);
+
     mTexImageDirtyBits.set(State::DIRTY_BIT_UNPACK_STATE);
     mTexImageDirtyBits.set(State::DIRTY_BIT_UNPACK_BUFFER_BINDING);
     // No dirty objects.
@@ -2295,8 +2303,7 @@ void Context::stencilFillPath(GLuint path, GLenum fillMode, GLuint mask)
     if (!pathObj)
         return;
 
-    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->stencilFillPath(pathObj, fillMode, mask);
 }
@@ -2307,8 +2314,7 @@ void Context::stencilStrokePath(GLuint path, GLint reference, GLuint mask)
     if (!pathObj)
         return;
 
-    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->stencilStrokePath(pathObj, reference, mask);
 }
@@ -2319,8 +2325,7 @@ void Context::coverFillPath(GLuint path, GLenum coverMode)
     if (!pathObj)
         return;
 
-    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->coverFillPath(pathObj, coverMode);
 }
@@ -2331,8 +2336,7 @@ void Context::coverStrokePath(GLuint path, GLenum coverMode)
     if (!pathObj)
         return;
 
-    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->coverStrokePath(pathObj, coverMode);
 }
@@ -2343,8 +2347,7 @@ void Context::stencilThenCoverFillPath(GLuint path, GLenum fillMode, GLuint mask
     if (!pathObj)
         return;
 
-    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->stencilThenCoverFillPath(pathObj, fillMode, mask, coverMode);
 }
@@ -2358,8 +2361,7 @@ void Context::stencilThenCoverStrokePath(GLuint path,
     if (!pathObj)
         return;
 
-    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->stencilThenCoverStrokePath(pathObj, reference, mask, coverMode);
 }
@@ -2374,8 +2376,7 @@ void Context::coverFillPathInstanced(GLsizei numPaths,
 {
     const auto &pathObjects = GatherPaths(*mState.mPaths, numPaths, pathNameType, paths, pathBase);
 
-    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->coverFillPathInstanced(pathObjects, coverMode, transformType, transformValues);
 }
@@ -2391,7 +2392,7 @@ void Context::coverStrokePathInstanced(GLsizei numPaths,
     const auto &pathObjects = GatherPaths(*mState.mPaths, numPaths, pathNameType, paths, pathBase);
 
     // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->coverStrokePathInstanced(pathObjects, coverMode, transformType,
                                               transformValues);
@@ -2409,7 +2410,7 @@ void Context::stencilFillPathInstanced(GLsizei numPaths,
     const auto &pathObjects = GatherPaths(*mState.mPaths, numPaths, pathNameType, paths, pathBase);
 
     // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->stencilFillPathInstanced(pathObjects, fillMode, mask, transformType,
                                               transformValues);
@@ -2426,8 +2427,7 @@ void Context::stencilStrokePathInstanced(GLsizei numPaths,
 {
     const auto &pathObjects = GatherPaths(*mState.mPaths, numPaths, pathNameType, paths, pathBase);
 
-    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->stencilStrokePathInstanced(pathObjects, reference, mask, transformType,
                                                 transformValues);
@@ -2445,8 +2445,7 @@ void Context::stencilThenCoverFillPathInstanced(GLsizei numPaths,
 {
     const auto &pathObjects = GatherPaths(*mState.mPaths, numPaths, pathNameType, paths, pathBase);
 
-    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->stencilThenCoverFillPathInstanced(pathObjects, coverMode, fillMode, mask,
                                                        transformType, transformValues);
@@ -2464,8 +2463,7 @@ void Context::stencilThenCoverStrokePathInstanced(GLsizei numPaths,
 {
     const auto &pathObjects = GatherPaths(*mState.mPaths, numPaths, pathNameType, paths, pathBase);
 
-    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
-    ANGLE_CONTEXT_TRY(syncState());
+    ANGLE_CONTEXT_TRY(syncStateForPathOperation());
 
     mImplementation->stencilThenCoverStrokePathInstanced(pathObjects, coverMode, reference, mask,
                                                          transformType, transformValues);
@@ -3385,7 +3383,7 @@ Error Context::prepareForDraw(PrimitiveMode mode)
         ANGLE_TRY(mGLES1Renderer->prepareForDraw(mode, this, &mGLState));
     }
 
-    ANGLE_TRY(syncDirtyObjects());
+    ANGLE_TRY(syncDirtyObjects(mDrawDirtyObjects));
 
     if (isRobustResourceInitEnabled())
     {
@@ -3414,13 +3412,6 @@ Error Context::prepareForClearBuffer(GLenum buffer, GLint drawbuffer)
     return NoError();
 }
 
-Error Context::syncState()
-{
-    ANGLE_TRY(syncDirtyObjects());
-    ANGLE_TRY(syncDirtyBits());
-    return NoError();
-}
-
 Error Context::syncState(const State::DirtyBits &bitMask, const State::DirtyObjects &objectMask)
 {
     ANGLE_TRY(syncDirtyObjects(objectMask));
@@ -3444,11 +3435,6 @@ Error Context::syncDirtyBits(const State::DirtyBits &bitMask)
     return NoError();
 }
 
-Error Context::syncDirtyObjects()
-{
-    return mGLState.syncDirtyObjects(this);
-}
-
 Error Context::syncDirtyObjects(const State::DirtyObjects &objectMask)
 {
     return mGLState.syncDirtyObjects(this, objectMask);
@@ -4309,6 +4295,16 @@ Error Context::syncStateForBlit()
     return syncState(mBlitDirtyBits, mBlitDirtyObjects);
 }
 
+Error Context::syncStateForPathOperation()
+{
+    ANGLE_TRY(syncDirtyObjects(mPathOperationDirtyObjects));
+
+    // TODO(svaisanen@nvidia.com): maybe sync only state required for path rendering?
+    ANGLE_TRY(syncDirtyBits());
+
+    return NoError();
+}
+
 void Context::activeShaderProgram(GLuint pipeline, GLuint program)
 {
     UNIMPLEMENTED();

src/libANGLE/Context.h

@@ -1498,15 +1498,14 @@ class Context final : public egl::LabeledObject, angle::NonCopyable
     Error prepareForDraw(PrimitiveMode mode);
     Error prepareForClear(GLbitfield mask);
     Error prepareForClearBuffer(GLenum buffer, GLint drawbuffer);
-    Error syncState();
     Error syncState(const State::DirtyBits &bitMask, const State::DirtyObjects &objectMask);
     Error syncDirtyBits();
     Error syncDirtyBits(const State::DirtyBits &bitMask);
-    Error syncDirtyObjects();
     Error syncDirtyObjects(const State::DirtyObjects &objectMask);
     Error syncStateForReadPixels();
     Error syncStateForTexImage();
     Error syncStateForBlit();
+    Error syncStateForPathOperation();
 
     VertexArray *checkVertexArrayAllocation(GLuint vertexArrayHandle);
     TransformFeedback *checkTransformFeedbackAllocation(GLuint transformFeedback);
@@ -1609,6 +1608,8 @@ class Context final : public egl::LabeledObject, angle::NonCopyable
     const bool mExtensionsEnabled;
     MemoryProgramCache *mMemoryProgramCache;
 
+    State::DirtyObjects mDrawDirtyObjects;
+    State::DirtyObjects mPathOperationDirtyObjects;
     State::DirtyBits mTexImageDirtyBits;
     State::DirtyObjects mTexImageDirtyObjects;
     State::DirtyBits mReadPixelsDirtyBits;

src/libANGLE/State.cpp

@@ -2629,14 +2629,6 @@ bool State::hasMappedBuffer(BufferBinding target) const
     }
 }
 
-Error State::syncDirtyObjects(const Context *context)
-{
-    if (!mDirtyObjects.any())
-        return NoError();
-
-    return syncDirtyObjects(context, mDirtyObjects);
-}
-
 Error State::syncDirtyObjects(const Context *context, const DirtyObjects &bitset)
 {
     const DirtyObjects &dirtyObjects = mDirtyObjects & bitset;
@@ -2882,7 +2874,7 @@ Error State::clearUnclearedActiveTextures(const Context *context)
         return NoError();
     }
 
-    ASSERT(!mDirtyObjects.any());
+    ASSERT(!mDirtyObjects[DIRTY_OBJECT_PROGRAM_TEXTURES]);
 
     for (auto textureIndex : mActiveTexturesMask)
     {

src/libANGLE/State.h

@@ -450,7 +450,6 @@ class State : public angle::ObserverInterface, angle::NonCopyable
     using DirtyObjects = angle::BitSet<DIRTY_OBJECT_MAX>;
     void clearDirtyObjects() { mDirtyObjects.reset(); }
     void setAllDirtyObjects() { mDirtyObjects.set(); }
-    Error syncDirtyObjects(const Context *context);
     Error syncDirtyObjects(const Context *context, const DirtyObjects &bitset);
     Error syncDirtyObject(const Context *context, GLenum target);
     void setObjectDirty(GLenum target);

src/tests/gl_tests/FramebufferTest.cpp

@@ -322,6 +322,46 @@ TEST_P(FramebufferFormatsTest, ZeroHeightRenderbuffer)
     testZeroHeightRenderbuffer();
 }
 
+// Test to cover a bug where the read framebuffer affects the completeness of the draw framebuffer.
+TEST_P(FramebufferFormatsTest, ReadDrawCompleteness)
+{
+    ANGLE_SKIP_TEST_IF(getClientMajorVersion() < 3);
+
+    GLTexture incompleteTexture;
+    glBindTexture(GL_TEXTURE_2D, incompleteTexture);
+
+    GLFramebuffer incompleteFBO;
+    glBindFramebuffer(GL_FRAMEBUFFER, incompleteFBO);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, incompleteTexture,
+                           0);
+    EXPECT_GLENUM_EQ(GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT,
+                     glCheckFramebufferStatus(GL_FRAMEBUFFER));
+
+    GLTexture completeTexture;
+    glBindTexture(GL_TEXTURE_2D, completeTexture);
+    glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, getWindowWidth(), getWindowHeight());
+
+    GLFramebuffer completeFBO;
+    glBindFramebuffer(GL_DRAW_FRAMEBUFFER, completeFBO);
+    glFramebufferTexture2D(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
+                           completeTexture, 0);
+
+    EXPECT_GLENUM_EQ(GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT,
+                     glCheckFramebufferStatus(GL_READ_FRAMEBUFFER));
+    EXPECT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_DRAW_FRAMEBUFFER));
+
+    ASSERT_GL_NO_ERROR();
+
+    // Simple draw program.
+    ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
+
+    drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f, 1.0f, true);
+    EXPECT_GL_NO_ERROR();
+
+    glBindFramebuffer(GL_READ_FRAMEBUFFER, completeFBO);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these
 // tests should be run against.
 ANGLE_INSTANTIATE_TEST(FramebufferFormatsTest,

src/tests/gl_tests/WebGLFramebufferTest.cpp

@@ -144,14 +144,17 @@ TEST_P(WebGLFramebufferTest, TestFramebufferRequiredCombinations)
     checkBufferBits(GL_DEPTH_ATTACHMENT, GL_NONE);
     glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, 0);
 
-    // 3. COLOR_ATTACHMENT0 = RGBA/UNSIGNED_BYTE texture + DEPTH_STENCIL_ATTACHMENT = DEPTH_STENCIL
-    // renderbuffer
-    glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH_STENCIL, width, height);
-    glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER,
-                              renderbuffer);
-    EXPECT_GL_NO_ERROR();
-    checkFramebufferForAllowedStatuses(ALLOW_COMPLETE);
-    checkBufferBits(GL_DEPTH_STENCIL_ATTACHMENT, GL_NONE);
+    if (getClientMajorVersion() == 2)
+    {
+        // 3. COLOR_ATTACHMENT0 = RGBA/UNSIGNED_BYTE texture + DEPTH_STENCIL_ATTACHMENT =
+        // DEPTH_STENCIL renderbuffer
+        glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH_STENCIL, width, height);
+        glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER,
+                                  renderbuffer);
+        EXPECT_GL_NO_ERROR();
+        checkFramebufferForAllowedStatuses(ALLOW_COMPLETE);
+        checkBufferBits(GL_DEPTH_STENCIL_ATTACHMENT, GL_NONE);
+    }
 }
 
 void testAttachment(GLint width,
@@ -370,6 +373,9 @@ void WebGLFramebufferTest::testDepthStencilRenderbuffer(GLint width,
 // Test various attachment combinations with WebGL framebuffers.
 TEST_P(WebGLFramebufferTest, TestAttachments)
 {
+    // GL_DEPTH_STENCIL renderbuffer format is only valid for WebGL1
+    ANGLE_SKIP_TEST_IF(getClientMajorVersion() != 2);
+
     for (GLint width = 2; width <= 2; width += 2)
     {
         for (GLint height = 2; height <= 2; height += 2)
@@ -856,6 +862,9 @@ ANGLE_INSTANTIATE_TEST(WebGLFramebufferTest,
                        ES2_D3D11(),
                        ES2_D3D11_FL9_3(),
                        ES2_OPENGL(),
-                       ES2_OPENGLES());
+                       ES2_OPENGLES(),
+                       ES3_D3D11(),
+                       ES3_OPENGL(),
+                       ES3_OPENGLES());
 
 }  // namespace