search

DISTRHO / Nekobi

DISTRHO Nekobi
GNU General Public License v2.0
49 stars 12 forks source link

Enable choosing which plugins to build #4

Closed simonvanderveldt closed 7 years ago

simonvanderveldt commented 7 years ago

The Makefile(s) for Nekobi didn't allow one to choose which plugins to build. This PR adds separate BUILD_ variables that allow one to choose which plugins to build. They all default to true, so nothing has changed for users who don't care about this. I'd like to do the same for the standalone binary build, but it seems like that would need a couple more changes.

I ran into a failure case when building the LV2 plugin without X11/DGL, it would still signal to a plugin host that a GUI was available which then wouldn't work caused by the modgui files being present. So I added an additional check if DGL/X11 is present before copying the modgui files. Note that the Nekobi.lv2/manifest.ttl still contains a reference to modgui.ttl. I haven't found a way to fix that, it seems to be created by the generate-ttl.sh script from dpf.

simonvanderveldt commented 7 years ago

Hmm, strange thing just happened, whereas before both Carla as well as jalv.gtk would run Nekobi as LV2 plugin just fine without the UI files present both of them are now crashing when trying to load Nekobi. I don't know what's different that I'm seeing this different behaviour now.

Anyway, once I remove the reference to modgui.ttl from Nekobi.lv2/manifest.ttl it works again in both with the generic UI. I haven't been able to determine why this is happening. Is there any way to not automatically add the modgui reference to the manifest?

[edit] Found it, it's caused by this line https://github.com/DISTRHO/Nekobi/blob/1f960011aeddc66454418c6f40cb9efa1163f3c1/plugins/Nekobi/DistrhoPluginInfo.h#L25 that sets DISTRHO_PLUGIN_HAS_UI. This header file isn't actually used by this project (and thus not part of this project's Makefile) but it's imported by DPF I think I'll go for a preprocessor option for now to disable it.

falkTX commented 7 years ago

the modgui is not an actual desktop UI, and doesn't need OpenGL. it will be ignored by all current desktop hosts (except carla). there is no real reason to skip installing the modgui files, besides saving disk space.

simonvanderveldt commented 7 years ago

the modgui is not an actual desktop UI, and doesn't need OpenGL. it will be ignored by all current desktop hosts (except carla). there is no real reason to skip installing the modgui files, besides saving disk space.

I understand. But they aren't needed when building without the UI, correct?

I've got it working though :) Had to make the DISTRHO_PLUGIN_HAS_UI and DISTRHO_PLUGIN_USES_MODGUI #defines in DisthroPluginInfo.h conditional . See latest commit. Now one can use Nekobi with the generic UI in Carla and jalv.

falkTX commented 7 years ago

they are needed for MOD stuff, so modguis will always be installed. you can always use the generic UI in carla, whatever the plugin has a native UI or not.

simonvanderveldt commented 7 years ago

you can always use the generic UI in carla, whatever the plugin has a native UI or not.

But that only works if you build everything, right? That's why I ran into issues when I did a build without the UI stuff (HAVE_DGL=false) Now it actually works for both the case when you build everything and if you build without the UI stuff. Useful to run it headless/GUI-less using for example jalv from the command-line.

falkTX commented 7 years ago

This has now been added to all DPF based plugins. Doing something like this for all of them is a pain :(

The modgui stuff remains, there was nothing wrong with it in the first place.

simonvanderveldt commented 7 years ago

This has now been added to all DPF based plugins. Doing something like this for all of them is a pain :(

Too many plugins! ;) Thanks for doing this though!

I'll have a look at doing some builds, if I run into any other problems I'll create a new issue.

simonvanderveldt commented 7 years ago

@falkTX Just looked through the canges and I've got some bad news :x Seems something went wrong with the VST option, in the initial definition it's called BUILD_VST2, but the ifeqs use BUILD_VST

falkTX commented 7 years ago

Ah yes, because sometime later VST3 support might be added. I'll handle this sometime soon.

simonvanderveldt commented 7 years ago

Ah yes, because sometime later VST3 support might be added. I'll handle this sometime soon.

I can also make a couple of PRs if you want

falkTX commented 7 years ago

I think you need ~8 PRs, for the different repos and then DPF-Plugins too. I can do this too, but if you're in a rush... I will click the merge button. :)