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

[Brian Paul]

On 11/02/2010 12:25 PM, Francisco Jerez wrote:
> Brian Paul<brianp at vmware.com>  writes:
>
>> On 11/01/2010 08:02 PM, Francisco Jerez wrote:
>>> Brian Paul<brianp at vmware.com>   writes:
>>>
>>>> [...]
>>>>>
>>>>> -   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.
>>>
>>> What if the current raster alpha value is non-0xff? I changed the
>>> original code because that case was misrendered. I understand your
>>> concern though.
>>
>> The original code checked if alpha test was enabled and fell back to
>> swrast so the current alpha ref was irrelevant.  In the meta code
>> which draws the bitmap with a textured quad, the current raster alpha
>> color is also irrelevant since we're getting the fragment alpha value
>> from the texture map.
>>
> Yeah, and the alpha value taken from the texture map was incorrect, it
> was constant 1.0, you just had to enable alpha blending to notice there
> was something wrong.

That was a bug then.  We should have checked for blending.


>>> According to the spec both reference and fragment alpha values are
>>> converted to the nearest fixed-point value (of unspecified but same
>>> precision) before comparison, so AFAICT this change should cause no
>>> problems as long as we observe the same rules in the raster color to
>>> texture value conversion.
>>
>> That part of the spec doesn't apply to floating point color buffers,
>> BTW.  Also, when converting to floats to GLubyte (or GLushorts, etc)
>
> Ah, that's true. Good point.
>
>> or vice versa we have to pay attention to whether doing it with the
>> CPU will result in the same thing as the GPU.
>>
>> As I wrote before, if the texture's ubyte alpha value is converted to
>> float by the GPU and it compares it to the alpha ref value using
>> floating point, testing for equality is tricky.  An inequality is
>> safer.
>>
>
> We have several options.
>
> - Fall back to swrast in the floating point renderbuffer case. In other
>    cases the conversion to fixed point guarantees that EQUAL will behave
>    as expected. Simple and good enough ATM because all the users of
>    _mesa_meta_Bitmap don't support floating point color buffers (no-one
>    does). Some of them might at some point though.
>
> - Choose a background alpha value different to the current raster color
>    that is known to be represented exactly all the way down the pipeline
>    (e.g. 0.0/1.0), then use NOTEQUAL to cull them out.
>
> - Stick to GREATER/LESS for maximum robustness/paranoia.
>
> I'm fine with any of them. What do you suggest?

Either of the later two.  Thanks.

-Brian