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