On 27/02/15 02:52, Laura Ekstrand wrote:


On Mon, Feb 16, 2015 at 6:14 AM, Martin Peres <martin.pe...@linux.intel.com <mailto:martin.pe...@linux.intel.com>> wrote:

    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                       | 147
    ++++++++++++++++++-------
     src/mesa/main/fbobject.h                       |   9 ++
     src/mesa/main/tests/dispatch_sanity.cpp        |   2 +
     4 files changed, 135 insertions(+), 38 deletions(-)

    diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml
    b/src/mapi/glapi/gen/ARB_direct_state_access.xml
    index 144c891..a5fb84b 100644
    --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
    +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
    @@ -55,6 +55,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 c2ea59c..3f079df 100644
    --- a/src/mesa/main/fbobject.c
    +++ b/src/mesa/main/fbobject.c
    @@ -1407,7 +1407,7 @@ create_render_buffers(struct gl_context
    *ctx, GLsizei n, GLuint *renderbuffers,
           renderbuffers[i] = name;

           if (dsa) {
    -         obj = _mesa_new_renderbuffer(ctx, name);
    +         obj = ctx->Driver.NewRenderbuffer(ctx, name);

What is the reason for this change? Also, you should use rebase to move squash this into the previous commit; it doesn't make sense to put it with this one.

Totally agree, it was a fix that I had forgotten to move back to the right patch.

           } else {
              obj = &DummyRenderbuffer;
           }
    @@ -1772,40 +1772,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) {
    @@ -1815,12 +1792,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;
        }

    @@ -1832,7 +1811,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);
    @@ -1840,7 +1819,6 @@ renderbuffer_storage(GLenum target, GLenum
    internalFormat,
           }
        }

    -   rb = ctx->CurrentRenderbuffer;
        if (!rb) {
           _mesa_error(ctx, GL_INVALID_OPERATION, "%s", func);

Maybe say something like "%s(invalid renderbuffer name"? I typically prefer to throw this error earlier so that you can print out the name of the renderbuffer in the error message, too, but that's not a big deal.

Good idea!

           return;
    @@ -1887,6 +1865,75 @@ renderbuffer_storage(GLenum target, GLenum
    internalFormat,
        }
     }

    +/**
    + * Helper function used by _mesa_namedRenderbufferStorage*().

Should be a capital letter "N"--------------^

    + * 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);
    +   if (rb == &DummyRenderbuffer) {
    +      /* ID was reserved, but no real renderbuffer object made yet */
    +      rb = NULL;
    +   }
    +
    +   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;
    +   }
    +
    +   renderbuffer_storage(ctx, ctx->CurrentRenderbuffer,
    internalFormat, width,
    +                        height, samples, func);
    +}
    +

     void GLAPIENTRY
     _mesa_EGLImageTargetRenderbufferStorageOES(GLenum target,
    GLeglImageOES image)
    @@ -1946,7 +1993,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");
     }


    @@ -1955,10 +2003,10 @@
    _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");
     }

    -
     /**
      * OpenGL ES version of glRenderBufferStorage.
      */
    @@ -1976,7 +2024,30 @@ _es_RenderbufferStorageEXT(GLenum target,
    GLenum internalFormat,
           break;
        }

    -   renderbuffer_storage(target, internalFormat, width, height, 0);
    +   renderbuffer_storage_target(target, internalFormat, width,
    height, 0,
    +  "glRenderBufferStorageEXT");

Should be lowercase "b"---------------^

    +}
    +
    +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");

Indent from here--------------^to here-^

    +}
    +
    +void GLAPIENTRY
    +_mesa_NamedRenderbufferStorageMultisample(GLuint renderbuffer,
    GLsizei samples,
    +                                     GLenum internalformat,
    +                                     GLsizei width, GLsizei height)

Indent from here--------------------^--to here-------------------^

    +{
    +   renderbuffer_storage_named(renderbuffer, internalformat,
    width, height,
    +                               samples,
    +  "glNamedRenderbufferStorageMultisample");

Indent from here--------------^--to here^

     }


    diff --git a/src/mesa/main/fbobject.h b/src/mesa/main/fbobject.h
    index b92149b..93686cc 100644
    --- a/src/mesa/main/fbobject.h
    +++ b/src/mesa/main/fbobject.h
    @@ -130,6 +130,15 @@ extern void GLAPIENTRY
     _es_RenderbufferStorageEXT(GLenum target, GLenum internalFormat,
                               GLsizei width, GLsizei height);

    +void GLAPIENTRY

You forgot "extern"

    +_mesa_NamedRenderbufferStorage(GLuint renderbuffer, GLenum
    internalformat,
    +                               GLsizei width, GLsizei height);

Wrong indentation-----------^

    +
    +extern void GLAPIENTRY
    +_mesa_NamedRenderbufferStorageMultisample(GLuint renderbuffer,
    GLsizei samples,
    +                                          GLenum internalformat,
    +                                          GLsizei width, GLsizei
    height);

Wrong indentation----------------------^

    +
     extern void GLAPIENTRY
     _mesa_EGLImageTargetRenderbufferStorageOES(GLenum target,
    GLeglImageOES image);

    diff --git a/src/mesa/main/tests/dispatch_sanity.cpp
    b/src/mesa/main/tests/dispatch_sanity.cpp
    index 1d7a57e..316a8df 100644
    --- a/src/mesa/main/tests/dispatch_sanity.cpp
    +++ b/src/mesa/main/tests/dispatch_sanity.cpp
    @@ -962,6 +962,8 @@ const struct function
    gl_core_functions_possible[] = {
        { "glGetTransformFeedbacki_v", 45, -1 },
        { "glGetTransformFeedbacki64_v", 45, -1 },
        { "glCreateRenderbuffers", 45, -1 },
    +   { "glNamedRenderbufferStorage", 45, -1 },
    +   { "glNamedRenderbufferStorageMultisample", 45, -1 },
        { "glGetNamedRenderbufferParameteriv", 45, -1 },
        { "glCreateTextures", 45, -1 },
        { "glTextureStorage1D", 45, -1 }


Thanks for your review Laura! I will send an updated version of this patch in a jiffy so as you can check that I fixed everything and then I'll push the whole series :)
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to