[Mesa-dev] [PATCH 1/3] main/framebuffer: refactor _mesa_get_color_read_format/type
Alejandro Piñeiro
apinheiro at igalia.com
Thu Feb 2 18:51:48 UTC 2017
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.
---
I also updated the spec quotes, as now there is a quote that justifies
INVALID_OPERATION when readbuffer is not available with GetIntegerv
As mentioned on the patch, there is no equivalent for GetFramebufferParameteriv
or GetNamedFramebufferParameteriv, but there is another quote that links
the value of those with GetIntegerv, so I think that makes sense that in
the situations that GetIntegerv raises INVALID_OPERATION the other two too.
If the patch is accepted, I would open a spec bug.
src/mesa/main/framebuffer.c | 77 +++++++++++++++++++++++++++++++++++----------
src/mesa/main/framebuffer.h | 8 +++--
src/mesa/main/get.c | 4 +--
src/mesa/main/readpix.c | 4 +--
4 files changed, 71 insertions(+), 22 deletions(-)
diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
index c06130d..9002020 100644
--- 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 745c1da..ee0690b 100644
--- 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 f0bb041..397f4a3 100644
--- 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 1cb06c7..2582323 100644
--- 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);
--
2.9.3
More information about the mesa-dev
mailing list