[Piglit] [RFC PATCH] KHR image from texture test
Eric Anholt
eric at anholt.net
Thu Jan 3 17:22:05 PST 2013
I'm really excited to see the KHR_image code see some testing. Things
are in bad shape and this will help a lot.
Abdiel Janulgue <abdiel.janulgue at linux.intel.com> writes:
> Initial piglit test for image from texture extension. This patch-set currently
> tests KHR_gl_texture_2D_image functionality. I plan to incrementally
> add tests for cubemaps and 3D textures at some point. The test basically
> makes sure that we create correct multiple EGL targets from a source texture.
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
> tests/spec/CMakeLists.txt | 1 +
> .../spec/khr_gl_texture_image/CMakeLists.gles2.txt | 17 ++
> tests/spec/khr_gl_texture_image/CMakeLists.txt | 1 +
> .../khr_gl_texture_image/khr_gl_texture_image.c | 205 ++++++++++++++++++++
You'll also need to add the call to the testcase from all.tests in order
for it to get run.
> diff --git a/tests/spec/khr_gl_texture_image/khr_gl_texture_image.c b/tests/spec/khr_gl_texture_image/khr_gl_texture_image.c
> new file mode 100644
> index 0000000..9250a81
> --- /dev/null
> +++ b/tests/spec/khr_gl_texture_image/khr_gl_texture_image.c
> @@ -0,0 +1,205 @@
> +
> +#define EGL_EGLEXT_PROTOTYPES 1
> +#define GL_GLEXT_PROTOTYPES 1
> +
> +#include "piglit-util-gl-common.h"
> +#include "piglit-util-egl.h"
> +
> +#include <EGL/eglext.h>
> +
> +#define WIDTH 512
> +#define HEIGHT 512
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_es_version = 20;
> +
> + config.window_width = WIDTH;
> + config.window_height = HEIGHT;
> + config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const char
> +vertex_shader[] =
> + "attribute vec4 pos_attrib;\n"
> + "attribute vec2 tex_attrib;\n"
> + "varying vec2 tex_coord;\n"
> + "void main () {\n"
> + "gl_Position = pos_attrib;\n"
> + "tex_coord = tex_attrib;\n"
> + "}\n";
> +
> +static const char
> +fragment_shader[] =
> + "uniform sampler2D tex;\n"
> + "varying vec2 tex_coord;\n"
> + "void main () {\n"
> + "gl_FragColor = texture2D(tex, tex_coord);\n"
> + "}\n";
> +
> +static GLuint
> +make_shader(GLenum type,
> + const char *source)
> +{
> + GLuint shader;
> + GLint length = strlen (source);
> + GLint status;
> +
> + shader = glCreateShader(type);
> + glShaderSource(shader, 1, &source, &length);
> + glCompileShader(shader);
> + glGetShaderiv(shader, GL_COMPILE_STATUS, &status);
> +
> + if (!status)
> + fprintf(stderr, "Shader compilation failed\n");
> +
> + return shader;
> +}
You should be able to use piglit_compile_shader_text() for this.
> + glGetProgramiv(program, GL_LINK_STATUS, &status);
> + if (!status) {
> + glGetProgramInfoLog(program, 255, &len, glerr);
> + fprintf(stderr, "Program linking failed, len: %d, error: %s\n",
> + len, glerr);
> + }
This could be replaced by checking piglit_link_check_status().
> +GLuint
clean up trailing whitespace throughout the file. chmod +x
.git/hooks/pre-commit can help catch this in the future.
> +create_egl_image_target(GLuint texture, GLuint level)
> +{
> + GLuint target_texture;
> + EGLint attribs[] = {
tabs for indentation (tabs are always 8 spaces in this project)
> + EGL_GL_TEXTURE_LEVEL_KHR, level,
> + EGL_IMAGE_PRESERVED_KHR, EGL_TRUE,
> + EGL_NONE };
We're pretty consistent that that closing brace is on its own line
> + EGLDisplay dpy = eglGetCurrentDisplay();
> + EGLContext ctx = eglGetCurrentContext();
> + EGLImageKHR image = eglCreateImageKHR(dpy, ctx, EGL_GL_TEXTURE_2D_KHR,
> + (EGLClientBuffer)(uintptr_t)texture,
> + attribs);
> + if (image == EGL_NO_IMAGE_KHR) {
> + printf("Warning EGL_NO_IMAGE_KHR 0x%x\n",
> + eglGetError());
> + }
> +
> + glGenTextures(1, &target_texture);
> + glBindTexture(GL_TEXTURE_2D, target_texture);
> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER,
> + GL_LINEAR);
> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,
> + GL_LINEAR);
> + glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, image);
> +
> + return target_texture;
> +}
> +
> +int miplevel_rendered_correctly(int level)
line break between return type and function name. Some people use the
line break to "git grep ^function_name" to find a function. (Of course,
we should all be using tags or some other IDE feature, but it's
traditional). More importantly, local functions to the file should be
declared static.
> +{
> + static float blue[] = {0.0, 0.0, 1.0};
> + /* approximate 5th mip level minification */
> + static float interp[] = {0.23, 0.19, 0.94};
> + /* smallest mip level interpolates to a single gray pixel */
> + static float gray[] = {0.5, 0.5, 0.5};
> +
> + switch(level) {
space between 'switch' and '('
> + case 6:
> + return piglit_probe_pixel_rgb(WIDTH - (WIDTH * 0.25),
> + HEIGHT - (HEIGHT * 0.25), gray);
> + case 5:
> + return piglit_probe_pixel_rgb(WIDTH - (WIDTH * 0.35),
> + HEIGHT - (HEIGHT * 0.60), interp);
> + case 3:
> + return piglit_probe_pixel_rgb(WIDTH - (WIDTH * 0.95),
> + HEIGHT - (HEIGHT * 0.60), blue);
I'd like to see a probe of the whole rendered texture (except maybe for
any questionable interpolated pixels), instead of a single pixel. It
would be really easy to make a broken implementation that gets just some
of the pixels right (like if alignment bits are getting truncated, for
example)
> +enum piglit_result
> +piglit_display(void)
> +{
> + EGLDisplay dpy = eglGetCurrentDisplay();
> + EGLSurface draw = eglGetCurrentSurface(EGL_DRAW);
> + GLboolean pass = GL_TRUE;
We are trying to avoid using GLboolean and the other awful GL types for
anything but interaction with the GL API. stdbool (bool, true, false)
is good.
> + /* Set up a source texture object with mipmap generation */
> + texture = piglit_rgbw_texture(GL_RGBA, 120, 120, GL_FALSE, GL_FALSE, 0);
> + glGenerateMipmap(GL_TEXTURE_2D);
Hmm, using the RGBW quadrants at every level seems unfortunate. The
implementation could, for example, ignore the EGL_TEXTURE_LEVEL
attribute and it would only show up as maybe a poorly interpolated
smallest miplevel. I'd consider unique, specific colors per image
level. In some cases where we have been concerned that flipping of an
image might occur (like the cubemap tests), we've used a solid color per
level with one quadrant of each having a different solid color.
> + /* Generate EGLImage target textures for some levels */
> + glBindTexture(GL_TEXTURE_2D, create_egl_image_target(texture, 3));
> + piglit_draw_rect_tex(-1, -1, 1.0, 1.0, 0, 0, 1, 1);
> +
> + glBindTexture(GL_TEXTURE_2D, create_egl_image_target(texture, 5));
> + piglit_draw_rect_tex(0, -1, 1.0, 1.0, 0, 0, 1, 1);
> +
> + glBindTexture(GL_TEXTURE_2D, create_egl_image_target(texture, 6));
> + piglit_draw_rect_tex(0, 0, 1.0, 1.0, 0, 0, 1, 1);
I'd like to see enumeration of each texture levels if possible, then
rendering each level at a 1:1 mapping of pixels, so that a developer
with a broken implementation can clearly see what's going wrong.
(Huh, looks like we should have the minify() function in the utils
code but don't currently).
> + /* Check for correctness */
> + if (miplevel_rendered_correctly(6) &&
> + miplevel_rendered_correctly(5) &&
> + miplevel_rendered_correctly(3))
> + pass &= GL_TRUE;
I don't think you actually end up setting pass to false in the presence
of an error, here.
> +void
> +piglit_init(int argc, char **argv)
> +{
> + const char *s;
> +
> + EGLDisplay dpy = eglGetCurrentDisplay();
> + s = (const char *) eglQueryString(dpy, EGL_EXTENSIONS);
> + if (!piglit_is_extension_in_string(s, "EGL_KHR_gl_texture_2D_image")) {
> + piglit_report_result(PIGLIT_SKIP);
> + exit(1);
piglit_report_result calls exit(), so you don't have to.
Oh, but it looks like there's actually a
piglit_is_egl_extension_supported(), which replaces most of this.
(there ought to be a piglit_require_egl_extension, though, just like
there is a piglit_require_extension() and
piglit_require_glx_extension())
This should also check for GL_OES_EGL_image, since it's using that for
binding the image back to a texture.
-------------- 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/20130103/ab2f563a/attachment.pgp>
More information about the Piglit
mailing list