[Mesa-dev] [PATCH 1/5] mesa: add support for ARB_sample_locations

Brian Paul brianp at vmware.com
Mon May 7 15:30:13 UTC 2018


A bunch of nit-picks below.


On 05/04/2018 06:09 AM, Rhys Perry wrote:
> Signed-off-by: Rhys Perry <pendingchaos02 at gmail.com>
> ---
>   src/mapi/glapi/gen/gl_API.xml           |  52 +++++++
>   src/mesa/main/config.h                  |   7 +
>   src/mesa/main/dd.h                      |   7 +
>   src/mesa/main/extensions_table.h        |   1 +
>   src/mesa/main/fbobject.c                | 247 ++++++++++++++++++++++++++++----
>   src/mesa/main/fbobject.h                |  20 +++
>   src/mesa/main/framebuffer.c             |  10 ++
>   src/mesa/main/get.c                     |  32 +++++
>   src/mesa/main/get_hash_params.py        |   6 +
>   src/mesa/main/mtypes.h                  |   6 +
>   src/mesa/main/multisample.c             |  18 +++
>   src/mesa/main/tests/dispatch_sanity.cpp |  10 ++
>   12 files changed, 386 insertions(+), 30 deletions(-)
> 
> diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
> index 38c1921047..a23094a548 100644
> --- a/src/mapi/glapi/gen/gl_API.xml
> +++ b/src/mapi/glapi/gen/gl_API.xml
> @@ -10891,6 +10891,58 @@
>   
>   <!-- Extension number 180 is not listed in the extension registry. -->
>   
> +<category name="GL_ARB_sample_locations" number="181">
> +    <enum name="SAMPLE_LOCATION_SUBPIXEL_BITS_ARB"             value="0x933D">
> +        <size name="Get" mode="get"/>
> +    </enum>
> +
> +    <enum name="SAMPLE_LOCATION_PIXEL_GRID_WIDTH_ARB"          value="0x933E">
> +        <size name="Get" mode="get"/>
> +    </enum>
> +
> +    <enum name="SAMPLE_LOCATION_PIXEL_GRID_HEIGHT_ARB"         value="0x933F">
> +        <size name="Get" mode="get"/>
> +    </enum>
> +
> +    <enum name="PROGRAMMABLE_SAMPLE_LOCATION_TABLE_SIZE_ARB"   value="0x9340">
> +        <size name="Get" mode="get"/>
> +    </enum>
> +
> +    <enum name="SAMPLE_LOCATION_ARB"                           value="0x8E50">
> +        <size name="GetMultisamplefv" mode="get"/>
> +    </enum>
> +
> +    <enum name="PROGRAMMABLE_SAMPLE_LOCATION_ARB"              value="0x9341">
> +        <size name="GetMultisamplefv" mode="get"/>
> +    </enum>
> +
> +    <enum name="FRAMEBUFFER_PROGRAMMABLE_SAMPLE_LOCATIONS_ARB" value="0x9342">
> +        <size name="FramebufferParameteri"/>
> +        <size name="GetFramebufferParameteri"/>
> +    </enum>
> +
> +    <enum name="FRAMEBUFFER_SAMPLE_LOCATION_PIXEL_GRID_ARB"    value="0x9343">
> +        <size name="FramebufferParameteri"/>
> +        <size name="GetFramebufferParameteri"/>
> +    </enum>
> +
> +    <function name="FramebufferSampleLocationsfvARB" no_error="true" es2="3.1">
> +        <param name="target" type="GLenum"/>
> +        <param name="start"  type="GLuint"/>
> +        <param name="count"  type="GLsizei"/>
> +        <param name="v"      type="const GLfloat *"/>
> +    </function>
> +
> +    <function name="NamedFramebufferSampleLocationsfvARB" no_error="true" es2="3.1">
> +        <param name="framebuffer" type="GLuint"/>
> +        <param name="start"       type="GLuint"/>
> +        <param name="count"       type="GLsizei"/>
> +        <param name="v"           type="const GLfloat *"/>
> +    </function>
> +
> +    <function name="EvaluateDepthValuesARB" es2="3.1"/>
> +</category>
> +
>   <category name="GL_SUN_convolution_border_modes" number="182">
>       <enum name="WRAP_BORDER_SUN"                          value="0x81D4"/>
>   </category>
> diff --git a/src/mesa/main/config.h b/src/mesa/main/config.h
> index 81573bfbf2..444e4dedad 100644
> --- a/src/mesa/main/config.h
> +++ b/src/mesa/main/config.h
> @@ -315,4 +315,11 @@
>   #define MAX_CLIPPED_VERTICES ((2 * (6 + MAX_CLIP_PLANES))+1)
>   
>   
> +/** For GL_ARB_sample_locations - maximum of SAMPLE_LOCATION_PIXEL_GRID_*_ARB */
> +#define MAX_SAMPLE_LOCATION_GRID_SIZE 4
> +/* It is theoretically possible for Consts.MaxSamples to be >32 but
> + * other code seems to assume that is not the case */
> +#define MAX_SAMPLE_LOCATION_TABLE_SIZE \
> +   (MAX_SAMPLE_LOCATION_GRID_SIZE*MAX_SAMPLE_LOCATION_GRID_SIZE*32)
> +
>   #endif /* MESA_CONFIG_H_INCLUDED */
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index d85d89ef50..8929a2e267 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -785,6 +785,13 @@ struct dd_function_table {
>                                 GLenum target, GLsizei numAttachments,
>                                 const GLenum *attachments);
>   
> +   /**
> +    * \name Functions for GL_ARB_sample_locations
> +    */
> +   void (*GetProgrammableSampleCaps)(struct gl_context *ctx, struct gl_framebuffer *fb,
> +                                     GLuint *bits, GLuint *width, GLuint *height);
> +   void (*EvaluateDepthValues)(struct gl_context *ctx, struct gl_framebuffer *fb);

