I've seen this happen once or twice. Earlier on (in #40) we could get
away with just reparenting the window another time. But here both
reparents succeed, and the issue still happens. But a reparent notify
for `parent_window` did appear on system affected by this issue, which
is very odd. Hopefully this will help.
This is the same way (minus the mapping check part) that `xprop` and
`xwininfo` filter windows when clicking on them. REAPER's toplevel
window apparently doesn't process any keyboard input when the mouse
cursor is located outside of that window.
REAPER initializes the plugin's editor first before reparenting the
parent window to the FX window, so our `topmost_window` didn't actually
refer to the FX window.
This was left over from the original implementation, before we had this
smart pointer-singleton hybrid thing. There's already a handle stored
inside of `Editor`.
We cannot integrate this into our event loop like we planned, because
Wine a) grabs the mouse pointer so we cannot do that, and b) blocks the
GUI thread. So instead we will spawn our own thread and do polling based
XDND. When Wine's tracker window gets destroyed, we know that the left
mouse button has been released.
Apparently X11 connections are a scarce resource, so it seems like a
good idea to not hang on to them for too long. Now this is sort of a
hybrid between COM-style memory management and a singleton.
Letting `std::unique_ptr<T>` do the thinking for us makes a lot more
sense. We only need manual memory management for the error because we
need to pass a pointer to that pointer to xcb, but at least we have the
macro there so it still stays nice and readable.
Some interaction between JUCE and Wine would cause these cursors to be
hidden and then never shown again. This is of course more of a temporary
workaround until the issue gets solved within Wine.
Instead of the parent Window. Tracktion Waveform does some weird things
with its VST2 editor embedding, so with the old approach this would
cause mouse clicks to be offset 27 pixels vertically and one pixel
horizontally.
Hopefully. Checking mapped state apparently wasn't enough to prevent
spurious assertion failures on `is_child_window_or_same()`. Now we'll
just use exceptions instead of assertions so we catch them and ignore
them.
This is probably a better idea than trying to be efficient. If we get
two events in a row, then it can be that the map status has changed
between the two events.
We did this before implementing the deferred close in yabridge 3.0.0. It
didn't seem necessary anymore so we got rid of it, but without this
closing an iZotope Rx plugin's editor in Renoise was guaranteed to
trigger an X11 error and crash Renoise. Doing this reparent doesn't seem
to cause any slowdown but it does at least fix the specific combination
of iZotope Rx and Renoise.
This is in some cases needed to get decent performance in REAPER, as
REAPER seems to query this information (which cannot change without the
plugin requesting a restart) four times per second.
In commit 9788f21e0e we changed the input
focus grabbing to only grab focus once per event cycle, because
otherwise Bitwig would fight over who was getting focus. This is a bit
hacky, and with this approach you could in theory still have an
unnecessary focus grab 60 times per second (i.e. the default event
polling rate). Now we instead check the current focus target, and only
request focus when it's actually necessary.
It's pretty hard to find a solution that checks all of the boxes. I want
something that:
- Closes instantly when you close the editor, and in REAPER you should
be able to instantly switch between docked and floating modes
- Where there should not be a delay in user interaction when quickly
reopening the editor (or doing that switching thing in REAPER since that's
the same thing)
- Where the window manager should not try to reparent the window during
the losing process as that can cause some jarring flickering
- And, of course, there should be no weird Wine X11Drv crashes
And it should do all of that in Bitwig, REAPER, Carla, Ardour and
Renoise. Apparently it's quite the task to find an approach that checks
all the boxes there.
Robbert van der Helm