[fuchsia] Fixes the HID usage handling (flutter/engine#29815)

The old code stripped the USB hid usage page value from the Fuchsia
key.  It shouldn't be doing that.  This change fixes the issue,
and adds tests that rely on the key constants to verify that the
change indeed does what it is supposed to do.

In the process of fixing this, filed a few known issues and marked them
as TODO. These issues should be handled in separate PRs.

Fixes: https://github.com/flutter/flutter/issues/93890
This commit is contained in:
Filip Filmar 2021-11-29 15:41:17 -08:00 committed by GitHub
parent 22c822564d
commit e2dbdf16bd
4 changed files with 79 additions and 12 deletions

View File

@ -285,7 +285,7 @@ bool Keyboard::IsShift() {
}
bool Keyboard::IsKeys() {
return (static_cast<uint64_t>(last_event_.key()) & 0xFFFF0000) == 0x00070000;
return LastHIDUsagePage() == 0x7;
}
uint32_t Keyboard::Modifiers() {
@ -298,12 +298,16 @@ uint32_t Keyboard::Modifiers() {
}
uint32_t Keyboard::LastCodePoint() {
// TODO(https://github.com/flutter/flutter/issues/93891):
// `KeyEvent::key_meaning` should be used here to do key mapping before US
// QWERTY is applied. The US QWERTY should be applied only if `key_meaning`
// is not available.
static const int qwerty_map_size =
sizeof(QWERTY_TO_CODE_POINTS) / sizeof(QWERTY_TO_CODE_POINTS[0]);
if (!IsKeys()) {
return 0;
}
const auto usage = LastHIDUsage();
const auto usage = LastHIDUsageID();
if (usage < qwerty_map_size) {
return QWERTY_TO_CODE_POINTS[usage][IsShift() & 1];
}
@ -311,8 +315,22 @@ uint32_t Keyboard::LastCodePoint() {
return 0;
}
uint32_t Keyboard::GetLastKey() {
// TODO(https://github.com/flutter/flutter/issues/93898): key is not always
// set. Figure out what to do then.
return static_cast<uint32_t>(last_event_.key());
}
uint32_t Keyboard::LastHIDUsage() {
return static_cast<uint64_t>(last_event_.key()) & 0xFFFF;
return GetLastKey() & 0xFFFFFFFF;
}
uint16_t Keyboard::LastHIDUsageID() {
return GetLastKey() & 0xFFFF;
}
uint16_t Keyboard::LastHIDUsagePage() {
return (GetLastKey() >> 16) & 0xFFFF;
}
} // namespace flutter_runner

View File

@ -28,9 +28,26 @@ class Keyboard final {
// the state of the modifier keys.
uint32_t LastCodePoint();
// Gets the last encountered HID usage.
// Gets the last encountered HID usage. This is a 32-bit number, with the
// upper 16 bits equal to `LastHidUsagePage()`, and the lower 16 bits equal
// to `LastHIDUsageID()`.
//
// The key corresponding to A will have the usage 0x7004. This function will
// return 0x7004 in that case.
uint32_t LastHIDUsage();
// Gets the last encountered HID usage page.
//
// The key corresponding to A will have the usage 0x7004. This function will
// return 0x7 in that case.
uint16_t LastHIDUsagePage();
// Gets the last encountered HID usage ID.
//
// The key corresponding to A will have the usage 0x7004. This function will
// return 0x4 in that case.
uint16_t LastHIDUsageID();
private:
// Return true if any level shift is active.
bool IsShift();
@ -39,6 +56,9 @@ class Keyboard final {
// point associated.
bool IsKeys();
// Returns the value of the last key as a uint32_t.
uint32_t GetLastKey();
// Set to false until any event is received.
bool any_events_received_ : 1;

View File

@ -60,12 +60,40 @@ class KeyboardTest : public testing::Test {
}
// Converts a pressed key to usage value.
uint32_t ToUsage(Key key) { return static_cast<uint64_t>(key) & 0xFFFF; }
uint32_t ToUsage(Key key) { return static_cast<uint64_t>(key) & 0xFFFFFFFF; }
private:
zx_time_t timestamp_ = 0;
};
// Checks whether the HID usage, page and ID values are reported correctly.
TEST_F(KeyboardTest, UsageValues) {
std::vector<KeyEvent> keys;
keys.emplace_back(NewKeyEvent(KeyEventType::SYNC, Key::CAPS_LOCK));
Keyboard keyboard;
ConsumeEvents(&keyboard, keys);
// Values for Caps Lock.
// See spec at:
// https://cs.opensource.google/fuchsia/fuchsia/+/main:sdk/fidl/fuchsia.input/keys.fidl;l=177;drc=e3b39f2b57e720770773b857feca4f770ee0619e
EXPECT_EQ(0x07u, keyboard.LastHIDUsagePage());
EXPECT_EQ(0x39u, keyboard.LastHIDUsageID());
EXPECT_EQ(0x70039u, keyboard.LastHIDUsage());
// Try also an usage that is not on page 7. This one is on page 0x0C.
// See spec at:
// https://cs.opensource.google/fuchsia/fuchsia/+/main:sdk/fidl/fuchsia.input/keys.fidl;l=339;drc=e3b39f2b57e720770773b857feca4f770ee0619e
// Note that Fuchsia does not define constants for every key you may think of,
// rather only those that we had the need for. However it is not an issue
// to add more keys if needed.
keys.clear();
keys.emplace_back(NewKeyEvent(KeyEventType::SYNC, Key::MEDIA_MUTE));
ConsumeEvents(&keyboard, keys);
EXPECT_EQ(0x0Cu, keyboard.LastHIDUsagePage());
EXPECT_EQ(0xE2u, keyboard.LastHIDUsageID());
EXPECT_EQ(0xC00E2u, keyboard.LastHIDUsage());
}
// This test checks that if a caps lock has been pressed when we didn't have
// focus, the effect of caps lock remains. Only this first test case is
// commented to explain how the test case works.

View File

@ -1246,18 +1246,19 @@ TEST_F(PlatformViewTests, OnKeyEvent) {
std::vector<EventFlow> events;
// Press A. Get 'a'.
// The HID usage for the key A is 0x70004, or 458756.
events.emplace_back(EventFlow{
MakeEvent(fuchsia::ui::input3::KeyEventType::PRESSED, std::nullopt,
fuchsia::input::Key::A),
fuchsia::ui::input3::KeyEventStatus::HANDLED,
R"({"type":"keydown","keymap":"fuchsia","hidUsage":4,"codePoint":97,"modifiers":0})",
R"({"type":"keydown","keymap":"fuchsia","hidUsage":458756,"codePoint":97,"modifiers":0})",
});
// Release A. Get 'a' release.
events.emplace_back(EventFlow{
MakeEvent(fuchsia::ui::input3::KeyEventType::RELEASED, std::nullopt,
fuchsia::input::Key::A),
fuchsia::ui::input3::KeyEventStatus::HANDLED,
R"({"type":"keyup","keymap":"fuchsia","hidUsage":4,"codePoint":97,"modifiers":0})",
R"({"type":"keyup","keymap":"fuchsia","hidUsage":458756,"codePoint":97,"modifiers":0})",
});
// Press CAPS_LOCK. Modifier now active.
events.emplace_back(EventFlow{
@ -1265,14 +1266,14 @@ TEST_F(PlatformViewTests, OnKeyEvent) {
fuchsia::ui::input3::Modifiers::CAPS_LOCK,
fuchsia::input::Key::CAPS_LOCK),
fuchsia::ui::input3::KeyEventStatus::HANDLED,
R"({"type":"keydown","keymap":"fuchsia","hidUsage":57,"codePoint":0,"modifiers":1})",
R"({"type":"keydown","keymap":"fuchsia","hidUsage":458809,"codePoint":0,"modifiers":1})",
});
// Pres A. Get 'A'.
// Press A. Get 'A'.
events.emplace_back(EventFlow{
MakeEvent(fuchsia::ui::input3::KeyEventType::PRESSED, std::nullopt,
fuchsia::input::Key::A),
fuchsia::ui::input3::KeyEventStatus::HANDLED,
R"({"type":"keydown","keymap":"fuchsia","hidUsage":4,"codePoint":65,"modifiers":1})",
R"({"type":"keydown","keymap":"fuchsia","hidUsage":458756,"codePoint":65,"modifiers":1})",
});
// Release CAPS_LOCK.
events.emplace_back(EventFlow{
@ -1280,7 +1281,7 @@ TEST_F(PlatformViewTests, OnKeyEvent) {
fuchsia::ui::input3::Modifiers::CAPS_LOCK,
fuchsia::input::Key::CAPS_LOCK),
fuchsia::ui::input3::KeyEventStatus::HANDLED,
R"({"type":"keyup","keymap":"fuchsia","hidUsage":57,"codePoint":0,"modifiers":1})",
R"({"type":"keyup","keymap":"fuchsia","hidUsage":458809,"codePoint":0,"modifiers":1})",
});
// Press A again. This time get 'A'.
// CAPS_LOCK is latched active even if it was just released.
@ -1288,7 +1289,7 @@ TEST_F(PlatformViewTests, OnKeyEvent) {
MakeEvent(fuchsia::ui::input3::KeyEventType::PRESSED, std::nullopt,
fuchsia::input::Key::A),
fuchsia::ui::input3::KeyEventStatus::HANDLED,
R"({"type":"keydown","keymap":"fuchsia","hidUsage":4,"codePoint":65,"modifiers":1})",
R"({"type":"keydown","keymap":"fuchsia","hidUsage":458756,"codePoint":65,"modifiers":1})",
});
for (const auto& event : events) {