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.

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

Fix Wine window wrapping and remove coordinate hack. #405

Closed
wants to merge 2 commits into from

Conversation

rbernon
Copy link

@rbernon rbernon commented Feb 27, 2025

This should be enough to get things properly working again with more recent Wine. I've only tried with some basic plugin and Carla but I don't think there's anything special to do or workaround here. Wine should be tracking any kind of window parents properly, as window managers can already do the exact same kind of things.

I also kept a couple of places where fix_local_coordinates was referenced but, as I wasn't sure if they were safe to drop too. XEMBED support could probably be dropped now, I don't expect it to be working well.

@PennRobotics
Copy link

I have a meeting but will compile this in two hours and report back!

Thanks Rémi!

@rbernon
Copy link
Author

rbernon commented Feb 27, 2025

Added a couple of comments and removed client_area_ which also isn't needed anymore.

@PennRobotics
Copy link

PennRobotics commented Feb 27, 2025

Hmmm, build failure for me, fatal error: xcb/xcb_icccm.h: File not found

Would I need a line added to meson.build, like dependency(...)?


nevermind, as easy as dnf install xcb-util-wm-devel

@PennRobotics
Copy link

PennRobotics commented Feb 27, 2025

Hmmm. Plugins are not loading, I get a single wine error, kf.kio.gui: Could not find an executable named: "konsole"

I'll keep checking if I'm misconfigured

winedbg was true in meson options. setting it false got plugins to load

@PennRobotics
Copy link

The context menu is still loading in the same place regardless of where I position the plugin window. Checking now if this PR works with 9.22 to 10.2 (before the Wine merge request)

@rbernon
Copy link
Author

rbernon commented Feb 27, 2025

I've been testing it with latest Wine on GNOME/X11, Carla and TripleCheese plugin, without any specific yabridge configuration other than just syncing the plugin. It worked fine for me, including with the popup menus, even after moving the plugin window around.

Please test it with 10.2, and if you could attach a log with WINEDEBUG=+x11drv,+event it would be nice, describing what interaction you did would be nice too.

@murl-digital
Copy link

i'll get to testing this as well, does this just use the latest release version of wine, or does it rely on the open PR on gitlab?

@rbernon
Copy link
Author

rbernon commented Feb 27, 2025

Just latest Wine should work, or at least it did for me but apparently other people have different experience, so maybe still some things to figure out.

@rbernon
Copy link
Author

rbernon commented Feb 27, 2025

Added tweak to also send the ConfigureNotify to Wine when host_window_ != parent_window_. I don't really understand all these nested levels, and I'm assuming all the windows are directly into one another without any offset but the idea is to notify Wine whenever the toplevel window has moved and that it changes the Wine window effective position on the screen, similar to what a window manager is supposed to do.

@ManuLinares
Copy link

ManuLinares commented Feb 27, 2025

I've tested this, now it works! Thanks!

yabridge-log.txt.zip

@murl-digital
Copy link

i've also tested it and can confirm it works perfectly with wine-10.1 (arch build), had to fenangle it because it tried to use the yabridge-host.exe from the build i had installed, but after that phase plant, IL harmor, and valhalla room worked without issue

@PennRobotics
Copy link

PennRobotics commented Feb 27, 2025

This isn't working in REAPER.

I compiled 10.2 without the patch and used that to compile 40ef7cd.

With that combination, all clicks seem to be offset the distance from the REAPER plugin window origin to the plugin window origin, no matter where the window is moved.

Screencast_20250227_204832.webm

yabridge-log.txt

@PennRobotics
Copy link

Should I add back +cursor and/or +win?

@rbernon
Copy link
Author

rbernon commented Feb 27, 2025

WINEDEBUG=+x11drv,+event,+cursor should be good

@PennRobotics
Copy link

@rbernon
Copy link
Author

rbernon commented Feb 27, 2025

But I guess this is one case I wasn't aware of and that it breaks the assumption I've described above where I expected every window to be at the top-left corner from each other. I'll adjust the changes accordingly. Thanks.

@robbert-vdh
Copy link
Owner

Thanks so much for looking into this! I spent a couple hours digging through Wine's x11drv over the last couple weeks trying to get the XEmbed support to work again (which used to work fine-ish until at least Wine 7.0), but I haven't had any luck figure out what which part isn't working correctly anymore (since all of the gravity notify machinery to keep screen coordinates up to date seemed to be working as intended). But if this approach works, then that's even better! This is more or less a much, much cleaner version of that fix_local_coordinates() trick. If it also works for older Wine versions, then that would be a best of all worlds kind of situation.

I sadly won't have the time to properly take a look at this until Saturday, so I'll get back to you then! From a super brief test run this approach seems to be 95% of the way there for both wine-10.2-92-g32a81ee2135 (the current latest master branch) and Wine 9.21 on KDE Wayland 6.3.1. I'm mostly seeing some issues with the client area initially being cropped (which seems to be a Win32-side thing, since xwininfo -tree shows the expected sizes), some fighting happening when resizing some plugin GUIs (e.g. with the KiloHearts plugins), and a missing coordinate translation on the ConfigureNotify event (e.g. when used within REAPER and presumably Ardour and Waveform where parent_window does not live at 0x0 in the host_window). All of these should be relatively easy fixes, so I'll look into that on Saturday!

XEMBED support could probably be dropped now, I don't expect it to be working well.

Yeah that's now no longer necessary since this approach would strictly work better than the XEmbed approach did.

Added tweak to also send the ConfigureNotify to Wine when host_window_ != parent_window_. I don't really understand all these nested levels, and I'm assuming all the windows are directly into one another without any offset but the idea is to notify Wine whenever the toplevel window has moved and that it changes the Wine window effective position on the screen, similar to what a window manager is supposed to do.

