On 24/03/15 00:37, Laura Ekstrand wrote:
On Mon, Mar 23, 2015 at 12:14 PM, Martin Peres
<martin.pe...@linux.intel.com <mailto:martin.pe...@linux.intel.com>>
wrote:
v2: Review from Laura Ekstrand
- get rid of a change that should not have happened in this patch
- improve the error messages
- fix alignments
- fix a capitalization in a function name in an error message
v3: Review from Laura Ekstrand
- move the test for the validity of the renderbuffer to less generic
functions
- get rid of some changes that accidentally landed in the wrong commit
- revert some alignment fixes
Signed-off-by: Martin Peres <martin.pe...@linux.intel.com
<mailto:martin.pe...@linux.intel.com>>
---
src/mapi/glapi/gen/ARB_direct_state_access.xml | 15 +++
src/mesa/main/fbobject.c | 157
++++++++++++++++++-------
src/mesa/main/fbobject.h | 9 ++
src/mesa/main/tests/dispatch_sanity.cpp | 2 +
4 files changed, 142 insertions(+), 41 deletions(-)
diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml
b/src/mapi/glapi/gen/ARB_direct_state_access.xml
index d4e1f7c..8a092d6 100644
--- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
+++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
@@ -159,6 +159,21 @@
<param name="renderbuffers" type="GLuint *" />
</function>
+ <function name="NamedRenderbufferStorage" offset="assign">
+ <param name="renderbuffer" type="GLuint" />
+ <param name="internalformat" type="GLenum" />
+ <param name="width" type="GLsizei" />
+ <param name="height" type="GLsizei" />
+ </function>
+
+ <function name="NamedRenderbufferStorageMultisample"
offset="assign">
+ <param name="renderbuffer" type="GLuint" />
+ <param name="samples" type="GLsizei" />
+ <param name="internalformat" type="GLenum" />
+ <param name="width" type="GLsizei" />
+ <param name="height" type="GLsizei" />
+ </function>
+
<function name="GetNamedRenderbufferParameteriv" offset="assign">
<param name="renderbuffer" type="GLuint" />
<param name="pname" type="GLenum" />
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index ee4ad1f..d688195 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -1785,40 +1785,17 @@ invalidate_rb(GLuint key, void *data, void
*userData)
/**
- * Helper function used by _mesa_RenderbufferStorage() and
- * _mesa_RenderbufferStorageMultisample().
- * samples will be NO_SAMPLES if called by
_mesa_RenderbufferStorage().
+ * Helper function used by renderbuffer_storage_direct() and
+ * renderbuffer_storage_target().
+ * samples will be NO_SAMPLES if called by a non-multisample
function.
*/
static void
-renderbuffer_storage(GLenum target, GLenum internalFormat,
- GLsizei width, GLsizei height, GLsizei samples)
+renderbuffer_storage(struct gl_context *ctx, struct
gl_renderbuffer *rb,
+ GLenum internalFormat, GLsizei width,
+ GLsizei height, GLsizei samples, const char
*func)
{
- const char *func = samples == NO_SAMPLES ?
- "glRenderbufferStorage" : "glRenderbufferStorageMultisample";
- struct gl_renderbuffer *rb;
GLenum baseFormat;
GLenum sample_count_error;
- GET_CURRENT_CONTEXT(ctx);
-
- if (MESA_VERBOSE & VERBOSE_API) {
- if (samples == NO_SAMPLES)
- _mesa_debug(ctx, "%s(%s, %s, %d, %d)\n",
- func,
- _mesa_lookup_enum_by_nr(target),
- _mesa_lookup_enum_by_nr(internalFormat),
- width, height);
- else
- _mesa_debug(ctx, "%s(%s, %s, %d, %d, %d)\n",
- func,
- _mesa_lookup_enum_by_nr(target),
- _mesa_lookup_enum_by_nr(internalFormat),
- width, height, samples);
- }
-
- if (target != GL_RENDERBUFFER_EXT) {
- _mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", func);
- return;
- }
baseFormat = _mesa_base_fbo_format(ctx, internalFormat);
if (baseFormat == 0) {
@@ -1828,12 +1805,14 @@ renderbuffer_storage(GLenum target, GLenum
internalFormat,
}
if (width < 0 || width > (GLsizei)
ctx->Const.MaxRenderbufferSize) {
- _mesa_error(ctx, GL_INVALID_VALUE, "%s(width)", func);
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(invalid width %d)",
func,
+ width);
return;
}
if (height < 0 || height > (GLsizei)
ctx->Const.MaxRenderbufferSize) {
- _mesa_error(ctx, GL_INVALID_VALUE, "%s(height)", func);
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(invalid height %d)",
func,
+ height);
return;
}
@@ -1845,7 +1824,7 @@ renderbuffer_storage(GLenum target, GLenum
internalFormat,
/* check the sample count;
* note: driver may choose to use more samples than what's
requested
*/
- sample_count_error = _mesa_check_sample_count(ctx, target,
+ sample_count_error = _mesa_check_sample_count(ctx,
GL_RENDERBUFFER,
internalFormat, samples);
if (sample_count_error != GL_NO_ERROR) {
_mesa_error(ctx, sample_count_error, "%s(samples)", func);
@@ -1853,12 +1832,6 @@ renderbuffer_storage(GLenum target, GLenum
internalFormat,
}
}
- rb = ctx->CurrentRenderbuffer;
- if (!rb) {
- _mesa_error(ctx, GL_INVALID_OPERATION, "%s", func);
- return;
- }
-
FLUSH_VERTICES(ctx, _NEW_BUFFERS);
if (rb->InternalFormat == internalFormat &&
@@ -1900,6 +1873,83 @@ renderbuffer_storage(GLenum target, GLenum
internalFormat,
}
}
+/**
+ * Helper function used by _mesa_NamedRenderbufferStorage*().
+ * samples will be NO_SAMPLES if called by a non-multisample
function.
+ */
+static void
+renderbuffer_storage_named(GLuint renderbuffer, GLenum
internalFormat,
+ GLsizei width, GLsizei height, GLsizei
samples,
+ const char *func)
+{
+ GET_CURRENT_CONTEXT(ctx);
+
+ if (MESA_VERBOSE & VERBOSE_API) {
+ if (samples == NO_SAMPLES)
+ _mesa_debug(ctx, "%s(%u, %s, %d, %d)\n",
+ func, renderbuffer,
+ _mesa_lookup_enum_by_nr(internalFormat),
+ width, height);
+ else
+ _mesa_debug(ctx, "%s(%u, %s, %d, %d, %d)\n",
+ func, renderbuffer,
+ _mesa_lookup_enum_by_nr(internalFormat),
+ width, height, samples);
+ }
+
+ struct gl_renderbuffer *rb = _mesa_lookup_renderbuffer(ctx,
renderbuffer);
Here, also, you need to check if !rb to make sure rb is not NULL. If
the hash table doesn't find data with a particular key, it will hand
back NULL.
Fixed, thanks.
+ if (rb == &DummyRenderbuffer) {
+ /* ID was reserved, but no real renderbuffer object made yet */
+ _mesa_error(ctx, GL_INVALID_OPERATION, "%s (invalid
renderbuffer %u)",
Remove extra space here ---------------------------------------------^
Done
+ func, renderbuffer);
+ return;
+ }
+
+ renderbuffer_storage(ctx, rb, internalFormat, width, height,
samples, func);
+}
+
+/**
+ * Helper function used by _mesa_RenderbufferStorage() and
+ * _mesa_RenderbufferStorageMultisample().
+ * samples will be NO_SAMPLES if called by
_mesa_RenderbufferStorage().
+ */
+static void
+renderbuffer_storage_target(GLenum target, GLenum internalFormat,
+ GLsizei width, GLsizei height,
GLsizei samples,
+ const char *func)
+{
+ GET_CURRENT_CONTEXT(ctx);
+
+ if (MESA_VERBOSE & VERBOSE_API) {
+ if (samples == NO_SAMPLES)
+ _mesa_debug(ctx, "%s(%s, %s, %d, %d)\n",
+ func,
+ _mesa_lookup_enum_by_nr(target),
+ _mesa_lookup_enum_by_nr(internalFormat),
+ width, height);
+ else
+ _mesa_debug(ctx, "%s(%s, %s, %d, %d, %d)\n",
+ func,
+ _mesa_lookup_enum_by_nr(target),
+ _mesa_lookup_enum_by_nr(internalFormat),
+ width, height, samples);
+ }
+
+ if (target != GL_RENDERBUFFER_EXT) {
+ _mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", func);
+ return;
+ }
+
+ if (!ctx->CurrentRenderbuffer) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "%s (no renderbuffer
bound)",
Remove extra space here ---------------------------------------------^
Done. Thanks for the review. Will send another version to the list soon.
+ func);
+ return;
+ }
+
+ renderbuffer_storage(ctx, ctx->CurrentRenderbuffer,
internalFormat, width,
+ height, samples, func);
+}
+
void GLAPIENTRY
_mesa_EGLImageTargetRenderbufferStorageOES(GLenum target,
GLeglImageOES image)
@@ -1959,7 +2009,8 @@ _mesa_RenderbufferStorage(GLenum target,
GLenum internalFormat,
* glRenderbufferStorageMultisample() with samples=0. We pass in
* a token value here just for error reporting purposes.
*/
- renderbuffer_storage(target, internalFormat, width, height,
NO_SAMPLES);
+ renderbuffer_storage_target(target, internalFormat, width, height,
+ NO_SAMPLES, "glRenderbufferStorage");
}
@@ -1968,7 +2019,8 @@ _mesa_RenderbufferStorageMultisample(GLenum
target, GLsizei samples,
GLenum internalFormat,
GLsizei width, GLsizei height)
{
- renderbuffer_storage(target, internalFormat, width, height,
samples);
+ renderbuffer_storage_target(target, internalFormat, width, height,
+ samples,
"glRenderbufferStorageMultisample");
}
@@ -1989,7 +2041,30 @@ _es_RenderbufferStorageEXT(GLenum target,
GLenum internalFormat,
break;
}
- renderbuffer_storage(target, internalFormat, width, height, 0);
+ renderbuffer_storage_target(target, internalFormat, width,
height, 0,
+ "glRenderbufferStorageEXT");
+}
+
+void GLAPIENTRY
+_mesa_NamedRenderbufferStorage(GLuint renderbuffer, GLenum
internalformat,
+ GLsizei width, GLsizei height)
+{
+ /* GL_ARB_fbo says calling this function is equivalent to calling
+ * glRenderbufferStorageMultisample() with samples=0. We pass in
+ * a token value here just for error reporting purposes.
+ */
+ renderbuffer_storage_named(renderbuffer, internalformat,
width, height,
+ NO_SAMPLES,
"glNamedRenderbufferStorage");
+}
+
+void GLAPIENTRY
+_mesa_NamedRenderbufferStorageMultisample(GLuint renderbuffer,
GLsizei samples,
+ GLenum internalformat,
+ GLsizei width, GLsizei
height)
+{
+ renderbuffer_storage_named(renderbuffer, internalformat,
width, height,
+ samples,
+ "glNamedRenderbufferStorageMultisample");
}
diff --git a/src/mesa/main/fbobject.h b/src/mesa/main/fbobject.h
index b92149b..61aa1f5 100644
--- a/src/mesa/main/fbobject.h
+++ b/src/mesa/main/fbobject.h
@@ -131,6 +131,15 @@ _es_RenderbufferStorageEXT(GLenum target,
GLenum internalFormat,
GLsizei width, GLsizei height);
extern void GLAPIENTRY
+_mesa_NamedRenderbufferStorage(GLuint renderbuffer, GLenum
internalformat,
+ GLsizei width, GLsizei height);
+
+extern void GLAPIENTRY
+_mesa_NamedRenderbufferStorageMultisample(GLuint renderbuffer,
GLsizei samples,
+ GLenum internalformat,
+ GLsizei width, GLsizei
height);
+
+extern void GLAPIENTRY
_mesa_EGLImageTargetRenderbufferStorageOES(GLenum target,
GLeglImageOES image);
extern void GLAPIENTRY
diff --git a/src/mesa/main/tests/dispatch_sanity.cpp
b/src/mesa/main/tests/dispatch_sanity.cpp
index bb573d4..eb83e4d 100644
--- a/src/mesa/main/tests/dispatch_sanity.cpp
+++ b/src/mesa/main/tests/dispatch_sanity.cpp
@@ -944,6 +944,8 @@ const struct function
gl_core_functions_possible[] = {
{ "glGetNamedBufferPointerv", 45, -1 },
{ "glGetNamedBufferSubData", 45, -1 },
{ "glCreateRenderbuffers", 45, -1 },
+ { "glNamedRenderbufferStorage", 45, -1 },
+ { "glNamedRenderbufferStorageMultisample", 45, -1 },
{ "glGetNamedRenderbufferParameteriv", 45, -1 },
{ "glCreateTextures", 45, -1 },
{ "glTextureStorage1D", 45, -1 },
--
2.3.3
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org>
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
