On 12/17/2015 02:44 PM, Emil Velikov wrote: > Hi Ian, > > On 17 December 2015 at 02:40, Ian Romanick <i...@freedesktop.org> wrote: >> From: Ian Romanick <ian.d.roman...@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.roman...@intel.com> >> Cc: Emil Velikov <emil.veli...@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 _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit