[Piglit] [PATCH] getteximage-formats: test GetTexImage after rendering to textures

[Brian Paul]

On 02/06/2013 05:06 PM, Marek Olšák wrote:
> This tests whether our GetTexImage doesn't actually rely on TexImage being
> called first. In this test, TexImage is not used to store data.
> ---
>   tests/all.tests                       |    1 +
>   tests/texturing/getteximage-formats.c |   75 +++++++++++++++++++++++++--------
>   2 files changed, 58 insertions(+), 18 deletions(-)
>
> diff --git a/tests/all.tests b/tests/all.tests
> index 5a76926..00756f7 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -1607,6 +1607,7 @@ add_plain_test(ext_framebuffer_object, 'fbo-readpixels-depth-formats')
>   add_plain_test(ext_framebuffer_object, 'fbo-scissor-bitmap')
>   add_plain_test(ext_framebuffer_object, 'fbo-storage-completeness')
>   add_plain_test(ext_framebuffer_object, 'fbo-storage-formats')
> +add_plain_test(ext_framebuffer_object, 'getteximage-formats init-by-rendering')
>
>
>   ext_packed_depth_stencil = Group()
> diff --git a/tests/texturing/getteximage-formats.c b/tests/texturing/getteximage-formats.c
> index affcfd5..1643f14 100644
> --- a/tests/texturing/getteximage-formats.c
> +++ b/tests/texturing/getteximage-formats.c
> @@ -48,9 +48,10 @@ PIGLIT_GL_TEST_CONFIG_END
>   static const char *TestName = "getteximage-formats";
>
>   static const GLfloat clearColor[4] = { 0.4, 0.4, 0.4, 0.0 };
> +static GLuint texture_id;
> +static GLboolean init_by_rendering;
>
> -#define TEX_WIDTH 128
> -#define TEX_HEIGHT 128
> +#define TEX_SIZE 128
>
>   #define DO_BLEND 1
>
> @@ -64,13 +65,14 @@ static const GLfloat clearColor[4] = { 0.4, 0.4, 0.4, 0.0 };
>   static GLboolean
>   make_texture_image(GLenum intFormat, GLubyte upperRightTexel[4])
>   {
> -	GLubyte tex[TEX_HEIGHT][TEX_WIDTH][4];
> +	GLubyte tex[TEX_SIZE][TEX_SIZE][4];
>   	int i, j;
> +	GLuint fb, status;
>
> -	for (i = 0; i<  TEX_HEIGHT; i++) {
> -		for (j = 0; j<  TEX_WIDTH; j++) {
> -			tex[i][j][0] = j * 255 / TEX_WIDTH;
> -			tex[i][j][1] = i * 255 / TEX_HEIGHT;
> +	for (i = 0; i<  TEX_SIZE; i++) {
> +		for (j = 0; j<  TEX_SIZE; j++) {
> +			tex[i][j][0] = j * 255 / TEX_SIZE;
> +			tex[i][j][1] = i * 255 / TEX_SIZE;
>   			tex[i][j][2] = 128;
>   			if (((i>>  4) ^ (j>>  4))&  1)
>   				tex[i][j][3] = 255;  /* opaque */
> @@ -79,10 +81,38 @@ make_texture_image(GLenum intFormat, GLubyte upperRightTexel[4])
>   		}
>   	}
>
> -	memcpy(upperRightTexel, tex[TEX_HEIGHT-1][TEX_WIDTH-1], 4);
> +	memcpy(upperRightTexel, tex[TEX_SIZE-1][TEX_SIZE-1], 4);
>
> -	glTexImage2D(GL_TEXTURE_2D, 0, intFormat, TEX_WIDTH, TEX_HEIGHT, 0,
> -					 GL_RGBA, GL_UNSIGNED_BYTE, tex);
> +	glTexImage2D(GL_TEXTURE_2D, 0, intFormat, TEX_SIZE, TEX_SIZE, 0,
> +		     GL_RGBA, GL_UNSIGNED_BYTE, init_by_rendering ? NULL : tex);
> +
> +	if (init_by_rendering) {
> +		/* Initialize the mipmap levels. */
> +		for (i = TEX_SIZE>>  1, j = 1; i; i>>= 1, j++) {
> +			glTexImage2D(GL_TEXTURE_2D, j, intFormat, i, i, 0,
> +				     GL_RGBA, GL_UNSIGNED_BYTE, NULL);
> +		}

Minor nit:  I'd restructure the code to generate all the empty mipmap 
levels inside the if (init_by_rendering) clause, and then do the 
non-rendering glTexImage2D call in an else clause.  Not a big deal though.

> +
> +		/* Initialize the texture with glDrawPixels. */
> +		glGenFramebuffers(1,&fb);
> +		glBindFramebuffer(GL_FRAMEBUFFER, fb);
> +		glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
> +				       GL_TEXTURE_2D, texture_id, 0);
> +		status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
> +		if (status != GL_FRAMEBUFFER_COMPLETE) {
> +			glDeleteFramebuffers(1,&fb);
> +			return GL_FALSE;
> +		}
> +
> +		glViewport(0, 0, TEX_SIZE, TEX_SIZE);
> +
> +		glWindowPos2iARB(0, 0);
> +		glDrawPixels(TEX_SIZE, TEX_SIZE, GL_RGBA, GL_UNSIGNED_BYTE, tex);
> +		glGenerateMipmap(GL_TEXTURE_2D);
> +
> +		glDeleteFramebuffers(1,&fb);
> +		glViewport(0, 0, piglit_width, piglit_height);
> +	}
>
>   	return glGetError() == GL_NO_ERROR;
>   }
> @@ -324,8 +354,8 @@ test_format(const struct test_desc *test,
>   	    const struct format_desc *fmt)
>   {
>   	int x, y;
> -	int w = TEX_WIDTH, h = TEX_HEIGHT;
> -	GLfloat readback[TEX_HEIGHT][TEX_WIDTH][4];
> +	int w = TEX_SIZE, h = TEX_SIZE;
> +	GLfloat readback[TEX_SIZE][TEX_SIZE][4];
>   	GLubyte upperRightTexel[4];
>   	int level;
>   	GLfloat expected[4], pix[4], tolerance[4];
> @@ -357,7 +387,7 @@ test_format(const struct test_desc *test,
>   		glDisable(GL_TEXTURE_2D);
>   		glDisable(GL_BLEND);
>
> -		x += TEX_WIDTH + 20;
> +		x += TEX_SIZE + 20;
>
>   		level = 0;
>   		while (w>  0) {
> @@ -485,21 +515,30 @@ piglit_display(void)
>   void
>   piglit_init(int argc, char **argv)
>   {
> -	GLuint t;
> +	int i;
>
>   	if ((piglit_get_gl_version()<  14)&&  !piglit_is_extension_supported("GL_ARB_window_pos")) {
>   		printf("Requires GL 1.4 or GL_ARB_window_pos");
>   		piglit_report_result(PIGLIT_SKIP);
>   	}
>
> -	fbo_formats_init(argc, argv, !piglit_automatic);
> +	fbo_formats_init(1, argv, !piglit_automatic);

What's that change for?


>   	(void) fbo_formats_display;
>
> -	glGenTextures(1,&t);
> -	glBindTexture(GL_TEXTURE_2D, t);
> +	for (i = 1; i<  argc; i++) {
> +		if (strcmp(argv[i], "init-by-rendering") == 0) {
> +			init_by_rendering = GL_TRUE;
> +			puts("The textures will be initialized by rendering "
> +			     "to them using glDrawPixels.");
> +			break;
> +		}
> +	}
> +
> +	glGenTextures(1,&texture_id);
> +	glBindTexture(GL_TEXTURE_2D, texture_id);
>   	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
>   	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
> -	glTexParameteri(GL_TEXTURE_2D, GL_GENERATE_MIPMAP, GL_TRUE);
> +	glTexParameteri(GL_TEXTURE_2D, GL_GENERATE_MIPMAP, !init_by_rendering);

Maybe move this call into the previously suggested else clause (and 
maybe use glGenerateMipmap(GL_TEXTURE_2D) instead).


>
>   	glTexEnvi(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_REPLACE);
>

Looks good otherwise.

Reviewed-by: Brian Paul <brianp at vmware.com>