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

Add Qt5 Linux VST subwindow implementation #3295

Closed
wants to merge 1 commit into from
Closed

Add Qt5 Linux VST subwindow implementation #3295

wants to merge 1 commit into from

Conversation

lukas-w
Copy link
Member

@lukas-w lukas-w commented Jan 25, 2017

Uses QWindow::fromWinId and QWidget::createWindowContainer instead of
QX11EmbedContainer which was removed in Qt5.
Adds a workaround for mouse events that aren't accepted by the window
container, and thus were being propagated to the parent.

Fixes #2855. (Does this work on other platforms than Linux with Qt5?)
screenshot_2017-01-25_18-47-17

Uses QWindow::fromWinId and QWidget::createWindowContainer instead of
QX11EmbedContainer which was removed in Qt5.
Adds a workaround for mouse events that aren't accepted by the window
container, and thus are being propagated to the parent.
@jasp00
Copy link
Member

jasp00 commented Jan 26, 2017

@lukas-w, have a look at #2856.

@lukas-w
Copy link
Member Author

lukas-w commented Jan 26, 2017

@jasp00 Oh, I missed that. Just tried it out. I don't experience any visual or moving glitches, I guess that's fixed in Qt 5.7.1? However, I get the following issues:

  • The VST instrument opens in a separate window outside of LMMS instead of as a subwindow
  • It's possible to resize the window to a size smaller than its contents
  • I can close the window, but I can't bring it back using the "Show/hide GUI" button, only re-loading the project works.
  • "Show/hide GUI" feature is slow, takes a few seconds to open the window

Would you mind testing this PR? If there are no major bugs, I'd suggest merging this one in favor of #2856, as it touches a lot less code and doesn't have the issues mentioned above, nor any glitches in Qt versions prior to 5.7.1 (at least that I know of).

@jasp00
Copy link
Member

jasp00 commented Jan 26, 2017

I'd suggest merging this one in favor of #2856

As you can see in 23c3b21, I just tried what you do now. Subwindows cause glitches in Qt 5.6.1 and 5.7.1. The issues that you have detected should be fixed:

  • The VST instrument opens in a separate window outside of LMMS instead of as a subwindow

This is unavoidable because subwindows cause glitches. This way it works like ZynAddSubFX.

  • It's possible to resize the window to a size smaller than its contents

QMainWindow::setFixedSize() does not work on your system. I will try setting the layout instead.

  • I can close the window, but I can't bring it back using the "Show/hide GUI" button, only re-loading the project works.

Do you use X11? Does ps -A report a defunct embed-window process?

  • "Show/hide GUI" feature is slow, takes a few seconds to open the window

It is DestroyWindow() from Wine actually. If you wait after closing, then you may open faster, right? This could be improved by using a (Windows) thread.

@lukas-w
Copy link
Member Author

lukas-w commented Jan 27, 2017

Do you use X11? Does ps -A report a defunct embed-window process?

Yes & Yes. Just tested it again and the window does reappear when clicking "Show/hide GUI". Sorry for the false negative.

QMainWindow::setFixedSize() does not work on your system. I will try setting the layout instead.

Yep, that could actually be my WM's (i3) fault.

Subwindows cause glitches in Qt 5.6.1 and 5.7.1

This is unavoidable because subwindows cause glitches.

My point is that I don't have any glitches in 5.7.1 with this PR, so it's possible the glitches you experience are not caused by Qt.

@lukas-w
Copy link
Member Author

lukas-w commented Jan 27, 2017

Just tested on Ubuntu 16.04 LTS with Qt 5.5.1. No glitches with this PR or with yours. However, when using 23c3b21, the VST subwindow is empty.

@jasp00
Copy link
Member

jasp00 commented Jan 27, 2017

it's possible the glitches you experience are not caused by Qt.

