[PATCH wfits v2] core: Add a wrapper for the keyboard

Neil Roberts neil at linux.intel.com
Wed Sep 25 16:32:36 PDT 2013


Ok, here's a second version of the patch with all of the suggested changes.

> So we must only want one xkb context per display. Ok.

Yes, I think we only really want one global xkb context but the
display seemed like a convenient place to attach it and ensure that it
will be destructed.

> We should explicitly initialize keys_ here with some default:

Ok

> I prefer to always use brackets in all control structures even
> if there is only one statement.

Ok. I also changed places where I was using brace hugging because I
had just assumed you were using the Wayland/Linux style.

> But, instead of iterating here why not use std::find(...):

Ok, I changed it to use std::find().

> How about:
> 	if (not keyPressed(key))
> 	{
> 		keys_.push_back(key);
> 	}

Sure.

> This is an odd way to remove the key from the vector, and was a bit
> hard to understand initially, but I suppose it's faster than
> keys_.erase(...).

Yeah, I copied this trick from Weston because I thought it was quite
neat. But in this case I guess there's absolutely no point in
optimising it so let's just use vector::erase() to make it more
readable.

> Probably prefer const function here:
> 	bool hasFocus(wl_surface*) const;

Ok. You might want to fix the one in pointer.cpp too in that case.

> I'd prefer to see all class scoped typedefs at the top of the class
> declaration, just before the "public" keyword (which effectively
> keeps them private by default). Also, for simplicity, we should
> typedef the vector, too:

Ok. I've added a typedef for the vector and moved it to the top. I've
removed the iterator types because once you have the typedef for the
vector then typing Keys::iterator doesn't seem such a pain.

Regards,
- Neil

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

This adds a wrapper for a wl_keyboard in a similar way to the pointer
wrapper. It keeps track of the keys that are pressed so that they can
be quickly verified. wayland-fits now depends on libxkbcommon so that
the keyboard wrapper can pass the keymap to it and get the modifier
indices.
---
 configure.ac               |   2 +
 src/test/Makefile.am       |   2 +
 src/test/core/Makefile.am  |   3 +
 src/test/core/display.cpp  |  16 ++++
 src/test/core/display.h    |   3 +
 src/test/core/keyboard.cpp | 216 +++++++++++++++++++++++++++++++++++++++++++++
 src/test/core/keyboard.h   | 106 ++++++++++++++++++++++
 src/test/efl/Makefile.am   |   2 +
 src/test/gtk+/Makefile.am  |   2 +
 9 files changed, 352 insertions(+)
 create mode 100644 src/test/core/keyboard.cpp
 create mode 100644 src/test/core/keyboard.h

diff --git a/configure.ac b/configure.ac
index de84adf..5ad59ca 100644
--- a/configure.ac
+++ b/configure.ac
@@ -61,6 +61,8 @@ PKG_CHECK_MODULES([WAYLAND],
 )
 PKG_CHECK_MODULES(WAYLAND_SERVER, [wayland-server >= wayland_req_version])
 
