[Piglit] [PATCH] util: implement eventloop for wayland platform

[Tapani Pälli]

On 10/25/2016 02:09 PM, Pekka Paalanen wrote:
> On Mon, 24 Oct 2016 14:31:25 +0300
> Tapani Pälli <tapani.palli at intel.com> wrote:
>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>  .../util/piglit-framework-gl/piglit_wl_framework.c | 137 +++++++++++++++++++--
>>  1 file changed, 129 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/util/piglit-framework-gl/piglit_wl_framework.c b/tests/util/piglit-framework-gl/piglit_wl_framework.c
>> index 78ffea6..6b832f6 100644
>> --- a/tests/util/piglit-framework-gl/piglit_wl_framework.c
>> +++ b/tests/util/piglit-framework-gl/piglit_wl_framework.c
>> @@ -24,14 +24,136 @@
>>  #include <assert.h>
>>  #include <stdlib.h>
>>  #include <unistd.h>
>> +#include <poll.h>
>>
>>  #include "piglit-util-gl.h"
>>  #include "piglit_wl_framework.h"
>>
>> +#include <wayland-client.h>
>> +#include <waffle_wayland.h>
>> +#include <linux/input-event-codes.h>
>> +
>> +static struct wl_seat *seat = NULL;
>> +static struct wl_keyboard *keyboard = NULL;
>> +
>> +static void keymap(void *data,
>> +	struct wl_keyboard *wl_keyboard,
>> +	uint32_t format,
>> +	int32_t fd,
>> +	uint32_t size)
>> +{
>> +}
>> +
>> +static void enter(void *data,
>> +	struct wl_keyboard *wl_keyboard,
>> +	uint32_t serial,
>> +	struct wl_surface *surface,
>> +	struct wl_array *keys)
>> +{
>> +}
>> +
>> +static void leave(void *data,
>> +	struct wl_keyboard *wl_keyboard,
>> +	uint32_t serial,
>> +	struct wl_surface *surface)
>> +{
>> +}
>> +
>> +static void key(void *data,
>> +	struct wl_keyboard *wl_keyboard,
>> +	uint32_t serial,
>> +	uint32_t time,
>> +	uint32_t key,
>> +	uint32_t state)
>> +{
>> +	struct piglit_winsys_framework *winsys_fw =
>> +		(struct piglit_winsys_framework *) data;
>> +
>> +	winsys_fw->need_redisplay = true;
>> +
>> +	switch (key) {
>> +	case KEY_ESC:
>> +                if (winsys_fw->user_keyboard_func)
>> +			winsys_fw->user_keyboard_func(27, 0, 0);
>
> Hi,
>
> do you want this to trigger on both key down and key up?
> If not, check 'state'.

Yeah, I saw that there but wanted to just make the most simplest 
implementation (at least for now). This could be improved to map event 
codes to what user_keyboard_func eats and always just send that mapped 
value, not just with KEY_ESC.

> I suppose all keyboards emit KEY_ESC when the user presses the key
> labeled "esc" so you don't need to handle keymaps?

Any suggestions here welcome, I did not find any examples of how keymap 
handler (empty above) is supposed to work so ... what I did was this :)

>> +		break;
>> +	}
>> +}
>> +
>> +static void modifiers(void *data,
>> +	struct wl_keyboard *wl_keyboard,
>> +	uint32_t serial,
>> +	uint32_t mods_depressed,
>> +	uint32_t mods_latched,
>> +	uint32_t mods_locked,
>> +	uint32_t group)
>> +{
>> +}
>> +
>> +static const struct wl_keyboard_listener keyboard_listener = {
>> +	keymap,
>> +	enter,
>> +	leave,
>> +	key,
>> +	modifiers
>> +};
>> +
>> +static void
>> +process_events(struct wl_display *dpy)
>> +{
>> +	struct pollfd fds = {
>> +		.fd = wl_display_get_fd(dpy),
>> +		.events = POLLIN,
>> +		.revents = 0
>> +	};
>> +
>> +	wl_display_sync(dpy);
>
> What did you intend to do here?
>
> wl_display_sync() sends the wl_display.sync request and creates a
> wl_callback object which you leak.

The intention here was to trigger flush for the event queue so that I 
would receive those queued events.

>> +
>> +	while (1) {
>> +		poll(&fds, 1, -1);
>> +		if (wl_display_dispatch(dpy) < 0)
>> +			break;
>> +	}
>> +}
>> +
>> +static void global(void *data,
>> +	struct wl_registry *wl_registry,
>> +	uint32_t name,
>> +	const char *interface,
>> +	uint32_t version)
>> +{
>> +	if (strcmp(interface, "wl_seat") != 0)
>> +		return;
>> +
>> +	seat = wl_registry_bind(wl_registry, name, &wl_seat_interface, 1);
>
> There can be more than one wl_seat advertised, so if you only want
> one (how do you pick which one?)...

Heh .. same explanation as for the keymap handler! Is there somewhere a 
'wayland seats for dummies'?

>> +	keyboard = wl_seat_get_keyboard(seat);
>> +
>> +	if (keyboard)
>> +		wl_keyboard_add_listener(keyboard, &keyboard_listener, data);
>> +}
>> +
>> +static void global_remove(void *data,
>> +	struct wl_registry *wl_registry,
>> +	uint32_t name)
>> +{
>> +}
>> +
>> +static const struct wl_registry_listener registry_listener = {
>> +	global,
>> +	global_remove
>> +};
>> +
>>  static void
>>  enter_event_loop(struct piglit_winsys_framework *winsys_fw)
>>  {
>> -	const struct piglit_gl_test_config *test_config = winsys_fw->wfl_fw.gl_fw.test_config;
>> +	const struct piglit_gl_test_config *test_config =
>> +		winsys_fw->wfl_fw.gl_fw.test_config;
>> +	union waffle_native_window *n_window =
>> +		waffle_window_get_native(winsys_fw->wfl_fw.window);
>> +	struct wl_display *dpy = n_window->wayland->display.wl_display;
>> +	struct wl_registry *registry = wl_display_get_registry(dpy);
>
> How do you intend to destroy the registry object created here, or
> do you just willfully leak it?

We leak a lot of things with Piglit but I can fix this. Can I make a 
simple callback to free it with some Wayland API?

Thanks for taking a look at this, this is my first time with Wayland so 
please bare with me!

>> +	enum piglit_result result = PIGLIT_PASS;
>> +
>> +	wl_registry_add_listener(registry, &registry_listener, winsys_fw);
>>
>>  	/* The Wayland window fails to appear on the first call to
>>  	 * swapBuffers (which occured in display_cb above). This is
>> @@ -40,14 +162,13 @@ enter_event_loop(struct piglit_winsys_framework *winsys_fw)
>>  	 * redraw as a workaround.
>>  	 */
>>  	if (test_config->display)
>> -		test_config->display();
>> +		result = test_config->display();
>>
>> -	/* FINISHME: Write event loop for Wayland.
>> -	 *
>> -	 * Until we have proper Wayland support, give the user enough time
>> -	 * to view the window by sleeping.
>> -	 */
>> -	sleep(8);
>> +	/* Do not proceed to event loop in case of piglit_automatic. */
>> +	if (piglit_automatic)
>> +		piglit_report_result(result);
>> +
>> +	process_events(dpy);
>>  }
>>
>>  static void
>> --
>
> Thanks,
> pq
>