Framebuffer: Handle errors in checkStatus.

src/libANGLE/Context.cpp

@@ -1593,7 +1593,7 @@ void Context::getIntegervImpl(GLenum pname, GLint *params)
             *params = mCaps.maxGeometryShaderStorageBlocks;
             break;
         default:
-            mGLState.getIntegerv(this, pname, params);
+            handleError(mGLState.getIntegerv(this, pname, params));
             break;
     }
 }
@@ -3339,7 +3339,9 @@ void Context::invalidateFramebuffer(GLenum target,
     Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target);
     ASSERT(framebuffer);
 
-    if (framebuffer->checkStatus(this) != GL_FRAMEBUFFER_COMPLETE)
+    bool complete = false;
+    ANGLE_CONTEXT_TRY(framebuffer->isComplete(this, &complete));
+    if (!complete)
     {
         return;
     }
@@ -3361,7 +3363,9 @@ void Context::invalidateSubFramebuffer(GLenum target,
     Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target);
     ASSERT(framebuffer);
 
-    if (framebuffer->checkStatus(this) != GL_FRAMEBUFFER_COMPLETE)
+    bool complete = false;
+    ANGLE_CONTEXT_TRY(framebuffer->isComplete(this, &complete));
+    if (!complete)
     {
         return;
     }
@@ -4419,7 +4423,14 @@ GLenum Context::checkFramebufferStatus(GLenum target)
     Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target);
     ASSERT(framebuffer);
 
-    return framebuffer->checkStatus(this);
+    GLenum status = GL_NONE;
+    Error err     = framebuffer->checkStatus(this, &status);
+    if (err.isError())
+    {
+        handleError(err);
+        return 0;
+    }
+    return status;
 }
 
 void Context::compileShader(GLuint shader)

src/libANGLE/Framebuffer.cpp

@@ -981,7 +981,7 @@ void Framebuffer::invalidateCompletenessCache()
     }
 }
 
-GLenum Framebuffer::checkStatus(const Context *context)
+Error Framebuffer::checkStatus(const Context *context, GLenum *statusOut)
 {
     // The default framebuffer is always complete except when it is surfaceless in which
     // case it is always unsupported. We return early because the default framebuffer may
@@ -991,18 +991,29 @@ GLenum Framebuffer::checkStatus(const Context *context)
         ASSERT(mCachedStatus.valid());
         ASSERT(mCachedStatus.value() == GL_FRAMEBUFFER_COMPLETE ||
                mCachedStatus.value() == GL_FRAMEBUFFER_UNDEFINED_OES);
-        return mCachedStatus.value();
+        *statusOut = mCachedStatus.value();
+        return NoError();
     }
 
     if (hasAnyDirtyBit() || !mCachedStatus.valid())
     {
-        mCachedStatus = checkStatusImpl(context);
+        mCachedStatus = checkStatusWithGLFrontEnd(context);
+
+        if (mCachedStatus.value() == GL_FRAMEBUFFER_COMPLETE)
+        {
+            ANGLE_TRY(syncState(context));
+            if (!mImpl->checkStatus(context))
+            {
+                mCachedStatus = GL_FRAMEBUFFER_UNSUPPORTED;
+            }
+        }
     }
 
-    return mCachedStatus.value();
+    *statusOut = mCachedStatus.value();
+    return NoError();
 }
 
-GLenum Framebuffer::checkStatusImpl(const Context *context)
+GLenum Framebuffer::checkStatusWithGLFrontEnd(const Context *context)
 {
     const ContextState &state = context->getContextState();
 
@@ -1199,13 +1210,6 @@ GLenum Framebuffer::checkStatusImpl(const Context *context)
         }
     }
 
-    // TODO(jmadill): Don't swallow an error here. http://anglebug.com/2372
-    ANGLE_SWALLOW_ERR(syncState(context));
-    if (!mImpl->checkStatus(context))
-    {
-        return GL_FRAMEBUFFER_UNSUPPORTED;
-    }
-
     return GL_FRAMEBUFFER_COMPLETE;
 }
 
@@ -1425,14 +1429,12 @@ Error Framebuffer::blit(const Context *context,
     return mImpl->blit(context, sourceArea, destArea, blitMask, filter);
 }
 