+PKG_CHECK_MODULES([XKBCOMMON], xkbcommon)
+
 have_weston="no"
 want_weston_extensions="auto"
 AC_ARG_ENABLE([weston-extensions],
diff --git a/src/test/Makefile.am b/src/test/Makefile.am
index ba5e5ce..e50c0fc 100644
--- a/src/test/Makefile.am
+++ b/src/test/Makefile.am
@@ -14,6 +14,7 @@ wfits_SOURCES =				\
 wfits_LDADD =				\
 	core/libwfits-core.la		\
 	$(CHECK_LIBS)			\
+	$(XKBCOMMON_LIBS)		\
 	$(WAYLAND_LIBS)			\
 	$(BOOST_LIBS)
 
@@ -23,6 +24,7 @@ wfits_LDFLAGS =				\
 wfits_CPPFLAGS =			\
 	-I$(top_srcdir)/src		\
 	$(CHECK_CFLAGS)			\
+	$(XKBCOMMON_CFLAGS)		\
 	$(WAYLAND_CFLAGS)		\
 	$(BOOST_CPPFLAGS)
 
diff --git a/src/test/core/Makefile.am b/src/test/core/Makefile.am
index bff1c5a..3a93a67 100644
--- a/src/test/core/Makefile.am
+++ b/src/test/core/Makefile.am
@@ -3,6 +3,7 @@ noinst_LTLIBRARIES = libwfits-core.la
 libwfits_core_la_SOURCES =		\
 	display.cpp			\
 	compositor.cpp			\
+	keyboard.cpp			\
 	shell.cpp			\
 	seat.cpp			\
 	pointer.cpp			\
@@ -23,11 +24,13 @@ libwfits_core_la_SOURCES =		\
 
 libwfits_core_la_LIBADD =		\
 	$(WAYLAND_LIBS)			\
+	$(XKBCOMMON_LIBS)		\
 	$(CHECK_LIBS)
 
 libwfits_core_la_CPPFLAGS =		\
 	-I$(top_srcdir)/src		\
 	$(WAYLAND_CFLAGS)		\
+	$(XKBCOMMON_CFLAGS)		\
 	$(CHECK_CFLAGS)
 
 AM_CXXFLAGS =				\
diff --git a/src/test/core/display.cpp b/src/test/core/display.cpp
index 1c29dbf..55cb673 100644
--- a/src/test/core/display.cpp
+++ b/src/test/core/display.cpp
@@ -30,6 +30,7 @@ Display::Display()
 	: wl_display_(wl_display_connect(0))
 	, wl_registry_(NULL)
 	, globals_()
+	, xkbContext_(NULL)
 {
 	ASSERT(wl_display_ != NULL);
 
@@ -49,6 +50,10 @@ Display::Display()
 
 /*virtual*/ Display::~Display()
 {
+	if (xkbContext_)
+	{
+		xkb_context_unref(xkbContext_);
+	}
 	wl_registry_destroy(wl_registry_);
 	wl_display_disconnect(*this);
 }
@@ -69,6 +74,17 @@ void Display::yield(const unsigned time) const
 	usleep(time);
 }
 
+struct xkb_context *Display::xkbContext() const
+{
+	if (xkbContext_ == NULL)
+	{
+		xkbContext_ = xkb_context_new((enum xkb_context_flags) 0);
+		ASSERT(xkbContext_ != NULL);
+	}
+
+	return xkbContext_;
+}
+
 /*static*/ void Display::global(
 	void *data, struct wl_registry *wl_registry, uint32_t id,
 	const char* interface, uint32_t version)
diff --git a/src/test/core/display.h b/src/test/core/display.h
index 0a4c10a..bf09057 100644
--- a/src/test/core/display.h
+++ b/src/test/core/display.h
@@ -25,6 +25,7 @@
 
 #include <map>
 #include <wayland-client.h>
+#include <xkbcommon/xkbcommon.h>
 #include "test/tools.h"
 
 namespace wfits {
@@ -63,6 +64,7 @@ public:
 	void roundtrip() const;
 	void dispatch() const;
 	void yield(const unsigned = 0.001 * 1e6) const;
+	struct xkb_context *xkbContext() const;
 
 	operator wl_display*() const { return wl_display_; }
 	operator wl_registry*() const { return wl_registry_; }
@@ -74,6 +76,7 @@ private:
 	wl_display	*wl_display_;
 	wl_registry	*wl_registry_;
 	Globals		globals_;
+	mutable struct xkb_context *xkbContext_;
 };
 
 } // namespace core
