[Piglit] [PATCH 2/2] query_renderer: Add test that compares values from glXQueryRendererIntegerMESA et al
Ian Romanick
idr at freedesktop.org
Sat Nov 23 13:51:12 PST 2013
On 11/11/2013 01:57 PM, Paul Berry wrote:
> On 25 October 2013 11:13, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>
> From: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at intel.com>>
>
> Verifies that all values from glXQueryRendererIntegerMESA,
> glXQueryCurrentRendererIntegerMESA, glXQueryRendererStringMESA, and
> glXQueryCurrentRendererStringMESA can be queried. It also verifies that
> each of the Current and non-Current versions returns the same value for
> each query.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at intel.com>>
> ---
> This test currently fails on Mesa master with the
> GLX_MESA_query_renderer patches on the mesa-dev list. It appears that
> Adam's d101204 commit breaks some calls to glXMakeContextCurrent. I'm
> tracking down the root cause of that problem now. With that patch
> reverted, everything here passes.
>
> tests/all.tests | 4 +
> tests/spec/CMakeLists.txt | 1 +
> .../spec/glx_mesa_query_renderer/CMakeLists.gl.txt | 28 +++
> tests/spec/glx_mesa_query_renderer/CMakeLists.txt | 1 +
> tests/spec/glx_mesa_query_renderer/coverage.c | 275
> +++++++++++++++++++++
> 5 files changed, 309 insertions(+)
> create mode 100644 tests/spec/glx_mesa_query_renderer/CMakeLists.gl.txt
> create mode 100644 tests/spec/glx_mesa_query_renderer/CMakeLists.txt
> create mode 100644 tests/spec/glx_mesa_query_renderer/coverage.c
>
> diff --git a/tests/all.tests b/tests/all.tests
> index e5e63e4..1d2a916 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -468,6 +468,10 @@ oml_sync_control['swapbuffersmsc-return
> swap_interval 0'] = concurrent_test('glx
> oml_sync_control['swapbuffersmsc-return swap_interval 1'] =
> concurrent_test('glx-oml-sync-control-swapbuffersmsc-return 1')
> oml_sync_control['waitformsc'] =
> concurrent_test('glx-oml-sync-control-waitformsc')
>
> +mesa_query_renderer = Group()
> +glx['GLX_MESA_query_renderer'] = mesa_query_renderer
> +mesa_query_renderer['coverage'] =
> concurrent_test('glx-query-renderer-coverage')
> +
> def texwrap_test(args):
> test = PlainExecTest(['texwrap', '-fbo', '-auto'] + args)
> test.runConcurrent = True
> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
> index 18b846d..a2903d8 100644
> --- a/tests/spec/CMakeLists.txt
> +++ b/tests/spec/CMakeLists.txt
> @@ -77,6 +77,7 @@ add_subdirectory (gles-2.0)
> add_subdirectory (gles-3.0)
> add_subdirectory (glx_arb_create_context)
> add_subdirectory (glx_ext_import_context)
> +add_subdirectory (glx_mesa_query_renderer)
> add_subdirectory (glx_oml_sync_control)
> add_subdirectory (arb_vertex_type_2_10_10_10_rev)
> add_subdirectory (ext_texture_array)
> diff --git a/tests/spec/glx_mesa_query_renderer/CMakeLists.gl.txt
> b/tests/spec/glx_mesa_query_renderer/CMakeLists.gl.txt
> new file mode 100644
> index 0000000..f51ceff
> --- /dev/null
> +++ b/tests/spec/glx_mesa_query_renderer/CMakeLists.gl.txt
> @@ -0,0 +1,28 @@
> +
> +include_directories(
> + ${GLEXT_INCLUDE_DIR}
> + ${OPENGL_INCLUDE_PATH}
> +)
> +
> +if(PIGLIT_BUILD_GLX_TESTS)
> + link_libraries (
> + piglitglxutil
> + )
> +endif(PIGLIT_BUILD_GLX_TESTS)
> +
> +link_libraries (
> + ${OPENGL_gl_LIBRARY}
> + ${OPENGL_glu_LIBRARY}
> +)
> +
> +IF(PIGLIT_BUILD_GLX_TESTS)
> + include_directories(
> + ${GLPROTO_INCLUDE_DIRS}
> + )
> + link_libraries (
> + ${X11_X11_LIB}
> + )
> + piglit_add_executable (glx-query-renderer-coverage
> coverage.c query-renderer-common.c)
> +ENDIF(PIGLIT_BUILD_GLX_TESTS)
> +
> +# vim: ft=cmake:
> diff --git a/tests/spec/glx_mesa_query_renderer/CMakeLists.txt
> b/tests/spec/glx_mesa_query_renderer/CMakeLists.txt
> new file mode 100644
> index 0000000..144a306
> --- /dev/null
> +++ b/tests/spec/glx_mesa_query_renderer/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> diff --git a/tests/spec/glx_mesa_query_renderer/coverage.c
> b/tests/spec/glx_mesa_query_renderer/coverage.c
> new file mode 100644
> index 0000000..2f5367c
> --- /dev/null
> +++ b/tests/spec/glx_mesa_query_renderer/coverage.c
> @@ -0,0 +1,275 @@
> +/* 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.
> + */
> +#include "piglit-util-gl-common.h"
> +#include "piglit-glx-util.h"
> +#include "query-renderer-common.h"
> +
> +struct test_vector {
> + const char *name_string;
> + int attribute;
> + int value_count;
> +};
> +
> +#define ENUM(name, count) { # name, name, count }
> +
> +static const struct test_vector all_valid_integer_enums[] = {
> + ENUM(GLX_RENDERER_VENDOR_ID_MESA, 1),
> + ENUM(GLX_RENDERER_DEVICE_ID_MESA, 1),
> + ENUM(GLX_RENDERER_VERSION_MESA, 3),
> + ENUM(GLX_RENDERER_ACCELERATED_MESA, 1),
> + ENUM(GLX_RENDERER_VIDEO_MEMORY_MESA, 1),
> + ENUM(GLX_RENDERER_UNIFIED_MEMORY_ARCHITECTURE_MESA, 1),
> + ENUM(GLX_RENDERER_PREFERRED_PROFILE_MESA, 1),
> + ENUM(GLX_RENDERER_OPENGL_CORE_PROFILE_VERSION_MESA, 2),
> +
> ENUM(GLX_RENDERER_OPENGL_COMPATIBILITY_PROFILE_VERSION_MESA, 2),
> + ENUM(GLX_RENDERER_OPENGL_ES_PROFILE_VERSION_MESA, 2),
> + ENUM(GLX_RENDERER_OPENGL_ES2_PROFILE_VERSION_MESA, 2),
> +};
> +
> +static const struct test_vector all_valid_string_enums[] = {
> + ENUM(GLX_RENDERER_VENDOR_ID_MESA, 0),
> + ENUM(GLX_RENDERER_DEVICE_ID_MESA, 0),
> +};
> +
> +static bool
> +verify_integer_values(const char *name, Bool success,
> + const struct test_vector *test,
> + const unsigned *buffer, unsigned buffer_size,
> + bool silent)
> +{
> + char text[512];
> + unsigned text_size;
> + bool pass = true;
> + unsigned j;
> +
> + if (!success) {
> + fprintf(stderr, "%s(%s) failed.\n", name,
> test->name_string);
> +
> + /* If the call failed, don't bother checking that
> the correct
> + * number of values were written.
> + */
> + return false;
> + }
> +
> + if (!silent) {
> + text_size = snprintf(text, sizeof(text), "%s(%s)
> values:\n ",
> + name,
> + test->name_string);
> + for (j = 0; j < test->value_count; j++) {
> + text_size += snprintf(&text[text_size],
> + sizeof(text) - text_size,
> + "%d ",
> + buffer[j]);
> + }
> +
> + printf("%s\n", text);
> + }
> +
> + for (j = 0; j < test->value_count; j++) {
> + if (buffer[j] == 0xDEADBEEF) {
> + fprintf(stderr,
> + "%s(%s) only wrote %d values,
> expected %d.\n",
> + name,
> + test->name_string,
> + j + 1,
>
> This should just be "j".
Ah, good catch.
> + test->value_count);
> + pass = false;
> + break;
> + }
> + }
> +
> + for (j = test->value_count; j < buffer_size; j++) {
> + if (buffer[j] != 0xDEADBEEF) {
> + fprintf(stderr,
> + "%s(%s) only wrote %d values,
> expected %d.\n",
> + name,
> + test->name_string,
> + j + 1,
> + test->value_count);
>
>
> This is a misleading message, since you're not actually measuring the
> number of values that got written--you're just checking whether the
> values that *shouldn't* have been written are preserved. How about
> printf("%s(%s) wrote more than %d values.", ..., test->value_count)?
Yeah... that was a copy-and-paste bug.
> + pass = false;
> + break;
> + }
> + }
>
>
> Alternatively, you might want to consider restructuring this code so
> that it measures the number of values that got written, and then
> displays an error if that value doesn't equal test->value_count.
I'm going to leave the logic as-is for now. There is a particular case
that I want to catch, and I'm not sure of another way to do it cleanly...
Say was have some function that we expect to write 5 values, 0 through
4. If this function writes values 0, 1, 2, 3, and 8, I want to emit two
errors: the function didn't write the data that it should have, and it
wrote some data that it should not have. That's why I have the two
separate loops. The first loop catches the first case, and the second
loop catches the second.
I use this idiom often. Perhaps we can come up with a better way to
implement it, and I can put it in a shared place. I'll submit follow-on
patches to make this and other tests use that.
> +
> + return pass;
> +}
>
>
> With that addressed, this patch is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>
More information about the Piglit
mailing list