On 09/28/2012 02:20 AM, Oliver McFadden wrote:
> On Fri, Sep 28, 2012 at 01:12:55AM -0700, Kenneth Graunke wrote:
>> On 09/27/2012 01:56 AM, Oliver McFadden wrote:
>>> Signed-off-by: Oliver McFadden <oliver.mcfad...@linux.intel.com>
>>> ---
>>> v4: Off-by-one on a couple of 'if (ctx->Mesa_DXTn)' lines, which could
>>> cause a
>>> crash.
>>>
>>> src/glx/glxextensions.h | 3 ++-
>>> src/mapi/glapi/gen/es_EXT.xml | 6 ++++++
>>> src/mesa/drivers/dri/intel/intel_extensions.c | 1 +
>>> src/mesa/main/APIspec.xml | 3 +++
>>> src/mesa/main/extensions.c | 3 +++
>>> src/mesa/main/glformats.c | 6 ++++--
>>> src/mesa/main/glheader.h | 11 +++++++++++
>>> src/mesa/main/mtypes.h | 1 +
>>> src/mesa/main/texformat.c | 9 ++++++---
>>> src/mesa/main/teximage.c | 10 ++++++++++
>>> 10 files changed, 47 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/glx/glxextensions.h b/src/glx/glxextensions.h
>>> index 90c27a7..9e072e4 100644
>>> --- a/src/glx/glxextensions.h
>>> +++ b/src/glx/glxextensions.h
>>> @@ -67,7 +67,8 @@ enum
>>>
>>> enum
>>> {
>>> - GL_ARB_depth_texture_bit = 0,
>>> + GL_ANGLE_texture_compression_dxt_bit = 0,
>>> + GL_ARB_depth_texture_bit,
>>> GL_ARB_draw_buffers_bit,
>>> GL_ARB_fragment_program_bit,
>>> GL_ARB_fragment_program_shadow_bit,
>>
>> You shouldn't need to change GLX code to add a purely client-side
>> extension. I believe you can safely drop this hunk (and please do).
>
> I need ANGLE_texture_compression_dxt somewhere in the extension table,
> see src/mesa/main/extensions.c. Unfortunately it's not possible to
> unconditionally enable this extension. Brian complained about the
> ordering of names (alphabetical order) which is why I've added ANGLE at
> the top of the enum...
Yeah, but you already defined that bit (correctly) in struct
gl_extensions in mtypes.h. This one is just superfluous, as far as I
can tell.
[snip]
>>> diff --git a/src/mesa/main/glheader.h b/src/mesa/main/glheader.h
>>> index e93ca30..33cda02 100644
>>> --- a/src/mesa/main/glheader.h
>>> +++ b/src/mesa/main/glheader.h
>>> @@ -59,6 +59,17 @@ extern "C" {
>>> #endif
>>>
>>>
>>> +/* GL_ANGLE_texture_compression_dxt3 */
>>> +#ifndef GL_ANGLE_texture_compression_dxt3
>>> +#define GL_COMPRESSED_RGBA_S3TC_DXT3_ANGLE 0x83F2
>>> +#endif
>>> +
>>> +/* GL_ANGLE_texture_compression_dxt5 */
>>> +#ifndef GL_ANGLE_texture_compression_dxt5
>>> +#define GL_COMPRESSED_RGBA_S3TC_DXT5_ANGLE 0x83F3
>>> +#endif
>>
>> You can actually drop this hunk if you change the teximage.c code to use
>> the _EXT extension instead of the _ANGLE extension. The enums are
>> identical (just as the formats are identical). I'd prefer that.
>
> Hmm, okay. I'd like to leave a GL_COMPRESSED_RGBA_S3TC_DXT[35]_ANGLE
> comment there for grep, but it's not a show-stopper though...
I guess I don't feel too strongly about it one way or another.
>> Otherwise, this looks good! With those changes,
>> Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
>
> I will also wait for Brian's comments on your suggestions and my replies
> before rolling another patch. I'll be happy to see this one done and
> pushed. :-)
Sounds good.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
