From 03de09d77f34a3ba3a89f0dbe5f8dbf349bc4c77 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 14 Apr 2020 16:51:24 +0200 Subject: [PATCH] Clean up the editor implementation --- README.md | 2 + src/wine-host/editor.cpp | 219 ++++++++++++++------------------ src/wine-host/editor.h | 61 ++++----- src/wine-host/plugin-bridge.cpp | 37 ++---- src/wine-host/plugin-bridge.h | 7 +- 5 files changed, 144 insertions(+), 182 deletions(-) diff --git a/README.md b/README.md index 1a75af73..4dec187f 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,8 @@ Yet Another way to use Windows VST2 plugins in Linux VST hosts. There are a few things that should be done before releasing this, including: - Fix implementation bugs: + - Fix hiding and showing of editor windows, closing a window with alt+f4 + freezes the editor, - Polish GUIs even further. There are some todos left in `src/wine-host/editor.{h,cpp}`. - There are likely some minor issues left. diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 11158c59..dc5ce5eb 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -30,59 +30,42 @@ xcb_window_t get_x11_handle(HWND win32_handle); 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) {} - -HWND Editor::open(AEffect* effect) { - // Create a window without any decoratiosn for easy embedding. The - // combination of `WS_EX_TOOLWINDOW` and `WS_POPUP` causes the window to be - // drawn without any decorations (making resizes behave as you'd expect) and - // also causes mouse coordinates to be relative to the window itself. - win32_handle = - std::unique_ptr, decltype(&DestroyWindow)>( - CreateWindowEx(WS_EX_TOOLWINDOW | WS_EX_ACCEPTFILES, - reinterpret_cast(window_class), - "yabridge plugin", WS_POPUP, CW_USEDEFAULT, - CW_USEDEFAULT, client_area_width, client_area_height, - nullptr, nullptr, GetModuleHandle(nullptr), this), - &DestroyWindow); - - // Needed to send update messages on a timer - plugin = effect; - +Editor::Editor(const std::string& window_class_name, + AEffect* effect, + const size_t parent_window_handle) + : // Needed to send update messages on a timer + plugin(effect), + window_class(register_window_class(window_class_name)), + // Create a window without any decoratiosn for easy embedding. The + // combination of `WS_EX_TOOLWINDOW` and `WS_POPUP` causes the window to + // be drawn without any decorations (making resizes behave as you'd + // expect) and also causes mouse coordinates to be relative to the window + // itself. + win32_handle(CreateWindowEx(WS_EX_TOOLWINDOW | WS_EX_ACCEPTFILES, + reinterpret_cast(window_class), + "yabridge plugin", + WS_POPUP, + CW_USEDEFAULT, + CW_USEDEFAULT, + client_area_width, + client_area_height, + nullptr, + nullptr, + GetModuleHandle(nullptr), + this), + &DestroyWindow), + parent_window(parent_window_handle), + child_window(get_x11_handle(win32_handle.get())), + x11_connection(xcb_connect(nullptr, nullptr), &xcb_disconnect) { // The Win32 API will block the `DispatchMessage` call when opening e.g. a // dropdown, but it will still allow timers to be run so the GUI can still // update in the background. Because of this we send `effEditIdle` to the // plugin on a timer. The refresh rate is purposely fairly low since we // we'll also trigger this manually in `Editor::handle_events()` whenever // the plugin is not busy. - SetTimer(win32_handle->get(), idle_timer_id, 100, nullptr); + SetTimer(win32_handle.get(), idle_timer_id, 100, nullptr); - return win32_handle->get(); -} - -void Editor::close() { - // RAII will destroy the window and tiemrs for us - win32_handle = std::nullopt; - - // TODO: We might want to manually unmap the X11 window instead of having - // the host do it. Right now the window editor window stays open for a - // second when removing a plugin. -} - -bool Editor::embed_into(const size_t parent_window_handle) { - if (!win32_handle.has_value()) { - return false; - } - - child_window = get_x11_handle(win32_handle->get()); - parent_window = parent_window_handle; - - // See the X11 events part of `Editor::handle_events`. - // const uint32_t child_event_mask = XCB_EVENT_MASK_STRUCTURE_NOTIFY; - // xcb_change_window_attributes(x11_connection.get(), child_window, - // XCB_CW_EVENT_MASK, &child_event_mask); + // 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); @@ -97,91 +80,85 @@ bool Editor::embed_into(const size_t parent_window_handle) { xcb_map_window(x11_connection.get(), child_window); xcb_flush(x11_connection.get()); - ShowWindow(win32_handle->get(), SW_SHOWNORMAL); - - return true; + ShowWindow(win32_handle.get(), SW_SHOWNORMAL); } void Editor::handle_events() { // Process any remaining events, otherwise we won't be able to interact with // the window - if (win32_handle.has_value()) { - bool gui_was_updated = false; - MSG msg; + bool gui_was_updated = false; + MSG msg; - // The second argument has to be null since we not only want to handle - // events for this window but also for all child windows (i.e. - // dropdowns). I spent way longer debugging this than I want to admit. - while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) { - TranslateMessage(&msg); - DispatchMessage(&msg); + // The second argument has to be null since we not only want to handle + // events for this window but also for all child windows (i.e. dropdowns). I + // spent way longer debugging this than I want to admit. + while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessage(&msg); - if (msg.message == WM_TIMER && msg.wParam == idle_timer_id) { - gui_was_updated = true; - } + if (msg.message == WM_TIMER && msg.wParam == idle_timer_id) { + gui_was_updated = true; } + } - // Make sure that the GUI always gets updated at least once for every - // `effEditIdle` call the host has sent to improve responsiveness when - // the GUI isn't being blocked. - if (!gui_was_updated) { - SendMessage(win32_handle->get(), WM_TIMER, idle_timer_id, 0); - } - - // Handle X11 events - // TODO: Check if we should forward other events mostly to prevent - // unnecessary GUI processing in the background. Since - // `effEditIdle` should only be called when the plugin's editor is - // open this should not cause any different in CPU though. - // TODO: Check whether drag and drop works out of the box - xcb_generic_event_t* generic_event; - while ((generic_event = xcb_poll_for_event(x11_connection.get())) != - nullptr) { - switch (generic_event->response_type & event_type_mask) { - 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. - xcb_configure_notify_event_t translated_event{}; - translated_event.response_type = XCB_CONFIGURE_NOTIFY; - translated_event.event = child_window; - translated_event.window = child_window; - translated_event.width = client_area_width; - translated_event.height = client_area_height; - translated_event.x = event.x; - translated_event.y = event.y; - - xcb_send_event(x11_connection.get(), false, child_window, - XCB_EVENT_MASK_STRUCTURE_NOTIFY | - XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY, - reinterpret_cast(&translated_event)); - xcb_flush(x11_connection.get()); - } break; - } - free(generic_event); + // Make sure that the GUI always gets updated at least once for every + // `effEditIdle` call the host has sent to improve responsiveness when the + // GUI isn't being blocked. + if (!gui_was_updated) { + SendMessage(win32_handle.get(), WM_TIMER, idle_timer_id, 0); + } + + // Handle X11 events + // TODO: Check if we should forward other events mostly to prevent + // unnecessary GUI processing in the background. Since `effEditIdle` + // should only be called when the plugin's editor is open this should + // not cause any different in CPU though. + // TODO: Check whether drag and drop works out of the box + xcb_generic_event_t* generic_event; + while ((generic_event = xcb_poll_for_event(x11_connection.get())) != + nullptr) { + switch (generic_event->response_type & event_type_mask) { + 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. + xcb_configure_notify_event_t translated_event{}; + translated_event.response_type = XCB_CONFIGURE_NOTIFY; + translated_event.event = child_window; + translated_event.window = child_window; + translated_event.width = client_area_width; + translated_event.height = client_area_height; + translated_event.x = event.x; + translated_event.y = event.y; + + xcb_send_event(x11_connection.get(), false, child_window, + XCB_EVENT_MASK_STRUCTURE_NOTIFY | + XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY, + reinterpret_cast(&translated_event)); + xcb_flush(x11_connection.get()); + } break; } + free(generic_event); } } diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 14651cae..44818365 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -18,11 +18,11 @@ /** * 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. + * windows. We can embed this window into the window provided by the host, and a + * VST plugin can then later embed itself in the window create here. * - * This was originally implemented using XEmbed. While that sounded like the - * right thing to do, there were a few small issues with Wine's XEmbed + * This was originally implemented using XEmbed. Even though that sounded like + * the right thing to do, there were a few small 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 @@ -32,41 +32,25 @@ * issues, please let me know and I'll switch to using XEmbed again. * * This workaround was inspired by LinVST. - * - * TODO: Refactor this so we don't need to stage initialization and just add an - * `std::optional` to `PluginBridge` instead */ class Editor { public: /** - * @param window_class_name The name for the window class for editor - * windows. - */ - Editor(std::string window_class_name); - - /** - * Open a window, embed it into the DAW's parent window and return a handle + * Open a window, embed it into the DAW's parent window and create a handle * to the new Win32 window that can be used by the hosted VST plugin. * + * @param window_class_name The name for the window class for editor + * windows. * @param effect The plugin this window is being created for. Used to send * `effEditIdle` messages on a timer. - * - * @return The Win32 window handle of the newly created window. - */ - HWND open(AEffect* effect); - - void close(); - - /** - * Embed the (open) window into a parent window. - * * @param parent_window_handle The X11 window handle passed by the VST host * for the editor to embed itself into. * - * @return Whether the embedding was succesful. Will return false if the - * window is not open. + * @see win32_handle */ - bool embed_into(const size_t parent_window_handle); + Editor(const std::string& window_class_name, + AEffect* effect, + const size_t parent_window_handle); /** * Pump messages from the editor GUI's event loop until all events are @@ -74,10 +58,23 @@ class Editor { * of Win32 limitations. I guess that's what `effEditIdle` is for. */ void handle_events(); - // Needed to handle idle updates through a timer AEffect* plugin; + private: + /** + * The Win32 window class registered for the windows window. + */ + ATOM window_class; + + public: + /** + * The handle for the window created through Wine that the plugin uses to + * embed itself in. + */ + const std::unique_ptr, decltype(&DestroyWindow)> + win32_handle; + private: /** * The window handle of the editor window created by the DAW. @@ -88,18 +85,10 @@ class Editor { */ xcb_window_t child_window; - /** - * The Win32 window class registered for the windows window. - */ - ATOM window_class; - /** * A pointer to the currently active window. Will be a null pointer if no * window is active. */ - std::optional< - std::unique_ptr, decltype(&DestroyWindow)>> - win32_handle; std::unique_ptr x11_connection; }; diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp index 0854dc93..0c2a300d 100644 --- a/src/wine-host/plugin-bridge.cpp +++ b/src/wine-host/plugin-bridge.cpp @@ -66,8 +66,7 @@ PluginBridge::PluginBridge(std::string plugin_dll_path, vst_host_callback(io_context), host_vst_parameters(io_context), host_vst_process_replacing(io_context), - vst_host_aeffect(io_context), - editor("yabridge plugin") { + vst_host_aeffect(io_context) { // Got to love these C APIs if (plugin_handle == nullptr) { throw std::runtime_error("Could not load a shared library at '" + @@ -225,39 +224,29 @@ intptr_t PluginBridge::dispatch_wrapper(AEffect* plugin, // To allow the GUI to update even when this thread gets blocked // (e.g. when a dropdown is open), the actual `effEditIdle` event // gets sent to the plugin on a timer. - editor.handle_events(); + editor->handle_events(); return 1; break; - case effClose: { - // Closing the editor will also shut down the thread that's - // currently handling events - editor.close(); - - return plugin->dispatcher(plugin, opcode, index, value, data, - option); - } break; case effEditOpen: { - const auto win32_handle = editor.open(plugin); - - const auto return_value = plugin->dispatcher( - plugin, opcode, index, value, win32_handle, option); - if (return_value == 0) { - return 0; - } - - // If opening the editor was succesful, reparent it to the window - // provided by the DAW + // Create a Win32 window through Wine, embed it into the window + // provided by the host, and let the plugin embed itself into the + // Wine window const auto x11_handle = reinterpret_cast(data); - editor.embed_into(x11_handle); + editor.emplace("yabridge plugin", plugin, x11_handle); - return return_value; + return plugin->dispatcher(plugin, opcode, index, value, + editor->win32_handle.get(), option); } break; case effEditClose: { const intptr_t return_value = plugin->dispatcher(plugin, opcode, index, value, data, option); - editor.close(); + // Cleanup is handled through RAII + // TODO: We might want to manually unmap the X11 window instead of + // having the host do it. Right now the window editor window + // stays open for a second when removing a plugin. + editor = std::nullopt; return return_value; } break; diff --git a/src/wine-host/plugin-bridge.h b/src/wine-host/plugin-bridge.h index 61c40af7..45355f1e 100644 --- a/src/wine-host/plugin-bridge.h +++ b/src/wine-host/plugin-bridge.h @@ -156,5 +156,10 @@ class PluginBridge { */ std::vector process_buffer; - Editor editor; + /** + * The plugin editor window. Allows embedding the plugin's editor into a + * Wine window, and embedding that Wine window into a window provided by the + * host. Should be empty when the editor is not open. + */ + std::optional editor; };