Update Queries to return Error objects instead of calling gl::error.

BUG=angle:520

Change-Id: If8f2bb1c4de7b9cc30861a06aab1d89c97305b26
Reviewed-on: https://chromium-review.googlesource.com/216699
Reviewed-by: Shannon Woods <shannonwoods@chromium.org>
Tested-by: Geoff Lang <geofflang@chromium.org>

11 files changed, 229 insertions, 155 deletions

diff --git a/src/libGLESv2/Context.cpp b/src/libGLESv2/Context.cpp
index aef626a6..eb71e71c 100644
--- a/src/libGLESv2/Context.cpp
+++ b/src/libGLESv2/Context.cpp
@@ -681,26 +681,35 @@ void Context::bindTransformFeedback(GLuint transformFeedback)
     mState.setTransformFeedbackBinding(getTransformFeedback(transformFeedback));
 }
 
-void Context::beginQuery(GLenum target, GLuint query)
+Error Context::beginQuery(GLenum target, GLuint query)
 {
     Query *queryObject = getQuery(query, true, target);
     ASSERT(queryObject);
 
-    // set query as active for specified target
+    // begin query
+    Error error = queryObject->begin();
+    if (error.isError())
+    {
+        return error;
+    }
+
+    // set query as active for specified target only if begin succeeded
     mState.setActiveQuery(target, queryObject);
 
-    // begin query
-    queryObject->begin();
+    return Error(GL_NO_ERROR);
 }
 
-void Context::endQuery(GLenum target)
+Error Context::endQuery(GLenum target)
 {
     Query *queryObject = mState.getActiveQuery(target);
     ASSERT(queryObject);
 
-    queryObject->end();
+    gl::Error error = queryObject->end();
 
+    // Always unbind the query, even if there was an error. This may delete the query object.
     mState.setActiveQuery(target, NULL);
+
+    return error;
 }
 
 void Context::setFramebufferZero(Framebuffer *buffer)
diff --git a/src/libGLESv2/Context.h b/src/libGLESv2/Context.h
index b2d53211..7c5494b4 100644
--- a/src/libGLESv2/Context.h
+++ b/src/libGLESv2/Context.h
@@ -137,8 +137,8 @@ class Context
     void setProgramBinary(GLuint program, GLenum binaryFormat, const void *binary, GLint length);
     void bindTransformFeedback(GLuint transformFeedback);
 
-    void beginQuery(GLenum target, GLuint query);
-    void endQuery(GLenum target);
+    Error beginQuery(GLenum target, GLuint query);
+    Error endQuery(GLenum target);
 
     void setFramebufferZero(Framebuffer *framebuffer);
 
diff --git a/src/libGLESv2/Query.cpp b/src/libGLESv2/Query.cpp
index a99461e7..4ee35255 100644
--- a/src/libGLESv2/Query.cpp
+++ b/src/libGLESv2/Query.cpp
@@ -19,31 +19,27 @@ Query::Query(rx::QueryImpl *impl, GLuint id)
 
 Query::~Query()
 {
-    delete mQuery;
+    SafeDelete(mQuery);
 }
 
-void Query::begin()
+Error Query::begin()
 {
-    // TODO: Rather than keeping track of whether the query was successfully
-    // created via a boolean in the GL-level Query object, we should probably
-    // use the error system to track these failed creations at the context level,
-    // and reset the active query ID for the target to 0 upon failure.
-    mStarted = mQuery->begin();
+    return mQuery->begin();
 }
 
-void Query::end()
+Error Query::end()
 {
-    mQuery->end();
+    return mQuery->end();
 }
 
-GLuint Query::getResult()
+Error Query::getResult(GLuint *params)
 {
-    return mQuery->getResult();
+    return mQuery->getResult(params);
 }
 
-GLboolean Query::isResultAvailable()
+Error Query::isResultAvailable(GLuint *available)
 {
-    return mQuery->isResultAvailable();
+    return mQuery->isResultAvailable(available);
 }
 
 GLenum Query::getType() const
@@ -51,9 +47,4 @@ GLenum Query::getType() const
     return mQuery->getType();
 }
 
-bool Query::isStarted() const
-{
-    return mStarted;
-}
-
 }
