Moore cleanup

modules/autonavigation/autonavigationhandler.cpp

@@ -224,7 +224,7 @@ void AutoNavigationHandler::createPath(PathInstruction& instruction) {
     // TODO: Improve how curve types are handled
     const int curveType = _defaultCurveOption;
 
-    std::vector<Waypoint> waypoints = instruction.waypoints();
+    std::vector<Waypoint> waypoints = instruction.waypoints;
     Waypoint waypointToAdd;
 
     if (waypoints.empty()) {
@@ -411,14 +411,15 @@ void AutoNavigationHandler::removeRollRotation(CameraPose& pose, double deltaTim
 
 // Test if the node lies within a given proximity radius of any relevant node in the scene
 SceneGraphNode* AutoNavigationHandler::findNodeNearTarget(const SceneGraphNode* node) {
-    glm::dvec3 nodePos = node->worldPosition();
-    std::string nodeId = node->identifier();
+    const glm::dvec3 nodePos = node->worldPosition();
+    const std::string nodeId = node->identifier();
 
     const float proximityRadiusFactor = 3.f;
 
     for (SceneGraphNode* n : _relevantNodes) {
-        if (n->identifier() == nodeId)
+        if (n->identifier() == nodeId) {
             continue;
+        }
 
         float bs = static_cast<float>(n->boundingSphere());
         float proximityRadius = proximityRadiusFactor * bs;
@@ -480,7 +481,7 @@ Waypoint AutoNavigationHandler::computeDefaultWaypoint(const PathInstruction& in
         stepDirection = glm::normalize(offsetRotation * targetToSun);
     }
 
-    const double radius = WaypointNodeDetails::findValidBoundingSphere(targetNode);
+    const double radius = Waypoint::findValidBoundingSphere(targetNode);
     const double defaultHeight = 2.0 * radius;
     const double height = ins.height.has_value() ? ins.height.value() : defaultHeight;
 
@@ -499,7 +500,7 @@ Waypoint AutoNavigationHandler::computeDefaultWaypoint(const PathInstruction& in
     const glm::dvec3 lookAtPos = targetNode->worldPosition();
     const glm::dquat targetRot = helpers::lookAtQuaternion(targetPos, lookAtPos, up);
 
-    return Waypoint{ targetPos, targetRot, ins.nodeIdentifier };
+    return Waypoint(targetPos, targetRot, ins.nodeIdentifier);
 }
 
 std::vector<SceneGraphNode*> AutoNavigationHandler::findRelevantNodes() {
@@ -508,8 +509,9 @@ std::vector<SceneGraphNode*> AutoNavigationHandler::findRelevantNodes() {
 
     const std::vector<std::string> relevantTags = _relevantNodeTags;
 
-    if (allNodes.empty() || relevantTags.empty())
-        return std::vector<SceneGraphNode*>{};
+    if (allNodes.empty() || relevantTags.empty()) {
+        return std::vector<SceneGraphNode*>();
+    }
 
     auto isRelevant = [&](const SceneGraphNode* node) {
         const std::vector<std::string> tags = node->tags();
@@ -528,7 +530,7 @@ std::vector<SceneGraphNode*> AutoNavigationHandler::findRelevantNodes() {
         return node->renderable() && (node->boundingSphere() > 0.0);
     };
 
-    std::vector<SceneGraphNode*> resultingNodes{};
+    std::vector<SceneGraphNode*> resultingNodes;
     std::copy_if(
         allNodes.begin(),
         allNodes.end(),

modules/autonavigation/curves/avoidcollisioncurve.cpp

@@ -54,8 +54,8 @@ AvoidCollisionCurve::AvoidCollisionCurve(const Waypoint& start, const Waypoint&
 
     const glm::dvec3 startNodeCenter = start.node()->worldPosition();
     const glm::dvec3 endNodeCenter = end.node()->worldPosition();
-    const double startNodeRadius = start.nodeDetails.validBoundingSphere;
-    const double endNodeRadius = end.nodeDetails.validBoundingSphere;
+    const double startNodeRadius = start.validBoundingSphere;
+    const double endNodeRadius = end.validBoundingSphere;
     const glm::dvec3 startViewDir = start.rotation() * glm::dvec3(0.0, 0.0, -1.0);
 
     // Add control points for a catmull-rom spline, first and last will not be intersected

modules/autonavigation/curves/zoomoutoverviewcurve.cpp

@@ -44,8 +44,8 @@ namespace openspace::autonavigation {
 
 // Go far out to get a view of both tagets, aimed to match lookAt orientation
 ZoomOutOverviewCurve::ZoomOutOverviewCurve(const Waypoint& start, const Waypoint& end) {
-    const double startNodeRadius = start.nodeDetails.validBoundingSphere;
-    const double endNodeRadius = end.nodeDetails.validBoundingSphere;
+    const double startNodeRadius = start.validBoundingSphere;
+    const double endNodeRadius = end.validBoundingSphere;
 
     const double endTangentsLengthFactor = 2.0;
     const double startTangentLength = endTangentsLengthFactor * startNodeRadius;
@@ -60,8 +60,8 @@ ZoomOutOverviewCurve::ZoomOutOverviewCurve(const Waypoint& start, const Waypoint
     _points.push_back(start.position());
     _points.push_back(start.position() + startTangentLength * startTangentDir);
 
-    const std::string& startNode = start.nodeDetails.identifier;
-    const std::string& endNode = end.nodeDetails.identifier;
+    const std::string& startNode = start.nodeIdentifier;
+    const std::string& endNode = end.nodeIdentifier;
 
     // Zoom out
     if (startNode != endNode) {
@@ -92,10 +92,12 @@ ZoomOutOverviewCurve::ZoomOutOverviewCurve(const Waypoint& start, const Waypoint
 }
 
 glm::dvec3 ZoomOutOverviewCurve::interpolate(double u) {
-    if (u <= 0.0)
+    if (u <= 0.0) {
         return _points[0];
-    if (u > 1.0)
+    }
+    if (u > 1.0) {
         return _points.back();
+    }
 
     return interpolation::piecewiseCubicBezier(u, _points, _parameterIntervals);
 }

modules/autonavigation/path.cpp

@@ -102,7 +102,7 @@ CameraPose Path::traversePath(double dt) {
 
 std::string Path::currentAnchor() const {
     bool pastHalfway = (_traveledDistance / pathLength()) > 0.5;
-    return (pastHalfway) ? _end.nodeDetails.identifier : _start.nodeDetails.identifier;
+    return (pastHalfway) ? _end.nodeIdentifier : _start.nodeIdentifier;
 }
 
 bool Path::hasReachedEnd() const {

modules/autonavigation/pathinstruction.cpp

@@ -92,7 +92,7 @@ PathInstruction::PathInstruction(const ghoul::Dictionary& dictionary) {
             }
 
             navigationState = NavigationState(p.navigationState.value());
-            _waypoints = { Waypoint(navigationState) };
+            waypoints = { Waypoint(navigationState) };
             break;
         }
         case Parameters::Type::Node: {
@@ -135,7 +135,7 @@ PathInstruction::PathInstruction(const ghoul::Dictionary& dictionary) {
                 const glm::dquat targetRot = helpers::lookAtQuaternion(targetPos, lookAtPos, up);
 
                 Waypoint wp{ targetPos, targetRot, nodeIdentifier };
-                _waypoints = { wp };
+                waypoints = { wp };
             }
             break;
         }
@@ -147,8 +147,5 @@ PathInstruction::PathInstruction(const ghoul::Dictionary& dictionary) {
     }
 }
 
-std::vector<Waypoint> PathInstruction::waypoints() const {
-    return _waypoints;
-}
 
 } // namespace openspace::autonavigation

modules/autonavigation/pathinstruction.h

@@ -41,8 +41,6 @@ struct PathInstruction {
 
     PathInstruction(const ghoul::Dictionary& dictionary);
 
-    std::vector<Waypoint> waypoints() const;
-
     static documentation::Documentation Documentation();
 
     Type type;
@@ -59,7 +57,7 @@ struct PathInstruction {
     // Navigation state details
     NavigationState navigationState;
 
-    std::vector<Waypoint> _waypoints;
+    std::vector<Waypoint> waypoints;
 };
 
 } // namespace openspace::autonavigation

modules/autonavigation/waypoint.cpp

@@ -37,60 +37,20 @@ namespace {
 
 namespace openspace::autonavigation {
 
-WaypointNodeDetails::WaypointNodeDetails(const std::string& nodeIdentifier) {
+Waypoint::Waypoint(const glm::dvec3& pos, const glm::dquat& rot, const std::string& ref)
+    : nodeIdentifier(ref)
+{
+    pose.position = pos;
+    pose.rotation = rot;
+
     const SceneGraphNode* node = sceneGraphNode(nodeIdentifier);
     if (!node) {
         LERROR(fmt::format("Could not find node '{}'.", nodeIdentifier));
         return;
     }
-
-    identifier = nodeIdentifier;
     validBoundingSphere = findValidBoundingSphere(node);
 }
 
-double WaypointNodeDetails::findValidBoundingSphere(const SceneGraphNode* node) {
-    double bs = static_cast<double>(node->boundingSphere());
-    const double minValidBoundingSphere =
-        global::moduleEngine->module<AutoNavigationModule>()->minValidBoundingSphere();
-
-    if (bs < minValidBoundingSphere) {
-        // If the bs of the target is too small, try to find a good value in a child node.
-        // Only check the closest children, to avoid deep traversal in the scene graph.
-        // Also, the possibility to find a bounding sphere represents the visual size of
-        // the target well is higher for these nodes.
-        for (SceneGraphNode* child : node->children()) {
-            bs = static_cast<double>(child->boundingSphere());
-            if (bs > minValidBoundingSphere) {
-                LWARNING(fmt::format(
-                    "The scene graph node '{}' has no, or a very small, bounding sphere. "
-                    "Using bounding sphere of child node '{}' in computations.",
-                    node->identifier(),
-                    child->identifier()
-                ));
-
-                return bs;
-            }
-        }
-
-        LWARNING(fmt::format(
-            "The scene graph node '{}' has no, or a very small, bounding sphere. Using "
-            "minimal value. This might lead to unexpected results.",
-            node->identifier())
-        );
-
-        bs = minValidBoundingSphere;
-    }
-
-    return bs;
-}
-
-Waypoint::Waypoint(const glm::dvec3& pos, const glm::dquat& rot, const std::string& ref)
-    : nodeDetails(ref)
-{
-    pose.position = pos;
-    pose.rotation = rot;
-}
-
 Waypoint::Waypoint(const NavigationState& ns) {
     // OBS! The following code is exactly the same as used in
     // NavigationHandler::applyNavigationState. Should probably be made into a function.
@@ -128,7 +88,14 @@ Waypoint::Waypoint(const NavigationState& ns) {
 
     pose.rotation = neutralCameraRotation * yawRotation * pitchRotation;
 
-    nodeDetails = WaypointNodeDetails{ ns.anchor };
+    nodeIdentifier = ns.anchor;
+
+    const SceneGraphNode* node = sceneGraphNode(nodeIdentifier);
+    if (!node) {
+        LERROR(fmt::format("Could not find node '{}'.", nodeIdentifier));
+        return;
+    }
+    validBoundingSphere = findValidBoundingSphere(node);
 }
 
 glm::dvec3 Waypoint::position() const {
@@ -140,7 +107,43 @@ glm::dquat Waypoint::rotation() const {
 }
 
 SceneGraphNode* Waypoint::node() const {
-    return sceneGraphNode(nodeDetails.identifier);
+    return sceneGraphNode(nodeIdentifier);
+}
+
+double Waypoint::findValidBoundingSphere(const SceneGraphNode* node) {
+    double bs = static_cast<double>(node->boundingSphere());
+    const double minValidBoundingSphere =
+        global::moduleEngine->module<AutoNavigationModule>()->minValidBoundingSphere();
+
+    if (bs < minValidBoundingSphere) {
+        // If the bs of the target is too small, try to find a good value in a child node.
+        // Only check the closest children, to avoid deep traversal in the scene graph.
+        // Also, the possibility to find a bounding sphere represents the visual size of
+        // the target well is higher for these nodes.
+        for (SceneGraphNode* child : node->children()) {
+            bs = static_cast<double>(child->boundingSphere());
+            if (bs > minValidBoundingSphere) {
+                LWARNING(fmt::format(
+                    "The scene graph node '{}' has no, or a very small, bounding sphere. "
+                    "Using bounding sphere of child node '{}' in computations.",
+                    node->identifier(),
+                    child->identifier()
+                ));
+
+                return bs;
+            }
+        }
+
+        LWARNING(fmt::format(
+            "The scene graph node '{}' has no, or a very small, bounding sphere. Using "
+            "minimal value. This might lead to unexpected results.",
+            node->identifier())
+        );
+
+        bs = minValidBoundingSphere;
+    }
+
+    return bs;
 }
 
 } // namespace openspace::autonavigation

modules/autonavigation/waypoint.h

@@ -27,7 +27,6 @@
 
 #include <openspace/interaction/navigationhandler.h>
 #include <ghoul/glm.h>
-#include <vector>
 
 namespace openspace::autonavigation {
 
@@ -36,32 +35,24 @@ struct CameraPose {
     glm::dquat rotation;
 };
 
-// The waypoint node is the anchor or target node.
-struct WaypointNodeDetails {
-    WaypointNodeDetails() = default;
-    WaypointNodeDetails(const std::string& nodeIdentifier);
-
-    static double findValidBoundingSphere(const SceneGraphNode* node);
-
-    std::string identifier;
-    double validBoundingSphere = 0.0; // to be able to handle nodes with faulty bounding spheres
-};
-
 struct Waypoint {
     using NavigationState = interaction::NavigationHandler::NavigationState;
 
-    // TODO: create waypoints from a dictionary
+    // TODO: create waypoints from a dictionary and skip instructions?
 
     Waypoint() = default;
     Waypoint(const glm::dvec3& pos, const glm::dquat& rot, const std::string& ref);
     Waypoint(const NavigationState& ns);
 
+    static double findValidBoundingSphere(const SceneGraphNode* node);
+
     glm::dvec3 position() const;
     glm::dquat rotation() const;
     SceneGraphNode* node() const;
 
     CameraPose pose;
-    WaypointNodeDetails nodeDetails;
+    std::string nodeIdentifier;
+    double validBoundingSphere = 0.0; // to be able to handle nodes with faulty bounding spheres
 };
 
 } // namespace openspace::autonavigation