[PATCH weston] tests: Test whether a simple EGL main loop uses too many buffers

Neil Roberts neil at linux.intel.com
Thu Dec 5 11:22:19 PST 2013


Hi,

Here is a rebased version of the patch. The old one didn't apply
cleanly anymore because of some changes in tests/Makefile.am

I think there is some confusion about why it ends up using 3 buffers
and I also keep getting in a muddle about it. I'll try to describe
what I think is happening here.

The client has a loop like this:

while (1) {
        draw_something();
        eglSwapBuffers();
}

It begins its first frame. In draw_something it will call get_back_bo
which will allocate a buffer because there are currently no buffers.
It will then call eglSwapBuffers which will install a frame callback
and give the buffer to the compositor. As this is the first buffer,
the compositor won't send a release event yet.

The client will now start rendering its second frame. It will call
get_back_bo which will detect that the first buffer is still locked so
it will allocate a second buffer. It will then call eglSwapBuffers
which will cause it to block for the frame callback. There is still no
release event so this won't cause any buffers to be unlocked. Once the
frame callback is received it will commit the second buffer. The
compositor only needs to keep hold of one buffer so it will
immediately queue a release event. (It could alternatively immediately
post the event here but that doesn't actually fix the problem.) After
the commit the client will install a frame callback. This will
guarantee that the release event will be flushed to the client
eventually.

Now the client will start rendering its third frame. The release event
may or may not have been received in the client's socket buffer
depending on the timing. It doesn't actually matter though and the bug
will be triggered either way. The first thing it does while rendering
is call get_back_bo. The first thing get_back_bo does is dispatch any
pending events in its queue. However this doesn't do anything for the
release event because nothing has read from the socket yet so it is
still only sitting the socket's buffer. At this point the client
hasn't seen any release events yet so it will think both buffers are
still locked and it will allocate a third buffer. Eventually the
client will finish rendering to this third buffer and it will call
eglSwapBuffers. At this point the client will block on the frame
callback which will end up reading from the socket and dispatching the
release event, but it is too late.

When the client calls get_back_bo for its fourth frame it will have
seen one release event so it will reuse the first buffer. It will
continue like this cycling between three buffers.

I hope that makes sense!

Regards,
- Neil

------- >8 --------------- (use git am --scissors to automatically chop here)

This adds a test that tries to simulate a simple game loop that would
be like this:

while (1) {
        draw_something();
        eglSwapBuffers();
}

In this case the test is relying on eglSwapBuffers to throttle to a
sensible frame rate.

The test then verifies that only 2 EGL buffers are used. This is done
via a new request and event in the wayland-test protocol.

Currently this causes 3 buffers to be created because the release
event generated by the swap buffers is not processed by Mesa until it
blocks for the frame complete event in the next swap buffers call, but
that is too late.

This can be fixed in Mesa by issuing a sync request after the swap
buffers and blocking on it before deciding whether to allocate a new
buffer.
---
 configure.ac                      |   7 +++
 protocol/wayland-test.xml         |   7 +++
 tests/Makefile.am                 |  15 ++++-
 tests/buffer-count-test.c         | 128 ++++++++++++++++++++++++++++++++++++++
 tests/weston-test-client-helper.c |  22 ++++++-
 tests/weston-test-client-helper.h |   4 ++
 tests/weston-test.c               |  51 ++++++++++++++-
 7 files changed, 231 insertions(+), 3 deletions(-)
 create mode 100644 tests/buffer-count-test.c

diff --git a/configure.ac b/configure.ac
index 86940d5..6e57c25 100644
--- a/configure.ac
+++ b/configure.ac
@@ -61,10 +61,17 @@ COMPOSITOR_MODULES="wayland-server >= 1.3.90 pixman-1"
 
 AC_ARG_ENABLE(egl, [  --disable-egl],,
               enable_egl=yes)
+AC_ARG_ENABLE(egl-tests, [  --enable-egl-tests],,
+              enable_egl_tests=yes)
 AM_CONDITIONAL(ENABLE_EGL, test x$enable_egl = xyes)
+AM_CONDITIONAL(ENABLE_EGL_TESTS, test x$enable_egl = xyes -a x$enable_egl_tests = xyes)
 if test x$enable_egl = xyes; then
 	AC_DEFINE([ENABLE_EGL], [1], [Build Weston with EGL support])
 	PKG_CHECK_MODULES(EGL, [egl >= 7.10 glesv2])
+
+        if test x$enable_egl_tests = xyes; then
+          PKG_CHECK_MODULES([EGL_TESTS], [egl >= 7.10 glesv2 wayland-client wayland-egl])
+        fi
 fi
 
 AC_ARG_ENABLE(xkbcommon,
diff --git a/protocol/wayland-test.xml b/protocol/wayland-test.xml
index 2993f08..18b6625 100644
--- a/protocol/wayland-test.xml
+++ b/protocol/wayland-test.xml
@@ -51,5 +51,12 @@
       <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>
   </interface>
 </protocol>
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 42788cb..2f726be 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -15,7 +15,8 @@ weston_tests =				\
 	button.weston			\
 	text.weston			\
 	subsurface.weston		\
-	$(xwayland_test)
+	$(xwayland_test)		\
+	$(egl_tests)
 
 AM_TESTS_ENVIRONMENT = \
 	abs_builddir='$(abs_builddir)'; export abs_builddir;
@@ -63,6 +64,10 @@ weston_test_la_SOURCES =		\
 	wayland-test-protocol.c		\
 	wayland-test-server-protocol.h
 
+if ENABLE_EGL_TESTS
+weston_test_la_LIBADD += $(EGL_TESTS_LIBS)
+endif
+
 libtest_runner_la_SOURCES =	\
 	weston-test-runner.c	\
 	weston-test-runner.h
@@ -111,6 +116,14 @@ text_weston_LDADD = libtest-client.la
 subsurface_weston_SOURCES = subsurface-test.c
 subsurface_weston_LDADD = libtest-client.la
 
+buffer_count_weston_SOURCES = buffer-count-test.c
+buffer_count_weston_CFLAGS = $(GCC_CFLAGS) $(EGL_TESTS_CFLAGS)
+buffer_count_weston_LDADD = libtest-client.la $(EGL_TESTS_LIBS)
+
+if ENABLE_EGL_TESTS
+egl_tests = buffer-count.weston
+endif
+
 xwayland_weston_SOURCES = xwayland-test.c
 xwayland_weston_CFLAGS = $(GCC_CFLAGS) $(XWAYLAND_TEST_CFLAGS)
 xwayland_weston_LDADD = libtest-client.la $(XWAYLAND_TEST_LIBS)
diff --git a/tests/buffer-count-test.c b/tests/buffer-count-test.c
new file mode 100644
index 0000000..ad2bcee
--- /dev/null
+++ b/tests/buffer-count-test.c
@@ -0,0 +1,128 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and
+ * its documentation for any purpose is hereby granted without fee, provided
+ * that the above copyright notice appear in all copies and that both that
+ * copyright notice and this permission notice appear in supporting
+ * documentation, and that the name of the copyright holders not be used in
+ * advertising or publicity pertaining to distribution of the software
+ * without specific, written prior permission.  The copyright holders make
+ * no representations about the suitability of this software for any
+ * purpose.  It is provided "as is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
+ * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
+ * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
+ * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
+ * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <string.h>
+
+#include "weston-test-client-helper.h"
+#include <stdio.h>
+#include <poll.h>
+#include <time.h>
+
+#include <EGL/egl.h>
+#include <wayland-egl.h>
+#include <GLES2/gl2.h>
+
+struct test_data {
+	struct client *client;
+
+	EGLDisplay egl_dpy;
+	EGLContext egl_ctx;
+	EGLConfig egl_conf;
+	EGLSurface egl_surface;
+};
+
+static void
+init_egl(struct test_data *test_data)
+{
+	struct wl_egl_window *native_window;
+	struct surface *surface = test_data->client->surface;
+
+	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 = eglGetDisplay((EGLNativeDisplayType)
+					   test_data->client->wl_display);
+	assert(test_data->egl_dpy);
+
+	ret = eglInitialize(test_data->egl_dpy, &major, &minor);
+	assert(ret == EGL_TRUE);
+	ret = eglBindAPI(EGL_OPENGL_ES_API);
+	assert(ret == EGL_TRUE);
+
+	ret = eglChooseConfig(test_data->egl_dpy, config_attribs,
+			      &test_data->egl_conf, 1, &n);
+	assert(ret && n == 1);
+
+	test_data->egl_ctx = eglCreateContext(test_data->egl_dpy,
+					      test_data->egl_conf,
+					      EGL_NO_CONTEXT, context_attribs);
+	assert(test_data->egl_ctx);
+
+	native_window =
+		wl_egl_window_create(surface->wl_surface,
+				     surface->width,
+				     surface->height);
+	test_data->egl_surface =
+		eglCreateWindowSurface(test_data->egl_dpy,
+				       test_data->egl_conf,
+				       (EGLNativeWindowType) native_window,
+				       NULL);
+
+	ret = eglMakeCurrent(test_data->egl_dpy, test_data->egl_surface,
+			     test_data->egl_surface, test_data->egl_ctx);
+	assert(ret == EGL_TRUE);
+}
+
+TEST(test_buffer_count)
+{
+	struct test_data test_data;
+	uint32_t buffer_count;
+	int i;
+
+	test_data.client = client_create(10, 10, 10, 10);
+	init_egl(&test_data);
+
+	/* 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);
+}
diff --git a/tests/weston-test-client-helper.c b/tests/weston-test-client-helper.c
index b19be40..fd265f2 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -111,6 +111,17 @@ 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;
+
+	wl_test_get_n_egl_buffers(client->test->wl_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,
@@ -340,8 +351,17 @@ test_handle_pointer_position(void *data, struct wl_test *wl_test,
 		test->pointer_x, test->pointer_y);
 }
 
+static void
+test_handle_n_egl_buffers(void *data, struct wl_test *wl_test, uint32_t n)
+{
+	struct test *test = data;
+
+	test->n_egl_buffers = n;
+}
+
 static const struct wl_test_listener test_listener = {
-	test_handle_pointer_position
+	test_handle_pointer_position,
+	test_handle_n_egl_buffers,
 };
 
 static void
diff --git a/tests/weston-test-client-helper.h b/tests/weston-test-client-helper.h
index a5edca9..c78fc28 100644
--- a/tests/weston-test-client-helper.h
+++ b/tests/weston-test-client-helper.h
@@ -51,6 +51,7 @@ struct test {
 	struct wl_test *wl_test;
 	int pointer_x;
 	int pointer_y;
+	uint32_t n_egl_buffers;
 };
 
 struct input {
@@ -120,4 +121,7 @@ frame_callback_set(struct wl_surface *surface, int *done);
 void
 frame_callback_wait(struct client *client, int *done);
 
+int
+get_n_egl_buffers(struct client *client);
+
 #endif
diff --git a/tests/weston-test.c b/tests/weston-test.c
index fc025fa..844059d 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -20,6 +20,8 @@
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include "config.h"
+
 #include <stdlib.h>
 #include <string.h>
 #include <assert.h>
@@ -28,6 +30,11 @@
 #include "../src/compositor.h"
 #include "wayland-test-server-protocol.h"
 
+#ifdef ENABLE_EGL
+#include <EGL/egl.h>
+#include <EGL/eglext.h>
+#endif /* ENABLE_EGL */
+
 struct weston_test {
 	struct weston_compositor *compositor;
 	struct weston_layer layer;
@@ -181,12 +188,54 @@ send_key(struct wl_client *client, struct wl_resource *resource,
 	notify_key(seat, 100, key, state, STATE_UPDATE_AUTOMATIC);
 }
 
+#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 */
+
+	wl_test_send_n_egl_buffers(resource, n_buffers);
+}
+
 static const struct wl_test_interface test_implementation = {
 	move_surface,
 	move_pointer,
 	send_button,
 	activate_surface,
-	send_key
+	send_key,
+	get_n_buffers,
 };
 
 static void
-- 
1.8.3.1



More information about the wayland-devel mailing list