Can the fb param be const qualified for these?


> +
>      /**
>       * \name Query objects
>       */
> diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
> index 492f7c3d20..3497cbea0e 100644
> --- a/src/mesa/main/extensions_table.h
> +++ b/src/mesa/main/extensions_table.h
> @@ -103,6 +103,7 @@ EXT(ARB_provoking_vertex                    , EXT_provoking_vertex
>   EXT(ARB_query_buffer_object                 , ARB_query_buffer_object                , GLL, GLC,  x ,  x , 2013)
>   EXT(ARB_robust_buffer_access_behavior       , ARB_robust_buffer_access_behavior      , GLL, GLC,  x ,  x , 2012)
>   EXT(ARB_robustness                          , dummy_true                             , GLL, GLC,  x ,  x , 2010)
> +EXT(ARB_sample_locations                    , ARB_sample_locations                   , GLL, GLC,  x , ES2, 2015)
>   EXT(ARB_sample_shading                      , ARB_sample_shading                     , GLL, GLC,  x ,  x , 2009)
>   EXT(ARB_sampler_objects                     , dummy_true                             , GLL, GLC,  x ,  x , 2009)
>   EXT(ARB_seamless_cube_map                   , ARB_seamless_cube_map                  , GLL, GLC,  x ,  x , 2009)
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index c72204e11a..33d7f0307d 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -35,6 +35,7 @@
>   
>   #include "buffers.h"
>   #include "context.h"
> +#include "debug_output.h"
>   #include "enums.h"
>   #include "fbobject.h"
>   #include "formats.h"
> @@ -1403,15 +1404,57 @@ _mesa_BindRenderbufferEXT(GLenum target, GLuint renderbuffer)
>      bind_renderbuffer(target, renderbuffer, true);
>   }
>   
> +static bool
> +_pname_writable_for_default_framebuffer(struct gl_context *ctx,
> +                                        GLenum pname)
> +{
> +   switch (pname) {
> +   case GL_FRAMEBUFFER_PROGRAMMABLE_SAMPLE_LOCATIONS_ARB:
> +   case GL_FRAMEBUFFER_SAMPLE_LOCATION_PIXEL_GRID_ARB:
> +      return true;
> +   default:
> +      return false;
> +   }
> +}

The _ prefix on the function is not needed for static functions.  ctx is 
not used.


> +
>   /**
> - * ARB_framebuffer_no_attachment - Application passes requested param's
> - * here. NOTE: NumSamples requested need not be _NumSamples which is
> - * what the hw supports.
> + * ARB_framebuffer_no_attachment and ARB_sample_locations - Application passes
> + * requested param's here. NOTE: NumSamples requested need not be _NumSamples
> + * which is what the hw supports.
>    */
>   static void
>   framebuffer_parameteri(struct gl_context *ctx, struct gl_framebuffer *fb,
>                          GLenum pname, GLint param, const char *func)
>   {
> +   switch (pname) {
> +   case GL_FRAMEBUFFER_DEFAULT_WIDTH:
> +   case GL_FRAMEBUFFER_DEFAULT_HEIGHT:
> +   case GL_FRAMEBUFFER_DEFAULT_LAYERS:
> +   case GL_FRAMEBUFFER_DEFAULT_SAMPLES:
> +   case GL_FRAMEBUFFER_DEFAULT_FIXED_SAMPLE_LOCATIONS:
> +      if (!ctx->Extensions.ARB_framebuffer_no_attachments)
> +         goto invalid_pname_enum;
> +      break;
> +   case GL_FRAMEBUFFER_PROGRAMMABLE_SAMPLE_LOCATIONS_ARB:
> +   case GL_FRAMEBUFFER_SAMPLE_LOCATION_PIXEL_GRID_ARB:
> +      if (!ctx->Extensions.ARB_sample_locations)
> +         goto invalid_pname_enum;
> +      break;
> +   default:
> +      goto invalid_pname_enum;
> +   }
> +
> +   if (_mesa_is_winsys_fbo(fb) &&
> +       !_pname_writable_for_default_framebuffer(ctx, pname)) {
> +      if (ctx->Extensions.ARB_sample_locations)
> +         _mesa_error(ctx, GL_INVALID_OPERATION,
> +                     "%s(invalid pname=0x%x for default framebuffer)", func, pname);
> +      else
> +         _mesa_error(ctx, GL_INVALID_OPERATION,
> +                     "%s(default framebuffer used)", func);
> +      return;
> +   }
> +
>      switch (pname) {
>      case GL_FRAMEBUFFER_DEFAULT_WIDTH:
>         if (param < 0 || param > ctx->Const.MaxFramebufferWidth)
> @@ -1448,13 +1491,29 @@ framebuffer_parameteri(struct gl_context *ctx, struct gl_framebuffer *fb,
>      case GL_FRAMEBUFFER_DEFAULT_FIXED_SAMPLE_LOCATIONS:
>         fb->DefaultGeometry.FixedSampleLocations = param;
>         break;
> -   default:
> -      _mesa_error(ctx, GL_INVALID_ENUM,
> -                  "%s(pname=0x%x)", func, pname);
> +   case GL_FRAMEBUFFER_PROGRAMMABLE_SAMPLE_LOCATIONS_ARB:
> +      fb->ProgrammableSampleLocations = param;

= !!param to be safe, and below.


> +      break;
> +   case GL_FRAMEBUFFER_SAMPLE_LOCATION_PIXEL_GRID_ARB:
> +      fb->SampleLocationPixelGrid = param;
> +      break;
>      }
>   
> -   invalidate_framebuffer(fb);
> +   switch (pname) {
> +   case GL_FRAMEBUFFER_PROGRAMMABLE_SAMPLE_LOCATIONS_ARB:
> +   case GL_FRAMEBUFFER_SAMPLE_LOCATION_PIXEL_GRID_ARB:
> +      break;

Can the _pname_writable_for_default_framebuffer() function be re-used here?


> +   default:
> +      invalidate_framebuffer(fb);
> +      break;
> +   }
>      ctx->NewState |= _NEW_BUFFERS;
> +
> +   return;
> +
> +invalid_pname_enum:
> +   _mesa_error(ctx, GL_INVALID_ENUM,
> +               "%s(pname=0x%x)", func, pname);

I think that could be on one line.

>   }
>   
>   void GLAPIENTRY
> @@ -1463,10 +1522,10 @@ _mesa_FramebufferParameteri(GLenum target, GLenum pname, GLint param)
>      GET_CURRENT_CONTEXT(ctx);
>      struct gl_framebuffer *fb;
>   
> -   if (!ctx->Extensions.ARB_framebuffer_no_attachments) {
> +   if (!ctx->Extensions.ARB_framebuffer_no_attachments && !ctx->Extensions.ARB_sample_locations) {
>         _mesa_error(ctx, GL_INVALID_OPERATION,
>                     "glFramebufferParameteriv not supported "
> -                  "(ARB_framebuffer_no_attachments not implemented)");
> +                  "(ARB_framebuffer_no_attachments or ARB_sample_locations not implemented)");
>         return;
>      }
>   
> @@ -1477,23 +1536,13 @@ _mesa_FramebufferParameteri(GLenum target, GLenum pname, GLint param)
>         return;
>      }
>   
> -   /* check framebuffer binding */
> -   if (_mesa_is_winsys_fbo(fb)) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION,
> -                  "glFramebufferParameteri");
> -      return;
> -   }
> -
>      framebuffer_parameteri(ctx, fb, pname, param, "glFramebufferParameteri");
>   }
>   
>   static bool
> -_pname_valid_for_default_framebuffer(struct gl_context *ctx,
> -                                     GLenum pname)
> +_pname_readable_for_default_framebuffer(struct gl_context *ctx,
> +                                        GLenum pname)

