[PATCH weston v2] tests: Remove buffer-count

Derek Foreman derekf at osg.samsung.com
Fri Jan 27 14:59:14 UTC 2017


On 27/01/17 08:56 AM, Daniel Stone wrote:
> buffer-count was introduced in line with a Mesa change which forced
> an earlier block on frame events to try to enforce double-buffering
> where available.
>
> The Mesa change has since been reverted (Mesa commit 9ca6711faa), as
> this had unpleasant interactions with buffer_age in particular, so this
> test is no longer valid.
>
> Additionally, it only worked on backends which initialised EGL (not
> headless-backend, where tests generally run), which can be flaky due to
> initialisation races. Not only that, but on the DRM backend, we can
> legitimately enter triple-buffering due to promoting the surface to a
> hardware plane, skipping GPU composition.
>
> In light of all this, just remove the test.
>
> Signed-off-by: Daniel Stone <daniels at collabora.com>

Yeah, seems reasonable.

Reviewed-by: Derek Foreman <derekf at osg.samsung.com>

> ---
>  Makefile.am                       |   7 --
>  protocol/weston-test.xml          |   7 --
>  tests/buffer-count-test.c         | 172 --------------------------------------
>  tests/weston-test-client-helper.c |  20 -----
>  tests/weston-test-client-helper.h |   3 -
>  tests/weston-test.c               |  42 ----------
>  6 files changed, 251 deletions(-)
>  delete mode 100644 tests/buffer-count-test.c
>
> v2: Remove stray hunk to weston-tests-env.
>
> diff --git a/Makefile.am b/Makefile.am
> index fd0a2a1c4..ea1886e33 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1387,13 +1387,6 @@ nodist_viewporter_weston_SOURCES =		\
>  viewporter_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
>  viewporter_weston_LDADD = libtest-client.la
>
> -if ENABLE_EGL
> -weston_tests += buffer-count.weston
> -buffer_count_weston_SOURCES = tests/buffer-count-test.c
> -buffer_count_weston_CFLAGS = $(AM_CFLAGS) $(EGL_TESTS_CFLAGS) $(TEST_CLIENT_CFLAGS)
> -buffer_count_weston_LDADD = libtest-client.la $(EGL_TESTS_LIBS)
> -endif
> -
>  if ENABLE_XWAYLAND_TEST
>  weston_tests +=	xwayland-test.weston
>  xwayland_test_weston_SOURCES = tests/xwayland-test.c
> diff --git a/protocol/weston-test.xml b/protocol/weston-test.xml
> index 587f0cf78..74a152146 100644
> --- a/protocol/weston-test.xml
> +++ b/protocol/weston-test.xml
> @@ -64,13 +64,6 @@
>        <arg name="x" type="fixed"/>
>        <arg name="y" type="fixed"/>
>      </event>
> -    <request name="get_n_egl_buffers">
> -      <!-- causes a n_egl_buffers event to be sent which reports how many
> -           buffer objects are live for the client -->
> -    </request>
> -    <event name="n_egl_buffers">
> -      <arg name="n" type="uint"/>
> -    </event>
>      <request name="capture_screenshot">
>        <description summary="records current screen image">
>          Records an image of what is currently displayed on a given
> diff --git a/tests/buffer-count-test.c b/tests/buffer-count-test.c
> deleted file mode 100644
> index 0df977405..000000000
> --- a/tests/buffer-count-test.c
> +++ /dev/null
> @@ -1,172 +0,0 @@
> -/*
> - * Copyright © 2013 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.
> - */
> -
> -#include "config.h"
> -
> -#include <string.h>
> -#include <stdint.h>
> -#include <stdio.h>
> -#include <EGL/egl.h>
> -#include <wayland-egl.h>
> -#include <GLES2/gl2.h>
> -
> -#include "weston-test-client-helper.h"
> -#include "shared/platform.h"
> -
> -#define fail(msg) { fprintf(stderr, "%s failed\n", msg); return -1; }
> -
> -struct test_data {
> -	struct client *client;
> -	struct wl_egl_window *native_window;
> -
> -	EGLDisplay egl_dpy;
> -	EGLContext egl_ctx;
> -	EGLConfig egl_conf;
> -	EGLSurface egl_surface;
> -};
> -
> -static int
> -init_egl(struct test_data *test_data)
> -{
> -	struct surface *surface = test_data->client->surface;
> -	const char *str, *mesa;
> -
> -	static const EGLint context_attribs[] = {
> -		EGL_CONTEXT_CLIENT_VERSION, 2,
> -		EGL_NONE
> -	};
> -
> -	EGLint config_attribs[] = {
> -		EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
> -		EGL_RED_SIZE, 1,
> -		EGL_GREEN_SIZE, 1,
> -		EGL_BLUE_SIZE, 1,
> -		EGL_ALPHA_SIZE, 0,
> -		EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
> -		EGL_NONE
> -	};
> -
> -	EGLint major, minor, n;
> -	EGLBoolean ret;
> -
> -	test_data->egl_dpy =
> -		weston_platform_get_egl_display(EGL_PLATFORM_WAYLAND_KHR,
> -						test_data->client->wl_display,
> -						NULL);
> -	if (!test_data->egl_dpy)
> -		fail("eglGetPlatformDisplay or eglGetDisplay");
> -
> -	if (eglInitialize(test_data->egl_dpy, &major, &minor) != EGL_TRUE)
> -		fail("eglInitialize");
> -	if (eglBindAPI(EGL_OPENGL_ES_API) != EGL_TRUE)
> -		fail("eglBindAPI");
> -
> -	ret = eglChooseConfig(test_data->egl_dpy, config_attribs,
> -			      &test_data->egl_conf, 1, &n);
> -	if (!(ret && n == 1))
> -		fail("eglChooseConfig");
> -
> -	test_data->egl_ctx = eglCreateContext(test_data->egl_dpy,
> -					      test_data->egl_conf,
> -					      EGL_NO_CONTEXT, context_attribs);
> -	if (!test_data->egl_ctx)
> -		fail("eglCreateContext");
> -
> -	test_data->native_window =
> -		wl_egl_window_create(surface->wl_surface,
> -				     surface->width,
> -				     surface->height);
> -	test_data->egl_surface =
> -		weston_platform_create_egl_surface(test_data->egl_dpy,
> -						   test_data->egl_conf,
> -						   test_data->native_window,
> -						   NULL);
> -
> -	ret = eglMakeCurrent(test_data->egl_dpy, test_data->egl_surface,
> -			     test_data->egl_surface, test_data->egl_ctx);
> -	if (ret != EGL_TRUE)
> -		fail("eglMakeCurrent");
> -
> -	/* This test is specific to mesa 10.1 and later, which is the
> -	 * first release that doesn't accidentally triple-buffer. */
> -	str = (const char *) glGetString(GL_VERSION);
> -	mesa = strstr(str, "Mesa ");
> -	if (mesa == NULL)
> -		skip("unknown EGL implementation (%s)\n", str);
> -	if (sscanf(mesa + 5, "%d.%d", &major, &minor) != 2)
> -		skip("unrecognized mesa version (%s)\n", str);
> -	if (major < 10 || (major == 10 && minor < 1))
> -		skip("mesa version too old (%s)\n", str);
> -
> -	return 0;
> -}
> -
> -static void
> -fini_egl(struct test_data *test_data)
> -{
> -	eglMakeCurrent(test_data->egl_dpy, EGL_NO_SURFACE, EGL_NO_SURFACE,
> -		       EGL_NO_CONTEXT);
> -	weston_platform_destroy_egl_surface(test_data->egl_dpy,
> -					    test_data->egl_surface);
> -	wl_egl_window_destroy(test_data->native_window);
> -	eglTerminate(test_data->egl_dpy);
> -}
> -
> -TEST(test_buffer_count)
> -{
> -	struct test_data test_data;
> -	uint32_t buffer_count;
> -	int i;
> -
> -	test_data.client = create_client_and_test_surface(10, 10, 10, 10);
> -	if (!test_data.client->has_wl_drm)
> -		skip("compositor has not bound its display to EGL\n");
> -
> -	if (init_egl(&test_data) < 0)
> -		skip("could not initialize egl, "
> -		     "possibly using the headless backend\n");
> -
> -	/* This is meant to represent a typical game loop which is
> -	 * expecting eglSwapBuffers to block and throttle the
> -	 * rendering to a sensible frame rate. Therefore it doesn't
> -	 * expect to have to install a frame callback itself. I'd
> -	 * imagine this is what a typical SDL game would end up
> -	 * doing */
> -
> -	for (i = 0; i < 10; i++) {
> -		glClear(GL_COLOR_BUFFER_BIT);
> -		eglSwapBuffers(test_data.egl_dpy, test_data.egl_surface);
> -	}
> -
> -	buffer_count = get_n_egl_buffers(test_data.client);
> -
> -	printf("buffers used = %i\n", buffer_count);
> -
> -	/* The implementation should only end up creating two buffers
> -	 * and cycling between them */
> -	assert(buffer_count == 2);
> -
> -	fini_egl(&test_data);
> -}
> diff --git a/tests/weston-test-client-helper.c b/tests/weston-test-client-helper.c
> index fd6ebaf8c..7957094fd 100644
> --- a/tests/weston-test-client-helper.c
> +++ b/tests/weston-test-client-helper.c
> @@ -115,17 +115,6 @@ move_client(struct client *client, int x, int y)
>  	frame_callback_wait(client, &done);
>  }
>
> -int
> -get_n_egl_buffers(struct client *client)
> -{
> -	client->test->n_egl_buffers = -1;
> -
> -	weston_test_get_n_egl_buffers(client->test->weston_test);
> -	wl_display_roundtrip(client->wl_display);
> -
> -	return client->test->n_egl_buffers;
> -}
> -
>  static void
>  pointer_handle_enter(void *data, struct wl_pointer *wl_pointer,
>  		     uint32_t serial, struct wl_surface *wl_surface,
> @@ -517,14 +506,6 @@ test_handle_pointer_position(void *data, struct weston_test *weston_test,
>  }
>
>  static void
> -test_handle_n_egl_buffers(void *data, struct weston_test *weston_test, uint32_t n)
> -{
> -	struct test *test = data;
> -
> -	test->n_egl_buffers = n;
> -}
> -
> -static void
>  test_handle_capture_screenshot_done(void *data, struct weston_test *weston_test)
>  {
>  	struct test *test = data;
> @@ -535,7 +516,6 @@ test_handle_capture_screenshot_done(void *data, struct weston_test *weston_test)
>
>  static const struct weston_test_listener test_listener = {
>  	test_handle_pointer_position,
> -	test_handle_n_egl_buffers,
>  	test_handle_capture_screenshot_done,
>  };
>
> diff --git a/tests/weston-test-client-helper.h b/tests/weston-test-client-helper.h
> index 8908ae7e6..a288af7ea 100644
> --- a/tests/weston-test-client-helper.h
> +++ b/tests/weston-test-client-helper.h
> @@ -182,9 +182,6 @@ frame_callback_wait_nofail(struct client *client, int *done);
>
>  #define frame_callback_wait(c, d) assert(frame_callback_wait_nofail((c), (d)))
>
> -int
> -get_n_egl_buffers(struct client *client);
> -
>  void
>  skip(const char *fmt, ...);
>
> diff --git a/tests/weston-test.c b/tests/weston-test.c
> index 2237c2efd..0123e9946 100644
> --- a/tests/weston-test.c
> +++ b/tests/weston-test.c
> @@ -244,47 +244,6 @@ device_add(struct wl_client *client,
>  	}
>  }
>
> -#ifdef ENABLE_EGL
> -static int
> -is_egl_buffer(struct wl_resource *resource)
> -{
> -	PFNEGLQUERYWAYLANDBUFFERWL query_buffer =
> -		(void *) eglGetProcAddress("eglQueryWaylandBufferWL");
> -	EGLint format;
> -
> -	if (query_buffer(eglGetCurrentDisplay(),
> -			 resource,
> -			 EGL_TEXTURE_FORMAT,
> -			 &format))
> -		return 1;
> -
> -	return 0;
> -}
> -#endif /* ENABLE_EGL */
> -
> -static void
> -get_n_buffers(struct wl_client *client, struct wl_resource *resource)
> -{
> -	int n_buffers = 0;
> -
> -#ifdef ENABLE_EGL
> -	struct wl_resource *buffer_resource;
> -	int i;
> -
> -	for (i = 0; i < 1000; i++) {
> -		buffer_resource = wl_client_get_object(client, i);
> -
> -		if (buffer_resource == NULL)
> -			continue;
> -
> -		if (is_egl_buffer(buffer_resource))
> -			n_buffers++;
> -	}
> -#endif /* ENABLE_EGL */
> -
> -	weston_test_send_n_egl_buffers(resource, n_buffers);
> -}
> -
>  enum weston_test_screenshot_outcome {
>  	WESTON_TEST_SCREENSHOT_SUCCESS,
>  	WESTON_TEST_SCREENSHOT_NO_MEMORY,
> @@ -535,7 +494,6 @@ static const struct weston_test_interface test_implementation = {
>  	send_key,
>  	device_release,
>  	device_add,
> -	get_n_buffers,
>  	capture_screenshot,
>  };
>
>



More information about the wayland-devel mailing list