Embed VST GUI in subwindow #2856

jasp00 · 2016-06-17T02:17:52Z

This fixes #2855, but the new window causes glitches. Initially, the window may not be painted properly. When any window is moved, it may shake.

BaraMGB · 2016-06-17T10:08:39Z

Synth1 works good, I only notice the shaking. Effect plugins (vst) seems not work.

BaraMGB · 2016-06-23T12:14:03Z

I'm on Ubuntu 14.04 now and have that graphic glitches too (on 16.04 I hadn't these glitches). If I click "show/hide Gui" twice everything works fine. But if I merge #2826 (which only changed QMdiSubindow to our Subwindow) it doesn't work anymore. I hoped this fixes the glitches on the title bar which I notice on Qt4 build.

tresf · 2016-07-05T15:31:13Z

but the new window causes glitches. Initially, the window may not be painted properly. When any window is moved, it may shake.

@jasp00 What do you think? Better to have a window than none at all or wait until this issue is sorted out?

tresf · 2016-07-06T05:37:35Z

Test results (screenshots provided below):

✅ Windows 10 x64: (Qt 5.4.2) Completely usable. Minor visual artifacts. Clicking title bar is the only way to bring plugin to foreground.
⚠️ Ubuntu 14.04 x64 (VM, Qt 5.2.1): Once VST window is displayed, all QMdiSubWindow instances become erratic. SubWindows can't be moved without some strange mouse behavior. This may be a VM-specific bug, but it only affects QT. The visual artifacts (sometimes falling behind other windows) is an annoyance, but the plugin is responsive and usable. @jasp00 @Umcaruje @mikobuntu can you test on a physical machine and confirm?
⛔️ Wine x64 (Ubuntu 14.04 x32, VM, Qt 5.4.2): Unusable. The visual artifacts are so bad that the plugin cannot be used 99% of the time.
⚠️ MacOS 10.11 x64: Out of scope. Can't test per VST plugins on OS X (Apple) #698.

Windows 10 x64 Native:

Ubuntu 14.04 x64 VM:

Wine x64 (in Ubuntu 14.04 32-bit):

jasp00 · 2016-07-06T20:18:44Z

On Linux, the visual artifacts are annoying.

Better to have a window than none at all or wait until this issue is sorted out?

I do not have a clear answer with the current proposal. Perhaps launching a helper application that embeds the VST GUI, out of MDI, would work around this.

tresf · 2016-07-06T20:22:02Z

I do not have a clear answer with the current proposal. Perhaps launching a helper application that embeds the VST GUI, out of MDI, would work around this.

Is there a way to launch it in a separate OS-decorated process?

Also

SubWindows can't be moved without some strange mouse behavior.

@jasp00 do you experience this or do you suspect it to be a VM-specific bug? Only Qt is affected, but once the VST window becomes active, I can't drag around any subwindows properly.

jasp00 · 2016-07-07T01:26:38Z

do you experience this or do you suspect it to be a VM-specific bug?

I experience it.

Is there a way to launch it in a separate OS-decorated process?

I was thinking of a helper application that used Qt/LMMS decorations, but using a commented function for RemoteVstPlugin would be another option. It would work like ZynAddSubFX.

tresf · 2016-08-31T13:48:34Z

FYI, without this patch, Windows seems to be OK in the latest tests: https://youtu.be/exQwD1Kjsmg

BaraMGB · 2016-09-02T14:32:38Z

I had some successes on put the Vst into a QDialog:

The most Instruments on my harddisk seems to work fine. But for example "Superwave P8" is not catched by the QDialog. I have to explore more on this.

jasp00 · 2016-09-24T16:26:35Z

To solve the Linux Qt5 program, I will use a separate window. To unify the implementation, Windows and Qt4 will use a separate window too.

Umcaruje · 2016-09-27T17:13:10Z

I really agree on separate VST windows, since this will allow you to put a, say large VST spectrum analyzer on a seperate screen, if you're running a dual or more monitor setup.

karmux · 2016-12-06T22:08:21Z

I tested this PR on Kubuntu 16.10 64-bit (Qt 5.6.1). Didn't notice any regression compared to Qt4 when when using various VST plugins. 👍

tresf · 2016-12-07T00:09:40Z

@karmux thanks for the testing. If your results are consistent with newer Qt versions, we should merge this PR, epecially considering it only applies to Qt > 5. Any objections?

jasp00 · 2016-12-07T04:04:26Z

I have an implementation that uses a separate window to avoid the detected glitches; I am sorry I cannot commit yet. Are you sure the glitches are gone with current Qt 5?

karmux · 2016-12-17T13:20:49Z

@jasp00 yes. Even in GTG DPC 3 (drum sampler on screenshots above) I couldn't see any artifacts.

jasp00 · 2016-12-26T02:28:35Z

The glitches are still there with Qt 5.7.1.

jasp00 · 2016-12-26T05:11:12Z

One glitch with Qt 5 when moving a separate window is that mouse events happen in the Qt application, and it may draw over the moving window. So there is no glitch-free method and the real solution would be to fix Qt. I will finish the separate window workaround: it is more complicated, but visually acceptable, and it does not leave a broken state when VST windows are gone.

jasp00 · 2016-12-27T13:50:36Z

I see that #2855 is the only bug that prevents the switch to Qt5 on Linux, and that the switch can be done on Windows. This PR could work with Qt4 on Linux, but may I implement Qt5 only? Qt5 implementation works, just needs cleaning and error checking. More important, to unify the behavior as much as possible, since there are no embedding functions with Qt4 on Windows, may I drop Qt4 support for Windows?

tresf · 2016-12-27T13:57:09Z

may I drop Qt4 support for Windows?

Sure so as long you're OK helping us revert it all if Qt5 exposes unexpected Windows issues in RC2.

jasp00 · 2016-12-27T14:20:16Z

Sure so as long you're OK helping us revert it all

There is the will, but no time. I will keep Qt4, write the implementation guidelines for Qt5, and delay unification until RC2 is tested.

karmux · 2017-01-12T17:57:12Z

On the first load of plugin, it's loaded separately from Qt window (there's empty Qt window and borderless VST window). Pressing show/hide GUI button twice loads it correctly. Using same setup as I wrote before.

jasp00 · 2017-01-20T10:27:33Z

Using same setup as I wrote before.

@karmux, GTG DPC 3 plug-in on Qt 5.6.1?

karmux · 2017-01-21T10:07:57Z

@jasp00 When VST is loaded in new project then it loads properly inside Qt's window. But when some project has VSTs saved then it loades like I described in my previous #2856 (comment). This applies to all VSTs.

zapashcanon · 2017-01-21T15:46:10Z

plugins/VstEffect/VstEffectControlDialog.h

@@ -62,6 +62,9 @@ class VstEffectControlDialog : public EffectControlDialog
 	VstPlugin * m_plugin;

 	QLabel * tbLabel;
+
+private slots:
+	void togglePluginUI( bool checked );


void togglePluginUI( const bool checked );

You should use const only when it is useful:

Pointers and references.

Read-only methods.

Constants.

As I said, only when it is useful, not when it is possible. Excessive use of const makes the code harder to read (this is subjective, but it is simpler). Without const, you can reuse parameters (simplicity) and you do not have to take const back if you need write access later (less typing and less thinking).

const bool and alike are useless in prototypes because values are copied. Do you see const in the dup2 prototype?

It used to be a good idea to add const to avoid if (x = 5) errors, but a good compiler with warnings will warn you. I will not ask you to remove const in a complex function body, but you should not ask others to use const unless there is a clear benefit.

zapashcanon · 2017-01-21T15:46:26Z

plugins/VstEffect/VstEffectControlDialog.cpp

+
+
+
+void VstEffectControlDialog::togglePluginUI( bool checked )


void VstEffectControlDialog::togglePluginUI( const bool checked )

zapashcanon · 2017-01-21T15:53:49Z

plugins/vst_base/CMakeLists.txt

@@ -3,7 +3,7 @@ IF(LMMS_SUPPORT_VST)
 INCLUDE(BuildPlugin)

 IF(LMMS_BUILD_WIN32)
-	ADD_DEFINITIONS(-DPTW32_STATIC_LIB)
+	ADD_DEFINITIONS(-DPTW32_STATIC_LIB -std=c++0x)


c++0x
The working draft of the upcoming ISO C ++ 0x standard. This option enables experimental features that are likely to be included in C ++ 0x. The working draft is constantly changing, and any feature that is enabled by this flag may be removed from future versions of GCC if it is not part of the C ++ 0x standard.

Your commit say you wanted to enable C++11, so, it's more like -std=c++11 I guess.

Or the compiler on win32 is old, and what's considered like experimental at the time is now C++11 ?

it's more like -std=c++11 I guess

Same effectiveness. c++0x is fine for now. We can switch it down the road as more compilers adopt the official standard. BTW, this specific change superseded by 607d3f4; Can be removed.

GCC supports -std=c++14 ; so I guess -std=c++0x includes C++14 and some features of C++17.

I guess -std=c++0x includes C++14 and some features of C++17.

Perhaps but that should be equally called C++1y, etc.

https://herbsutter.com/2011/08/12/we-have-an-international-standard-c0x-is-unanimously-approved/

I guess -std=c++0x includes C++14 and some features of C++17.

It does not.

zapashcanon · 2017-01-21T15:56:18Z

plugins/vst_base/RemoteVstPlugin.cpp

@@ -464,24 +478,103 @@ RemoteVstPlugin::~RemoteVstPlugin()



+#ifdef USE_LINUX_EMBEDDER
+static void checkExitStatus( int status )


static void checkExitStatus( const int status )

zapashcanon · 2017-01-21T15:58:05Z

plugins/vst_base/RemoteVstPlugin.cpp

@@ -619,9 +712,34 @@ void RemoteVstPlugin::init( const std::string & _plugin_file )



+#ifdef USE_LINUX_EMBEDDER
+static void assert_dup2( int oldfd, int newfd )


static void assert_dup2( const int oldfd, const int newfd )

zapashcanon · 2017-01-21T16:03:43Z

plugins/vst_base/embed-window.cpp

+
+void EmbedderApplication::readCommand()
+{
+	int c = getchar();


const int c = getchar();

zapashcanon · 2017-01-21T16:04:59Z

plugins/vst_base/embed-window.cpp

+	const char * title = argv[1];
+	unsigned int windowId = strtol( argv[2], NULL, 16 );
+	int width = strtol( argv[3], NULL, 16 );
+	int height = strtol( argv[4], NULL, 16 );


const unsigned int windowId = strtol( argv[2], NULL, 16 ); const int width = strtol( argv[3], NULL, 16 ); const int height = strtol( argv[4], NULL, 16 );

zapashcanon · 2017-01-21T16:05:24Z

plugins/vst_base/embed-window.cpp

+
+	QTimer::singleShot( 0, app, SLOT( applicationReady() ) );
+
+	int ret = app->exec();


const int ret = app->exec();

zapashcanon · 2017-01-21T16:06:41Z

plugins/vst_base/EmbedderApplication.h

+{
+public:
+	void init( const char * title, unsigned int windowId, int width,
+								int height );


void init( const char * title, const unsigned int windowId, const int width, const int height );

zapashcanon · 2017-01-21T16:07:11Z

plugins/vst_base/EmbedderApplication.h

+	virtual ~EmbedderApplication();
+
+	void init( const char * title, unsigned int windowId, int width,
+								int height );


void init( const char * title, const unsigned int windowId, const int width, const int height );

jasp00 · 2017-01-23T14:17:03Z

Unreproducible with Qt 5.7.1 and Wine 1.8.6. If I tamper with the window system, I get that behavior. @karmux, do you see any errors in the console? When you have the empty window, what is the output of ps -A -o args | grep embed-window?

karmux · 2017-03-29T20:26:43Z

@jasp00 I'd like to test this again with ouput of command you gave but can't compile this anymore.
I made a staging branch from latest master and merged my wine-devel PR and this PR into it.
Wine 2.4.0 and Qt 5.6.1 on Kubuntu 16.10.

Compilation error:

In file included from /usr/include/c++/6/cinttypes:35:0,
                 from /home/karmo/Dev/lmms/plugins/vst_base/RemoteVstPlugin.cpp:70:
/usr/include/c++/6/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options.
 #error This file requires compiler and library support \
  ^~~~~
/home/karmo/Dev/lmms/plugins/vst_base/RemoteVstPlugin.cpp: In member function ‘void RemoteVstPlugin::initEditor()’:
/home/karmo/Dev/lmms/plugins/vst_base/RemoteVstPlugin.cpp:844:25: error: expected ‘)’ before ‘PRIxPTR’
    sprintf( widStr, "%" PRIxPTR, m_windowID );
                         ^~~~~~~
/home/karmo/Dev/lmms/plugins/vst_base/RemoteVstPlugin.cpp:844:45: warning: spurious trailing ‘%’ in format [-Wformat=]
    sprintf( widStr, "%" PRIxPTR, m_windowID );
                                             ^
/home/karmo/Dev/lmms/plugins/vst_base/RemoteVstPlugin.cpp:844:45: warning: too many arguments for format [-Wformat-extra-args]
winegcc: g++ failed
plugins/vst_base/CMakeFiles/vstbase.dir/build.make:61: recipe for target 'plugins/RemoteVstPlugin' failed
make[2]: *** [plugins/RemoteVstPlugin] Error 2
CMakeFiles/Makefile2:9200: recipe for target 'plugins/vst_base/CMakeFiles/vstbase.dir/all' failed
make[1]: *** [plugins/vst_base/CMakeFiles/vstbase.dir/all] Error 2

wine-devel and qt5-embed merge result:

ADD_CUSTOM_COMMAND(
	SOURCE "${CMAKE_CURRENT_SOURCE_DIR}/RemoteVstPlugin.cpp"
	COMMAND ${WINE_CXX}
	ARGS "-I\"${CMAKE_BINARY_DIR}\""
		"-I\"${CMAKE_SOURCE_DIR}/include\""
		"-I\"${CMAKE_INSTALL_PREFIX}/include/wine/windows\""
		"-I\"${CMAKE_INSTALL_PREFIX}/include\""
		"-I\"${WINE_INCLUDE_DIR_REPLACED}\""
		"-L\"${WINE_LIBRARY_REPLACED}\""
		"\"${CMAKE_CURRENT_SOURCE_DIR}/RemoteVstPlugin.cpp\""
		-std=c++11 -ansi -mwindows -lpthread ${EXTRA_FLAGS} -fno-omit-frame-pointer
		${WINE_BUILD_FLAGS}
		-o ../RemoteVstPlugin
	COMMAND sh -c "mv ../RemoteVstPlugin.exe ../RemoteVstPlugin || true"
	TARGET vstbase
	OUTPUTS ../RemoteVstPlugin
)

karmux · 2017-04-01T13:29:56Z

It compiles now after your new commits but VST instruments and effects are separate from Qt window. Clicking show/hide doesn't fix it anymore.
Wine 2.4.0 and Qt 5.6.1 on Kubuntu 16.10.

jasp00 · 2017-04-01T15:47:43Z

@karmux, could you explain the screenshot? In the taskbar on the left, what application does the white window icon represent? The one between the selected VeSTige logo and the LMMS logo.

Could you try running Wine 2.0?

karmux · 2017-04-01T16:43:25Z

@jasp00, white icon is actual VST instrument window on the left and red/green VST icon is blank Qt window on the right.

jasp00 · 2017-04-01T18:33:05Z

Wine 2.0 works. @karmux, according to #2856 (comment) and #3295 (comment), it looks like your Qt 5 environment has trouble embedding native windows. Could you try 1ded76b and post the debug information?

lukas-w · 2017-04-01T18:40:31Z

@jasp00 That's an issue with certain window managers, as I have stated in #3295 (comment). I've been able to reproduce it with both PRs on Plasma and i3, bot not on Xfce for example.

I already debugged this issue in #3295 with lines similar to those in 1ded76b, which showed the correct window ids with Qt 5.6.1, Plasma desktop, and Wine 2.4. I expect @karmux will get the same results. I could work around it by sleeping (QThread::sleep(1)) before calling createWindowContainer, so there seems to be some sort of synchronization issue, i.e. createWindowContainer is probably being called before the window has been mapped.

karmux · 2017-04-01T20:52:04Z

// Here I opened SuperRiff Bass

kf5.kio.core: Refilling KProtocolInfoFactory cache in the hope to find ""
Error loading icon pixmap "vestige/track_op_menu_active": File not found
Error loading icon pixmap "vestige/track_op_menu": File not found
Error loading icon pixmap "vestige/project_open": File not found
Error loading icon pixmap "vestige/project_open": File not found
Error loading icon pixmap "vestige/stepper-left-press": File not found
Error loading icon pixmap "vestige/stepper-left": File not found
Error loading icon pixmap "vestige/project_save": File not found
Error loading icon pixmap "vestige/project_save": File not found
Error loading icon pixmap "vestige/stepper-right-press": File not found
Error loading icon pixmap "vestige/stepper-right": File not found
Error loading icon pixmap "vestige/stepper-down": File not found
Error loading icon pixmap "state_stop": File not found
kf5.kio.core: Refilling KProtocolInfoFactory cache in the hope to find ""
unique ID: 1999
RemotePlugin::DebugMessage: inputs: 0  output: 2
RemotePlugin::DebugMessage: creating editor
m_windowID 9200001
RemotePlugin::DebugMessage: editor successfully created
m_pluginWindowID 9200001
window 0x5571be8db120
RemotePlugin::DebugMessage: failed getting shared memory

// Here I opened massTURBOTar

kf5.kio.core: Refilling KProtocolInfoFactory cache in the hope to find ""
QProcess: Destroyed while process ("/home/karmo/Dev/lmms/build/plugins/RemoteVstPlugin") is still running.
unique ID: mtub
RemotePlugin::DebugMessage: inputs: 0  output: 2
RemotePlugin::DebugMessage: creating editor
m_windowID 9000001
RemotePlugin::DebugMessage: editor successfully created
m_pluginWindowID 9000001
window 0x5571bdb5c940
RemotePlugin::DebugMessage: failed getting shared memory

// Here I opened GTG DPC 3

kf5.kio.core: Refilling KProtocolInfoFactory cache in the hope to find ""
QProcess: Destroyed while process ("/home/karmo/Dev/lmms/build/plugins/RemoteVstPlugin") is still running.
unique ID: dpc3
RemotePlugin::DebugMessage: inputs: 0  output: 2
RemotePlugin::DebugMessage: creating editor
m_windowID 9000001
RemotePlugin::DebugMessage: editor successfully created
m_pluginWindowID 9000001
window 0x5571bdb80820
RemotePlugin::DebugMessage: failed getting shared memory

Interestingly GTG DPC 3 once opened correctly but I couldn't reproduce it.

jasp00 · 2017-04-02T06:24:35Z

Because embedding native windows on Qt 5 is not reliable on some systems, I propose to let Wine handle the window interface. There is an issue with this procedure:

User closes the window to hide it.
User wants to show the window again.
Window manager may not bring the window to front.

This may happen because Wine has already accepted the close event. This solution affects usability, but it is the best compromise.

jasp00 · 2017-04-10T19:37:24Z

Continued at #3497.

jasp00 force-pushed the qt5-embed branch 2 times, most recently from ddcc57f to e20b648 Compare January 3, 2017 10:37

zapashcanon suggested changes Jan 21, 2017

View reviewed changes

jasp00 mentioned this pull request Jan 26, 2017

Add Qt5 Linux VST subwindow implementation #3295

Closed

jasp00 force-pushed the qt5-embed branch from e0064c2 to f71e14c Compare March 31, 2017 23:15

Javier Serrano Polo added 7 commits April 1, 2017 14:24

Embed VST GUI in subwindow

8c7ddd6

Embed in a separate window

f37d821

Fixes for Windows builds

daf29ca

Enable C++11

20c7974

Enable C++11 on Windows

ef4fc1f

C++11 is enabled by default now

f533246

Use window instead of embedder application

a6400c9

jasp00 force-pushed the qt5-embed branch from 70a4601 to a6400c9 Compare April 1, 2017 12:29

Debug window ID

1ded76b

lukas-w mentioned this pull request Apr 1, 2017

Add X11EmbedContainer Qt5 port #3475

Closed

jasp00 mentioned this pull request Apr 10, 2017

Support VST windows on Qt5 #3497

Closed

jasp00 closed this Apr 10, 2017

jasp00 deleted the qt5-embed branch April 10, 2017 19:38

		@@ -464,24 +478,103 @@ RemoteVstPlugin::~RemoteVstPlugin()



		#ifdef USE_LINUX_EMBEDDER
		static void checkExitStatus( int status )

		@@ -619,9 +712,34 @@ void RemoteVstPlugin::init( const std::string & _plugin_file )



		#ifdef USE_LINUX_EMBEDDER
		static void assert_dup2( int oldfd, int newfd )


		QTimer::singleShot( 0, app, SLOT( applicationReady() ) );

		int ret = app->exec();

Embed VST GUI in subwindow #2856

Embed VST GUI in subwindow #2856

Conversation

jasp00 commented Jun 17, 2016

BaraMGB commented Jun 17, 2016

BaraMGB commented Jun 23, 2016

tresf commented Jul 5, 2016 • edited Loading

tresf commented Jul 6, 2016 • edited Loading

jasp00 commented Jul 6, 2016

tresf commented Jul 6, 2016

jasp00 commented Jul 7, 2016

tresf commented Aug 31, 2016 • edited Loading

BaraMGB commented Sep 2, 2016

jasp00 commented Sep 24, 2016

Umcaruje commented Sep 27, 2016

karmux commented Dec 6, 2016

tresf commented Dec 7, 2016

jasp00 commented Dec 7, 2016

karmux commented Dec 17, 2016

jasp00 commented Dec 26, 2016

jasp00 commented Dec 26, 2016

jasp00 commented Dec 27, 2016

tresf commented Dec 27, 2016

jasp00 commented Dec 27, 2016

karmux commented Jan 12, 2017

jasp00 commented Jan 20, 2017

karmux commented Jan 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tresf Jan 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tresf Jan 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasp00 commented Jan 23, 2017

karmux commented Mar 29, 2017 • edited Loading

karmux commented Apr 1, 2017

jasp00 commented Apr 1, 2017

karmux commented Apr 1, 2017

jasp00 commented Apr 1, 2017

lukas-w commented Apr 1, 2017

karmux commented Apr 1, 2017

jasp00 commented Apr 2, 2017

jasp00 commented Apr 10, 2017

tresf commented Jul 5, 2016 •

edited

Loading

tresf commented Jul 6, 2016 •

edited

Loading

tresf commented Aug 31, 2016 •

edited

Loading

karmux commented Jan 21, 2017 •

edited

Loading

tresf Jan 21, 2017 •

edited

Loading

tresf Jan 21, 2017 •

edited

Loading

karmux commented Mar 29, 2017 •

edited

Loading