From cc9226a3fca5e8cb9c9a7fa8e8f438615dbfaa54 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 16 Aug 2021 21:10:16 +0200 Subject: [PATCH] Remove editor_double_embed It's no longer needed after the `fix_local_coordinates()` change from a fa12c64866a8b79e862bc5db4c4b092a4b762689. --- CHANGELOG.md | 10 +++++++++- README.md | 24 +++++++++++------------- src/common/configuration.cpp | 6 ------ src/common/configuration.h | 15 --------------- src/plugin/bridges/common.h | 3 --- src/wine-host/editor.cpp | 29 ++--------------------------- src/wine-host/editor.h | 13 +------------ 7 files changed, 23 insertions(+), 77 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06ad4b1b..99f7bc45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,13 +8,21 @@ Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Removed + +- The `editor_double_embed` option added in yabridge 1.4.0 has been removed. + This option was added to work around _PSPaudioware E27_ which used its parent + window's position as an offset for drawing its GUI, and without this option + that GUI would be misaligned. The below change to yabridge's embedding method + supersedes this option, as it also fixes a similar issue for another plugin. + ### Changed - Yabridge's Wine window embedding now takes more measures to make sure that the plugin draws itself properly in the top left corner of the window. This is needed for some buggy plugins that draw window based on absolute screen coordinates, instead of their positioning within the parent window, like the - _Soundtoys_ plugins. + _Soundtoys_ plugins and older _PSPaudioware_ plugins. ### Fixed diff --git a/README.md b/README.md index 5868d801..f356996a 100644 --- a/README.md +++ b/README.md @@ -336,16 +336,16 @@ plugin._ ### Compatibility options -| Option | Values | Description | -| --------------------- | ----------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `disable_pipes` | `{true,false,}` | When this option is enabled, yabridge will redirect the Wine plugin host's output streams to a file without any further processing. See the [known issues](#runtime-dependencies-and-known-issues) section for a list of plugins where this may be useful. This can be set to a boolean, in which case the output will be written to `$XDG_RUNTIME_DIR/yabridge-plugin-output.log`, or to an absolute path (with no expansion for tildes or environment variables). Defaults to `false`. | -| `editor_double_embed` | `{true,false}` | Compatibility option for plugins that rely on the absolute screen coordinates of the window they're embedded in. Since the Wine window gets embedded inside of a window provided by your DAW, these coordinates won't match up and the plugin would end up drawing in the wrong location without this option. Currently the only known plugins that require this option are _PSPaudioware_ plugins with expandable GUIs, such as E27. Defaults to `false`. | -| `editor_force_dnd` | `{true,false}` | This option forcefully enables drag-and-drop support in _REAPER_. Because REAPER's FX window supports drag-and-drop itself, dragging a file onto a plugin editor will cause the drop to be intercepted by the FX window. This makes it impossible to drag files onto plugins in REAPER under normal circumstances. Setting this option to `true` will strip drag-and-drop support from the FX window, thus allowing files to be dragged onto the plugin again. Defaults to `false`. | -| `editor_xembed` | `{true,false}` | Use Wine's XEmbed implementation instead of yabridge's normal window embedding method. Some plugins will have redrawing issues when using XEmbed and editor resizing won't always work properly with it, but it could be useful in certain setups. You may need to use [this Wine patch](https://github.com/psycha0s/airwave/blob/master/fix-xembed-wine-windows.patch) if you're getting blank editor windows. Defaults to `false`. | -| `frame_rate` | `` | The rate at which Win32 events are being handled and usually also the refresh rate of a plugin's editor GUI. When using plugin groups all plugins share the same event handling loop, so in those the last loaded plugin will set the refresh rate. Defaults to `60`. | -| `hide_daw` | `{true,false}` | Don't report the name of the actual DAW to the plugin. See the [known issues](#runtime-dependencies-and-known-issues) section for a list of situations where this may be useful. This affects both VST2 and VST3 plugins. Defaults to `false`. | -| `vst3_no_scaling` | `{true,false}` | Disable HiDPI scaling for VST3 plugins. Wine currently does not have proper fractional HiDPI support, so you might have to enable this option if you're using a HiDPI display. In most cases setting the font DPI in `winecfg`'s graphics tab to 192 will cause plugins to scale correctly at 200% size. Defaults to `false`. | -| `vst3_prefer_32bit` | `{true,false}` | Use the 32-bit version of a VST3 plugin instead the 64-bit version if both are installed and they're in the same VST3 bundle inside of `~/.vst3/yabridge`. You likely won't need this. | +| Option | Values | Description | +| --------------------- | ----------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `disable_pipes` | `{true,false,}` | When this option is enabled, yabridge will redirect the Wine plugin host's output streams to a file without any further processing. See the [known issues](#runtime-dependencies-and-known-issues) section for a list of plugins where this may be useful. This can be set to a boolean, in which case the output will be written to `$XDG_RUNTIME_DIR/yabridge-plugin-output.log`, or to an absolute path (with no expansion for tildes or environment variables). Defaults to `false`. | +| `editor_double_embed` | `{true,false}` | Compatibility option for plugins that rely on the absolute screen coordinates of the window they're embedded in. Since the Wine window gets embedded inside of a window provided by your DAW, these coordinates won't match up and the plugin would end up drawing in the wrong location without this option. Currently the only known plugins that require this option are _PSPaudioware_ plugins with expandable GUIs, such as E27. Defaults to `false`. _This option has been remvoed in the master branch version of yabridge._ | +| `editor_force_dnd` | `{true,false}` | This option forcefully enables drag-and-drop support in _REAPER_. Because REAPER's FX window supports drag-and-drop itself, dragging a file onto a plugin editor will cause the drop to be intercepted by the FX window. This makes it impossible to drag files onto plugins in REAPER under normal circumstances. Setting this option to `true` will strip drag-and-drop support from the FX window, thus allowing files to be dragged onto the plugin again. Defaults to `false`. | +| `editor_xembed` | `{true,false}` | Use Wine's XEmbed implementation instead of yabridge's normal window embedding method. Some plugins will have redrawing issues when using XEmbed and editor resizing won't always work properly with it, but it could be useful in certain setups. You may need to use [this Wine patch](https://github.com/psycha0s/airwave/blob/master/fix-xembed-wine-windows.patch) if you're getting blank editor windows. Defaults to `false`. | +| `frame_rate` | `` | The rate at which Win32 events are being handled and usually also the refresh rate of a plugin's editor GUI. When using plugin groups all plugins share the same event handling loop, so in those the last loaded plugin will set the refresh rate. Defaults to `60`. | +| `hide_daw` | `{true,false}` | Don't report the name of the actual DAW to the plugin. See the [known issues](#runtime-dependencies-and-known-issues) section for a list of situations where this may be useful. This affects both VST2 and VST3 plugins. Defaults to `false`. | +| `vst3_no_scaling` | `{true,false}` | Disable HiDPI scaling for VST3 plugins. Wine currently does not have proper fractional HiDPI support, so you might have to enable this option if you're using a HiDPI display. In most cases setting the font DPI in `winecfg`'s graphics tab to 192 will cause plugins to scale correctly at 200% size. Defaults to `false`. | +| `vst3_prefer_32bit` | `{true,false}` | Use the 32-bit version of a VST3 plugin instead the 64-bit version if both are installed and they're in the same VST3 bundle inside of `~/.vst3/yabridge`. You likely won't need this. | These options are workarounds for issues mentioned in the [known issues](#runtime-dependencies-and-known-issues) section. Depending on the hosts @@ -370,9 +370,6 @@ group = "melda" ["ToneBoosters"] group = "toneboosters" -["PSPaudioware"] -editor_double_embed = true - ["Analog Lab 3.so"] editor_xembed = true @@ -512,6 +509,7 @@ include: - **PSPaudioware** plugins with expandable GUIs, such as E27, may have their GUI appear in the wrong location after the GUI has been expanded. You can enable an alternative [editor hosting mode](#compatibility-options) to fix this. + _This is no longer needed in the master branch version of yabridge._ - When using recent _Applied Acoustics_ plugins like **Chromaphone 3** under _Bitwig Studio_, text entry will cause the plugin to crash because Chromaphone uses a different text entry method when it detects Bitwig. You can use the diff --git a/src/common/configuration.cpp b/src/common/configuration.cpp index c71998be..7fabf027 100644 --- a/src/common/configuration.cpp +++ b/src/common/configuration.cpp @@ -101,12 +101,6 @@ Configuration::Configuration(const fs::path& config_path, } else { invalid_options.push_back(key); } - } else if (key == "editor_double_embed") { - if (const auto parsed_value = value.as_boolean()) { - editor_double_embed = parsed_value->get(); - } else { - invalid_options.push_back(key); - } } else if (key == "editor_force_dnd") { if (const auto parsed_value = value.as_boolean()) { editor_force_dnd = parsed_value->get(); diff --git a/src/common/configuration.h b/src/common/configuration.h index b3da404d..25325580 100644 --- a/src/common/configuration.h +++ b/src/common/configuration.h @@ -94,20 +94,6 @@ class Configuration { */ std::optional disable_pipes; - /** - * If this is set to `true`, then the plugin editor should be embedded in - * yet another window. This would result in an embedding sequence of - * ` <-> <-> - * <-> `, where - * `` is the new addition. The only plugin I've - * encountered where this was necessary was PSPaudioware E27 (and it likely - * also applies to other PSPaudioware plugins with expandable GUIs). I also - * haven't noticed any issues caused from having this enabled, but having it - * behind a flag reduces the amount of moving parts so that's probably a - * better idea. - */ - bool editor_double_embed = false; - /** * If set to `true`, we'll remove the `XdndAware` property all ancestor * windows in `editor.cpp`. This is needed for REAPER as REAPER implements @@ -202,7 +188,6 @@ class Configuration { s.ext(disable_pipes, bitsery::ext::InPlaceOptional(), [](S& s, auto& v) { s.ext(v, bitsery::ext::BoostPath{}); }); - s.value1b(editor_double_embed); s.value1b(editor_force_dnd); s.value1b(editor_xembed); s.ext(frame_rate, bitsery::ext::InPlaceOptional(), diff --git a/src/plugin/bridges/common.h b/src/plugin/bridges/common.h index 436ce979..a3c18344 100644 --- a/src/plugin/bridges/common.h +++ b/src/plugin/bridges/common.h @@ -255,9 +255,6 @@ class PluginBridge { "hack: pipes disabled, plugin output will go to \"" + config.disable_pipes->string() + "\""); } - if (config.editor_double_embed) { - other_options.push_back("editor: double embed"); - } if (config.editor_force_dnd) { other_options.push_back("editor: force drag-and-drop"); } diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index b046d899..657369eb 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -299,10 +299,6 @@ Editor::Editor(MainContext& main_context, nullptr, GetModuleHandle(nullptr), this)), - // If `config.editor_double_embed` is set, then we'll also create a child - // window in `win32_child_window`. If we do this before calling - // `ShowWindow()` on `win32_window` we'll run into X11 errors. - win32_child_window(std::nullopt), idle_timer( Win32Timer(win32_window.handle, idle_timer_id, @@ -427,23 +423,7 @@ Editor::Editor(MainContext& main_context, // described in `Editor`'s docstring'. do_reparent(wine_window, wrapper_window.window); - // If we're using the double embedding option, then the child window - // should only be created after the parent window is visible ShowWindow(win32_window.handle, SW_SHOWNORMAL); - if (config.editor_double_embed) { - // As explained above, we can't do this directly in the initializer - // list - win32_child_window.emplace( - main_context, x11_connection, - CreateWindowEx(WS_EX_TOOLWINDOW, - reinterpret_cast(get_window_class()), - "yabridge plugin child", WS_CHILD, 0, 0, - client_area.width, client_area.height, - win32_window.handle, nullptr, - GetModuleHandle(nullptr), this)); - - ShowWindow(win32_child_window->handle, SW_SHOWNORMAL); - } } } @@ -727,12 +707,7 @@ void Editor::handle_x11_events() noexcept { } HWND Editor::get_win32_handle() const noexcept { - // FIXME: The double embed and XEmbed options don't work together right now - if (win32_child_window && !use_xembed) { - return win32_child_window->handle; - } else { - return win32_window.handle; - } + return win32_window.handle; } void Editor::fix_local_coordinates() const { @@ -937,7 +912,7 @@ std::optional Editor::get_current_pointer_position() const noexcept { // expose a function that just lets us translate X11 coordinates into // Windows coordinates. RECT win32_pos{}; - if (!GetWindowRect(get_win32_handle(), &win32_pos)) { + if (!GetWindowRect(win32_window.handle, &win32_pos)) { return std::nullopt; } diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index daa55cfb..f3c4f5c5 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -213,8 +213,7 @@ class Editor { /** * Get the Win32 window handle so it can be passed to an `effEditOpen()` - * call. This will return the child window's handle if double editor - * embedding is enabled. + * call. */ HWND get_win32_handle() const noexcept; @@ -365,16 +364,6 @@ class Editor { */ DeferredWin32Window win32_window; - /** - * A child window embedded inside of `win32_window`. This is only used if - * the `editor_double_embed` option is enabled. It can be used as a - * workaround for plugins that rely on their parent window's screen - * coordinates instead of their own (see the 'Editor hosting modes' section - * of the readme for more details). The plugin should then embed itself - * within this child window. - */ - std::optional win32_child_window; - /** * A timer we'll use to periodically run the X11 event loop plus * `idle_timer_proc`, if that is set. We handle X11 events from within the