[Piglit] [PATCH 6/8] util: fix piglit-util for GLES1

Ian Romanick idr at freedesktop.org
Sun Sep 4 17:26:05 PDT 2011


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

On 09/03/2011 09:29 AM, Chia-I Wu wrote:
> Add piglit-util-gles1.c.  Note that it includes piglit-util-gles2.c.  It
> might be better to rename piglit-util-gles2.c to piglit-util-gles.c and
> let GLES1 and GLES2 share it.  Suggestions?

That would be much better.  Having one .c file include another .c file
is unpleasant.  Since all but 2 functions are shared, putting it all in
piglit-util-gles.c should be okay.

Aren't piglit_gen_ortho_projection and piglit_ortho_projection also in
piglit-util.c?  Maybe those two functions should just get broken out to
piglit-ortho.c that gets used for both GLES1 and desktop GL.

> ---
>  tests/util/piglit-util-gles1.c |   35 +++++++++++++++++++++++++++++++++++
>  tests/util/piglit-util-gles2.c |   21 +++++++++++++++++++++
>  tests/util/piglit-util.c       |   17 +++++++++++++++--
>  3 files changed, 71 insertions(+), 2 deletions(-)
>  create mode 100644 tests/util/piglit-util-gles1.c
> 
> diff --git a/tests/util/piglit-util-gles1.c b/tests/util/piglit-util-gles1.c
> new file mode 100644
> index 0000000..e7d0034
> --- /dev/null
> +++ b/tests/util/piglit-util-gles1.c
> @@ -0,0 +1,35 @@
> +/* include everything in GLES2 */
> +#include "piglit-util-gles2.c"
> +
> +/**
> + * Convenience function to configure an abitrary orthogonal projection matrix
> + */
> +void
> +piglit_gen_ortho_projection(double left, double right, double bottom,
> +			    double top, double near_val, double far_val,
> +			    GLboolean push)
> +{
> +        glMatrixMode(GL_PROJECTION);
> +        glLoadIdentity();
> +	if (push)
> +		glPushMatrix();
> +        glOrthof(left, right, bottom, top, near_val, far_val);
> +
> +        glMatrixMode(GL_MODELVIEW);
> +	if (push)
> +		glPushMatrix();
> +        glLoadIdentity();
> +}

Mixed tabs and spaces makes in indentation look really weird here.

