[Mesa-dev] [PATCH 4/5] mesa: Add driver method to determine the possible sample counts

[Ian Romanick]

On 01/05/2013 11:40 AM, Jordan Justen wrote:
> On Fri, Jan 4, 2013 at 5:43 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Use this method in _mesa_GetInternalformativ for both GL_SAMPLES and
>> GL_NUM_SAMPLE_COUNTS.
>>
>> v2: internalFormat may not be color renderable by the driver, so zero
>> can be returned as a sample count.  Require that drivers supporting the
>> extension provide a QuerySamplesForFormat function.  The later was
>> suggested by Eric Anholt.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Cc: Eric Anholt <eric at anholt.net>
>> ---
>>   src/mesa/drivers/common/driverfuncs.c |  1 +
>>   src/mesa/main/dd.h                    | 16 ++++++++++++++++
>>   src/mesa/main/formatquery.c           | 31 +++++++++++++++++++++++++++----
>>   3 files changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/drivers/common/driverfuncs.c b/src/mesa/drivers/common/driverfuncs.c
>> index 93fa3c7..3de5199 100644
>> --- a/src/mesa/drivers/common/driverfuncs.c
>> +++ b/src/mesa/drivers/common/driverfuncs.c
>> @@ -91,6 +91,7 @@ _mesa_init_driver_functions(struct dd_function_table *driver)
>>
>>      /* Texture functions */
>>      driver->ChooseTextureFormat = _mesa_choose_tex_format;
>> +   driver->QuerySamplesForFormat = NULL;
>>      driver->TexImage = _mesa_store_teximage;
>>      driver->TexSubImage = _mesa_store_texsubimage;
>>      driver->GetTexImage = _mesa_meta_GetTexImage;
>> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
>> index 70c5324..07787d4 100644
>> --- a/src/mesa/main/dd.h
>> +++ b/src/mesa/main/dd.h
>> @@ -201,6 +201,22 @@ struct dd_function_table {
>>                                        GLenum srcFormat, GLenum srcType );
>>
>>      /**
>> +    * Determine sample counts support for a particular format
>> +    *
>> +    * \param ctx            GL context
>> +    * \param internalFormat GL format enum
>> +    * \param samples        Buffer to hold the returned sample counts.
>> +    *                       Drivers \b must \b not return more than 16 counts.
>> +    *
>> +    * \returns
>> +    * The number of sample counts actually written to \c samples.  If
>> +    * \c internaFormat is not renderable, zero is returned.
>> +    */
>> +   size_t (*QuerySamplesForFormat)(struct gl_context *ctx,
>> +                                   GLenum internalFormat,
>> +                                   int samples[16]);
>> +
>> +   /**
>>       * Called by glTexImage[123]D() and glCopyTexImage[12]D()
>>       * Allocate texture memory and copy the user's image to the buffer.
>>       * The gl_texture_image fields, etc. will be fully initialized.
>> diff --git a/src/mesa/main/formatquery.c b/src/mesa/main/formatquery.c
>> index 64f10cd..f08ab66 100644
>> --- a/src/mesa/main/formatquery.c
>> +++ b/src/mesa/main/formatquery.c
>> @@ -42,6 +42,8 @@ _mesa_GetInternalformativ(GLenum target, GLenum internalformat, GLenum pname,
>>         return;
>>      }
>>
>> +   assert(ctx->Driver.QuerySamplesForFormat != NULL);
>> +
>>      /* The ARB_internalformat_query spec says:
>>       *
>>       *     "If the <target> parameter to GetInternalformativ is not one of
>> @@ -91,13 +93,34 @@ _mesa_GetInternalformativ(GLenum target, GLenum internalformat, GLenum pname,
>>
>>      switch (pname) {
>>      case GL_SAMPLES:
>> -      buffer[0] = ctx->Const.MaxSamples;
>> -      count = 1;
>> +      count = ctx->Driver.QuerySamplesForFormat(ctx, internalformat, buffer);
>>         break;
>> -   case GL_NUM_SAMPLE_COUNTS:
>> -      buffer[0] = 1;
>> +   case GL_NUM_SAMPLE_COUNTS: {
>> +      /* The driver can return 0, and we should pass that along to the
>> +       * application.  The ARB decided that ARB_internalformat_query should
>> +       * behave as ARB_internalformat_query2 in this situation.
>> +       *
>> +       * The ARB_internalformat_query2 spec says:
>> +       *
>> +       *     "- NUM_SAMPLE_COUNTS: The number of sample counts that would be
>> +       *        returned by querying SAMPLES is returned in <params>.
>> +       *        * If <internalformat> is not color-renderable,
>> +       *          depth-renderable, or stencil-renderable (as defined in
>> +       *          section 4.4.4), or if <target> does not support multiple
>> +       *          samples (ie other than TEXTURE_2D_MULTISAMPLE,
>> +       *          TEXTURE_2D_MULTISAMPLE_ARRAY, or RENDERBUFFER), 0 is
>> +       *          returned."
>> +       */
>> +      const size_t num_samples =
>> +         ctx->Driver.QuerySamplesForFormat(ctx, internalformat, buffer);
>> +
>> +      /* QuerySamplesForFormat writes some stuff to buffer, so we have to
>> +       * separately over-write it with the requested value.
>> +       */
>> +      buffer[0] = (GLint) num_samples;
>
> What about removing the assert?
> if (ctx->Driver.QuerySamplesForFormat) {
>     num_samples =
>        ctx->Driver.QuerySamplesForFormat(ctx, internalformat, buffer);
> } else {
>     num_samples = 1;
> }

I think we want to required drivers provide this function.  Applications 
could request information about formats the driver can't render (but 
other drivers could).  In those cases, the driver should return 0.  This 
is also the reason that the assertion is after the extension check. :)

> -Jordan
>