On Fri, Sep 28, 2012 at 02:29:12AM -0700, Kenneth Graunke wrote:
> 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.
Oh, yes. You're right; I haven't had my coffee this morning (nobody
check the actual time in EEST! ;-))
>
> [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.
OK.
>
> >> 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.
>
--
Oliver McFadden.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
