Keyboard support #9

charafau · 2022-03-16T03:31:23Z

This PR targets https://github.com/PlayPulseCom/flutter_wlroots/issues/1

Current status:

Forward input from wlroots backend to the flutter engine. This includes handling the new keyboard signal emitted by wlroots and registering handlers for key events which call into the flutter engine API.
Implement keyboard event handling in SurfaceWidget. This would involve introducing a focus node and adding a handler for keyboard events. Implementing focus-on-click vs focus-on-hover (or some other mechanism) is best left to the consumer, but the dart library should make this easy.
Make sure the surface receives keyboard events correctly. This involves making a new message handler for keyboard events in native code which forwards the events to wlroots. This would be called by the SurfaceWidget when input events are received.
Make sure the surface receives/loses keyboard focus when the focus node receives/loses focus. This involves making a new message handler for managing focus changes in the native code, then forwarding that to wlroots. This would then be called by the SurfaceWidget when the focus node changes state.

^ We might want to add this in separate request when we have more than 1 SurfaceWidget and easy way to switch between them

Add json serialization lib Add collection lib Add key mappings Add sending key to flutter Add sending key from flutter to wlroots Add passing key from wlroots to view

Add sending key events from flutter to wayland client Add json serialization library Add collections header lib Add key maps for xcb to flutter keys

hansihe · 2022-03-16T11:29:44Z

src/key_mapping.cc

+
+// DO NOT EDIT -- DO NOT EDIT -- DO NOT EDIT
+// This file is generated by
+// flutter/flutter@dev/tools/gen_keycodes/bin/gen_keycodes.dart and should not


Is this file unmodified from the generator, or has there been modifications?

original is from generator, this one is not. I will remove comment

hansihe · 2022-03-16T11:30:01Z

compositor_dart/lib/constants.dart

+const int pointerKindUnknown = 3;
+
+const Map<int, int> physicalToXkbMap = {


Where is this mapping from? Is there any reason not to do this on the C side where the other parts of the mapping live?

This is taken from the gtk embedding iirc and it's just a reversed version of the xkb to physical map. Imo it should be in dart as it's easier to handle there as in c, but I'm open to anything really

What @HrX03 said. This mapping is take from Linux embedder https://github.com/flutter/engine/blob/master/shell/platform/linux/key_mapping.cc#L20
it's documented here https://github.com/flutter/flutter/blob/master/dev/tools/gen_keycodes/README.md

hansihe · 2022-03-16T11:30:57Z

compositor_dart/lib/compositor_dart.dart

+    platform.channel
+        .invokeMethod("testing")
+        .then((value) => {print("testing message response")})
+        .catchError((error) => {print("error")});
+
+    platform.addHandler("flutter/keyevent", (call) async {
+      print('got event: ${call.arguments["keymap"]}');
+      print('got event: ${call.arguments["keyCode"]}');
+      print('got event: ${call.arguments["type"]}');
+    });


This should likely be removed, debug code

hansihe · 2022-03-16T11:33:11Z

compositor_dart/lib/surface.dart

+            status = KeyStatus.released;
+          }
+
+          int? keycode = physicalToXkbMap[event.physicalKey.usbHidUsage];


Use the function for this in the compositor

hansihe · 2022-03-16T11:34:53Z

compositor_dart/lib/surface.dart

+          if (event is KeyDownEvent) {
+            status = KeyStatus.pressed;
+          } else {
+            status = KeyStatus.released;
+          }


Is this necessarily Down OR Up? What about key repeat event?

It would at least be nice to do an is check on both of these branches, then debug print if we get an event type we didn't expect.

Wlroots doesn't have a concept for repeat, instead it decides to "repeat" once it gets enough down events for the same key (at least by observing how it behaves + no enum entry for repeat)

It does seem like wlroots has the concept of repeats, as evident by wlr_keyboard_set_repeat_info.

So wlroots uses subsequent PRESSED events as the signal of a repeat key then? This should be converted to the Repeat event in the flutter engine on the C side.

Pardon, i corrected myself in a comment later on. As i stated there, as long as a key is signaled as being pressed then it will be "repeated". Once the key is signaled as up then it will stop being repeated

Basically what @HrX03 said. wlroots will send up events which will work as repeat.

hansihe · 2022-03-16T11:57:22Z

src/input.c

+      goto error;
+  }
+
+  wlr_seat_keyboard_notify_key(instance->seat, message.timestamp / NS_PER_MS, message.keycode - 8, type);


No management of active seat surface at all? Might be considered part of point 4?

By looking at various wl impls keyboard focus should be moved only on view focus, like when opening a new surface or when the user clicks the view to focus. We should change the flutter side to have one big global listener indeed instead of one for each surface as it doesn't really match with how wlroots handles keyboard