-int Framebuffer::getSamples(const Context *context)
+Error Framebuffer::getSamples(const Context *context, int *samplesOut)
 {
-    if (complete(context))
-    {
-        return getCachedSamples(context);
-    }
-
-    return 0;
+    bool completeness = false;
+    ANGLE_TRY(isComplete(context, &completeness));
+    *samplesOut = completeness ? getCachedSamples(context) : 0;
+    return NoError();
 }
 
 int Framebuffer::getCachedSamples(const Context *context)
@@ -1724,6 +1726,8 @@ void Framebuffer::updateAttachment(const Context *context,
     mDirtyBits.set(dirtyBit);
     mState.mResourceNeedsInit.set(dirtyBit, attachment->initState() == InitState::MayNeedInit);
     onDirtyBinding->bind(resource ? resource->getSubject() : nullptr);
+
+    invalidateCompletenessCache();
 }
 
 void Framebuffer::resetAttachment(const Context *context, GLenum binding)
@@ -1738,10 +1742,6 @@ Error Framebuffer::syncState(const Context *context)
         mDirtyBitsGuard = mDirtyBits;
         ANGLE_TRY(mImpl->syncState(context, mDirtyBits));
         mDirtyBits.reset();
-        if (mId != 0)
-        {
-            mCachedStatus.reset();
-        }
         mDirtyBitsGuard.reset();
     }
     return NoError();
@@ -1788,9 +1788,12 @@ FramebufferAttachment *Framebuffer::getAttachmentFromSubjectIndex(angle::Subject
     }
 }
 
-bool Framebuffer::complete(const Context *context)
+Error Framebuffer::isComplete(const Context *context, bool *completeOut)
 {
-    return (checkStatus(context) == GL_FRAMEBUFFER_COMPLETE);
+    GLenum status = GL_NONE;
+    ANGLE_TRY(checkStatus(context, &status));
+    *completeOut = (status == GL_FRAMEBUFFER_COMPLETE);
+    return NoError();
 }
 
 bool Framebuffer::formsRenderingFeedbackLoopWith(const State &state) const
@@ -1901,24 +1904,28 @@ void Framebuffer::setDefaultWidth(GLint defaultWidth)
 {
     mState.mDefaultWidth = defaultWidth;
     mDirtyBits.set(DIRTY_BIT_DEFAULT_WIDTH);
+    invalidateCompletenessCache();
 }
 
 void Framebuffer::setDefaultHeight(GLint defaultHeight)
 {
     mState.mDefaultHeight = defaultHeight;
     mDirtyBits.set(DIRTY_BIT_DEFAULT_HEIGHT);
+    invalidateCompletenessCache();
 }
 
 void Framebuffer::setDefaultSamples(GLint defaultSamples)
 {
     mState.mDefaultSamples = defaultSamples;
     mDirtyBits.set(DIRTY_BIT_DEFAULT_SAMPLES);
+    invalidateCompletenessCache();
 }
 
 void Framebuffer::setDefaultFixedSampleLocations(bool defaultFixedSampleLocations)
 {
     mState.mDefaultFixedSampleLocations = defaultFixedSampleLocations;
     mDirtyBits.set(DIRTY_BIT_DEFAULT_FIXED_SAMPLE_LOCATIONS);
+    invalidateCompletenessCache();
 }
 
 GLsizei Framebuffer::getNumViews() const

src/libANGLE/Framebuffer.h

@@ -214,7 +214,7 @@ class Framebuffer final : public angle::ObserverInterface, public LabeledObject
     bool usingExtendedDrawBuffers() const;
 
     // This method calls checkStatus.
-    int getSamples(const Context *context);
+    Error getSamples(const Context *context, int *samplesOut);
 
     Error getSamplePosition(size_t index, GLfloat *xy) const;
 
@@ -229,13 +229,13 @@ class Framebuffer final : public angle::ObserverInterface, public LabeledObject
 
     void invalidateCompletenessCache();
 
-    GLenum checkStatus(const Context *context);
+    Error checkStatus(const Context *context, GLenum *statusOut);
 
     // For when we don't want to check completeness in getSamples().
     int getCachedSamples(const Context *context);
 
     // Helper for checkStatus == GL_FRAMEBUFFER_COMPLETE.