diff --git a/src/test/core/keyboard.cpp b/src/test/core/keyboard.cpp
new file mode 100644
index 0000000..2c766c6
--- /dev/null
+++ b/src/test/core/keyboard.cpp
@@ -0,0 +1,216 @@
+/*
+ * 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 <sys/mman.h>
+
+#include "keyboard.h"
+
+namespace wfits {
+namespace test {
+namespace core {
+
+Keyboard::Keyboard(const Seat& seat)
+	: seat_(seat)
+	, focus_(NULL)
+	, focusSerial_(0)
+	, wl_keyboard_(NULL)
+	, keys_()
+	, modsDepressed_(0)
+	, modsLatched_(0)
+	, modsLocked_(0)
+	, group_(0)
+	, keymap_(NULL)
+{
+	ASSERT(seat.capabilities() & WL_SEAT_CAPABILITY_KEYBOARD);
+
+	wl_keyboard_ = wl_seat_get_keyboard(seat);
+
+	ASSERT(wl_keyboard_ != NULL);
+
+	wl_keyboard_set_user_data(*this, this);
+
+	static const wl_keyboard_listener listener = {
+		keymap, enter, leave, key, modifiers
+	};
+
+	wl_keyboard_add_listener(*this, &listener, this);
+}
+
+/*virtual*/ Keyboard::~Keyboard()
+{
+	if (keymap_)
+	{
+		xkb_map_unref(keymap_);
+	}
+
+	wl_keyboard_destroy(*this);
+}
+
+bool Keyboard::hasFocus(wl_surface* surface) const
+{
+	return focus() == surface;
+}
+
+bool Keyboard::keyPressed(uint32_t key) const
+{
+	return std::find(keys_.begin(), keys_.end(), key) != keys_.end();
+}
+
+void Keyboard::pressKey(uint32_t key)
+{
+	if (not keyPressed(key))
+	{
+		keys_.push_back(key);
+	}
+}
+
+void Keyboard::releaseKey(uint32_t key)
+{
+	Keys::iterator it = std::find(keys_.begin(), keys_.end(), key);
+
+	if (it != keys_.end())
+	{
+		keys_.erase(it);
+	}
+}
+
+/* static */ void Keyboard::keymap(void *data,
+				   struct wl_keyboard *wl_keyboard,
+				   uint32_t format,
+				   int32_t fd,
+				   uint32_t size)
+{
+	Keyboard* keyboard = static_cast<Keyboard*>(data);
+	struct xkb_keymap *keymap;
+	char *map_str;
+
+	ASSERT(wl_keyboard == *keyboard);
+	ASSERT(format == WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1);
+
+	map_str = (char *) mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
+	ASSERT(map_str != MAP_FAILED);
+
+	keymap = xkb_map_new_from_string(keyboard->seat_.display().xkbContext(),
+					 map_str,
+					 XKB_KEYMAP_FORMAT_TEXT_V1,
+					 (enum xkb_keymap_compile_flags) 0);
+	munmap(map_str, size);
+	close(fd);
+
+	ASSERT(keymap != NULL);
+
+	if (keyboard->keymap_ != NULL)
+	{
+		xkb_map_unref(keyboard->keymap_);
+	}
+
+	keyboard->keymap_ = keymap;
+}
+
+/* static */ void Keyboard::enter(void *data,
+				  struct wl_keyboard *wl_keyboard,
+				  uint32_t serial,
+				  struct wl_surface *surface,
+				  struct wl_array *keys)
+{
+	Keyboard* keyboard = static_cast<Keyboard*>(data);
+	ASSERT(wl_keyboard == *keyboard);
+
+	keyboard->focus_ = surface;
+	keyboard->focusSerial_ = serial;
+
+	keyboard->keys_.resize(keys->size / sizeof(uint32_t));
+	memcpy(&keyboard->keys_[0], keys->data, keys->size);
+}
+
+/* static */ void Keyboard::leave(void *data,
+				  struct wl_keyboard *wl_keyboard,
+				  uint32_t serial,
+				  struct wl_surface *surface)
+{
+	Keyboard* keyboard = static_cast<Keyboard*>(data);
+	ASSERT(wl_keyboard == *keyboard);
+
+	keyboard->focus_ = NULL;
+	keyboard->keys_.resize(0);
+
+	/* We don't want to reset the modifiers here because the
+	 * compositor may still send updated modifier state if the
+	 * surface has pointer focus */
+}
+
+/* static */ void Keyboard::key(void *data,
+				struct wl_keyboard *wl_keyboard,
+				uint32_t serial,
+				uint32_t time,
+				uint32_t key,
+				uint32_t state)
+{
+	Keyboard* keyboard = static_cast<Keyboard*>(data);
+	ASSERT(wl_keyboard == *keyboard);
+
+	if (state == 0)
+	{
+		keyboard->releaseKey(key);
+	}
+	else
+	{
+		keyboard->pressKey(key);
+	}
+}
+
+/* static */ void Keyboard::modifiers(void *data,
+				      struct wl_keyboard *wl_keyboard,
+				      uint32_t serial,
+				      uint32_t modsDepressed,
+				      uint32_t modsLatched,
+				      uint32_t modsLocked,
+				      uint32_t group)
+{
+	Keyboard* keyboard = static_cast<Keyboard*>(data);
+	ASSERT(wl_keyboard == *keyboard);
+
+	keyboard->modsDepressed_ = modsDepressed;
+	keyboard->modsLatched_ = modsLatched;
+	keyboard->modsLocked_ = modsLocked;
+	keyboard->group_ = group;
+}
+
+namespace wrapper {
+
+	TEST(Keyboard)
+	{
+		Display display;
+		Seat seat(display);
+		Keyboard keyboard(seat);
+
+		FAIL_UNLESS_EQUAL(&keyboard.seat(), &seat);
+		FAIL_IF((wl_keyboard*)keyboard == NULL);
+		FAIL_UNLESS_EQUAL(wl_keyboard_get_user_data(keyboard), &keyboard);
+		FAIL_UNLESS(keyboard.hasFocus(NULL));
+	}
+
+} // namespace wrapper
+
+} // namespace core
+} // namespace test
+} // namespace wfits
diff --git a/src/test/core/keyboard.h b/src/test/core/keyboard.h
new file mode 100644
index 0000000..53398a2
--- /dev/null
+++ b/src/test/core/keyboard.h
@@ -0,0 +1,106 @@
+/*
+ * 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.
+ */
+
+#ifndef __WFITS_CORE_KEYBOARD_H__
+#define __WFITS_CORE_KEYBOARD_H__
+
+#include <xkbcommon/xkbcommon.h>
+#include <vector>
+#include "seat.h"
+
+namespace wfits {
+namespace test {
+namespace core {
+
+class Keyboard
+{
+	typedef std::vector<uint32_t> Keys;
+
+public:
+	Keyboard(const Seat&);
+
+	virtual ~Keyboard();
+
+	operator wl_keyboard*() const { return wl_keyboard_; }
+	const Seat& seat() const { return seat_; }
+	wl_surface* focus() const { return focus_; }
+	xkb_keymap* keymap() const { return keymap_; }
+	const uint32_t focusSerial() const { return focusSerial_; }
+
+	bool hasFocus(wl_surface*) const;
+
+	const uint32_t modsDepressed() const { return modsDepressed_; }
+	const uint32_t modsLatched() const { return modsLatched_; }
+	const uint32_t modsLocked() const { return modsLocked_; }
+	const uint32_t group() const { return group_; }
+	bool keyPressed(uint32_t key) const;
+
+private:
+	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);
+	static void modifiers(void *data,
+			      struct wl_keyboard *wl_keyboard,
+			      uint32_t serial,
+			      uint32_t modsDepressed,
+			      uint32_t modsLatched,
+			      uint32_t modsLocked,
+			      uint32_t group);
+
+	void pressKey(uint32_t key);
+	void releaseKey(uint32_t key);
+
+	const Seat&	seat_;
+	wl_surface*	focus_;
+	uint32_t	focusSerial_;
+	wl_keyboard*	wl_keyboard_;
+
+	std::vector<uint32_t> keys_;
+	uint32_t modsDepressed_;
+	uint32_t modsLatched_;
+	uint32_t modsLocked_;
+	uint32_t group_;
+	struct xkb_keymap *keymap_;
+};
+
+} // namespace core
+} // namespace test
+} // namespace wfits
+
+#endif
diff --git a/src/test/efl/Makefile.am b/src/test/efl/Makefile.am
index 104213d..85084be 100644
--- a/src/test/efl/Makefile.am
+++ b/src/test/efl/Makefile.am
@@ -46,6 +46,7 @@ libwfits_efl_la_SOURCES = \
 	test_window_fullscreen.cpp
 
 libwfits_efl_la_LIBADD =		\
