[Mesa-dev] [PATCH 1/4] i965: Also examine _BaseFormat when deciding to perform xRGB_alpha fixups

[Eric Anholt]

Carl Worth <cworth at cworth.org> writes:

> The renderbuffer's Format field may have an alpha channel even when the
> underlying _BaseFormat does not. This can happen when mesa chooses to use
> RGBA16 for an RGB16 format, for example.

>
> So look at both fields when deciding whether to fixup the blend factors.
>
> This test improves the results of at least the following piglit tests:
>
> 	EXT_frambebuffer_object/fbo-blending-formats
>         	{GL_RGB10, GL_RGB12, GL_RGB16}
> 	EXT_texture_snorm/fbo-blending-formats
> 		{GL_RGB16_SNORM, GLRGB8_SNORM, GL_RGB_SNORM}
>
> But none of these actually change from FAIL to PASS yet. The R, G, and B probe
> values are fixed with this commit, but the tests still fail because the alpha
> values are still wrong.
> ---
>
> Note: The test results mentioned here, (and in the following patches), expect
> the fixes I recently submitted to the fbo-* tests in piglit. That series
> consists of 5 patches starting with:
>
> 	[PATCH 1/3] fbo-blending-formats: Enable testing for channels with 0
> 	bits of data.
>
> Without that series, most of these tests report a buggy PASS even without the
> current fixes, (even though the implementation is providing incorrect results
> in many cases).
>
>  src/mesa/drivers/dri/i965/gen6_cc.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen6_cc.c b/src/mesa/drivers/dri/i965/gen6_cc.c
> index fcd1794..f1ba9c2 100644
> --- a/src/mesa/drivers/dri/i965/gen6_cc.c
> +++ b/src/mesa/drivers/dri/i965/gen6_cc.c
> @@ -32,6 +32,7 @@
>  #include "intel_batchbuffer.h"
>  #include "main/macros.h"
>  #include "main/enums.h"
> +#include "main/glformats.h"
>  
>  static void
>  gen6_upload_blend_state(struct brw_context *brw)
> @@ -125,7 +126,8 @@ gen6_upload_blend_state(struct brw_context *brw)
>            * not read the alpha channel, but will instead use the correct
>            * implicit value for alpha.
>            */
> -         if (_mesa_get_format_bits(rb->Format, GL_ALPHA_BITS) == 0)
> +         if (_mesa_get_format_bits(rb->Format, GL_ALPHA_BITS) == 0 ||
> +             !_mesa_base_format_has_channel(rb->_BaseFormat, GL_TEXTURE_ALPHA_TYPE))

I think the _mesa_get_format_bits could just be dropped -- we shouldn't
have ALPHA_BITS==0 unless the baseformat has no alpha.

Patches 3 and 4 are:

Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130123/bc51c0ce/attachment.pgp>