Skip to content

Comments

android: Add mapping from NDK Keycode to VirtualKeyCode#2226

Merged
msiglreith merged 2 commits intorust-windowing:masterfrom
torokati44:android-keycodes
Apr 1, 2022
Merged

android: Add mapping from NDK Keycode to VirtualKeyCode#2226
msiglreith merged 2 commits intorust-windowing:masterfrom
torokati44:android-keycodes

Conversation

@torokati44
Copy link
Contributor

@torokati44 torokati44 commented Mar 26, 2022 •
edited
Loading

When working on https://github.com/torokati44/ruffle-android, I discovered that all keypress events arrive with their scancode set to 0, and their keycode set to None. Which I found less than useful.

The NDK docs for AKeyEvent_getScanCode mention that "These values are not reliable and vary from device to device.".
So I guess always setting them to 0 is acceptable from their end.

Tested this on Android 12, with a keyboard connected to my computer, through wired adb and https://github.com/Genymobile/scrcpy/, and it seemed to work fine.

AFAICT This fixes #1867.

  • Tested on all platforms changed
  • Only Android is affected, and I am testing on it.
  • 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
  • I have not found any existing documentation that would have to be updated due to this change.
  • Created or updated an example program if it would help users understand this functionality
  • No new functionality, only implementing/fixing an existing one.
  • Updated feature matrix, if new features were added or implemented
  • The change is not significant enough to warrant updating the matrix.

@msiglreith msiglreith added the DS - android Affects the Android backend label Mar 26, 2022
@torokati44 torokati44 force-pushed the android-keycodes branch 2 times, most recently from 1d35f8e to ea442b0 Compare March 26, 2022 19:54
@torokati44 torokati44 marked this pull request as ready for review March 26, 2022 19:57
@torokati44 torokati44 changed the title WIP android: Add mapping from NDK Keycode to VirtualKeyCode android: Add mapping from NDK Keycode to VirtualKeyCode Mar 26, 2022
@torokati44
Copy link
Contributor Author

torokati44 commented Mar 26, 2022 •
edited
Loading

I think this is now reviewable.

I have left quite a few ??? comments in there where it wasn't immediately obvious what the mapping should be.
What should I do about those? Delete those comments and accept the current mapping as a whole as "good enough", or remove any mapped keys which may not be a perfect match? Or leave the uncertainty in there with those comments?

@msiglreith msiglreith self-requested a review March 26, 2022 21:52
@msiglreith msiglreith added the C - waiting on maintainer A maintainer must review this code label Mar 26, 2022
msiglreith
msiglreith approved these changes Apr 1, 2022
View reviewed changes
Copy link
Member

@msiglreith msiglreith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! the less clear cases are fine in my eyes

@msiglreith msiglreith merged commit 52c4670 into rust-windowing:master Apr 1, 2022
@msiglreith msiglreith mentioned this pull request Apr 28, 2022
Open
MarijnS95
MarijnS95 reviewed May 26, 2022
View reviewed changes
src/platform_impl/android/mod.rs
Keycode::MoveEnd => Some(VirtualKeyCode::End),
Keycode::Insert => Some(VirtualKeyCode::Insert),

Keycode::Del => Some(VirtualKeyCode::Back), // Backspace (above Enter)
Copy link
Member

@MarijnS95 MarijnS95 May 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started playing about with these codes, and it seems you haven't implemented a mapping for KeyCode::Back which would be needed to handle a graceful app exit (or going to the previous page) when the user presses it. The implementation for VirtualKeyCode::Back reads:

winit/src/event.rs

Lines 962 to 964 in f04fa5d

/// The Backspace key, right over Enter.
// TODO: rename
Back,

So I don't think we should add a new Back VirtualKeyCode and rename existing uses to Backspace, as that's really hard to notice for end-users upgrading their winit. Perhaps we can piggyback NavigateBackward but android already supports that as KeyCode::NavigatePrevious:

Keycode::Back => Some(VirtualKeyCode::NavigateBackward),

How do you think we should go forward?

@MarijnS95 MarijnS95 mentioned this pull request May 26, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MarijnS95 MarijnS95 MarijnS95 left review comments

+1 more reviewer

@msiglreith msiglreith msiglreith approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

C - waiting on maintainer A maintainer must review this code DS - android Affects the Android backend

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Android: Provide keycode for KeyEvents

3 participants

@torokati44 @MarijnS95 @msiglreith