Fix avoid polygons again

[nilsnolde]

Fixes #3182

The issue was a little subtle: we register std::vector<PointLL> as a bg::polygon so the edge's shape was interpreted as a polygon geometry and then used to intersect with the avoid_polygons, which I guess is sorta undefined behavior (at least for me).

I also added a to_geojson() for debugging as you suggested @kevinkreiser, as a trace log, very helpful..

Could be done a bit different, by registering another structure with bg, e.g. std::deque<PointLL>, to avoid casting every edge to a linestring. With a few experiments I didn't see any significant performance deviation from master though, but let me know if you'd prefer that. that only works if we register std::vector as linestring, of course bg::intersects needs two geometry types. For polygons we wouldn't need to register our own geometry type, we could use bg::model::polygon, but the Tiles::Intersect method somehow won't take that type as an argument.

[nilsnolde]

that's the actual change

[nilsnolde]

@purew maybe this also fixes the clang issues over at #2750 (comment) ?

[purew]

@purew maybe this also fixes the clang issues over at #2750 (comment) ?

Initially getting one error

../src/loki/polygon_search.cc:31:19: error: unused variable 'Haversine' [-Werror,-Wunused-const-variable]
static const auto Haversine = [] {
                  ^
1 error generated.
[17/81] Building CXX object src/meili/CMakeFiles/valhalla-meili.dir/map_matcher.cc.o

Fixing that, I still get the error in the unittest:

[ RUN      ] AvoidTest.TestAvoidShortcutsTruck                                                                                         ../test/gurka/test_avoids.cc:214: Failure                        
Expected equality of these values:                                                                                                     
  avoid_edges.size()                                                                                                                   
    Which is: 0                                                                                                                        
  4                                                                                                                                    [  FAILED  ] AvoidTest.TestAvoidShortcutsTruck (5 ms)

[nilsnolde]

@kevinkreiser thanks for the review, I added haversine perimeter calculation and removed our own ring closing logic when parsing the request.

@purew the unused-variable warning is gone, forgot to add that function as a strategy.. No idea what to make of that failing test, maybe I'll try clang one of these days

[kevinkreiser]

this looks good to me. are we saying that compiling with clang fails the tests though?

[nilsnolde]

clang fails the tests

seems so @kevinkreiser . though that's not new with this PR, @purew reported it a couple of weeks back in that initial PR. Must've somehow been introduced with clang 12.0, no idea what's so compiler-specific about that test.. I'll open another issue about it.

[purew]

this looks good to me. are we saying that compiling with clang fails the tests though?

Yea, I'm seeing this I haven't dug into why this is though.