I think you could const-qualify ctx while you're here.


>   {
> -   if (!_mesa_is_desktop_gl(ctx))
> -      return false;
> -
>      switch (pname) {
>      case GL_DOUBLEBUFFER:
>      case GL_IMPLEMENTATION_COLOR_READ_FORMAT:
> @@ -1501,6 +1550,9 @@ _pname_valid_for_default_framebuffer(struct gl_context *ctx,
>      case GL_SAMPLES:
>      case GL_SAMPLE_BUFFERS:
>      case GL_STEREO:
> +      return _mesa_is_desktop_gl(ctx);
> +   case GL_FRAMEBUFFER_PROGRAMMABLE_SAMPLE_LOCATIONS_ARB:
> +   case GL_FRAMEBUFFER_SAMPLE_LOCATION_PIXEL_GRID_ARB:
>         return true;
>      default:
>         return false;
> @@ -1519,10 +1571,10 @@ get_framebuffer_parameteriv(struct gl_context *ctx, struct gl_framebuffer *fb,
>       *     SAMPLE_POSITION."
>       *
>       * For OpenGL ES, using default framebuffer still raises INVALID_OPERATION
> -    * for any pname.
> +    * for any pname other than the ARB_sample_location names.
>       */
>      if (_mesa_is_winsys_fbo(fb) &&
> -       !_pname_valid_for_default_framebuffer(ctx, pname)) {
> +       !_pname_readable_for_default_framebuffer(ctx, pname)) {
>         _mesa_error(ctx, GL_INVALID_OPERATION,
>                     "%s(invalid pname=0x%x for default framebuffer)", func, pname);
>         return;
> @@ -1570,6 +1622,20 @@ get_framebuffer_parameteriv(struct gl_context *ctx, struct gl_framebuffer *fb,
>      case GL_STEREO:
>         *params = fb->Visual.stereoMode;
>         break;
> +   case GL_FRAMEBUFFER_PROGRAMMABLE_SAMPLE_LOCATIONS_ARB:
> +      if (!ctx->Extensions.ARB_sample_locations) {
> +         _mesa_error(ctx, GL_INVALID_ENUM, "%s(pname=0x%x)", func, pname);
> +         break;
> +      }
> +      *params = fb->ProgrammableSampleLocations;
> +      break;
> +   case GL_FRAMEBUFFER_SAMPLE_LOCATION_PIXEL_GRID_ARB:
> +      if (!ctx->Extensions.ARB_sample_locations) {
> +         _mesa_error(ctx, GL_INVALID_ENUM, "%s(pname=0x%x)", func, pname);
> +         break;
> +      }

I think we could use a "goto invalid_enum" error handler in this function.


> +      *params = fb->SampleLocationPixelGrid;
> +      break;
>      default:
>         _mesa_error(ctx, GL_INVALID_ENUM,
>                     "%s(pname=0x%x)", func, pname);
> @@ -1582,10 +1648,12 @@ _mesa_GetFramebufferParameteriv(GLenum target, GLenum pname, GLint *params)
>      GET_CURRENT_CONTEXT(ctx);
>      struct gl_framebuffer *fb;
>   
> -   if (!ctx->Extensions.ARB_framebuffer_no_attachments) {
> +   if (!ctx->Extensions.ARB_framebuffer_no_attachments &&
> +       !ctx->Extensions.ARB_sample_locations) {
>         _mesa_error(ctx, GL_INVALID_OPERATION,
>                     "glGetFramebufferParameteriv not supported "
> -                  "(ARB_framebuffer_no_attachments not implemented)");
> +                  "(both ARB_framebuffer_no_attachments and ARB_sample_locations"
> +                  " not implemented)");

Maybe instead of saying "not implemented", say "not available"?


>         return;
>      }
>   
> @@ -4224,15 +4292,21 @@ _mesa_NamedFramebufferParameteri(GLuint framebuffer, GLenum pname,
>      GET_CURRENT_CONTEXT(ctx);
>      struct gl_framebuffer *fb = NULL;
>   
> -   if (!ctx->Extensions.ARB_framebuffer_no_attachments) {
> +   if (!ctx->Extensions.ARB_framebuffer_no_attachments &&
> +       !ctx->Extensions.ARB_sample_locations) {
>         _mesa_error(ctx, GL_INVALID_OPERATION,
>                     "glNamedFramebufferParameteri("
> -                  "ARB_framebuffer_no_attachments not implemented)");
> +                  "both ARB_framebuffer_no_attachments and "
> +                  "ARB_sample_locations not implemented)");
>         return;
>      }
>   
> -   fb = _mesa_lookup_framebuffer_err(ctx, framebuffer,
> -                                     "glNamedFramebufferParameteri");
> +   if (framebuffer) {
> +      fb = _mesa_lookup_framebuffer_err(ctx, framebuffer,
> +                                        "glNamedFramebufferParameteri");
> +   } else {
> +      fb = ctx->WinSysDrawBuffer;
> +   }
>   
>      if (fb) {
>         framebuffer_parameteri(ctx, fb, pname, param,
> @@ -4251,7 +4325,8 @@ _mesa_GetNamedFramebufferParameteriv(GLuint framebuffer, GLenum pname,
>      if (!ctx->Extensions.ARB_framebuffer_no_attachments) {
>         _mesa_error(ctx, GL_INVALID_OPERATION,
>                     "glNamedFramebufferParameteriv("
> -                  "ARB_framebuffer_no_attachments not implemented)");
> +                  "both ARB_framebuffer_no_attachments and ARB_sample_locations"
> +                  " not implemented)");
>         return;
>      }
>   
> @@ -4595,3 +4670,115 @@ invalid_enum:
>                  "glDiscardFramebufferEXT(attachment %s)",
>                 _mesa_enum_to_string(attachments[i]));
>   }
> +
> +static void
> +sample_locations(struct gl_context *ctx, struct gl_framebuffer *fb,
> +                 GLuint start, GLsizei count, const GLfloat *v, bool no_error,
> +                 const char *name)
> +{
> +   GLsizei i;
> +
> +   if (!no_error && !ctx->Extensions.ARB_sample_locations) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "%s not supported "
> +                  "(ARB_sample_locations not implemented)", name);
> +      return;
> +   }
> +
> +   if (!no_error && start+count>MAX_SAMPLE_LOCATION_TABLE_SIZE) {

Spaces before/after + and >.


> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "%s(start+size > sample location table size)", name);
> +      return;
> +   }
> +
> +   if (!fb->SampleLocationTable) {
> +      size_t size = MAX_SAMPLE_LOCATION_TABLE_SIZE * 2 * sizeof(GLfloat);
> +      fb->SampleLocationTable = malloc(size);

I suspect that Coverity might warn about not checking the return value 
of malloc() for NULL.


> +      for (i = 0; i < MAX_SAMPLE_LOCATION_TABLE_SIZE * 2; i++)
> +         fb->SampleLocationTable[i] = 0.5f;
> +   }
> +
> +   for (i = 0; i < count * 2; i++) {
> +      if (v[i] != v[i]

We might have an is_nan() helper function for that somewhere.


  || v[i] < 0.0 || v[i] > 1.0) {
> +         static GLuint msg_id = 0;
> +         static const char* msg = "Invalid sample location specified";
> +         _mesa_debug_get_id(&msg_id);
> +
> +         fb->SampleLocationTable[start * 2 + i] = CLAMP(v[i], 0.0, 1.0);
> +         _mesa_log_msg(ctx, MESA_DEBUG_SOURCE_API, MESA_DEBUG_TYPE_UNDEFINED,
> +                       msg_id, MESA_DEBUG_SEVERITY_HIGH, strlen(msg), msg);
> +      } else
> +         fb->SampleLocationTable[start * 2 + i] = v[i];
> +   }
> +
> +   ctx->NewState |= _NEW_BUFFERS;
> +}
> +
> +void GLAPIENTRY
> +_mesa_FramebufferSampleLocationsfvARB(GLenum target, GLuint start,
> +                                      GLsizei count, const GLfloat *v)
> +{
> +   struct gl_framebuffer *fb;
> +
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   fb = get_framebuffer_target(ctx, target);
> +   if (!fb) {
> +      _mesa_error(ctx, GL_INVALID_ENUM,
> +         "glFramebufferSampleLocationsfvARB(target %s)",
> +         _mesa_enum_to_string(target));
> +      return;
> +   }
> +
> +   sample_locations(ctx, fb, start, count, v, false,
> +                    "glFramebufferSampleLocationsfvARB");
> +}
> +
> +void GLAPIENTRY
> +_mesa_NamedFramebufferSampleLocationsfvARB(GLuint framebuffer, GLuint start,
> +                                           GLsizei count, const GLfloat *v)
> +{
> +   struct gl_framebuffer *fb;
> +
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   if (framebuffer) {
> +      fb = _mesa_lookup_framebuffer_err(ctx, framebuffer,
> +                                        "glNamedFramebufferSampleLocationsfvARB");
> +      if (!fb)
> +         return;
> +   }
> +   else
> +      fb = ctx->WinSysDrawBuffer;
> +
> +   sample_locations(ctx, fb, start, count, v, false,
> +                    "glNamedFramebufferSampleLocationsfvARB");
> +}
> +
> +void GLAPIENTRY
> +_mesa_FramebufferSampleLocationsfvARB_no_error(GLenum target, GLuint start,
> +                                               GLsizei count, const GLfloat *v)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   sample_locations(ctx, get_framebuffer_target(ctx, target), start,
> +                    count, v, true, "glFramebufferSampleLocationsfvARB");
> +}
> +
> +void GLAPIENTRY
> +_mesa_NamedFramebufferSampleLocationsfvARB_no_error(GLuint framebuffer,
> +                                                    GLuint start, GLsizei count,
> +                                                    const GLfloat *v)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   sample_locations(ctx, _mesa_lookup_framebuffer(ctx, framebuffer), start,
> +                    count, v, true, "glNamedFramebufferSampleLocationsfvARB");
> +}
> +
> +void GLAPIENTRY
> +_mesa_EvaluateDepthValuesARB(void)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   if (ctx->Driver.EvaluateDepthValues)
> +      ctx->Driver.EvaluateDepthValues(ctx, ctx->DrawBuffer);
> +}
> diff --git a/src/mesa/main/fbobject.h b/src/mesa/main/fbobject.h
> index 31d743d3fb..5ba62d6cb1 100644
> --- a/src/mesa/main/fbobject.h
> +++ b/src/mesa/main/fbobject.h
> @@ -355,4 +355,24 @@ _mesa_FramebufferParameteri(GLenum target, GLenum pname, GLint param);
>   extern void GLAPIENTRY
>   _mesa_GetFramebufferParameteriv(GLenum target, GLenum pname, GLint *params);
>   
> +extern void GLAPIENTRY
> +_mesa_FramebufferSampleLocationsfvARB(GLenum target, GLuint start,
> +                                      GLsizei count, const GLfloat *v);
> +
> +extern void GLAPIENTRY
> +_mesa_NamedFramebufferSampleLocationsfvARB(GLuint framebuffer, GLuint start,
> +                                           GLsizei count, const GLfloat *v);
> +
> +extern void GLAPIENTRY
> +_mesa_FramebufferSampleLocationsfvARB_no_error(GLenum target, GLuint start,
> +                                               GLsizei count, const GLfloat *v);
> +
> +extern void GLAPIENTRY
> +_mesa_NamedFramebufferSampleLocationsfvARB_no_error(GLuint framebuffer,
> +                                                    GLuint start, GLsizei count,
> +                                                    const GLfloat *v);
> +
> +extern void GLAPIENTRY
> +_mesa_EvaluateDepthValuesARB(void);
> +
>   #endif /* FBOBJECT_H */
> diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
> index 249e775f8c..02f84dca75 100644
> --- a/src/mesa/main/framebuffer.c
> +++ b/src/mesa/main/framebuffer.c
> @@ -160,6 +160,10 @@ _mesa_initialize_window_framebuffer(struct gl_framebuffer *fb,
>      fb->_HasSNormOrFloatColorBuffer = visual->floatMode;
>      fb->_HasAttachments = true;
>   
> +   fb->SampleLocationTable = NULL;
> +   fb->ProgrammableSampleLocations = 0;
> +   fb->SampleLocationPixelGrid = 0;
> +
>      compute_depth_max(fb);
>   }
>   
> @@ -183,6 +187,9 @@ _mesa_initialize_user_framebuffer(struct gl_framebuffer *fb, GLuint name)
>      fb->_ColorDrawBufferIndexes[0] = BUFFER_COLOR0;
>      fb->ColorReadBuffer = GL_COLOR_ATTACHMENT0_EXT;
>      fb->_ColorReadBufferIndex = BUFFER_COLOR0;
> +   fb->SampleLocationTable = NULL;
> +   fb->ProgrammableSampleLocations = 0;
> +   fb->SampleLocationPixelGrid = 0;
>      fb->Delete = _mesa_destroy_framebuffer;
>      simple_mtx_init(&fb->Mutex, mtx_plain);
>   }
> @@ -229,6 +236,9 @@ _mesa_free_framebuffer_data(struct gl_framebuffer *fb)
>         assert(!att->Texture);
>         att->Type = GL_NONE;
>      }
> +
> +   free(fb->SampleLocationTable);
> +   fb->SampleLocationTable = NULL;
>   }
>   
>   
> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
> index 90ab7ca60f..2a43937ec3 100644
> --- a/src/mesa/main/get.c
> +++ b/src/mesa/main/get.c
> @@ -188,6 +188,7 @@ union value {
>      GLint value_int_4[4];
>      GLint64 value_int64;
>      GLenum value_enum;
> +   GLuint value_uint;
>   
>      /* Sigh, see GL_COMPRESSED_TEXTURE_FORMATS_ARB handling */
>      struct {
> @@ -506,6 +507,7 @@ EXTRA_EXT(OES_primitive_bounding_box);
>   EXTRA_EXT(ARB_compute_variable_group_size);
>   EXTRA_EXT(KHR_robustness);
>   EXTRA_EXT(ARB_sparse_buffer);
> +EXTRA_EXT(ARB_sample_locations);
>   
>   static const int
>   extra_ARB_color_buffer_float_or_glcore[] = {
> @@ -1187,6 +1189,36 @@ find_custom_value(struct gl_context *ctx, const struct value_desc *d, union valu
>            simple_mtx_unlock(&ctx->Shared->Mutex);
>         }
>         break;
> +   /* GL_ARB_sample_locations */
> +   case GL_SAMPLE_LOCATION_SUBPIXEL_BITS_ARB:
> +   case GL_SAMPLE_LOCATION_PIXEL_GRID_WIDTH_ARB:
> +   case GL_SAMPLE_LOCATION_PIXEL_GRID_HEIGHT_ARB:
> +      {
> +         GLuint bits=0, width=1, height=1;

spaces before/after =.


> +
> +         if (ctx->NewState & _NEW_BUFFERS)
> +            _mesa_update_state(ctx);
> +
> +         if (ctx->DrawBuffer->_Status != GL_FRAMEBUFFER_COMPLETE) {
> +            v->value_uint = 0;
> +            break;
> +         }
> +
> +         if (ctx->Driver.GetProgrammableSampleCaps)
> +            ctx->Driver.GetProgrammableSampleCaps(ctx, ctx->DrawBuffer,
> +                                                  &bits, &width, &height);
> +
> +         if (d->pname == GL_SAMPLE_LOCATION_PIXEL_GRID_WIDTH_ARB)
> +            v->value_uint = width;
> +         else if (d->pname == GL_SAMPLE_LOCATION_PIXEL_GRID_HEIGHT_ARB)
> +            v->value_uint = height;
> +         else
> +            v->value_uint = bits;
> +      }
> +      break;
> +   case GL_PROGRAMMABLE_SAMPLE_LOCATION_TABLE_SIZE_ARB:
> +      v->value_uint = MAX_SAMPLE_LOCATION_TABLE_SIZE;
> +      break;
>      }
>   }
>   
> diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py
> index f38a87df88..eba68a6321 100644
> --- a/src/mesa/main/get_hash_params.py
> +++ b/src/mesa/main/get_hash_params.py
> @@ -545,6 +545,12 @@ descriptor=[
>   
>     # GL_NUM_SHADING_LANGUAGE_VERSIONS
>     [ "NUM_SHADING_LANGUAGE_VERSIONS", "LOC_CUSTOM, TYPE_INT, 0, extra_version_43" ],
> +
> +  # GL_ARB_sample_locations
> +  [ "SAMPLE_LOCATION_SUBPIXEL_BITS_ARB", "LOC_CUSTOM, TYPE_UINT, 0, extra_ARB_sample_locations" ],
> +  [ "SAMPLE_LOCATION_PIXEL_GRID_WIDTH_ARB", "LOC_CUSTOM, TYPE_UINT, 0, extra_ARB_sample_locations" ],
> +  [ "SAMPLE_LOCATION_PIXEL_GRID_HEIGHT_ARB", "LOC_CUSTOM, TYPE_UINT, 0, extra_ARB_sample_locations" ],
> +  [ "PROGRAMMABLE_SAMPLE_LOCATION_TABLE_SIZE_ARB", "LOC_CUSTOM, TYPE_UINT, 0, extra_ARB_sample_locations" ],
>   ]},
>   
>   # Enums in OpenGL Core profile and ES 3.0
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index b65e7b2c3c..1feb3a658a 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3441,6 +3441,11 @@ struct gl_framebuffer
>      GLenum16 ColorDrawBuffer[MAX_DRAW_BUFFERS];
>      GLenum16 ColorReadBuffer;
>   
> +   /* GL_ARB_sample_locations */
> +   GLfloat* SampleLocationTable; /**< If NULL, no table has been specified */
> +   GLint ProgrammableSampleLocations;
> +   GLint SampleLocationPixelGrid;

