[Mesa-dev] [PATCH 2/2] meta: Handle bitmaps with alpha test enabled.

[Brian Paul]

On 11/01/2010 05:34 PM, Francisco Jerez wrote:
> Brian Paul <brianp at vmware.com> writes:
>
>> > Unless a fragment shader is active (very rare), all the fragments
>> > emitted for a bitmap should have the same RGBA color.  So the only two
>> > possible outcomes of applying the alpha test to glBitmap is either the
>> > bitmap is drawn normally (as if alpha test is disabled) or nothing is
>> > drawn at all.  I think we could test for this and either no-op the
>> > glBitmap or draw the bitmap with the current code as-is.
>> >
>> > What do you think?
>> >
> Yeah, that makes sense, fixed patch follows. Thank you for the review.
>
>> > -Brian
>
>
> 0001-meta-Handle-bitmaps-with-alpha-test-enabled.patch
>
> From ce79202f9bed892666539ef626ad49808c0428f9 Mon Sep 17 00:00:00 2001
> From: Francisco Jerez <currojerez at riseup.net>
> Date: Fri, 29 Oct 2010 21:20:34 +0200
> Subject: [PATCH] meta: Handle bitmaps with alpha test enabled.
>
> ---
>  src/mesa/drivers/common/meta.c |   56 +++++++++++++++++++++++++++++++++------
>  1 files changed, 47 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index 9615b52..8ebf5cf 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -104,6 +104,8 @@ struct save_state
>
>     /** META_ALPHA_TEST */
>     GLboolean AlphaEnabled;
> +   GLenum AlphaFunc;
> +   GLclampf AlphaRef;
>
>     /** META_BLEND */
>     GLbitfield BlendEnabled;
> @@ -328,6 +330,8 @@ _mesa_meta_begin(struct gl_context *ctx, GLbitfield state)
>
>     if (state & META_ALPHA_TEST) {
>        save->AlphaEnabled = ctx->Color.AlphaEnabled;
> +      save->AlphaFunc = ctx->Color.AlphaFunc;
> +      save->AlphaRef = ctx->Color.AlphaRef;
>        if (ctx->Color.AlphaEnabled)
>           _mesa_set_enable(ctx, GL_ALPHA_TEST, GL_FALSE);
>     }
> @@ -577,6 +581,7 @@ _mesa_meta_end(struct gl_context *ctx)
>     if (state & META_ALPHA_TEST) {
>        if (ctx->Color.AlphaEnabled != save->AlphaEnabled)
>           _mesa_set_enable(ctx, GL_ALPHA_TEST, save->AlphaEnabled);
> +      _mesa_AlphaFunc(save->AlphaFunc, save->AlphaRef);
>     }
>
>     if (state & META_BLEND) {
> @@ -1975,13 +1980,41 @@ _mesa_meta_DrawPixels(struct gl_context *ctx,
>     _mesa_meta_end(ctx);
>  }
>


I think this function should be renamed to better indicate what it 
does, plus we need a comment.  It's really just performing the alpha 
test using the current raster color's alpha (nothing specific to bitmaps).

> +static GLboolean
> +alpha_test_bitmap(struct gl_context *ctx)
> +{
> +   struct gl_colorbuffer_attrib *color = &ctx->Color;
> +   GLenum func = (color->AlphaEnabled ? color->AlphaFunc : GL_ALWAYS);
> +   GLubyte ref = FLOAT_TO_UBYTE(color->AlphaRef);
> +   GLubyte alpha = FLOAT_TO_UBYTE(ctx->Current.RasterColor[ACOMP]);

I don't think we should convert the alpha values to GLubyte here.  If 
the hardware does the alpha comparison with floats, the outcome could 
be different than comparing with GLubytes.  More on that below.


> +
> +   switch (func) {
> +      case GL_NEVER:
> +	 return GL_FALSE;
> +      case GL_LESS:
> +	 return alpha < ref;
> +      case GL_EQUAL:
> +	 return alpha == ref;
> +      case GL_LEQUAL:
> +	 return alpha <= ref;
> +      case GL_GREATER:
> +	 return alpha > ref;
> +      case GL_NOTEQUAL:
> +	 return alpha != ref;
> +      case GL_GEQUAL:
> +	 return alpha >= ref;
> +      case GL_ALWAYS:
> +	 return GL_TRUE;
> +      default:
> +	 assert(0);
> +	 return GL_FALSE;
> +   }
> +}
>
>  /**
> - * Do glBitmap with a alpha texture quad.  Use the alpha test to
> - * cull the 'off' bits.  If alpha test is already enabled, fall back
> - * to swrast (should be a rare case).
> - * A bitmap cache as in the gallium/mesa state tracker would
> - * improve performance a lot.
> + * Do glBitmap with a alpha texture quad.  Use the alpha test to cull
> + * the 'off' bits.  A bitmap cache as in the gallium/mesa state
> + * tracker would improve performance a lot.
>   */
>  void
>  _mesa_meta_Bitmap(struct gl_context *ctx,
> @@ -1993,6 +2026,7 @@ _mesa_meta_Bitmap(struct gl_context *ctx,
>     struct temp_texture *tex = get_bitmap_temp_texture(ctx);
>     const GLenum texIntFormat = GL_ALPHA;
>     const struct gl_pixelstore_attrib unpackSave = *unpack;
> +   GLubyte alpha = FLOAT_TO_UBYTE(ctx->Current.RasterColor[ACOMP]);
>     struct vertex {
>        GLfloat x, y, z, s, t, r, g, b, a;
>     };
> @@ -2004,7 +2038,7 @@ _mesa_meta_Bitmap(struct gl_context *ctx,
>      * Check if swrast fallback is needed.
>      */
>     if (ctx->_ImageTransferState ||
> -       ctx->Color.AlphaEnabled ||
> +       ctx->FragmentProgram._Enabled ||
>         ctx->Fog.Enabled ||
>         ctx->Texture._EnabledUnits ||
>         width > tex->MaxSize ||
> @@ -2013,6 +2047,9 @@ _mesa_meta_Bitmap(struct gl_context *ctx,
>        return;
>     }
>
> +   if (!alpha_test_bitmap(ctx))
> +      return;
> +
>     /* Most GL state applies to glBitmap (like blending, stencil, etc),
>      * but a there's a few things we need to override:
>      */
> @@ -2100,15 +2137,16 @@ _mesa_meta_Bitmap(struct gl_context *ctx,
>        return;
>     }
>
> -   bitmap8 = (GLubyte *) calloc(1, width * height);
> +   bitmap8 = (GLubyte *) malloc(width * height);
>     if (bitmap8) {
> +      memset(bitmap8, ~alpha, width * height);
>        _mesa_expand_bitmap(width, height, &unpackSave, bitmap1,
> -                          bitmap8, width, 0xff);
> +                          bitmap8, width, alpha);
>
>        _mesa_set_enable(ctx, tex->Target, GL_TRUE);
>
>        _mesa_set_enable(ctx, GL_ALPHA_TEST, GL_TRUE);
> -      _mesa_AlphaFunc(GL_GREATER, 0.0);
> +      _mesa_AlphaFunc(GL_EQUAL, ctx->Current.RasterColor[ACOMP]);

I think the code in this area should stay as-is.  If the ubyte value 
in the texture is converted to float, that might produce a value 
that's different from the alpha you're specifying to 
_mesa_AlphaFunc().  The test for equality could fail unexpectedly.

The original code is more robust in that regard.


>        setup_drawpix_texture(ctx, tex, newTex, texIntFormat, width, height,
>                              GL_ALPHA, GL_UNSIGNED_BYTE, bitmap8);
> -- 1.6.4.4

-Brian