Move state query validation out of Context.

src/libGLESv2/Context.cpp

@@ -763,6 +763,11 @@ GLuint Context::getSamplerHandle(GLuint textureUnit) const
     return mState.samplers[textureUnit];
 }
 
+unsigned int Context::getActiveSampler() const
+{
+    return mState.activeSampler;
+}
+
 GLuint Context::getArrayBufferHandle() const
 {
     return mState.arrayBuffer.id();
@@ -1626,7 +1631,7 @@ Texture *Context::getSamplerTexture(unsigned int sampler, TextureType type) cons
     return mState.samplerTexture[type][sampler].get();
 }
 
-bool Context::getBooleanv(GLenum pname, GLboolean *params)
+void Context::getBooleanv(GLenum pname, GLboolean *params)
 {
     switch (pname)
     {
@@ -1652,13 +1657,12 @@ bool Context::getBooleanv(GLenum pname, GLboolean *params)
       case GL_TRANSFORM_FEEDBACK_ACTIVE: *params = getCurrentTransformFeedback()->isStarted(); break;
       case GL_TRANSFORM_FEEDBACK_PAUSED: *params = getCurrentTransformFeedback()->isPaused();  break;
       default:
-        return false;
+        UNREACHABLE();
+        break;
     }
-
-    return true;
 }
 
-bool Context::getFloatv(GLenum pname, GLfloat *params)
+void Context::getFloatv(GLenum pname, GLfloat *params)
 {
     // Please note: DEPTH_CLEAR_VALUE is included in our internal getFloatv implementation
     // because it is stored as a float, despite the fact that the GL ES 2.0 spec names
@@ -1696,35 +1700,24 @@ bool Context::getFloatv(GLenum pname, GLfloat *params)
         params[3] = mState.blendColor.alpha;
         break;
       case GL_MAX_TEXTURE_MAX_ANISOTROPY_EXT:
-        if (!supportsTextureFilterAnisotropy())
-        {
-            return false;
-        }
+        ASSERT(supportsTextureFilterAnisotropy());
         *params = mMaxTextureAnisotropy;
         break;
       default:
-        return false;
+        UNREACHABLE();
+        break;
     }
-
-    return true;
 }
 
