commit e98b1b5d41bf7ca2ecc43c75354c47e6756374fa
author Jamie Madill <jmadill@chromium.org> Thu Mar 08 09:47:23 2018 -0500
committer Commit Bot <commit-bot@chromium.org> Mon Mar 12 16:36:52 2018 +0000
tree 6297b38f8b5ad0755af95d021ed2f7f8060956d7
parent 7c9492eb2494b4831a0f8048fad5ef4519c87afe

Framebuffer: Handle errors in checkStatus.

This pipes a lot more errors around in the Validation, where
they now will be caught.

Bug: angleproject:2372
Change-Id: Ibb4e47ddc932995a02dd92e10578b7a4097182a9
Reviewed-on: https://chromium-review.googlesource.com/954406
Reviewed-by: Luc Ferron <lucferron@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>

src/libANGLE/validationES.cpp

diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index 47cc3c7..86c898f 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -1245,15 +1245,13 @@
         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 @@
         return false;
     }
 
-    if (drawFramebuffer->getSamples(context) != 0)
+    if (!ValidateFramebufferNotMultisampled(context, drawFramebuffer))
     {
-        context->handleError(InvalidOperation());
         return false;
     }
 
@@ -2183,23 +2180,21 @@
         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 @@
         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 @@
         }
     }
 
-    if (framebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+    if (!ValidateFramebufferComplete(context, framebuffer, true))
     {
-        context->handleError(InvalidFramebufferOperation());
         return false;
     }
 
@@ -5169,15 +5161,13 @@
 
     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 @@
     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