Yes, it's 1 surface for now.

hansihe · 2022-03-16T11:57:39Z

src/input.c

+
+
+// void fwr_handle_key_press(struct fwr_instance *instance, const FlutterPlatformMessageResponseHandle *handle, struct dart_value *args){
+//   struct wlr_seat *seat = instance->seat;
+
+//   struct key_event_message message;
+//   if (!decode_key_event_message(args, &message)) {
+//     wlr_log(WLR_ERROR, "Invalid key event message");
+//     return;
+//   }
+
+//   uint32_t xkb_key = physical_to_xkb_key(message.physical_key_id - 8);
+//   // wlr_log(WLR_INFO, "should transform  key %s", xkb_key);
+
+
+//   wlr_seat_keyboard_notify_key(seat, message.timestamp,
+// 			xkb_key, message.key_state);
+// }


Remove unused code

hansihe · 2022-03-16T11:57:59Z

src/surface.c

@@ -14,6 +15,43 @@ static void cb(const uint8_t *data, size_t size, void *user_data) {
  wlr_log(WLR_INFO, "callback");
 }

+static void focus_view(struct fwr_view *view, struct wlr_surface *surface) {
+	/* Note: this function only deals with keyboard focus. */


inconsistent formatting

hansihe · 2022-03-16T11:58:35Z

src/surface.c

@@ -63,7 +101,10 @@ static void xdg_toplevel_map(struct wl_listener *listener, void *data) {

  instance->fl_proc_table.PlatformMessageReleaseResponseHandle(instance->engine,
                                                               response_handle);
+
+  focus_view(view, view->surface);


No proper focus management? Part of point 4?

hansihe · 2022-03-16T11:59:11Z

src/surface.c

-  view->instance = instance;
-  view->surface = xdg_surface;


Any reason for this move?

Not really, was having some issues and moved stuff around to test, just forgot to put it back where it once was

HrX03 · 2022-03-16T13:04:13Z

include/jsmn.h

+  JSMN_UNDEFINED = 0,
+  JSMN_OBJECT = 1,
+  JSMN_ARRAY = 2,
+  JSMN_STRING = 3,


Why use 1 2 3 4 instead of the bit shifted constants?

looks like original lib has bitshift operatoions but flutter-pi has constants. I reverted to bit shift operators because I'd rather no to modify extrenal library

HrX03 · 2022-03-16T13:05:12Z

include/key_mapping.h

+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef KEYBOARD_MAP_H_
+#define KEYBOARD_MAP_H_
+
+#include <xkbcommon/xkbcommon.h>
+
+#define WLR_USE_UNSTABLE
+#include <wlr/types/wlr_keyboard.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+
+// Mask for the 32-bit value portion of the key code.
+extern const uint64_t kValueMask;
+
+// The plane value for keys which have a Unicode representation.
+extern const uint64_t kUnicodePlane;
+
+// The plane value for the private keys defined by the GTK embedding.
+extern const uint64_t kGtkPlane;
+
+uint64_t apply_id_plane(uint64_t logical_id, uint64_t plane);
+
+uint64_t event_to_physical_key(struct wlr_event_keyboard_key *event);
+
+uint64_t event_to_logical_key(struct wlr_event_keyboard_key *event, struct xkb_state *state);
+
+uint32_t event_to_unicode(struct wlr_event_keyboard_key *event, struct xkb_state *state);
+
+uint64_t event_to_timestamp(struct wlr_event_keyboard_key *event);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif  // KEYBOARD_MAP_H_


This + the c impl should be replaced with the xkb based methods

can you elaborate on that?

HrX03 · 2022-03-16T13:07:25Z

src/input.c

+  // uint32_t unicode = event_to_unicode(event, kb_state);
+
+  struct xkb_state *kb_state = keyboard->device->keyboard->xkb_state;
+  uint32_t modifiers = keyboard_state_is_shift_active(kb_state)


This is directly taken from the flutter-pi docs but I think I should be able to get some docs from flutter code

HrX03 · 2022-03-16T13:07:41Z

src/input.c

+        | (keyboard_state_is_alt_active(kb_state) << 3)
+        | (keyboard_state_is_numlock_active(kb_state) << 4)
+        | (keyboard_state_is_meta_active(kb_state) << 28);
+  uint16_t scan_code = (uint16_t)event->keycode + 8;


Same as above, but instead of flutter some xkb/libinput docs

HrX03 · 2022-03-16T13:08:35Z

src/input.c

+  switch(event->state) {
+    case WL_KEYBOARD_KEY_STATE_PRESSED:
+      type = "keydown";
+      flType = kFlutterKeyEventTypeDown;
+      break;
+    case WL_KEYBOARD_KEY_STATE_RELEASED:
+    default:
+      type = "keyup";
+      flType = kFlutterKeyEventTypeUp;
+      break;
+  }


This is correct for the flutter part but wlroots has no concept of repeat as far as I'm concerned, until one key is kept pressed it's going to keep it pressed apparently

HrX03 · 2022-03-16T13:09:21Z

src/input.c

+  wlr_keyboard_set_keymap(device->keyboard, keymap);
+  xkb_keymap_unref(keymap);
+  xkb_context_unref(context);
+  wlr_keyboard_set_repeat_info(device->keyboard, 25, 600);


Tinywl source code iirc

HrX03 · 2022-03-16T13:11:13Z

src/input.c

+      goto error;
+  }
+
+  wlr_seat_keyboard_notify_key(instance->seat, message.timestamp / NS_PER_MS, message.keycode - 8, type);


By looking at various wl impls keyboard focus should be moved only on view focus, like when opening a new surface or when the user clicks the view to focus. We should change the flutter side to have one big global listener indeed instead of one for each surface as it doesn't really match with how wlroots handles keyboard

HrX03 · 2022-03-16T13:11:48Z

src/surface.c

-  view->instance = instance;
-  view->surface = xdg_surface;


Not really, was having some issues and moved stuff around to test, just forgot to put it back where it once was

removed duplicated key map in surface removed debug code

hansihe · 2022-03-26T14:36:22Z

Todos:

Remove collections.h, put asserts part into asserts.h
Revert unnecessary changes in surface.c

Move assert code to asserts.h Revert unnecessary code changes

hansihe · 2022-03-31T13:25:54Z

Thanks!

charafau added 9 commits March 7, 2022 22:25

Add passing key press from wlroots to flutter

aad3c0a

Merge branch 'master' into keyboard_support

3b96e14

Add sending keybord events to flutter

161f7aa

Changed sending key from method channel to send key engine method

adfa1af

add logging

2844856

Add focused surface id in instance

b14b4da

Add json serialization lib Add collection lib Add key mappings Add sending key to flutter Add sending key from flutter to wlroots Add passing key from wlroots to view

Add sending wlroots keyboard events to flutter

2c96148

Add sending key events from flutter to wayland client Add json serialization library Add collections header lib Add key maps for xcb to flutter keys

Remove calculator example

6b4d813

remove binary search and replace with find

a8668d4

hansihe requested changes Mar 16, 2022

View reviewed changes

HrX03 reviewed Mar 16, 2022

View reviewed changes

charafau added 3 commits March 18, 2022 02:39

removed unused code

52deb80

removed duplicated key map in surface removed debug code

removed debug code

3381910

Removed unused key mapping files

c4b4bf1

charafau added 2 commits March 28, 2022 02:12

Remove collection.h

9c64170

Move assert code to asserts.h Revert unnecessary code changes

Merge branch 'master' into keyboard_support_send_key

6db884f

hansihe merged commit 0202384 into FlutterWayland:master Mar 31, 2022

charafau deleted the keyboard_support_send_key branch April 1, 2022 16:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keyboard support #9

Keyboard support #9

charafau commented Mar 16, 2022

hansihe Mar 16, 2022

charafau Mar 18, 2022

hansihe Mar 16, 2022

HrX03 Mar 16, 2022

charafau Mar 18, 2022

hansihe Mar 16, 2022

hansihe Mar 16, 2022

hansihe Mar 16, 2022

HrX03 Mar 16, 2022

hansihe Mar 16, 2022

HrX03 Mar 16, 2022

charafau Mar 18, 2022

hansihe Mar 16, 2022

HrX03 Mar 16, 2022

charafau Mar 18, 2022

hansihe Mar 16, 2022

hansihe Mar 16, 2022

hansihe Mar 16, 2022

hansihe Mar 16, 2022

HrX03 Mar 16, 2022

charafau Mar 18, 2022

HrX03 Mar 16, 2022

charafau Mar 18, 2022

HrX03 Mar 16, 2022

charafau Mar 18, 2022

HrX03 Mar 16, 2022

HrX03 Mar 16, 2022

HrX03 Mar 16, 2022

HrX03 Mar 16, 2022

HrX03 Mar 16, 2022

HrX03 Mar 16, 2022

hansihe commented Mar 26, 2022

hansihe commented Mar 31, 2022

		const int pointerKindUnknown = 3;

		const Map<int, int> physicalToXkbMap = {

Keyboard support #9

Keyboard support #9

Conversation

charafau commented Mar 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansihe commented Mar 26, 2022

hansihe commented Mar 31, 2022