[Piglit] [PATCH] namespace-pollution: Initial framework and test cases for namespace pollution
Ian Romanick
idr at freedesktop.org
Mon Dec 21 11:42:53 PST 2015
On 12/17/2015 02:44 PM, Emil Velikov wrote:
> Hi Ian,
>
> On 17 December 2015 at 02:40, Ian Romanick <idr at freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> See the giant comment in the top of
>> tests/general/object-namespace-pollution.c for more details. Currently
>> only buffer objects and texture objects are supported with glClear and
>> glGenerateMipmap. Addtional objects and operations will be added.
>>
>> Both glClear and glGenerateMipmap fail with buffer objects in Mesa
>> 11.0.6 and earlier. These texture object tests pass.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Cc: Emil Velikov <emil.velikov at collabora.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92363
>> ---
>>
>> I have tried to send this out a couple times, but git-send-mail keeps
>> failing in a way that I don't know how to debug.
>>
>> There will be more subtests here. I want to send the framework out for
>> review sooner rather than later.
>>
>> I do not like the first_unused_texture hack. Comments in the code
>> explain why it is necessary. I believe this will need to be extended to
>> support framebuffer objects as well. An alternative approach is to
>> modify the piglit infrastructure to not use glGenTextures or
>> glGenFramebuffers with -fbo on non-core profile. I'm not convinced
>> that's better.
>>
>> tests/all.py | 8 +
>> tests/general/CMakeLists.gl.txt | 1 +
>> tests/general/object-namespace-pollution.c | 603 +++++++++++++++++++++++++++++
>> 3 files changed, 612 insertions(+)
>> create mode 100644 tests/general/object-namespace-pollution.c
>>
>> diff --git a/tests/all.py b/tests/all.py
>> index e3719ee..cfd298e 100644
>> --- a/tests/all.py
>> +++ b/tests/all.py
>> @@ -4579,5 +4579,13 @@ with profile.group_manager(
>> for sample_count in (str(x) for x in MSAA_SAMPLE_COUNTS):
>> g(['ext_shader_samples_identical', sample_count])
>>
>> +with profile.group_manager(
>> + PiglitGLTest,
>> + grouptools.join('object namespace pollution')) as g:
>> + for object_type in ("buffer", "texture"):
>> + for operation in ("glClear", "glGenerateMipmap"):
>> + g(['object-namespace-pollution', operation, object_type],
>> + '{} with {}'.format(object_type, operation))
>> +
>> if platform.system() is 'Windows':
>> profile.filter_tests(lambda p, _: not p.startswith('glx'))
>> diff --git a/tests/general/CMakeLists.gl.txt b/tests/general/CMakeLists.gl.txt
>> index 298f59c..4b59d1f 100644
>> --- a/tests/general/CMakeLists.gl.txt
>> +++ b/tests/general/CMakeLists.gl.txt
>> @@ -76,6 +76,7 @@ piglit_add_executable (line-flat-clip-color line-flat-clip-color.c)
>> piglit_add_executable (lineloop lineloop.c)
>> piglit_add_executable (longprim longprim.c)
>> piglit_add_executable (masked-clear masked-clear.c)
>> +piglit_add_executable (object-namespace-pollution object-namespace-pollution.c)
>> piglit_add_executable (pos-array pos-array.c)
>> piglit_add_executable (pbo-drawpixels pbo-drawpixels.c)
>> piglit_add_executable (pbo-read-argb8888 pbo-read-argb8888.c)
>> diff --git a/tests/general/object-namespace-pollution.c b/tests/general/object-namespace-pollution.c
>> new file mode 100644
>> index 0000000..1d8859e
>> --- /dev/null
>> +++ b/tests/general/object-namespace-pollution.c
>> @@ -0,0 +1,603 @@
>> +/*
>> + * Copyright © 2015 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 object-namespace-pollution.c
>> + * Verify that the GL implementation does not pollute the object namespace.
>> + *
>> + * At least through Mesa 11.1.0, Mesa drivers that use "meta" have some
>> + * problems with respect to the OpenGL object namespace. Many places inside
>> + * meta allocate objects using a mechanism similar to \c glGen*. This poses
>> + * serious problems for applications that create objects without using the
>> + * associated \c glGen* function (so called "user generated names").
>> + *
>> + * Section 3.8.12 (Texture Objects) of the OpenGL 2.1 (May 16, 2008) spec
>> + * says:
>> + *
>> + * "The command
>> + *
>> + * void GenTextures( sizei n, uint *textures );
>> + *
>> + * returns n previously unused texture object names in textures. These
>> + * names are marked as used, for the purposes of GenTextures only, but
>> + * they acquire texture state and a dimensionality only when they are
>> + * first bound, just as if they were unused."
>> + *
>> + * Calling glBindTexture on an unused name makes that name be used. An
>> + * application can mix user generated names and GL generated names only if it
>> + * is careful not to reuse names that were previously returned by
>> + * glGenTextures. In practice this means that all user generated names must
>> + * be used (i.e., bound) before calling glGenTextures.
>> + *
>> + * This effectively means that the GL implementation (or, realistically, GL
>> + * middleware) is \b never allowed to use glGenTextures because the
>> + * application cannot know what names were returned.
>> + *
>> + * This applies to most kinds of GL objects.
>> + *
>> + * - buffers
>> + * - textures
>> + * - samplers
>> + * - framebuffers
>> + * - renderbuffers
>> + * - queries
>> + * - vertex programs (from GL_ARB_vertex_program)
>> + * - fragment programs (from GL_ARB_fragment_program)
>> + * - vertex arrays (from GL_APPLE_vertex_array_object)
>> + * - fragment shaders (from GL_ATI_fragment_shader)
>> + *
>> + * Many object types (ARB vertex array objects, transform feedback objects,
>> + * program pipeline objects, GLSL shader / program objects, Intel performance
>> + * query objects, etc.) forbid user generated names.
>> + *
>> + * Some object types (NVIDIA or APPLE fences, EXT vertex shaders, NVIDIA
>> + * transform feedback objects) could probably also suffer from this problem.
>> + * However, Mesa does not support these objects, so we don't need to test
>> + * them.
>> + *
>> + * GL_AMD_performance_monitor does not specify whether or not user generated
>> + * names are allowed.
>> + *
>> + * This test attempts to observe this kind of invalid behavior. First a GL
>> + * operation is performed that may need to create an object. Then the test
>> + * creates several objects with user generated names. Finally, the GL
>> + * operations are performed again. If the GL implemented generated new names
>> + * for the purpose of the operations, those names will likely conflict with
>> + * one of the user generated names. This should be observable in one of three
>> + * ways.
>> + *
>> + * - When the test tries to create the object, the object will already exist.
>> + * This is detected by the \c glIs* functions.
>> + *
>> + * - After the second call to the GL operation, the application's object will
>> + * have modified state.
>> + *
>> + * - The second call to the GL operation will fail to perform correctly
>> + * because the application modified its data.
>> + *
>> + * Only the first two methods are employed by this test. This should catch
>> + * the vast majority of possible failures. Verifying the correctness of the
>> + * GL operation would add a lot of complexity to the test.
>> + */
>> +
>> +#include "piglit-util-gl.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> + config.supports_gl_compat_version = 12;
>> +
> Do we want to have these tests in GLES flavour ? Can send a trivial
> patch that does so.
We will want GLES1 so that we can test glDrawTexture. Since a lot of
the functions and object types we want to test don't exist in GLES1,
that's going to require a bunch of #ifdef garbage. I was going to wait
to do that until the very end. It seemed less annoying that way. :)
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +struct enum_value_pair {
>> + GLenum value;
>> + GLint expected;
>> +};
>> +
>> +/**
>> + * Spare objects used by test cases.
>> + *
>> + * Some tests need to use objects for the GL operation begin tested. For
>> + * example, the \c glGenerateMipmap test needs a texture. These objects
>> + * cannot be created using \c glGen* because that would conflict with the rest
>> + * of the test. Instead statically allocate object names starting with some
>> + * high number that we hope the GL won't use or generate during the test.
>> + */
>> +#define FIRST_SPARE_OBJECT 600
>> +
>> +/**
>> + * Linear feedback shift register random number generator
>> + *
>> + * Simple Galois LFSRs that is loosely based on
>> + * https://en.wikipedia.org/wiki/Linear_feedback_shift_register
>> + *
>> + * The value of \c state is updated to reflect the new state of the LFSR.
>> + * This new value should be passed in for the next iteration.
>> + *
>> + * \return
>> + * Either 0 or 1 based on the incoming value of \c state.
>> + */
>> +static unsigned
>> +lfsr(uint16_t *state)
>> +{
>> + unsigned output = *state & 1;
>> +
>> + *state = (*state >> 1) ^ (~output & 0x0000B400);
>> +
>> + return output;
>> +}
>> +
>> +/**
>> + * Fill some memory with pseudorandom values
>> + *
>> + * Using two seed values, a pair of LFSRs are used to generate pseudorandom
>> + * values to fill the specified memory buffer. Seprarate invocations with
>> + * identical \c bytes, \c seed1, and \c seed2 parameters will generate
>> + * identical data. This can be used generate data to initialize a buffer and
>> + * regenerate the same data to validate the buffer.
>> + */
>> +static void
>> +generate_random_data(void *_output, size_t bytes, uint16_t seed1,
>> + uint16_t seed2)
>> +{
>> + uint8_t *output = (uint8_t *) _output;
>> +
>> + for (unsigned i = 0; i < bytes; i++) {
>> + uint8_t byte = 0;
>> +
>> + for (unsigned bit = 0; bit < 8; bit++) {
>> + byte <<= 1;
>> + byte |= lfsr(&seed1) ^ lfsr(&seed2);
>> + }
>> +
>> + output[i] = byte;
>> + }
>> +}
>> +
> [side note] Can imagine that others might will use of these. We can
> get those in piglit util at a later point.
I thought about that. At some point I would like to add a robust set of
random number generators to the piglit infrastructure. LFSRs are easy
(which is why I used them here), but their period is really small. That
may make them unsuitable for a lot of things in piglit.
>> +/** \name Methods for operating on buffer objects */
>> +/*@{*/
>> +#define BUFFER_DATA_SIZE 1024
>> +
>> +static bool
>> +create_buffer(unsigned name, bool silent_skip)
>> +{
>> + uint8_t data[BUFFER_DATA_SIZE];
>> +
>> + if (!piglit_is_extension_supported("GL_ARB_vertex_buffer_object") &&
>> + piglit_get_gl_version() < 14) {
>> + if (silent_skip)
>> + return true;
>> +
>> + printf("%s requires vertex buffer objects.\n", __func__);
>> + piglit_report_result(PIGLIT_SKIP);
>> + }
>> +
>> + generate_random_data(data, sizeof(data), GL_ARRAY_BUFFER, name);
>> +
>> + if (glIsBuffer(name)) {
>> + printf("\t%s,%d: %u is already a buffer\n",
>> + __func__, __LINE__, name);
>> + return false;
>> + }
>> +
>> + glBindBuffer(GL_ARRAY_BUFFER, name);
>> + glBufferData(GL_ARRAY_BUFFER, sizeof(data), data, GL_STATIC_DRAW);
>> + glBindBuffer(GL_ARRAY_BUFFER, 0);
>> +
>> + return piglit_check_gl_error(GL_NO_ERROR);
>> +}
>> +
>> +static bool
>> +validate_buffer(unsigned name, bool silent_skip)
>> +{
>> + static const struct enum_value_pair test_vectors[] = {
>> + { GL_BUFFER_SIZE, BUFFER_DATA_SIZE },
>> + { GL_BUFFER_USAGE, GL_STATIC_DRAW },
>> + { GL_BUFFER_ACCESS, GL_READ_WRITE },
>> + { GL_BUFFER_MAPPED, GL_FALSE },
>> + };
>> + bool pass = true;
>> + uint8_t data[BUFFER_DATA_SIZE];
>> + void *map;
>> +
>> + if (!piglit_is_extension_supported("GL_ARB_vertex_buffer_object") &&
>> + piglit_get_gl_version() < 14) {
>> + if (silent_skip)
>> + return true;
>> +
>> + printf("%s requires vertex buffer objects.\n", __func__);
>> + piglit_report_result(PIGLIT_SKIP);
>> + }
> Unless we expect silent_skip=true in setup and false here we can drop this hunk.
Yeah, that's true.
>> +
>> + if (!glIsBuffer(name)) {
>> + printf("\t%s,%d: %u is not a buffer\n",
>> + __func__, __LINE__, name);
>> + return false;
>> + }
>> +
>> + glBindBuffer(GL_ARRAY_BUFFER, name);
>> +
>> + for (unsigned i = 0; i < ARRAY_SIZE(test_vectors); i++) {
>> + GLint got;
>> +
>> + glGetBufferParameteriv(GL_ARRAY_BUFFER,
>> + test_vectors[i].value,
>> + &got);
>> +
>> + if (got != test_vectors[i].expected) {
>> + printf("\t%s,%d: %s of %u: got 0x%x, expected 0x%x\n",
>> + __func__, __LINE__,
>> + piglit_get_gl_enum_name(test_vectors[i].value),
>> + name, got, test_vectors[i].expected);
>> + pass = false;
>> + }
>> + }
>> +
>> + generate_random_data(data, sizeof(data), GL_ARRAY_BUFFER, name);
>> + map = glMapBuffer(GL_ARRAY_BUFFER, GL_READ_ONLY);
>> + if (map != NULL) {
>> + if (memcmp(map, data, sizeof(data)) != 0) {
>> + printf("\t%s,%d: Data mismatch in %u\n",
>> + __func__, __LINE__, name);
>> + pass = false;
>> + }
>> + } else {
>> + printf("\t%s,%d: Unable to map %u\n",
>> + __func__, __LINE__, name);
>> + pass = false;
>> + }
>> +
>> + glUnmapBuffer(GL_ARRAY_BUFFER);
>> +
>> + pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>> +
>> + return pass;
>> +}
>> +/*@}*/
>> +
>> +/** \name Methods for operating on texture objects */
>> +/*@{*/
>> +#define TEXTURE_DATA_SIZE (16 * 16 * sizeof(GLuint))
>> +
>> +static bool
>> +create_texture(unsigned name, bool silent_skip)
>> +{
>> + uint8_t data[TEXTURE_DATA_SIZE];
>> +
>> + /* Texture objects are always supported, so there is no opportunity to
>> + * skip.
>> + */
>> + (void) silent_skip;
>> +
>> + generate_random_data(data, sizeof(data), GL_TEXTURE_2D, name);
>> +
>> + if (glIsTexture(name)) {
>> + printf("\t%s,%d: %u is already a texture\n",
>> + __func__, __LINE__, name);
>> + return false;
>> + }
>> +
>> + glBindTexture(GL_TEXTURE_2D, name);
>> + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 16, 16, 0, GL_RGBA,
>> + GL_UNSIGNED_INT_8_8_8_8, data);
>> + glBindTexture(GL_TEXTURE_2D, 0);
>> +
>> + return piglit_check_gl_error(GL_NO_ERROR);
>> +}
>> +
>> +static bool
>> +validate_texture(unsigned name, bool silent_skip)
>> +{
>> + static const struct enum_value_pair tex_test_vectors[] = {
>> + { GL_TEXTURE_WRAP_S, GL_REPEAT },
>> + { GL_TEXTURE_WRAP_T, GL_REPEAT },
>> + { GL_TEXTURE_WRAP_R, GL_REPEAT },
>> + { GL_TEXTURE_MIN_FILTER, GL_NEAREST_MIPMAP_LINEAR },
>> + { GL_TEXTURE_MAG_FILTER, GL_LINEAR },
>> + { GL_TEXTURE_BASE_LEVEL, 0 },
>> + { GL_TEXTURE_MAX_LEVEL, 1000 },
>> + };
>> + static const struct enum_value_pair tex_level_test_vectors[] = {
>> + { GL_TEXTURE_WIDTH, 16 },
>> + { GL_TEXTURE_HEIGHT, 16 },
>> + { GL_TEXTURE_INTERNAL_FORMAT, GL_RGBA8 },
>> + };
>> + bool pass = true;
>> + uint8_t data[TEXTURE_DATA_SIZE];
>> + uint8_t texels[TEXTURE_DATA_SIZE];
>> +
>> + /* Texture objects are always supported, so there is no opportunity to
>> + * skip.
>> + */
>> + (void) silent_skip;
>> +
>> + if (!glIsTexture(name)) {
>> + printf("\t%s,%d: %u is not a texture\n",
>> + __func__, __LINE__, name);
>> + return false;
>> + }
>> +
>> + glBindTexture(GL_TEXTURE_2D, name);
>> +
>> + for (unsigned i = 0; i < ARRAY_SIZE(tex_test_vectors); i++) {
>> + GLint got;
>> +
>> + glGetTexParameteriv(GL_TEXTURE_2D,
>> + tex_test_vectors[i].value,
>> + &got);
>> +
>> + if (got != tex_test_vectors[i].expected) {
>> + printf("\t%s,%d: %s of %u: got 0x%x, expected 0x%x\n",
>> + __func__, __LINE__,
>> + piglit_get_gl_enum_name(tex_test_vectors[i].value),
>> + name, got, tex_test_vectors[i].expected);
>> + pass = false;
>> + }
>> + }
>> +
>> + for (unsigned i = 0; i < ARRAY_SIZE(tex_level_test_vectors); i++) {
>> + GLint got;
>> +
>> + glGetTexLevelParameteriv(GL_TEXTURE_2D, 0,
>> + tex_level_test_vectors[i].value,
>> + &got);
>> +
>> + if (got != tex_level_test_vectors[i].expected) {
>> + printf("\t%s,%d: %s of %u: got 0x%x, expected 0x%x\n",
>> + __func__, __LINE__,
>> + piglit_get_gl_enum_name(tex_level_test_vectors[i].value),
>> + name, got, tex_level_test_vectors[i].expected);
>> + pass = false;
>> + }
>> + }
>> +
>> + /* Try to use glGetnTexImageARB. If the test's 16x16 texture was
>> + * replaced with something larger, the call to glGetTexImage will
>> + * probably segfault. This could be worked around, but it doesn't
>> + * seem worth it.
>> + *
>> + * If the texture size did change, the glGetTexLevelParameteriv loop
>> + * above will have already detected it.
>> + */
>> + generate_random_data(data, sizeof(data), GL_TEXTURE_2D, name);
>> + if (piglit_is_extension_supported("GL_ARB_robustness")) {
>> + /* Note: if sizeof(texels) is less than the size of the image,
>> + * glGetnTexImageARB will generate GL_INVALID_OPERATION.
>> + */
>> + glGetnTexImageARB(GL_TEXTURE_2D, 0, GL_RGBA,
>> + GL_UNSIGNED_INT_8_8_8_8,
>> + sizeof(texels), texels);
>> + } else {
>> + glGetTexImage(GL_TEXTURE_2D, 0, GL_RGBA,
>> + GL_UNSIGNED_INT_8_8_8_8,
>> + texels);
>> + }
>> +
>> + if (memcmp(texels, data, sizeof(data)) != 0) {
>> + printf("\t%s,%d: Data mismatch in %u\n",
>> + __func__, __LINE__, name);
>> + pass = false;
>> + }
>> +
>> + glBindTexture(GL_TEXTURE_2D, 0);
>> +
>> + return piglit_check_gl_error(GL_NO_ERROR) && pass;
>> +}
>> +/*@}*/
>> +
>> +/** \name GL operation wrapper functions. */
>> +/*@{*/
>> +static bool
>> +do_Clear(bool silent_skip)
>> +{
>> + /* glClear is always supported, so there is no opportunity to skip. */
>> + (void) silent_skip;
>> +
>> + glClear(GL_COLOR_BUFFER_BIT);
>> + return piglit_check_gl_error(GL_NO_ERROR);
>> +}
>> +
>> +static bool
>> +do_GenerateMipmap(bool silent_skip)
>> +{
>> + const GLuint tex = FIRST_SPARE_OBJECT;
>> + bool pass = true;
>> +
>> + if (!piglit_is_extension_supported("GL_EXT_framebuffer_object") &&
>> + !piglit_is_extension_supported("GL_ARB_framebuffer_object") &&
>> + piglit_get_gl_version() < 30) {
>> + if (silent_skip)
>> + return true;
>> +
>> + printf("%s requires framebuffer objects.\n", __func__);
>> + piglit_report_result(PIGLIT_SKIP);
>> + }
>> +
>> + if (glIsTexture(tex)) {
>> + printf("\t%s,%d: %u is already a texture\n",
>> + __func__, __LINE__, tex);
>> + pass = false;
>> + }
>> +
>> + glBindTexture(GL_TEXTURE_2D, tex);
>> + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 16, 16, 0, GL_RGBA,
>> + GL_UNSIGNED_INT_8_8_8_8, NULL);
>> + glGenerateMipmap(GL_TEXTURE_2D);
>> +
>> + glBindTexture(GL_TEXTURE_2D, 0);
>> + glDeleteTextures(1, &tex);
>> +
>> + return piglit_check_gl_error(GL_NO_ERROR) && pass;
>> +}
>> +/*@}*/
>> +
>> +static const struct {
>> + const char *name;
>> + bool (*func)(bool silent_skip);
>> +} operations[] = {
>> + { "glClear", do_Clear },
>> + { "glGenerateMipmap", do_GenerateMipmap }
>> +};
>> +
>> +static const struct {
>> + const char *name;
>> + bool (*create)(unsigned name, bool silent_skip);
>> + bool (*validate)(unsigned name, bool silent_skip);
>> +} object_types[] = {
>> + { "buffer", create_buffer, validate_buffer },
>> + { "texture", create_texture, validate_texture },
>> +};
>> +
>> +static void
> nit: annotate as NORETURN
Ooh. I didn't know we were doing that in piglit. I'll add that.
>
>> +usage(const char *prog)
>> +{
>> + unsigned i;
>> +
>> + printf("Usage is one of:\n");
>> + printf("\t%s\n", prog);
>> + printf("\t%s operation object-type\n\n", prog);
>> + printf("Where operation is one of:\n");
>> +
>> + for (i = 0; i < ARRAY_SIZE(operations); i++)
>> + printf("\t%s\n", operations[i].name);
>> +
>> + printf("\nAnd object-type is one of:\n");
>> +
>> + for (i = 0; i < ARRAY_SIZE(object_types); i++)
>> + printf("\t%s\n", object_types[i].name);
>> +
>> + piglit_report_result(PIGLIT_FAIL);
>> +}
>> +
>> +void
>> +piglit_init(int argc, char **argv)
>> +{
>> + bool pass = true;
>> + unsigned i;
>> + unsigned first_object_type = 0;
>> + unsigned last_object_type = ARRAY_SIZE(object_types) - 1;
>> + unsigned first_operation = 0;
>> + unsigned last_operation = ARRAY_SIZE(operations) - 1;
>> + bool silent = true;
>> + unsigned first_unused_texture;
>> +
>> + /* The test is either invoked with no command line parameters (in
>> + * which case all test combinations are run) or with an operation name
>> + * and an object type (in which case a single test combination is
>> + * run).
>> + */
>> + if (argc != 1 && argc != 3)
>> + usage(argv[0]);
>> +
>> + if (argc == 3) {
>> + silent = false;
>> +
>> + for (i = 0; i < ARRAY_SIZE(operations); i++) {
>> + if (strcmp(argv[1], operations[i].name) == 0) {
>> + first_operation = i;
>> + last_operation = i;
>> + break;
>> + }
>> + }
>> +
>> + if (i == ARRAY_SIZE(operations))
>> + usage(argv[0]);
>> +
>> + for (i = 0; i < ARRAY_SIZE(object_types); i++) {
>> + if (strcmp(argv[2], object_types[i].name) == 0) {
>> + first_object_type = i;
>> + last_object_type = i;
>> + break;
>> + }
>> + }
>> +
> Fwiw one could replace these first/last with a struct pointers for the
> operation and object.
I'm not sure how you mean. The first_ and last_ values set to 0 and
ARRAY_SIZE() if no command line option is used. If parameters are
specified on the command line, first_ == last_ so that only one test is
run. That simplifies the control logic below.
>> + if (i == ARRAY_SIZE(object_types))
>> + usage(argv[0]);
>> +
>> + printf("Single test case %s with %s\n",
>> + object_types[first_object_type].name,
>> + operations[first_operation].name);
>> + }
>> +
>> + /* This is a bit ugly, but it is necessary. When a test is run with
>> + * -fbo, the piglit framework will create some textures before calling
>> + * piglit_init. These textures will likely have names that conflict
>> + * with the names that are used by this test, so the test should avoid
>> + * them.
>> + *
>> + * HOWEVER, the test should only avoid lower numbered textures. If
>> + * the piglit framework created a texture named 1, the test should
>> + * still try to use a buffer object named 1.
>> + */
>> + for (first_unused_texture = 1;
>> + first_unused_texture < 16;
>> + first_unused_texture++) {
>> + if (!glIsTexture(first_unused_texture))
>> + break;
>> + }
>> +
>> + if (first_unused_texture >= 16)
>> + piglit_report_result(PIGLIT_FAIL);
>> +
>> + for (i = first_operation; i <= last_operation; i++)
>> + pass = operations[i].func(silent) && pass;
>> +
>> + pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>> +
>> + for (i = first_object_type; i <= last_object_type; i++) {
>> + for (unsigned name = 1; name < 16; name++) {
> IMHO here comes the tricky bit(s)
> - 16 does not always cover all the names for every time mentioned in
> the comment above. I'd imagine that we want to check them all ?
That would mean checking 0 to UINT_MAX. I don't think we have that kind
of time. We could test values larger than 16, but I don't know how much
value there would be. Most glGen* functions, and certainly the ones in
Mesa, begin allocating at small values. If a problem hasn't been
detected by the time we get to 16, it seems unlikely that a problem will
be detected (which is different from it being unlikely that there is a
problem).
> To handle this one could just fold the loop within the create/validate
> functions. To avoid rechecking things in validate (and even pass the
> set values to validate) one can use void **state between create and
> validate.
>
> - some object types do not allow binding an object which name did not
> come from the respective glGen* function. Such as samplers,
> framebuffer...
Samplers are framebuffers do not require glGen*. However, transform
feedback objects, ARB vertex array objects, and a few others do. I
listed them in the block comment at the top of the file.
I'm going to have a v2 out later today. That will include some of your
suggested changes and some new tests.
> I'm a bit short on workarounds for this one, considering (if I
> understand correctly) the idea is to avoid glGen at setup/create
> stage.
>
> -Emil
More information about the Piglit
mailing list