Lmms: Strange problems when loading a project containing VST effects.

[Sauerstoffdioxid]

Please change the title to something more descriptive. I don't really know how to summarize this bug briefly.

I think this is best explained by a short video
Capture_3.zip (it's in the zip file)

Basically, when I load up a project with an effect plugin and open the controls window of an effect on the master channel (different channels not yet tested), there is a strange box with no (?) content, which keeps whatever was printed last in it.
I don't know how to describe it. That's why recorded the video. However, the recording software didn't record the cursor correctly, so imagine a resize arrow whenever I'm resizing the box (or am having trouble picking up the effect window).

I think this might be related to the rendering of the VST GUI. In this screenshot I resized it over the actual GUI.

And here is another image, very clearly showing that the black square of the NaN-Detector isn't drawn into the background.

Also, the affected projects I tested it in had in common, that there was a (almost) empty window of the size of the effect GUI leftover after loading the project (as seen in the video).
I can post a test project if needed.

"Works" with both plugin embedding methods: Win32 API and Qt. Although Qt is showing the detached GUI of the empty window, just not the way it should, I guess:

Windows 10 64bit, LMMS 1.2.0-rc5

[lukas-w]

Investigations by @DomClark from Discord (I hope you don't mind me copying them here):

When the plugin is saved visible, createUI is called twice upon load, which leaves an empty plugin container as the VST's UI is moved into the second one created.

I think I've found where the "strange box" mentioned in the bug report comes from:

The effect's container window is wrapped in a vstSubWin in VstPlugin::createUI. VstPlugin::m_pluginWidget is then set to the container window, and VstPlugin::m_pluginSubWindow to the vstSubWin wrapper.

But in VstEffectControlDialog::VstEffectControlDialog, m_pluginWidget is set to m_plugin->pluginWidget( false ), which returns VstPlugin::m_pluginWidget, which is then added into the VstEffectControlDialog. This leaves a childless vstSubWin, which is the strange box.
Presumably vstSubWin is supposed to be used when the plugin is meant to be a direct MDI child, i.e. when it is a VSTi loaded into VeSTige, but not when the plugin is inside another window, i.e. when it is an effect.
I've tried it with a fresh plugin (not loaded from a project) and the window is still added, it's just 0x0 so it's never seen.

The empty window is the vstSubWin from the first call to createUI that still contains a plugin container; the plugin is moved to a new container on the second call to createUI and the new container is put in the VstEffectControlDialog, leaving the second vstSubWin as the box.
When the plugin is hidden on project load, the first call to createUI never happens (hideUI is called instead of showUI), and in that case the empty window doesn't appear (since the plugin isn't moved to a new container), but there is still a box (since the container is still reparented to the control dialog).
Diagram of case when plugin is saved as visible:

Ignore the top row for the case when the plugin is not visible when saved

[PhysSong]

I can see a strange box even after loading projects which contains VST effects with guivisible="0".

I believe we can remove the empty window easily, by not calling createUI( NULL, false ) for VST effects. Removing the box looks trickier than the window, since it will require some modification of createUI's logic.
Creating container without parent window will remove the need for creating useless vstSubWin for VST effects, but it will require some more changes for VST instruments and might break embedding.

[lukas-w]

Creating container without parent window will remove the need for creating useless vstSubWin for VST effects, but it will require some more changes for VST instruments

I have a local WIP that does this. I'll polish it up a bit and push.

[lukas-w]

@PhysSong Pushed to fix/vst-effect-load. I've only tested this on Linux with Qt5.

[PhysSong]

Okay. I'll test it on other platforms and/or with Qt4 as soon as possible.

[zonkmachine]

On Linuxmint 17.3, Qt4, 826a5289ebe0f979eefe7f9361a27dd6885e6a9a

[zonkmachine]

Starting using X11Embed protocol.
unique ID: mbnc
RemotePlugin::DebugMessage: inputs: 2  output: 2
RemotePlugin::DebugMessage: creating editor
fixme:ole:RemUnknown_QueryInterface No interface for iid {00000019-0000-0000-c000-000000000046}
RemotePlugin::DebugMessage: editor successfully created

[lukas-w]

I won't have any time to continue working on this in the near future, but I believe most of the work needed has been done so I invite anyone to finish this on the fix/vst-effect-load branch.

[PhysSong]

VST effect dialog seems broken on Qt4, but it is broken on RC5 + Qt4 too.

[DomClark]

There's a slight inconsistency between the UI toggle buttons for VST instruments and effects here: for VST instruments, VeSTige has a simple push button, whereas the VST effect dialog has a toggleable button if the plugin is embedded, otherwise a push button. I feel this would be better if they were both always toggle buttons or always not. Any preferences here?

[tresf]

Something that indicates visibility is my vote. The pressed/maybe pressed buttons will always make me question myself. :)