We see environments without glitches and others with them. The glitches are triggered by the switch from Qt 4 to Qt 5 and they persist after removing the embedding subwindow. Even with separated windows, Qt 5 applications behave erratically, not just LMMS. Is it really a Qt bug? I will not know for sure until I debug Qt 5, but Qt 4 does not trigger this.

@BaraMGB
Copy link
Contributor

BaraMGB commented Feb 6, 2017

In my opinion we should really consider to have the VST plugins into a QDialog instead of a QSubWindow. The user can move the window to a second desktop and we prepare a future single window solution for lmms which wouldn't work with subwindows.

@jasp00
Copy link
Member

jasp00 commented Feb 13, 2017

we should really consider to have the VST plugins into a QDialog

This is an ill concept. Why would a dialog work when a widget does not?

we prepare a future single window solution for lmms which wouldn't work with subwindows.

Yes, it does not work with subwindows, it works with widgets, which is essentially the same.

@BaraMGB, you mentioned QDialog at #2856 (comment). Could you provide the implementation so I could confirm whether the glitches are present?

@lukas-w
Copy link
Member Author

lukas-w commented Feb 13, 2017

@jasp00 Did you actually test this PR with Qt 5.7 or were you just referring to experiences made with your implementation? (23c3b21)

@BaraMGB
Copy link
Contributor

BaraMGB commented Feb 13, 2017

Could you provide the implementation so I could confirm whether the glitches are present?

This could be difficult. Unfortunately I never pushed it to github. Perhaps I have the branch on my laptop. I will look tonight when I get home.

@BaraMGB
Copy link
Contributor

BaraMGB commented Feb 13, 2017

Okay, found the branch. Rebased it: https://github.com/BaraMGB/lmms/tree/vstembedqt5
I guess I made a lot of stupid stuff there. The problem was the window doesn't show the plugin at first. You have to click hide/show Gui for it.

@jasp00
Copy link
Member

jasp00 commented Feb 15, 2017

Did you actually test this PR [...] ?

It is essentially the same code. Anyway, I have tested and it has the glitches.

we should really consider to have the VST plugins into a QDialog

I have tested your branch. QDialog is not needed, QWidget is enough. A dialog, by default, is modal and cannot be sent to the background. However, it uses a different window and the glitches are not there. The glitches appear when using QMdiSubWindow. Using QWidget with Qt::Window flag works; this flag may not be necessary in a single window interface.

So an external embedder is not needed. Protocol enhancements from #2856 should be merged into a Qt::Window solution. There are some XCB errors that do not seem to do harm.

I will pause development for some days.

@tresf
Copy link
Member

tresf commented Feb 15, 2017

A dialog, by default, is modal and cannot be sent to the background.

Only if setModal(true); but that can be toggled off with setModal(false);, so I'm not sure the purpose of this point.

@jasp00
Copy link
Member

jasp00 commented Feb 16, 2017

A dialog, by default, is modal and cannot be sent to the background.

My bad, it is modeless by default. Anyway, it cannot be sent to the background because of the default Qt::Dialog flag.

@lukas-w
Copy link
Member Author

lukas-w commented Feb 16, 2017

It is essentially the same code. Anyway, I have tested and it has the glitches.

Thanks @jasp00. Would you mind sharing some details about your environment (such as distro, Qt version, GPU/graphics driver)? I'd really like to be able to reproduce this.

So far, I tested (without glitches):

  • Arch Linux, Intel GPU, i3 window manager, Qt 5.7.1
  • Arch Linux, Nvidia GPU, Nvidia proprietary driver, i3 window manager, Qt 5.7.1
  • Arch Linux, Nvidia GPU, Nouveau driver, i3 window manager, Qt 5.8.0
  • Ubuntu 16.04 inside VirtualBox, Qt 5.5.1

@jasp00
Copy link
Member

jasp00 commented Mar 16, 2017

I'd really like to be able to reproduce this.

QMdiSubWindow is not required and I am the one able to fix that case. Anyway...

Would you mind sharing some details about your environment (such as distro, Qt version, GPU/graphics driver)?

