main/framebuffer: refactor _mesa_get_color_read_format/type

Current implementation returns the value for the currently bound read
framebuffer. GetNamedFramebufferParameteriv allows to get it for any
given framebuffer. GetFramebufferParameteriv would be also interested
on that method

It was refactored by allowing to pass a given framebuffer. If NULL is
passed, it used the currently bound framebuffer.

It also adds a call to _mesa_update_state. When used only by
GetIntegerv, this one was called as part of the extra checks defined
at get_hash. But now that the method is used by more methods, and the
update is needed, it makes sense (and it is safer) just calling it on
the method itself, instead of rely on the caller.

Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>

diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
index c06130dc8d7f65479267c34c1de860e6a0fdfb5c..9002020b7a7ef5b22a425af416f0b5e222ca2646 100644 (file)
--- a/src/mesa/main/framebuffer.c
+++ b/src/mesa/main/framebuffer.c
@@ -44,6 +44,7 @@
 #include "renderbuffer.h"
 #include "texobj.h"
 #include "glformats.h"
+#include "state.h"
 
 
 
@@ -835,22 +836,54 @@ _mesa_dest_buffer_exists(struct gl_context *ctx, GLenum format)
 
 
 /**
- * Used to answer the GL_IMPLEMENTATION_COLOR_READ_FORMAT_OES query.
+ * Used to answer the GL_IMPLEMENTATION_COLOR_READ_FORMAT_OES queries (using
+ * GetIntegerv, GetFramebufferParameteriv, etc)
+ *
+ * If @fb is NULL, the method returns the value for the current bound
+ * framebuffer.
  */
 GLenum
