On 04/28/2015 10:42 AM, Rogovin, Kevin wrote:
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.

Yes, I agree here. The original intention was to test the specified interaction with GL_EXT_direct_state_access, I will test this. Also as noted by Arthur, there are different errors specified for EXT vs ARB so these should be separate tests. ARB_direct_state_access tests can be additionally extended to cover these cases.

I'll send a patch in a minute to fix things.

-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

Reply via email to