diff --git a/src/libGLESv2/Query.h b/src/libGLESv2/Query.h
index b98a78e5..a7ec404f 100644
--- a/src/libGLESv2/Query.h
+++ b/src/libGLESv2/Query.h
@@ -9,6 +9,7 @@
 #ifndef LIBGLESV2_QUERY_H_
 #define LIBGLESV2_QUERY_H_
 
+#include "libGLESv2/Error.h"
 #include "common/angleutils.h"
 #include "common/RefCountObject.h"
 
@@ -28,20 +29,17 @@ class Query : public RefCountObject
     Query(rx::QueryImpl *impl, GLuint id);
     virtual ~Query();
 
-    void begin();
-    void end();
+    Error begin();
+    Error end();
 
-    GLuint getResult();
-    GLboolean isResultAvailable();
+    Error getResult(GLuint *params);
+    Error isResultAvailable(GLuint *available);
 
     GLenum getType() const;
-    bool isStarted() const;
 
   private:
     DISALLOW_COPY_AND_ASSIGN(Query);
 
-    bool mStarted;
-
     rx::QueryImpl *mQuery;
 };
 
diff --git a/src/libGLESv2/libGLESv2.cpp b/src/libGLESv2/libGLESv2.cpp
index 28e3d28f..d41d2c23 100644
--- a/src/libGLESv2/libGLESv2.cpp
+++ b/src/libGLESv2/libGLESv2.cpp
@@ -110,7 +110,12 @@ void __stdcall glBeginQueryEXT(GLenum target, GLuint id)
             return;
         }
 
-        context->beginQuery(target, id);
+        gl::Error error = context->beginQuery(target, id);
+        if (error.isError())
+        {
+            context->recordError(error);
+            return;
+        }
     }
 }
 
@@ -1451,7 +1456,12 @@ void __stdcall glEndQueryEXT(GLenum target)
             return;
         }
 
-        context->endQuery(target);
+        gl::Error error = context->endQuery(target);
+        if (error.isError())
+        {
+            context->recordError(error);
+            return;
+        }
     }
 }
 
@@ -2604,11 +2614,27 @@ void __stdcall glGetQueryObjectuivEXT(GLuint id, GLenum pname, GLuint *params)
         switch(pname)
         {
           case GL_QUERY_RESULT_EXT:
-            params[0] = queryObject->getResult();
+            {
+                gl::Error error = queryObject->getResult(params);
+                if (error.isError())
+                {
+                    context->recordError(error);
+                    return;
+                }
+            }
             break;
+
           case GL_QUERY_RESULT_AVAILABLE_EXT:
-            params[0] = queryObject->isResultAvailable();
+            {
+                gl::Error error = queryObject->isResultAvailable(params);
+                if (error.isError())
+                {
+                    context->recordError(error);
+                    return;
+                }
+            }
             break;
+
           default:
             context->recordError(gl::Error(GL_INVALID_ENUM));
             return;
@@ -5301,7 +5327,13 @@ void __stdcall glBeginQuery(GLenum target, GLuint id)
         {
             return;
         }
-        context->beginQuery(target, id);
+
+        gl::Error error = context->beginQuery(target, id);
+        if (error.isError())
+        {
+            context->recordError(error);
+            return;
+        }
     }
 }
 
@@ -5323,7 +5355,12 @@ void __stdcall glEndQuery(GLenum target)
             return;
         }
 
-        context->endQuery(target);
+        gl::Error error = context->endQuery(target);
+        if (error.isError())
+        {
+            context->recordError(error);
+            return;
+        }
     }
 }
 
