Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: forwarding certain key events of 'wine_window' to 'host_window' #263

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

WIP: forwarding certain key events of 'wine_window' to 'host_window' #263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions src/wine-host/editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ constexpr uint32_t wrapper_event_mask = XCB_EVENT_MASK_STRUCTURE_NOTIFY |
XCB_EVENT_MASK_KEY_PRESS |
XCB_EVENT_MASK_KEY_RELEASE;

/**
* The X11 event mask for the wine window. We'll use this for detecting specific
* key events that ought to be forwarded to host window (i.e. a REAPER thing).
*/
constexpr uint32_t wine_event_mask = XCB_EVENT_MASK_KEY_PRESS |
XCB_EVENT_MASK_KEY_RELEASE;

/**
* The name of the X11 property on the root window used to denote the active
* window in EWMH compliant window managers.
Expand Down Expand Up @@ -116,6 +123,31 @@ constexpr uint32_t xembed_focus_first = 1;
*/
static const HCURSOR arrow_cursor = LoadCursor(nullptr, IDC_ARROW);

/**
* List of keycodes that shall be forwarded to host (REAPER) when
* 'forward_key_events_to_host_' is true.
*
* NOTE: List needs to be in sorted order to work with binary search.
* 'spacebar' and 'function keys' were choosen, because those are what
* is typical for REAPER to receive even when editor (wine) window has
* focus (i.e. native behavior on all OS).
*/
const std::vector<int> keycodes_to_forward {
Copy link
Owner

Choose a reason for hiding this comment

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

Part of the reason why I didn't really want to look into myself (and I'd much rather have REAPER implement similar behavior as Bitwig) is that doing something like this is quite brittle, and doing it the right way is a bit involved. These key codes are specific to QWERTY keyboard layouts (and potentially other system settings). The proper way to do this is to map a list of KEYSYMs to KEYCODEs first. See the keyboard section of the X11 spec here: https://www.x.org/releases/X11R7.7/doc/xproto/x11protocol.html#Keyboards

(this should also be an std::unordered_set for constant time lookups, but I can fix those things up later)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Part of the reason why I didn't really want to look into myself (and I'd much rather have REAPER implement similar behavior as Bitwig) is that doing something like this is quite brittle, and doing it the right way is a bit involved. These key codes are specific to QWERTY keyboard layouts (and potentially other system settings). The proper way to do this is to map a list of KEYSYMs to KEYCODEs first. See the keyboard section of the X11 spec here: https://www.x.org/releases/X11R7.7/doc/xproto/x11protocol.html#Keyboards

I've just read through that section you linked (couple times actually). I guess I can see for why this would be the "proper way" of making this robust for any KEYSYM.

Maybe I just wrongly assumed that at least keys like spacebar> and function keys would be quite universally always the same KEYCODE on X11. At least that's the impression I got after finding things like this one:
https://gist.github.com/rickyzhang82/8581a762c9f9fc6ddb8390872552c250

I did find this library mentioned during my research: https://xkbcommon.org/
Using the KEYCODE directly just seemed the only feasible thing for me at the time without adding another dependency like this one. But I might have been mistaken of what can be done "natively" with xcb.

(this should also be an std::unordered_set for constant time lookups, but I can fix those things up later)

Just had to read up on it. Yeah, I guess that would be the correct choice for a lookup scenario in general. I didn't even think of this, because I figured for such a small list that won't grow any further (assuming this keeps being just about spacebar and function keys), any kind of performance gains from a more sophisticated data structure than a simple array seemed negligible. Or if it's about avoiding any kind of issues due to "variable execution time" down the line, well, I'd assume even a simple for-loop for such a small list would be hard to notice.
But of course, I didn't make the measurement. Really just tried to learn something from this.

65, // spacebar
67, // F1
68, // F2
69, // F3
70, // F4
71, // F5
72, // F6
73, // F7
74, // F8
75, // F9
76, // F10
95, // F11
96 // F12
};

/**
* Find the the ancestors for the given window. This returns a list of window
* IDs that starts with `starting_at`, and then iteratively contains the parent
Expand Down Expand Up @@ -393,6 +425,8 @@ Editor::Editor(MainContext& main_context,
XCB_CW_EVENT_MASK, &parent_event_mask);
xcb_change_window_attributes(x11_connection_.get(), wrapper_window_.window_,
XCB_CW_EVENT_MASK, &wrapper_event_mask);
xcb_change_window_attributes(x11_connection_.get(), wine_window_,
XCB_CW_EVENT_MASK, &wine_event_mask);
xcb_flush(x11_connection_.get());

// First reparent our dumb wrapper window to the host's window, and then
Expand Down Expand Up @@ -643,6 +677,16 @@ void Editor::handle_x11_events() noexcept {
set_input_focus(true);
}
}

const auto mask = get_active_modifiers().value_or(0);
if (mask == XCB_MOD_MASK_SHIFT) {
forward_key_events_to_host_ = false;
} else if (mask == 0) {
// Only reset on actual mouse hover (not LMB click,
// which is value 256 and itself triggers 'LeaveNotify'
// and 'EnterNotify' in close sucession)
forward_key_events_to_host_ = true;
}
} break;
// When the user moves their mouse away from the Wine window
// _while the window provided by the host it is contained in is
Expand Down Expand Up @@ -719,6 +763,14 @@ void Editor::handle_x11_events() noexcept {
!is_cursor_in_wine_window(windows_pointer_pos)) {
set_input_focus(false);
}

const auto mask = get_active_modifiers().value_or(0);
if (mask == 0) {
// Only reset on actual mouse hover (not LMB click,
// which is value 256 and itself triggers 'LeaveNotify'
// and 'EnterNotify' in close sucession)
forward_key_events_to_host_ = true;
}
} break;
// We need to forward synthetic keyboard events sent by the host
// from the wrapper window to the Wine window
Expand Down Expand Up @@ -763,6 +815,30 @@ void Editor::handle_x11_events() noexcept {
reinterpret_cast<const char*>(event));
xcb_flush(x11_connection_.get());
}

if ((forward_key_events_to_host_ == true) &&
(is_wine_window_active()) &&
(event->event == wine_window_)) {
// Only forward desired keycodes. Skip all others.
if (std::binary_search(keycodes_to_forward.begin(),
keycodes_to_forward.end(), event->detail)) {
// NOTE: Deliberately setting
// XCB_EVENT_MASK_NO_EVENT (opposed to
// something like XCB_EVENT_MASK_KEY_PRESS)
// makes REAPER receive the event.
// Why it behaves like that is not fully clear
// at this point in time.
const uint32_t event_mask = XCB_EVENT_MASK_NO_EVENT;
event->event = host_window_;
xcb_send_event(
x11_connection_.get(),
false, host_window_, event_mask,
reinterpret_cast<const char*>(event));
xcb_flush(x11_connection_.get());
} else {
//printf("Skip forward keycode: %d\n", event->detail);
}
}
} break;
default: {
logger_.log_editor_trace([&]() {
Expand Down
10 changes: 10 additions & 0 deletions src/wine-host/editor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,4 +486,14 @@ class Editor {
* The atom corresponding to `_XEMBED`.
*/
xcb_atom_t xcb_xembed_message_;

/**
* Used to indicate whether key events detected on wine window should
* additionally be forwarded to host window. Toggled via a 'shift + hover
* + XCB_ENTER_NOTIFY' mechanism.
*
* NOTE: This is more of a "top-level switch" and only really used for
* REAPER. Furthermore, only a subset of key events is forwarded.
*/
bool forward_key_events_to_host_ = true;
};