-    bool complete(const Context *context);
+    Error isComplete(const Context *context, bool *completeOut);
 
     bool hasValidDepthStencil() const;
 
@@ -328,7 +328,7 @@ class Framebuffer final : public angle::ObserverInterface, public LabeledObject
                                   GLenum matchType,
                                   GLuint matchId,
                                   size_t dirtyBit);
-    GLenum checkStatusImpl(const Context *context);
+    GLenum checkStatusWithGLFrontEnd(const Context *context);
     void setAttachment(const Context *context,
                        GLenum type,
                        GLenum binding,

src/libANGLE/State.cpp

@@ -1831,7 +1831,7 @@ void State::getFloatv(GLenum pname, GLfloat *params)
     }
 }
 
-void State::getIntegerv(const Context *context, GLenum pname, GLint *params)
+Error State::getIntegerv(const Context *context, GLenum pname, GLint *params)
 {
     if (pname >= GL_DRAW_BUFFER0_EXT && pname <= GL_DRAW_BUFFER15_EXT)
     {
@@ -1839,7 +1839,7 @@ void State::getIntegerv(const Context *context, GLenum pname, GLint *params)
         ASSERT(colorAttachment < mMaxDrawBuffers);
         Framebuffer *framebuffer = mDrawFramebuffer;
         *params = framebuffer->getDrawBufferState(colorAttachment);
-        return;
+        return NoError();
     }
 
     // Please note: DEPTH_CLEAR_VALUE is not included in our internal getIntegerv implementation
@@ -1934,12 +1934,16 @@ void State::getIntegerv(const Context *context, GLenum pname, GLint *params)
       case GL_SAMPLES:
         {
             Framebuffer *framebuffer = mDrawFramebuffer;
-            if (framebuffer->checkStatus(context) == GL_FRAMEBUFFER_COMPLETE)
+            bool complete            = false;
+            ANGLE_TRY(framebuffer->isComplete(context, &complete));
+            if (complete)
             {
+                GLint samples = 0;
+                ANGLE_TRY(framebuffer->getSamples(context, &samples));
                 switch (pname)
                 {
                     case GL_SAMPLE_BUFFERS:
-                        if (framebuffer->getSamples(context) != 0)
+                        if (samples != 0)
                         {
                             *params = 1;
                         }
@@ -1949,7 +1953,7 @@ void State::getIntegerv(const Context *context, GLenum pname, GLint *params)
                         }
                         break;
                     case GL_SAMPLES:
-                        *params = framebuffer->getSamples(context);
+                        *params = samples;
                         break;
                 }
             }
@@ -2123,6 +2127,8 @@ void State::getIntegerv(const Context *context, GLenum pname, GLint *params)
         UNREACHABLE();
         break;
     }
+
+    return NoError();
 }
 
 void State::getPointerv(GLenum pname, void **params) const

src/libANGLE/State.h

@@ -333,7 +333,7 @@ class State : public angle::ObserverInterface, angle::NonCopyable
     // State query functions
     void getBooleanv(GLenum pname, GLboolean *params);
     void getFloatv(GLenum pname, GLfloat *params);
-    void getIntegerv(const Context *context, GLenum pname, GLint *params);
+    Error getIntegerv(const Context *context, GLenum pname, GLint *params);
     void getPointerv(GLenum pname, void **params) const;
     void getIntegeri_v(GLenum target, GLuint index, GLint *data);
     void getInteger64i_v(GLenum target, GLuint index, GLint64 *data);

src/libANGLE/validationES.cpp

@@ -1245,15 +1245,13 @@ bool ValidateBlitFramebufferParameters(Context *context,
         return false;
     }
 