Yeah it's a bit confusing:

  • host_window_ is the actual 'window' from a user point of view, e.g. the top level window that would also show up in your typical task manager and task switcher.
  • parent_window_ is the window provided by the plugin host for the plugin to embed itself into. Depending on the host this can be the same as host_window_, a child window of host_window_ at offset 0x0, or sometimes a window that's nested much deeper and that's at a different offset. REAPER is a prime example of a host that does that.
  • wine_window_ is the __wine_x11_whole_window of a Win32 window we create for the Windows plugin to embed itself in.
  • And lastly wrapper_window_ is a window that sits in between parent_window_ and wine_window_ that was unfortunately necessary to prevent some hosts from misbehaving because they'd listen to the ConfigureNotify events on the Wine window through a SubStructureNotify on the parent_window_. Adding another window in between there prevents that from happening and causing issues. But with these changes that may no longer be necessary, so I'll check if that can be removed again.

@rbernon
Copy link
Author

rbernon commented Feb 28, 2025

Added a couple of more changes to address the broken assumption with host_window / parent_window relative positions. This should fix cases where the plugin is at a non zero offset within its host window.

@bzeiss
Copy link

bzeiss commented Feb 28, 2025

I'm fiddling around with fabfilter and this merge request on Fedora 41 with wine staging 10.2 from WineHQ, Gnome on Xorg. I'm using Renoise 3.4.4, Bitwig 5.3.1. I have also updated to the latest fixes 1 hour ago. Overall it works pretty well for VST2 plugins from my initial tests (Fabfilter, Serum, Kilohearts, Scaler). In Bitwig, I have to move the window of FabFilter plugins or Serum initially before they are usable (it does not matter whether I use VST2 or Clap versions). In the initial state, the mouse does not appear on the plugin window. This is working fine in Renoise though.

This is looking like it will be a good fix though. Thanks a lot for the effort!

@PennRobotics
Copy link

PennRobotics commented Feb 28, 2025

Works perfect (also on 9.22) w/ at least Valhalla. @rbernon ? You're the king.

@PennRobotics
Copy link

In Bitwig, I have to move the window of FabFilter plugins or Serum initially before they are usable (it does not matter whether I use VST2 or Clap versions)

FabFilter Pro-Q 4 (all 3 plugin types) works correctly for me; in REAPER, Pro-Q shows its EQ line and the mouse hover dot without needing to first move the window. If you upload a log, I can do the same and we figure out what about Bitwig is loading differently than REAPER.

(I know there have been discussions about Bitwig e.g. #354 and that it won't work as a Flatpak from the README, but I don't know if that relates to what you've described)

@bzeiss
Copy link

bzeiss commented Feb 28, 2025

Attached is the log file. Not sure whether this is a lot of help though...
yabridge.log

@PennRobotics
Copy link

For comparison, a log of the working version
yabridge-log.txt

@PennRobotics
Copy link

PennRobotics commented Feb 28, 2025

At minimum, mine has a handful more VisibilityNotify and ConfigureNotify, while yours has an extra ReparentNotify.

Would Bitwig be open to a bug report? (I imagine they'd know more than anyone what to change if they see a working implementation next to their own)

I guess the other suggestion would be to try some of the compatibility options, like editor_disable_host_scaling or hide_daw?

@bzeiss
Copy link

bzeiss commented Feb 28, 2025

Went back to wine 10.0 stable, didn't help either. Are you using Bitwig 5.3 as well or an earlier version? I think they have changed stuff with gpu acceleration of the UI in that version.

@PennRobotics
Copy link

I'm using REAPER, but it displays totally fine. I figure if there's a difference in how Bitwig works vs other DAWs, that might be obvious from the logs.

I can't help w/ Bitwig specifically 😢

@bzeiss
Copy link

bzeiss commented Feb 28, 2025

I have just tried REAPER, it has the same problem as Bitwig. I have to move the Window a bit after first opening (or reopening). So it doesn't seem to be a pure Bitwig problem. Oddly, Renoise doesn't show this behavior.

@Kilgarragh
Copy link

Kilgarragh commented Mar 1, 2025

Just tried Ardour 8.1 on wine 10(staging), and I'm happy to report it works perfectly(just as it did on wine 9.0).

EDIT: there is still some vertical cursor offset, seemingly caused by ardour's secondary ribbon on primary plugin windows(offset is not present on plugin child windows which lack the ribbon)

@robbert-vdh
Copy link
Owner

robbert-vdh commented Mar 1, 2025

Again, thanks so much for looking into this! I merged this into the (hopefully, fairly short lived) new-wine10-embedding branch so we can work out the remaining issues. I've created a tracking issue for this branch to collect bugs and feedback here: #409

Some observations with the current state:

  • I had issues with the Win32 window's client area not being resized correctly. For Wine 9.21 this always happened and the window would be cropped to 128x128 (the initial wine_window_'s size). With master branch Wine the initial resize would come through, but then if there was a second resize right after that that seemed to be ignored. This happens for JUCE plugins when using DPI scaling (e.g. with the font DPI in winecfg set to anything other than 96). I fixed this for most cases by swapping the xcb_configure_window() for wine_window_ in Editor::resize() out for a SetWindowPos(), but that hasn't solved it for every case. VST3 Valhalla Supermassive 3.0.0 still reliably triggers the problem for me.
  • That wrapper_window_ still cannot be removed without making window very jittery in hosts that intercept ConfigureNotify events on the plugin's window, like Carla.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants