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! > + 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. :-) > +} > + > +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. > + } > +} > + > +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. > + > + 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, pq
pgpG0_J7SSeQd.pgp
Description: OpenPGP digital signature
_______________________________________________ Piglit mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/piglit
