search

tudelft3d / prepair

Automatic repair of single polygons (according to the OGC Simple Features / ISO19107 rules) using a constrained triangulation.
GNU General Public License v3.0
115 stars 24 forks source link

Fix compatibility with CGAL>=5.0 #37

Closed sgiraudot closed 3 years ago

sgiraudot commented 3 years ago

Some functions of the constrained Delaunay triangulation were removed in CGAL 5.0 and made prepair fail to compile using CGAL>=5.0.

This PR fixes it mainly by introducing a function getContraint() to recover the constraint ID from a pair of vertices.

Note: while it compiles and mostly runs fine, there is most probably a problem with the parts that were called "hacks" and that consisted in inserting and then removing a constraint. I'm not sure why this was done and it's not clear to me if this was to address a bug in CGAL or something else. But I feel like something is broken here, because I get a different output for the example:

    $ ./prepair --isr 2 --wkt "POLYGON((0 0, 10 0, 15 5, 10 0, 10 10, 0 10, 0 0))"
    MULTIPOLYGON (((11 1,11 11,1 11,1 1,11 1)))

My output is:

MULTIPOLYGON (((0 0,10 0,10 10,0 10,0 0)))

The other examples seem to work fine but from what I've observed, they never go into the "hack" sections, which sort of confirms my feeling that these sections become problematic with my fix.

kenohori commented 3 years ago

Could you have a look at the v2 branch? I haven't had the time to finish it, but it should be fully ready for CGAL 5.0. There are some parts that are not implemented yet, so it's not fully ready for a public release.

prepair needs to remove constraints in the cases where two edges partly overlap along a line segment, which result in an even count for a given subconstraint.

sgiraudot commented 3 years ago

I tried the V2 branch and indeed it compiles fine with CGAL 5.0. I could not test the --isr option which, I guess, is part of what's not implemented yet.

kenohori commented 3 years ago

Indeed the --isr option hasn't been implemented yet in v2.

I'll merge the PR so that v1 has a working version in the meantime. Thanks!

sgiraudot commented 3 years ago

As said in my original comment, this PR was only partially correct and makes your code return wrong output when the "hack" sections are called, so I'm not sure it's a good idea to have it merged as such. Then again, I don't know how to fix these sections (and if you're working on your V2, it's maybe useless to spend time fixing the V1?).