New keyboard API for the web #1888

maroider · 2021-03-18T18:52:29Z

Tested on all platforms changed
Compilation warnings were addressed
cargo fmt has been run on this branch
cargo doc builds successfully
Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
~~Created or updated an example program if it would help users understand this functionality~~
Updated feature matrix, if new features were added or implemented

This implements #753 for WASM.

TODOs

Decide if a stdweb backend is worthwhile, since it's deprecated.
- ~~Implement a stdweb backend if need be.~~
A bunch of TODO comments in the code.
Debug why my example program sometimes freezes with certain input sequences.

src/keyboard.rs

ArturKovacs · 2021-04-29T11:10:00Z

For the record: I'll test and properly review this once the merge conflicts are resolved.

maroider · 2021-05-14T16:14:40Z

Welp, I messed up the merge more times than I'd like to admit, but the merge conflicts should be resolved now.

ArturKovacs

Coming along nicely

src/keyboard.rs

src/platform_impl/web/event_loop/window_target.rs

src/platform_impl/web/mod.rs

src/platform_impl/web/keyboard.rs

src/platform_impl/web/event_loop/window_target.rs

src/platform_impl/web/web_sys/canvas.rs

ArturKovacs · 2021-05-15T15:52:09Z

src/platform_impl/web/web_sys/event.rs

+pub fn key_text(event: &KeyboardEvent) -> Option<&'static str> {
+    let key = event.key();
+    if let Key::Character(text) = Key::from_key_attribute_value(&key) {
+        // TODO: Fix unbounded leak
+        Some(Box::leak(String::from(text).into_boxed_str()))
+    } else {
+        None
+    }


The way I handled the leaked string, was by creating a global (lazy static) Mutex<HashSet<&'static str>>. I think the same approach is suitable here as well.

I copied over what I used in the Linux PR, which is about what you described. I'm a bit uncertain of how mutexes work on WASM, but it seemed to run fine when I tested it.

@ryanisaacg, how do mutexes work on WASM?

WebAssembly has instructions to block until a value in shared memory changes, which I believe is what mutexes use. Issue is, I'm pretty sure it causes an error if the main thread blocks, so we probably shouldn't use them here.

src/platform_impl/web/web_sys/event.rs

ArturKovacs · 2021-05-15T16:14:26Z

Furthermore, Space, Tab, and Enter emit None for the text field; they should emit a string. (I believe Enter should be "\r")

maroider · 2021-06-21T08:48:01Z

Furthermore, Space, Tab, and Enter emit None for the text field; they should emit a string. (I believe Enter should be "\r")

This should be fixed now.

What remains now is a single TODO comment as well as debugging the strange freezing.

ArturKovacs · 2021-06-21T20:53:06Z

src/platform_impl/web/web_sys/event.rs