> +
> +
> +/**
> + * Convenience function to configure projection matrix for window coordinates
> + */
> +void
> +piglit_ortho_projection(int w, int h, GLboolean push)
> +{
> +        /* Set up projection matrix so we can just draw using window
> +         * coordinates.
> +         */
> +	piglit_gen_ortho_projection(0, w, 0, h, -1, 1, push);
> +}
> diff --git a/tests/util/piglit-util-gles2.c b/tests/util/piglit-util-gles2.c
> index 3f7addd..7888f6b 100644
> --- a/tests/util/piglit-util-gles2.c
> +++ b/tests/util/piglit-util-gles2.c
> @@ -188,6 +188,24 @@ piglit_escape_exit_key(unsigned char key, int x, int y)
>  static void
>  draw_arrays(const GLvoid *verts, const GLvoid *tex)
>  {
> +#if defined(USE_OPENGL_ES1)
> +	if (verts) {
> +		glVertexPointer(4, GL_FLOAT, 0, verts);
> +		glEnableClientState(GL_VERTEX_ARRAY);
> +	}
> +
> +	if (tex) {
> +		glTexCoordPointer(2, GL_FLOAT, 0, tex);
> +		glEnableClientState(GL_TEXTURE_COORD_ARRAY);
> +	}
> +
> +	glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> +
> +	if (verts)
> +		glDisableClientState(GL_VERTEX_ARRAY);
> +	if (tex)
> +		glDisableClientState(GL_TEXTURE_COORD_ARRAY);
> +#elif defined(USE_OPENGL_ES2)
>  	if (verts) {
>  		glVertexAttribPointer(PIGLIT_ATTRIB_POS, 4, GL_FLOAT, GL_FALSE, 0, verts);
>  		glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);
> @@ -204,6 +222,9 @@ draw_arrays(const GLvoid *verts, const GLvoid *tex)
>  		glDisableVertexAttribArray(PIGLIT_ATTRIB_POS);
>  	if (tex)
>  		glDisableVertexAttribArray(PIGLIT_ATTRIB_TEX);
> +#else
> +#	error
> +#endif
>  }
>  
>  /**
> diff --git a/tests/util/piglit-util.c b/tests/util/piglit-util.c
> index 248d46a..c050b57 100644
> --- a/tests/util/piglit-util.c
> +++ b/tests/util/piglit-util.c
> @@ -48,6 +48,8 @@ void piglit_glutInit(int argc, char **argv)
>  
>  #if defined USE_EGLUT && defined USE_OPENGL
>  	glutInitAPIMask(GLUT_OPENGL_BIT);
> +#elif defined USE_EGLUT && defined USE_OPENGL_ES1
> +	glutInitAPIMask(GLUT_OPENGL_ES1_BIT);
>  #elif defined USE_EGLUT && defined USE_OPENGL_ES2
>  	glutInitAPIMask(GLUT_OPENGL_ES2_BIT);
>  #elif defined USE_EGLUT
> @@ -129,15 +131,17 @@ const char* piglit_get_gl_error_name(GLenum error)
>  #define CASE(x) case x: return #x; 
>      switch (error) {
>      CASE(GL_INVALID_ENUM)
> -    CASE(GL_INVALID_FRAMEBUFFER_OPERATION)
>      CASE(GL_INVALID_OPERATION)
>      CASE(GL_INVALID_VALUE)
>      CASE(GL_NO_ERROR)
>      CASE(GL_OUT_OF_MEMORY)
> -#if defined(USE_OPENGL)
> +#if defined(USE_OPENGL) || defined(USE_OPENGL_ES1)
>      CASE(GL_STACK_OVERFLOW)
>      CASE(GL_STACK_UNDERFLOW)
>  #endif
> +#if defined(USE_OPENGL) || defined(USE_OPENGL_ES2)
> +    CASE(GL_INVALID_FRAMEBUFFER_OPERATION)
> +#endif

It would be better to guard these with '#if
defined(GL_ERROR_BEING_CHECKED'.  So,

#if defined(GL_INVALID_FRAMEBUFFER_OPERATION)
   CASE(GL_INVALID_FRAMEBUFFER_OPERATION)
#endif

>      default:
>          return "(unrecognized error)";
>      }
> @@ -225,12 +229,21 @@ const char *cube_face_names[6] = {
>  };
>  
>  const GLenum cube_face_targets[6] = {
> +#ifdef USE_OPENGL_ES1
> +	GL_TEXTURE_CUBE_MAP_POSITIVE_X_OES,
> +	GL_TEXTURE_CUBE_MAP_POSITIVE_Y_OES,
> +	GL_TEXTURE_CUBE_MAP_POSITIVE_Z_OES,
> +	GL_TEXTURE_CUBE_MAP_NEGATIVE_X_OES,
> +	GL_TEXTURE_CUBE_MAP_NEGATIVE_Y_OES,
> +	GL_TEXTURE_CUBE_MAP_NEGATIVE_Z_OES,
> +#else

I don't know how other people feel, but I *hate* this.  It really sucks
that we have to work around having 47 names for the same thing. :( I'd
much rather see this wrapped in piglit-util.h (or somewhere!) like:

#ifndef GL_TEXTURE_CUBE_MAP_POSITIVE_X
#define GL_TEXTURE_CUBE_MAP_POSITIVE_X 0x8515
...
#endif

This lets us hide all of the ugliness in one place.  It should also help
make it easier to port some of the existing cubemap tests to GLES1.  It
works because the values of GL_TEXTURE_CUBE_MAP_POSITIVE_X_EXT,
GL_TEXTURE_CUBE_MAP_POSITIVE_X_ARB, GL_TEXTURE_CUBE_MAP_POSITIVE_X_OES,
and GL_TEXTURE_CUBE_MAP_POSITIVE_X are all the same.

>  	GL_TEXTURE_CUBE_MAP_POSITIVE_X,
>  	GL_TEXTURE_CUBE_MAP_POSITIVE_Y,
>  	GL_TEXTURE_CUBE_MAP_POSITIVE_Z,
>  	GL_TEXTURE_CUBE_MAP_NEGATIVE_X,
>  	GL_TEXTURE_CUBE_MAP_NEGATIVE_Y,
>  	GL_TEXTURE_CUBE_MAP_NEGATIVE_Z,
> +#endif
>  };
>  
>  /** Returns the line in the program string given the character position. */
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk5kFxwACgkQX1gOwKyEAw+01wCfd6inv5LAF4nOu/8gyz/D01nY
5f8AnRVZThkvQOS/Vgk4LU7Pa3h2AkAy
=TwW5
-----END PGP SIGNATURE-----


More information about the Piglit mailing list