[Mesa-dev] [PATCH v3 16/17] main: Added entry points for NamedRenderbufferStorage/Multisample

Laura Ekstrand laura at jlekstrand.net
Mon Mar 23 15:37:09 PDT 2015


On Mon, Mar 23, 2015 at 12:14 PM, Martin Peres <martin.peres at 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.peres at 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.

> +   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 ---------------------------------------------^

> +                  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 ---------------------------------------------^

> +                  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150323/5ed7f603/attachment-0001.html>


More information about the mesa-dev mailing list