delfrrr / delaunator-cpp

A really fast C++ library for Delaunay triangulation of 2D points
MIT License
498 stars 98 forks source link

Restructure #14

Open flippmoke opened 5 years ago

flippmoke commented 5 years ago

This is still a work in progress ⚠️

This is a large amount of changes that was hinted at in #13. I probably put in more then I wanted to in this PR, but I like the way the library is laid out more personally. There is not a large wrapping class here but a series of small structures that all operate inside a single method. The return from this method is just the halfedges and triangles indexes.

auto result = delaunator(coords);

What you input as coords is now changed too, it is a templated point type that you can assign a custom getter and setter. This means that any custom point types now work. Example below:

struct my_fancy_point {
    int64_t my_x;
    int64_t my_y;
    int64_t some_z;
};

template <>
double delaunator::get_x_value(my_fancy_point const& pt) {
    return static_cast<double>(pt.my_x);
}

template <>
double delaunator::get_y_value(my_fancy_point const& pt) {
    return static_cast<double>(pt.my_y);
}

std::vector<my_fancy_point> coords = get_coords();

auto result = delaunator::delaunator(coords);

Internally there are lots of other changes, the hash is now its own struct and the finding items in the cache is pulled out of the main loop into a method. The hull is also pulled out into its own structure as well.

The current performance bottleneck is in the legalize() method. I have commented the biggest culprit of that is caused by a cache miss. This isn't any different in the previous implementation which has a big cache miss here as well when it travels to the coords in that previous version to get the x,y values for the in_circle() function.

screen shot 2018-10-30 at 4 22 04 pm

We might be able to optimize this away further if we wanted to store the x,y values in the triangle index structure, but this would likely be a potentially massive allocation as the number of triangles is rather large.

flippmoke commented 5 years ago

Some more debugging screenshots to share context:

legalize() currently takes up the vast majority of processing time.

screen shot 2018-10-30 at 4 26 54 pm

After that the main loop takes up the second most amount of time, and then finally the sorting of the points is the 3rd.

mourner commented 5 years ago

@flippmoke awesome! Did you manage to get some benchmarks running? I'm very curious to see how much improvement the change to pointers brought.

flippmoke commented 5 years ago

@mourner I am not sure it does currently provide an improvement yet. Still blocked primarily in performance in the single section above, need to analyze current master branch as well for what is the blocker.