+    match &key {
+        Key::Character(text) => Some(*text),
+        Key::Tab => Some("\t"),
+        Key::Enter => Some("\r"),
+        Key::Space => Some(" "),


Note that the Key has the to_text method which does pretty much this.

Woyten · 2021-12-21T16:41:48Z

@maroider Is there any update on this topic? I would like to see #2092 fixed. I could do the fixing myself but I am not sure whether my fix would be accepted and/or is in conflict with this PR.

This is to appease `serde` for the time being.

As a direct result of this, `NativeKeyCode`, `Key`, `KeyCode` and `RawKeyEvent` no longer implement `Copy`.

Liamolucko · 2022-06-13T07:48:15Z

src/platform_impl/web/keyboard.rs

+            "F33" => Key::F33,
+            "F34" => Key::F34,
+            "F35" => Key::F35,
+            string @ _ => Key::Character(string),


I don't think the @ _ is needed here.

Liamolucko · 2022-06-13T08:04:19Z

src/platform_impl/web/web_sys/event.rs

+fn cached_string<S: AsRef<str>>(string: S) -> &'static str {
+    use std::collections::HashSet;
+    use std::sync::Mutex;
+
+    lazy_static::lazy_static! {
+        static ref STRING_CACHE: Mutex<HashSet<&'static str>> = Mutex::new(HashSet::new());
+    };
+
+    let string = string.as_ref();
+    let mut cache = STRING_CACHE.lock().unwrap();
+    if let Some(string) = cache.get(string) {
+        *string
+    } else {
+        // borrowck couldn't quite figure out this one on its own
+        let string: &'static str = Box::leak(String::from(string).into_boxed_str());
+        cache.insert(string);
+        string
+    }


What's the reason why we need an &'static str in the first place? It seems reasonable to me for KeyEvent and Key to contain owned strings instead and avoid this complexity.

Liamolucko · 2022-06-13T08:08:53Z

src/platform_impl/web/event_loop/window_target.rs

+#[derive(Default)]
+struct ModifiersShared(Rc<Cell<ModifiersState>>);
+
+impl ModifiersShared {
+    fn set(&self, new: ModifiersState) {
+        self.0.set(new)
+    }
+
+    fn get(&self) -> ModifiersState {
+        self.0.get()
+    }
+}
+
+impl Clone for ModifiersShared {
+    fn clone(&self) -> Self {
+        Self(Rc::clone(&self.0))
+    }
+}


I think this could be replaced with type ModifiersShared = Rc<Cell<ModifiersState>> instead of having to declare a newtype.

Liamolucko · 2022-06-13T08:25:22Z

src/platform_impl/web/web_sys/event.rs

-    event.key().chars().next().unwrap()
+// TODO: What should be done about `KeyboardEvent.isComposing`?
+
+pub fn keyboard_modifiers(key: &Key<'_>) -> ModifiersState {


Suggested change

pub fn keyboard_modifiers(key: &Key<'_>) -> ModifiersState {

/// Returns the keyboard modifiers that changed with this keypress.

///

/// For a pressed key, this is the keyboard modifiers that are newly pressed,

/// and for a released key, this is the keyboard modifiers that are newly released.

pub fn keyboard_modifiers(key: &Key<'_>) -> ModifiersState {

Reading the code that was using this function, I assumed that this still had the old behaviour of returning the current state of the modifiers rather than the change to the modifiers. (It might also be a good idea to rename it to changed_modifiers or something.)

As a side-note, why was it changed to this in the first place? The old way of getting the current state seems easier to work with IMO.

Liamolucko · 2022-06-13T09:52:18Z

src/platform_impl/web/web_sys/event.rs

+    use std::sync::Mutex;
+
+    lazy_static::lazy_static! {
+        static ref STRING_CACHE: Mutex<HashSet<&'static str>> = Mutex::new(HashSet::new());


The main thread isn't allowed to block on the web, so a Mutex probably shouldn't be used here. A thread_local! should work.

To be fair, I don't think the event loop can be run outside of the main thread so this probably won't be a problem in practice, but that's even more of a reason to use a thread local.

Liamolucko · 2022-06-13T09:54:52Z

src/platform_impl/web/web_sys/event.rs

+// pub fn codepoint(event: &KeyboardEvent) -> char {
+//     // `event.key()` always returns a non-empty `String`. Therefore, this should
+//     // never panic.
+//     // https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key
+//     event.key().chars().next().unwrap()
+// }


This should probably be removed.

erwanvivien · 2023-01-03T19:03:04Z

Is there any update on this and how can we contribute if we need to ?

maroider closed this Mar 18, 2021

maroider reopened this Mar 18, 2021

ArturKovacs suggested changes Mar 18, 2021

View reviewed changes

src/keyboard.rs Outdated Show resolved Hide resolved

src/keyboard.rs Outdated Show resolved Hide resolved

Kethku mentioned this pull request Mar 21, 2021

OpenGL Renderer neovide/neovide#491

Closed

maroider mentioned this pull request Mar 22, 2021

KeyCode: Numpad key triggers regular key bevyengine/bevy#1666

Closed

maroider mentioned this pull request Apr 7, 2021

Meta Issue: Keyboard input #1806

Closed

This was referenced Apr 18, 2021

A keyboard input model #753

Closed

New keyboard API for Windows #1788

Merged

mockersf mentioned this pull request May 2, 2021

Improve keyboard input with Input<ScanCode> bevyengine/bevy#2052

Closed

maroider added DS - x11 C - in progress Implementation is proceeding smoothly DS - web and removed DS - x11 labels May 7, 2021

maroider mentioned this pull request May 7, 2021

New keyboard API for Linux #1932

Closed

20 tasks

maroider added this to the Keyboard Events Overhaul milestone May 7, 2021

maroider force-pushed the new-keyboard-web branch 3 times, most recently from 9e9b317 to d7c9b81 Compare May 14, 2021 16:13

maroider force-pushed the new-keyboard-web branch 2 times, most recently from 3391b55 to 16bb871 Compare May 14, 2021 16:29

ArturKovacs suggested changes May 15, 2021

View reviewed changes

ArturKovacs reviewed Jun 21, 2021

View reviewed changes

maroider mentioned this pull request Aug 25, 2021

Can not input the Chinese character on wasm32 platform #2012

Closed

maroider mentioned this pull request Dec 21, 2021

scan_code and virtual_key_code are swapped #2092

Closed

maroider force-pushed the new-keyboard-web branch from c75b536 to 2adcc7e Compare January 10, 2022 17:07

maroider added 13 commits January 26, 2022 03:03

Get the web-sys backend to compile

a1ff0d3

Get a super-minimal MVP for web-sys

1ab522b

Use a literal whitespace character

c687a4e

Move conversion functions into a web submodule

ccef4cf

Remove embedding of non-matched strings

e8d14a4

This is to appease `serde` for the time being.

Wire up ReceivedImeText

74851a4

Add ModifiersChanged events

7e41dd9

Remove redundant key_repeat function

8aa2b4f

Resolve decision on key location values

ffb3220

Move the KeyEventExtra struct somewhere else

fcb24eb

Use a String in NativeKeyCode::Web.

46f22cf

As a direct result of this, `NativeKeyCode`, `Key`, `KeyCode` and `RawKeyEvent` no longer implement `Copy`.

Fix text for Space, Tab, and Enter

39e84cc

Use a proper string cache

2c7ba77

maroider force-pushed the new-keyboard-web branch from 2adcc7e to 2c7ba77 Compare January 26, 2022 02:03

ArturKovacs force-pushed the new-keyboard branch from 1ef0b49 to 54bb969 Compare June 12, 2022 08:19

Liamolucko reviewed Jun 13, 2022

View reviewed changes

rparrett mentioned this pull request Sep 2, 2022

web: Input is broken on international or non-qwerty keyboard layouts rparrett/taipo#1

Closed

maroider mentioned this pull request Jan 31, 2023

Keyboard Input 2.0 #2662

Merged

5 tasks

kchibisov mentioned this pull request May 5, 2023

[FOLLOW UP] Keyboard Input 2.0 #2789

Closed

kchibisov closed this in #2662 May 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New keyboard API for the web #1888

New keyboard API for the web #1888

maroider commented Mar 18, 2021 •

edited

Loading

ArturKovacs commented Apr 29, 2021

maroider commented May 14, 2021

ArturKovacs left a comment

ArturKovacs May 15, 2021

maroider Jun 21, 2021

maroider Jun 21, 2021

Liamolucko Jun 13, 2022

ArturKovacs commented May 15, 2021

maroider commented Jun 21, 2021

ArturKovacs Jun 21, 2021

Woyten commented Dec 21, 2021

Liamolucko Jun 13, 2022

Liamolucko Jun 13, 2022

Liamolucko Jun 13, 2022

Liamolucko Jun 13, 2022

Liamolucko Jun 13, 2022

Liamolucko Jun 13, 2022

Liamolucko Jun 13, 2022

erwanvivien commented Jan 3, 2023

-pub fn keyboard_modifiers(key: &Key<'_>) -> ModifiersState {
+/// Returns the keyboard modifiers that changed with this keypress.
+///
+/// For a pressed key, this is the keyboard modifiers that are newly pressed,
+/// and for a released key, this is the keyboard modifiers that are newly released.
+pub fn keyboard_modifiers(key: &Key<'_>) -> ModifiersState {

New keyboard API for the web #1888

New keyboard API for the web #1888

Conversation

maroider commented Mar 18, 2021 • edited Loading

TODOs

ArturKovacs commented Apr 29, 2021

maroider commented May 14, 2021

ArturKovacs left a 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

ArturKovacs commented May 15, 2021

maroider commented Jun 21, 2021

Choose a reason for hiding this comment

Woyten commented Dec 21, 2021

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

erwanvivien commented Jan 3, 2023

maroider commented Mar 18, 2021 •

edited

Loading