From 9c6fc7847158f76e38d8ea8c0a853d4104310c21 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 2 May 2020 18:28:43 +0200 Subject: [PATCH] Fix editor window handling in Reaper And other hosts that embed the parent window into another window. --- README.md | 13 +++--- src/wine-host/editor.cpp | 94 ++++++++++++++++++++++++++++++---------- src/wine-host/editor.h | 9 ++++ 3 files changed, 87 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index ec8dab95..1782d213 100644 --- a/README.md +++ b/README.md @@ -8,19 +8,21 @@ also staying easy to debug and maintain. ## TODOs -There are a few things that should be done before releasing this, including: +Everything is implemented and ready for release after a few documentation +updates: - Add missing details if any to the architecture section. - Rewrite parts of this readme. -- Briefly verify that this also works fine in Reaper and Ardour. +- Add a screenshot, because why not? ## Tested with Yabridge has been verified to work correctly with: - Bitwig Studio 3.1 and the beta releases of 3.2 -- Carla 2.1 +- Carla 2.1 (does not support opening multiple symlinked plugins) - Ardour 5.12 +- REAPER 6.09 (does not support symlinks) - Wine Staging 5.5 and 5.6 (the wine-staging-5.7-1 package currently in Arch and Manjaro's repositories is broken because of a regression in application startup behavior) @@ -103,8 +105,9 @@ because I haven't needed them myself. Let me know if any of these features are required for a certain plugin or plugin host: - Double precision audio (`processDoubleReplacing`). -- Vendor specific extensions (for instance, for - [Reaper](https://www.reaper.fm/sdk/vst/vst_ext.php)). +- Vendor specific extension (for instance, for + [REAPER](https://www.reaper.fm/sdk/vst/vst_ext.php), though most of these + extension functions will work out of the box without any modifications). ## Building diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index efc59153..7279e2f3 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -25,6 +25,18 @@ constexpr size_t idle_timer_id = 1337; */ constexpr uint16_t event_type_mask = ((1 << 7) - 1); +/** + * Find the topmost window (i.e. the window before the root window in the window + * tree) starting from a certain window. + * + * @param x11_connection The X11 connection to use. + * @param starting_at The window we want to know the topmost window of. + * + * @return Either `starting_at`, if its parent is already the root window, or + * another another window that has `starting_at` as its descendent. + */ +xcb_window_t find_topmost_window(xcb_connection_t& x11_connection, + xcb_window_t starting_at); /** * Compute the size a window would have to be to be allowed to fullscreened on * any of the connected screens. @@ -69,6 +81,7 @@ Editor::Editor(const std::string& window_class_name, DestroyWindow), parent_window(parent_window_handle), child_window(get_x11_handle(win32_handle.get())), + topmost_window(find_topmost_window(*x11_connection, parent_window)), // Needed to send update messages on a timer plugin(effect) { // The Win32 API will block the `DispatchMessage` call when opening e.g. a @@ -79,10 +92,13 @@ Editor::Editor(const std::string& window_class_name, // the plugin is not busy. SetTimer(win32_handle.get(), idle_timer_id, 100, nullptr); - // See the x11 events part of `editor::handle_events` - const uint32_t parent_event_mask = XCB_EVENT_MASK_STRUCTURE_NOTIFY; - xcb_change_window_attributes(x11_connection.get(), parent_window, - XCB_CW_EVENT_MASK, &parent_event_mask); + // We need to tell the Wine window it has been moved whenever the window + // it's been embedded in gets moved around. In most cases this is + // `parent_window`, but for instance REAPER reparents `parent_window` in + // another window so we'll have to find the correct window first. + const uint32_t topmost_event_mask = XCB_EVENT_MASK_STRUCTURE_NOTIFY; + xcb_change_window_attributes(x11_connection.get(), topmost_window, + XCB_CW_EVENT_MASK, &topmost_event_mask); xcb_flush(x11_connection.get()); // Embed the Win32 window into the window provided by the host. Instead of @@ -151,30 +167,40 @@ void Editor::handle_events() { while ((generic_event = xcb_poll_for_event(x11_connection.get())) != nullptr) { switch (generic_event->response_type & event_type_mask) { + // We're listening for ConfigureNotify events on the topmost window + // before the root window, i.e. the window that's actually going to + // get dragged around the by the user. In most cases this is the + // same as `parent_window`. case XCB_CONFIGURE_NOTIFY: { - xcb_configure_notify_event_t event = - *reinterpret_cast( - generic_event); - if (event.window != parent_window) { - break; - } - // We're purposely not using XEmbed. This has the consequence // that wine still thinks that any X and Y coordinates are // relative to the x11 window root instead of the parent window // provided by the DAW, causing all sorts of GUI interactions to // break. To alleviate this we'll just lie to Wine and tell it - // that it's located at the parent window's location. We'll only - // send the event instead of actually configuring the window. - // NOTE: We're not actually using `SetWindowPos()` to resize the - // window. The editor's client area will likely always be - // big enough Since we specified the Window to be - // 2560x1440 pixels large at the time of its creation. - // This works because the embedding hierarchy is DAW - // window -> Win32 window (created in this class) -> VST - // plugin window created by the plugin itself. This makes - // the drag-to-resize functionality many plugin editors - // have feel smooth and native. + // that it's located at the parent window's location on the root + // window. We also will keep the child window at its largest + // possible size to allow for smooth resizing. This works + // because the embedding hierarchy is DAW window -> Win32 window + // (created in this class) -> VST plugin window created by the + // plugin itself. In this case it doesn't matter that the Win32 + // window is larger than the part of the client area the plugin + // draws to since any excess will be clipped off by the parent + // window. + const auto query_cookie = + xcb_query_tree(x11_connection.get(), parent_window); + xcb_window_t root = xcb_query_tree_reply(x11_connection.get(), + query_cookie, nullptr) + ->root; + + // We can't directly use the `event.x` and `event.y` coordinates + // because the parent window may also be embedded inside another + // window. + const auto translate_cookie = xcb_translate_coordinates( + x11_connection.get(), parent_window, root, 0, 0); + const xcb_translate_coordinates_reply_t* + translated_coordiantes = xcb_translate_coordinates_reply( + x11_connection.get(), translate_cookie, nullptr); + xcb_configure_notify_event_t translated_event{}; translated_event.response_type = XCB_CONFIGURE_NOTIFY; translated_event.event = child_window; @@ -186,8 +212,8 @@ void Editor::handle_events() { // (such as those by Valhalla DSP) would break. translated_event.width = client_area.width; translated_event.height = client_area.height; - translated_event.x = event.x; - translated_event.y = event.y; + translated_event.x = translated_coordiantes->dst_x; + translated_event.y = translated_coordiantes->dst_y; xcb_send_event(x11_connection.get(), false, child_window, XCB_EVENT_MASK_STRUCTURE_NOTIFY | @@ -241,6 +267,26 @@ LRESULT CALLBACK window_proc(HWND handle, return DefWindowProc(handle, message, wParam, lParam); } +xcb_window_t find_topmost_window(xcb_connection_t& x11_connection, + xcb_window_t starting_at) { + xcb_window_t current_window = starting_at; + + xcb_query_tree_cookie_t query_cookie = + xcb_query_tree(&x11_connection, starting_at); + xcb_query_tree_reply_t* query_reply = + xcb_query_tree_reply(&x11_connection, query_cookie, nullptr); + xcb_window_t root = query_reply->root; + while (query_reply->parent != root) { + current_window = query_reply->parent; + + query_cookie = xcb_query_tree(&x11_connection, current_window); + query_reply = + xcb_query_tree_reply(&x11_connection, query_cookie, nullptr); + } + + return current_window; +} + Size get_maximum_screen_dimensions(xcb_connection_t& x11_connection) { xcb_screen_iterator_t iter = xcb_setup_roots_iterator(xcb_get_setup(&x11_connection)); diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 51b64c6d..1d127665 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -147,6 +147,15 @@ class Editor { * The X11 window handle of the window belonging to `win32_handle`. */ const xcb_window_t child_window; + /** + * The X11 window that's at the top of the window tree starting from + * `parent_window`, i.e. a direct child of the root window. In most cases + * this is going to be the same as `parent_window`, but some DAWs (such as + * REAPER) embed `parent_window` into another window. We have to listen for + * configuration changes on this topmost window to know when the window is + * being dragged around. + */ + const xcb_window_t topmost_window; /** *Needed to handle idle updates through a timer