[zonkmachine]

At 56e39924d68f3f5d078fe97542329aa0d3e8a3af, fix/vst-effect-load still won't load the gui but pressing Show/hide will instead toggle between a minimized (height) version and an empty window, as in my earlier comment, the size of the acutal vst gui.

_Edit: Fixed commit link._

[PhysSong]

The Qt4 issue is introduced in #3786. I've found that adef05fb71ddf595138e3eee8dddf0514072b8cf broke VST effect dialog on Qt4. The relevant parts are changes in VstPlugin.cpp and RemoteVstPlugin.cpp.
However, VST code has changed too much since that commit. :confused:

[DomClark]

@PhysSong thanks for finding the responsible commit; I've now got this working somehow at least. Here's what appears to be happening: QX11EmbedContainer wants to have a parent at construction, and moving it out of this parent breaks the embedding. Also, if its parent is moved somewhere where it no longer belongs to the main window, the embedding is broken (although it need not belong to the main window when the container is constructed).

Here's a diff with a potential fix. It's fairly ugly; since effect control dialogs aren't added to the main window until after creation, adding the plugin to the dialog during construction breaks the embedding, so instead I'm doing it when the dialog is shown (however, any time after the dialog is added to the main window should work).

diff --git a/plugins/VstEffect/VstEffectControlDialog.cpp b/plugins/VstEffect/VstEffectControlDialog.cpp
index e6d7764..62965d5 100644
--- a/plugins/VstEffect/VstEffectControlDialog.cpp
+++ b/plugins/VstEffect/VstEffectControlDialog.cpp
@@ -47,7 +47,8 @@ VstEffectControlDialog::VstEffectControlDialog( VstEffectControls * _ctl ) :
    m_pluginWidget( NULL ),

    m_plugin( NULL ),