@@ -5388,12 +5425,26 @@ void __stdcall glGetQueryObjectuiv(GLuint id, GLenum pname, GLuint* params)
 
         switch(pname)
         {
-          case GL_QUERY_RESULT:
-            params[0] = queryObject->getResult();
+          case GL_QUERY_RESULT_EXT:
+            {
+                gl::Error error = queryObject->getResult(params);
+                if (error.isError())
+                {
+                    context->recordError(error);
+                    return;
+                }
+            }
             break;
 
-          case GL_QUERY_RESULT_AVAILABLE:
-            params[0] = queryObject->isResultAvailable();
+          case GL_QUERY_RESULT_AVAILABLE_EXT:
+            {
+                gl::Error error = queryObject->isResultAvailable(params);
+                if (error.isError())
+                {
+                    context->recordError(error);
+                    return;
+                }
+            }
             break;
 
           default:
diff --git a/src/libGLESv2/renderer/QueryImpl.h b/src/libGLESv2/renderer/QueryImpl.h
index 1ecfbdb8..6b45810a 100644
--- a/src/libGLESv2/renderer/QueryImpl.h
+++ b/src/libGLESv2/renderer/QueryImpl.h
@@ -9,6 +9,8 @@
 #ifndef LIBGLESV2_RENDERER_QUERYIMPL_H_
 #define LIBGLESV2_RENDERER_QUERYIMPL_H_
 
+#include "libGLESv2/Error.h"
+
 #include "common/angleutils.h"
 
 #include <GLES2/gl2.h>
@@ -22,10 +24,10 @@ class QueryImpl
     explicit QueryImpl(GLenum type) { mType = type; }
     virtual ~QueryImpl() { }
 
-    virtual bool begin() = 0;
-    virtual void end() = 0;
-    virtual GLuint getResult() = 0;
-    virtual GLboolean isResultAvailable() = 0;
+    virtual gl::Error begin() = 0;
+    virtual gl::Error end() = 0;
+    virtual gl::Error getResult(GLuint *params) = 0;
+    virtual gl::Error isResultAvailable(GLuint *available) = 0;
 
     GLenum getType() const { return mType;  }
 
diff --git a/src/libGLESv2/renderer/d3d/d3d11/Query11.cpp b/src/libGLESv2/renderer/d3d/d3d11/Query11.cpp
index bc2f9419..7109be3e 100644
--- a/src/libGLESv2/renderer/d3d/d3d11/Query11.cpp
+++ b/src/libGLESv2/renderer/d3d/d3d11/Query11.cpp
@@ -16,24 +16,13 @@
 namespace rx
 {
 
-static bool checkOcclusionQuery(ID3D11DeviceContext *context, ID3D11Query *query, UINT64 *numPixels)
+Query11::Query11(rx::Renderer11 *renderer, GLenum type)
+    : QueryImpl(type),
+      mResult(0),
+      mQueryFinished(false),
+      mRenderer(renderer),
+      mQuery(NULL)
 {
-    HRESULT result = context->GetData(query, numPixels, sizeof(UINT64), 0);
-    return (result == S_OK);
-}
-
-static bool checkStreamOutPrimitivesWritten(ID3D11DeviceContext *context, ID3D11Query *query, UINT64 *numPrimitives)
-{
-    D3D11_QUERY_DATA_SO_STATISTICS soStats = { 0 };
-    HRESULT result = context->GetData(query, &soStats, sizeof(D3D11_QUERY_DATA_SO_STATISTICS), 0);
-    *numPrimitives = soStats.NumPrimitivesWritten;
-    return (result == S_OK);
-}
-
-Query11::Query11(rx::Renderer11 *renderer, GLenum type) : QueryImpl(type), mStatus(GL_FALSE), mResult(0)
-{
-    mRenderer = renderer;
-    mQuery = NULL;
 }
 
 Query11::~Query11()
@@ -41,7 +30,7 @@ Query11::~Query11()
     SafeRelease(mQuery);
 }
 
-bool Query11::begin()
+gl::Error Query11::begin()
 {
     if (mQuery == NULL)
     {
@@ -49,70 +38,85 @@ bool Query11::begin()
         queryDesc.Query = gl_d3d11::ConvertQueryType(getType());
         queryDesc.MiscFlags = 0;
 
-        if (FAILED(mRenderer->getDevice()->CreateQuery(&queryDesc, &mQuery)))
+        HRESULT result = mRenderer->getDevice()->CreateQuery(&queryDesc, &mQuery);
+        if (FAILED(result))
         {
-            return gl::error(GL_OUT_OF_MEMORY, false);
+            return gl::Error(GL_OUT_OF_MEMORY, "Internal query creation failed, result: 0x%X.", result);
         }
     }
 
     mRenderer->getDeviceContext()->Begin(mQuery);
-    return true;
+    return gl::Error(GL_NO_ERROR);
 }
 
-void Query11::end()
+gl::Error Query11::end()
 {
     ASSERT(mQuery);
     mRenderer->getDeviceContext()->End(mQuery);
 
-    mStatus = GL_FALSE;
+    mQueryFinished = false;
     mResult = GL_FALSE;
+
+    return gl::Error(GL_NO_ERROR);
 }
 
-GLuint Query11::getResult()
+gl::Error Query11::getResult(GLuint *params)
 {
-    if (mQuery != NULL)
+    while (!mQueryFinished)
     {
-        while (!testQuery())
+        gl::Error error = testQuery();
+        if (error.isError())
+        {
+            return error;
+        }
+
+        if (!mQueryFinished)
         {
             Sleep(0);
-            // explicitly check for device loss, some drivers seem to return S_FALSE
-            // if the device is lost
-            if (mRenderer->testDeviceLost(true))
-            {
-                return gl::error(GL_OUT_OF_MEMORY, 0);
-            }
         }
     }
 
-    return mResult;
+    ASSERT(mQueryFinished);
+    *params = mResult;
+
+    return gl::Error(GL_NO_ERROR);
 }
 
-GLboolean Query11::isResultAvailable()
+gl::Error Query11::isResultAvailable(GLuint *available)
 {
-    if (mQuery != NULL)
+    gl::Error error = testQuery();
+    if (error.isError())
     {
-        testQuery();
+        return error;
     }
 
-    return mStatus;
+    *available = (mQueryFinished ? GL_TRUE : GL_FALSE);
+
+    return gl::Error(GL_NO_ERROR);
 }
 
-GLboolean Query11::testQuery()
+gl::Error Query11::testQuery()
 {
-    if (mQuery != NULL && mStatus != GL_TRUE)
+    if (!mQueryFinished)
     {
-        ID3D11DeviceContext *context = mRenderer->getDeviceContext();
+        ASSERT(mQuery);
 
-        bool queryFinished = false;
+        ID3D11DeviceContext *context = mRenderer->getDeviceContext();
         switch (getType())
         {
           case GL_ANY_SAMPLES_PASSED_EXT:
           case GL_ANY_SAMPLES_PASSED_CONSERVATIVE_EXT:
             {
                 UINT64 numPixels = 0;
-                queryFinished = checkOcclusionQuery(context, mQuery, &numPixels);
-                if (queryFinished)
+                HRESULT result = context->GetData(mQuery, &numPixels, sizeof(numPixels), 0);
+                if (FAILED(result))
+                {
+                    return gl::Error(GL_OUT_OF_MEMORY, "Failed to get the data of an internal query, result: 0x%X.", result);
+                }
+
+                if (result == S_OK)
                 {
+                    mQueryFinished = true;
                     mResult = (numPixels > 0) ? GL_TRUE : GL_FALSE;
                 }
             }
@@ -120,11 +124,17 @@ GLboolean Query11::testQuery()
 
           case GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN:
             {
-                UINT64 numPrimitives = 0;
-                queryFinished = checkStreamOutPrimitivesWritten(context, mQuery, &numPrimitives);
-                if (queryFinished)
+                D3D11_QUERY_DATA_SO_STATISTICS soStats = { 0 };
+                HRESULT result = context->GetData(mQuery, &soStats, sizeof(soStats), 0);
+                if (FAILED(result))
                 {
-                    mResult = static_cast<GLuint>(numPrimitives);
+                    return gl::Error(GL_OUT_OF_MEMORY, "Failed to get the data of an internal query, result: 0x%X.", result);
+                }
+
+                if (result == S_OK)
+                {
+                    mQueryFinished = true;
+                    mResult = static_cast<GLuint>(soStats.NumPrimitivesWritten);
                 }
             }
             break;
@@ -134,19 +144,13 @@ GLboolean Query11::testQuery()
             break;
         }
 
-        if (queryFinished)
+        if (!mQueryFinished && mRenderer->testDeviceLost(true))
         {
-            mStatus = GL_TRUE;
+            return gl::Error(GL_OUT_OF_MEMORY, "Failed to test get query result, device is lost.");
         }
-        else if (mRenderer->testDeviceLost(true))
-        {
-            return gl::error(GL_OUT_OF_MEMORY, GL_TRUE);
-        }
-
-        return mStatus;
     }
 
-    return GL_TRUE; // prevent blocking when query is null
+    return gl::Error(GL_NO_ERROR);
 }
 
 }
