Introduce proper thread safety around audio effects, and make sure key press detection is global

Source/EffectsComponent.cpp

@@ -18,12 +18,14 @@ EffectsComponent::EffectsComponent(OscirenderAudioProcessor& p) : audioProcessor
         }
     };
 
-    auto effects = audioProcessor.allEffects;
-	for (int i = 0; i < effects.size(); i++) {
-		auto effect = effects[i];
-        effect->setValue(effect->getValue());
-		itemData.data.push_back(effect);
-	}
+    {
+        juce::SpinLock::ScopedLockType lock(audioProcessor.effectsLock);
+        for (int i = 0; i < audioProcessor.allEffects.size(); i++) {
+            auto effect = audioProcessor.allEffects[i];
+            effect->setValue(effect->getValue());
+            itemData.data.push_back(effect);
+        }
+    }
 
     /*addBtn.setButtonText("Add Item...");
     addBtn.onClick = [this]()

Source/MainComponent.cpp

@@ -23,7 +23,7 @@ MainComponent::MainComponent(OscirenderAudioProcessor& p, OscirenderAudioProcess
 			}
 			pluginEditor.addCodeEditor(audioProcessor.getCurrentFileIndex());
 			pluginEditor.updateCodeEditor();
-			pluginEditor.fileUpdated(audioProcessor.getCurrentFile());
+			pluginEditor.fileUpdated(std::make_unique<juce::File>(audioProcessor.getCurrentFile()));
 			updateFileLabel();
 		});
 	};
@@ -40,7 +40,11 @@ MainComponent::MainComponent(OscirenderAudioProcessor& p, OscirenderAudioProcess
 		pluginEditor.removeCodeEditor(audioProcessor.getCurrentFileIndex());
 		audioProcessor.removeFile(audioProcessor.getCurrentFileIndex());
 		pluginEditor.updateCodeEditor();
-		pluginEditor.fileUpdated(audioProcessor.getCurrentFile());
+		std::unique_ptr<juce::File> file;
+		if (audioProcessor.getCurrentFileIndex() != -1) {
+			file = std::make_unique<juce::File>(audioProcessor.getCurrentFile());
+		}
+		pluginEditor.fileUpdated(std::move(file));
 		updateFileLabel();
 	};
 

Source/ObjComponent.cpp

@@ -67,12 +67,8 @@ void ObjComponent::mouseMove(const juce::MouseEvent& e) {
 	}
 }
 
-// listen for when escape is pressed to disable mouse rotation
-bool ObjComponent::keyPressed(const juce::KeyPress& key) {
-	if (key == juce::KeyPress::escapeKey) {
-		mouseRotate.setToggleState(false, juce::NotificationType::dontSendNotification);
-	}
-	return true;
+void ObjComponent::disableMouseRotation() {
+	mouseRotate.setToggleState(false, juce::NotificationType::dontSendNotification);
 }
 
 void ObjComponent::resized() {
@@ -89,5 +85,4 @@ void ObjComponent::resized() {
 	resetRotation.setBounds(row.removeFromLeft(120));
 	row.removeFromLeft(20);
 	mouseRotate.setBounds(row);
-
 }

Source/ObjComponent.h

@@ -12,7 +12,7 @@ public:
 
     void resized() override;
     void mouseMove(const juce::MouseEvent& event) override;
-    bool keyPressed(const juce::KeyPress& key) override;
+    void disableMouseRotation();
 private:
     OscirenderAudioProcessor& audioProcessor;
     OscirenderAudioProcessorEditor& pluginEditor;

Source/PluginEditor.cpp

@@ -106,7 +106,7 @@ void OscirenderAudioProcessorEditor::removeCodeEditor(int index) {
 }
 
 
-// parsersLock must be locked before calling this function
+// parsersLock AND effectsLock must be locked before calling this function
 void OscirenderAudioProcessorEditor::updateCodeEditor() {
     // check if any code editors are visible
     bool visible = false;
@@ -127,22 +127,24 @@ void OscirenderAudioProcessorEditor::updateCodeEditor() {
     resized();
 }
 
-void OscirenderAudioProcessorEditor::fileUpdated(juce::File file) {
+void OscirenderAudioProcessorEditor::fileUpdated(std::unique_ptr<juce::File> file) {
     lua.setVisible(false);
     obj.setVisible(false);
-    if (file.getFileExtension() == ".lua") {
+    if (file == nullptr) {
+		return;
+	} else if (file->getFileExtension() == ".lua") {
         lua.setVisible(true);
-    } else if (file.getFileExtension() == ".obj") {
+    } else if (file->getFileExtension() == ".obj") {
 		obj.setVisible(true);
 	}
 }
 
-// parsersLock must be locked before calling this function
+// parsersLock AND effectsLock must be locked before calling this function
 void OscirenderAudioProcessorEditor::codeDocumentTextInserted(const juce::String& newText, int insertIndex) {
     updateCodeDocument();
 }
 
-// parsersLock must be locked before calling this function
+// parsersLock AND effectsLock must be locked before calling this function
 void OscirenderAudioProcessorEditor::codeDocumentTextDeleted(int startIndex, int endIndex) {
     updateCodeDocument();
 }
@@ -154,30 +156,41 @@ void OscirenderAudioProcessorEditor::updateCodeDocument() {
 }
 
 bool OscirenderAudioProcessorEditor::keyPressed(const juce::KeyPress& key) {
-    juce::SpinLock::ScopedLockType lock(audioProcessor.parsersLock);
+    juce::SpinLock::ScopedLockType parserLock(audioProcessor.parsersLock);
+    juce::SpinLock::ScopedLockType effectsLock(audioProcessor.effectsLock);
+
     int numFiles = audioProcessor.numFiles();
     int currentFile = audioProcessor.getCurrentFileIndex();
-    bool updated = false;
+    bool changedFile = false;
+    bool consumeKey = true;
     if (key.getTextCharacter() == 'j') {
-        currentFile++;
-        if (currentFile == numFiles) {
-            currentFile = 0;
+        if (numFiles > 1) {
+            currentFile++;
+            if (currentFile == numFiles) {
+                currentFile = 0;
+            }
+            changedFile = true;
         }
-        updated = true;
     } else if (key.getTextCharacter() == 'k') {
-        currentFile--;
-        if (currentFile < 0) {
-            currentFile = numFiles - 1;
-        }
-        updated = true;
-    }
+        if (numFiles > 1) {
+			currentFile--;
+            if (currentFile < 0) {
+				currentFile = numFiles - 1;
+			}
+			changedFile = true;
+		}
+    } else if (key.isKeyCode(juce::KeyPress::escapeKey)) {
+        obj.disableMouseRotation();
+    } else {
+		consumeKey = false;
+	}
 
-    if (updated) {
+    if (changedFile) {
         audioProcessor.changeCurrentFile(currentFile);
-        fileUpdated(audioProcessor.getCurrentFile());
+        fileUpdated(std::make_unique<juce::File>(audioProcessor.getCurrentFile()));
         updateCodeEditor();
         main.updateFileLabel();
     }
 
-    return updated;
+    return consumeKey;
 }

Source/PluginEditor.h

@@ -31,7 +31,7 @@ public:
     void addCodeEditor(int index);
     void removeCodeEditor(int index);
     void updateCodeEditor();
-    void fileUpdated(juce::File file);
+    void fileUpdated(std::unique_ptr<juce::File> file);
 private:
     OscirenderAudioProcessor& audioProcessor;
     

Source/PluginProcessor.cpp

@@ -33,9 +33,11 @@ OscirenderAudioProcessor::OscirenderAudioProcessor()
     {
     producer = std::make_unique<FrameProducer>(*this, std::make_shared<FileParser>());
     producer->startThread();
+    
+    juce::SpinLock::ScopedLockType lock(effectsLock);
 
-	allEffects.push_back(std::make_shared<Effect>(std::make_unique<BitCrushEffect>(), "Bit Crush", "bitCrush"));
-	allEffects.push_back(std::make_shared<Effect>(std::make_unique<BulgeEffect>(), "Bulge", "bulge"));
+    allEffects.push_back(std::make_shared<Effect>(std::make_unique<BitCrushEffect>(), "Bit Crush", "bitCrush"));
+    allEffects.push_back(std::make_shared<Effect>(std::make_unique<BulgeEffect>(), "Bulge", "bulge"));
     allEffects.push_back(std::make_shared<Effect>(std::make_unique<RotateEffect>(), "2D Rotate Speed", "rotateSpeed"));
     allEffects.push_back(std::make_shared<Effect>(std::make_unique<VectorCancellingEffect>(), "Vector cancelling", "vectorCancelling"));
     allEffects.push_back(std::make_shared<Effect>(std::make_unique<DistortEffect>(true), "Vertical shift", "verticalDistort"));
@@ -152,6 +154,7 @@ bool OscirenderAudioProcessor::isBusesLayoutSupported (const BusesLayout& layout
 }
 #endif
 
+// effectsLock should be held when calling this
 void OscirenderAudioProcessor::addLuaSlider() {
     juce::String sliderName = "";
 
@@ -165,6 +168,7 @@ void OscirenderAudioProcessor::addLuaSlider() {
     luaEffects.push_back(std::make_shared<Effect>(std::make_unique<LuaEffect>(sliderName, *this), "Lua " + sliderName, "lua" + sliderName));
 }
 
+// effectsLock should be held when calling this
 void OscirenderAudioProcessor::updateLuaValues() {
     for (auto& effect : luaEffects) {
         effect->apply();
@@ -183,58 +187,44 @@ void OscirenderAudioProcessor::updateAngleDelta() {
 	thetaDelta = cyclesPerSample * 2.0 * juce::MathConstants<double>::pi;
 }
 
+// effectsLock MUST be held when calling this
 void OscirenderAudioProcessor::enableEffect(std::shared_ptr<Effect> effect) {
-	// need to make a new vector because the old one is being iterated over in another thread
-	std::shared_ptr<std::vector<std::shared_ptr<Effect>>> newEffects = std::make_shared<std::vector<std::shared_ptr<Effect>>>();
-	for (auto& e : *enabledEffects) {
-		newEffects->push_back(e);
-	}
 	// remove any existing effects with the same id
-	for (auto it = newEffects->begin(); it != newEffects->end();) {
+	for (auto it = enabledEffects->begin(); it != enabledEffects->end();) {
 		if ((*it)->getId() == effect->getId()) {
-			it = newEffects->erase(it);
+			it = enabledEffects->erase(it);
 		} else {
 			it++;
 		}
 	}
 	// insert according to precedence (sorts from lowest to highest precedence)
-	auto it = newEffects->begin();
-	while (it != newEffects->end() && (*it)->getPrecedence() <= effect->getPrecedence()) {
+	auto it = enabledEffects->begin();
+	while (it != enabledEffects->end() && (*it)->getPrecedence() <= effect->getPrecedence()) {
 		it++;
 	}
-	newEffects->insert(it, effect);
-	enabledEffects = newEffects;
+    enabledEffects->insert(it, effect);
 }
 
+// effectsLock MUST be held when calling this
 void OscirenderAudioProcessor::disableEffect(std::shared_ptr<Effect> effect) {
-	// need to make a new vector because the old one is being iterated over in another thread
-	std::shared_ptr<std::vector<std::shared_ptr<Effect>>> newEffects = std::make_shared<std::vector<std::shared_ptr<Effect>>>();
-	for (auto& e : *enabledEffects) {
-		newEffects->push_back(e);
-	}
 	// remove any existing effects with the same id
-	for (auto it = newEffects->begin(); it != newEffects->end();) {
+	for (auto it = enabledEffects->begin(); it != enabledEffects->end();) {
 		if ((*it)->getId() == effect->getId()) {
-			it = newEffects->erase(it);
+			it = enabledEffects->erase(it);
 		} else {
 			it++;
 		}
 	}
-	enabledEffects = newEffects;
 }
 
+// effectsLock MUST be held when calling this
 void OscirenderAudioProcessor::updateEffectPrecedence() {
-	// need to make a new vector because the old one is being iterated over in another thread
-	std::shared_ptr<std::vector<std::shared_ptr<Effect>>> newEffects = std::make_shared<std::vector<std::shared_ptr<Effect>>>();
-	for (auto& e : *enabledEffects) {
-		newEffects->push_back(e);
-	}
-	std::sort(newEffects->begin(), newEffects->end(), [](std::shared_ptr<Effect> a, std::shared_ptr<Effect> b) {
+	std::sort(enabledEffects->begin(), enabledEffects->end(), [](std::shared_ptr<Effect> a, std::shared_ptr<Effect> b) {
 		return a->getPrecedence() < b->getPrecedence();
     });
-	enabledEffects = newEffects;
 }
 
+// parsersLock AND effectsLock must be locked before calling this function
 void OscirenderAudioProcessor::updateFileBlock(int index, std::shared_ptr<juce::MemoryBlock> block) {
     if (index < 0 || index >= fileBlocks.size()) {
 		return;
@@ -243,6 +233,7 @@ void OscirenderAudioProcessor::updateFileBlock(int index, std::shared_ptr<juce::
 	openFile(index);
 }
 
+// parsersLock AND effectsLock must be locked before calling this function
 void OscirenderAudioProcessor::addFile(juce::File file) {
     fileBlocks.push_back(std::make_shared<juce::MemoryBlock>());
     files.push_back(file);
@@ -252,6 +243,7 @@ void OscirenderAudioProcessor::addFile(juce::File file) {
     openFile(fileBlocks.size() - 1);
 }
 
+// parsersLock AND effectsLock must be locked before calling this function
 void OscirenderAudioProcessor::removeFile(int index) {
 	if (index < 0 || index >= fileBlocks.size()) {
 		return;
@@ -266,18 +258,20 @@ int OscirenderAudioProcessor::numFiles() {
     return fileBlocks.size();
 }
 
+// used for opening NEW files. Should be the default way of opening files as
+// it will reparse any existing files, so it is safer.
+// parsersLock AND effectsLock must be locked before calling this function
 void OscirenderAudioProcessor::openFile(int index) {
 	if (index < 0 || index >= fileBlocks.size()) {
 		return;
 	}
     parsers[index]->parse(files[index].getFileExtension(), std::make_unique<juce::MemoryInputStream>(*fileBlocks[index], false));
-    producer->setSource(parsers[index], index);
-    currentFile = index;
-	invalidateFrameBuffer = true;
-    updateLuaValues();
-    updateObjValues();
+    changeCurrentFile(index);
 }
 
+// used ONLY for changing the current file to an EXISTING file.
+// much faster than openFile(int index) because it doesn't reparse any files.
+// parsersLock AND effectsLock must be locked before calling this function
 void OscirenderAudioProcessor::changeCurrentFile(int index) {
     if (index == -1) {
         currentFile = -1;
@@ -289,6 +283,8 @@ void OscirenderAudioProcessor::changeCurrentFile(int index) {
 	producer->setSource(parsers[index], index);
     currentFile = index;
 	invalidateFrameBuffer = true;
+    updateLuaValues();
+    updateObjValues();
 }
 
 int OscirenderAudioProcessor::getCurrentFileIndex() {
@@ -415,12 +411,13 @@ void OscirenderAudioProcessor::processBlock (juce::AudioBuffer<float>& buffer, j
             double drawingProgress = length == 0.0 ? 1 : shapeDrawn / length;
             channels = shape->nextVector(drawingProgress);
         }
-        
-		auto enabledEffectsCopy = enabledEffects;
 
-		for (auto effect : *enabledEffectsCopy) {
-			channels = effect->apply(sample, channels);
-		}
+        {
+            juce::SpinLock::ScopedLockType lock(effectsLock);
+            for (auto effect : *enabledEffects) {
+                channels = effect->apply(sample, channels);
+            }
+        }
 
 		x = channels.x;
 		y = channels.y;

Source/PluginProcessor.h

@@ -68,9 +68,9 @@ public:
 
     double currentSampleRate = 0.0;
 
+    juce::SpinLock effectsLock;
 	std::vector<std::shared_ptr<Effect>> allEffects;
 	std::shared_ptr<std::vector<std::shared_ptr<Effect>>> enabledEffects = std::make_shared<std::vector<std::shared_ptr<Effect>>>();
-
     std::vector<std::shared_ptr<Effect>> luaEffects;
 
     // TODO see if there is a way to move this code to .cpp

Source/components/EffectsListComponent.cpp

@@ -8,16 +8,22 @@ EffectsListComponent::EffectsListComponent(DraggableListBox& lb, AudioEffectList
         ((AudioEffectListBoxItemData&)modelData).setValue(rowNum, this->effectComponent->slider.getValue());
 	};
 
-    // check if effect is in audioProcessor enabled effects
 	bool isSelected = false;
-	for (auto effect : *data.audioProcessor.enabledEffects) {
-		if (effect->getId() == data.getId(rn)) {
-			isSelected = true;
-			break;
-		}
+
+	{
+		juce::SpinLock::ScopedLockType lock(data.audioProcessor.effectsLock);
+		// check if effect is in audioProcessor enabled effects
+		for (auto effect : *data.audioProcessor.enabledEffects) {
+			if (effect->getId() == data.getId(rn)) {
+				isSelected = true;
+				break;
+			}
+		}
 	}
 	effectComponent->selected.setToggleState(isSelected, juce::dontSendNotification);
 	effectComponent->selected.onClick = [this] {
+		auto data = (AudioEffectListBoxItemData&)modelData;
+		juce::SpinLock::ScopedLockType lock(data.audioProcessor.effectsLock);
 		((AudioEffectListBoxItemData&)modelData).setSelected(rowNum, this->effectComponent->selected.getToggleState());
 	};
 }

Source/components/EffectsListComponent.h

@@ -17,15 +17,19 @@ struct AudioEffectListBoxItemData : public DraggableListBoxItemData
         return data.size();
     }
 
+    // CURRENTLY NOT USED
     void deleteItem(int indexOfItemToDelete) override {
-		data.erase(data.begin() + indexOfItemToDelete);
+		// data.erase(data.begin() + indexOfItemToDelete);
     }
 
+    // CURRENTLY NOT USED
     void addItemAtEnd() override {
         // data.push_back(juce::String("Yahoo"));
     }
 
 	void moveBefore(int indexOfItemToMove, int indexOfItemToPlaceBefore) override {
+        juce::SpinLock::ScopedLockType lock(audioProcessor.effectsLock);
+
 		auto effect = data[indexOfItemToMove];
 
         if (indexOfItemToMove < indexOfItemToPlaceBefore) {
@@ -42,6 +46,8 @@ struct AudioEffectListBoxItemData : public DraggableListBoxItemData
 	}
 
     void moveAfter(int indexOfItemToMove, int indexOfItemToPlaceAfter) override {
+        juce::SpinLock::ScopedLockType lock(audioProcessor.effectsLock);
+
         auto temp = data[indexOfItemToMove];
         
         if (indexOfItemToMove <= indexOfItemToPlaceAfter) {

Source/components/LuaListComponent.cpp

@@ -28,12 +28,14 @@ void LuaListBoxModel::paintListBoxItem(int rowNumber, juce::Graphics& g, int wid
 
 juce::Component* LuaListBoxModel::refreshComponentForRow(int rowNum, bool isRowSelected, juce::Component *existingComponentToUpdate) {
 	if (rowNum < getNumRows() - 1) {
+		juce::SpinLock::ScopedLockType lock(audioProcessor.effectsLock);
 		std::unique_ptr<LuaListComponent> item(dynamic_cast<LuaListComponent*>(existingComponentToUpdate));
 		if (juce::isPositiveAndBelow(rowNum, getNumRows())) {
 			item = std::make_unique<LuaListComponent>(audioProcessor, *audioProcessor.luaEffects[rowNum]);
 		}
 		return item.release();
 	} else {
+		juce::SpinLock::ScopedLockType lock(audioProcessor.effectsLock);
 		std::unique_ptr<juce::TextButton> item(dynamic_cast<juce::TextButton*>(existingComponentToUpdate));
 		item = std::make_unique<juce::TextButton>("+");
 		item->onClick = [this]() {