Hi Grigori, On 19 July 2017 at 22:41, Grigori Goronzy <g...@chown.ath.cx> wrote: > This test verifies context creation with the > EGL_KHR_create_context_no_error extension, which includes interaction > with debug and robustness flags. The test also verifies that the > KHR_no_error mode is successfully enabled with a check of context > flags and glGetError() behavior. > > Both GL 2.0 and GLES2 are tested. > --- > tests/all.py | 2 + > .../egl_khr_create_context/CMakeLists.gles2.txt | 3 + > tests/egl/spec/egl_khr_create_context/no-error.c | 216 > +++++++++++++++++++++ > 3 files changed, 221 insertions(+) > create mode 100644 tests/egl/spec/egl_khr_create_context/no-error.c > > diff --git a/tests/all.py b/tests/all.py > index 4d19da1..4495c90 100644 > --- a/tests/all.py > +++ b/tests/all.py > @@ -4608,6 +4608,8 @@ with profile.test_list.group_manager( > run_concurrent=False) > g(['egl-create-context-valid-flag-debug-gl', 'gl'], 'valid debug flag > GL', > run_concurrent=False) > + g(['egl-create-context-no-error'], 'KHR_no_error enable', > + run_concurrent=False) > I'm assuming that you've copied the run_concurrent=False piece from another? AFAICT there is nothing specific in the test (or in egl-create-context-valid-flag-debug IIRC) that requires it.
> for api in ('gles1', 'gles2', 'gles3'): > g(['egl-create-context-valid-flag-debug-gles', api], > diff --git a/tests/egl/spec/egl_khr_create_context/CMakeLists.gles2.txt > b/tests/egl/spec/egl_khr_create_context/CMakeLists.gles2.txt > index 23bf145..a27ef2d 100644 > --- a/tests/egl/spec/egl_khr_create_context/CMakeLists.gles2.txt > +++ b/tests/egl/spec/egl_khr_create_context/CMakeLists.gles2.txt > @@ -19,4 +19,7 @@ piglit_add_executable > (egl-create-context-invalid-gl-version invalid-gl-version. > piglit_add_executable (egl-create-context-verify-gl-flavor > verify-gl-flavor.c common.c) > piglit_add_executable (egl-create-context-valid-flag-debug-gles > valid-flag-debug.c common.c) > > +# Tests that use ES 2 and Desktop GL. > +piglit_add_executable (egl-create-context-no-error no-error.c common.c) > + > # vim: ft=cmake: > diff --git a/tests/egl/spec/egl_khr_create_context/no-error.c > b/tests/egl/spec/egl_khr_create_context/no-error.c > new file mode 100644 > index 0000000..652c794 > --- /dev/null > +++ b/tests/egl/spec/egl_khr_create_context/no-error.c > @@ -0,0 +1,216 @@ > +/* Copyright 2017 Grigori Goronzy <g...@chown.ath.cx> > + * > + * 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 <stdio.h> > +#include <stdbool.h> > + > +#include "piglit-util-gl.h" > +#include "piglit-util-egl.h" > +#include "common.h" > + > +#define BOOLSTR(x) ((x) ? "yes" : "no") > + > +static void > +fold_results(enum piglit_result *a, enum piglit_result b) > +{ > + if (*a == PIGLIT_FAIL || b == PIGLIT_FAIL) > + *a = PIGLIT_FAIL; > + else if (*a == PIGLIT_PASS || b == PIGLIT_PASS) > + *a = PIGLIT_PASS; > + else > + *a = PIGLIT_SKIP; I haven't seen this kind of construct in piglit before. One option is to let the (sub)test report the status (with piglit_report_subtest_result). While the test overall returns a bool, with true for the PIGLIT_SKIP case. > +} > + > +static void > +check_extension(EGLint mask) > +{ > + if (!EGL_KHR_create_context_setup(mask)) > + piglit_report_result(PIGLIT_SKIP); > + > + piglit_require_egl_extension(egl_dpy, "EGL_KHR_create_context_no_error"); > + Extension requires EGL 1.4 instead of EGL_KHR_create_context or EGL_KHR_surfaceless_context. Although there's no obvious helpers, so I guess this will do for now. > + EGL_KHR_create_context_teardown(); > +} > + > +static enum piglit_result > +check_no_error(EGLenum api, bool no_error, bool debug, bool robust) I don't know if we need the no_error here. The test should check if things work correctly when it's set. Everything else falls outside the scope of the extension. > +{ > + static bool is_dispatch_init = false; > + enum piglit_result pass = PIGLIT_SKIP; > + EGLContext ctx; > + EGLint attribs[11]; > + size_t ai = 0; > + GLint context_flags = 0; > + EGLint mask = (api == EGL_OPENGL_API) ? EGL_OPENGL_BIT : > EGL_OPENGL_ES2_BIT; > + > + printf("info: %s no_error=%s debug=%s robustness=%s\n", > + (api == EGL_OPENGL_API) ? "OpenGL" : "OpenGL ES", > + BOOLSTR(no_error), BOOLSTR(debug), BOOLSTR(robust)); > + > + if (!EGL_KHR_create_context_setup(mask)) > + goto out; > + > + if (eglBindAPI(api) != EGL_TRUE) > + goto out; > + > + if (api == EGL_OPENGL_ES_API) { > + attribs[ai++] = EGL_CONTEXT_CLIENT_VERSION; > + attribs[ai++] = 2; > + } EGL_CONTEXT_CLIENT_VERSION and EGL_CONTEXT_MAJOR_VERSION_KHR are aliases. So I'd drop this hunk. > + if (debug || robust) { I think that one could pass EGL_CONTEXT_FLAGS_KHR 0 to eglCreateContext and things should still work. > + EGLint flags = 0; > + flags |= debug ? EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR : 0; > + flags |= robust ? EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR : 0; Just pass "flags" into the function? This way the attribs[] list become a nice const EGLint attribsp[] = { ... }. > + attribs[ai++] = EGL_CONTEXT_FLAGS_KHR; > + attribs[ai++] = flags; > + } > + /* Always use OpenGL 2.0 or OpenGL ES 2.0 to keep this test reasonably > + * simple; there are enough variants as-is. Another hint - GL_KHR_no_error requires OpenGL{,ES} 2.0 > + */ > + attribs[ai++] = EGL_CONTEXT_MAJOR_VERSION_KHR; > + attribs[ai++] = 2; > + attribs[ai++] = EGL_CONTEXT_MINOR_VERSION_KHR; > + attribs[ai++] = 0; Don't think we need to set minor here - it defaults to 0. > + attribs[ai++] = EGL_CONTEXT_OPENGL_NO_ERROR_KHR; > + attribs[ai++] = no_error ? EGL_TRUE : EGL_FALSE; > + attribs[ai++] = EGL_NONE; > + > + ctx = eglCreateContext(egl_dpy, cfg, EGL_NO_CONTEXT, attribs); > + if (ctx == NULL) { > + if (eglGetError() == EGL_BAD_MATCH) { > + if ((debug || robust) && no_error) { > + /* KHR_no_error doesn't allow the no error mode to be enabled > + * with KHR_debug or ARB_robustness, so context creation is > + * expected to fail in these cases. > + */ > + pass = PIGLIT_PASS; > + goto out; > + } > + > + /* Most likely the API/version is not supported. */ > + pass = PIGLIT_SKIP; > + goto out; > + } > + > + printf("error: context creation failed\n"); > + pass = PIGLIT_FAIL; > + goto out; With the earlier suggestions one could simplify this to if (ctx == NULL) { if (flags && piglit_check_egl_error(EGL_BAD_MATCH)) result = PIGLIT_PASS; goto out; } else if (!piglit_check_egl_error(EGL_BAD_ATTRIBUTE)) // spec said that bad attrib. is set if we feed unknown data via attib_list result = PIGLIT_SKIP; goto out; result = PIGLIT_FAIL; goto out; } > + } > + > + if (!piglit_check_egl_error(EGL_SUCCESS)) { > + printf("error: unexpected EGL error\n"); The helper already does the nicer printing for us. > + pass = PIGLIT_FAIL; > + goto out; > + } > + > + if (eglMakeCurrent(egl_dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, > + ctx) != EGL_TRUE) { > + printf("error: failed to make context current\n"); > + pass = PIGLIT_FAIL; > + goto out; > + } > + > + if (!is_dispatch_init) { > + /* We must postpone initialization of piglit-dispatch until > + * a context is current. > + */ > + piglit_dispatch_default_init(PIGLIT_DISPATCH_GL); > + is_dispatch_init = true; > + } > + > + /* The EGL_KHR_create_context_no_error extension unfortunately allows > + * "no-op" implementations. That is, the EGL extension can be supported > + * without any support on the GL side of things. This means we can't > + * fail if KHR_no_error turns out to be not enabled at this point. > + */ > + if (no_error) { Always true, with above suggestion. > + /* Verify that the KHR_no_error extension is available in the > + * extension list. > + */ > + if (!piglit_is_extension_supported("GL_KHR_no_error")) { > + printf("error: context does not report GL_KHR_no_error " > + "availability\n"); > + pass = PIGLIT_SKIP; > + goto out; > + } > + > + /* Verify that constant flags report KHR_no_error to be enabled. > + * Unfortunately, OpenGL ES does not support the necessary flag until > + * OpenGL ES 3.2, so the check is skipped for ES. We just have to > + * assume that it is supported when the extension appears to be > + * available in the context, according to extension list. > + */ > + if (api == EGL_OPENGL_API) { Requesting GLES2 v2.0 can (should) give you highest compatible version. So we could be using ES 3.2 after all. Thus the above can become if (api == EGL_OPENGL_API || api == EGL_OPENGL_ES2_BIT /* added for posterity */ && piglit_get_gl_version() >= 32) { > + glGetIntegerv(GL_CONTEXT_FLAGS, &context_flags); > + if (!(context_flags & GL_CONTEXT_FLAG_NO_ERROR_BIT_KHR)) { > + printf("error: context does not have GL_KHR_no_error " > + "enabled\n"); > + pass = PIGLIT_SKIP; > + goto out; > + } > + } > + } > + > + /* The number of texture units is a small, unsigned number. Craft an > + * illegal call by using a very large number that should fail on any > + * OpenGL implementation in practice. > + */ > + glActiveTexture(-1); > + if (glGetError() != 0 && no_error) { > + printf("error: error observed with KHR_no_error enabled\n"); With no_error gone, this becomes if (!piglit_check_gl_error(GL_NO_ERROR)) goto failed; > + pass = PIGLIT_FAIL; > + goto out; > + } > + > + /* Everything turned out to be fine */ > + pass = PIGLIT_PASS; > +out: > + printf("info: %s\n", piglit_result_to_string(pass)); > + EGL_KHR_create_context_teardown(); > + return pass; > +} > + > +int main(int argc, char **argv) > +{ > + enum piglit_result pass = PIGLIT_SKIP; > + > + check_extension(EGL_OPENGL_BIT); > + check_extension(EGL_OPENGL_ES2_BIT); > + > + /* "Control group": Check that errors are indeed generated without > + * KHR_no_error enabled. */ > + fold_results(&pass, check_no_error(EGL_OPENGL_API, false, false, false)); > + fold_results(&pass, check_no_error(EGL_OPENGL_ES_API, false, false, > false)); > + fold_results(&pass, check_no_error(EGL_OPENGL_API, false, true, false)); > + fold_results(&pass, check_no_error(EGL_OPENGL_ES_API, false, true, > false)); > + > + /* Check that KHR_no_error gets enabled and its interaction with debug > and > + * robustness context flags. */ > + fold_results(&pass, check_no_error(EGL_OPENGL_API, true, false, false)); > + fold_results(&pass, check_no_error(EGL_OPENGL_ES_API, true, false, > false)); > + fold_results(&pass, check_no_error(EGL_OPENGL_API, true, true, false)); > + fold_results(&pass, check_no_error(EGL_OPENGL_ES_API, true, true, > false)); > + fold_results(&pass, check_no_error(EGL_OPENGL_API, true, false, true)); > + fold_results(&pass, check_no_error(EGL_OPENGL_API, true, true, true)); > + With the earlier suggestions in place the function becomes bool pass = true; check_extension(EGL_OPENGL_BIT); check_extension(EGL_OPENGL_ES2_BIT); pass = check_no_error(EGL_OPENGL_API, 0) && pass; pass = check_no_error(EGL_OPENGL_API, EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR) && pass; pass = check_no_error(EGL_OPENGL_API, EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR) && pass; pass = check_no_error(EGL_OPENGL_API, EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR | EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR) && pass; pass = check_no_error(EGL_OPENGL_ES_API, 0) && pass; pass = check_no_error(EGL_OPENGL_ES_API, EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR) && pass; piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL); HTH Emil _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit