Implement Cocoa event loop in Scsynth/Supernova #4499

Spacechild1 · 2019-07-23T00:58:14Z

Purpose and Motivation

Add the possibility to run a Cocoa event loop on Scsynth's/Supernova's main thread so we can show native GUI windows on the Server. ¹ Currently, the main thread just waits for a semaphore, so it might just as well handle GUI events instead. This is very useful for projects like https://git.iem.at/pd/vstplugin. Note that we only need this for macOS because the event loop must run on the main thread (which I don't control). On Windows and Linux, the event loop can run on any thread and I can therefore create my own event loop.*)

Types of changes

cross platform EventLoop class (does nothing on Windows and Linux so far)
Cocoa event loop functions implemented in SC_Apple.hpp/.mm
called in scsynth_main.cpp resp. supernova/server/main.cpp (I didn't need to touch any other files!)
~~"-e" command line option to enable the event loop (disabled by default)~~
~~"eventLoop" member in ServerOption class (default: false)~~

The idea is that plugin clients can simply execute methods on the main thread with dispatch_sync/dispatch_async, e.g. to create/destroy windows https://developer.apple.com/documentation/dispatch/1453057-dispatch_async?language=objc

The reason for the "-e" argument is that with the Cocoa event loop, Scsynth will show up as an app in the taskbar and people might find this annoying. Probably "-e" should have an argument (0/1), so we can later change the default value...

EDIT: I've removed the "-e" argument because I've decided to always run the Cocoa event loop and not add an icon to the taskbar (I rather do this in VSTPlugin after the fact).

Disclaimer: I'm not a Cocoa expert my any means!

Testing

I've tested this with the latest develop branch of VSTPlugin (https://git.iem.at/pd/vstplugin/tree/develop) and the VST plugin GUI works fine on both Scsynth and Supernova.

Here's a pre-compiled binary for macOS: VSTPlugin.zip

And here's a test .scd file: VSTPlugin_editor_cocoa.scd.zip

Notes

The addition of the event loop shouldn't be noticable by the user. Also, there shouldn't be any impact on performance because the event loop would just sit idle (like the main thread already does), but this has to be tested!

I would like to see this included in the next SuperCollider release as a "private" feature which can be tested in the wild with the next version of VSTPlugin. If this turns out to work well, we can think about adding a Windows and Linux implementation and exposing the event loop via the plugin API.
(Windows and Linux don't have the concept of a "main thread", i.e. there is no equivalent to Cocoa's dispatch_get_main_queue, so at the very least we need method to return a handle to the GUI thread. We could also add methods for passing messages between GUI thread and RT thread, similar to SendMsgToEngine/SendMsgFromEngine).

Finally, note that this PR is not specifically about VSTPlugin! It allows any plugin to create native windows for all kinds of things: video playback, visualization, complex GUIs, etc.

Code is tested
This PR is ready for review

scztt · 2019-07-24T07:35:56Z

I think there's definitely some more review and analysis needed for this PR, but at a glance I think it's doing roughly the correct thing. My immediate concerns for this would be:

Does making scsynth an NSApplication / have an event loop have any important effects? Performance / threading concerns? Priority? Does it have an icon in the dock? Will it process events that we don't want it to receive?
Will we be able to share most of this implementation with VST3? VST2 is very deprecated (IIRC Steinberg is pulling the SDK and not allowing licenses, if not now then any day) - we should at least make sure that we e.g. won't have to add ANOTHER new flag and set of different event loop hacks to support VST3 in the future.
Testing VST GUI's is laborious - what is our plan to try to get good test coverage before we merge this? This may be a simple change now, but it could get much more complex as we work around real-world plugin behavior. It would be nice to know that we had a good range of VST's smoke tested before we merged this and users start to expect it to work.
It's reasonable to have separate PR's for separate platforms, but it would be good to have at least a WIP for windows VST GUI before this was merged, lest we get stuck with a one-platform implementation (unless I'm totally wrong and Windows GUI is already working? If so, that would be wonderful - I haven't tested it!)

I can give a closer code review on this soon, and happy to help brainstorm / discuss the issues I mentioned above.

scztt · 2019-07-24T08:06:12Z

One other code comment: I would somewhat prefer a solution where the #ifdef __APPLE__'s were removed and instead we had a mostly macro-free cpp file, with platform-specific includes and a no-op case for unsupported platforms. E.g.:

        case 'e':
            checkNumArgs(1);
            eventLoop = IsApple ? true : false;
            break;

// EventLoop_Mac.h
struct EventLoop {
    static void startRunLoop(function<void()> f) {
      auto thread = SC_Thread([&]() {
            f();
            SC::Apple::EventLoop::quit();
        });
        thread.detach();
        SC::Apple::EventLoop::run();
    }
}

// EventLoop_Win.h
struct EventLoop { 
    static void startRunLoop(function<void()> f) { f(); } // or assert, if we want to ensure this is unreachable
}

// ...
EventLoop::startRunLoop([&](){ World_WaitForQuit(world, true); });

Not sure if others have preferences here? In terms of code readability and maintainability, I find this kind of separation much better, even if it means sometimes you need to jump around to find out what's going on.

This could also be done simply as a run-loop function in run() for each platform. It's super reasonable that each platform would have different details in their run loop.

Spacechild1 · 2019-07-24T12:02:21Z

Performance / threading concerns? Priority?

I don't think so. The main thread is already there, it just sleeps. Note that a GUI event loop also sleeps unless it receives events.

Does it have an icon in the dock?

Yes, that's actual reason why I made it optional because it might annoy people. I've updated the PR decription.

Will it process events that we don't want it to receive?

The event loop doesn't care about the events, it simply pops them from the queue and dispatches them. Plugins are supposed to create a window and set the window delegate which handles the input events. Furthermore, it can call methods on the window and send events. But the event loop itself doesn't know anything about it.

* Will we be able to share most of this implementation with VST3? VST2

Actually, this PR is not about VST. It simply allows any SuperCollider plugin to use the Cocoa event loop (e.g. to create native Windows). Again, the event loop is agnostic about the implementation of each Window.

* Testing VST GUI's is laborious - what is our plan to try to get good test coverage before we merge this?

Again, this PR is not specifically about VST and the event loop itself is "dumb". Any problems with VSTPlugin should go to the VSTPlugin bug tracker and will be handled by me.

* It's reasonable to have separate PR's for separate platforms, but it would be good to have at least a WIP for windows VST GUI before this was merged, lest we get stuck with a one-platform implementation (unless I'm totally wrong and Windows GUI is already working? If so, that would be wonderful - I haven't tested it!)

Yes, the Windows and Linux GUI is already working! Personally, I don't need the Windows and Linux equivalent of this PR because on those platforms the event loop can be created on any thread - by me! (Actually, there can even be more than one event loop! So Cocoa sucks...)

On the other hand it could be nice to add a Windows and Linux event loop, so that future plugins don't have to create their own event loops. But it would need an API extension because Windows and Linux don't have the concept of a "main thread", i.e. there is no equivalent to dispatch_get_main_queue. This means, plugins need a way to get a handle to the GUI thread. It could be void * member in World or a plugin API function. Anyway, we would have to increment the plugin API version.

I would really love to see the Cocoa event loop merged rather sooner than later. I would consider it an "experimental" and "private" feature because I'll be the only one using it, anyway... After a while of testing it in the real world with VSTPlugin we can think about exposing it via the plugin API as described above.

Spacechild1 · 2019-07-24T12:24:14Z

One other code comment: I would somewhat prefer a solution where the #ifdef __APPLE__'s were removed and instead we had a mostly macro-free cpp file, with platform-specific includes and a no-op case for unsupported platforms.

I agree that dummy implementations for Windows and Linux would be better and I think your examples looks quite nice. I'll go ahead and add this but I think we only need a single SC_EventLoop.hpp header with a couple of #ifdefs. We already have files for platform specific code.

Spacechild1 · 2019-07-24T12:37:16Z

BTW, I'm still looking for a way to hide the Scsynth icon from the docker, in which case we might not need the "-e" flag because the users wouldn't notice anything. On the other hand, it might be still a good idea to keep the event loop optional. Dunno... You tell me!

muellmusik · 2019-07-24T13:00:03Z

Actually, this PR is not about VST. It simply allows any SuperCollider plugin to use the Cocoa event loop (e.g. to create native Windows). Again, the event loop is agnostic about the implementation of each Window.

So this could, for instance, be used to support video playback in scsynth?

BTW, I'm still looking for a way to hide the Scsynth icon from the docker, in which case we might not need the "-m" flag because the users wouldn't notice anything. On the other hand, it might be still a good idea to keep the event loop optional. Dunno... You tell me!

If it's a GUI app, it should have a dock icon I think. This is important as it provides a conventional way to front the app etc. I think best not to contradict that. That said, at one point scsynth did have an icon, and some people found that annoying in the absence of a GUI. So the command line might be a good compromise.

Spacechild1 · 2019-07-24T13:11:59Z

@muellmusik

So this could, for instance, be used to support video playback in scsynth?

totally!

If it's a GUI app, it should have a dock icon I think. This is important as it provides a conventional way to front the app etc. I think best not to contradict that.

I agree! For example, when you click on the icon, all its windows will move to the foreground. IIRC this was the reason why I added the icon in the first place in the original Pd version, I just forgot... (I must remember to document this!)

That said, at one point scsynth did have an icon, and some people found that annoying in the absence of a GUI. So the command line might be a good compromise.

Thanks for this insight! The question is: should the new flag only be about showing the icon (with the event loop running either way)? Or should it be about running the event loop + showing the icon?

I guess the option of not introducing any flag is out of question (because it would imply not showing the icon).

Spacechild1 · 2019-07-24T13:22:31Z

Another option: VSTPlugin performs the foreground application transformation the first time a GUI window is requested.

Later this can be done automatically in the new API methods (e.g. whenever the GUI thread handle is requested for the first time by a plugin).

Then the only question would be if the event loop should always run or only if requested with the "-e" option... I think there is no performance hit with running an idle event loop but of course I'm not 100% sure.

muellmusik · 2019-07-24T13:58:13Z

totally!

Great! That would also be an exciting feature!

Thanks for this insight! The question is: should the new flag only be about showing the icon (with the event loop running either way)? Or should it be about running the event loop + showing the icon?

Well, as I said, IMO whenever it is a GUI app it should show the dock icon.

Another option: VSTPlugin performs the foreground application transformation the first time a GUI window is requested.

Later this can be done automatically in the new API methods (e.g. whenever the GUI thread handle is requested for the first time by a plugin).

So the icon would appear the first time you created a GUI? That could be nice, although perhaps slightly unusual in behaviour. Personally I never minded scsynth having a dock icon, but others found it odd. If in the longer run GUI elements became more common there might be less objection.

Then the only question would be if the event loop should always run or only if requested with the "-e" option... I think there is no performance hit with running an idle event loop but of course I'm not 100% sure.

Well, if there's concern about another command line arg being bloat, and that could avoid it, that's a plus I'd guess. Also nice for users not to have to remember to launch it in GUI mode. I suspect also that the load is insignificant, but would be good to know. A compromise might be to boot it in GUI mode by default, so you only need to change an arg if you don't want the dock icon.

Spacechild1 · 2019-07-24T14:33:44Z

So the icon would appear the first time you created a GUI? That could be nice, although perhaps slightly unusual in behaviour.

FWIW, I've just tested this (VSTPlugin performing the transformation after the fact) and it works!

Spacechild1 · 2019-07-24T14:34:34Z

So the icon would appear the first time you created a GUI? That could be nice, although perhaps slightly unusual in behaviour.

Another option:

We can add another flag for DefineUnit, e.g.

enum { 
    kUnitDef_CantAliasInputsToOutputs = 1,
    kUnitDef_NeedGui = 2
 };

and scsynth does the transformation the first time it finds a plugin which sets this flag.
Until then, VSTPlugin could do the transformation in its setup function (instead of "on demand").

What is better? I'm not a Mac user, so I don't have an opinion...

muellmusik · 2019-07-24T14:53:44Z

What is better? I'm not a Mac user, so I don't have an opinion...

I don't have a strong preference. People will definitely want to run 'GUI-less' in some contexts though, so that should be made possible on some level. (Could be a compile option.)

telephon · 2019-07-24T16:30:11Z

People will definitely want to run 'GUI-less' in some contexts though, so that should be made possible on some level. (Could be a compile option.)

I don't think it should be a compile option, but a server option. E.g. when you do network music and you start 5 servers you don't want 5 icons popping up, do you? And you don't want to use a different program for that I think.

Spacechild1 · 2019-07-25T01:21:12Z

OK, I've decided to remove the "-e" flag and always run the Cocao event loop. This way the feature becomes 100% private. Mac users are invited to run their existing projects with this patch and observe any differences in performance. I think there should be no impact at all because the event loop just waits for a condition - like the current main thread already does.

Also, I don't do the foreground application transformation on the Server, I rather do it in VSTPlugin the first time a GUI editor is requested. This means that scsynth only gets an icon in the taskbar if there is at least one GUI window.

My suggestion would be to test this for a while with VSTPlugin and if it works out well, I can offer to add implementations for Windows and Linux and extend the plugin API, so the feature can be easily used by other plugins.

I've updated the PR description.

geoffroymontel · 2019-08-06T10:08:43Z

Hello @Spacechild1 ! Is your PR still WIP or Ready for Review ?

Spacechild1 · 2019-08-06T11:18:43Z

Hey, I've removed the [WIP] label, but I guess this PR still needs discussion. Personally, I think it could be merged as an experimental "private" feature which shouldn't affect normal operation.

I've added some testing instructions to the PR description, give it a try!

geoffroymontel · 2019-08-12T08:49:55Z

Hi !
Exciting feature !
I'm running macOS 10.12.6 with built-in soundcard

I've just tried this branch with the pre-compiled binary for macOS and I can't get the UI to open with Dexed VSTi

Here is a reproducer :

s.boot;
VSTPlugin.search;

returns

searching in '/Users/geoffroy/Library/Audio/Plug-Ins/VST'...
found 0 plugins
searching in '/Library/Audio/Plug-Ins/VST'...
/Library/Audio/Plug-Ins/VST/Dexed.vst
etc.

then

(
SynthDef(\dexed, { |out = 0|
	Out.ar(out, VSTPlugin.ar(nil, 2, id: \dexed));
}).add;
)

~synth = Synth(\dexed);
~dexed = VSTPluginController.new(~synth, \dexed);
~dexed.open("Dexed", editor: true);
~dexed.gui;

I can see the scsynth icon but the generic Qt GUI opens.
Did I miss something ?
Best !

Spacechild1 · 2019-08-12T12:10:59Z

~dexed.editor; ;-)

geoffroymontel · 2019-08-12T13:51:45Z

Ah ah :) sorry ! It's working like a charm ! Thanks a lot Christof for that, I hope it will make more people consider SC as a host !!

Spacechild1 · 2019-08-12T14:04:06Z

Cool! Now I'd need to know if macOS users notice any sort of noticable performance regression when running their existing projects (which don't use VSTPlugin) on this branch. I think there shouldn't be any difference, but I would like to get some feedback.

pikseliahky · 2019-10-19T21:59:08Z

Hi,
The VST seems to load OK but then I am getting this error: "EventLoop::destroy: can't dispatch to main thread - no NSApp!" when running the example on Mac OS 10.12.6 and SC 3.10.3. Any idea what could be wrong?

Spacechild1 · 2019-10-19T22:47:56Z

@pikseliahky

SC 3.10.3

you have to build and run this branch of SuperCollider.

Spacechild1 · 2019-10-19T23:54:35Z

@geoffroymontel would you mind sharing your macOS build here?

geoffroymontel · 2019-10-22T11:00:39Z

Here is a build for MacOS 10.14 but it's only available for 7 days : https://we.tl/t-FoWaw2s0yq

pikseliahky · 2019-10-28T09:08:39Z

Thank you. I'm on 10.12 but got it finally working!

Spacechild1 · 2019-10-28T09:28:44Z

@pikseliahky cool, so the VST GUI works for you as well?

mossheim

this all looks good to me, and seems to work fine once I rebase on latest develop. i am not an expert on cocoa either but from what i can tell this is a good way to implement it. i appreciate you taking the time to make it generic this way!

common/SC_EventLoop.hpp

server/scsynth/scsynth_main.cpp

common/SC_Apple.mm

mossheim · 2019-12-03T03:00:49Z

i did not notice any regression in performance in terms of CPU usage.

Spacechild1 · 2019-12-03T09:16:24Z

@brianlheim Thanks a lot for reviewing and testing! I'll apply the requested changes today when I get back home.

I think this PR could be a good first step and would make a lot of Mac users happy :-). Later I want to add a Win32 and X11 event loop (so plugin authors don't have to create their own) and add plugin API methods to dispatch method calls between the UI thread and NRT thread. But before that I want to write a RFC to improve the plugin API to make it extensible while keeping backwards compatibility.

Spacechild1 · 2019-12-06T00:54:30Z

@brianlheim Hey, I finally got some time to do the changes! I've tested the new code on my MacBook.

…ive GUI windows * added "-e" command line option to enable the event loop (disabled by default) * added "eventLoop" member to sclangs's ServerOption class * didn't have to touch WorldOptions!

…mented yet)

We don't transform the process into a foreground application on the Server!

Spacechild1 · 2019-12-07T00:52:24Z

Adding the event loop to Supernova was trivial and I've tested it sucessfully.

common/SC_Apple.mm

common/SC_EventLoop.hpp

mossheim

tested both supernova and scsynth in a release build, no performance regression as far as CPU usage goes, thanks for the work on this, especially the immaculately written comments!

Spacechild1 · 2019-12-07T20:35:31Z

Thanks :-)

Spacechild1 changed the title ~~[WIP] implement Cocoa event loop in Scsynth~~ Implement Cocoa event loop in Scsynth Aug 6, 2019

joshpar self-requested a review September 29, 2019 19:45

mossheim suggested changes Dec 3, 2019

View reviewed changes

Spacechild1 force-pushed the cocoarunloop branch 2 times, most recently from caa4b4b to 1247ee5 Compare December 6, 2019 00:36

Christof Ressi and others added 9 commits December 6, 2019 12:49

implement Cocoa event loop and run it in scsynth_main.cpp to show nat…

d47688d

…ive GUI windows * added "-e" command line option to enable the event loop (disabled by default) * added "eventLoop" member to sclangs's ServerOption class * didn't have to touch WorldOptions!

fix formatting

4b6300f

more selective linker flag (also for to Supernova, although not imple…

2c4a639

…mented yet)

apply hideous formatting to keep the linter quiet...

9a6bad4

Remove "-e" argument and always run Cocoa event loop

e3e4011

We don't transform the process into a foreground application on the Server!

add cross-platform EventLoop class + more documentation

160ffa7

the linter has struck again...

5b4e101

use explicit lambda captures + small cleanups

e65205f

exit event loop instead of terminating the app

64f1c73

Spacechild1 force-pushed the cocoarunloop branch from 1247ee5 to e3dd8a5 Compare December 7, 2019 00:43

Spacechild1 changed the title ~~Implement Cocoa event loop in Scsynth~~ Implement Cocoa event loop in Scsynth/Supernova Dec 7, 2019

Spacechild1 force-pushed the cocoarunloop branch from e3dd8a5 to 9f167f0 Compare December 7, 2019 00:53

add Cocoa event loop to Supernova

9f167f0

mossheim reviewed Dec 7, 2019

View reviewed changes

common/SC_Apple.mm Outdated Show resolved Hide resolved

mossheim reviewed Dec 7, 2019

View reviewed changes

common/SC_EventLoop.hpp Show resolved Hide resolved

rename bRunning to g_running and make it static

73e3d7c

mossheim approved these changes Dec 7, 2019

View reviewed changes

joshpar approved these changes Dec 8, 2019

View reviewed changes

mossheim merged commit 50c14aa into supercollider:develop Dec 8, 2019

mossheim mentioned this pull request Jun 3, 2020

scsynth: linker error due to undefined reference to stopServerBootDelayWarningTimer() #4992

Closed

Spacechild1 mentioned this pull request Aug 11, 2020

add (optional) Cocoa event loop on macOS pure-data/pure-data#1117

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Cocoa event loop in Scsynth/Supernova #4499

Implement Cocoa event loop in Scsynth/Supernova #4499

Spacechild1 commented Jul 23, 2019 •

edited

Loading

scztt commented Jul 24, 2019

scztt commented Jul 24, 2019 •

edited

Loading

Spacechild1 commented Jul 24, 2019 •

edited

Loading

Spacechild1 commented Jul 24, 2019 •

edited

Loading

Spacechild1 commented Jul 24, 2019 •

edited

Loading

muellmusik commented Jul 24, 2019

Spacechild1 commented Jul 24, 2019 •

edited

Loading

Spacechild1 commented Jul 24, 2019

muellmusik commented Jul 24, 2019

Spacechild1 commented Jul 24, 2019

Spacechild1 commented Jul 24, 2019

muellmusik commented Jul 24, 2019

telephon commented Jul 24, 2019

Spacechild1 commented Jul 25, 2019 •

edited

Loading

geoffroymontel commented Aug 6, 2019

Spacechild1 commented Aug 6, 2019

geoffroymontel commented Aug 12, 2019

Spacechild1 commented Aug 12, 2019 •

edited

Loading

geoffroymontel commented Aug 12, 2019

Spacechild1 commented Aug 12, 2019 •

edited

Loading

pikseliahky commented Oct 19, 2019

Spacechild1 commented Oct 19, 2019

Spacechild1 commented Oct 19, 2019

geoffroymontel commented Oct 22, 2019

pikseliahky commented Oct 28, 2019

Spacechild1 commented Oct 28, 2019

mossheim left a comment

mossheim commented Dec 3, 2019

Spacechild1 commented Dec 3, 2019

Spacechild1 commented Dec 6, 2019 •

edited

Loading

Spacechild1 commented Dec 7, 2019

mossheim left a comment •

edited

Loading

Spacechild1 commented Dec 7, 2019

Implement Cocoa event loop in Scsynth/Supernova #4499

Implement Cocoa event loop in Scsynth/Supernova #4499

Conversation

Spacechild1 commented Jul 23, 2019 • edited Loading

Purpose and Motivation

Types of changes

Testing

Notes

scztt commented Jul 24, 2019

scztt commented Jul 24, 2019 • edited Loading

Spacechild1 commented Jul 24, 2019 • edited Loading

Spacechild1 commented Jul 24, 2019 • edited Loading

Spacechild1 commented Jul 24, 2019 • edited Loading

muellmusik commented Jul 24, 2019

Spacechild1 commented Jul 24, 2019 • edited Loading

Spacechild1 commented Jul 24, 2019

muellmusik commented Jul 24, 2019

Spacechild1 commented Jul 24, 2019

Spacechild1 commented Jul 24, 2019

muellmusik commented Jul 24, 2019

telephon commented Jul 24, 2019

Spacechild1 commented Jul 25, 2019 • edited Loading

geoffroymontel commented Aug 6, 2019

Spacechild1 commented Aug 6, 2019

geoffroymontel commented Aug 12, 2019

Spacechild1 commented Aug 12, 2019 • edited Loading

geoffroymontel commented Aug 12, 2019

Spacechild1 commented Aug 12, 2019 • edited Loading

pikseliahky commented Oct 19, 2019

Spacechild1 commented Oct 19, 2019

Spacechild1 commented Oct 19, 2019

geoffroymontel commented Oct 22, 2019

pikseliahky commented Oct 28, 2019

Spacechild1 commented Oct 28, 2019

mossheim left a comment

Choose a reason for hiding this comment

mossheim commented Dec 3, 2019

Spacechild1 commented Dec 3, 2019

Spacechild1 commented Dec 6, 2019 • edited Loading

Spacechild1 commented Dec 7, 2019

mossheim left a comment • edited Loading

Choose a reason for hiding this comment

Spacechild1 commented Dec 7, 2019

Spacechild1 commented Jul 23, 2019 •

edited

Loading

scztt commented Jul 24, 2019 •

edited

Loading

Spacechild1 commented Jul 24, 2019 •

edited

Loading

Spacechild1 commented Jul 24, 2019 •

edited

Loading

Spacechild1 commented Jul 24, 2019 •

edited

Loading

Spacechild1 commented Jul 24, 2019 •

edited

Loading

Spacechild1 commented Jul 25, 2019 •

edited

Loading

Spacechild1 commented Aug 12, 2019 •

edited

Loading

Spacechild1 commented Aug 12, 2019 •

edited

Loading

Spacechild1 commented Dec 6, 2019 •

edited

Loading

mossheim left a comment •

edited

Loading