Restructure ParamHandle Engine internals.

include/engine/Engine.hpp

@@ -57,12 +57,16 @@ struct Engine {
 	float getParam(Module *module, int paramId);
 	void setSmoothParam(Module *module, int paramId, float value);
 	float getSmoothParam(Module *module, int paramId);
+
+	// ParamHandles
 	void addParamHandle(ParamHandle *paramHandle);
 	void removeParamHandle(ParamHandle *paramHandle);
 	/** Returns the unique ParamHandle for the given paramId */
-	ParamHandle *getParamHandle(Module *module, int paramId);
+	ParamHandle *getParamHandle(int moduleId, int paramId);
+	/** Use getParamHandle(int, int) instead. */
+	DEPRECATED ParamHandle *getParamHandle(Module *module, int paramId);
 	/** Sets the ParamHandle IDs and module pointer.
-	If the given ParamHandle is added to the engine and another ParamHandle points to the same param, unsets that one and replaces it with the given handle.
+	If `overwrite` is true and another ParamHandle points to the same param, unsets that one and replaces it with the given handle.
 	*/
 	void updateParamHandle(ParamHandle *paramHandle, int moduleId, int paramId, bool overwrite = true);
 };

include/engine/ParamHandle.hpp

@@ -11,21 +11,11 @@ namespace engine {
 /** A weak handle to a Param. Managed by Engine */
 struct ParamHandle {
 	/** Do not set these directly.
-	They are handled by Engine methods.
+	Use Engine ParamHandle methods.
 	*/
-	int moduleId;
-	int paramId;
-	Module *module;
-
-	ParamHandle() {
-		reset();
-	}
-
-	void reset() {
-		moduleId = -1;
-		paramId = 0;
-		module = NULL;
-	}
+	int moduleId = -1;
+	int paramId = 0;
+	Module *module = NULL;
 };
 
 

src/app/ParamWidget.cpp

@@ -100,7 +100,7 @@ struct ParamFineItem : ui::MenuItem {
 struct ParamUnmapItem : ui::MenuItem {
 	ParamWidget *paramWidget;
 	void onAction(const event::Action &e) override {
-		engine::ParamHandle *paramHandle = APP->engine->getParamHandle(paramWidget->paramQuantity->module, paramWidget->paramQuantity->paramId);
+		engine::ParamHandle *paramHandle = APP->engine->getParamHandle(paramWidget->paramQuantity->module->id, paramWidget->paramQuantity->paramId);
 		if (paramHandle) {
 			APP->engine->updateParamHandle(paramHandle, -1, 0);
 		}
@@ -126,7 +126,7 @@ void ParamWidget::draw(const DrawArgs &args) {
 	Widget::draw(args);
 
 	// Param map indicator
-	engine::ParamHandle *paramHandle = paramQuantity ? APP->engine->getParamHandle(paramQuantity->module, paramQuantity->paramId) : NULL;
+	engine::ParamHandle *paramHandle = paramQuantity ? APP->engine->getParamHandle(paramQuantity->module->id, paramQuantity->paramId) : NULL;
 	if (paramHandle) {
 			NVGcolor color = nvgRGB(0xff, 0x40, 0xff);
 			nvgBeginPath(args.vg);
@@ -210,7 +210,7 @@ void ParamWidget::createContextMenu() {
 	// fineItem->disabled = true;
 	// menu->addChild(fineItem);
 
-	engine::ParamHandle *paramHandle = paramQuantity ? APP->engine->getParamHandle(paramQuantity->module, paramQuantity->paramId) : NULL;
+	engine::ParamHandle *paramHandle = paramQuantity ? APP->engine->getParamHandle(paramQuantity->module->id, paramQuantity->paramId) : NULL;
 	if (paramHandle) {
 		ParamUnmapItem *unmapItem = new ParamUnmapItem;
 		unmapItem->text = "Unmap";

src/engine/Engine.cpp

@@ -170,7 +170,8 @@ struct EngineWorker {
 struct Engine::Internal {
 	std::vector<Module*> modules;
 	std::vector<Cable*> cables;
-	std::vector<ParamHandle*> paramHandles;
+	std::set<ParamHandle*> paramHandles;
+	std::map<std::tuple<int, int>, ParamHandle*> paramHandleCache;
 	bool paused = false;
 
 	bool running = false;
@@ -216,6 +217,7 @@ Engine::~Engine() {
 	assert(internal->cables.empty());
 	assert(internal->modules.empty());
 	assert(internal->paramHandles.empty());
+	assert(internal->paramHandleCache.empty());
 
 	delete internal;
 }
@@ -509,7 +511,7 @@ void Engine::addModule(Module *module) {
 	internal->modules.push_back(module);
 	// Trigger Add event
 	module->onAdd();
-	// Update ParamHandles
+	// Update ParamHandles' module pointers
 	for (ParamHandle *paramHandle : internal->paramHandles) {
 		if (paramHandle->moduleId == module->id)
 			paramHandle->module = module;
@@ -532,7 +534,7 @@ void Engine::removeModule(Module *module) {
 		assert(cable->outputModule != module);
 		assert(cable->inputModule != module);
 	}
-	// Update ParamHandles
+	// Update ParamHandles' module pointers
 	for (ParamHandle *paramHandle : internal->paramHandles) {
 		if (paramHandle->moduleId == module->id)
 			paramHandle->module = NULL;
@@ -715,69 +717,97 @@ float Engine::getSmoothParam(Module *module, int paramId) {
 	return getParam(module, paramId);
 }
 
+static void Engine_refreshParamHandleCache(Engine *that) {
+	// Clear cache
+	that->internal->paramHandleCache.clear();
+	// Add active ParamHandles to cache
+	for (ParamHandle *paramHandle : that->internal->paramHandles) {
+		if (paramHandle->moduleId >= 0) {
+			that->internal->paramHandleCache[{paramHandle->moduleId, paramHandle->paramId}] = paramHandle;
+		}
+	}
+}
+
 void Engine::addParamHandle(ParamHandle *paramHandle) {
 	VIPLock vipLock(internal->vipMutex);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
 
+	// New ParamHandles must be blank.
+	// This means we don't have to refresh the cache.
+	assert(paramHandle->moduleId < 0);
+
 	// Check that the ParamHandle is not already added
-	auto it = std::find(internal->paramHandles.begin(), internal->paramHandles.end(), paramHandle);
+	auto it = internal->paramHandles.find(paramHandle);
 	assert(it == internal->paramHandles.end());
 
-	// New ParamHandles must be blank
-	assert(paramHandle->moduleId < 0);
-	internal->paramHandles.push_back(paramHandle);
+	// Add it
+	internal->paramHandles.insert(paramHandle);
 }
 
 void Engine::removeParamHandle(ParamHandle *paramHandle) {
 	VIPLock vipLock(internal->vipMutex);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
 
-	paramHandle->module = NULL;
 	// Check that the ParamHandle is already added
-	auto it = std::find(internal->paramHandles.begin(), internal->paramHandles.end(), paramHandle);
+	auto it = internal->paramHandles.find(paramHandle);
 	assert(it != internal->paramHandles.end());
+
+	// Remove it
+	paramHandle->module = NULL;
 	internal->paramHandles.erase(it);
+	Engine_refreshParamHandleCache(this);
 }
 
-ParamHandle *Engine::getParamHandle(Module *module, int paramId) {
-	// VIPLock vipLock(internal->vipMutex);
-	// std::lock_guard<std::recursive_mutex> lock(internal->mutex);
+ParamHandle *Engine::getParamHandle(int moduleId, int paramId) {
+	// Don't lock because this method is called potentially thousands of times per screen frame.
 
-	for (ParamHandle *paramHandle : internal->paramHandles) {
-		if (paramHandle->module == module && paramHandle->paramId == paramId)
-			return paramHandle;
-	}
-	return NULL;
+	auto it = internal->paramHandleCache.find({moduleId, paramId});
+	if (it == internal->paramHandleCache.end())
+		return NULL;
+	return it->second;
+}
+
+ParamHandle *Engine::getParamHandle(Module *module, int paramId) {
+	return getParamHandle(module->id, paramId);
 }
 
 void Engine::updateParamHandle(ParamHandle *paramHandle, int moduleId, int paramId, bool overwrite) {
 	VIPLock vipLock(internal->vipMutex);
 	std::lock_guard<std::recursive_mutex> lock(internal->mutex);
 
+	// Check that it exists
+	auto it = internal->paramHandles.find(paramHandle);
+	assert(it != internal->paramHandles.end());
+
 	// Set IDs
 	paramHandle->moduleId = moduleId;
 	paramHandle->paramId = paramId;
 	paramHandle->module = NULL;
-
-	auto it = std::find(internal->paramHandles.begin(), internal->paramHandles.end(), paramHandle);
-
-	if (it != internal->paramHandles.end() && paramHandle->moduleId >= 0) {
-		// Remove existing ParamHandles pointing to the same param
-		for (ParamHandle *p : internal->paramHandles) {
-			if (p != paramHandle && p->moduleId == moduleId && p->paramId == paramId) {
-				if (overwrite)
-					p->reset();
-				else
-					paramHandle->reset();
+	// At this point, the ParamHandle cache might be invalid.
+
+	if (paramHandle->moduleId >= 0) {
+		// Replace old ParamHandle, or reset the current ParamHandle
+		ParamHandle *oldParamHandle = getParamHandle(moduleId, paramId);
+		if (oldParamHandle) {
+			if (overwrite) {
+				oldParamHandle->moduleId = -1;
+				oldParamHandle->paramId = 0;
+				oldParamHandle->module = NULL;
 			}
-		}
-		// Find module with same moduleId
-		for (Module *module : internal->modules) {
-			if (module->id == paramHandle->moduleId) {
-				paramHandle->module = module;
+			else {
+				paramHandle->moduleId = -1;
+				paramHandle->paramId = 0;
+				paramHandle->module = NULL;
 			}
 		}
 	}
+
+	// Set module pointer if the above block didn't reset it
+	if (paramHandle->moduleId >= 0) {
+		paramHandle->module = getModule(paramHandle->moduleId);
+	}
+
+	Engine_refreshParamHandleCache(this);
 }