diff --git a/src/libGLESv2/renderer/d3d/d3d11/Query11.h b/src/libGLESv2/renderer/d3d/d3d11/Query11.h
index 40cfbb6d..822f2542 100644
--- a/src/libGLESv2/renderer/d3d/d3d11/Query11.h
+++ b/src/libGLESv2/renderer/d3d/d3d11/Query11.h
@@ -21,18 +21,19 @@ class Query11 : public QueryImpl
     Query11(rx::Renderer11 *renderer, GLenum type);
     virtual ~Query11();
 
-    virtual bool begin();
-    virtual void end();
-    virtual GLuint getResult();
-    virtual GLboolean isResultAvailable();
+    virtual gl::Error begin();
+    virtual gl::Error end();
+    virtual gl::Error getResult(GLuint *params);
+    virtual gl::Error isResultAvailable(GLuint *available);
 
   private:
     DISALLOW_COPY_AND_ASSIGN(Query11);
 
-    GLboolean testQuery();
+    gl::Error testQuery();
 
     GLuint mResult;
-    GLboolean mStatus;
+
+    bool mQueryFinished;
 
     rx::Renderer11 *mRenderer;
     ID3D11Query *mQuery;
diff --git a/src/libGLESv2/renderer/d3d/d3d9/Query9.cpp b/src/libGLESv2/renderer/d3d/d3d9/Query9.cpp
index c8752bb8..815fc01a 100644
--- a/src/libGLESv2/renderer/d3d/d3d9/Query9.cpp
+++ b/src/libGLESv2/renderer/d3d/d3d9/Query9.cpp
@@ -15,10 +15,13 @@
 
 namespace rx
 {
-Query9::Query9(rx::Renderer9 *renderer, GLenum type) : QueryImpl(type), mStatus(GL_FALSE), mResult(0)
+Query9::Query9(rx::Renderer9 *renderer, GLenum type)
+    : QueryImpl(type),
+      mResult(GL_FALSE),
+      mQueryFinished(false),
+      mRenderer(renderer),
+      mQuery(NULL)
 {
-    mRenderer = renderer;
-    mQuery = NULL;
 }
 
 Query9::~Query9()
@@ -26,74 +29,91 @@ Query9::~Query9()
     SafeRelease(mQuery);
 }
 
