On Mon, Feb 24, 2014 at 11:41 AM, Eric Anholt <[email protected]> wrote:
> ---
>  tests/egl/egl-util.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/egl/egl-util.c b/tests/egl/egl-util.c
> index 5f4fffe..226ba0e 100644
> --- a/tests/egl/egl-util.c
> +++ b/tests/egl/egl-util.c
> @@ -30,6 +30,7 @@
>   * \author Kristian Høgsberg <[email protected]>
>   */
>
> +#include <X11/XKBlib.h>
>  #include "piglit-util-gl-common.h"
>  #include "egl-util.h"
>
> @@ -154,8 +155,9 @@ event_loop(struct egl_state *state, const struct egl_test 
> *test)
>                 }
>
>                 if (event.type == KeyPress) {
> -                       KeySym sym = XKeycodeToKeysym (state->dpy,
> -                                                      event.xkey.keycode, 0);
> +                       KeySym sym = XkbKeycodeToKeysym (state->dpy,

I believe this assumes that all piglit users are using an X server
with Xkb, an assumption that may not have been present before.
It could be that your change actually *fixes* things for the (common)
case where Xkb is present, but can you please at least add that caveat
to the commit message.

It looks like one possible way to support Xkb when possible, and still
keep compatibility with non-XKB X servers/Xlibs is like this:
https://www.mail-archive.com/[email protected]/msg02818.html
Alternatively, if we don't care about working on old non-XKB servers,
we should probably explicitly make piglit depend on a version of Xlib
that is guaranteed to have XkbKeycodeToKeysym (the above patch has a
similar check in its configure.ac).

However, adding that complexity is really only necessary if there are
actually people trying to run piglit with an old Xlib/Xserver.
So, I'm happy pushing thing and adding the complication later only if
it people complain.

With an amended commit message this one is...
Reviewed-by: Daniel Kurtz <[email protected]>

> +                                                        event.xkey.keycode,
> +                                                        0, 0);
>
>                         if (sym == XK_Escape || sym == XK_q || sym == XK_Q)
>                                 break;
> --
> 1.9.rc1
>
> _______________________________________________
> Piglit mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/piglit
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to