[Mesa-dev] [PATCH] gles2: a stub implementation for GL_EXT_discard_framebuffer

[Chad Versace]

On 02/11/2013 12:19 AM, Tapani Pälli wrote:
> This patch implements a stub for GL_EXT_discard_framebuffer with
> required checks listed by the extension specification. This extension
> is required by GLBenchmark 2.5 when compiled with OpenGL ES 2.0
> as the rendering backend.
> 
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>  src/mapi/glapi/gen/es_EXT.xml                 | 13 +++++++++
>  src/mesa/drivers/dri/intel/intel_extensions.c |  1 +
>  src/mesa/main/dd.h                            |  4 ++-
>  src/mesa/main/extensions.c                    |  1 +
>  src/mesa/main/fbobject.c                      | 40 +++++++++++++++++++++++++++
>  src/mesa/main/fbobject.h                      |  4 +++
>  src/mesa/main/mtypes.h                        |  1 +
>  src/mesa/main/tests/dispatch_sanity.cpp       |  1 +
>  8 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mapi/glapi/gen/es_EXT.xml b/src/mapi/glapi/gen/es_EXT.xml
> index 0f6746d..4aa7b03 100644
> --- a/src/mapi/glapi/gen/es_EXT.xml
> +++ b/src/mapi/glapi/gen/es_EXT.xml
> @@ -663,6 +663,19 @@
>      <enum name="MAX_TEXTURE_LOD_BIAS_EXT"                 value="0x84FD"/>
>  </category>
>  
> +<!-- 64. GL_EXT_discard_framebuffer -->
> +
> +<category name="GL_EXT_discard_framebuffer" number="64">
> +    <function name="DiscardFramebufferEXT" es2="2.0" offset="assign" desktop="false">
                                              ^^^^^^^^^
According to the extension spec, it's a valid extension for ES1. Since
supporting the extension requires no real driver support, we should enable it in
ES1 too.
	

>  
>  <category name="GL_EXT_read_format_bgra" number="66">
> diff --git a/src/mesa/drivers/dri/intel/intel_extensions.c b/src/mesa/drivers/dri/intel/intel_extensions.c
> index bf5e2b5..a182139 100755
> --- a/src/mesa/drivers/dri/intel/intel_extensions.c
> +++ b/src/mesa/drivers/dri/intel/intel_extensions.c
> @@ -65,6 +65,7 @@ intelInitExtensions(struct gl_context *ctx)
>     ctx->Extensions.EXT_blend_equation_separate = true;
>     ctx->Extensions.EXT_blend_func_separate = true;
>     ctx->Extensions.EXT_blend_minmax = true;
> +   ctx->Extensions.EXT_discard_framebuffer = true;
>     ctx->Extensions.EXT_framebuffer_blit = true;
>     ctx->Extensions.EXT_framebuffer_object = true;
>     ctx->Extensions.EXT_fog_coord = true;


> diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
> index 04435e0..f7bd6f2 100644
> --- a/src/mesa/main/extensions.c
> +++ b/src/mesa/main/extensions.c
> @@ -168,6 +168,7 @@ static const struct extension extension_table[] = {
>     { "GL_EXT_blend_color",                         o(EXT_blend_color),                         GLL,            1995 },
>     { "GL_EXT_blend_equation_separate",             o(EXT_blend_equation_separate),             GL,             2003 },
>     { "GL_EXT_blend_func_separate",                 o(EXT_blend_func_separate),                 GLL,            1999 },
> +   { "GL_EXT_discard_framebuffer",                 o(EXT_discard_framebuffer),                             ES2, 2009 },
                                                                                                          ^^^^^^^^
Again, let's also enable on ES1.
                          

> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 257f839..9467bd0 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -3310,3 +3310,43 @@ _mesa_InvalidateFramebuffer(GLenum target, GLsizei numAttachments,
>                                    0, 0, MAX_VIEWPORT_WIDTH, MAX_VIEWPORT_HEIGHT,
>                                    "glInvalidateFramebuffer");
>  }
> +
> +void GLAPIENTRY
> +_mesa_DiscardFramebufferEXT(GLenum target, GLsizei numAttachments,
> +                            const GLenum *attachments)
> +{
> +   struct gl_framebuffer *fb;
> +   GLint i;
> +
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   if (target != GL_FRAMEBUFFER) {
> +      _mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", __func__);
> +      return;
> +   }
> +
> +   if (numAttachments < 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "%s(numAttachments < 0)", __func__);
> +      return;
> +   }
> +
> +   for(i = 0; i < numAttachments; i++) {
> +      switch (attachments[i]) {
> +         case GL_COLOR:
> +         case GL_DEPTH:
> +         case GL_STENCIL:
> +         case GL_COLOR_ATTACHMENT0:
> +         case GL_DEPTH_ATTACHMENT:
> +         case GL_STENCIL_ATTACHMENT:
> +         break;
> +         default:
> +            _mesa_error(ctx, GL_INVALID_ENUM, "%s(attachment)", __func__);
> +            return;
> +      }
> +   }

To echo Eric: this error behavior is incorrect.

> +
> + if (ctx->Driver.DiscardFramebuffer)
> +     ctx->Driver.DiscardFramebuffer(ctx, target, numAttachments, attachments);
> +} 

To ensure that the check for null works as expected, the pointer needs to
initialized to null in driverfuncs.c. The Doxygen for this struct says

 * Note: when new functions are added here, the drivers/common/driverfuncs.c
 * file should be updated too!!!
 */
struct dd_function_table {


Other than the error string formatting and the issues I mentioned, I
think the patch looks good.