-bool Query9::begin()
+gl::Error Query9::begin()
 {
     if (mQuery == NULL)
     {
-        if (FAILED(mRenderer->getDevice()->CreateQuery(D3DQUERYTYPE_OCCLUSION, &mQuery)))
+        HRESULT result = mRenderer->getDevice()->CreateQuery(D3DQUERYTYPE_OCCLUSION, &mQuery);
+        if (FAILED(result))
         {
-            return gl::error(GL_OUT_OF_MEMORY, false);
+            return gl::Error(GL_OUT_OF_MEMORY, "Internal query creation failed, result: 0x%X.", result);
         }
     }
 
     HRESULT result = mQuery->Issue(D3DISSUE_BEGIN);
-    UNUSED_ASSERTION_VARIABLE(result);
     ASSERT(SUCCEEDED(result));
-    return true;
+    if (FAILED(result))
+    {
+        return gl::Error(GL_OUT_OF_MEMORY, "Failed to begin internal query, result: 0x%X.", result);
+    }
+
+    return gl::Error(GL_NO_ERROR);
 }
 
-void Query9::end()
+gl::Error Query9::end()
 {
     ASSERT(mQuery);
 
     HRESULT result = mQuery->Issue(D3DISSUE_END);
-    UNUSED_ASSERTION_VARIABLE(result);
     ASSERT(SUCCEEDED(result));
+    if (FAILED(result))
+    {
+        return gl::Error(GL_OUT_OF_MEMORY, "Failed to end internal query, result: 0x%X.", result);
+    }
 
-    mStatus = GL_FALSE;
+    mQueryFinished = false;
     mResult = GL_FALSE;
+
+    return gl::Error(GL_NO_ERROR);
 }
 
