CGAL / cgal-swig-bindings

CGAL bindings using SWIG
Other
338 stars 92 forks source link

insert_constraint error on Constrained_Delaunay_Triangulation_plus_2 #210

Open Jiunixo opened 2 years ago

Jiunixo commented 2 years ago

Hello, We have a project using CGAL through cgal-bindings with CGAL 4.14.3. While upgrading the project to CGAL 5.4, we encountered an error in CDTP2.insert_constraint().

I reproduced the issue by adapting the example of the wiki conforming.py. (The original example works perfectly in our environment).

Code to reproduce failure :

from CGAL.CGAL_Kernel import Point_2
from CGAL.CGAL_Triangulation_2 import Constrained_Delaunay_triangulation_plus_2
from CGAL.CGAL_Triangulation_2 import Constrained_Delaunay_triangulation_plus_2_Vertex_handle
from CGAL import CGAL_Mesh_2
cdt=Constrained_Delaunay_triangulation_plus_2()
#construct a constrained triangulation
va = cdt.insert(Point_2( 5., 5.))
vb = cdt.insert(Point_2(-5., 5.))
vc = cdt.insert(Point_2( 4., 3.))
vd = cdt.insert(Point_2( 5.,-5.))
ve = cdt.insert(Point_2( 6., 6.))
vf = cdt.insert(Point_2(-6., 6.))
vg = cdt.insert(Point_2(-6.,-6.))
vh = cdt.insert(Point_2( 6.,-6.))
cdt.insert_constraint(va,vb) # => ERROR
cdt.insert_constraint(vb,vc)
cdt.insert_constraint(vc,vd)
cdt.insert_constraint(vd,va)
cdt.insert_constraint(ve,vf)
cdt.insert_constraint(vf,vg)
cdt.insert_constraint(vg,vh)
cdt.insert_constraint(vh,ve)
print("Number of vertices before: {}".format(cdt.number_of_vertices()))
#make it conforming Delaunay
CGAL_Mesh_2.make_conforming_Delaunay_2(cdt)
print("Number of vertices after make_conforming_Delaunay_2: {}".format(cdt.number_of_vertices()))
#then make it conforming Gabriel
CGAL_Mesh_2.make_conforming_Gabriel_2(cdt)

print("Number of vertices after make_conforming_Gabriel_2: {}".format(cdt.number_of_vertices()))

Error message : TypeError: 'Constrained_Delaunay_triangulation_plus_2_Vertex_handle' object is not iterable Additional information: Wrong number or type of arguments for overloaded function 'Constrained_Delaunay_triangulation_plus_2_insert_constraint'. Possible C/C++ prototypes are: Constrained_triangulation_plus_2_wrapper< CGAL_CDTplus2,Constrained_Delaunay_triangulation_2_wrapper< CGAL_CDTplus2,SWIG_Triangulation_2::CGAL_Vertex_handle< CGAL_CDTplus2,Point_2 >,SWIG_Triangulation_2::CGAL_Face_handle< CGAL_CDTplus2,Point_2 > >,SWIG_Triangulation_2::CGAL_Vertex_handle< CGAL_CDTplus2,Point_2 > >::insert_constraint(Polygon_2 const &) Constrained_triangulation_plus_2_wrapper< CGAL_CDTplus2,Constrained_Delaunay_triangulation_2_wrapper< CGAL_CDTplus2,SWIG_Triangulation_2::CGAL_Vertex_handle< CGAL_CDTplus2,Point_2 >,SWIG_Triangulation_2::CGAL_Face_handle< CGAL_CDTplus2,Point_2 > >,SWIG_Triangulation_2::CGAL_Vertex_handle< CGAL_CDTplus2,Point_2 > >::insert_constraint(Wrapper_iterator_helper< Point_2 >::input,bool) Constrained_triangulation_plus_2_wrapper< CGAL_CDTplus2,Constrained_Delaunay_triangulation_2_wrapper< CGAL_CDTplus2,SWIG_Triangulation_2::CGAL_Vertex_handle< CGAL_CDTplus2,Point_2 >,SWIG_Triangulation_2::CGAL_Face_handle< CGAL_CDTplus2,Point_2 > >,SWIG_Triangulation_2::CGAL_Vertex_handle< CGAL_CDTplus2,Point_2 > >::insert_constraint(Wrapper_iterator_helper< Point_2 >::input)

Investigation : CGAL : We haven't see any changes on this method between CGAL 4.14.3 and CGAL 5.4 in CGAL documentation. cgal_bindings :

We keep on investigating to find a solution and any help is welcome.

Thanks in advance. Jean.

lrineau commented 2 years ago

@sloriot It seems there is a sort of confusion: the Python code should call: insert_constraint(Vertex_handle va, Vertex_handle vb) but instead it calls insert_constraint(PointIterator first, PointIterator last, bool close=false)

sloriot commented 2 years ago

I suspect it is a bug with SWIG. I tried 4.3.3 with the 4.x branch and it does not work either. Maybe it was working for you but with a earlier version of SWIG. The issue is because of overloads. It I rename the function in the base wrapper class, it works. I don't know yet what is the best solution (without breaking the API).

Jiunixo commented 2 years ago

Thank you for your return. Our version of swig was and is still 4.0.1.

I've tried to add SWIG_CGAL_FORWARD_CALL_2(void,insert_constraint,Vertex_handle,Vertex_handle) in Constrained_triangulation_plus_2.h. The first insert_constraint works but the second one raises a new exception : SystemError: returned a result with an error set

Jiunixo commented 2 years ago

I've got a new track. Our project is using a fork of cgal-swig-bindings developed by Logilab : https://github.com/logilab/cgal-swig-bindings On the actual Head of official cgal-swig-bindings, I've reverted the commit https://github.com/logilab/cgal-swig-bindings/commit/d423fe33ab33a6cbcba0246b6acd943501bf138a as Logilab did on the last commit of their fork. Adapting the code a little, the cdt plus example works.

Obviously, it's not very satisfying to revert a commit which is probably useful for other users. But by analysing the changes introduced on this commit, it can give us some tips on how to solve the problem.

In first analysis, the problem could be related with the introduction of Contraint_id_wrapper, as the suppression of this class in Triangulation_2 package, solves the problem.

sloriot commented 2 years ago

As I said, SWIG is confused by the overload insert_constraint(Wrapper_iterator_helper<Point_2>::input input, bool closed=false) which get introduced in the aforementioned commit. The easiest fix would be to renamed that function but it would mean breaking the compatibility.

Jiunixo commented 9 months ago

We found this workaround which works : cdt.insert_constraint([va.point(), vb.point()])

I hope it can help anyone who encounters same problem. For us, this issue can be closed.