[Piglit] [PATCH] tests/spec/gl-4.5: Added test case for glCreateBuffers

Laura Ekstrand laura at jlekstrand.net
Mon Mar 9 10:28:14 PDT 2015


Hi Aditya,

Thank you for your interest in testing ARB_direct_state_access.  Martin,
Fredrik, and I have been working to implement this extension in Mesa and
generating tests for the new entry points.

I'm confused about the purpose of this test.  It seems that you are trying
to test errors thrown by CreateBuffers as well as the behavior of
CreateBuffers when used with a VAO, but it's not quite clear what you are
trying to accomplish.

Martin and I have developed a style for testing Create[object_name] tests
that checks both error conditions and trivial use cases.  Last week, I sent
out a test to the Piglit ML in this style for glCreateBuffers.  I think our
style makes the function of the test more clear, and I'd encourage you to
take a look at this test as well as the one for CreateFramebuffers,
CreateTextures, and CreateTransformFeedbackObjects.

If you want to chat about this, you can send me an email or chat on IRC (I
am ldeks).

On Sun, Mar 8, 2015 at 5:31 PM, Aditya Atluri <adityaavinash1 at gmail.com>
wrote:

> ---
>  tests/spec/CMakeLists.txt             |   1 +
>  tests/spec/gl-4.5/CMakeLists.gl.txt   |  14 +++++
>  tests/spec/gl-4.5/CMakeLists.txt      |   1 +
>  tests/spec/gl-4.5/gl_create_buffers.c | 100
> ++++++++++++++++++++++++++++++++++
>  4 files changed, 116 insertions(+)
>  create mode 100644 tests/spec/gl-4.5/CMakeLists.gl.txt
>  create mode 100644 tests/spec/gl-4.5/CMakeLists.txt
>  create mode 100644 tests/spec/gl-4.5/gl_create_buffers.c
>
> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
> index 7423589..b3f0fa5 100644
> --- a/tests/spec/CMakeLists.txt
> +++ b/tests/spec/CMakeLists.txt
> @@ -100,6 +100,7 @@ add_subdirectory (gl-3.1)
>  add_subdirectory (gl-3.2)
>  add_subdirectory (gl-3.3)
>  add_subdirectory (gl-4.4)
> +add_subdirectory (gl-4.5)
>  add_subdirectory (gles-2.0)
>  add_subdirectory (gles-3.0)
>  add_subdirectory (glx_arb_create_context)
> diff --git a/tests/spec/gl-4.5/CMakeLists.gl.txt
> b/tests/spec/gl-4.5/CMakeLists.gl.txt
> new file mode 100644
> index 0000000..b85e031
> --- /dev/null
> +++ b/tests/spec/gl-4.5/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 (gl-4.5-create_buffers gl_create_buffers.c)
>
We already have quite a few ARB_direct_state_access tests in
tests/spec/arb_direct_state_access.  Can you put this test there instead?

> +
> +# vim: ft=cmake:
> diff --git a/tests/spec/gl-4.5/CMakeLists.txt
> b/tests/spec/gl-4.5/CMakeLists.txt
> new file mode 100644
> index 0000000..144a306
> --- /dev/null
> +++ b/tests/spec/gl-4.5/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> diff --git a/tests/spec/gl-4.5/gl_create_buffers.c
> b/tests/spec/gl-4.5/gl_create_buffers.c
> new file mode 100644
> index 0000000..e44d9c2
> --- /dev/null
> +++ b/tests/spec/gl-4.5/gl_create_buffers.c
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright (c) 2015 Aditya Atluri <adityaavinash1 at gmail.com>
> + *
> + * 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
> + * on the rights to use, copy, modify, merge, publish, distribute, sub
> + * license, 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
> + * NON-INFRINGEMENT.  IN NO EVENT SHALL AUTHORS AND/OR THEIR SUPPLIERS
> + * 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.
> + */
> +
> +#include "piglit-util-gl.h"
>
Why do you need this include?  I don't see where you use it.

> +#include "minmax-test.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +       config.supports_gl_core_version = 45;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
>
Most of this code does not match Piglit style.  Please review the coding
style in Piglit/HACKING.

> +static bool check_create_buffers(char *function, bool check_valid)
>
+{
> +       bool pass = true;
> +
> +       if (check_valid) {
> +               if (!piglit_check_gl_error(GL_NO_ERROR)) {
> +                       fprintf(stderr, "error when testing valid "
> +                               "glCreateBuffers with %s\n",
> +                               function);
> +                       pass = false;
> +               }
> +       } else {
> +               if (!piglit_check_gl_error(GL_INVALID_VALUE)) {
> +                       fprintf(stderr, "GL_INVALID_VALUE should be
> generated when setting"
> +                               " %s stride too value large than
> MAX_VERTEX_ATTRIB_STRIDE\n",
> +                               function);
> +                       pass = false;
> +               }
> +       }
> +
> +       return pass;
> +}
> +
> +static bool test_create_buffers(GLuint n,
>
Please align ------------------------------^^---------<<

> +                                                       uint *buffers,
> +                                                       bool check_valid){
> +
> +       glCreateBuffers(n, buffers);
> +       if(n > 0){
> +               if(buffers[n-1] != n-1){
> +                       check_valid = false;
> +               }
> +       }else{
> +               check_valid = false;
> +       }
> +       return check_create_buffers("glCreateBuffers", check_valid);
> +}
> +
> +
> +void piglit_init(int argc, char **argv)
> +{
> +       bool pass = true;
> +
> +       GLuint vao;
> +       /* Create and bind a vertex array object, this is needed
> +          for glBindBuffer* tests */
> +       glGenVertexArrays(1, &vao);
> +       glBindVertexArray(vao);
> +
> +       const GLuint num_buffers = 10;
> +
> +       GLuint buf_objs[10];
> +
> +       /* Try passing the number of  value */
> +       pass = test_create_buffers(num_buffers, buf_objs, true) && pass;
> +
> +       /* Try passing a negative value */
> +       pass = test_create_buffers(0-num_buffers, buf_objs, false) && pass;
> +
> +       piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
> +}
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +       return PIGLIT_PASS;
> +}
> --
> 1.9.1
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150309/fdc20353/attachment-0001.html>


More information about the Piglit mailing list