-GLuint Query9::getResult()
+gl::Error Query9::getResult(GLuint *params)
 {
-    if (mQuery != NULL)
+    while (!mQueryFinished)
     {
-        while (!testQuery())
+        gl::Error error = testQuery();
+        if (error.isError())
+        {
+            return error;
+        }
+
+        if (!mQueryFinished)
         {
             Sleep(0);
-            // explicitly check for device loss
-            // some drivers seem to return S_FALSE even if the device is lost
-            // instead of D3DERR_DEVICELOST like they should
-            if (mRenderer->testDeviceLost(true))
-            {
-                return gl::error(GL_OUT_OF_MEMORY, 0);
-            }
         }
     }
 
-    return mResult;
+    ASSERT(mQueryFinished);
+    *params = mResult;
+
+    return gl::Error(GL_NO_ERROR);
 }
 
-GLboolean Query9::isResultAvailable()
+gl::Error Query9::isResultAvailable(GLuint *available)
 {
-    if (mQuery != NULL)
+    gl::Error error = testQuery();
+    if (error.isError())
     {
-        testQuery();
+        return error;
     }
 
-    return mStatus;
+    *available = (mQueryFinished ? GL_TRUE : GL_FALSE);
+
+    return gl::Error(GL_NO_ERROR);
 }
 
-GLboolean Query9::testQuery()
+gl::Error Query9::testQuery()
 {
-    if (mQuery != NULL && mStatus != GL_TRUE)
+    if (!mQueryFinished)
     {
+        ASSERT(mQuery);
+
         DWORD numPixels = 0;
 
         HRESULT hres = mQuery->GetData(&numPixels, sizeof(DWORD), D3DGETDATA_FLUSH);
         if (hres == S_OK)
         {
-            mStatus =  GL_TRUE;
+            mQueryFinished = true;
 
             switch (getType())
             {
@@ -101,20 +121,24 @@ GLboolean Query9::testQuery()
               case GL_ANY_SAMPLES_PASSED_CONSERVATIVE_EXT:
                 mResult = (numPixels > 0) ? GL_TRUE : GL_FALSE;
                 break;
+
               default:
-                ASSERT(false);
+                UNREACHABLE();
+                break;
             }
         }
         else if (d3d9::isDeviceLostError(hres))
         {
             mRenderer->notifyDeviceLost();
-            return gl::error(GL_OUT_OF_MEMORY, GL_TRUE);
+            return gl::Error(GL_OUT_OF_MEMORY, "Failed to test get query result, device is lost.");
+        }
+        else if (mRenderer->testDeviceLost(true))
+        {
+            return gl::Error(GL_OUT_OF_MEMORY, "Failed to test get query result, device is lost.");
         }
-
-        return mStatus;
     }
 
-    return GL_TRUE; // prevent blocking when query is null
+    return gl::Error(GL_NO_ERROR);
 }
 
 }
diff --git a/src/libGLESv2/renderer/d3d/d3d9/Query9.h b/src/libGLESv2/renderer/d3d/d3d9/Query9.h
index fc7f0b31..513e0ba6 100644
--- a/src/libGLESv2/renderer/d3d/d3d9/Query9.h
+++ b/src/libGLESv2/renderer/d3d/d3d9/Query9.h
@@ -21,18 +21,18 @@ class Query9 : public QueryImpl
     Query9(rx::Renderer9 *renderer, GLenum type);
     virtual ~Query9();
 
-    virtual bool begin();
-    virtual void end();
-    virtual GLuint getResult();
-    virtual GLboolean isResultAvailable();
+    virtual gl::Error begin();
+    virtual gl::Error end();
+    virtual gl::Error getResult(GLuint *params);
+    virtual gl::Error isResultAvailable(GLuint *available);
 
   private:
     DISALLOW_COPY_AND_ASSIGN(Query9);
 
-    GLboolean testQuery();
+    gl::Error testQuery();
 
     GLuint mResult;
-    GLboolean mStatus;
+    bool mQueryFinished;
 
     rx::Renderer9 *mRenderer;
     IDirect3DQuery9 *mQuery;
diff --git a/src/libGLESv2/validationES.cpp b/src/libGLESv2/validationES.cpp
index 8016dfbf..6a843dc8 100644
--- a/src/libGLESv2/validationES.cpp
+++ b/src/libGLESv2/validationES.cpp
@@ -1038,12 +1038,6 @@ bool ValidateEndQuery(gl::Context *context, GLenum target)
         return false;
     }
 
-    if (!queryObject->isStarted())
-    {
-        context->recordError(Error(GL_INVALID_OPERATION));
-        return false;
-    }
-
     return true;
 }