[Piglit] [PATCH] Test that dFdx/dFdy work properly on fbos
Paul Berry
stereotype441 at gmail.com
Thu Jun 21 10:11:24 PDT 2012
On 20 June 2012 17:44, Chad Versace <chad.versace at linux.intel.com> wrote:
> On 06/20/2012 03:16 PM, Paul Berry wrote:
> > This test exposes a bug in Mesa on i965 hardware (as of commit
> > f2f05e5): the GLSL function dFdy() produces incorrect results when
> > rendering to an FBO. This is a consequence of the fact that FBOs
> > place the origin at the upper left, whereas windowsystem framebuffers
> > place the origin at the lower left.
> >
> > Verified using nVidia's proprietary binary driver for Linux.
> > ---
> > tests/all.tests | 1 +
> > tests/fbo/CMakeLists.gl.txt | 1 +
> > tests/fbo/fbo-deriv.c | 150
> +++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 152 insertions(+), 0 deletions(-)
> > create mode 100644 tests/fbo/fbo-deriv.c
> >
> > diff --git a/tests/all.tests b/tests/all.tests
> > index 76bf001..bf081d9 100644
> > --- a/tests/all.tests
> > +++ b/tests/all.tests
> > @@ -203,6 +203,7 @@ add_plain_test(fbo, 'fbo-copyteximage-simple')
> > add_plain_test(fbo, 'fbo-depth-array')
> > add_plain_test(fbo, 'fbo-depthtex')
> > add_plain_test(fbo, 'fbo-depth-sample-compare')
> > +add_plain_test(fbo, 'fbo-deriv')
> > add_plain_test(fbo, 'fbo-drawbuffers')
> > add_plain_test(fbo, 'fbo-drawbuffers-arbfp')
> > add_plain_test(fbo, 'fbo-drawbuffers-blend-add')
> > diff --git a/tests/fbo/CMakeLists.gl.txt b/tests/fbo/CMakeLists.gl.txt
> > index 7025019..7e15ae2 100644
> > --- a/tests/fbo/CMakeLists.gl.txt
> > +++ b/tests/fbo/CMakeLists.gl.txt
> > @@ -37,6 +37,7 @@ piglit_add_executable (fbo-copypix fbo-copypix.c)
> > piglit_add_executable (fbo-readdrawpix fbo-readdrawpix.c)
> > piglit_add_executable (fbo-clearmipmap fbo-clearmipmap.c)
> > piglit_add_executable (fbo-clear-formats fbo-clear-formats.c)
> > +piglit_add_executable (fbo-deriv fbo-deriv.c)
> > piglit_add_executable (fbo-drawbuffers fbo-drawbuffers.c)
> > piglit_add_executable (fbo-drawbuffers-arbfp fbo-drawbuffers-arbfp.c)
> > piglit_add_executable (fbo-drawbuffers-blend-add
> fbo-drawbuffers-blend-add.c)
> > diff --git a/tests/fbo/fbo-deriv.c b/tests/fbo/fbo-deriv.c
> > new file mode 100644
> > index 0000000..56779c6
> > --- /dev/null
> > +++ b/tests/fbo/fbo-deriv.c
> > @@ -0,0 +1,150 @@
> > +/*
> > + * Copyright © 2012 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> obtaining a
> > + * copy of this software and associated documentation files (the
> "Software"),
> > + * to deal in the Software without restriction, including without
> limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> next
> > + * paragraph) shall be included in all copies or substantial portions
> of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +
> > +/**
> > + * \file fbo-deriv.cp
> ^ small typo
>
Fixed, thanks.
> > + *
> > + * Verify that the implementation produces correct values for the GLSL
> > + * dFdx() and dFdy() functions, both in fbos and in the default
> > + * framebuffer.
> > + *
> > + * Note: the reason that we test both fbos and the default framebuffer
> > + * in the same test is that some implementations (e.g. Mesa/i965) need
> > + * to compile the dFdy() function differently depending whether we are
> > + * rendering to an FBO or to the default framebuffer; testing both in
> > + * the same test allows us to verify that the implementation
> > + * recompiles the shader if necessary.
> > + *
> > + * This test draws a pair of squares in which dFdx and dFdy are
> > + * expected to both be 1.0. It colors the rectangles red=0.5*dFdx and
> > + * green=0.5*dFdy, so the expected color is (0.5, 0.5, 0, 0). The
> > + * left rectangle is drawn in the default framebuffer; the right
> > + * rectangle is drawn in an FBO and then blitted to the default
> > + * framebuffer.
> > + */
> > +
> > +#include "piglit-util.h"
> > +
> > +int piglit_width = 256, piglit_height = 128;
> > +int piglit_window_mode = GLUT_DOUBLE | GLUT_RGBA;
>
> I just pushed the series that moves main() to each test. Be sure to replace
> the above with
> PIGLIT_GL_TEST_MAIN(256 /*window_width*/,
> 128 /*window_height*/,
> GLUT_DOUBLE | GLUT_RGBA)
> before committing.
>
Oh, cool. Fortunately I get a nice compile error if I forget :) I've
updated this patch (and all of my other pending Piglit patches).
>
> > +static const int fbo_width = 128, fbo_height = 128;
> > +
> > +static GLuint fbo;
> > +static GLint prog;
> > +
> > +static const char *vert =
> > + "void main()\n"
> > + "{\n"
> > + " gl_Position = gl_Vertex;\n"
> > + "}\n";
> > +
> > +static const char *frag =
> > + "void main()\n"
> > + "{\n"
> > + " gl_FragColor = vec4(0.5*dFdx(gl_FragCoord.x),\n"
> > + " 0.5*dFdy(gl_FragCoord.y), 0.0, 0.0);\n"
> > + "}\n";
>
> Yes, dx/dx and dy/dy should be 1 :)
>
> > +
> > +static void
> > +create_fbo()
> > +{
> > + GLenum status;
> > + GLuint rb;
> > +
> > + glGenRenderbuffers(1, &rb);
> > + glBindRenderbuffer(GL_RENDERBUFFER, rb);
> > + glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA, fbo_width,
> fbo_height);
> > + glGenFramebuffers(1, &fbo);
> > + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, fbo);
> > + glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER,
> GL_COLOR_ATTACHMENT0,
> > + GL_RENDERBUFFER, rb);
> > + status = glCheckFramebufferStatusEXT (GL_FRAMEBUFFER_EXT);
> > +
> > + if (!piglit_check_gl_error(GL_NO_ERROR))
> > + piglit_report_result(PIGLIT_FAIL);
> > +
> > + switch (status) {
> > + case GL_FRAMEBUFFER_UNSUPPORTED:
> > + printf("Framebuffer unsupported\n");
> > + piglit_report_result(PIGLIT_SKIP);
> > + break;
> > + case GL_FRAMEBUFFER_COMPLETE:
> > + break;
> > + default:
> > + printf("Framebuffer incomplete\n");
> > + piglit_report_result(PIGLIT_FAIL);
> > + break;
> > + }
> > +}
> > +
> > +void
> > +piglit_init(int argc, char **argv)
> > +{
> > + GLint vs, fs;
> > +
> > + if (piglit_get_gl_version() < 20) {
> > + printf("Requires OpenGL 2.0\n");
> > + piglit_report_result(PIGLIT_SKIP);
> > + }
>
> You can instead use `piglit_require_gl_version(20)` here, which does the
> same thing. Meh.
>
Oh, yeah. That's what I get for cribbing off of old tests. I think I'll
go with piglit_require_gl_version(20).
>
> On the other hand, is this check even necessary for this test, since
> piglit-dispatch will skip if certain functions are
> not found? I'm asking out of curiosity, and not to passively request that
> you change anything.
>
I don't know. On the one hand, it's nice to have the tests explicitly
declare what they depend on. On the other hand, if we forget to declare a
dependency, we'll never notice because piglit-dispatch will take care of
the problem.
Back when I was writing piglit-dispatch I was considering making it so that
it would only give you access to GL functions in the particular GL version
and extensions that you explicitly required. If you tried to call a
function that wasn't available in the GL version and extensions that you
explicitly required, it would give you a run-time error, regardless of
whether that function was supported by the implementation. I figured it
would be a good way of alerting us to situations where we mean to test
feature A in isolation, but without realizing it we wind up writing a test
that also requires feature B. The idea didn't seem too popular at the time
so I dropped it :)
>
> > + vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vert);
> > + fs = piglit_compile_shader_text(GL_FRAGMENT_SHADER, frag);
> > + prog = piglit_link_simple_program(vs, fs);
> > +
> > + create_fbo();
> > +}
> > +
> > +enum piglit_result
> > +piglit_display(void)
> > +{
> > + float expected[3] = { 0.5, 0.5, 0.0 };
> > + GLboolean pass = GL_TRUE;
> > +
> > + glUseProgram(prog);
> > +
> > + /* Draw a square to the left half of the window */
> > + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0);
> > + glViewport(0, 0, piglit_width, piglit_height);
> > + piglit_draw_rect(-1, -1, 1, 2);
> > +
> > + /* Draw a square to the FBO */
> > + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, fbo);
> > + glViewport(0, 0, fbo_width, fbo_height);
> > + piglit_draw_rect(-1, -1, 2, 2);
> > +
> > + /* Blit the square from the FBO to the right half of the window */
> > + glBindFramebuffer(GL_READ_FRAMEBUFFER, fbo);
> > + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0);
> > + glBlitFramebuffer(0, 0, fbo_width, fbo_height,
> > + piglit_width/2, 0, piglit_width, piglit_height,
> > + GL_COLOR_BUFFER_BIT, GL_NEAREST);
> > +
> > + /* Check that both squares have the correct color */
> > + glBindFramebuffer(GL_READ_FRAMEBUFFER, 0);
> > + pass = piglit_probe_rect_rgb(0, 0, piglit_width, piglit_height,
> > + expected) && pass;
> > +
> > + piglit_present_results();
> > +
> > + return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> > +}
>
> Other than the little typo and need for PIGLIT_GL_TEST_MAIN(), this is
> Reviewed-by: Chad Versace <chad.versace at linux.intel.com>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20120621/9e794277/attachment-0001.html>
More information about the Piglit
mailing list