[Mesa-dev] [PATCH 2/5] mesa: implement EXT_texture_shared_exponent

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/26/2011 05:21 PM, Marek Olšák wrote:
> [snip]
> 
>     > +static INLINE float rgb9e5_MaxOf3(float x, float y, float z)
>     > +{
>     > +   if (x > y) {
>     > +      return MAX2(x, z);
>     > +   } else {
>     > +      return MAX2(y, z);
>     > +   }
>     > +}
> 
>     On 04/26/2011 03:57 AM, Marek Olšák wrote:
>     This could probably be a MAX3 macro defined in the same place as the
>     existing MAX2 macro.  Also, why did you implement as you did instead of
> 
>            MAX2(MAX2(x,y),z)
> 
>     Just curious...
> 
> 
> It might be faster without common subexpression elimination. Not sure
> what gcc can do. I haven't given it much thought.

Okay, that's fair.

>     > diff --git a/src/mesa/main/texformat.c b/src/mesa/main/texformat.c
>     > index cbebec9..3520f24 100644
>     > --- a/src/mesa/main/texformat.c
>     > +++ b/src/mesa/main/texformat.c
>     > @@ -382,6 +382,16 @@ _mesa_choose_tex_format( struct gl_context
>     *ctx, GLint internalFormat,
>     >        }
>     >     }
>     >
>     > +   if (ctx->Extensions.EXT_texture_shared_exponent) {
>     > +      switch (internalFormat) {
>     > +         case GL_RGB9_E5:
>     > +            RETURN_IF_SUPPORTED(MESA_FORMAT_RGB9_E5_FLOAT);
> 
>     I could envision the extension being enabled but the format not being
>     supported by the hardware.  This would be like the ARB_half_float_pixel
>     code.  In this case, the fallbacks should be:
> 
>       MESA_FORMAT_RGB_FLOAT16
>       MESA_FORMAT_RGBA_FLOAT16
>       MESA_FORMAT_RGB_FLOAT32
>       MESA_FORMAT_RGBA_FLOAT32
> 
>     The real question is if we ever want to enable any of these "extra
>     format" extensions without requireing real hardware support.  If we
>     don't, then these chases should assert that the format is supported.
> 
> I don't think it's useful to fake the format with some other one. It's
> like RGTC: if hardware can't do it, let's disable it. RGB9_E5 is
> unsigned, and faking the "unsignedness" in the texstore path for RGBA32F
> would be unfun. Is there actually otherwise-GL3-capable hw which doesn't
> support it?

Ah, that makes sense.  I can agree with that.  In that case, I'd change
the RETURN_IF_SUPPORTED to:

	assert(ctx->TextureFormatSupported[MESA_FORMAT_RGB9_E5_FLOAT]);
	return MESA_FORMAT_RGB9_E5_FLOAT;
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk23ZDwACgkQX1gOwKyEAw85ywCfaHBqrIhZsvYxpQppfiZrahwe
SuQAmwW1Xu+k8vuoc3x+vy7OeeRxL1Ki
=oG/Z
-----END PGP SIGNATURE-----