[Piglit] [PATCH] Add GL_SELECT render mode test

[Eric Anholt]

On Tue, 9 Aug 2011 00:05:23 +0200, Pierre-Eric Pelloux-Prayer <pelloux at gmail.com> wrote:
Non-text part: multipart/mixed
> Hi,
> Here's a patch adding a simple test for GL_SELECT render mode.
> It tries to represent the typical use case of selection by rendering a
> bunch of squares, and then verifying  the select buffer content.
> It also verifies that alpha, depth, or stencil tests have no effect on
> selection results.
> 
> Regards,
> Pierre-Eric
> From 81904158fe417c6e0baaa86a6726d082fe732781 Mon Sep 17 00:00:00 2001
> From: Pierre-Eric Pelloux-Prayer <pelloux at gmail.com>
> Date: Tue, 9 Aug 2011 00:00:23 +0200
> Subject: [PATCH] Add GL_SELECT render mode test

Thanks.  I've got a few comments inline.


> +/**
> + * Convenience function to draw an axis-aligned rectangle (from depthfunc.c)
> + */
> +static void
> +draw_rect_depth(float x, float y, float w, float h, float d)
> +{
> +	float verts[4][4];
> +
> +	verts[0][0] = x;
> +	verts[0][1] = y;
> +	verts[0][2] = d;
> +	verts[0][3] = 1.0;
> +	verts[1][0] = x + w;
> +	verts[1][1] = y;
> +	verts[1][2] = d;
> +	verts[1][3] = 1.0;
> +	verts[2][0] = x + w;
> +	verts[2][1] = y + h;
> +	verts[2][2] = d;
> +	verts[2][3] = 1.0;
> +	verts[3][0] = x;
> +	verts[3][1] = y + h;
> +	verts[3][2] = d;
> +	verts[3][3] = 1.0;
> +
> +	glVertexPointer(4, GL_FLOAT, 0, verts);
> +	glEnableClientState(GL_VERTEX_ARRAY);
> +
> +	glDrawArrays(GL_QUADS, 0, 4);
> +
> +	glDisableClientState(GL_VERTEX_ARRAY);
> +}

This looks like piglit_draw_rect_z() -- can we just reuse that?

> +GLuint ReferenceHitEntries[3][64];
> +#define NAME_STACK_DEPTH 0
> +#define MIN_Z            1
> +#define MAX_Z            2
> +#define NAME_STACK_0     3
> +
> +/** 
> + * Draw 3 rectangles and handle name stack
> + */
> +static void
> +draw_objects()
> +{
> +   float zscale = (float)(~0u);
> +   glInitNames();

Unlike Mesa, piglit uses sensible 8-space tab indentation.


> +/**
> + * Helper function to check select buffer
> + */
> +static bool
> +validate_select_buffer(GLuint* buffer)
> +{
> +   int i,j;
> +   GLint hits = glRenderMode(GL_RENDER);
> +   bool object_hit_found[3] = {false, false, false};
> +   GLuint* ptr;
> +
> +   if (hits != 3) {
> +      printf("\t%s : unexpected hit count:%d\n", __FUNCTION__, hits);
> +      goto failure;
> +   }

"return false;" instead of the gotos?

> +
> +   ptr = buffer;
> +   for(i=0; i<hits; i++) {

Please put a space between "for" and "(".

> +      for(j=0; j<3; j++) {
> +         if (!object_hit_found[j]) {
> +            if (compare_hit_record(ptr, ReferenceHitEntries[j])) {
> +               object_hit_found[j] = true;
> +               break;
> +            }
> +         }
> +      }
> +      if (j == 3)
> +         goto failure;
> +
> +      /* advance pointer */
> +      ptr += 3 + ptr[NAME_STACK_DEPTH];
> +   }
> +
> +   return true;
> +
> +failure:
> +   return false;
> +}
> +
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +   bool pass = true;
> +   GLuint buff[64] = {0};
> +
> +   glSelectBuffer(64, buff);
> +
> +   piglit_ortho_projection(piglit_width, piglit_height, GL_FALSE);
> +   /* Clear gray. */
> +	glClearColor(0.5, 0.5, 0.5, 0.0);
> +
> +   /* 
> +    * First test case
> +    */
> +   printf("First test case...\n");
> +   glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
> +   glDisable(GL_DEPTH_TEST);
> +   glDisable(GL_STENCIL_TEST);
> +   glDisable(GL_ALPHA_TEST);
> +   glRenderMode(GL_SELECT);
> +   draw_objects();
> +   pass = pass && validate_select_buffer(buff);
> +
> +   /* 
> +    * Second test case : depth test enabled
> +    */
> +   printf("Second test case (z test enabled)...\n");
> +   glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
> +   glEnable(GL_DEPTH_TEST);
> +   glDepthFunc(GL_NEVER);
> +   glRenderMode(GL_SELECT);
> +   draw_objects();
> +   pass = validate_select_buffer(buff) && pass;

If there are multiple testcases doing different rendering paths like
this, we'd like to see multiple tests in all.tests for it.  Sometimes we
make multiple binaries (some of the fbo-*-formats stuff does this while
still sharing some code), and sometimes we have different command line
arguments to the one binary.  The motivation behind doing that is that
we don't want our later addition of some other variant (say, scissor
test) to make everyone's driver go from pass to fail on an existing
testcase.  Having a new test show up gets people less freaked out.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20110810/874086e1/attachment.pgp>