I belive these fields should be GLboolean.


> +
>      /** Computed from ColorDraw/ReadBuffer above */
>      GLuint _NumColorDrawBuffers;
>      gl_buffer_index _ColorDrawBufferIndexes[MAX_DRAW_BUFFERS];
> @@ -4043,6 +4048,7 @@ struct gl_extensions
>      GLboolean ARB_post_depth_coverage;
>      GLboolean ARB_query_buffer_object;
>      GLboolean ARB_robust_buffer_access_behavior;
> +   GLboolean ARB_sample_locations;
>      GLboolean ARB_sample_shading;
>      GLboolean ARB_seamless_cube_map;
>      GLboolean ARB_shader_atomic_counter_ops;
> diff --git a/src/mesa/main/multisample.c b/src/mesa/main/multisample.c
> index dfe6a37142..08f6acb7e1 100644
> --- a/src/mesa/main/multisample.c
> +++ b/src/mesa/main/multisample.c
> @@ -101,6 +101,24 @@ _mesa_GetMultisamplefv(GLenum pname, GLuint index, GLfloat * val)
>         return;
>      }
>   
> +   case GL_PROGRAMMABLE_SAMPLE_LOCATION_ARB:
> +      if (!ctx->Extensions.ARB_sample_locations) {
> +         _mesa_error( ctx, GL_INVALID_ENUM, "glGetMultisamplefv(pname)" );
> +         return;
> +      }
> +
> +      if (index >= MAX_SAMPLE_LOCATION_TABLE_SIZE*2) {
> +         _mesa_error( ctx, GL_INVALID_VALUE, "glGetMultisamplefv(index)" );
> +         return;
> +      }
> +
> +      if (ctx->DrawBuffer->SampleLocationTable)
> +         *val = ctx->DrawBuffer->SampleLocationTable[index];
> +      else
> +         *val = 0.5f;
> +
> +      return;
> +
>      default:
>         _mesa_error( ctx, GL_INVALID_ENUM, "glGetMultisamplefv(pname)" );
>         return;
> diff --git a/src/mesa/main/tests/dispatch_sanity.cpp b/src/mesa/main/tests/dispatch_sanity.cpp
> index 83a4b04654..884f0c2e8c 100644
> --- a/src/mesa/main/tests/dispatch_sanity.cpp
> +++ b/src/mesa/main/tests/dispatch_sanity.cpp
> @@ -1026,6 +1026,11 @@ const struct function common_desktop_functions_possible[] = {
>      /* GL_EXT_shader_framebuffer_fetch_non_coherent */
>      { "glFramebufferFetchBarrierEXT", 20, -1 },
>   
> +   /* GL_ARB_sample_locations */
> +   { "glFramebufferSampleLocationsfvARB", 30, -1 },
> +   { "glNamedFramebufferSampleLocationsfvARB", 30, -1 },
> +   { "glEvaluateDepthValuesARB", 30, -1 },
> +
>      { NULL, 0, -1 }
>   };
>   
> @@ -2752,5 +2757,10 @@ const struct function gles31_functions_possible[] = {
>      { "glDepthRangeIndexedfOES", 31, -1 },
>      { "glGetFloati_vOES", 31, -1 },
>   
> +   /* GL_ARB_sample_locations */
> +   { "glFramebufferSampleLocationsfvARB", 31, -1 },
> +   { "glNamedFramebufferSampleLocationsfvARB", 31, -1 },
> +   { "glEvaluateDepthValuesARB", 31, -1 },
> +
>      { NULL, 0, -1 },
>    };
> 



More information about the mesa-dev mailing list