On 21.05.2018 18:01, Brian Paul wrote:
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.pa...@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?
Yeah, read_texure_via_fbo() sounds fine to me.
+{ + 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, ¤t_texture); + glGetIntegerv(GL_READ_FRAMEBUFFER_BINDING, ¤t_read_fbo); + glGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, ¤t_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?
Function supports reading a single layer of 2D array (called from piglit_probe_texel_volume_rgba). This could be refactored a bit cleaner I guess but it fulfills the current requirements of the callers. One option would be to move buffer alloc to the callers and not have any offsets calculated 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.
Sure, I can add a bit more comments what is going on.
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);
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