+	$(XKBCOMMON_LIBS)		\
 	$(WAYLAND_LIBS)			\
 	$(EFL_LIBS)			\
 	$(CHECK_LIBS)
@@ -53,6 +54,7 @@ libwfits_efl_la_LIBADD =		\
 libwfits_efl_la_CPPFLAGS =		\
 	-I$(top_srcdir)/src		\
 	-DMEDIA_PATH=$(MEDIA)		\
+	$(XKBCOMMON_CFLAGS)		\
 	$(WAYLAND_CFLAGS)		\
 	$(EFL_CFLAGS)			\
 	$(CHECK_CFLAGS)
diff --git a/src/test/gtk+/Makefile.am b/src/test/gtk+/Makefile.am
index b19aa9c..aa12963 100644
--- a/src/test/gtk+/Makefile.am
+++ b/src/test/gtk+/Makefile.am
@@ -10,12 +10,14 @@ libwfits_gtk_la_SOURCES =		\
 
 libwfits_gtk_la_LIBADD =		\
 	$(WAYLAND_LIBS)			\
+	$(XKBCOMMON_LIBS)		\
 	$(GTK_LIBS)			\
 	$(CHECK_LIBS)
 
 libwfits_gtk_la_CPPFLAGS =		\
 	-I$(top_srcdir)/src		\
 	$(WAYLAND_CFLAGS)		\
+	$(XKBCOMMON_CFLAGS)		\
 	$(GTK_CFLAGS)			\
 	$(CHECK_CFLAGS)
 
-- 
1.8.3.1



More information about the wayland-devel mailing list