diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 19b27632..46583887 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -3,38 +3,11 @@ // The Win32 API requires you to hardcode identifiers for tiemrs constexpr size_t idle_timer_id = 1337; -constexpr char xembed_proeprty[] = "_XEMBED"; -constexpr char xembed_info_proeprty[] = "_XEMBED_INFO"; - -// Constants from the XEmbed spec -constexpr uint32_t xembed_protocol_version = 0; - -constexpr uint32_t xembed_embedded_notify_msg = 0; -constexpr uint32_t xembed_window_activate_msg = 1; -constexpr uint32_t xembed_focus_in_msg = 4; - -constexpr uint32_t xembed_focus_first = 1; - ATOM register_window_class(std::string window_class_name); Editor::Editor(std::string window_class_name) : window_class(register_window_class(window_class_name)), - x11_connection(xcb_connect(nullptr, nullptr), &xcb_disconnect) { - // We need a bunch of property atoms for the XEmbed protocol - xcb_generic_error_t* err; - - const auto xembed_cookie = xcb_intern_atom( - x11_connection.get(), 0, strlen(xembed_proeprty), xembed_proeprty); - const auto xembed_info_cookie = - xcb_intern_atom(x11_connection.get(), 0, strlen(xembed_info_proeprty), - xembed_info_proeprty); - - xcb_xembed = - xcb_intern_atom_reply(x11_connection.get(), xembed_cookie, &err)->atom; - xcb_xembed_info = - xcb_intern_atom_reply(x11_connection.get(), xembed_info_cookie, &err) - ->atom; -} + x11_connection(xcb_connect(nullptr, nullptr), &xcb_disconnect) {} HWND Editor::open(AEffect* effect, xcb_window_t parent_window_handle) { // Create a window without any decoratiosn for easy embedding. The @@ -46,7 +19,7 @@ HWND Editor::open(AEffect* effect, xcb_window_t parent_window_handle) { CreateWindowEx(WS_EX_TOOLWINDOW | WS_EX_ACCEPTFILES, reinterpret_cast(window_class), "yabridge plugin", WS_POPUP, CW_USEDEFAULT, - CW_USEDEFAULT, 2048, 2048, nullptr, nullptr, + CW_USEDEFAULT, 2560, 1440, nullptr, nullptr, GetModuleHandle(nullptr), this), &DestroyWindow); @@ -62,11 +35,23 @@ HWND Editor::open(AEffect* effect, xcb_window_t parent_window_handle) { // the plugin is not busy. SetTimer(win32_handle->get(), idle_timer_id, 100, nullptr); + // Embed the Win32 window into the window provided by the host. Instead of + // using the XEmbed protocol, we'll register a few events and manage the + // child window ourselves. This is a hack to work around the issue's + // described in `Editor`'s docstring'. + const size_t child_window = get_x11_handle().value(); + xcb_reparent_window(x11_connection.get(), child_window, parent_window, 0, + 0); + xcb_map_window(x11_connection.get(), child_window); + xcb_flush(x11_connection.get()); + const uint32_t event_mask = XCB_EVENT_MASK_VISIBILITY_CHANGE; xcb_change_window_attributes(x11_connection.get(), parent_window, XCB_CW_EVENT_MASK, &event_mask); xcb_flush(x11_connection.get()); + ShowWindow(win32_handle->get(), SW_SHOWNORMAL); + return win32_handle->get(); } @@ -78,38 +63,6 @@ void Editor::close() { // everything for us? } -bool Editor::xembed() { - if (!win32_handle.has_value()) { - return false; - } - - // This follows the embedding procedure specified in the XEmbed sped: - // https://specifications.freedesktop.org/xembed-spec/xembed-spec-latest.html - // under 'Embedding life cycle - // Sadly there's doesn't seem to be any implementation of this available as - // a library - const size_t child_window = get_x11_handle().value(); - - xcb_reparent_window(x11_connection.get(), child_window, parent_window, 0, - 0); - - // Tell the window from Wine it's embedded into the window provided by the - // host - send_xembed_event(child_window, xembed_embedded_notify_msg, 0, - parent_window, xembed_protocol_version); - - send_xembed_event(child_window, xembed_focus_in_msg, xembed_focus_first, 0, - 0); - send_xembed_event(child_window, xembed_window_activate_msg, 0, 0, 0); - - xcb_map_window(x11_connection.get(), child_window); - xcb_flush(x11_connection.get()); - - ShowWindow(win32_handle->get(), SW_SHOWNORMAL); - - return true; -} - void Editor::handle_events() { // Process any remaining events, otherwise we won't be able to interact with // the window @@ -137,32 +90,13 @@ void Editor::handle_events() { } // Handle X11 events - // TODO: We don't listen for XEmbed property changes for mapping and - // unmapping the window as described in the spec. Is this - // something we should do? - // TODO: Also, increasing window size does not work reliably, any way we - // could make this work better? xcb_generic_event_t* event; while ((event = xcb_poll_for_event(x11_connection.get())) != nullptr) { // The most significant bit in an event's response type is used to // indicate whether the event source switch (event->response_type & ((1 << 7) - 1)) { case XCB_VISIBILITY_NOTIFY: { - xcb_visibility_notify_event_t visibility_event = - *reinterpret_cast( - event); - - // Wine's XEmbed implementation is a bit iffy when it comes - // to window size changes, including editor windows being - // increased in size by the DAW when first opening the - // editor. Restarting the XEmbed protocol on visibility and - // size changes is an easy workaround, but it's not 100% - // reliable. - if (visibility_event.window == parent_window && - visibility_event.state != - XCB_VISIBILITY_FULLY_OBSCURED) { - xembed(); - } + // TODO: Handle configuration changes } break; } @@ -180,26 +114,6 @@ std::optional Editor::get_x11_handle() { GetProp(win32_handle.value().get(), "__wine_x11_whole_window")); } -void Editor::send_xembed_event(const xcb_window_t& window, - const uint32_t message, - const uint32_t detail, - const uint32_t data1, - const uint32_t data2) { - xcb_client_message_event_t event; - event.response_type = XCB_CLIENT_MESSAGE; - event.type = xcb_xembed; - event.window = window; - event.format = 32; - event.data.data32[0] = XCB_CURRENT_TIME; - event.data.data32[1] = message; - event.data.data32[2] = detail; - event.data.data32[3] = data1; - event.data.data32[4] = data2; - - xcb_send_event(x11_connection.get(), false, window, XCB_EVENT_MASK_NO_EVENT, - reinterpret_cast(&event)); -} - LRESULT CALLBACK window_proc(HWND handle, UINT message, WPARAM wParam, diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 9799ffe2..a0faf770 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -20,6 +20,18 @@ * A wrapper around the win32 windowing API to create and destroy editor * windows. A VST plugin can embed itself in that window, and we can then later * embed the window in a VST host provided X11 window. + * + * This was originally implemented using XEmbed. While that sounded like the + * right thing to do, there were a few minor issues with Wine's XEmbed + * implementation. The most important of which is that resizing GUIs sometimes + * works fine, but often fails to expand the embedded window's client area + * leaving part of the window inaccessible. There are also some a small number + * of plugins (such as Serum) that have rendering issues when using XEmbed but + * otherwise draw fine when running standalone or when just reparenting the + * window without using XEmbed. If anyone knows how to work around these two + * issues, please let me know and I'll switch to using XEmbed again. + * + * This workaround was inspired by LinVST. */ class Editor { public: @@ -30,8 +42,8 @@ class Editor { Editor(std::string window_class_name); /** - * Open a window and return a handle to the new Win32 window that can be - * used by the hosted VST plugin. + * Open a window, embed it into the DAW's parent window and return a handle + * to the new Win32 window that can be used by the hosted VST plugin. * * @param effect The plugin this window is being created for. Used to send * `effEditIdle` messages on a timer. @@ -48,14 +60,6 @@ class Editor { */ void handle_events(); - /** - * Embed the (open) window into the parent window. - * - * @return Whether the embedding was succesful. Will return false if the - * window is not open. - */ - bool xembed(); - // Needed to handle idle updates through a timer AEffect* plugin; @@ -71,18 +75,7 @@ class Editor { std::optional get_x11_handle(); /** - * Send an XEmbed message to a window. See the spec for more information. - * - * https://specifications.freedesktop.org/xembed-spec/xembed-spec-latest.html#lifecycle - */ - void send_xembed_event(const xcb_window_t& window, - const uint32_t message, - const uint32_t detail, - const uint32_t data1, - const uint32_t data2); - - /** - * The Win32 window class registered for the window window. + * The Win32 window class registered for the windows window. */ ATOM window_class; @@ -95,6 +88,4 @@ class Editor { win32_handle; std::unique_ptr x11_connection; - xcb_atom_t xcb_xembed; - xcb_atom_t xcb_xembed_info; }; diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp index 61cfbe3f..518e4722 100644 --- a/src/wine-host/plugin-bridge.cpp +++ b/src/wine-host/plugin-bridge.cpp @@ -241,8 +241,8 @@ intptr_t PluginBridge::dispatch_wrapper(AEffect* plugin, const auto x11_handle = reinterpret_cast(data); const auto win32_handle = editor.open(plugin, x11_handle); - // The actual XEmbed handling is done after the host's window has - // been set to the correct size, in `Editor::handle_events()` + // The created Win32 window has already been reparented to the host + // provided window return plugin->dispatcher(plugin, opcode, index, value, win32_handle, option); } break;