-   tbLabel( NULL )
+   tbLabel( NULL ),
+   m_needsEmbed(false)
 {
    QGridLayout * l = new QGridLayout( this );
    l->setContentsMargins( 10, 10, 10, 10 );
@@ -61,6 +62,7 @@ VstEffectControlDialog::VstEffectControlDialog( VstEffectControls * _ctl ) :
    {
        m_plugin = _ctl->m_effect->m_plugin;
        embed_vst = m_plugin->embedMethod() != "none";
+       m_needsEmbed = embed_vst;

        if (embed_vst) {
            if (! m_plugin->pluginWidget()) {
@@ -232,9 +234,6 @@ VstEffectControlDialog::VstEffectControlDialog( VstEffectControls * _ctl ) :
        l->addItem( new QSpacerItem( newSize - 20, 30, QSizePolicy::Fixed,
                        QSizePolicy::Fixed ), 1, 0 );
        l->addWidget( resize, 2, 0, 1, 1, Qt::AlignCenter );
-       if (embed_vst) {
-           l->addWidget( m_pluginWidget, 3, 0, 1, 1, Qt::AlignCenter );
-       }
        l->setRowStretch( 5, 1 );
        l->setColumnStretch( 1, 1 );

@@ -272,6 +271,15 @@ void VstEffectControlDialog::paintEvent( QPaintEvent * )
 }


+void VstEffectControlDialog::showEvent( QShowEvent * )
+{
+   if( m_needsEmbed )
+   {
+       m_needsEmbed = false;
+       static_cast<QGridLayout *>(layout())->addWidget(
+           m_pluginWidget, 3, 0, 1, 1, Qt::AlignCenter );
+   }
+}


 VstEffectControlDialog::~VstEffectControlDialog()
diff --git a/plugins/VstEffect/VstEffectControlDialog.h b/plugins/VstEffect/VstEffectControlDialog.h
index e209150..c15b13c 100644
--- a/plugins/VstEffect/VstEffectControlDialog.h
+++ b/plugins/VstEffect/VstEffectControlDialog.h
@@ -32,6 +32,7 @@
 #include <QPainter>
 #include <QLabel>
 #include <QSharedPointer>
+#include <QShowEvent>


 class VstEffectControls;
@@ -50,6 +51,7 @@ public:

 protected:
    virtual void paintEvent( QPaintEvent * _pe );
+   virtual void showEvent( QShowEvent * _se );

 private:
    QWidget * m_pluginWidget;
@@ -65,6 +67,8 @@ private:

    QLabel * tbLabel;

+   bool m_needsEmbed;
+
 public slots:
    void togglePluginUI( bool checked );
 } ;
diff --git a/plugins/vst_base/VstPlugin.cpp b/plugins/vst_base/VstPlugin.cpp
index 32230cd..a5ac5fc 100644
--- a/plugins/vst_base/VstPlugin.cpp
+++ b/plugins/vst_base/VstPlugin.cpp
@@ -656,10 +656,11 @@ void VstPlugin::createUI( QWidget * parent )
 #ifdef LMMS_BUILD_LINUX
    if (m_embedMethod == "xembed" )
    {
-       QX11EmbedContainer * embedContainer = new QX11EmbedContainer( parent );
+       container = new QWidget( parent );
+       QX11EmbedContainer * embedContainer = new QX11EmbedContainer( container );
        connect(embedContainer, SIGNAL(clientIsEmbedded()), this, SLOT(handleClientEmbed()));
        embedContainer->embedClient( m_pluginWindowID );
-       container = embedContainer;
+       embedContainer->setFixedSize( m_pluginGeometry );
    } else
 #endif
    {
@@ -671,8 +672,6 @@ void VstPlugin::createUI( QWidget * parent )
    container->setWindowTitle( name() );

    m_pluginWidget = container;
-
-   container->setFixedSize( m_pluginGeometry );
 }

 bool VstPlugin::eventFilter(QObject *obj, QEvent *event)

[zonkmachine]

Here's a diff with a potential fix.

Tested, works fine here! (linuxmint 17.3/wine-1.6.2)

[DomClark]

Done some more testing and it seems to cause Qt embedding to segfault with VST effects on both Windows and Linux, and messes up z-ordering on Win32 embedding (although this sorts itself out after clicking on other windows a bit). Since we're dropping Qt4 support after 1.2 (right?), I'm just going to wrap it in defined(LMMS_BUILD_LINUX) && QT_VERSION < 0x050000 and not worry too much.

[zonkmachine]

Since we're dropping Qt4 support after 1.2 (right?)...

Correct.

[zonkmachine]

Is fix/vst-effect-load ready to merge?

[DomClark]

There's a couple more things I want to add, where there are few minor glitches with VST effects, but I'm away until late on Friday so if you're aiming to get RC6 out within the week (I see this is the last thing left) you might want to merge and I'll finish off later. It's certainly functional at least, but not exactly optimal.

[zonkmachine]

I think we'll wait for you then. No hurry!

[Umcaruje]

Ok, I tested this out for a bit.

No Embedding causes BuzMaxi3 to crash under wine, (Here's the wine output) though some other effects I tried don't seem to crash. Everything else seems to work fine though.

I don't like that the VST windows don't remember their positions when I show/hide them. I also think it would be beneficial if the VST window actually had a flag to always stay on top, since it dissapeares every time I click over to LMMS, which is bad UX IMO (Out of scope for this Pull Request though)

The Qt API Embedding method worked fine with instruments (synth1) but it crashes my X server if I show/hide a VST effect more than once.

The XEmbed protocol works good with VST instruments, but there's a problem when loading a project containing VST effects:

Saved VST effects don't show properly, while if you load a new VST it gets embeded properly. In addition the first time you click the controls button, the window is extremely small, until you move it

Tested using elementary OS loki running wine-2.0.1 and Qt 5.9.2, I'll test to see if updating my wine fixes some problems
Edit: same problems occur on wine-3.6.0 (Staging)

[DomClark]

No Embedding causes BuzMaxi3 to crash under wine

Unfortunately I won't have access to my Linux machine for a few months, so I can't test this, but can you try reverting 4ba0861 (in particular, the changes to RemoteVstPlugin.cpp) and see if that fixes it? I'm assuming it works fine in RC5.

I don't like that the VST windows don't remember their positions when I show/hide them.

Is this when they're not embedded? I believe this is the same behaviour as RC5, but I think I can fix this fairly easily.

I also think it would be beneficial if the VST window actually had a flag to always stay on top

I agree - would it be worth making this user-configurable? As for being out of scope, PhysSong is wanting to fix an additional issue using this PR, so this could be included too.

The Qt API Embedding method worked fine with instruments (synth1) but it crashes my X server if I show/hide a VST effect more than once.

Does this happen with RC5? Is there an easy way to get a backtrace for this?

As for the other embedding issues, I don't recall experiencing these but I was short on time and didn't do extensive Linux testing for commits later than c26bd5b (not that those commits look like likely culprits). If these issues were caused by this PR, can you bisect or something? Otherwise someone else will have to fix them or they can go in a later version.

[Umcaruje]

Ok did tests in RC5, seems like the BuzMaxi3 Crash happens in RC5 too so not caused by this PR.

As for being out of scope, PhysSong is wanting to fix an additional issue using this PR, so this could be included too.

If you could implement the no embed improvements that I proposed that would be awesome

The Qt API Embedding method worked fine with instruments (synth1) but it crashes my X server if I show/hide a VST effect more than once.
Does this happen with RC5? Is there an easy way to get a backtrace for this?

It does not happen with RC5, only with this pull request.

As for the other embedding issues, I don't recall experiencing these but I was short on time and didn't do extensive Linux testing for commits later than c26bd5b (not that those commits look like likely culprits). If these issues were caused by this PR, can you bisect or something?

The XEmbed method won't show any of the VST effects on RC5 for my machine

[DomClark]

If you could implement the no embed improvements that I proposed that would be awesome

Mostly done in c4d610e - windows should now remember their position, but always-on-top behaviour only works on Windows - I don't know how to do this on Linux, since it needs cooperation from both the LMMS window and the VST window. (Unless you go for system-wide rather than application-wide always-on-top, but that would probably more annoying than useful...) Edit: done via 4d27863.

The Qt API Embedding method worked fine with instruments (synth1) but it crashes my X server if I show/hide a VST effect more than once. ... It does not happen with RC5, only with this pull request.

Can you bisect or provide a backtrace? I don't really have anything to go off at the moment. Alternatively, can anyone else reproduce this and investigate?

The XEmbed method won't show any of the VST effects on RC5 for my machine

So it's broken both with RC5 and this PR? I don't recall having that problem on Ubuntu, perhaps someone else can help out here.

[qnebra]

Hmm, maybe completly ripoff "Show/Hide GUI" option in Vestige subwindow, and set show value pernamently to enabled? Both for instruments and effects.

[zonkmachine]

Since c26bd5bbd62ecd8cf2b3624ad151e73fcf4bdf82 (I believe), the VST gui will be always on top but its container window will work as for other effects, so a window on top of the VST will be in between the VST and its container.
_Edit: LinuxMint 17.3/Qt4_

The 3osc in this example is selected and dragged over the Ambience reverb VST but the reverb is on top and its constainer underneath the 3osc.

[qnebra]

@zonkmachine Well, that's enormous glitch. Glitchilla.

But also I think "Show/hide" should be rip off. Or, maybe it's a stupid idea, but what about making "vst graphic gui" and "those lmms thing with knobs of vst parameters" as separate tabs? Also, maybe, "tltwkovp" should have some sort of filter tool in it. Because in especially instruments plugins number of parameters going to crazy levels, 100+ and it's confusing to find needed one.

For example, you need to automate "cutoff" parametr. Open "knobs window" in vestige, and search row by row for those needed "cutoff" in bunch of other parametrs. Filter, I think, will be very useful in this situation.

[zonkmachine]

In the mean time. Should we go ahead and release RC6 now as it looks like this issue will take a while to solve?

[zonkmachine]

@DomClark I tested your latest commits and the problem with the VST gui always on top is now fixed.

[DomClark]

I got Ubuntu on a VM in order to do a little more work here:

VST gui z-order problems now fixed in 93cfb1d, solving https://github.com/LMMS/lmms/issues/4110#issuecomment-384383045.
Always-on-top behaviour for non-embedded VSTs implemented for Linux in 4d27863, finishing @Umcaruje's suggestions.

Up-to-date replies to Umcaruje:

The Qt API Embedding method worked fine with instruments (synth1) but it crashes my X server if I show/hide a VST effect more than once. ... It does not happen with RC5, only with this pull request.

Cannot reproduce (Ubuntu 18.04, Qt 5.9.5, Wine 3.0) - can you investigate further, or can anyone else reproduce?

The XEmbed protocol works good with VST instruments, but there's a problem when loading a project containing VST effects [image not quoted] Saved VST effects don't show properly, while if you load a new VST it gets embeded properly

I experience this with RC5 but not this PR - is this still reproducible with the current state of this PR?

In addition the first time you click the controls button, the window is extremely small, until you move it

I get this too, both with this PR and RC5 (despite the fact the effect won't show properly), so it's not caused here but is definitely in scope for fixing. A cursory investigation hasn't shown up any obvious cause though - can you (or anyone else) bisect this or something? (I don't have time to do it myself right now.)

[lukas-w]

In addition the first time you click the controls button, the window is extremely small, until you move it

Added a workaround for this via e963287321257862ebfebdabbef6f614916b3aad.

[lukas-w]

The Qt API Embedding method worked fine with instruments (synth1) but it crashes my X server if I show/hide a VST effect more than once. ... It does not happen with RC5, only with this pull request.

Cannot reproduce (Ubuntu 18.04, Qt 5.9.5, Wine 3.0) - can you investigate further, or can anyone else reproduce?

Can't reproduce either.

The XEmbed protocol works good with VST instruments, but there's a problem when loading a project containing VST effects [image not quoted] Saved VST effects don't show properly, while if you load a new VST it gets embeded properly

I experience this with RC5 but not this PR - is this still reproducible with the current state of this PR?

I too had this problem since c26bd5bbd62ecd8cf2b3624ad151e73fcf4bdf82 with Qt5, but 93cfb1d6756400f324deec64c307a7d4d6d4f7e6 fixed it.