From ff7301fbc7d3e64ccc52ff65839fced7b313c7e8 Mon Sep 17 00:00:00 2001 From: Emma Broman Date: Tue, 8 Jun 2021 09:00:30 +0200 Subject: [PATCH] Moore cleanup --- .../autonavigation/autonavigationhandler.cpp | 20 ++-- .../curves/avoidcollisioncurve.cpp | 4 +- .../curves/zoomoutoverviewcurve.cpp | 14 +-- modules/autonavigation/path.cpp | 2 +- modules/autonavigation/pathinstruction.cpp | 7 +- modules/autonavigation/pathinstruction.h | 4 +- modules/autonavigation/waypoint.cpp | 99 ++++++++++--------- modules/autonavigation/waypoint.h | 19 +--- 8 files changed, 81 insertions(+), 88 deletions(-) diff --git a/modules/autonavigation/autonavigationhandler.cpp b/modules/autonavigation/autonavigationhandler.cpp index 679942f6b9..52073c0711 100644 --- a/modules/autonavigation/autonavigationhandler.cpp +++ b/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 waypoints = instruction.waypoints(); + std::vector 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(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 AutoNavigationHandler::findRelevantNodes() { @@ -508,8 +509,9 @@ std::vector AutoNavigationHandler::findRelevantNodes() { const std::vector relevantTags = _relevantNodeTags; - if (allNodes.empty() || relevantTags.empty()) - return std::vector{}; + if (allNodes.empty() || relevantTags.empty()) { + return std::vector(); + } auto isRelevant = [&](const SceneGraphNode* node) { const std::vector tags = node->tags(); @@ -528,7 +530,7 @@ std::vector AutoNavigationHandler::findRelevantNodes() { return node->renderable() && (node->boundingSphere() > 0.0); }; - std::vector resultingNodes{}; + std::vector resultingNodes; std::copy_if( allNodes.begin(), allNodes.end(), diff --git a/modules/autonavigation/curves/avoidcollisioncurve.cpp b/modules/autonavigation/curves/avoidcollisioncurve.cpp index 2bf8cb5539..ae86f84d72 100644 --- a/modules/autonavigation/curves/avoidcollisioncurve.cpp +++ b/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 diff --git a/modules/autonavigation/curves/zoomoutoverviewcurve.cpp b/modules/autonavigation/curves/zoomoutoverviewcurve.cpp index a3de584f13..3bfd47c1fa 100644 --- a/modules/autonavigation/curves/zoomoutoverviewcurve.cpp +++ b/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); } diff --git a/modules/autonavigation/path.cpp b/modules/autonavigation/path.cpp index b31cae1b17..5da1b4ea3e 100644 --- a/modules/autonavigation/path.cpp +++ b/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 { diff --git a/modules/autonavigation/pathinstruction.cpp b/modules/autonavigation/pathinstruction.cpp index 38998135f1..6babed7679 100644 --- a/modules/autonavigation/pathinstruction.cpp +++ b/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 PathInstruction::waypoints() const { - return _waypoints; -} } // namespace openspace::autonavigation diff --git a/modules/autonavigation/pathinstruction.h b/modules/autonavigation/pathinstruction.h index 302805a7c1..b5573cd208 100644 --- a/modules/autonavigation/pathinstruction.h +++ b/modules/autonavigation/pathinstruction.h @@ -41,8 +41,6 @@ struct PathInstruction { PathInstruction(const ghoul::Dictionary& dictionary); - std::vector waypoints() const; - static documentation::Documentation Documentation(); Type type; @@ -59,7 +57,7 @@ struct PathInstruction { // Navigation state details NavigationState navigationState; - std::vector _waypoints; + std::vector waypoints; }; } // namespace openspace::autonavigation diff --git a/modules/autonavigation/waypoint.cpp b/modules/autonavigation/waypoint.cpp index a38322814d..80f1d600a5 100644 --- a/modules/autonavigation/waypoint.cpp +++ b/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(node->boundingSphere()); - const double minValidBoundingSphere = - global::moduleEngine->module()->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(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(node->boundingSphere()); + const double minValidBoundingSphere = + global::moduleEngine->module()->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(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 diff --git a/modules/autonavigation/waypoint.h b/modules/autonavigation/waypoint.h index 2be93e9083..cb54e94cd9 100644 --- a/modules/autonavigation/waypoint.h +++ b/modules/autonavigation/waypoint.h @@ -27,7 +27,6 @@ #include #include -#include 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