On 11/15/2016 10:29 AM, Pekka Paalanen wrote:
On Wed, 2 Nov 2016 13:16:27 +0200
Tapani Pälli <[email protected]> wrote:
v2: lots of fixes based on review from Pekka Paalanen
- fix leak of fd in keymap handler
- fix leak of wayland registry
- just call wl_display_dispatch in process_events
- act when key pressed (matches x11 backend behavior)
- just pass any keys, now tests that do special stuff
with keys work (like fbo-clear-formats)
Signed-off-by: Tapani Pälli <[email protected]>
---
.../util/piglit-framework-gl/piglit_wl_framework.c | 253 +++++++++++++++++++--
1 file changed, 237 insertions(+), 16 deletions(-)
Moi Tapani,
some comments inline.
diff --git a/tests/util/piglit-framework-gl/piglit_wl_framework.c
b/tests/util/piglit-framework-gl/piglit_wl_framework.c
index 78ffea6..8bf2afb 100644
--- a/tests/util/piglit-framework-gl/piglit_wl_framework.c
+++ b/tests/util/piglit-framework-gl/piglit_wl_framework.c
@@ -1,5 +1,7 @@
/*
* Copyright © 2012 Intel Corporation
+ * Copyright © 2008 Kristian Høgsberg
+ * Copyright © 2012-2013 Collabora, Ltd.
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the "Software"),
@@ -28,10 +30,208 @@
#include "piglit-util-gl.h"
#include "piglit_wl_framework.h"
+#include <wayland-client.h>
+#include <waffle_wayland.h>
+#include <xkbcommon/xkbcommon.h>
+#include <sys/mman.h>
+
+struct piglit_wl_framework {
+ struct piglit_winsys_framework winsys_fw;
+
+ struct wl_display *dpy;
+ struct wl_registry *registry;
+
+ struct {
+ struct xkb_context *context;
+ struct xkb_keymap *keymap;
+ struct xkb_state *state;
+ } xkb;
+};
+
+static struct wl_seat *seat = NULL;
+static struct wl_keyboard *keyboard = NULL;
+
+/* Most of the code here taken from window.c in Weston source code. */
+static void keymap(void *data,
+ struct wl_keyboard *wl_keyboard,
+ uint32_t format,
+ int32_t fd,
+ uint32_t size)
+{
+ struct piglit_wl_framework *wl_fw =
+ (struct piglit_wl_framework *) data;
+ struct xkb_keymap *keymap;
+ struct xkb_state *state;
+ char *locale;
+ char *map_str;
+
+ if (!data || format != WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1) {
+ close(fd);
+ return;
+ }
+
+ map_str = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
+ if (map_str == MAP_FAILED) {
+ close(fd);
+ return;
+ }
+
+ /* Set up XKB keymap */
+ keymap = xkb_keymap_new_from_string(wl_fw->xkb.context,
+ map_str,
+ XKB_KEYMAP_FORMAT_TEXT_V1,
+ 0);
+
+ if (!keymap) {
+ fprintf(stderr, "failed to compile keymap\n");
Leaks fd. Move munmap and close above this and be happy!
fixed
+ return;
+ }
+
+ munmap(map_str, size);
+ close(fd);
+
+ /* Set up XKB state */
+ state = xkb_state_new(keymap);
+ if (!state) {
+ fprintf(stderr, "failed to create XKB state\n");
+ xkb_keymap_unref(keymap);
+ return;
+ }
+
+ /* Look up the preferred locale, falling back to "C" as default */
+ if (!(locale = getenv("LC_ALL")))
+ if (!(locale = getenv("LC_CTYPE")))
+ if (!(locale = getenv("LANG")))
+ locale = "C";
+
+ xkb_keymap_unref(wl_fw->xkb.keymap);
+ xkb_state_unref(wl_fw->xkb.state);
+ wl_fw->xkb.keymap = keymap;
+ wl_fw->xkb.state = state;
+}
+
+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_wl_framework *wl_fw =
+ (struct piglit_wl_framework *) data;
+ struct piglit_winsys_framework *winsys_fw = &wl_fw->winsys_fw;
+ const struct piglit_gl_test_config *test_config =
+ winsys_fw->wfl_fw.gl_fw.test_config;
+
+ const xkb_keysym_t *syms;
+ xkb_keysym_t sym;
+
+ uint32_t code = key + 8;
+ uint32_t num_syms;
+
+ if (!wl_fw->xkb.state)
+ return;
+
+ num_syms = xkb_state_key_get_syms(wl_fw->xkb.state, code, &syms);
+ sym = XKB_KEY_NoSymbol;
+ if (num_syms == 1)
+ sym = syms[0];
+
+ winsys_fw->need_redisplay = true;
+
+ if (state != WL_KEYBOARD_KEY_STATE_PRESSED)
+ return;
+
+ if (winsys_fw->user_keyboard_func)
+ winsys_fw->user_keyboard_func(sym, 0, 0);
+
+ if (winsys_fw->need_redisplay) {
+ enum piglit_result result = PIGLIT_PASS;
+ if (test_config->display)
+ result = test_config->display();
+ if (piglit_automatic)
+ piglit_report_result(result);
+ winsys_fw->need_redisplay = false;
+ }
+}
+
+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)
+{
I suspect leaving this empty would make you miss... modifiers' effects?
Up to you. :-)
current Piglit keyboard usage is very simple so I did not want to
implement 'extra', when/if there will be modifier usage then this can be
filled in
+}
+
+static const struct wl_keyboard_listener keyboard_listener = {
+ keymap,
+ enter,
+ leave,
+ key,
+ modifiers
+};
+
+static void
+process_events(struct wl_display *dpy)
+{
+ while (1) {
+ if (wl_display_dispatch(dpy) == -1)
+ break;
How do you break out from this gracefully? Is there some piglit infra
e.g. reacting to some key and calling exit() or something? Just
wondering how to quit this program.
Piglit calls exit(0) when escape key gets pressed and key handler calls
user_keyboard_func.
+ }
+}
+
+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);
+ 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;
+ struct piglit_wl_framework *wl_fw =
+ (struct piglit_wl_framework *) winsys_fw;
+ const struct piglit_gl_test_config *test_config =
+ winsys_fw->wfl_fw.gl_fw.test_config;
+ enum piglit_result result = PIGLIT_PASS;
/* The Wayland window fails to appear on the first call to
* swapBuffers (which occured in display_cb above). This is
@@ -40,14 +240,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(wl_fw->dpy);
}
static void
@@ -59,30 +258,52 @@ show_window(struct piglit_winsys_framework *winsys_fw)
static void
destroy(struct piglit_gl_framework *gl_fw)
{
- struct piglit_winsys_framework *winsys_fw=
piglit_winsys_framework(gl_fw);
+ struct piglit_wl_framework *wl_fw =
+ (struct piglit_wl_framework *) gl_fw;
- if (winsys_fw == NULL)
+ if (wl_fw == NULL)
return;
- piglit_winsys_framework_teardown(winsys_fw);
- free(winsys_fw);
+ xkb_context_unref(wl_fw->xkb.context);
Does this also free xkb.keymap and xkb.state? I'd kind of expect to see
them freed here, but I'm not sure.
fixed
+
+ wl_registry_destroy(wl_fw->registry);
+
+ piglit_winsys_framework_teardown(&wl_fw->winsys_fw);
+ free(wl_fw);
}
struct piglit_gl_framework*
piglit_wl_framework_create(const struct piglit_gl_test_config *test_config)
{
+ struct piglit_wl_framework *wl_fw = NULL;
struct piglit_winsys_framework *winsys_fw = NULL;
struct piglit_gl_framework *gl_fw = NULL;
bool ok = true;
- winsys_fw = calloc(1, sizeof(*winsys_fw));
- gl_fw = &winsys_fw->wfl_fw.gl_fw;
+ wl_fw = calloc(1, sizeof(*wl_fw));
+ winsys_fw = &wl_fw->winsys_fw;
+ gl_fw = &wl_fw->winsys_fw.wfl_fw.gl_fw;
- ok = piglit_winsys_framework_init(winsys_fw, test_config,
- WAFFLE_PLATFORM_WAYLAND);
+ ok = piglit_winsys_framework_init(&wl_fw->winsys_fw,
+ test_config,
WAFFLE_PLATFORM_WAYLAND);
if (!ok)
goto fail;
+ wl_fw->xkb.context = xkb_context_new(0);
+ wl_fw->xkb.keymap = NULL;
+ wl_fw->xkb.state = NULL;
+
+ if (!wl_fw->xkb.context)
+ goto fail;
+
+ union waffle_native_window *n_window =
+ waffle_window_get_native(winsys_fw->wfl_fw.window);
+
+ wl_fw->dpy = n_window->wayland->display.wl_display;
+ wl_fw->registry = wl_display_get_registry(wl_fw->dpy);
+
+ wl_registry_add_listener(wl_fw->registry, ®istry_listener, wl_fw);
+
winsys_fw->show_window = show_window;
winsys_fw->enter_event_loop = enter_event_loop;
gl_fw->destroy = destroy;
It all looks pretty good to me, just the couple freeing issues, and few
questions. I didn't compare this to what we have in Weston repository
nor do I know anything about Piglit's framework, but with that
disclaimer, I have nothing else to complain about. :-)
Not sure that really counts as R-b...
Thanks for taking a look!
If you want to still check/test I've pushed a v3 here with fixes to
issues you found:
https://cgit.freedesktop.org/~tpalli/piglit/log/?h=wayland
If nobody complains, I'll push this in after some days.
// Tapani
_______________________________________________
Piglit mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/piglit
