diff --git a/CMakeLists.txt b/CMakeLists.txt index cb9a9b0d7ee..a5bf65a8073 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -339,7 +339,6 @@ set(SURGE_SHARED_SOURCES set(SURGE_SYNTH_SOURCES src/common/SurgeSynthesizer.cpp - src/common/SurgeSynthesizerIDManagement.cpp src/common/SurgeSynthesizerIO.cpp ) diff --git a/src/common/SurgeSynthesizer.h b/src/common/SurgeSynthesizer.h index 74f4f82cb36..1a1db1e7f93 100644 --- a/src/common/SurgeSynthesizer.h +++ b/src/common/SurgeSynthesizer.h @@ -141,26 +141,26 @@ class alignas(16) SurgeSynthesizer public: /* - * For more on this IFDEF see the comment in SurgeSynthesizerIDManagement.cpp + * So when surge was pre-juce we contemplated writing our own ID remapping between + * internal indices and DAW IDs so put an indirectin and class in place. JUCE obviates + * the need for that by using hash of stremaing name as an ID consistently throuhg its + * param mechanism. I could, in theory, have gone right back to int as my accessor class + * but there's something compelilng about keeping that indirection I plumbed in just in case + * i need it in the future. So the ID class is now just a simple wrapper on an int which is + * entirely inline. */ struct ID { - int getDawSideIndex() const { return dawindex; } - int getDawSideId() const { return dawid; } int getSynthSideId() const { return synthid; } std::string toString() const { std::ostringstream oss; - oss << "ID[ dawidx=" << dawindex << ", dawid=" << dawid << " synthid=" << synthid - << " ]"; + oss << "ID[" << synthid << "]"; return oss.str(); } - bool operator==(const ID &other) const - { - return dawindex == other.dawindex && dawid == other.dawid && synthid == other.synthid; - } + bool operator==(const ID &other) const { return synthid == other.synthid; } bool operator!=(const ID &other) const { return !(*this == other); } friend std::ostream &operator<<(std::ostream &os, const ID &id) @@ -170,16 +170,20 @@ class alignas(16) SurgeSynthesizer } private: - int dawindex = -1, dawid = -1, synthid = -1; + int synthid = -1; friend SurgeSynthesizer; }; - bool fromDAWSideId(int i, ID &q); - bool fromDAWSideIndex(int i, ID &q); - bool fromSynthSideId(int i, ID &q); - bool fromSynthSideIdWithGuiOffset(int i, int start_paramtags, int start_metacontrol_tag, ID &q); + bool fromSynthSideId(int i, ID &q) const + { + if (i < 0 || i >= n_total_params) + return false; + + q.synthid = i; + return true; + } - const ID idForParameter(const Parameter *p) + ID idForParameter(const Parameter *p) const { // We know this will always work ID i; diff --git a/src/common/SurgeSynthesizerIDManagement.cpp b/src/common/SurgeSynthesizerIDManagement.cpp deleted file mode 100644 index 88aa67ab282..00000000000 --- a/src/common/SurgeSynthesizerIDManagement.cpp +++ /dev/null @@ -1,183 +0,0 @@ -// -*-c++--*- -/* -** Surge Synthesizer is Free and Open Source Software -** -** Surge is made available under the Gnu General Public License, v3.0 -** https://www.gnu.org/licenses/gpl-3.0.en.html -** -** Copyright 2004-2020 by various individuals as described by the Git transaction log -** -** All source at: https://github.com/surge-synthesizer/surge.git -** -** Surge was a commercial product from 2004-2018, with Copyright and ownership -** in that period held by Claes Johanson at Vember Audio. Claes made Surge -** open source in September 2018. -*/ - -/* -** This file also includes the 2020 non-fiction work, "A Short Essay on Indices in -** Surge". -** -** A Short Essay on Indices in Surge -** -** Indices in Surge are complicated, and are complicated for a couple of reasons -** -** 1. The SurgePatch object has a list of parameters which have indices which are -** handed out linearly by their declaration in SurgePatch. -** 2. The SurgeStorage object indicates a layout of types and subtypes, but the -** code is full of implicit and explicit assumptions that the parameter indices -** of those parameters are adjacent. That is, the parameters in OSCStorage -** are a contiguious non-overlapping bunch of parameter IDs. Moreover, SurgeStorage -** assumes everywhere that given a parameter with ID, that param_ptr[p->id] is that -** parameter, so the internal IDs are also the array indices. -** 3. The DAWs expose a variety of phantom IDs. For instance, the Macros are -** the first 8 DAW parameters, but they are not actually parameters at all -** in Surge. -** 4. SurgeGUIEditor uses the VSTGUI concept of 'tag' on a control, but some controls -** also don't bind to parameters (like the Menu button) so it has a collection -** of control tags which are 'up front' in tag space. That means a SurgeGUIEditor -** tag from control->getTag() will either be internal (if c->getTag() < start_paramtags) -** or refer to an internal parameter index (control->getTag() - start_paramtags is an index -** into the synth->param_ptr array). -** 5. The VST3 makes it worse since, as VST3 doesn't support MIDI control inputs, we -** have had to create MIDI control port virtual parameters after the end of Surge -** params, so the VST3 is full of ifs which work but are not very well documented. -** -** Luckily, patch streaming is immune to all of this since patches (and LV2 control ports) -** use the streaming name. So this problem only really rears its hed if those DAW phantom -** IDs align with the parameter IDs as 1:1 offsets. If that was the case, then adding -** a parameter would change VST IDs. And that is the case in Surge 1.7.1 and earlier. -** -** So what does it mean to add a parameter? Well let's consider adding a parameter to -** OSCStorage so oscillators had 8, not 7, slots each. If you just did that willy nilly -** today here's what would happen: -** -** 1. You would change n_osc_params from 7 to 8 and recompile. Great. Everything would work -** in headless and the synth would come up no problem and load patches and run. If we had -** some place put in a '7' rather than an 'n_osc_params' you would have to find and fix that. -** 2. The VST2/3/AU ID of every item after scene 1 / osc 1 would increase by 3 (in scene A) -** and 6 (in scene B) -** 3. So almost all your old automation would map to the wrong spot in your DAW. -** -** So this file exists to "not have that happen" (or more accurately "make it so that cannot -* happen when we expand"). How does it work? -* -* Well first, SurgeSynthesizer has had all of its internal APIs to do things like get and set -* parameters converted from int as index to an ID object as index. This solves one of the biggest -* confusing points, which is "is the int I am taking about the param index, the GUI index, the DAW -* index, or the DAW ID". Once that's done you get obvious constructors for those, which are the -* from methods. (Note the fromGUITag is a static on SurgeGUIEditor since that requires GUI -*information but also is only called from GUI-aware clients). -* -* And then you have two strategies. -* -* For hosts which match the DAW index and DAW ID (! PLUGIN_ID_AND_INDEX_ARE_DISTINCT, which in -* this implementation are the VST2, LV2 and AU) you build everything on the DAWSideIndex and -* the SynthID and basically convert back and forth using get methods at control points. -* -* For the VST3 which can issue IDs for its parameters non-monotonically, we have the DAWSideIndex -* and the DAWSideID as distinct values. This means that when we add an item in 'order' we can -* keep its ID as whatever we want and not break stremaing in the future. -* -* And then these classes implement the 1.8-style ID management which is -* -* (INDEX version) -* DAW params 0->7 map to the custom controls, which have SynthID metaparam_offset_i -* DAW params 8->n_params map to the params, which have identical SynthID values -* DAW params n_params -> n_params + 7 map to the first 8 params, displaces by the controls, which -*have SynthID 0-7. -* -* The ID version basically keeps the DAW ID and the SynthID the same for now. In a future version -* where we expand SynthIDs (which remember will need to be continugous) that constraint will break -* to preserve streaming. -*/ - -#include "SurgeSynthesizer.h" -#include "DebugHelpers.h" - -bool SurgeSynthesizer::fromDAWSideIndex(int i, ID &q) -{ - q.dawindex = i; - if (i < num_metaparameters) - { - q.synthid = i + metaparam_offset; -#if PLUGIN_ID_AND_INDEX_ARE_DISTINCT - q.dawid = i + metaparam_offset; -#endif - return true; - } - else if (i < n_total_params) - { - q.synthid = i; -#if PLUGIN_ID_AND_INDEX_ARE_DISTINCT - q.dawid = i; -#endif - return true; - } - else if (i >= n_total_params && i < n_total_params + num_metaparameters) - { - q.synthid = i - n_total_params; -#if PLUGIN_ID_AND_INDEX_ARE_DISTINCT - q.dawid = i - n_total_params; -#endif - return true; - } - return false; -} - -#if PLUGIN_ID_AND_INDEX_ARE_DISTINCT -bool SurgeSynthesizer::fromDAWSideId(int i, ID &q) -{ - q.dawid = i; - q.synthid = i; - - if (i >= metaparam_offset) - { - q.dawindex = i - metaparam_offset; - } - else if (i < num_metaparameters) - { - q.dawindex = i + n_total_params; - } - else - { - q.dawindex = i; - } - - // TODO - generate the index - return true; -} -#endif - -bool SurgeSynthesizer::fromSynthSideId(int i, ID &q) -{ -#if PLUGIN_ID_AND_INDEX_ARE_DISTINCT - q.dawid = i; -#endif - q.synthid = i; - - if (i >= metaparam_offset) - { - q.dawindex = i - metaparam_offset; - } - else if (i < num_metaparameters) - { - q.dawindex = i + n_total_params; - } - else - { - q.dawindex = i; - } - return true; -} - -bool SurgeSynthesizer::fromSynthSideIdWithGuiOffset(int i, int start_paramtags, - int start_metacontrol_tag, ID &q) -{ - bool res = false; - if (i >= start_paramtags && i <= start_paramtags + n_total_params) - res = fromSynthSideId(i - start_paramtags, q); // wrong for macros and stuff - else if (i >= start_metacontrol_tag && i <= start_metacontrol_tag + num_metaparameters) - res = fromSynthSideId(i - start_metacontrol_tag + metaparam_offset, q); - return res; -} \ No newline at end of file diff --git a/src/gui/SurgeGUIEditor.cpp b/src/gui/SurgeGUIEditor.cpp index 882b8348b76..04322807f68 100644 --- a/src/gui/SurgeGUIEditor.cpp +++ b/src/gui/SurgeGUIEditor.cpp @@ -82,12 +82,6 @@ using namespace Surge::ParamConfig; int SurgeGUIEditor::start_paramtag_value = start_paramtags; -bool SurgeGUIEditor::fromSynthGUITag(SurgeSynthesizer *synth, int tag, SurgeSynthesizer::ID &q) -{ - // This is wrong for macros and params but is close - return synth->fromSynthSideIdWithGuiOffset(tag, start_paramtags, tag_mod_source0 + ms_ctrl1, q); -} - SurgeGUIEditor::SurgeGUIEditor(SurgeSynthEditor *jEd, SurgeSynthesizer *synth) { assert(n_paramslots >= n_total_params); diff --git a/src/gui/SurgeGUIEditor.h b/src/gui/SurgeGUIEditor.h index 6bbfc128f32..b83e17882c3 100644 --- a/src/gui/SurgeGUIEditor.h +++ b/src/gui/SurgeGUIEditor.h @@ -144,7 +144,6 @@ class SurgeGUIEditor : public Surge::GUI::IComponentTagValue::Listener, void showSettingsMenu(const juce::Point<int> &where, Surge::GUI::IComponentTagValue *launchFrom); - static bool fromSynthGUITag(SurgeSynthesizer *synth, int tag, SurgeSynthesizer::ID &q); // If n_scenes > 2, then this initialization and the modsource_editor one below will need to // adjust int current_scene = 0, current_osc[n_scenes] = {0, 0}, current_fx = 0; diff --git a/src/headless/UnitTestsID.cpp b/src/headless/UnitTestsID.cpp deleted file mode 100644 index 39b0ea29eee..00000000000 --- a/src/headless/UnitTestsID.cpp +++ /dev/null @@ -1,61 +0,0 @@ -#include <iostream> -#include <iomanip> -#include <sstream> -#include <algorithm> - -#include "HeadlessUtils.h" -#include "Player.h" - -#include "catch2/catch2.hpp" - -#include "UnitTestUtilities.h" - -using namespace Surge::Test; - -TEST_CASE("ID Issuance is Reflexive", "[id]") -{ - auto surge = Surge::Headless::createSurge(44100); - REQUIRE(surge); - SECTION("Valid for all index points") - { - for (int i = 0; i < n_total_params + num_metaparameters; ++i) - { - auto id = SurgeSynthesizer::ID(); - REQUIRE(surge->fromDAWSideIndex(i, id)); - } - } - SECTION("Generate by Any Means") - { - std::vector<SurgeSynthesizer::ID> ids; - for (int i = 0; i < n_total_params + num_metaparameters; ++i) - { - auto id = SurgeSynthesizer::ID(); - REQUIRE(surge->fromDAWSideIndex(i, id)); - ids.push_back(id); - } - - INFO("Checking recretable by dawIndex"); - for (const auto &sid : ids) - { - auto id = SurgeSynthesizer::ID(); - REQUIRE(surge->fromDAWSideIndex(sid.getDawSideIndex(), id)); - REQUIRE(sid == id); - } - - INFO("Checking recretable by dawID"); - for (const auto &sid : ids) - { - auto id = SurgeSynthesizer::ID(); - REQUIRE(surge->fromDAWSideId(sid.getDawSideId(), id)); - REQUIRE(sid == id); - } - - INFO("Checking recretable by syntID"); - for (const auto &sid : ids) - { - auto id = SurgeSynthesizer::ID(); - REQUIRE(surge->fromSynthSideId(sid.getSynthSideId(), id)); - REQUIRE(sid == id); - } - } -} diff --git a/src/python_bindings/surgepy.cpp b/src/python_bindings/surgepy.cpp index 889f408fbd2..3f47c0d0c63 100644 --- a/src/python_bindings/surgepy.cpp +++ b/src/python_bindings/surgepy.cpp @@ -824,8 +824,6 @@ PYBIND11_MODULE(surgepy, m) "getVersion", []() { return Surge::Build::FullVersionStr; }, "Get the version of Surge"); py::class_<SurgeSynthesizer::ID>(m, "SurgeSynthesizer_ID") .def(py::init<>()) - .def("getDawSideIndex", &SurgeSynthesizer::ID::getDawSideIndex) - .def("getDawSideId", &SurgeSynthesizer::ID::getDawSideId) .def("getSynthSideId", &SurgeSynthesizer::ID::getSynthSideId) .def("__repr__", &SurgeSynthesizer::ID::toString); diff --git a/src/surge_synth_juce/SurgeSynthVST3Extensions.cpp b/src/surge_synth_juce/SurgeSynthVST3Extensions.cpp index f4912243a96..a040102a3c4 100644 --- a/src/surge_synth_juce/SurgeSynthVST3Extensions.cpp +++ b/src/surge_synth_juce/SurgeSynthVST3Extensions.cpp @@ -27,206 +27,3 @@ void SurgeSynthEditorSpecificExtensions(SurgeSynthEditor *e, SurgeGUIEditor *sed { std::cout << "SynthEditor: " << __FILE__ << std::endl; } - -#if YOU_WANT_TO_SEE_THE_OLD_MENU_CODE -/* - * When I removed TARGET_VST3 from everywhere I kept the menu code herefor when we restore it - */ -// SGE.h -Steinberg::Vst::IContextMenu * -addVst3MenuForParams(VSTGUI::COptionMenu *c, const SurgeSynthesizer::ID &, - int &eid); // just a noop if you aren't a vst3 of course - -#include "pluginterfaces/vst/ivstcontextmenu.h" -#include "pluginterfaces/base/ustring.h" - -#include "vstgui/lib/cvstguitimer.h" - -#include "SurgeVst3Processor.h" - -template <typename T> struct RememberForgetGuard -{ - RememberForgetGuard(T *tg) - { - t = tg; - - int rc = -1; - if (t) - rc = t->addRef(); - } - RememberForgetGuard(const RememberForgetGuard &other) - { - int rc = -1; - if (t) - { - rc = t->release(); - } - t = other.t; - if (t) - { - rc = t->addRef(); - } - } - ~RememberForgetGuard() - { - if (t) - { - t->release(); - } - } - T *t = nullptr; -}; - -// later on - -Steinberg::Vst::IContextMenu *hostMenu = nullptr; -SurgeSynthesizer::ID mid; -if (synth->fromSynthSideId(modsource - ms_ctrl1 + metaparam_offset, mid)) - hostMenu = addVst3MenuForParams(contextMenu, mid, eid); -// then ... - -if (hostMenu) - hostMenu->release(); - -#if TARGET_VST3 -auto hostMenu = addVst3MenuForParams(contextMenu, synth->idForParameter(p), eid); -#endif - -#if TARGET_VST3 -if (hostMenu) - hostMenu->release(); -#endif - -Steinberg::Vst::IContextMenu *SurgeGUIEditor::addVst3MenuForParams(VSTGUI::COptionMenu *contextMenu, - const SurgeSynthesizer::ID &pid, - int &eid) -{ - CRect menuRect; - Steinberg::Vst::IComponentHandler *componentHandler = getController()->getComponentHandler(); - Steinberg::FUnknownPtr<Steinberg::Vst::IComponentHandler3> componentHandler3(componentHandler); - Steinberg::Vst::IContextMenu *hostMenu = nullptr; - if (componentHandler3) - { - std::stack<COptionMenu *> menuStack; - menuStack.push(contextMenu); - std::stack<int> eidStack; - eidStack.push(eid); - - Steinberg::Vst::ParamID param = pid.getDawSideId(); - hostMenu = componentHandler3->createContextMenu(this, ¶m); - - int N = hostMenu ? hostMenu->getItemCount() : 0; - if (N > 0) - { - contextMenu->addSeparator(); - eid++; - } - - std::deque<COptionMenu *> parentMenus; - for (int i = 0; i < N; i++) - { - Steinberg::Vst::IContextMenu::Item item = {0}; - Steinberg::Vst::IContextMenuTarget *target = {0}; - - hostMenu->getItem(i, item, &target); - - // char nm[1024]; - // Steinberg::UString128(item.name, 128).toAscii(nm, 1024); -#if WINDOWS - // https://stackoverflow.com/questions/32055357/visual-studio-c-2015-stdcodecvt-with-char16-t-or-char32-t - std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>, wchar_t> conversion; - std::string nm = conversion.to_bytes((wchar_t *)(item.name)); -#else - std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t> conversion; - std::string nm = conversion.to_bytes((char16_t *)(item.name)); -#endif - - if (nm[0] == - '-') // FL sends us this as a separator with no VST indication so just strip the '-' - { - int pos = 1; - while (nm[pos] == ' ' && nm[pos] != 0) - pos++; - nm = nm.substr(pos); - } - - auto itag = item.tag; - /* - ** Leave this here so we can debug if another vst3 problem comes up - std::cout << nm << " FL=" << item.flags << " jGS=" << - Steinberg::Vst::IContextMenuItem::kIsGroupStart - << " and=" << ( item.flags & Steinberg::Vst::IContextMenuItem::kIsGroupStart ) - << " IGS=" - << ( ( item.flags & Steinberg::Vst::IContextMenuItem::kIsGroupStart ) == - Steinberg::Vst::IContextMenuItem::kIsGroupStart ) << " IGE=" - << ( ( item.flags & Steinberg::Vst::IContextMenuItem::kIsGroupEnd ) == - Steinberg::Vst::IContextMenuItem::kIsGroupEnd ) << " " - << std::endl; - - if( item.flags != 0 ) - printf( "FLAG %d IGS %d IGE %d SEP %d\n", - item.flags, - item.flags & Steinberg::Vst::IContextMenuItem::kIsGroupStart, - item.flags & Steinberg::Vst::IContextMenuItem::kIsGroupEnd, - item.flags & Steinberg::Vst::IContextMenuItem::kIsSeparator - ); - */ - if ((item.flags & Steinberg::Vst::IContextMenuItem::kIsGroupStart) == - Steinberg::Vst::IContextMenuItem::kIsGroupStart) - { - COptionMenu *subMenu = new COptionMenu( - menuRect, 0, 0, 0, 0, - VSTGUI::COptionMenu::kNoDrawStyle | VSTGUI::COptionMenu::kMultipleCheckStyle); - menuStack.top()->addEntry(subMenu, nm.c_str()); - menuStack.push(subMenu); - subMenu->forget(); - eidStack.push(0); - - /* - VSTGUI doesn't seem to allow a disabled or checked grouping menu. - if( item.flags & Steinberg::Vst::IContextMenuItem::kIsDisabled ) - { - subMenu->setEnabled(false); - } - if( item.flags & Steinberg::Vst::IContextMenuItem::kIsChecked ) - { - subMenu->setChecked(true); - } - */ - } - else if ((item.flags & Steinberg::Vst::IContextMenuItem::kIsGroupEnd) == - Steinberg::Vst::IContextMenuItem::kIsGroupEnd) - { - menuStack.pop(); - eidStack.pop(); - } - else if (item.flags & Steinberg::Vst::IContextMenuItem::kIsSeparator) - // separator not group end. Thanks for the insane definition of these constants VST3! - // (See #3090) - { - menuStack.top()->addSeparator(); - } - else - { - RememberForgetGuard<Steinberg::Vst::IContextMenuTarget> tg(target); - RememberForgetGuard<Steinberg::Vst::IContextMenu> hm(hostMenu); - - auto menu = addCallbackMenu( - menuStack.top(), nm, [this, hm, tg, itag]() { tg.t->executeMenuItem(itag); }); - eidStack.top()++; - if (item.flags & Steinberg::Vst::IContextMenuItem::kIsDisabled) - { - menu->setEnabled(false); - } - if (item.flags & Steinberg::Vst::IContextMenuItem::kIsChecked) - { - menu->setChecked(true); - } - } - // hostMenu->addItem(item, &target); - } - eid = eidStack.top(); - } - return hostMenu; -} -#endif