-bool Context::getIntegerv(GLenum pname, GLint *params)
+void Context::getIntegerv(GLenum pname, GLint *params)
 {
     if (pname >= GL_DRAW_BUFFER0_EXT && pname <= GL_DRAW_BUFFER15_EXT)
     {
         unsigned int colorAttachment = (pname - GL_DRAW_BUFFER0_EXT);
-
-        if (colorAttachment >= mRenderer->getMaxRenderTargets())
-        {
-            // return true to stop further operation in the parent call
-            return gl::error(GL_INVALID_OPERATION, true);
-        }
-
+        ASSERT(colorAttachment < mRenderer->getMaxRenderTargets());
         Framebuffer *framebuffer = getDrawFramebuffer();
-
         *params = framebuffer->getDrawBufferState(colorAttachment);
-        return true;
+        return;
     }
 
     // Please note: DEPTH_CLEAR_VALUE is not included in our internal getIntegerv implementation
@@ -1806,19 +1799,8 @@ bool Context::getIntegerv(GLenum pname, GLint *params)
         params[0] = mNumCompressedTextureFormats;
         break;
       case GL_MAX_SAMPLES_ANGLE:
-        {
-            GLsizei maxSamples = getMaxSupportedSamples();
-            if (maxSamples != 0)
-            {
-                *params = maxSamples;
-            }
-            else
-            {
-                return false;
-            }
-
+        *params = static_cast<GLint>(getMaxSupportedSamples());
         break;
-        }
       case GL_SAMPLE_BUFFERS:
       case GL_SAMPLES:
         {
@@ -1852,14 +1834,12 @@ bool Context::getIntegerv(GLenum pname, GLint *params)
       case GL_IMPLEMENTATION_COLOR_READ_FORMAT:
         {
             GLenum internalFormat, format, type;
-            if (getCurrentReadFormatType(&internalFormat, &format, &type))
-            {
+            getCurrentReadFormatType(&internalFormat, &format, &type);
             if (pname == GL_IMPLEMENTATION_COLOR_READ_FORMAT)
                 *params = format;
             else
                 *params = type;
         }
-        }
         break;
       case GL_MAX_VIEWPORT_DIMS:
         {
@@ -1953,48 +1933,20 @@ bool Context::getIntegerv(GLenum pname, GLint *params)
         }
         break;
       case GL_TEXTURE_BINDING_2D:
-        {
-            if (mState.activeSampler > mRenderer->getMaxCombinedTextureImageUnits() - 1)
-            {
-                gl::error(GL_INVALID_OPERATION);
-                return false;
-            }
-
+        ASSERT(mState.activeSampler < mRenderer->getMaxCombinedTextureImageUnits());
         *params = mState.samplerTexture[TEXTURE_2D][mState.activeSampler].id();
-        }
         break;
       case GL_TEXTURE_BINDING_CUBE_MAP:
-        {
-            if (mState.activeSampler > mRenderer->getMaxCombinedTextureImageUnits() - 1)
-            {
-                gl::error(GL_INVALID_OPERATION);
-                return false;
-            }
-
+        ASSERT(mState.activeSampler < mRenderer->getMaxCombinedTextureImageUnits());
         *params = mState.samplerTexture[TEXTURE_CUBE][mState.activeSampler].id();
-        }
         break;
       case GL_TEXTURE_BINDING_3D:
-        {
-            if (mState.activeSampler > mRenderer->getMaxCombinedTextureImageUnits() - 1)
-            {
-                gl::error(GL_INVALID_OPERATION);
-                return false;
-            }
-
+        ASSERT(mState.activeSampler < mRenderer->getMaxCombinedTextureImageUnits());
         *params = mState.samplerTexture[TEXTURE_3D][mState.activeSampler].id();
-        }
         break;
       case GL_TEXTURE_BINDING_2D_ARRAY:
-        {
-            if (mState.activeSampler > mRenderer->getMaxCombinedTextureImageUnits() - 1)
-            {
-                gl::error(GL_INVALID_OPERATION);
-                return false;
-            }
-
+        ASSERT(mState.activeSampler < mRenderer->getMaxCombinedTextureImageUnits());
         *params = mState.samplerTexture[TEXTURE_2D_ARRAY][mState.activeSampler].id();
-        }
         break;
       case GL_RESET_NOTIFICATION_STRATEGY_EXT:
         *params = mResetStrategy;
@@ -2027,13 +1979,12 @@ bool Context::getIntegerv(GLenum pname, GLint *params)
         *params = static_cast<GLint>(getNumExtensions());
         break;
       default:
-        return false;
+        UNREACHABLE();
+        break;
     }
-
-    return true;
 }
 
-bool Context::getInteger64v(GLenum pname, GLint64 *params)
+void Context::getInteger64v(GLenum pname, GLint64 *params)
 {
     switch (pname)
     {
@@ -2062,10 +2013,9 @@ bool Context::getInteger64v(GLenum pname, GLint64 *params)
         *params = 0;
         break;
       default:
-        return false;
+        UNREACHABLE();
+        break;
     }
-
-    return true;
 }
 
 bool Context::getIndexedIntegerv(GLenum target, GLuint index, GLint *data)
@@ -3393,25 +3343,17 @@ float Context::getTextureMaxAnisotropy() const
     return mMaxTextureAnisotropy;
 }
 
-bool Context::getCurrentReadFormatType(GLenum *internalFormat, GLenum *format, GLenum *type)
+void Context::getCurrentReadFormatType(GLenum *internalFormat, GLenum *format, GLenum *type)
 {
     Framebuffer *framebuffer = getReadFramebuffer();
-    if (!framebuffer || framebuffer->completeness() != GL_FRAMEBUFFER_COMPLETE)
-    {
-        return gl::error(GL_INVALID_OPERATION, false);
-    }
+    ASSERT(framebuffer && framebuffer->completeness() == GL_FRAMEBUFFER_COMPLETE);
 
     Renderbuffer *renderbuffer = framebuffer->getReadColorbuffer();
-    if (!renderbuffer)
-    {
-        return gl::error(GL_INVALID_OPERATION, false);
-    }
+    ASSERT(renderbuffer);
 
     *internalFormat = renderbuffer->getActualFormat();
     *format = gl::GetFormat(renderbuffer->getActualFormat(), mClientVersion);
     *type = gl::GetType(renderbuffer->getActualFormat(), mClientVersion);
-
-    return true;
 }
 
 void Context::detachBuffer(GLuint buffer)

