8

Using the CGAL lib, I'm trying to implement the Shortest Path methods.

I've been kind of successful, but the time it takes to map a path is not nearly acceptable, taking up to 1.5 seconds running in Release.

I'm aware that the input might be overwhelmingly big, having 50000 faces, but that is what I have to work with.

To be more detailed on what I'm trying to do is being able to draw a spline along the surface of a mesh by clicking in two different places and generating a path from them just like in the image:enter image description here

My type definitions are:

typedef CGAL::Exact_predicates_inexact_constructions_kernel Kernel;
typedef CGAL::Surface_mesh<Kernel::Point_3> Triangle_mesh;
typedef CGAL::Surface_mesh_shortest_path_traits<Kernel, Triangle_mesh> Traits;
// default property maps
typedef boost::property_map<Triangle_mesh,
    boost::vertex_external_index_t>::type  Vertex_index_map;
typedef boost::property_map<Triangle_mesh,
    CGAL::halfedge_external_index_t>::type Halfedge_index_map;
typedef boost::property_map<Triangle_mesh,
    CGAL::face_external_index_t>::type     Face_index_map;
typedef CGAL::Surface_mesh_shortest_path<Traits> Surface_mesh_shortest_path;
typedef boost::graph_traits<Triangle_mesh> Graph_traits;
typedef Graph_traits::vertex_iterator vertex_iterator;
typedef Graph_traits::halfedge_iterator halfedge_iterator;
typedef Graph_traits::face_iterator face_iterator;

My code looks like the following:

Traits::Barycentric_coordinates src_face_location = { { p1.barycentric[2], p1.barycentric[0], p1.barycentric[1] } };
face_iterator src_face_it = faces(map->m_cgal_mesh).first;
std::advance(src_face_it, src_faceIndex);

map->m_shortest_paths->remove_all_source_points();
map->m_shortest_paths->add_source_point(*src_face_it, src_face_location);

Traits::Barycentric_coordinates dest_face_location = { { p2.barycentric[2], p2.barycentric[0], p2.barycentric[1] } };
face_iterator dest_face_it = faces(map->m_cgal_mesh).first;
std::advance(dest_face_it, dest_faceIndex);

std::vector<Traits::Point_3> cgal_points;
auto r = map->m_shortest_paths->shortest_path_points_to_source_points(*dest_face_it, dest_face_location, std::back_inserter(cgal_points));

points.resize(cgal_points.size(), 3);

for (int i = 0; i < cgal_points.size(); ++i) {
    auto const& p = cgal_points[i];
    points.row(i) = RowVector3d(p.x(), p.y(), p.z());
}

The process that takes 99% of the total time is on this line:

auto r = map->m_shortest_paths->shortest_path_points_to_source_points(*dest_face_it, dest_face_location, std::back_inserter(cgal_points));

Any idea on how to improve performance?

phuclv
  • 37,963
  • 15
  • 156
  • 475
Alexandre Severino
  • 1,563
  • 1
  • 16
  • 38
  • 3
    For working code you probably want https://codereview.stackexchange.com – Jesper Juhl Oct 18 '18 at 20:10
  • That is complicated, since they don't even have a tag for CGAL. – Alexandre Severino Oct 18 '18 at 20:11
  • @JesperJuhl: Nope - this is OK here. Fundamentally, this is an algorithm question. Those are on-topic here. And the other things we're looking for are also present: current code and a description of the problem (=slow). And it's even an interesting problem. Do you always have a convex object? A common trick to speeding up generic algorithms is to exploit a more limited input space, and I suspect convexity matters. – MSalters Oct 18 '18 at 20:33
  • "Any idea on how to improve performance?" - Step 1 would be to make sure you are building with optimization enabled. – Jesper Juhl Oct 18 '18 at 20:33
  • @MSalters You'll note I did *not* downvote the question.. – Jesper Juhl Oct 18 '18 at 20:34
  • @JesperJuhl Yes. They are always convex. Unfortunately, CGAL requires the entire mesh as input, which makes it difficult to limit the search area for the path finding. Besides, It could be very troublesome on objects such as a torus or a "serpent" style geometry. – Alexandre Severino Oct 18 '18 at 22:32
  • @JesperJuhl I'm using /O2. Would anything else help? – Alexandre Severino Oct 18 '18 at 22:35
  • What is taking time is the first query because the whole data structure for your set of source points is built. If you only have one query then you pay the high price. Is your input always convex? – sloriot Oct 18 '18 at 23:47
  • 1
    @sloriot yes. The inputs are always convex. I don't see any alternative solution to avoid rebuild of the data structure, since the source points change every time. – Alexandre Severino Oct 19 '18 at 12:31
  • 1
    Have you considered rolling your own shortest path implementation? In this case reinventing the wheel might be just what the doctor ordered because CGAL is generic and feature-rich, while yours is a simple case and you have special knowledge. Additionally, shortest path algorithms are usually straightforward to implement. The C developer in me wants to say that this can be done less verbosely. –  Nov 12 '18 at 19:04

2 Answers2

5

The CGAL docs state the shortest route is always a straight line when you would unfold the mesh on a 2D plane. The input for the shortest path algorithm is a vertex or plane with barycentric coordinates. You could map these input coordinates to a 2D texture which was mapped on your mesh. Draw a red line between start and end point on your texture. You will have to dig deeper on how to translate the vertices input coordinates into absolute XY coordinates in the texture. Also keep in mind that the shortest path could be running over the back of the mesh. Depending on how the texture is mapped it could be possible that you need to draw more than 1 line.

3

It is pretty clear from the documention. You need to call build_sequence_tree first. My suggestion is you put in this performance hit somewhere before the user clicks the destination - this could be when the source is selected first, so that it is not felt when the user starts clicking around. Even better if you can find a way to safely run this in the background.

2.1.3 Building the Internal Sequence Tree

A time consuming operation for shortest path queries consists in building an internal data structure used to make the queries. This data structure is called the sequence tree. It will be built automatically when the first shortest path query is done and will be reused for any subsequent query as long as the set of source points does not change. Each time the set of source points is changed the sequence tree needs to be rebuilt (if already built). Note that it can also be built manually by a call to Surface_mesh_shortest_path::build_sequence_tree().

https://doc.cgal.org/latest/Surface_mesh_shortest_path/index.html

Additional, it looks like the algorithm runs in worst case polynomial time. As others have pointed it could potentially be optimized if you know your problem is convex in all cases.

darune
  • 10,480
  • 2
  • 24
  • 62
  • It's not clear if that's possible, see the above comment: I don't see any alternative solution to avoid rebuild of the data structure, since the source points change every time. – Frederik De Ruyck Nov 15 '18 at 15:50
  • @FrederikDeRuyck It is actually pretty clear, OP writes: "by clicking in two different places" – darune Nov 16 '18 at 14:17
  • I meant the poster wrote in an OP comment that the source points change every time. I believe that means the sequence tree needs to be rebuilt every time but it's not clear because the OP doesn't mention this. – Frederik De Ruyck Nov 19 '18 at 15:40
  • @FrederikDeRuyck but even so, the point is that the performance hit can be taken when the source is selected - the time the user then spends to select the dest is then effectively subtracted. If its possible to run in background then he could be looking at a solution where it will only be noticeable if the user clicks really fast. – darune Nov 20 '18 at 10:52