-_mesa_get_color_read_format(struct gl_context *ctx)
+_mesa_get_color_read_format(struct gl_context *ctx,
+                            struct gl_framebuffer *fb,
+                            const char *caller)
 {
-   if (!ctx->ReadBuffer || !ctx->ReadBuffer->_ColorReadBuffer) {
-      /* The spec is unclear how to handle this case, but NVIDIA's
-       * driver generates GL_INVALID_OPERATION.
+   if (ctx->NewState)
+      _mesa_update_state(ctx);
+
+   if (fb == NULL)
+      fb = ctx->ReadBuffer;
+
+   if (!fb || !fb->_ColorReadBuffer) {
+      /*
+       * From OpenGL 4.5 spec, section 18.2.2 "ReadPixels":
+       *
+       *    "An INVALID_OPERATION error is generated by GetIntegerv if pname
+       *     is IMPLEMENTATION_COLOR_READ_FORMAT or IMPLEMENTATION_COLOR_-
+       *     READ_TYPE and any of:
+       *      * the read framebuffer is not framebuffer complete.
+       *      * the read framebuffer is a framebuffer object, and the selected
+       *        read buffer (see section 18.2.1) has no image attached.
+       *      * the selected read buffer is NONE."
+       *
+       * There is not equivalent quote for GetFramebufferParameteriv or
+       * GetNamedFramebufferParameteriv, but from section 9.2.3 "Framebuffer
+       * Object Queries":
+       *
+       *    "Values of framebuffer-dependent state are identical to those that
+       *     would be obtained were the framebuffer object bound and queried
+       *     using the simple state queries in that table."
+       *
+       * Where "using the simple state queries" refer to use GetIntegerv. So
+       * we will assume that on that situation the same error should be
+       * triggered too.
        */
       _mesa_error(ctx, GL_INVALID_OPERATION,
-                  "glGetIntegerv(GL_IMPLEMENTATION_COLOR_READ_FORMAT: "
-                  "no GL_READ_BUFFER)");
+                  "%s(GL_IMPLEMENTATION_COLOR_READ_FORMAT: no GL_READ_BUFFER)",
+                  caller);
       return GL_NONE;
    }
    else {
-      const mesa_format format = ctx->ReadBuffer->_ColorReadBuffer->Format;
+      const mesa_format format = fb->_ColorReadBuffer->Format;
       const GLenum data_type = _mesa_get_format_datatype(format);
 
       if (format == MESA_FORMAT_B8G8R8A8_UNORM)
@@ -872,22 +905,34 @@ _mesa_get_color_read_format(struct gl_context *ctx)
 
 
 /**
- * Used to answer the GL_IMPLEMENTATION_COLOR_READ_TYPE_OES query.
+ * Used to answer the GL_IMPLEMENTATION_COLOR_READ_TYPE_OES queries (using
+ * GetIntegerv, GetFramebufferParameteriv, etc)
+ *
+ * If @fb is NULL, the method returns the value for the current bound
+ * framebuffer.
  */
 GLenum
-_mesa_get_color_read_type(struct gl_context *ctx)
+_mesa_get_color_read_type(struct gl_context *ctx,
+                          struct gl_framebuffer *fb,
+                          const char *caller)
 {
-   if (!ctx->ReadBuffer || !ctx->ReadBuffer->_ColorReadBuffer) {
-      /* The spec is unclear how to handle this case, but NVIDIA's
-       * driver generates GL_INVALID_OPERATION.
+   if (ctx->NewState)
+      _mesa_update_state(ctx);
+
+   if (fb == NULL)
+      fb = ctx->ReadBuffer;
+
+   if (!fb || !fb->_ColorReadBuffer) {
+      /*
+       * See comment on _mesa_get_color_read_format
        */
       _mesa_error(ctx, GL_INVALID_OPERATION,
-                  "glGetIntegerv(GL_IMPLEMENTATION_COLOR_READ_TYPE: "
-                  "no GL_READ_BUFFER)");
+                  "%s(GL_IMPLEMENTATION_COLOR_READ_TYPE: no GL_READ_BUFFER)",
+                  caller);
       return GL_NONE;
    }
    else {
-      const GLenum format = ctx->ReadBuffer->_ColorReadBuffer->Format;
+      const GLenum format = fb->_ColorReadBuffer->Format;
       const GLenum data_type = _mesa_get_format_datatype(format);
 
       if (format == MESA_FORMAT_B5G6R5_UNORM)

diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h
index 745c1dabf64c84e6192e5201133c2806dff4f09b..ee0690b068bb57ca9ce2eda79923ef9843ef7ee5 100644 (file)
--- a/src/mesa/main/framebuffer.h
+++ b/src/mesa/main/framebuffer.h
@@ -128,10 +128,14 @@ extern GLboolean
 _mesa_dest_buffer_exists(struct gl_context *ctx, GLenum format);
 
 extern GLenum
-_mesa_get_color_read_type(struct gl_context *ctx);
+_mesa_get_color_read_type(struct gl_context *ctx,
+                          struct gl_framebuffer *fb,
+                          const char *caller);
 
 extern GLenum
-_mesa_get_color_read_format(struct gl_context *ctx);
+_mesa_get_color_read_format(struct gl_context *ctx,
+                            struct gl_framebuffer *fb,
+                            const char *caller);
 
 extern struct gl_renderbuffer *
 _mesa_get_read_renderbuffer_for_format(const struct gl_context *ctx,

diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index f0bb041fc383203d12b140dfc4134ecbf904d6a4..397f4a3547e1b4a15d67b2831504738a9456a1e7 100644 (file)
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -787,10 +787,10 @@ find_custom_value(struct gl_context *ctx, const struct value_desc *d, union valu
       break;
 
    case GL_IMPLEMENTATION_COLOR_READ_TYPE_OES:
-      v->value_int = _mesa_get_color_read_type(ctx);
+      v->value_int = _mesa_get_color_read_type(ctx, NULL, "glGetIntegerv");
       break;
    case GL_IMPLEMENTATION_COLOR_READ_FORMAT_OES:
-      v->value_int = _mesa_get_color_read_format(ctx);
+      v->value_int = _mesa_get_color_read_format(ctx, NULL, "glGetIntegerv");
       break;
 
    case GL_CURRENT_MATRIX_STACK_DEPTH_ARB:

diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index 1cb06c78a5a8fea8147bf2141235a83531101a9d..25823230d62f8824a8e4c3abb613dd61e9d59c81 100644 (file)
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -1033,8 +1033,8 @@ _mesa_ReadnPixelsARB( GLint x, GLint y, GLsizei width, GLsizei height,
    if (_mesa_is_gles(ctx)) {
       if (ctx->API == API_OPENGLES2 &&
           _mesa_is_color_format(format) &&
-          _mesa_get_color_read_format(ctx) == format &&
-          _mesa_get_color_read_type(ctx) == type) {
+          _mesa_get_color_read_format(ctx, NULL, "glReadPixels") == format &&
+          _mesa_get_color_read_type(ctx, NULL, "glReadPixels") == type) {
          err = GL_NO_ERROR;
       } else if (ctx->Version < 30) {
          err = _mesa_es_error_check_format_and_type(ctx, format, type, 2);