-    if (readFramebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+    if (!ValidateFramebufferComplete(context, readFramebuffer, true))
     {
-        context->handleError(InvalidFramebufferOperation());
         return false;
     }
 
-    if (drawFramebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+    if (!ValidateFramebufferComplete(context, drawFramebuffer, true))
     {
-        context->handleError(InvalidFramebufferOperation());
         return false;
     }
 
@@ -1263,9 +1261,8 @@ bool ValidateBlitFramebufferParameters(Context *context,
         return false;
     }
 
-    if (drawFramebuffer->getSamples(context) != 0)
+    if (!ValidateFramebufferNotMultisampled(context, drawFramebuffer))
     {
-        context->handleError(InvalidOperation());
         return false;
     }
 
@@ -2183,23 +2180,21 @@ bool ValidateStateQuery(Context *context, GLenum pname, GLenum *nativeType, unsi
         case GL_IMPLEMENTATION_COLOR_READ_TYPE:
         case GL_IMPLEMENTATION_COLOR_READ_FORMAT:
         {
-            if (context->getGLState().getReadFramebuffer()->checkStatus(context) !=
-                GL_FRAMEBUFFER_COMPLETE)
+            Framebuffer *readFramebuffer = context->getGLState().getReadFramebuffer();
+            ASSERT(readFramebuffer);
+
+            if (!ValidateFramebufferComplete(context, readFramebuffer, false))
             {
-                context->handleError(InvalidOperation());
                 return false;
             }
 
-            const Framebuffer *framebuffer = context->getGLState().getReadFramebuffer();
-            ASSERT(framebuffer);
-
-            if (framebuffer->getReadBufferState() == GL_NONE)
+            if (readFramebuffer->getReadBufferState() == GL_NONE)
             {
                 ANGLE_VALIDATION_ERR(context, InvalidOperation(), ReadBufferNone);
                 return false;
             }
 
-            const FramebufferAttachment *attachment = framebuffer->getReadColorbuffer();
+            const FramebufferAttachment *attachment = readFramebuffer->getReadColorbuffer();
             if (!attachment)
             {
                 context->handleError(InvalidOperation());
@@ -2293,17 +2288,15 @@ bool ValidateCopyTexImageParametersBase(Context *context,
         return false;
     }
 
-    const auto &state    = context->getGLState();
+    const gl::State &state       = context->getGLState();
     Framebuffer *readFramebuffer = state.getReadFramebuffer();
-    if (readFramebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+    if (!ValidateFramebufferComplete(context, readFramebuffer, true))
     {
-        context->handleError(InvalidFramebufferOperation());
         return false;
     }
 
-    if (readFramebuffer->id() != 0 && readFramebuffer->getSamples(context) != 0)
+    if (readFramebuffer->id() != 0 && !ValidateFramebufferNotMultisampled(context, readFramebuffer))
     {
-        context->handleError(InvalidOperation());
         return false;
     }
 
@@ -2510,9 +2503,8 @@ bool ValidateDrawBase(Context *context, GLenum mode, GLsizei count)
         }
     }
 
-    if (framebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+    if (!ValidateFramebufferComplete(context, framebuffer, true))
     {
-        context->handleError(InvalidFramebufferOperation());
         return false;
     }
 
@@ -5169,15 +5161,13 @@ bool ValidateReadPixelsBase(Context *context,
 
     Framebuffer *readFramebuffer = context->getGLState().getReadFramebuffer();
 
-    if (readFramebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+    if (!ValidateFramebufferComplete(context, readFramebuffer, true))
     {
-        context->handleError(InvalidFramebufferOperation());
         return false;
     }
 
-    if (readFramebuffer->id() != 0 && readFramebuffer->getSamples(context) != 0)
+    if (readFramebuffer->id() != 0 && !ValidateFramebufferNotMultisampled(context, readFramebuffer))
     {
-        context->handleError(InvalidOperation());
         return false;
     }
 
@@ -5924,4 +5914,38 @@ bool ValidateGetInternalFormativBase(Context *context,
     return true;
 }
 
+// We should check with Khronos if returning INVALID_FRAMEBUFFER_OPERATION is OK when querying
+// implementation format info for incomplete framebuffers. It seems like these queries are
+// incongruent with the other errors.
+bool ValidateFramebufferComplete(Context *context, Framebuffer *framebuffer, bool isFramebufferOp)
+{
+    bool complete = false;
+    ANGLE_VALIDATION_TRY(framebuffer->isComplete(context, &complete));
+    if (!complete)
+    {
+        if (isFramebufferOp)
+        {
+            context->handleError(InvalidFramebufferOperation());
+        }
+        else
+        {
+            context->handleError(InvalidOperation());
+        }
+        return false;
+    }
+    return true;
+}
+
+bool ValidateFramebufferNotMultisampled(Context *context, Framebuffer *framebuffer)
+{
+    GLint samples = 0;
+    ANGLE_VALIDATION_TRY(framebuffer->getSamples(context, &samples));
+    if (samples != 0)
+    {
+        context->handleError(InvalidOperation());
+        return false;
+    }
+    return true;
+}
+
 }  // namespace gl

src/libANGLE/validationES.h

@@ -26,6 +26,7 @@ namespace gl
 {
 class Context;
 struct Format;
+class Framebuffer;
 struct LinkedUniform;
 class Program;
 class Shader;
@@ -592,6 +593,15 @@ bool ValidateGetInternalFormativBase(Context *context,
                                      GLsizei bufSize,
                                      GLsizei *numParams);
 
+bool ValidateFramebufferComplete(Context *context, Framebuffer *framebuffer, bool isFramebufferOp);
+bool ValidateFramebufferNotMultisampled(Context *context, Framebuffer *framebuffer);
+
+// Utility macro for handling implementation methods inside Validation.
+#define ANGLE_HANDLE_VALIDATION_ERR(X) \
+    context->handleError(X);           \
+    return false;
+#define ANGLE_VALIDATION_TRY(EXPR) ANGLE_TRY_TEMPLATE(EXPR, ANGLE_HANDLE_VALIDATION_ERR);
+
 }  // namespace gl
 
 #endif  // LIBANGLE_VALIDATION_ES_H_

src/libANGLE/validationES2.cpp

@@ -2493,7 +2493,9 @@ bool ValidateBlitFramebufferANGLE(Context *context,
                 }
             }
 
-            if (readFramebuffer->getSamples(context) != 0 &&
+            GLint samples = 0;
+            ANGLE_VALIDATION_TRY(readFramebuffer->getSamples(context, &samples));
+            if (samples != 0 &&
                 IsPartialBlit(context, readColorAttachment, drawColorAttachment, srcX0, srcY0,
                               srcX1, srcY1, dstX0, dstY0, dstX1, dstY1))
             {
@@ -2542,9 +2544,9 @@ bool ValidateBlitFramebufferANGLE(Context *context,
 bool ValidateClear(Context *context, GLbitfield mask)
 {
     Framebuffer *fbo = context->getGLState().getDrawFramebuffer();
-    if (fbo->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+
+    if (!ValidateFramebufferComplete(context, fbo, true))
     {
-        context->handleError(InvalidFramebufferOperation());
         return false;
     }
 

src/libANGLE/validationES3.cpp

@@ -808,15 +808,13 @@ bool ValidateES3CopyTexImageParametersBase(Context *context,
     gl::Framebuffer *framebuffer = state.getReadFramebuffer();
     GLuint readFramebufferID     = framebuffer->id();
 
-    if (framebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+    if (!ValidateFramebufferComplete(context, framebuffer, true))
     {
-        context->handleError(InvalidFramebufferOperation());
         return false;
     }
 
-    if (readFramebufferID != 0 && framebuffer->getSamples(context) != 0)
+    if (readFramebufferID != 0 && !ValidateFramebufferNotMultisampled(context, framebuffer))
     {
-        context->handleError(InvalidOperation());
         return false;
     }
 
@@ -1246,9 +1244,8 @@ bool ValidateClearBuffer(Context *context)
         return false;
     }
 
-    if (context->getGLState().getDrawFramebuffer()->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+    if (!ValidateFramebufferComplete(context, context->getGLState().getDrawFramebuffer(), true))
     {
-        context->handleError(InvalidFramebufferOperation());
         return false;
     }
 

src/libANGLE/validationES31.cpp

@@ -1048,8 +1048,10 @@ bool ValidateGetMultisamplefv(Context *context, GLenum pname, GLuint index, GLfl
     }
 
     Framebuffer *framebuffer = context->getGLState().getDrawFramebuffer();
+    GLint samples            = 0;
+    ANGLE_VALIDATION_TRY(framebuffer->getSamples(context, &samples));
 
-    if (index >= static_cast<GLuint>(framebuffer->getSamples(context)))
+    if (index >= static_cast<GLuint>(samples))
     {
         context->handleError(InvalidValue() << "Index must be less than the value of SAMPLES.");
         return false;