Commit 79fe108c authored by Tom Anderson's avatar Tom Anderson Committed by Commit Bot

[XProto] Convert keycodes and keysyms using XKB when available

This CL removes usage of XLookupString() that was temporarily
introduced by [1].

The issue is that the XKB keyboard map wasn't being used when XKB was
available -- the core keyboard map was always being used.  This meant
that XKB-specific features like keyboard groups and fifth-level shift
became unsupported.

This CL separates the XKB and non-XKB codepaths into 2 classes:
XkbKeyboardState and CoreKeyboardState.  This CL also moves the
keyboard-related code out of connection.cc into keyboard_state.cc.

[1] https://source.chromium.org/chromium/chromium/src/+/a2fd175c6a692351827aacd8e1285d2190ccd4e4

R=sky
BUG=1066670

Change-Id: I26023f87c396f5728ce32c0d1fb0275e0335db15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2464786
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821414}
parent 9a17da72
......@@ -102,7 +102,7 @@ bool GlobalShortcutListenerX11::RegisterAcceleratorImpl(
auto modifiers = GetNativeModifiers(accelerator);
auto keysym = XKeysymForWindowsKeyCode(accelerator.key_code(), false);
auto keycode = connection_->KeysymToKeycode(static_cast<x11::KeySym>(keysym));
auto keycode = connection_->KeysymToKeycode(keysym);
// Because XGrabKey only works on the exact modifiers mask, we should register
// our hot keys with modifiers that we want to ignore, including Num lock,
......@@ -134,7 +134,7 @@ void GlobalShortcutListenerX11::UnregisterAcceleratorImpl(
auto modifiers = GetNativeModifiers(accelerator);
auto keysym = XKeysymForWindowsKeyCode(accelerator.key_code(), false);
auto keycode = connection_->KeysymToKeycode(static_cast<x11::KeySym>(keysym));
auto keycode = connection_->KeysymToKeycode(keysym);
for (auto mask : kModifiersMasks)
connection_->UngrabKey({keycode, x_root_window_, modifiers | mask});
......
......@@ -112,10 +112,8 @@ bool GetXModifierMask(x11::Connection* connection,
int max_mod_keys = mod_map->keycodes_per_modifier;
for (int mod_index = 0; mod_index <= 8; ++mod_index) {
for (int key_index = 0; key_index < max_mod_keys; ++key_index) {
auto key = static_cast<uint8_t>(
mod_map->keycodes[mod_index * max_mod_keys + key_index]);
int keysym =
static_cast<int>(x11::Connection::Get()->KeycodeToKeysym(key, 0));
auto key = mod_map->keycodes[mod_index * max_mod_keys + key_index];
auto keysym = x11::Connection::Get()->KeycodeToKeysym(key, 0);
if (modifier == kAltKeyModifierMask)
found = keysym == XK_Alt_L || keysym == XK_Alt_R;
else if (modifier == kMetaKeyModifierMask)
......
......@@ -124,9 +124,9 @@ void UserInputMonitorLinuxCore::DispatchXEvent(x11::Event* event) {
? ui::ET_KEY_PRESSED
: ui::ET_KEY_RELEASED;
auto key_sym = connection_->KeycodeToKeysym(raw->detail, 0);
ui::KeyboardCode key_code =
ui::KeyboardCodeFromXKeysym(static_cast<uint32_t>(key_sym));
auto key_sym =
connection_->KeycodeToKeysym(static_cast<x11::KeyCode>(raw->detail), 0);
ui::KeyboardCode key_code = ui::KeyboardCodeFromXKeysym(key_sym);
counter_.OnKeyboardEvent(type, key_code);
// Update count value in shared memory.
......
......@@ -385,8 +385,8 @@ bool InputInjectorX11::Core::IsLockKey(KeyCode keycode) {
if (!state)
return false;
auto mods = state->baseMods | state->latchedMods | state->lockedMods;
auto keysym = static_cast<uint32_t>(
connection_.KeycodeToKeysym(keycode, static_cast<unsigned>(mods)));
auto keysym = connection_.KeycodeToKeysym(static_cast<x11::KeyCode>(keycode),
static_cast<unsigned>(mods));
if (state && keysym)
return keysym == XK_Caps_Lock || keysym == XK_Num_Lock;
else
......
......@@ -188,18 +188,17 @@ void LocalHotkeyInputMonitorX11::Core::DispatchXEvent(x11::Event* event) {
DCHECK(input_task_runner_->BelongsToCurrentThread());
// Ignore input if we've already initiated a disconnect.
if (!disconnect_callback_) {
if (!disconnect_callback_)
return;
}
auto* raw = event->As<x11::Input::RawDeviceEvent>();
const auto* raw = event->As<x11::Input::RawDeviceEvent>();
DCHECK(raw);
DCHECK(raw->opcode == x11::Input::RawDeviceEvent::RawKeyPress ||
raw->opcode == x11::Input::RawDeviceEvent::RawKeyRelease);
bool down = raw->opcode == x11::Input::RawDeviceEvent::RawKeyPress;
auto key_sym =
static_cast<uint32_t>(connection_->KeycodeToKeysym(raw->detail, 0));
const bool down = raw->opcode == x11::Input::RawDeviceEvent::RawKeyPress;
const auto key_sym =
connection_->KeycodeToKeysym(static_cast<x11::KeyCode>(raw->detail), 0);
if (key_sym == XK_Control_L || key_sym == XK_Control_R)
ctrl_pressed_ = down;
......
......@@ -16,11 +16,10 @@
namespace {
bool FindKeycodeForKeySym(x11::Connection* connection,
x11::KeySym key_sym,
uint32_t key_sym,
uint32_t* keycode,
uint32_t* modifiers) {
auto found_keycode =
static_cast<uint32_t>(connection->KeysymToKeycode(key_sym));
auto found_keycode = connection->KeysymToKeycode(key_sym);
const x11::KeyButMask kModifiersToTry[] = {
{},
......@@ -38,7 +37,7 @@ bool FindKeycodeForKeySym(x11::Connection* connection,
auto mods = static_cast<uint32_t>(i);
if (connection->KeycodeToKeysym(found_keycode, mods) == key_sym) {
*modifiers = mods;
*keycode = found_keycode;
*keycode = static_cast<uint8_t>(found_keycode);
return true;
}
}
......@@ -97,10 +96,8 @@ bool X11KeyboardImpl::FindKeycode(uint32_t code_point,
uint32_t* keycode,
uint32_t* modifiers) {
for (uint32_t keysym : GetKeySymsForUnicode(code_point)) {
if (FindKeycodeForKeySym(connection_, static_cast<x11::KeySym>(keysym),
keycode, modifiers)) {
if (FindKeycodeForKeySym(connection_, keysym, keycode, modifiers))
return true;
}
}
return false;
}
......
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
......@@ -58,7 +58,7 @@ bool UIControlsX11::SendKeyPressNotifyWhenDone(gfx::NativeWindow window,
if (command)
SetKeycodeAndSendThenMask(&xevent, XK_Meta_L, x11::KeyButMask::Mod4);
xevent.detail = x11::Connection::Get()->KeysymToKeycode(
static_cast<x11::KeySym>(ui::XKeysymForWindowsKeyCode(key, shift)));
ui::XKeysymForWindowsKeyCode(key, shift));
PostEventToWindowTreeHost(host_, &xevent);
// Send key release events.
......@@ -191,8 +191,7 @@ void UIControlsX11::RunClosureAfterAllPendingUIEvents(
void UIControlsX11::SetKeycodeAndSendThenMask(x11::KeyEvent* xevent,
KeySym keysym,
x11::KeyButMask mask) {
xevent->detail =
x11::Connection::Get()->KeysymToKeycode(static_cast<x11::KeySym>(keysym));
xevent->detail = x11::Connection::Get()->KeysymToKeycode(keysym);
PostEventToWindowTreeHost(host_, xevent);
xevent->state = xevent->state | mask;
}
......@@ -202,8 +201,7 @@ void UIControlsX11::UnmaskAndSetKeycodeThenSend(x11::KeyEvent* xevent,
KeySym keysym) {
xevent->state = static_cast<x11::KeyButMask>(
static_cast<uint32_t>(xevent->state) ^ static_cast<uint32_t>(mask));
xevent->detail =
x11::Connection::Get()->KeysymToKeycode(static_cast<x11::KeySym>(keysym));
xevent->detail = x11::Connection::Get()->KeysymToKeycode(keysym);
PostEventToWindowTreeHost(host_, xevent);
}
......
......@@ -78,7 +78,7 @@ void X11UIControlsTestHelper::SendKeyPressEvent(gfx::AcceleratedWidget widget,
if (alt)
SetKeycodeAndSendThenMask(widget, &xevent, XK_Alt_L, x11::KeyButMask::Mod1);
xevent.detail = x11::Connection::Get()->KeysymToKeycode(
static_cast<x11::KeySym>(ui::XKeysymForWindowsKeyCode(key, shift)));
ui::XKeysymForWindowsKeyCode(key, shift));
PostEventToWindowTreeHost(widget, &xevent);
// Send key release events.
......@@ -183,8 +183,7 @@ void X11UIControlsTestHelper::SetKeycodeAndSendThenMask(
x11::KeyEvent* xevent,
KeySym keysym,
x11::KeyButMask mask) {
xevent->detail =
x11::Connection::Get()->KeysymToKeycode(static_cast<x11::KeySym>(keysym));
xevent->detail = x11::Connection::Get()->KeysymToKeycode(keysym);
PostEventToWindowTreeHost(widget, xevent);
xevent->state = xevent->state | mask;
}
......@@ -196,8 +195,7 @@ void X11UIControlsTestHelper::UnmaskAndSetKeycodeThenSend(
KeySym keysym) {
xevent->state = static_cast<x11::KeyButMask>(
static_cast<uint32_t>(xevent->state) ^ static_cast<uint32_t>(mask));
xevent->detail =
x11::Connection::Get()->KeysymToKeycode(static_cast<x11::KeySym>(keysym));
xevent->detail = x11::Connection::Get()->KeysymToKeycode(keysym);
PostEventToWindowTreeHost(widget, xevent);
}
......
......@@ -27,6 +27,7 @@
#include "ui/events/x/events_x_utils.h"
#include "ui/events/x/x11_window_event_manager.h"
#include "ui/gfx/x/connection.h"
#include "ui/gfx/x/keysyms/keysyms.h"
#include "ui/gfx/x/x11.h"
#include "ui/gfx/x/xproto.h"
......@@ -34,8 +35,6 @@ namespace ui {
namespace {
constexpr x11::KeySym kEscKeysym = static_cast<x11::KeySym>(0xff1b);
// XGrabKey requires the modifier mask to explicitly be specified.
constexpr x11::ModMask kModifiersMasks[] = {
{}, // No additional modifier.
......@@ -224,7 +223,7 @@ void X11WholeScreenMoveLoop::EndMoveLoop() {
UpdateCursor(initial_cursor_);
auto* connection = x11::Connection::Get();
auto esc_keycode = connection->KeysymToKeycode(kEscKeysym);
auto esc_keycode = connection->KeysymToKeycode(XK_Escape);
for (auto mask : kModifiersMasks)
connection->UngrabKey({esc_keycode, grab_input_window_, mask});
......@@ -255,7 +254,7 @@ bool X11WholeScreenMoveLoop::GrabPointer(scoped_refptr<X11Cursor> cursor) {
void X11WholeScreenMoveLoop::GrabEscKey() {
auto* connection = x11::Connection::Get();
auto esc_keycode = connection->KeysymToKeycode(kEscKeysym);
auto esc_keycode = connection->KeysymToKeycode(XK_Escape);
for (auto mask : kModifiersMasks) {
connection->GrabKey({false, grab_input_window_, mask, esc_keycode,
x11::GrabMode::Async, x11::GrabMode::Async});
......
......@@ -238,8 +238,7 @@ TEST(WebInputEventTest, TestMakeWebKeyboardEventKeyPadKeyCode) {
xev.InitKeyEvent(ET_KEY_PRESSED, test_case.ui_keycode, EF_NONE);
x11::Event* x11_event = xev;
auto keycode = x11::Connection::Get()->KeysymToKeycode(
static_cast<x11::KeySym>(test_case.x_keysym));
auto keycode = x11::Connection::Get()->KeysymToKeycode(test_case.x_keysym);
if (keycode == x11::KeyCode{})
continue;
x11_event->As<x11::KeyEvent>()->detail = keycode;
......
......@@ -557,9 +557,9 @@ bool IsTtyFunctionOrSpaceKey(KeySym keysym) {
return false;
}
::KeySym TranslateKey(uint32_t keycode, uint32_t modifiers) {
auto* connection = x11::Connection::Get();
return static_cast<::KeySym>(connection->KeycodeToKeysym(keycode, modifiers));
uint32_t TranslateKey(uint32_t keycode, uint32_t modifiers) {
return x11::Connection::Get()->KeycodeToKeysym(
static_cast<x11::KeyCode>(keycode), modifiers);
}
void GetKeycodeAndModifiers(const x11::Event& event,
......@@ -1484,9 +1484,8 @@ unsigned int XKeyCodeForWindowsKeyCode(ui::KeyboardCode key_code,
// crbug.com/386066 and crbug.com/390263 are examples of problems
// associated with this.
//
auto keysym =
static_cast<x11::KeySym>(XKeysymForWindowsKeyCode(key_code, false));
return static_cast<unsigned int>(connection->KeysymToKeycode(keysym));
return static_cast<uint8_t>(
connection->KeysymToKeycode(XKeysymForWindowsKeyCode(key_code, false)));
}
} // namespace ui
......@@ -98,6 +98,8 @@ component("xprotos") {
"event.cc",
"error.h",
"error.cc",
"keyboard_state.h",
"keyboard_state.cc",
"scoped_ignore_errors.h",
"scoped_ignore_errors.cc",
"x11_switches.cc",
......
include_rules = [
"+ui/events/platform/platform_event_source.h",
"+third_party/libx11",
"+third_party/libxcb-keysyms",
"+third_party/x11proto",
]
This diff is collapsed.
......@@ -17,6 +17,8 @@
namespace x11 {
class KeyboardState;
// Represents a socket to the X11 server.
class COMPONENT_EXPORT(X11) Connection : public XProto,
public ExtensionManager {
......@@ -130,9 +132,9 @@ class COMPONENT_EXPORT(X11) Connection : public XProto,
// doesn't exist or only exists on a non-default screen.
const VisualInfo* GetVisualInfoFromId(VisualId id) const;
KeyCode KeysymToKeycode(KeySym keysym);
KeyCode KeysymToKeycode(uint32_t keysym) const;
KeySym KeycodeToKeysym(uint32_t keycode, unsigned int modifiers);
uint32_t KeycodeToKeysym(KeyCode keycode, uint32_t modifiers) const;
// Access the event buffer. Clients can add, delete, or modify events.
std::list<Event>& events() {
......@@ -176,12 +178,6 @@ class COMPONENT_EXPORT(X11) Connection : public XProto,
int ScreenIndexFromRootWindow(x11::Window root) const;
void ResetKeyboardState();
KeySym KeyCodetoKeySym(KeyCode keycode, int column) const;
KeySym TranslateKey(uint32_t keycode, unsigned int modifiers) const;
// This function is implemented in the generated read_error.cc.
void InitErrorParsers();
......@@ -203,12 +199,7 @@ class COMPONENT_EXPORT(X11) Connection : public XProto,
std::unordered_map<VisualId, VisualInfo> default_screen_visuals_;
// Keyboard state.
GetKeyboardMappingReply keyboard_mapping_;
GetModifierMappingReply modifier_mapping_;
uint16_t lock_meaning_ = 0;
uint8_t mode_switch_ = 0;
uint8_t num_lock_ = 0;
std::unique_ptr<KeyboardState> keyboard_state_;
std::list<Event> events_;
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "ui/gfx/x/keyboard_state.h"
#include "base/i18n/case_conversion.h"
#include "ui/gfx/x/connection.h"
#include "ui/gfx/x/keysyms/keysyms.h"
#include "ui/gfx/x/xkb.h"
#include "ui/gfx/x/xproto.h"
namespace x11 {
namespace {
constexpr KeySym kNoSymbol = static_cast<KeySym>(0);
void ConvertCaseImpl(uint32_t sym, uint32_t* lower, uint32_t* upper);
void ConvertCase(KeySym sym, KeySym* lower, KeySym* upper) {
uint32_t lower32;
uint32_t upper32;
ConvertCaseImpl(static_cast<uint32_t>(sym), &lower32, &upper32);
*lower = static_cast<KeySym>(lower32);
*upper = static_cast<KeySym>(upper32);
}
bool IsPublicOrPrivateKeypadKey(KeySym keysym) {
auto key = static_cast<uint32_t>(keysym);
return (key >= XK_KP_Space && key <= XK_KP_Equal) ||
(key >= 0x11000000 && key <= 0x1100FFFF);
}
int GetXkbGroupFromState(int state) {
return (state >> 13) & 0x3;
}
#include "third_party/libx11/src/KeyBind.c"
#include "third_party/libx11/src/xkb/XKBBind.c"
#include "third_party/libxcb-keysyms/keysyms/keysyms.c"
} // namespace
KeyboardState::KeyboardState() = default;
KeyboardState::~KeyboardState() = default;
// Non-XKB (core protocol) implementation of KeysymToKeycode and
// KeycodeToKeysym.
class CoreKeyboardState : public KeyboardState {
public:
explicit CoreKeyboardState(Connection* connection) : connection_(connection) {
UpdateMapping();
}
~CoreKeyboardState() override = default;
KeyCode KeysymToKeycode(uint32_t keysym) const override {
auto min_keycode = static_cast<uint8_t>(connection_->setup().min_keycode);
auto max_keycode = static_cast<uint8_t>(connection_->setup().max_keycode);
int count = max_keycode - min_keycode + 1;
DCHECK_EQ(count * keyboard_mapping_.keysyms_per_keycode,
static_cast<int>(keyboard_mapping_.keysyms.size()));
for (size_t i = 0; i < keyboard_mapping_.keysyms.size(); i++) {
auto keycode = min_keycode + i / keyboard_mapping_.keysyms_per_keycode;
if (keyboard_mapping_.keysyms[i] == static_cast<x11::KeySym>(keysym))
return static_cast<KeyCode>(keycode);
}
return {};
}
uint32_t KeycodeToKeysym(KeyCode keycode, uint32_t modifiers) const override {
auto sym = static_cast<uint32_t>(KeycodeToKeysymCoreImpl(
keycode, modifiers, connection_, keyboard_mapping_, lock_meaning_,
mode_switch_, num_lock_));
return sym == XK_VoidSymbol ? 0 : sym;
}
private:
void UpdateMapping() override {
UpdateMappingImpl(connection_, &keyboard_mapping_, &lock_meaning_,
&mode_switch_, &num_lock_);
}
x11::Connection* const connection_;
GetKeyboardMappingReply keyboard_mapping_;
uint16_t lock_meaning_ = 0;
uint8_t mode_switch_ = 0;
uint8_t num_lock_ = 0;
};
// XKB implementation of KeysymToKeycode and KeycodeToKeysym.
class XkbKeyboardState : public KeyboardState {
public:
explicit XkbKeyboardState(Connection* connection) : connection_(connection) {
UpdateMapping();
}
~XkbKeyboardState() override = default;
KeyCode KeysymToKeycode(uint32_t keysym) const override {
int first_keycode = static_cast<int>(map_.firstKeySym);
for (int keycode = 0; keycode < map_.nKeySyms; keycode++) {
for (auto sym : map_.syms_rtrn->at(keycode).syms) {
if (static_cast<uint32_t>(sym) == keysym)
return static_cast<KeyCode>(keycode + first_keycode);
}
}
return {};
}
uint32_t KeycodeToKeysym(KeyCode key, uint32_t modifiers) const override {
return KeycodeToKeysymXkbImpl(key, modifiers, map_);
}
private:
void UpdateMapping() override {
auto future = connection_->xkb().GetMap(
{static_cast<Xkb::DeviceSpec>(Xkb::Id::UseCoreKbd),
Xkb::MapPart::KeyTypes | Xkb::MapPart::KeySyms});
if (auto response = future.Sync())
map_ = std::move(*response.reply);
}
x11::Connection* const connection_;
Xkb::GetMapReply map_;
};
std::unique_ptr<KeyboardState> CreateKeyboardState(Connection* connection) {
if (connection->xkb().present())
return std::make_unique<XkbKeyboardState>(connection);
return std::make_unique<CoreKeyboardState>(connection);
}
} // namespace x11
// Copyright 2020 The Chromium 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 UI_GFX_X_KEYBOARD_STATE_H_
#define UI_GFX_X_KEYBOARD_STATE_H_
#include <memory>
#include "ui/gfx/x/xproto.h"
namespace x11 {
class Connection;
// This is an interface used by Connection to manage conversion between keycodes
// (8 bit values) and keysyms (32 bit values).
class KeyboardState {
public:
KeyboardState();
virtual ~KeyboardState();
virtual KeyCode KeysymToKeycode(uint32_t keysym) const = 0;
virtual uint32_t KeycodeToKeysym(KeyCode keycode,
uint32_t modifiers) const = 0;
private:
friend class Connection;
virtual void UpdateMapping() = 0;
};
std::unique_ptr<KeyboardState> CreateKeyboardState(Connection* connection);
} // namespace x11
#endif // UI_GFX_X_KEYBOARD_STATE_H_
This diff is collapsed.
......@@ -25,12 +25,12 @@ std::unique_ptr<AtkKeyEventStruct> AtkKeyEventFromXEvent(
: ATK_KEY_EVENT_RELEASE;
auto state = static_cast<int>(xkey->state);
auto keycode = static_cast<int>(xkey->detail);
auto keycode = xkey->detail;
auto keysym = x11::Connection::Get()->KeycodeToKeysym(keycode, state);
atk_key_event->state = state;
atk_key_event->keyval = static_cast<uint32_t>(keysym);
atk_key_event->keycode = keycode;
atk_key_event->keyval = keysym;
atk_key_event->keycode = static_cast<uint8_t>(keycode);
atk_key_event->timestamp = static_cast<uint32_t>(xkey->time);
// This string property matches the one that was removed from GdkEventKey. In
......
......@@ -95,8 +95,7 @@ TEST(XEventTranslationTest, KeyEventXEventPropertiesSet) {
auto hw_keycode_it = properties->find(ui::kPropertyKeyboardHwKeyCode);
EXPECT_NE(hw_keycode_it, properties->end());
EXPECT_EQ(1u, hw_keycode_it->second.size());
EXPECT_EQ(static_cast<uint8_t>(
connection->KeysymToKeycode(static_cast<x11::KeySym>(XK_a))),
EXPECT_EQ(static_cast<uint8_t>(connection->KeysymToKeycode(XK_a)),
hw_keycode_it->second[0]);
auto kbd_group_it = properties->find(ui::kPropertyKeyboardGroup);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment