[Piglit] [PATCH 1/9] ARB_viewport_array: Validity test for viewport index/first/count params

Jon Ashburn jon at lunarg.com
Thu Oct 31 16:44:08 CET 2013


On 10/29/2013 05:47 PM, Brian Paul wrote:
> On 10/29/2013 03:55 PM, Jon Ashburn wrote:
>> Tests GL_ARB_viewport_array regarding the validity for the indices.
>> Use both valid and invalid parameters (index, first, count) for these 
>> new
>> API entry points:
>>    glViewportArrayv, glViewportIndexedf, glViewportIndexedfv, 
>> glGetFloati_v.
>>
>> Also test that writing to an invalid viewport index for Viewport, 
>> DepthRange,
>> Scissor Box, Scissor Test does not modify any of the state for the valid
>> range of indices.
>>
>> Tested on Nvidia Quadro 600 all tests pass.
>> ---
>>   tests/all.tests                                  |   4 +
>>   tests/spec/CMakeLists.txt                        |   1 +
>>   tests/spec/arb_viewport_array/CMakeLists.gl.txt  |  14 ++
>>   tests/spec/arb_viewport_array/CMakeLists.txt     |   1 +
>>   tests/spec/arb_viewport_array/viewport_indices.c | 252 
>> +++++++++++++++++++++++
>>   5 files changed, 272 insertions(+)
>>   create mode 100644 tests/spec/arb_viewport_array/CMakeLists.gl.txt
>>   create mode 100644 tests/spec/arb_viewport_array/CMakeLists.txt
>>   create mode 100644 tests/spec/arb_viewport_array/viewport_indices.c
>>
>> diff --git a/tests/all.tests b/tests/all.tests
>> index 550729d..e3d64ff 100644
>> --- a/tests/all.tests
>> +++ b/tests/all.tests
>> @@ -1729,6 +1729,10 @@ add_plain_test(arb_vertex_program, 
>> 'vp-address-04')
>>   add_plain_test(arb_vertex_program, 'vp-bad-program')
>>   add_plain_test(arb_vertex_program, 'vp-max-array')
>>
>> +arb_viewport_array = Group()
>> +spec['ARB_viewport_array'] = arb_viewport_array
>> +arb_viewport_array['viewport-indices'] = 
>> concurrent_test('arb_viewport_array-viewport-indices')
>> +
>>   nv_vertex_program = Group()
>>   spec['NV_vertex_program'] = nv_vertex_program
>>   add_vpfpgeneric(nv_vertex_program, 'nv-mov')
>> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
>> index 18b846d..d22d8a4 100644
>> --- a/tests/spec/CMakeLists.txt
>> +++ b/tests/spec/CMakeLists.txt
>> @@ -41,6 +41,7 @@ add_subdirectory (arb_texture_storage_multisample)
>>   add_subdirectory (arb_texture_view)
>>   add_subdirectory (arb_timer_query)
>>   add_subdirectory (arb_transform_feedback2)
>> +add_subdirectory (arb_viewport_array)
>>   add_subdirectory (ati_envmap_bumpmap)
>>   add_subdirectory (ext_fog_coord)
>>   add_subdirectory (ext_framebuffer_multisample)
>> diff --git a/tests/spec/arb_viewport_array/CMakeLists.gl.txt 
>> b/tests/spec/arb_viewport_array/CMakeLists.gl.txt
>> new file mode 100644
>> index 0000000..56ec330
>> --- /dev/null
>> +++ b/tests/spec/arb_viewport_array/CMakeLists.gl.txt
>> @@ -0,0 +1,14 @@
>> +include_directories(
>> +    ${GLEXT_INCLUDE_DIR}
>> +    ${OPENGL_INCLUDE_PATH}
>> +    )
>> +
>> +link_libraries(
>> +    piglitutil_${piglit_target_api}
>> +    ${OPENGL_gl_LIBRARY}
>> +    ${OPENGL_glu_LIBRARY}
>> +    )
>> +
>> +piglit_add_executable(arb_viewport_array-viewport-indices 
>> viewport_indices.c)
>> +
>> +# vim: ft=cmake:
>> diff --git a/tests/spec/arb_viewport_array/CMakeLists.txt 
>> b/tests/spec/arb_viewport_array/CMakeLists.txt
>> new file mode 100644
>> index 0000000..144a306
>> --- /dev/null
>> +++ b/tests/spec/arb_viewport_array/CMakeLists.txt
>> @@ -0,0 +1 @@
>> +piglit_include_target_api()
>> diff --git a/tests/spec/arb_viewport_array/viewport_indices.c 
>> b/tests/spec/arb_viewport_array/viewport_indices.c
>> new file mode 100644
>> index 0000000..986925b
>> --- /dev/null
>> +++ b/tests/spec/arb_viewport_array/viewport_indices.c
>> @@ -0,0 +1,252 @@
>> +/*
>> + * Copyright © 2013 LunarG, Inc.
>> + *
>> + * 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.
>> + *
>> + * Author: Jon Ashburn <jon at lunarg.com>
>> + */
>> +
>> +/**
>> + * Tests GL_ARB_viewport_array regarding the validity for the indices.
>> + * Use both valid and invalid parameters (index, first, count)
>> + * for these new API entry points:
>> + * glViewportArrayv, glViewportIndexedf, glViewportIndexedfv, 
>> glGetFloati_v.
>> + *
>> + * Also test that writing to an invalid viewport index for Viewport, 
>> DepthRange,
>> + * Scissor Box, Scissor Test does not modify any of the state for 
>> the valid
>> + * range of indices.
>> + *
>> + */
>> +
>> +#include "piglit-util-gl-common.h"
>> +#include <stdarg.h>
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> +    config.supports_gl_compat_version = 32;
>> +    config.supports_gl_core_version = 32;
>> +
>> +    config.window_visual = PIGLIT_GL_VISUAL_RGBA | 
>> PIGLIT_GL_VISUAL_DOUBLE;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +/**
>> + * Test that ViewportArrayv, ViewportIndexedf(v), GetFloati_v give the
>> + * "expected_error" gl error.  Given the values for "first" and "count"
>> + * or "index" in range [first, first+count).
>> + */
>> +static bool
>> +check_vp_index(GLuint first, GLsizei count, GLenum expected_error)
>> +{
>> +    GLfloat v[] = {0.2,-2.3, 50.0, 1000.3};
>
> const qualify?
>
>
>> +    GLfloat *mv;
>> +    unsigned int i, numIterate;
>> +
>> +    /* only iterate multiple indices for no error case */
>> +    if (expected_error == GL_NO_ERROR)
>> +        numIterate = count;
>> +    else
>> +        numIterate = 1;
>> +    for (i = count; i > count - numIterate; i--) {
>> +        glViewportIndexedf(first+i-1, v[0], v[1], v[2], v[3]);
>> +        if (!piglit_check_gl_error(expected_error)) {
>> +            printf("error from call to glViewportIndexedf\n");
>> +            return false;
>> +        }
>> +
>> +        glViewportIndexedfv(first+i-1, v);
>> +        if (!piglit_check_gl_error(expected_error)) {
>> +            printf("error from call to glViewportIndexedfv\n");
>> +            return false;
>> +        }
>> +
>> +        glGetFloati_v(GL_VIEWPORT,first+i-1, v);
>
> Get into another array and check that that array's contents == the 
> original 'v'?  I guess you're doing that in a separate test (5/9).
>
>
Yes in test from patch 5.
>> +        if (!piglit_check_gl_error(expected_error)) {
>> +            printf("error from call to glGetFloati_v\n");
>> +            return false;
>> +        }
>> +
>> +    }
>> +
>> +    mv = malloc(sizeof(GLfloat) * 4 * count);
>> +    if (mv == NULL)
>> +        return false;
>> +    for (i =0; i < count; i++) {
>> +        mv[i*4] = v[0];
>> +        mv[i*4 + 1] = v[1];
>> +        mv[i*4 + 2] = v[2];
>> +        mv[i*4 + 3] = v[3];
>> +    }
>> +    glViewportArrayv(first, count, mv);
>> +    free(mv);
>> +    if (!piglit_check_gl_error(expected_error)) {
>> +        printf("error from call to glViewportArrayv\n");
>> +        return false;
>> +    }
>> +
>> +    return true;
>> +}
>> +
>> +/**
>> + * Test first + count or index valid invalid values.
>> + * Valid range is 0 thru (MAX_VIEWPORTS-1).
>> + * Also test the Enable, Disable, IsEnabled  with invalid index.
>> + */
>> +static bool test_vp_indices(GLint maxVP)
>
> static bool
> test_vp_indices(GLint maxVP)
>
>
>> +{
>> +    bool pass = true;
>> +
>> +    if (!piglit_check_gl_error(GL_NO_ERROR)) {
>> +        printf("GL error prior to ViewportArray testing\n");
>> +        return false;
>> +    }
>> +
>> +    /**
>> +     * valid largest range viewport index
>> +     * OpenGL Core 4.3 Spec, section 13.6.1 ref:
>> +     *    "Multiple viewports are available and are numbered zero
>> +     *    through the value of MAX_VIEWPORTS minus one."
>> +     */
>> +    if (!check_vp_index(0, maxVP, GL_NO_ERROR)) {
>> +        printf("Got error for valid viewport range, max range=%u\n",
>> +               maxVP);
>> +        pass = false;
>> +    }
>> +    /**
>> +     *  invalid first index for viewport
>> +     * OpenGL Spec Core 4.3 Spec, section 13.6.1 ref:
>> +     *     "An INVALID_VALUE error is generated if first + count
>> +     *     is greater than the valuue of MAX_VIEWPORTS."
>> +     */
>> +    if (!check_vp_index(maxVP, 1, GL_INVALID_VALUE)) {
>> +        printf("Wrong error for invalid viewport first index\n");
>> +        pass = false;
>> +    }
>> +    /* invalid count + first for viewport */
>> +    if (!check_vp_index(maxVP-1, 2, GL_INVALID_VALUE)) {
>> +        printf("Wrong error for invalid viewport range\n");
>> +        pass = false;
>> +    }
>> +    /**
>> +     * invalid count for viewport
>> +     * OpenGL Spec Core 4.3 Spec, section 13.6.1 ref:
>> +     *    "An INVALID_VALUE error is generated if count is negative."
>> +     */
>> +    glViewportArrayv(0, -1, NULL);
>> +    if (!piglit_check_gl_error(GL_INVALID_VALUE)) {
>> +        printf("Wrong error for invalid viewport count\n");
>> +        pass = false;
>> +    }
>> +
>> +    return pass;
>> +}
>
> Actually, this all seems a bit over-engineered.  I think you could 
> simply call glViewportIndexedf(), etc. once with a single invalid 
> index (probably = maxVP) and check that the right error is generated.
>
> The Mesa code for checking indexes like this will be really simple so 
> I think a simple test is fine.
>
Okay, I think just removing one of the calls to 
check_vp_index(GL_INVALID_VALUE) is appropriate.
Leaving 1) no_error case which iterates over all the valid  first, count 
combinations, 2) first+count is too large  and 3) the call to 
glViewportArray with negative count.
>
>> +
>> +/**
>> + * Test values for viewports, detph_range and scissor boxes/test are 
>> preserved
>
> depth_range
>
>
>> + * with invalid indices.
>> + * OpenGL Core 4.3 Spec, section 13.6.1 ref:
>> + *    "Viewports whose indices lie outside the range [first, first + 
>> count)
>> + *    are not modified."
>> + */
>> +static bool
>> +test_preserve_invalid_index(GLint maxVP)
>> +{
>> +    bool pass = true;
>> +    GLfloat vp[4] = {1.5555, 2.433, 3.777, 4.888};
>
> const here and for the others?
>
>
>> +    GLfloat vpGet[4];
>> +    GLint sc[4] = {3, 9, 17, 23};
>> +    GLint scGet[4];
>> +    GLdouble dr[2] = {0.3333, 0.66666};
>> +    GLdouble drGet[2];
>> +    GLboolean scEnabled;
>> +    int i;
>> +
>> +    /* intialize all indices to know values */
>> +    for (i = 0; i < maxVP; i++) {
>> +        glViewportIndexedfv(i, vp);
>> +        glDepthRangeIndexed(i, dr[0], dr[1]);
>> +        glScissorIndexedv(i, sc);
>> +        glEnablei(GL_SCISSOR_TEST, i);
>> +    }
>> +    pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>> +
>> +    /* set an illegal index and then test that no indices changed*/
>> +    glViewportIndexedf(maxVP, 0.0, 0.0, 1.0, 1.0);
>> +    glScissorIndexed(maxVP, 0, 0, 1, 1);
>> +    glDepthRangeIndexed(maxVP, 0.0, 0.0);
>> +    glDisablei(GL_SCISSOR_TEST, maxVP);
>> +    pass = piglit_check_gl_error(GL_INVALID_VALUE) && pass;
>> +    for (i = 0; i < maxVP; i++) {
>> +        glGetFloati_v(GL_VIEWPORT, i, vpGet);
>> +        if (vpGet[0] != vp[0] || vpGet[1] != vp[1] || vpGet[2] != vp[2]
>> +            || vpGet[3] != vp[3]) {
>> +            printf("Viewport index %d got erroneously changed\n",
>> +                   i);
>> +            pass = false;
>> +            break;
>> +        }
>> +        glGetDoublei_v(GL_DEPTH_RANGE, i, drGet);
>> +        if (drGet[0] != dr[0] || drGet[1] != dr[1]) {
>> +            printf("DepthRange index %d got erroneously changed\n",
>> +                   i);
>> +            pass = false;
>> +            break;
>> +        }
>> +        glGetIntegeri_v(GL_SCISSOR_BOX, i, scGet);
>> +        if (scGet[0] != sc[0] || scGet[1] != sc[1] || scGet[2] != sc[2]
>> +            || scGet[3] != sc[3]) {
>> +            printf("Scissor Box for index %d got erroneously 
>> changed\n",
>> +                   i);
>> +            pass = false;
>> +            break;
>> +        }
>> +        scEnabled = glIsEnabledi(GL_SCISSOR_TEST, i);
>> +        if (scEnabled == GL_FALSE) {
>> +            printf("Scissor Test for index %d got erroneously 
>> changed\n",
>> +                   i);
>> +            pass = false;
>> +            break;
>> +        }
>> +    }
>> +    return pass;
>> +}
>> +
>> +enum piglit_result
>> +piglit_display(void)
>> +{
>> +    return PIGLIT_FAIL;
>> +}
>> +
>> +void
>> +piglit_init(int argc, char **argv)
>> +{
>> +    bool pass = true;
>> +    GLint maxVP;
>> +
>> +    piglit_require_extension("GL_ARB_viewport_array");
>> +
>> +    glGetIntegerv(GL_MAX_VIEWPORTS, &maxVP);
>> +
>> +    pass = test_preserve_invalid_index(maxVP) && pass;
>> +    pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>> +    pass = test_vp_indices(maxVP);
>> +    pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>> +    piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
>> +}
>>
>
> I think my comments here apply to 2 and 3 also.
>
> -Brian
>
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit



More information about the Piglit mailing list