Hey Emil,
On 2017-08-02 18:01, Emil Velikov wrote:
Hi Grigori,
+++ 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.
Yes, that's true. A bit of cargo culting I guess. I'll change it.
+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.
I actually derived this from some other test (don't remember which one).
piglit_report_subtest_result sounds like a better choice, I didn't know
about it. Thanks for pointing it out.
+}
+
+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.
Maybe I should add a helper, I'll look if this is easily possible.
+ 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.
I added testing with no-error==false because there were some issues with
the EGL implementation of the no-error extension, which then switched on
no-error mode if any context flag was set. Some other tests *might* also
catch such issues, but here's the obvious place. Anyway, that's the
motivation for doing these "control tests". What do you think?
+{
+ 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.
Ok.
+ 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[] = {
... }.
Yep, that's easier.
+ 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
OK, I'll add a check to validate that the implementation doesn't accept
no-error for GL/ES 1.x.
+ 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) {
Interesting, I didn't know about it. That might make the check more
reliable on modern implementations, which is good.
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);
Looks good.
Thanks for your feedback!
Grigori
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
