RobinSchmidt / RS-MET

Codebase for RS-MET products (Robin Schmidt's Music Engineering Tools)
Other
56 stars 6 forks source link

discussion: Creating Soundemote's version of toolchain #286

Open elanhickler opened 5 years ago

elanhickler commented 5 years ago

I think my first step will just be to use toolchain as is. Then I can begin to customize it and let you know the problems I run into, then you can begin to rework some of the code to include additional flexibility that I need.

image

RobinSchmidt commented 5 years ago

maybe i could make these two members pointers instead of direct objects:

  ModulationManager modManager;
  AudioModuleFactory moduleFactory;

to avoid excessive data objects and allow sharing between several ToolChain objects. and/or maybe that one could be factored out into a baseclass: std::vector<AudioModule*> modules; ...and then you would only need the baseclass and not the full ToolChain class to derive from. the baseclass would already have to implement all these add/remove/replace operations. we'll see

elanhickler commented 5 years ago

I started by subclass ToolChain as SeToolChainModule and ToolChainEditor as SeToolChainEditor.

First problem: I need to register my own modules. The constructor of ToolChain registers a bunch of modules and I cannot avoid that because... I don't know how to avoid ToolChain constructor.

image

going to unprotected moduleFactory moduleInfos and clear the registered modules for now.

elanhickler commented 5 years ago

I need a ToolChain "getActiveSlot" function as activeSlot is protected.

elanhickler commented 5 years ago

removed almost everything, but can't figure out how the selectors and editor is getting its position:

image

elanhickler commented 5 years ago

progress k6Dfy5T 1

RobinSchmidt commented 5 years ago

I need to register my own modules. The constructor of ToolChain registers a bunch of modules and I cannot avoid that because... I don't know how to avoid ToolChain constructor.

right. you can't (i think...subclass constructors always invoke their baseclass constructors...that's one of the purposes of constructors). i'm starting to think, it would make sense to factor out a baseclass from which you may derive

can't figure out how the selectors and editor is getting its position...progress

did you figure it out?

elanhickler commented 5 years ago

i figured it out almost... I think I still need to remove your selectors from appearing.

we need an index move function for draggable tabs.

this is somewhat tested...

edit: oh wait no it doesn't work if oldIndex > newIndex.

edit: maybe this? it seems to work https://coliru.stacked-crooked.com/a/8a1f8b6b92ebaede

what the hell, it was working in the online compiler but not in visual studio. This works: https://coliru.stacked-crooked.com/a/5c31007000b9eeba

and also the function is super simple:

template <typename t> void move(std::vector<t>& v, size_t oldIndex, size_t newIndex)
{
    if (oldIndex > newIndex)
        std::rotate(v.rend() - oldIndex - 1, v.rend() - oldIndex, v.rend() - newIndex);
    else        
        std::rotate(v.begin() + oldIndex, v.begin() + oldIndex + 1, v.begin() + newIndex + 1);
}
elanhickler commented 5 years ago

tabs now completely work:

cs1R5nX 1

elanhickler commented 5 years ago

I think you have a bug, -1 index is causing me issues. The index should go to the next available module instead of -1. Edit: It should go to -1 only if there are no modules.

image

Did you not anticipate having the 0th module removed while modules still exist in other indexes?

Edit: I see, in ToolChain vst, if you remove a non-last module, the editor & selector just gets changed to none instead of getting deleted, probably using the ReplaceModule function, I don't want to rely on that. I plan on supporting a blank tool chain editor.

elanhickler commented 5 years ago

Why does ToolChain keep track of the active slot? Seems that should be on the editor side. I can't think of a way that the active slot has any audio consequences.

Edit: P.S. I had to do a number of overrides just to have ToolChain never refer to selectors.

Edit: i would rename audioModuleWillBeDeleted to audioModuleWasDeleted to stay consistent.I don't see how the module "will be deleted" in the future, it seems it's just deleted.

elanhickler commented 5 years ago

progress... I really need to start mixing the outputs of all the tabs. Maybe you should add a "preocessStereoFrameAccumulate" or something to the AudioModule base class and we can override that for all our modules and we can either use "processStereoFrame" when we don't want to mix the input and "processStereoFrameAccumulate" when we do. What do you want to call it? processStereoFrameMixed? processStereoFrameIncludeInput? processStereoFrameWithInput?

Or we could add a simple bool doMixInput or wantsInputMixed = false to the function.

image

elanhickler commented 5 years ago

Progress! PrettyScope is now easy to add to an audio module.

RobinSchmidt commented 5 years ago

