Apply remarks from code review with additional cosmethics

This commit is contained in:
tamasmeszaros 2022-06-03 10:08:11 +02:00
parent f9fb7f947d
commit d9ed45be56
2 changed files with 24 additions and 24 deletions

View File

@ -1,6 +1,7 @@
#ifndef ASTAR_HPP #ifndef ASTAR_HPP
#define ASTAR_HPP #define ASTAR_HPP
#include <cmath> // std::isinf() is here
#include <unordered_map> #include <unordered_map>
#include "libslic3r/Point.hpp" #include "libslic3r/Point.hpp"
@ -83,23 +84,23 @@ size_t unique_id(const T &tracer, const TracerNodeT<T> &n)
} // namespace astar_detail } // namespace astar_detail
constexpr size_t UNASSIGNED = size_t(-1); constexpr size_t Unassigned = size_t(-1);
template<class Tracer> template<class Tracer>
struct QNode // Queue node. Keeps track of scores g, and h struct QNode // Queue node. Keeps track of scores g, and h
{ {
TracerNodeT<Tracer> node; // The actual node itself TracerNodeT<Tracer> node; // The actual node itself
size_t queue_id; // Position in the open queue or UNASSIGNED if closed size_t queue_id; // Position in the open queue or Unassigned if closed
size_t parent; // unique id of the parent or UNASSIGNED size_t parent; // unique id of the parent or Unassigned
float g, h; float g, h;
float f() const { return g + h; } float f() const { return g + h; }
QNode(TracerNodeT<Tracer> n = {}, QNode(TracerNodeT<Tracer> n = {},
size_t p = UNASSIGNED, size_t p = Unassigned,
float gval = std::numeric_limits<float>::infinity(), float gval = std::numeric_limits<float>::infinity(),
float hval = 0.f) float hval = 0.f)
: node{std::move(n)}, parent{p}, queue_id{UNASSIGNED}, g{gval}, h{hval} : node{std::move(n)}, parent{p}, queue_id{Unassigned}, g{gval}, h{hval}
{} {}
}; };
@ -143,15 +144,15 @@ bool search_route(const Tracer &tracer,
}, },
LessPred{cached_nodes}); LessPred{cached_nodes});
QNode initial{source, /*parent = */ UNASSIGNED, /*g = */0.f}; QNode initial{source, /*parent = */ Unassigned, /*g = */0.f};
size_t source_id = unique_id(tracer, source); size_t source_id = unique_id(tracer, source);
cached_nodes[source_id] = initial; cached_nodes[source_id] = initial;
qopen.push(source_id); qopen.push(source_id);
size_t goal_id = goal_heuristic(tracer, source) < 0.f ? source_id : size_t goal_id = goal_heuristic(tracer, source) < 0.f ? source_id :
UNASSIGNED; Unassigned;
while (goal_id == UNASSIGNED && !qopen.empty()) { while (goal_id == Unassigned && !qopen.empty()) {
size_t q_id = qopen.top(); size_t q_id = qopen.top();
qopen.pop(); qopen.pop();
QNode &q = cached_nodes[q_id]; QNode &q = cached_nodes[q_id];
@ -160,7 +161,7 @@ bool search_route(const Tracer &tracer,
assert(!std::isinf(q.g)); assert(!std::isinf(q.g));
foreach_reachable(tracer, q.node, [&](const Node &succ_nd) { foreach_reachable(tracer, q.node, [&](const Node &succ_nd) {
if (goal_id != UNASSIGNED) if (goal_id != Unassigned)
return true; return true;
float h = goal_heuristic(tracer, succ_nd); float h = goal_heuristic(tracer, succ_nd);
@ -184,7 +185,7 @@ bool search_route(const Tracer &tracer,
// The cache needs to be updated either way // The cache needs to be updated either way
prev_nd = qsucc_nd; prev_nd = qsucc_nd;
if (queue_id == UNASSIGNED) if (queue_id == decltype(qopen)::invalid_id())
// was in closed or unqueued, rescheduling // was in closed or unqueued, rescheduling
qopen.push(succ_id); qopen.push(succ_id);
else // was in open, updating else // was in open, updating
@ -192,24 +193,23 @@ bool search_route(const Tracer &tracer,
} }
} }
return goal_id != UNASSIGNED; return goal_id != Unassigned;
}); });
} }
// Write the output, do not reverse. Clients can do so if they need to. // Write the output, do not reverse. Clients can do so if they need to.
if (goal_id != UNASSIGNED) { if (goal_id != Unassigned) {
const QNode *q = &cached_nodes[goal_id]; const QNode *q = &cached_nodes[goal_id];
while (!std::isinf(q->g) && q->parent != UNASSIGNED) { while (q->parent != Unassigned) {
assert(!std::isinf(q->g)); // Uninitialized nodes are NOT allowed
*out = q->node; *out = q->node;
++out; ++out;
q = &cached_nodes[q->parent]; q = &cached_nodes[q->parent];
} }
if (std::isinf(q->g)) // Something went wrong
goal_id = UNASSIGNED;
} }
return goal_id != UNASSIGNED; return goal_id != Unassigned;
} }
}} // namespace Slic3r::astar }} // namespace Slic3r::astar

View File

@ -384,11 +384,11 @@ TEST_CASE("Zero heuristic function should result in dijsktra's algo", "[AStar]")
REQUIRE(out.empty()); REQUIRE(out.empty());
// Source node should have it's parent unset // Source node should have it's parent unset
REQUIRE(graph.nodes[0].parent == astar::UNASSIGNED); REQUIRE(graph.nodes[0].parent == astar::Unassigned);
// All other nodes should have their parents set // All other nodes should have their parents set
for (size_t i = 1; i < graph.nodes.size(); ++i) for (size_t i = 1; i < graph.nodes.size(); ++i)
REQUIRE(graph.nodes[i].parent != astar::UNASSIGNED); REQUIRE(graph.nodes[i].parent != astar::Unassigned);
std::array<float, 9> ref_distances = {0.f, 4.f, 12.f, 19.f, 21.f, std::array<float, 9> ref_distances = {0.f, 4.f, 12.f, 19.f, 21.f,
11.f, 9.f, 8.f, 14.f}; 11.f, 9.f, 8.f, 14.f};
@ -398,9 +398,9 @@ TEST_CASE("Zero heuristic function should result in dijsktra's algo", "[AStar]")
for (size_t i = 0, k = 0; i < graph.nodes.size(); ++i, k = 0) { for (size_t i = 0, k = 0; i < graph.nodes.size(); ++i, k = 0) {
GraphTracer::QNode *q = &graph.nodes[i]; GraphTracer::QNode *q = &graph.nodes[i];
REQUIRE(q->g == Approx(ref_distances[i])); REQUIRE(q->g == Approx(ref_distances[i]));
while (k++ < graph.nodes.size() && q->parent != astar::UNASSIGNED) while (k++ < graph.nodes.size() && q->parent != astar::Unassigned)
q = &graph.nodes[q->parent]; q = &graph.nodes[q->parent];
REQUIRE(q->parent == astar::UNASSIGNED); REQUIRE(q->parent == astar::Unassigned);
} }
} }