[Piglit] [PATCH] Test calling glViewport while using EGL_KHR_surfaceless_context

[Neil Roberts]

Ian Romanick <idr at freedesktop.org> writes:

>> As far as I can tell this is the first test for surfaceless contexts
>> so it also adds the directory and the test group.
>
> There are a couple other tests that use surfaceless contexts
> (tests/egl/spec/egl-1.4/egl-terminate-then-unbind-context.c and
> implicitly all the tests in tests/egl/spec/egl_khr_create_context), but
> this is the right thing for this test.

Ah ok, I'll add the word ‘explicit’ in there.

>> +static void
>> +check_extensions(void)
>> +{
>> +	const char *extension_string = eglQueryString(egl_dpy, EGL_EXTENSIONS);
>> +	static const char surfaceless_extension[] =
>> +		"EGL_KHR_surfaceless_context";
>> +
>> +	if (!strstr(extension_string, surfaceless_extension)) {
>
> Use piglit_require_egl_extension.  strstr() is known to not be a valid
> way to check extensions due to the possibility of false positives.
>
> You can also CC me on a patch that fixes
> tests/egl/spec/egl_khr_create_context/common.c... which is probably
> where this came from ;)

Ok, will do.

>> +	egl_dpy = eglGetDisplay(dpy);
>> +	if (egl_dpy == EGL_NO_DISPLAY) {
>> +		fprintf(stderr, "eglGetDisplay() failed\n");
>> +		piglit_report_result(PIGLIT_FAIL);
>> +	}
>
> I think this should use piglit_egl_get_default_display, but it probably
> doesn't make too much difference.  I'm not that familiar with EGL, so I
> might be completely wrong...

Good idea, thanks. I've changed it to just call
eglGetDisplay(EGL_DEFAULT_DISPLAY) directly because that is all
piglit_egl_get_default_display does if you call it without specifying a
platform anyway and there are other tests that do this.

>> +	if (!ctx) {
>> +		if (piglit_check_egl_error(EGL_BAD_MATCH)) {
>> +			piglit_report_result(PIGLIT_SKIP);
>> +		} else {
>> +			piglit_report_result(PIGLIT_FAIL);
>> +		}
>
> I'd usually prefer this sort of thing to use ?:, but meh.

Ok.

>> +	piglit_report_result(PIGLIT_PASS);
>> +
>> +	abort();
>> +	return EXIT_FAILURE;
>
> Eh... left over debug cruft?

This was in the test that I copied from and I thought it was probably
trying to make sure that piglit_report_result doesn't return. But yeah,
it's a bit weird so I'll just take it out. piglit_report_result is
marked as NORETURN so it doesn't warn if I also take out the return.

Thanks for looking at the patch.

Regards,
- Neil