[Piglit] [PATCH 3/3 V2] GL 3.2: Test layered framebuffers clear the depth attachment properly.
Paul Berry
stereotype441 at gmail.com
Fri Sep 13 14:40:23 PDT 2013
On 10 September 2013 17:47, Chad Versace <chad.versace at linux.intel.com>wrote:
> On 09/10/2013 01:47 PM, Jacob Penner wrote:
>
>> ---
>> tests/all.tests | 1 +
>> .../gl-3.2/layered-rendering/**CMakeLists.gl.txt | 1 +
>> tests/spec/gl-3.2/layered-**rendering/clear-depth.c | 224
>> +++++++++++++++++++++
>> 3 files changed, 226 insertions(+)
>> create mode 100644 tests/spec/gl-3.2/layered-**rendering/clear-depth.c
>>
>> diff --git a/tests/all.tests b/tests/all.tests
>> index e1366ca..2444337 100644
>> --- a/tests/all.tests
>> +++ b/tests/all.tests
>> @@ -750,6 +750,7 @@ spec['!OpenGL 3.2/get-integer-64iv'] =
>> concurrent_test('gl-3.2-get-**integer-64iv'
>> spec['!OpenGL 3.2/get-integer-64v'] = concurrent_test('gl-3.2-get-**
>> integer-64v')
>> spec['!OpenGL 3.2/layered-rendering/blit'] = concurrent_test('gl-3.2-**
>> layered-rendering-blit')
>> spec['!OpenGL 3.2/layered-rendering/clear-**color'] =
>> concurrent_test('gl-3.2-**layered-rendering-clear-color'**)
>> +spec['!OpenGL 3.2/layered-rendering/clear-**depth'] =
>> concurrent_test('gl-3.2-**layered-rendering-clear-depth'**)
>> spec['!OpenGL 3.2/layered-rendering/**framebuffertexture-buffer-**textures']
>> = concurrent_test('gl-3.2-**layered-rendering-**
>> framebuffertexture-buffer-**textures')
>> spec['!OpenGL 3.2/layered-rendering/**readpixels'] =
>> concurrent_test('gl-3.2-**layered-rendering-readpixels')
>> spec['!OpenGL 3.2/layered-rendering/gl-**layer'] =
>> concurrent_test('gl-3.2-**layered-rendering-gl-layer')
>> diff --git a/tests/spec/gl-3.2/layered-**rendering/CMakeLists.gl.txt
>> b/tests/spec/gl-3.2/layered-**rendering/CMakeLists.gl.txt
>> index 9a26b34..bc4415d 100644
>> --- a/tests/spec/gl-3.2/layered-**rendering/CMakeLists.gl.txt
>> +++ b/tests/spec/gl-3.2/layered-**rendering/CMakeLists.gl.txt
>> @@ -11,6 +11,7 @@ link_libraries (
>>
>> piglit_add_executable (gl-3.2-layered-rendering-blit blit.c)
>> piglit_add_executable (gl-3.2-layered-rendering-**clear-color
>> clear-color.c)
>> +piglit_add_executable (gl-3.2-layered-rendering-**clear-depth
>> clear-depth.c)
>> piglit_add_executable (gl-3.2-layered-rendering-**
>> framebuffertexture-buffer-**textures framebuffertexture-buffer-**
>> textures.c)
>> piglit_add_executable (gl-3.2-layered-rendering-**readpixels
>> readpixels.c)
>> piglit_add_executable (gl-3.2-layered-rendering-gl-**layer gl-layer.c)
>> diff --git a/tests/spec/gl-3.2/layered-**rendering/clear-depth.c
>> b/tests/spec/gl-3.2/layered-**rendering/clear-depth.c
>> new file mode 100644
>> index 0000000..d680780
>> --- /dev/null
>> +++ b/tests/spec/gl-3.2/layered-**rendering/clear-depth.c
>> @@ -0,0 +1,224 @@
>> +/*
>> + * Copyright © 2013 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 clear-depth.c
>> + *
>> + * Section 4.4.7(Framebuffer Objects) From GL spec 3.2 core:
>> + * When the Clear or ClearBuffer* commands are used to clear a layered
>> + * framebuffer attachment, all layers of the attachment are cleared.
>> + *
>> + * Test Layout
>> + * Tex1 Tex2
>> + * *--------*--------*
>> + * | layer4 | layer4 |
>> + * *--------*--------* Each Layer for both tex1 and tex2 will be
>> + * | layer3 | layer3 | different depths.
>> + * *--------*--------*
>> + * | layer2 | layer2 | Tex1 will be cleared using glClear()
>> + * *--------*--------*
>> + * | layer1 | layer1 | Tex2 will be cleared using glClearBuffer()
>> + * *--------*--------*
>> + *
>> + * Result:
>> + * Layer 1-4 of both tex1 and tex2 should be the clearDepth
>> + */
>> +
>> +#include "piglit-util-gl-common.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> + config.supports_gl_compat_**version = 32;
>> + config.supports_gl_core_**version = 32;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +const char *vs_source = {
>> + "#version 150\n"
>> + "in vec4 piglit_vertex;\n"
>> + "void main()\n"
>> + "{\n"
>> + " gl_Position = piglit_vertex;\n"
>> + "}\n"
>> +};
>> +
>> +bool
>> +probe_texture_layered_depth(**GLuint texture, int x, int y, int z,
>> + int w, int h, int d, float *expected)
>> +{
>> + GLint prev_read_fbo;
>> + GLint prev_draw_fbo;
>> +
>> + GLuint fbo;
>> + int i, j, k;
>> +
>> + GLfloat *probe;
>> + GLfloat *pixels = malloc(w*h*sizeof(float));
>> +
>> + glGetIntegerv(GL_DRAW_**FRAMEBUFFER_BINDING, &prev_draw_fbo);
>> + glGetIntegerv(GL_READ_**FRAMEBUFFER_BINDING, &prev_read_fbo);
>> +
>> + glGenFramebuffers(1, &fbo);
>>
>
> Even though this is just a test, please cleanup by deleting the temporary
> fbo.
>
>
> + glBindFramebuffer(GL_**FRAMEBUFFER, fbo);
>> +
>> + for(k = 0; k < d; k++ ) {
>> +
>> + glFramebufferTextureLayer(GL_**FRAMEBUFFER,
>> GL_DEPTH_ATTACHMENT,
>> + texture, 0, k+z);
>> +
>>
>
> There already exists a function for probing depth values,
> piglit_probe_rect_depth.
> Just use that instead of open-coding the below probing.
>
>
> + glReadPixels(x, y, w, h, GL_DEPTH_COMPONENT, GL_FLOAT,
>> pixels);
>> +
>> + for(j = 0; j < h; j++) {
>> + for(i = 0; i < w; i++) {
>> + probe = &pixels[j*w+i];
>> + if (fabs(*probe - expected[k]) >= 0.01) {
>> + printf("Probe depth at
>> (%i,%i,%i)\n",
>> + x+i, y+j,z+k);
>> + printf(" Expected: %f\n",
>> expected[k]);
>> + printf(" Observed: %f\n",
>> *probe);
>> +
>> + free(pixels);
>> + return false;
>> + }
>> + }
>> + }
>> + }
>> + free(pixels);
>> +
>> + glBindFramebuffer(GL_DRAW_**FRAMEBUFFER, prev_draw_fbo);
>> + glBindFramebuffer(GL_READ_**FRAMEBUFFER, prev_read_fbo);
>> + return true;
>> +}
>> +
>> +void
>> +piglit_init(int argc, char **argv)
>> +{
>> +
>> + int i, j;
>> + GLenum fbstatus;
>> + bool pass = true;
>> + GLuint fbo[2], texture[2];
>> + GLint program;
>> +
>> + float depths[4] = {
>> + 0.25, 0.5, 0.75, 1.0
>> + };
>> +
>> + GLfloat clearDepth = 0.0;
>> + float expected[4] = { 0.0, 0.0, 0.0, 0.0 };
>> +
>> + program = piglit_build_simple_program(**vs_source, NULL);
>> + glUseProgram(program);
>> +
>> + glGenTextures(2, texture);
>> + glGenFramebuffers(2, fbo);
>> + for(i = 0; i < 2; i++) {
>> + glBindTexture(GL_TEXTURE_2D_**ARRAY, texture[i]);
>> + glTexParameteri(GL_TEXTURE_2D_**ARRAY,
>> GL_TEXTURE_MIN_FILTER,
>> + GL_LINEAR);
>> + glTexParameteri(GL_TEXTURE_2D_**ARRAY,
>> GL_TEXTURE_MAG_FILTER,
>> + GL_LINEAR);
>> + glTexParameteri(GL_TEXTURE_2D_**ARRAY, GL_TEXTURE_WRAP_S,
>> + GL_REPEAT);
>> + glTexParameteri(GL_TEXTURE_2D_**ARRAY, GL_TEXTURE_WRAP_T,
>> + GL_REPEAT);
>> + glTexImage3D(GL_TEXTURE_2D_**ARRAY, 0,
>> GL_DEPTH_COMPONENT32,
>> + 10, 10, 4, 0, GL_DEPTH_COMPONENT, GL_FLOAT,
>> NULL);
>> +
>> + glEnable(GL_DEPTH_TEST);
>> + glDepthFunc(GL_ALWAYS);
>> + glDepthRange(0, 1);
>> + glDrawBuffer(GL_NONE);
>>
>
> Ah... I was unaware of piglit_draw_rect_z. Since we're using that function,
> there is no need to call glDepthRange.
>
> I'm confused by the call to glDrawBuffer. What are you trying to
> accomplish with it?
Looking at the specs yesterday, I *thought* the call is necessary in order
to ensure framebuffer completeness, since there's no color attachment to
the framebuffer. In GL 3.2 (section 4.4.4 Framebuffer Completeness, under
the "Whole Framebuffer Completeness" section), the following is listed as a
criterion for framebuffer completeness:
The value of FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE must not be
NONE for any color attachment point(s) named by DRAW_BUFFERi.
{ FRAMEBUFFER_INCOMPLETE_DRAW_BUFFER }
However, maybe I'm interpreting the spec wrong, because neither Mesa nor
the NVIDIA proprietary driver requires the glDrawBuffer() call in order for
the framebuffer to be complete.
Any insight on this, Jacob?
>
>
> +
>> + glBindFramebuffer(GL_**FRAMEBUFFER, fbo[i]);
>> +
>> + for(j = 0; j < 4; j++) {
>> + glFramebufferTextureLayer(GL_**FRAMEBUFFER,
>> + GL_DEPTH_ATTACHMENT,
>> + texture[i], 0, j);
>> +
>> +
>> + fbstatus = glCheckFramebufferStatus(GL_**
>> FRAMEBUFFER);
>> + if(fbstatus != GL_FRAMEBUFFER_COMPLETE){
>> + printf("%s\n", piglit_get_gl_enum_name(**
>> fbstatus));
>> + piglit_report_result(PIGLIT_**FAIL);
>> + }
>> +
>> + piglit_check_gl_error(GL_NO_**ERROR);
>>
>
> If you want the test to fail, tt's not enough to call
> piglit_check_gl_error. That
> function doesn't terminate test. You need this:
>
> if (!piglit_check_gl_error(GL_NO_**ERROR))
> piglit_report_result(PIGLIT_**FAIL);
>
>
> + piglit_draw_rect_z(depths[j], -1, -1, 2, 2);
>> + piglit_check_gl_error(GL_NO_**ERROR);
>> + }
>> +
>> + glDisable(GL_DEPTH_TEST);
>>
>
> The code from here...
>
>
> +
>> + /* Once values are set, attach the texture to the fbo
>> + * as a layered texture
>> + */
>> + glFramebufferTexture(GL_**FRAMEBUFFER,
>> GL_DEPTH_ATTACHMENT,
>> + texture[i], 0);
>> +
>> +
>> + fbstatus = glCheckFramebufferStatus(GL_**FRAMEBUFFER);
>> + if(fbstatus != GL_FRAMEBUFFER_COMPLETE){
>> + printf("%s\n", piglit_get_gl_enum_name(**
>> fbstatus));
>> + piglit_report_result(PIGLIT_**FAIL);
>> + }
>> +
>> + if(!piglit_check_gl_error(GL_**NO_ERROR))
>> + piglit_report_result(PIGLIT_**FAIL);
>>
>
> ... to here. I'm unsure what it is trying to do. I don't see the need to
> test
> for framebuffer completeness if nothing is rendered to the framebuffer
> with those attachments.
IMHO the framebuffer completeness check is useful because if for some
unexpected reason the framebuffer isn't complete, the clear calls below
will likely fail. It's easier to debug a test failure caused by unexpected
framebuffer incompleteness if the test explicitly checks framebuffer
completeness before doing its rendering.
I'm mildly in favor of keeping the framebuffer completeness check but I
don't feel terribly strongly.
>
>
> + }
>> +
>> +
>> + glBindFramebuffer(GL_**FRAMEBUFFER, fbo[0]);
>> + /* Clear fbo 0 with glClear() */
>> + glClearDepth(clearDepth);
>> + glClear(GL_DEPTH_BUFFER_BIT);
>> +
>> + if(!probe_texture_layered_**depth(texture[0], 0, 0, 0, 10,
>> + 10, 4, expected)) {
>> + printf("Incorrect depth values recieved with
>> glClear()\n");
>> + pass = false;
>> + }
>> +
>> + /* Clear fbo 1 with glClearBuffer() */
>> + glBindFramebuffer(GL_**FRAMEBUFFER, fbo[1]);
>> + glClearBufferfv(GL_DEPTH, 0, &clearDepth);
>> +
>> + if(!probe_texture_layered_**depth(texture[1], 0, 0, 0, 10,
>> + 10, 4, expected)) {
>> + printf("Incorrect depth values recieved with
>> glClearBuffer()\n");
>> + pass = false;
>> + }
>> +
>> + pass = piglit_check_gl_error(GL_NO_**ERROR) && pass;
>> +
>> + piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
>> +}
>> +
>> +
>> +enum piglit_result
>> +piglit_display(void)
>> +{
>> + /* UNREACHABLE */
>> + return PIGLIT_FAIL;
>> +}
>>
>
> The rest of the test looks good to me. Of course, we should get Paul's
> opinion too.
I agree with Chad's comments about deleting the temporary fbo, using
piglit_probe_rect_depth, removing the call to glDepthRange(), and checking
the return value of piglit_check_gl_error(). With those changes, the patch
is:
Reviewed-by: Paul Berry <stereotype441 at gmail.com>
I could go either way on the framebuffer completeness check and the use of
glDrawBuffer().
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130913/2b203010/attachment-0001.html>
More information about the Piglit
mailing list