Implement reference counting for SPICE kernels to shield SPICE from loading kernels multiple times

Passing result of surface intercept method by parameter and returning success status

include/openspace/util/spicemanager.h

@@ -329,7 +329,8 @@ public:
     *   \param directionVector Ray's direction vector. 
     *   \param surfaceIntercept Surface intercept point on the target body. 
     *   \param surfaceVector Vector from observer to intercept point. 
-    *   \return Flag indicating whether intercept was found. 
+    *   \param isVisible Flag indicating whether intercept was found. 
+    *   \return <code>true</code> if not error occurred, <code>false</code> otherwise
 	*   For further details, refer to
 	*   http://naif.jpl.nasa.gov/pub/naif/toolkit_docs/C/cspice/sincpt_c.html
 	*/
@@ -343,7 +344,9 @@ public:
 							 double& targetEpoch,
 							 glm::dvec3& directionVector,
 							 glm::dvec3& surfaceIntercept,
-							 glm::dvec3& surfaceVector) const;
+							 glm::dvec3& surfaceVector,
+                             bool& isVisible
+                             ) const;
 
 	/**
 	*  Determine if a specified ephemeris object is within the
@@ -647,8 +650,9 @@ public:
 
 private:
 	struct KernelInformation {
-		std::string path;
-		KernelIdentifier id;
+		std::string path; /// The path from which the kernel was loaded
+		KernelIdentifier id; /// A unique identifier for each kernel
+        int refCount; /// How many parts loaded this kernel and are interested in it
 	};
 
 	SpiceManager() = default;
@@ -656,7 +660,10 @@ private:
 	SpiceManager& operator=(const SpiceManager& r) = delete;
 	SpiceManager(SpiceManager&& r) = delete;
 
+    /// A list of all loaded kernels
 	std::vector<KernelInformation> _loadedKernels;
+
+    /// The last assigned kernel-id, used to determine the next free kernel id
 	KernelIdentifier _lastAssignedKernel;
 
 	static SpiceManager* _manager;

src/rendering/renderablefov.cpp

@@ -210,9 +210,10 @@ glm::dvec3 RenderableFov::interpolate(glm::dvec3 p0, glm::dvec3 p1, float t){
 // This method is the current bottleneck.
 psc RenderableFov::checkForIntercept(glm::dvec3 ray){
 	double targetEt;
-	bool intercepted = openspace::SpiceManager::ref().getSurfaceIntercept(_fovTarget, _spacecraft, _instrumentID,
+	bool intercepted = false;
+    openspace::SpiceManager::ref().getSurfaceIntercept(_fovTarget, _spacecraft, _instrumentID,
 																	      _frame, _method, _aberrationCorrection, 
-																		  _time, targetEt, ray, ipoint, ivec);
+																		  _time, targetEt, ray, ipoint, ivec, intercepted);
 	_interceptVector = PowerScaledCoordinate::CreatePowerScaledCoordinate(ivec[0], ivec[1], ivec[2]);
 	_interceptVector[3] += 3;
 
@@ -236,16 +237,17 @@ glm::dvec3 RenderableFov::bisection(glm::dvec3 p1, glm::dvec3 p2, double toleran
 	//check if point is on surface
 	double targetEt;
 	glm::dvec3 half = interpolate(p1, p2, 0.5f);
-	bool intercepted = openspace::SpiceManager::ref().getSurfaceIntercept(_fovTarget, _spacecraft, _instrumentID,
+	bool intercepted = false;
+    openspace::SpiceManager::ref().getSurfaceIntercept(_fovTarget, _spacecraft, _instrumentID,
 													                      _frame, _method, _aberrationCorrection, 
-																		  _time, targetEt, half, ipoint, ivec);
+																		  _time, targetEt, half, ipoint, ivec, intercepted);
 	if (glm::distance(_previousHalf, half) < tolerance){
 		_previousHalf = glm::dvec3(0);
 		return half;
 	}
 	_previousHalf = half;
 	//recursive search
-	if (intercepted == false){
+	if (!intercepted){
 		return bisection(p1, half, tolerance);
 	}else{
 		return bisection(half, p2, tolerance);
@@ -458,9 +460,9 @@ void RenderableFov::render(const RenderData& data){
 			// for each FOV vector
 			for (int i = 0; i < 4; i++){
 				// compute surface intercept
-				_interceptTag[i] = openspace::SpiceManager::ref().getSurfaceIntercept(_fovTarget, _spacecraft, _instrumentID,
+				openspace::SpiceManager::ref().getSurfaceIntercept(_fovTarget, _spacecraft, _instrumentID,
 					_frame, _method, _aberrationCorrection,
-					_time, targetEpoch, bounds[i], ipoint, ivec);
+					_time, targetEpoch, bounds[i], ipoint, ivec, _interceptTag[i]);
 				// if not found, use the orthogonal projected point
 				if (!_interceptTag[i]) _projectionBounds[i] = orthogonalProjection(bounds[i]);
 

src/rendering/renderengine.cpp

@@ -893,7 +893,7 @@ void RenderEngine::changeViewPoint(std::string origin) {
 
         return;
     }
-    ghoul_assert(false, "??");
+    ghoul_assert(false, "This function is being misused");
 
 }
 

src/util/spicemanager.cpp

@@ -80,8 +80,6 @@ SpiceManager::KernelIdentifier SpiceManager::loadKernel(const std::string& fileP
 		return KernelFailed;
 	}
 
-	KernelIdentifier kernelId = ++_lastAssignedKernel;
-
 	// We need to set the current directory as meta-kernels are usually defined relative
 	// to the directory they reside in. The directory change is not necessary for regular
 	// kernels
@@ -93,6 +91,22 @@ SpiceManager::KernelIdentifier SpiceManager::loadKernel(const std::string& fileP
 		LERROR("Could not find directory for kernel '" << path << "'");
 		return KernelFailed;
 	}
+
+    auto it = std::find_if(
+        _loadedKernels.begin(),
+        _loadedKernels.end(),
+        [path](const KernelInformation& info) { return info.path == path; });
+
+    if (it != _loadedKernels.end())
+    {
+        it->refCount++;
+        LDEBUG("Kernel '" << path << "' was already loaded. "
+            "New reference count: " << it->refCount);
+        return it->id;
+    }
+
+    KernelIdentifier kernelId = ++_lastAssignedKernel;
+
 	FileSys.setCurrentDirectory(fileDirectory);
 
     LINFO("Loading SPICE kernel '" << path << "'");
@@ -115,7 +129,7 @@ SpiceManager::KernelIdentifier SpiceManager::loadKernel(const std::string& fileP
 	if (hasError)
 		return KernelFailed;
 	else {
-		KernelInformation&& info = { path, std::move(kernelId) };
+		KernelInformation&& info = { path, std::move(kernelId), 1 };
 		_loadedKernels.push_back(info);
 		return kernelId;
 	}
@@ -126,10 +140,18 @@ void SpiceManager::unloadKernel(KernelIdentifier kernelId) {
 		[&kernelId](const KernelInformation& info) { return info.id == kernelId ; });
 
 	if (it != _loadedKernels.end()) {
-		// No need to check for errors as we do not allow empty path names
-        LINFO("Unloading SPICE kernel '" << it->path << "'");
-		unload_c(it->path.c_str());
-		_loadedKernels.erase(it);
+        // If there is only one part interested in the kernel, we can unload it
+        if (it->refCount == 1) {
+		    // No need to check for errors as we do not allow empty path names
+            LINFO("Unloading SPICE kernel '" << it->path << "'");
+		    unload_c(it->path.c_str());
+		    _loadedKernels.erase(it);
+        }
+        else {
+            // Otherwise, we hold on to it, but reduce the reference counter by 1
+            it->refCount--;
+            LDEBUG("Reducing reference counter. New reference count: " << it->refCount);
+        }
 	}
 }
 
@@ -144,9 +166,17 @@ void SpiceManager::unloadKernel(const std::string& filePath) {
 		[&path](const KernelInformation& info) { return info.path == path; });
 
 	if (it != _loadedKernels.end()) {
-        LINFO("Unloading SPICE kernel '" << path << "'");
-		unload_c(path.c_str());
-		_loadedKernels.erase(it);
+        // If there is only one part interested in the kernel, we can unload it
+        if (it->refCount == 1) {
+            LINFO("Unloading SPICE kernel '" << path << "'");
+            unload_c(path.c_str());
+            _loadedKernels.erase(it);
+        }
+        else {
+            // Otherwise, we hold on to it, but reduce the reference counter by 1
+            it->refCount--;
+            LDEBUG("Reducing reference counter. New reference count: " << it->refCount);
+        }
 	}
 }
 
@@ -413,7 +443,10 @@ bool SpiceManager::getSurfaceIntercept(const std::string& target,
 									   double& targetEpoch,
 									   glm::dvec3& directionVector,
 									   glm::dvec3& surfaceIntercept,
-									   glm::dvec3& surfaceVector) const{
+									   glm::dvec3& surfaceVector,
+                                       bool& isVisible
+                                       ) const
+{
 	// make pretty latr
 	double dvec[3], spoint[3], et;
 	glm::dvec3 srfvec;
@@ -446,21 +479,25 @@ bool SpiceManager::getSurfaceIntercept(const std::string& target,
 			 glm::value_ptr(srfvec), 
 			 &found);
 
+    isVisible = (found == SPICETRUE);
+
 	bool hasError = checkForError("Error retrieving surface intercept on target '" + target + "'" +
 								  "viewed from observer '" + observer + "' in " +
 								  "reference frame '" + bodyfixed + "' at time '" +
 								  std::to_string(ephemerisTime) + "'");
 
-	if (convert) frameConversion(srfvec, bodyfixed, referenceFrame, ephemerisTime);
-
-	if (!hasError && found){
+    if (hasError)
+        return false;
 
+	if (convert)
+        frameConversion(srfvec, bodyfixed, referenceFrame, ephemerisTime);
 
+	if (found){
 		memcpy(glm::value_ptr(directionVector), dvec, sizeof(dvec));
 		memcpy(glm::value_ptr(surfaceIntercept), spoint, sizeof(spoint));
 		surfaceVector = srfvec;
 	}
-	return found;
+	return true;
 }
 
 bool SpiceManager::getTargetState(const std::string& target,