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

[Chad Versace]

On 02/18/2013 07:20 AM, Brian Paul wrote:
> On 02/18/2013 12:12 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/common/driverfuncs.c   |  1 +
>>   src/mesa/main/dd.h                      |  4 ++-
>>   src/mesa/main/extensions.c              |  1 +
>>   src/mesa/main/fbobject.c                | 54 +++++++++++++++++++++++++++++++++
>>   src/mesa/main/fbobject.h                |  4 +++
>>   src/mesa/main/tests/dispatch_sanity.cpp |  2 ++
>>   7 files changed, 78 insertions(+), 1 deletion(-)
> 
> Just a couple very minor nits.
> 
> 
>> +void GLAPIENTRY
>> +_mesa_DiscardFramebufferEXT(GLenum target, GLsizei numAttachments,
>> +                            const GLenum *attachments)
>> +{
>> +   struct gl_framebuffer *fb;
>> +   GLint i;
>> +
>> +   GET_CURRENT_CONTEXT(ctx);
>> +
>> +   fb = get_framebuffer_target(ctx, target);
>> +   if (!fb) {
>> +      _mesa_error(ctx, GL_INVALID_ENUM,
>> +         "glDiscardFramebufferEXT(target %s)",
>> +         _mesa_lookup_enum_by_nr(target));
>> +      return;
>> +   }
>> +
>> +   if (numAttachments<  0) {
>> +      _mesa_error(ctx, GL_INVALID_VALUE,
>> +                  "glDiscardFramebufferEXT(numAttachments<  0)");
>> +      return;
>> +   }
>> +
>> +   for(i = 0; i<  numAttachments; i++) {
>> +
> 
> Please put a space in "for (" and you can remove the blank line between 'for'
> and 'switch'.
> 
> 
>> +      switch (attachments[i]) {
>> +      case GL_COLOR:
>> +      case GL_DEPTH:
>> +      case GL_STENCIL:
>> +         if (_mesa_is_user_fbo(fb))
>> +            goto invalid_enum;
>> +         break;
>> +      case GL_COLOR_ATTACHMENT0:
>> +      case GL_DEPTH_ATTACHMENT:
>> +      case GL_STENCIL_ATTACHMENT:
>> +         if (_mesa_is_winsys_fbo(fb))
>> +            goto invalid_enum;
>> +         break;
>> +      default:
>> +         goto invalid_enum;
>> +      }
>> +   }
>> +
>> +   if (ctx->Driver.DiscardFramebuffer)
>> +      ctx->Driver.DiscardFramebuffer(ctx, target, numAttachments, attachments);
>> +
>> +   return;
>> +
>> +invalid_enum:
>> +   _mesa_error(ctx, GL_INVALID_ENUM,
>> +               "glDiscardFramebufferEXT(attachment %s)",
>> +              _mesa_lookup_enum_by_nr(attachments[i]));
>> +}
> 
> Looks good otherwise.
> 
> Reviewed-by: Brian Paul <brianp at vmware.com>

Reviewed-and-tested-by: Chad Versace <chad.versace at linux.intel.com>

To avoid another lengthy turnaround due to the timezone difference, and so
that we have another checked box in tomorrow's Android meeting, I took the
liberty to make Brian's whitespace changes and commit the patch.