[Cogl] [PATCH] Don't include any GL header from the public GL headers

Robert Bragg robert at sixbynine.org
Fri Mar 23 11:22:11 PDT 2012


A surprisingly small patch to remove the need to implicitly pull in
the gl header, nice.

Since you mentioned that this breaks mutter currently due to it
manually needing to create TEXTURE_RECTANGLE textures, it sounds like
we should hold off landing this until we have a patch for mutter to
make it use the cogl_texture_rectangle api instead. Other than that it
looks good to land to me:

Reviewed-by: Robert Bragg <robert at linux.intel.com>

On Fri, Mar 23, 2012 at 6:05 PM, Neil Roberts <neil at linux.intel.com> wrote:
> I forgot to add the test-no-gl-header.c file to the patch. Here it is
> again. Sorry!
>
> --- >8 ---
>
> This splits the GL header inclusion from cogl-defines.h into a
> separate headear called cogl-gl-header.h which we will only include
> internally. That way we don't leak GL declarations out of our public
> headers. The texture functions that were using GLenum and GLuint in
> the public header have now changed to just use unsigned int. Note
> however that if an EGL winsys is enabled then it will still publicly
> include an EGL header. This is a bit more awkward to fix because we
> have public API which returns an EGLDisplay and we can't determine
> what type that is.
>
> There is also a conformance test which just verifies that no GL header
> has been included while compiling. The test isn't added to
> test-conform-main because it doesn't actually test anything at
> runtime.
> ---
>  cogl/Makefile.am                  |    6 ++--
>  cogl/cogl-buffer-private.h        |    1 +
>  cogl/cogl-context-private.h       |    1 +
>  cogl/cogl-defines.h.in            |    4 ---
>  cogl/cogl-gl-header.h.in          |   39 +++++++++++++++++++++++++++++++++++++
>  cogl/cogl-internal.h              |    1 +
>  cogl/cogl-shader-private.h        |    1 +
>  cogl/cogl-texture.h               |   16 +++++++-------
>  cogl/tesselator/tesselator.h      |    1 +
>  configure.ac                      |    1 +
>  tests/conform/Makefile.am         |    1 +
>  tests/conform/test-no-gl-header.c |   17 ++++++++++++++++
>  12 files changed, 74 insertions(+), 15 deletions(-)
>  create mode 100644 cogl/cogl-gl-header.h.in
>  create mode 100644 tests/conform/test-no-gl-header.c
>
> diff --git a/cogl/Makefile.am b/cogl/Makefile.am
> index 69cb5df..55b93f8 100644
> --- a/cogl/Makefile.am
> +++ b/cogl/Makefile.am
> @@ -34,9 +34,9 @@ AM_CPPFLAGS = \
>
>  AM_CFLAGS = $(COGL_DEP_CFLAGS) $(COGL_EXTRA_CFLAGS) $(MAINTAINER_CFLAGS)
>
> -BUILT_SOURCES += cogl-defines.h
> -DISTCLEANFILES += cogl-defines.h
> -EXTRA_DIST += cogl-defines.h.in
> +BUILT_SOURCES += cogl-defines.h cogl-gl-header.h
> +DISTCLEANFILES += cogl-defines.h cogl-gl-header.h
> +EXTRA_DIST += cogl-defines.h.in cogl-gl-header.h.in
>
>  # Note: The cogl-1.0/cogl-gl-1.0 files are essentially for
>  # compatability only.  I'm not really sure who could possibly be using
> diff --git a/cogl/cogl-buffer-private.h b/cogl/cogl-buffer-private.h
> index 62bcf45..0457af0 100644
> --- a/cogl/cogl-buffer-private.h
> +++ b/cogl/cogl-buffer-private.h
> @@ -33,6 +33,7 @@
>  #include "cogl-object-private.h"
>  #include "cogl-buffer.h"
>  #include "cogl-context.h"
> +#include "cogl-gl-header.h"
>
>  G_BEGIN_DECLS
>
> diff --git a/cogl/cogl-context-private.h b/cogl/cogl-context-private.h
> index a79672f..8b1ab2b 100644
> --- a/cogl/cogl-context-private.h
> +++ b/cogl/cogl-context-private.h
> @@ -47,6 +47,7 @@
>  #include "cogl-texture-2d.h"
>  #include "cogl-texture-3d.h"
>  #include "cogl-texture-rectangle.h"
> +#include "cogl-gl-header.h"
>
>  typedef struct
>  {
> diff --git a/cogl/cogl-defines.h.in b/cogl/cogl-defines.h.in
> index 3d4b17d..ccdca06 100644
> --- a/cogl/cogl-defines.h.in
> +++ b/cogl/cogl-defines.h.in
> @@ -29,7 +29,6 @@
>  #define __COGL_DEFINES_H__
>
>  #include <glib.h>
> - at COGL_GL_HEADER_INCLUDES@
>
>  G_BEGIN_DECLS
>
> @@ -41,9 +40,6 @@ G_BEGIN_DECLS
>  #define NativeWindowType EGLNativeWindowType
>  #endif
>
> -#ifndef GL_OES_EGL_image
> -#define GLeglImageOES void *
> -#endif
>  G_END_DECLS
>
>  #endif
> diff --git a/cogl/cogl-gl-header.h.in b/cogl/cogl-gl-header.h.in
> new file mode 100644
> index 0000000..30bc837
> --- /dev/null
> +++ b/cogl/cogl-gl-header.h.in
> @@ -0,0 +1,39 @@
> +/*
> + * Cogl
> + *
> + * An object oriented GL/GLES Abstraction/Utility Layer
> + *
> + * Copyright (C) 2012 Intel Corporation.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see <http://www.gnu.org/licenses/>.
> + *
> + *
> + */
> +
> +#if !defined(CLUTTER_COMPILATION)
> +#error "cogl-gl-header.h should only be included when compiling Cogl"
> +#endif
> +
> +#ifndef __COGL_GL_HEADER_H__
> +#define __COGL_GL_HEADER_H__
> +
> +#include "cogl-defines.h"
> +
> + at COGL_GL_HEADER_INCLUDES@
> +
> +#ifndef GL_OES_EGL_image
> +#define GLeglImageOES void *
> +#endif
> +
> +#endif /* __COGL_GL_HEADER_H__ */
> diff --git a/cogl/cogl-internal.h b/cogl/cogl-internal.h
> index 284fc97..bd13547 100644
> --- a/cogl/cogl-internal.h
> +++ b/cogl/cogl-internal.h
> @@ -25,6 +25,7 @@
>  #define __COGL_INTERNAL_H
>
>  #include "cogl-bitmask.h"
> +#include "cogl-gl-header.h"
>
>  #ifdef COGL_HAS_XLIB_SUPPORT
>  #include <X11/Xutil.h>
> diff --git a/cogl/cogl-shader-private.h b/cogl/cogl-shader-private.h
> index 6c05b3f..06575e6 100644
> --- a/cogl/cogl-shader-private.h
> +++ b/cogl/cogl-shader-private.h
> @@ -26,6 +26,7 @@
>
>  #include "cogl-handle.h"
>  #include "cogl-shader.h"
> +#include "cogl-gl-header.h"
>
>  typedef struct _CoglShader CoglShader;
>
> diff --git a/cogl/cogl-texture.h b/cogl/cogl-texture.h
> index 4eeffb5..c73ea8a 100644
> --- a/cogl/cogl-texture.h
> +++ b/cogl/cogl-texture.h
> @@ -207,12 +207,12 @@ cogl_texture_new_from_data (unsigned int      width,
>  * Since: 0.8
>  */
>  CoglTexture *
> -cogl_texture_new_from_foreign (GLuint          gl_handle,
> -                               GLenum          gl_target,
> -                               GLuint          width,
> -                               GLuint          height,
> -                               GLuint          x_pot_waste,
> -                               GLuint          y_pot_waste,
> +cogl_texture_new_from_foreign (unsigned int gl_handle,
> +                               unsigned int gl_target,
> +                               unsigned int width,
> +                               unsigned int height,
> +                               unsigned int x_pot_waste,
> +                               unsigned int y_pot_waste,
>                                CoglPixelFormat format);
>
>  /**
> @@ -353,8 +353,8 @@ cogl_texture_is_sliced (CoglTexture *texture);
>  */
>  gboolean
>  cogl_texture_get_gl_texture (CoglTexture *texture,
> -                             GLuint      *out_gl_handle,
> -                             GLenum      *out_gl_target);
> +                             unsigned int *out_gl_handle,
> +                             unsigned int *out_gl_target);
>
>  /**
>  * cogl_texture_get_data:
> diff --git a/cogl/tesselator/tesselator.h b/cogl/tesselator/tesselator.h
> index 69a6ece..5b651be 100644
> --- a/cogl/tesselator/tesselator.h
> +++ b/cogl/tesselator/tesselator.h
> @@ -35,6 +35,7 @@
>  /* This just includes the defines needed by the tesselator code */
>
>  #include "cogl/cogl-defines.h"
> +#include "cogl/cogl-gl-header.h"
>
>  typedef struct GLUtesselator GLUtesselator;
>
> diff --git a/configure.ac b/configure.ac
> index 67fc4cf..55fa37c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1120,6 +1120,7 @@ cogl/Makefile
>  cogl/cogl-1.0.pc
>  cogl/cogl-2.0-experimental.pc
>  cogl/cogl-defines.h
> +cogl/cogl-gl-header.h
>  cogl/cogl.rc
>  cogl-pango/Makefile
>  cogl-pango/cogl-pango-1.0.pc
> diff --git a/tests/conform/Makefile.am b/tests/conform/Makefile.am
> index e6ffc7b..5a82b46 100644
> --- a/tests/conform/Makefile.am
> +++ b/tests/conform/Makefile.am
> @@ -52,6 +52,7 @@ test_sources = \
>        test-write-texture-formats.c \
>        test-point-size.c \
>        test-point-sprite.c \
> +       test-no-gl-header.c \
>        $(NULL)
>
>  test_conformance_SOURCES = $(common_sources) $(test_sources)
> diff --git a/tests/conform/test-no-gl-header.c b/tests/conform/test-no-gl-header.c
> new file mode 100644
> index 0000000..49dae6c
> --- /dev/null
> +++ b/tests/conform/test-no-gl-header.c
> @@ -0,0 +1,17 @@
> +#include <cogl/cogl.h>
> +
> +#include "test-utils.h"
> +
> +/* If you just include cogl/cogl.h, you shouldn't end up including any
> +   GL headers */
> +#ifdef GL_TRUE
> +#error "Including cogl.h shouldn't be including any GL headers"
> +#endif
> +
> +void
> +test_no_gl_header (void)
> +{
> +  if (cogl_test_verbose ())
> +    g_print ("OK\n");
> +}
> +
> --
> 1.7.3.16.g9464b
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl


More information about the Cogl mailing list