Hi, The tests for glNamedFramebufferParameteriEXT and the Get, in my opinion belong with the test jazz for GL_ARB_framebuffe_no_attachments (subject to that it is skipped if GL_EXT_direct_state_access is not present). My thinking is that the extension GL_ARB_framebuffe_no_attachments does define those if the extension GL_EXT_direct_state_access is present.
However, the DSA function without the EXT is defined in the extension ARB_direct_state_access (not the extension GL_ARB_framebuffe_no_attachments). Whether or not it belongs here is a judgment call, though it is definitely easier to be in that test. -Kevin -----Original Message----- From: Palli, Tapani Sent: Tuesday, April 28, 2015 7:42 AM To: Ilia Mirkin Cc: Fredrik Höglund; [email protected]; Rogovin, Kevin Subject: Re: [Piglit] [PATCH v2] arb_framebuffer_no_attachments: add params set&get test On 04/28/2015 07:17 AM, Ilia Mirkin wrote: > On Tue, Apr 28, 2015 at 12:09 AM, Tapani <[email protected]> wrote: >> On 04/27/2015 10:11 PM, Fredrik Höglund wrote: >>> On Wednesday 22 April 2015, Tapani Pälli wrote: >>>> All other tests except invalid_enum_check pass on Nvidia binary >>>> driver (version 346.35), tests also pass currently on upcoming Mesa >>>> implementation. >>>> >>>> v2: >>>> - make test use Core context (for DSA), change test >>>> concurrent (Ilia Mirkin) >>>> - code cleanup, use ARRAY_SIZE (Emil Velikov) >>>> - fix DSA subtest result when skipped >>>> >>>> Signed-off-by: Tapani Pälli <[email protected]> >>>> --- >>>> tests/all.py | 1 + >>>> .../CMakeLists.gl.txt | 1 + >>>> tests/spec/arb_framebuffer_no_attachments/params.c | 298 >>>> +++++++++++++++++++++ >>>> 3 files changed, 300 insertions(+) >>>> create mode 100644 >>>> tests/spec/arb_framebuffer_no_attachments/params.c >>>> >>>> diff --git a/tests/all.py b/tests/all.py index 18124b7..5db2785 >>>> 100755 >>>> --- a/tests/all.py >>>> +++ b/tests/all.py >>>> @@ -2295,6 +2295,7 @@ with profile.group_manager( >>>> PiglitGLTest, >>>> grouptools.join('spec', >>>> 'ARB_framebuffer_no_attachments')) as >>>> g: >>>> g(['arb_framebuffer_no_attachments-minmax'], >>>> run_concurrent=False) >>>> + g(['arb_framebuffer_no_attachments-params']) >>>> # Group ARB_explicit_uniform_location >>>> with profile.group_manager( >>>> diff --git >>>> a/tests/spec/arb_framebuffer_no_attachments/CMakeLists.gl.txt >>>> b/tests/spec/arb_framebuffer_no_attachments/CMakeLists.gl.txt >>>> index 894b95e..53aba8c 100755 >>>> --- a/tests/spec/arb_framebuffer_no_attachments/CMakeLists.gl.txt >>>> +++ b/tests/spec/arb_framebuffer_no_attachments/CMakeLists.gl.txt >>>> @@ -10,3 +10,4 @@ link_libraries ( >>>> ) >>>> piglit_add_executable (arb_framebuffer_no_attachments-minmax >>>> minmax.c) >>>> +piglit_add_executable (arb_framebuffer_no_attachments-params >>>> +params.c) >>>> diff --git a/tests/spec/arb_framebuffer_no_attachments/params.c >>>> b/tests/spec/arb_framebuffer_no_attachments/params.c >>>> new file mode 100644 >>>> index 0000000..577d90c >>>> --- /dev/null >>>> +++ b/tests/spec/arb_framebuffer_no_attachments/params.c >>>> @@ -0,0 +1,298 @@ >>>> +/* >>>> + * 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 params.c >>>> + * >>>> + * Test for set and get functions of framebuffer parameters >>>> +defined by >>>> the >>>> + * GL_ARB_framebuffer_no_attachments specification. >>>> + * >>>> + * There is interaction with following extensions: >>>> + * - EXT_texture_array >>>> + * - EXT_direct_state_access >>>> + */ >>>> + >>>> +#include "piglit-util-gl.h" >>>> + >>>> +PIGLIT_GL_TEST_CONFIG_BEGIN >>>> + >>>> + config.supports_gl_core_version = 31; >>>> + >>>> +PIGLIT_GL_TEST_CONFIG_END >>>> + >>>> +static struct test_t { >>>> + GLenum param; >>>> + GLint default_value; >>>> + GLint value; >>>> + GLint max_value; >>>> + const char *extension; >>>> +} tests[] = { >>>> + { GL_FRAMEBUFFER_DEFAULT_WIDTH, >>>> + 0, 0, GL_MAX_FRAMEBUFFER_WIDTH, NULL }, >>>> + { GL_FRAMEBUFFER_DEFAULT_HEIGHT, >>>> + 0, 0, GL_MAX_FRAMEBUFFER_HEIGHT, NULL }, >>>> + { GL_FRAMEBUFFER_DEFAULT_LAYERS, >>>> + 0, 0, GL_MAX_FRAMEBUFFER_LAYERS, "EXT_texture_array" }, >>>> + { GL_FRAMEBUFFER_DEFAULT_SAMPLES, >>>> + 0, 0, GL_MAX_FRAMEBUFFER_SAMPLES, NULL }, >>>> + { GL_FRAMEBUFFER_DEFAULT_FIXED_SAMPLE_LOCATIONS, >>>> + 0, 0, 0, NULL } >>>> +}; >>>> + >>>> +enum piglit_result >>>> +piglit_display(void) >>>> +{ >>>> + return PIGLIT_FAIL; >>>> +} >>>> + >>>> +static bool >>>> +set_value(GLenum par, GLint val, GLenum target, GLenum error, >>>> +GLuint >>>> fbo) >>>> +{ >>>> + if (fbo) >>>> + glNamedFramebufferParameteriEXT(fbo, par, val); >>>> + else >>>> + glFramebufferParameteri(target, par, val); >>>> + if (!piglit_check_gl_error(error)) >>>> + return false; >>>> + return true; >>>> +} >>>> + >>>> +static bool >>>> +get_value(GLenum par, GLint *val, GLenum target, GLenum error, >>>> +GLuint >>>> fbo) >>>> +{ >>>> + if (fbo) >>>> + glGetNamedFramebufferParameterivEXT(fbo, par, val); >>>> + else >>>> + glGetFramebufferParameteriv(target, par, val); >>>> + if (!piglit_check_gl_error(error)) >>>> + return false; >>>> + return true; >>>> +} >>>> + >>>> +static bool >>>> +test_values(struct test_t *test, GLenum target, GLuint fbo) { >>>> + GLint max = -1; >>>> + GLint value; >>>> + >>>> + /* If has a max value, do range test. */ >>>> + if (test->max_value != 0) >>>> + glGetIntegerv(test->max_value, &max); >>>> + >>>> + if (!piglit_check_gl_error(GL_NO_ERROR)) >>>> + return false; >>>> + >>>> + /* If parameter has no max, it is a boolean type. */ >>>> + if (max == -1) { >>>> + if (!set_value(test->param, GL_TRUE, target, >>>> + GL_NO_ERROR, >>>> fbo)) >>>> + return false; >>>> + >>>> + if (!get_value(test->param, &value, target, >>>> + GL_NO_ERROR, >>>> fbo)) >>>> + return false; >>>> + >>>> + if (value != GL_TRUE) >>>> + return false; >>>> + return true; >>>> + } >>>> + >>>> + /* Parameter has a max, test that values max + 1 and -1 >>>> + give >>>> error */ >>>> + if (!set_value(test->param, max + 1, target, >>>> + GL_INVALID_VALUE, >>>> fbo)) >>>> + return false; >>>> + if (!set_value(test->param, -1, target, GL_INVALID_VALUE, fbo)) >>>> + return false; >>>> + >>>> + /* Valid value (max - 1) for set and get. */ >>>> + if (!set_value(test->param, max - 1, target, GL_NO_ERROR, fbo)) >>>> + return false; >>>> + if (!get_value(test->param, &value, target, GL_NO_ERROR, fbo)) >>>> + return false; >>>> + >>>> + if (value != max - 1) >>>> + return false; >>>> + >>>> + return true; >>>> +} >>>> + >>>> +static bool >>>> +invalid_enum_check() >>>> +{ >>>> + bool pass = true; >>>> + GLint value; >>>> + >>>> + /* Test invalid enums for functions. */ >>>> + if (!set_value(GL_NO_ERROR, 0, GL_FRAMEBUFFER, >>>> + GL_INVALID_ENUM, >>>> 0)) >>>> + pass = false; >>>> + if (!get_value(GL_NO_ERROR, &value, GL_FRAMEBUFFER, >>>> GL_INVALID_ENUM, 0)) >>>> + pass = false; >>>> + >>>> + if (!piglit_is_extension_supported("GL_ARB_direct_state_access")) >>>> + return pass; >>>> + >>>> + /* Test INVALID_VALUE error when fbo given does not exist. */ >>>> + if (!set_value(GL_FRAMEBUFFER_DEFAULT_WIDTH, 42, GL_FRAMEBUFFER, >>>> + GL_INVALID_VALUE, 666)) >>>> + pass = false; >>>> + if (!get_value(GL_FRAMEBUFFER_DEFAULT_WIDTH, &value, >>>> GL_FRAMEBUFFER, >>>> + GL_INVALID_VALUE, 666)) >>>> + pass = false; >>>> + >>>> + piglit_report_subtest_result(pass ? PIGLIT_PASS : PIGLIT_FAIL, >>>> + "invalid enums"); >>>> + >>>> + return pass; >>>> +} >>>> + >>>> +static bool >>>> +dsa_subtest(GLuint fbo) >>>> +{ >>>> + unsigned i; >>>> + bool pass = true; >>>> + >>>> + if >>>> + (!piglit_is_extension_supported("GL_ARB_direct_state_access")) >>>> { >>>> + piglit_report_subtest_result(PIGLIT_SKIP, "dsa"); >>>> + return pass; >>>> + } >>> You are checking for ARB_direct_state_access here, but the functions >>> you are using (glNamedFramebufferParameteriEXT and >>> glGetNamedFramebufferivEXT) belong to EXT_direct_state_access. >> >> True, will change this to GL_EXT since that is enough for this test. > Except Mesa won't support EXT_direct_state_access... Oh ok, so for the modern one then. I guess the support would be just to implement a few functions that call ARB ones? // Tapani _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