src/libGLESv2/Context.h

@@ -225,6 +225,7 @@ class Context
     GLuint getRenderbufferHandle() const;
     GLuint getVertexArrayHandle() const;
     GLuint getSamplerHandle(GLuint textureUnit) const;
+    unsigned int getActiveSampler() const;
 
     GLuint getArrayBufferHandle() const;
 
@@ -368,10 +369,10 @@ class Context
 
     bool isSampler(GLuint samplerName) const;
 
-    bool getBooleanv(GLenum pname, GLboolean *params);
-    bool getFloatv(GLenum pname, GLfloat *params);
-    bool getIntegerv(GLenum pname, GLint *params);
-    bool getInteger64v(GLenum pname, GLint64 *params);
+    void getBooleanv(GLenum pname, GLboolean *params);
+    void getFloatv(GLenum pname, GLfloat *params);
+    void getIntegerv(GLenum pname, GLint *params);
+    void getInteger64v(GLenum pname, GLint64 *params);
 
     bool getIndexedIntegerv(GLenum target, GLuint index, GLint *data);
     bool getIndexedInteger64v(GLenum target, GLuint index, GLint64 *data);
@@ -448,7 +449,7 @@ class Context
     bool supportsTextureFilterAnisotropy() const;
     bool supportsPBOs() const;
 
-    bool getCurrentReadFormatType(GLenum *internalFormat, GLenum *format, GLenum *type);
+    void getCurrentReadFormatType(GLenum *internalFormat, GLenum *format, GLenum *type);
 
     float getTextureMaxAnisotropy() const;
 

src/libGLESv2/libGLESv2.cpp