I develop with Debian Sid, updated from time to time; as of today, glitches are still there. Qt version is 5.7.1+dfsg-3+b1. Graphics go through X11 without DRI drivers.

@karmux
Copy link
Contributor

karmux commented Mar 29, 2017

I tested this PR with Wine 2.4.0 and Qt 5.6.1 on Kubuntu 16.10.

VST effects from effects chain work without issues but VST instruments loaded into Vestige opens separately from Qt subwindow. There is empty Qt subwindow and borderless VST instrument, just like in @jasp00's PR when I tested it couple of months ago.

Difference is that in jasp00's PR VST instrument went inside Qt subwindow after clicking "Show/hide GUI" button twice in Vestige but here it always opens without window borders.

screenshot_20170329_235402

After testing some more I managed to make VST effects also open without window borders. This happened several times after playing and editing project a bit. Had to restart LMMS to make them open correctly again.

@lukas-w
Copy link
Member Author

lukas-w commented Apr 1, 2017

I can reproduce this with Qt 5.6.1 and Plasma and i3. Xfce doesn't have this issue.

I started a new approach on branch qt5-vst-2, porting QX11EmbedContainer to Qt5. @karmux Could you test the new implementation and check if it fixes the floating-window issue on your system?

@karmux
Copy link
Contributor

karmux commented Apr 1, 2017

@lukas-w, I tried this new approach and didn't get any window issues. 👍
I noticed only that instrument window border style is a bit different from effect window border style.

screenshot_20170401_193342

On the screenshot Kontakt 5 is instrument and Guitar Rig 5 is effect.

Had to install following new packages to be able to compile qt5-vst-2:

libqt5x11extras5-dev
qtbase5-private-dev
libxcb-util-dev
libxcb-keysyms1-dev

@jasp00
Copy link
Member

jasp00 commented Apr 1, 2017

I started a new approach on branch qt5-vst-2, porting QX11EmbedContainer to Qt5.

Same glitches there.

@lukas-w
Copy link
Member Author

lukas-w commented Apr 1, 2017

@jasp00 I guess you have the same glitches using Qt4 then, as it's the same code, right? What WM/desktop environment are you using?

@karmux Great, thanks for testing! 👍 I'll open a new PR to track qt5-vst-2's issues there.

@jasp00
Copy link
Member

jasp00 commented Apr 1, 2017

I guess you have the same glitches using Qt4 then

No, I only have glitches on Qt 5 when embedding in a subwindow.

What WM/desktop environment are you using?

Metacity and GNOME.

@lukas-w
Copy link
Member Author

lukas-w commented Apr 3, 2017

Metacity and GNOME.

So you're still using Gnome 2?

@jasp00
Copy link
Member

jasp00 commented Apr 4, 2017

So you're still using Gnome 2?

I do in this environment, GNOME 2 speaks X11.

@lukas-w
Copy link
Member Author

lukas-w commented Apr 8, 2017

@jasp00 I pushed some fixes to #3475. You're invited to re-test if you're interested.

However, even if your glitches persist, supporting a platform as old as GNOME 2 is out of scope for this feature and for LMMS in general. Support for it has been long dropped by most projects, and I don't know about any distro that still provides it, let alone provide GNOME 2 and Qt5 at the same time.

Note that prior to my most recent commits, almost all of the code in QX11EmbedContainer was unused because of a bug in Qt's XEMBED protocol implementation (QTBUG-36446). So the code you tested basically only consisted of a single XReparentWindow call. If this simple call causes issues on you're environment, I'm afraid there's little we can (or should) do.

Closing this as it'll most likely be superseded by #3475.

@lukas-w lukas-w deleted the qt5-vst branch May 7, 2018 17:27
@lukas-w lukas-w restored the qt5-vst branch May 7, 2018 17:27
@lukas-w lukas-w deleted the qt5-vst branch May 7, 2018 17:27
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.

5 participants