[Piglit] [PATCH 6/8] util: fix piglit-util for GLES1
Chia-I Wu
olvaffe at gmail.com
Sun Sep 4 19:05:46 PDT 2011
On Mon, Sep 5, 2011 at 8:26 AM, Ian Romanick <idr at freedesktop.org> wrote:
> -----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.
They are in piglit-util-gl.c. There is a difference in that GLES1
uses glOrthof while GL uses glOrtho.
>> ---
>> 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.
Will fix it. This hunk is copy-and-pasted from piglit-util-gl.c.
>> +
>> +
>> +/**
>> + * 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
This is indeed better. Will make the change here and in patch 1.
>> 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.
Will do.
>> 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-----
>
--
olv at LunarG.com
More information about the Piglit
mailing list