-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Strange problems when loading a project containing VST effects. #4110
Comments
Investigations by @DomClark from Discord (I hope you don't mind me copying them here):
|
I have a local WIP that does this. I'll polish it up a bit and push. |
Fixes bugs where during project loading (observed with VST effects), empty widgets and sub-windows would be left floating around. These were caused by inconsistencies between the way VST UIs were created when loading a project and when adding an effect in an existing project. In some situations, this caused createUI to be called twice, leaving over multiple empty widgets. This commit refactors some code in order to avoid creating unnecessary sub- windows, which aren't needed with VST effects, but were still created, usually being invisible. All sub-window related code was moved out of VstPlugin into vestige.cpp, which is the only place where sub-window VSTs are actually used. A new sub-class of VstPlugin, VstInstrumentPlugin, now handles VST sub-windows and is used by vestigeInstrument. "guivisible" attribute loading was moved out of VstPlugin as well and is now done in VstEffectControls' and vestigeInstrument's loadSettings method respectively. This causes some minor code duplication unfortunately. Closes #4110
@PhysSong Pushed to fix/vst-effect-load. I've only tested this on Linux with Qt5. |
Okay. I'll test it on other platforms and/or with Qt4 as soon as possible. |
On Linuxmint 17.3, Qt4, 826a528 |
|
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 |
VST effect dialog seems broken on Qt4, but it is broken on RC5 + Qt4 too. |
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? |
Something that indicates visibility is my vote. The pressed/maybe pressed buttons will always make me question myself. :) |
Fixes bugs where during project loading (observed with VST effects), empty widgets and sub-windows would be left floating around. These were caused by inconsistencies between the way VST UIs were created when loading a project and when adding an effect in an existing project. In some situations, this caused createUI to be called twice, leaving over multiple empty widgets. This commit refactors some code in order to avoid creating unnecessary sub- windows, which aren't needed with VST effects, but were still created, usually being invisible. All sub-window related code was moved out of VstPlugin into vestige.cpp, which is the only place where sub-window VSTs are actually used. A new sub-class of VstPlugin, VstInstrumentPlugin, now handles VST sub-windows and is used by vestigeInstrument. "guivisible" attribute loading was moved out of VstPlugin as well and is now done in VstEffectControls' and vestigeInstrument's loadSettings method respectively. This causes some minor code duplication unfortunately. Closes #4110
At 56e3992, 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 thanks for finding the responsible commit; I've now got this working somehow at least. Here's what appears to be happening: 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) |
Tested, works fine here! (linuxmint 17.3/wine-1.6.2) |
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 |
Correct. |
Is fix/vst-effect-load ready to merge? |
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. |
I think we'll wait for you then. No hurry! |
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 |
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
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 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.
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. |
Ok did tests in RC5, seems like the BuzMaxi3 Crash happens in RC5 too so not caused by this PR.
If you could implement the no embed improvements that I proposed that would be awesome
It does not happen with RC5, only with this pull request.
The XEmbed method won't show any of the VST effects on RC5 for my machine |
Mostly done in c4d610e - windows should now remember their position,
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?
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. |
Hmm, maybe completly ripoff "Show/Hide GUI" option in Vestige subwindow, and set show value pernamently to enabled? Both for instruments and effects. |
Since c26bd5b (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. 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. |
@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. |
|
@DomClark I tested your latest commits and the problem with the VST gui always on top is now fixed. |
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 #4110 (comment). Up-to-date replies to Umcaruje:
Cannot reproduce (Ubuntu 18.04, Qt 5.9.5, Wine 3.0) - can you investigate further, or can anyone else reproduce?
I experience this with RC5 but not this PR - is this still reproducible with the current state of this PR?
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.) |
Added a workaround for this via e963287. |
Can't reproduce either.
I too had this problem since c26bd5b with Qt5, but 93cfb1d fixed it. |
Fixes bugs where during project loading (observed with VST effects), empty widgets and sub-windows would be left floating around. These were caused by inconsistencies between the way VST UIs were created when loading a project and when adding an effect in an existing project. In some situations, this caused createUI to be called twice, leaving over multiple empty widgets. This commit refactors some code in order to avoid creating unnecessary sub- windows, which aren't needed with VST effects, but were still created, usually being invisible. All sub-window related code was moved out of VstPlugin into vestige.cpp, which is the only place where sub-window VSTs are actually used. A new sub-class of VstPlugin, VstInstrumentPlugin, now handles VST sub-windows and is used by vestigeInstrument. "guivisible" attribute loading was moved out of VstPlugin as well and is now done in VstEffectControls' and vestigeInstrument's loadSettings method respectively. This causes some minor code duplication unfortunately. Closes LMMS#4110
Fixes bugs where during project loading (observed with VST effects), empty widgets and sub-windows would be left floating around. These were caused by inconsistencies between the way VST UIs were created when loading a project and when adding an effect in an existing project. In some situations, this caused createUI to be called twice, leaving over multiple empty widgets. This commit refactors some code in order to avoid creating unnecessary sub- windows, which aren't needed with VST effects, but were still created, usually being invisible. All sub-window related code was moved out of VstPlugin into vestige.cpp, which is the only place where sub-window VSTs are actually used. A new sub-class of VstPlugin, VstInstrumentPlugin, now handles VST sub-windows and is used by vestigeInstrument. "guivisible" attribute loading was moved out of VstPlugin as well and is now done in VstEffectControls' and vestigeInstrument's loadSettings method respectively. This causes some minor code duplication unfortunately. Closes #4110
Fixes bugs where during project loading (observed with VST effects), empty widgets and sub-windows would be left floating around. These were caused by inconsistencies between the way VST UIs were created when loading a project and when adding an effect in an existing project. In some situations, this caused createUI to be called twice, leaving over multiple empty widgets. This commit refactors some code in order to avoid creating unnecessary sub- windows, which aren't needed with VST effects, but were still created, usually being invisible. All sub-window related code was moved out of VstPlugin into vestige.cpp, which is the only place where sub-window VSTs are actually used. A new sub-class of VstPlugin, VstInstrumentPlugin, now handles VST sub-windows and is used by vestigeInstrument. "guivisible" attribute loading was moved out of VstPlugin as well and is now done in VstEffectControls' and vestigeInstrument's loadSettings method respectively. This causes some minor code duplication unfortunately. Closes #4110
Fixes bugs where during project loading (observed with VST effects), empty widgets and sub-windows would be left floating around. These were caused by inconsistencies between the way VST UIs were created when loading a project and when adding an effect in an existing project. In some situations, this caused createUI to be called twice, leaving over multiple empty widgets. This commit refactors some code in order to avoid creating unnecessary sub- windows, which aren't needed with VST effects, but were still created, usually being invisible. All sub-window related code was moved out of VstPlugin into vestige.cpp, which is the only place where sub-window VSTs are actually used. A new sub-class of VstPlugin, VstInstrumentPlugin, now handles VST sub-windows and is used by vestigeInstrument. "guivisible" attribute loading was moved out of VstPlugin as well and is now done in VstEffectControls' and vestigeInstrument's loadSettings method respectively. This causes some minor code duplication unfortunately. Closes LMMS#4110
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
The text was updated successfully, but these errors were encountered: