[Piglit] [PATCH v2] util: provide way to read a texture in ES compatible way

[Brian Paul]

On 05/21/2018 05:53 AM, Tapani Pälli wrote:
> Implementation supports GL_TEXTURE_2D, GL_TEXTURE_2D_ARRAY
> and affects following functions:
> 
>     - piglit_probe_texel_rect_rgba
>     - piglit_probe_texel_volume_rgba
> 
> v2: use read_texture only on GL ES
>      fix indentation issues
> 
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>   tests/util/piglit-util-gl.c | 121 +++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 115 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/util/piglit-util-gl.c b/tests/util/piglit-util-gl.c
> index 2443be03e..271cca4a8 100644
> --- a/tests/util/piglit-util-gl.c
> +++ b/tests/util/piglit-util-gl.c
> @@ -1882,6 +1882,99 @@ piglit_probe_image_ubyte(int x, int y, int w, int h, GLenum format,
>   	return 1;
>   }
>   
> +static GLuint
> +create_fbo_from_texture(GLenum target, GLint texture, GLint level, GLint layer)
> +{
> +	GLuint fbo;
> +
> +	glGenFramebuffers(1, &fbo);
> +	glBindFramebuffer(GL_FRAMEBUFFER, fbo);
> +
> +	if (layer > 0) {
> +		glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
> +					  texture, level, layer);
> +	} else {
> +		glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
> +				       target, texture, level);
> +	}
> +
> +	assert(glCheckFramebufferStatus(GL_FRAMEBUFFER) ==
> +	       GL_FRAMEBUFFER_COMPLETE);
> +	return fbo;
> +}
> +
> +static GLenum
> +binding_from_target(GLenum target)
> +{
> +	switch (target)	{
> +	case GL_TEXTURE_2D:
> +		return GL_TEXTURE_BINDING_2D;
> +	case GL_TEXTURE_2D_ARRAY:
> +		return GL_TEXTURE_BINDING_2D_ARRAY;
> +	default:
> +		fprintf(stderr, "%s: unsupported target 0x%x\n",
> +			__func__, target);
> +		return 0;
> +	}
> +}
> +
> +/**
> + * Read texels in OpenGL ES compatible way.
> + *
> + * Currently bound texture is attached to a framebuffer object and
> + * contents are read using glReadPixels. Supported targets are
> + * GL_TEXTURE_2D, GL_TEXTURE_2D_ARRAY.
> + */
> +static GLfloat *
> +read_texture(int target, int level, int x, int y, int layer, int w, int h)

Maybe read_texture_es() or read_texture_via_fbo() to be a bit less generic?


> +{
> +	GLint width, height, depth;
> +	GLint current_read_fbo, current_draw_fbo, current_texture;
> +	GLenum binding = binding_from_target(target);
> +	unsigned char *buf;
> +	GLfloat *buffer;
> +	unsigned offset;
> +
> +	assert(binding != 0);
> +
> +	glGetIntegerv(binding, &current_texture);
> +	glGetIntegerv(GL_READ_FRAMEBUFFER_BINDING, &current_read_fbo);
> +	glGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, &current_draw_fbo);
> +
> +	assert(target == GL_TEXTURE_2D || target == GL_TEXTURE_2D_ARRAY);
> +
> +	glGetTexLevelParameteriv(target, level, GL_TEXTURE_WIDTH, &width);
> +	glGetTexLevelParameteriv(target, level, GL_TEXTURE_HEIGHT, &height);
> +	glGetTexLevelParameteriv(target, level, GL_TEXTURE_DEPTH, &depth);
> +
> +	buffer = malloc(width * height * depth * 4 * sizeof(GLfloat));
> +	buf = malloc(width * height * depth * 4 * sizeof(unsigned char));

Aren't we always returning a 2D image?  If so, why 'depth' here?

> +
> +	GLuint fbo =
> +		create_fbo_from_texture(target, current_texture, level, layer);
> +	assert(fbo != 0);
> +
> +	/* Offset to the layer we are expected to read. */
> +	offset = layer * (width * height * 4);

If we're always returning a 2D image, I don't think we need the offset.


> +
> +	glReadPixels(0, 0, width, height, GL_RGBA, GL_UNSIGNED_BYTE,
> +		     buf + offset);
> +
> +	for (unsigned k = offset; k < offset + width * height * 4; k++)
> +		buffer[k] = ((float) buf[k]) / 255.0;

Wondering about the offset here too.

At the very least, I think more comments are needed to explain what's 
going on if I'm misunderstanding this.

Thanks.

-Brian

> +
> +	free(buf);
> +
> +	glDeleteFramebuffers(1, &fbo);
> +
> +	/* Restore FBO state. */
> +	glBindFramebuffer(GL_READ_FRAMEBUFFER, current_read_fbo);
> +	glBindFramebuffer(GL_DRAW_FRAMEBUFFER, current_draw_fbo);
> +
> +	return buffer;
> +}
> +
> +
>   /**
>    * Read a texel rectangle from the given location and compare its RGB value to
>    * the given expected values.
> @@ -1957,10 +2050,18 @@ int piglit_probe_texel_rect_rgba(int target, int level, int x, int y,
>   
>   	glGetTexLevelParameteriv(target, level, GL_TEXTURE_WIDTH, &width);
>   	glGetTexLevelParameteriv(target, level, GL_TEXTURE_HEIGHT, &height);
> -	buffer = malloc(width * height * 4 * sizeof(GLfloat));
> -
> -	glGetTexImage(target, level, GL_RGBA, GL_FLOAT, buffer);
>   
> +#ifndef PIGLIT_USE_OPENGL
> +	if (target == GL_TEXTURE_2D) {
> +		buffer = read_texture(target, level, x, y, 0, w, h);
> +	} else {
> +#else
> +		buffer = malloc(width * height * 4 * sizeof(GLfloat));
> +		glGetTexImage(target, level, GL_RGBA, GL_FLOAT, buffer);
> +#endif
> +#ifndef PIGLIT_USE_OPENGL
> +	}
> +#endif
>   	assert(x >= 0);
>   	assert(y >= 0);
>   	assert(x+w <= width);
> @@ -2021,10 +2122,18 @@ int piglit_probe_texel_volume_rgba(int target, int level, int x, int y, int z,
>   	glGetTexLevelParameteriv(target, level, GL_TEXTURE_WIDTH, &width);
>   	glGetTexLevelParameteriv(target, level, GL_TEXTURE_HEIGHT, &height);
>   	glGetTexLevelParameteriv(target, level, GL_TEXTURE_DEPTH, &depth);
> -	buffer = malloc(width * height * depth * 4 * sizeof(GLfloat));
> -
> -	glGetTexImage(target, level, GL_RGBA, GL_FLOAT, buffer);
>   
> +#ifndef PIGLIT_USE_OPENGL
> +	if (target == GL_TEXTURE_2D_ARRAY) {
> +		buffer = read_texture(target, level, x, y, z, w, h);
> +	} else {
> +#else
> +		buffer = malloc(width * height * depth * 4 * sizeof(GLfloat));
> +		glGetTexImage(target, level, GL_RGBA, GL_FLOAT, buffer);
> +#endif
> +#ifndef PIGLIT_USE_OPENGL
> +	}
> +#endif
>   	assert(x >= 0);
>   	assert(y >= 0);
>   	assert(d >= 0);
>