@@ -2450,12 +2450,10 @@ void __stdcall glGetBooleanv(GLenum pname, GLboolean* params)
         {
             GLenum nativeType;
             unsigned int numParams = 0;
-            if (!context->getQueryParameterInfo(pname, &nativeType, &numParams))
-                return gl::error(GL_INVALID_ENUM);
-
-            // pname is valid, but there are no parameters to return
-            if (numParams == 0)
+            if (!ValidateStateQuery(context, pname, &nativeType, &numParams))
+            {
                 return;
+            }
 
             if (nativeType == GL_BOOL)
             {
@@ -2598,12 +2596,10 @@ void __stdcall glGetFloatv(GLenum pname, GLfloat* params)
         {
             GLenum nativeType;
             unsigned int numParams = 0;
-            if (!context->getQueryParameterInfo(pname, &nativeType, &numParams))
-                return gl::error(GL_INVALID_ENUM);
-
-            // pname is valid, but that there are no parameters to return.
-            if (numParams == 0)
+            if (!ValidateStateQuery(context, pname, &nativeType, &numParams))
+            {
                 return;
+            }
 
             if (nativeType == GL_FLOAT)
             {
@@ -2957,12 +2953,11 @@ void __stdcall glGetIntegerv(GLenum pname, GLint* params)
         {
             GLenum nativeType;
             unsigned int numParams = 0;
-            if (!context->getQueryParameterInfo(pname, &nativeType, &numParams))
-                return gl::error(GL_INVALID_ENUM);
 
-            // pname is valid, but there are no parameters to return
-            if (numParams == 0)
+            if (!ValidateStateQuery(context, pname, &nativeType, &numParams))
+            {
                 return;
+            }
 
             if (nativeType == GL_INT)
             {
@@ -8779,12 +8774,10 @@ void __stdcall glGetInteger64v(GLenum pname, GLint64* params)
 
             GLenum nativeType;
             unsigned int numParams = 0;
-            if (!context->getQueryParameterInfo(pname, &nativeType, &numParams))
-                return gl::error(GL_INVALID_ENUM);
-
-            // pname is valid, but that there are no parameters to return.
-            if (numParams == 0)
+            if (!ValidateStateQuery(context, pname, &nativeType, &numParams))
+            {
                 return;
+            }
 
             if (nativeType == GL_INT_64_ANGLEX)
             {

src/libGLESv2/validationES.cpp

@@ -829,6 +829,7 @@ bool ValidateReadPixelsParameters(gl::Context *context, GLint x, GLint y, GLsize
                                   GLenum format, GLenum type, GLsizei *bufSize, GLvoid *pixels)
 {
     gl::Framebuffer *framebuffer = context->getReadFramebuffer();
+    ASSERT(framebuffer);
 
     if (framebuffer->completeness() != GL_FRAMEBUFFER_COMPLETE)
     {
@@ -840,16 +841,15 @@ bool ValidateReadPixelsParameters(gl::Context *context, GLint x, GLint y, GLsize
         return gl::error(GL_INVALID_OPERATION, false);
     }
 
+    if (!framebuffer->getReadColorbuffer())
+    {
+        return gl::error(GL_INVALID_OPERATION, false);
+    }
+
     GLenum currentInternalFormat, currentFormat, currentType;
     int clientVersion = context->getClientVersion();
 
-    // Failure in getCurrentReadFormatType indicates that no color attachment is currently bound,
-    // and attempting to read back if that's the case is an error. The error will be registered
-    // by getCurrentReadFormat.
-    // Note: we need to explicitly check for framebuffer completeness here, before we call
-    // getCurrentReadFormatType, because it generates a different (wrong) error for incomplete FBOs
-    if (!context->getCurrentReadFormatType(&currentInternalFormat, &currentFormat, &currentType))
-        return false;
+    context->getCurrentReadFormatType(&currentInternalFormat, &currentFormat, &currentType);
 
     bool validReadFormat = (clientVersion < 3) ? ValidES2ReadFormatType(context, format, type) :
                                                  ValidES3ReadFormatType(context, currentInternalFormat, format, type);
@@ -1038,4 +1038,64 @@ bool ValidateUniformMatrix(gl::Context *context, GLenum matrixType, GLint locati
     return true;
 }
 
+bool ValidateStateQuery(gl::Context *context, GLenum pname, GLenum *nativeType, unsigned int *numParams)
+{
+    if (!context->getQueryParameterInfo(pname, nativeType, numParams))
+    {
+        return gl::error(GL_INVALID_ENUM, false);
+    }
+
+    if (pname >= GL_DRAW_BUFFER0 && pname <= GL_DRAW_BUFFER15)
+    {
+        unsigned int colorAttachment = (pname - GL_DRAW_BUFFER0);
+
+        if (colorAttachment >= context->getMaximumRenderTargets())
+        {
+            return gl::error(GL_INVALID_OPERATION, false);
+        }
+    }
+
+    switch (pname)
+    {
+      case GL_TEXTURE_BINDING_2D:
+      case GL_TEXTURE_BINDING_CUBE_MAP:
+      case GL_TEXTURE_BINDING_3D:
+      case GL_TEXTURE_BINDING_2D_ARRAY:
+        if (context->getActiveSampler() >= context->getMaximumCombinedTextureImageUnits())
+        {
+            return gl::error(GL_INVALID_OPERATION, false);
+        }
+        break;
+
+      case GL_IMPLEMENTATION_COLOR_READ_TYPE:
+      case GL_IMPLEMENTATION_COLOR_READ_FORMAT:
+        {
+            Framebuffer *framebuffer = context->getReadFramebuffer();
+            ASSERT(framebuffer);
+            if (framebuffer->completeness() != GL_FRAMEBUFFER_COMPLETE)
+            {
+                return gl::error(GL_INVALID_OPERATION, false);
+            }
+
+            Renderbuffer *renderbuffer = framebuffer->getReadColorbuffer();
+            if (!renderbuffer)
+            {
+                return gl::error(GL_INVALID_OPERATION, false);
+            }
+        }
+        break;
+
+      default:
+        break;
+    }
+
+    // pname is valid, but there are no parameters to return
+    if (numParams == 0)
+    {
+        return false;
+    }
+
+    return true;
+}
+
 }

src/libGLESv2/validationES.h

@@ -52,6 +52,8 @@ bool ValidateUniform(gl::Context *context, GLenum uniformType, GLint location, G
 bool ValidateUniformMatrix(gl::Context *context, GLenum matrixType, GLint location, GLsizei count,
                            GLboolean transpose);
 
+bool ValidateStateQuery(gl::Context *context, GLenum pname, GLenum *nativeType, unsigned int *numParams);
+
 }
 
 #endif // LIBGLESV2_VALIDATION_ES_H