Did you not anticipate having the 0th module removed while modules still exist in other indexes?

yep - i definitely need to fix this. how could this go undetected for so long?

elanhickler commented 5 years ago

because in toolchain it's impossible to remove the 0th module while another module still exists.

edit: "remove" in this case means remove it from the interface. Setting module to "none" is not removing.

RobinSchmidt commented 5 years ago

Setting module to "none" is not removing.

ah - right - i'm leaving a dummy in the slot - leaving the slot as placeholder

RobinSchmidt commented 5 years ago

The index should go to the next available module instead of -1. Edit: It should go to -1 only if there are no modules.

hmmm...i guess the issue here is that i didn't anticipate a completely empty module list. in my toolchain, there is always at least one module present. ...and you may have a completely empty list

elanhickler commented 5 years ago

yeah I wanted to be able to have no modules, which is working for my toolchain. I think this is all I had to do, avoid resizing a nullptr editor:

image

RobinSchmidt commented 5 years ago

i just added this special case handling to the end of the function

void ToolChain::deleteModule(int index)
{
  ScopedLock scopedLock(*lock);
  jassert(index >= 0 && index < size(modules)); // index out of range
  if(activeSlot == index) 
    activeSlot--;
  sendAudioModuleWillBeDeletedNotification(modules[index], index);
  removeFromModulatorsIfApplicable(modules[index]);
  delete modules[index];
  remove(modules, index);

  // handle case when 0th module was deleted (needs test):
  if(activeSlot == -1 && size(modules) > 0 )
    activeSlot = 0;
  // ...in ToolChain, it's impossible to have a completely empty module list, but subclasses (like 
  // Elan's) may want to allow this, so we need to support it
}

...not sure, if this is the best (or even right) way to do it. i guess, i will find out, when i try with your version - because in mine, i don't get into this situation

RobinSchmidt commented 5 years ago

i can't build your version due to missing module "elan_juce_helpers".

elanhickler commented 5 years ago

Did you want to have a side bar for modules that you drag into the toolchain? If so you probably should clean up your code by supporting no modules. For my toolchain, I'm going to have a menu that pops up and you click to add a module (so, no dragging from one component to another).

https://docs.juce.com/master/classDragAndDropContainer.html https://docs.juce.com/master/classDragAndDropTarget.html

If you want to do simple dragging of a component like my tabs:

https://docs.juce.com/master/classComponentDragger.html

My implementation is not how you're supposed to do it. You're supposed to make the draggable component have a ComponentDragger. Instead, I made the parent component have a ComponentDragger.

  1. On mouse down, get component that was clicked, assign to component dragger.

  2. On mouse drag, instead of dragging the actual child component, I drag an ImageComponent which I assign the image of child component and make it transparent and the user thinks they are dragging the actual child component, but it's just an image of the component. This seemed to work better and be easier to implement then actually moving the component around with the mouse.

// ImageComoponent dragImage declared in parent class
dragImage.setImage(tab->createComponentSnapshot(tab->getLocalBounds()));        
dragImage.setAlpha(0.5);
dragImage.setAlwaysOnTop(true);
dragImage.setBounds(tab->getBounds());
addChildComponent(dragImage);
dragger.startDraggingComponent(&dragImage, event);

8caa19fc2b 1

  1. Using paintOverChildren method I draw a black transparent square to grey out the child component that is being dragged or a red square if the drop location would be the same place as the child component being dragged. Or draw the red square inbetween tabs where the drop location will be.

The math to check for drop location was really confusing for me to figure out.

auto dragX = dragImage.getBounds().getCentreX();

int absDeltaBuffer = INT_MAX;
for (int i = 0; i < getNumTabs(); ++i)
{
    auto* component = getTabButton(i);

    int startX = component->getX();
    int absDelta = abs(dragX - startX);

    if (absDelta > absDeltaBuffer)
        break;

    absDeltaBuffer = absDelta;
    dragPlacementX = startX;
    dragPlacementIndex = i;        
}

// see if tab is being placed beyond all tabs (last spot)
if ( (getTabButton(getNumTabs() - 1)->getBounds().getRight() - dragX) < absDeltaBuffer)
    {
        ++dragPlacementIndex;
        dragPlacementX = getTabButton(getNumTabs() - 1)->getBounds().getRight();
    }

// tab original index and 1 more is the same spot
if (dragPlacementIndex > originalDragIndex)
    --dragPlacementIndex;
elanhickler commented 5 years ago

Robin, you are not following Git repository etiquette! read the readme 👍

https://gitlab.com/Hickler/elanjucehelpers

image

elanhickler commented 5 years ago

just want to bring your attention to the new ListenerList class in JUCE which now exclusevily uses std::function which makes things so much easier because you can tell your listeners exactly what happened and what function to use, you can give the function a clear name, a lot less confusing. Here's how I use it for my TabbedButtonBar component (children are called

class EditorTabBar : public TabbedButtonBar
{
public:
    EditorTabBar() : TabbedButtonBar(TabbedButtonBar::Orientation::TabsAtTop)
    {
        setInterceptsMouseClicks(true, true);
    }

    class Listener
    {
    public:
        virtual ~Listener() = default;

        virtual void tabWasRemoved(int index) {}
        virtual void tabWasAdded(int index) {}
        virtual void tabWasMoved(int oldIndex, int newIndex) {}
        virtual void tabWasActivated(int index) {}
    };

    void addListener(Listener* l) { listeners.add(l); }
    void removeListener(Listener* l) { listeners.remove(l); }

    virtual void moveTab(const int currentIndex, const int newIndex, const bool animate, bool notifyListeners = true, EditorTabBar::Listener* callingObject = nullptr)
    {
        TabbedButtonBar::moveTab(currentIndex, newIndex, animate);

        if (currentIndex != newIndex)
            if (notifyListeners)
                // std::function HERE
                listeners.callExcluding(callingObject, [&](Listener& listener) { listener.tabWasMoved(currentIndex, newIndex); });
    }

    virtual void addTab(const String& tabName, Colour tabBackgroundColour, int insertIndex, bool notifyListeners = true, EditorTabBar::Listener* callingObject = nullptr)
    {
        TabbedButtonBar::addTab(tabName, tabBackgroundColour, insertIndex);

        if (notifyListeners)
            // std::function HERE
            listeners.callExcluding(callingObject, [&](Listener& listener) { listener.tabWasAdded(insertIndex == -1 ? getNumTabs() - 1 : insertIndex); });
    }

     // etc...
protected:
    ListenerList<Listener> listeners;
RobinSchmidt commented 5 years ago

read the readme

ah - ok - i see. but it seems to have problems with using the functionality from there. i'm getting a bunch of compile errors of the type:

BasicEditor.h(87,1): error C2653:  'FileHelper': is not a class or namespace name
BasicEditor.h(87,34): error C3861:  'toUniversalSlash': identifier not found

i can see the class and function - but apparently, the compiler can't. is some include messed up? or a namespace issue? dunno

elanhickler commented 5 years ago

you're compiling Soundemote\AudioPlugins\SeToolChain ?

I'm going to re-make the project with jucer now and try.

ps: still trying to release the new Flower Child Filter, have you solved all the build errors you can? More important that getting my toolchain to work is compiling Flower Child Filter on mac.

elanhickler commented 5 years ago

there might be some include issues, pull the latest from ElanJuceHelpers, just updated it.

RobinSchmidt commented 5 years ago

ok - i can compile now - but i had to add the /bigobj option to the compiler settings (of the "...SharedCode" project): image without it, i got errors that said something about object files exceeding maximum allowed size (in debug/x64 mode)

RobinSchmidt commented 5 years ago

still trying to release the new Flower Child Filter, have you solved all the build errors you can?

didn't you say in your mail that you have fixed the build issues? after updating my codeblocks, i got some new errors which i'm still struggling with. problem is, even the new codeblocks (including a new version of mingw) doesn't allow x64 builds - and juce doesn't seem to support being compiled in x86 anymore - at least not on windows with gcc. when trying, i get compiler errors from juce. with the old version of codeblocks, i can compile my TestsRosicAnRapt project (because it doesn't drag in any juce stuff) but as soon as try ToolChain, it fails. hmm...maybe i should try on linux

RobinSchmidt commented 5 years ago

okay - i may have fixed the crash. drag an update of my library (code changes only affect my library). you'll find a new comment in ToolChain's destructor about what - i think - may have gone wrong

elanhickler commented 5 years ago

in jura_AudioPlugin.cpp I added what is in the red square to prevent crash. For some reason all of a sudden this started happening, also keep in mind the getXmlFromBinary may return nullptr according to the description, so it's probably a good idea to have this in the first place:

image

elanhickler commented 5 years ago

in jura_ModulatableParameter.cpp getting crash if I add then remove TorusGenerator the close SeToolChain.

You may have fixed the crash of adding TorusGenerator WITHOUT removing and then closing SeToolChain.

image

elanhickler commented 5 years ago

Crash if I register Flower Child Filter:

image

image