Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

insert_constraint error on Constrained_Delaunay_Triangulation_plus_2 #210

Open
Jiunixo opened this issue Apr 6, 2022 · 6 comments
Open
Assignees

Comments

@Jiunixo
Copy link

Jiunixo commented Apr 6, 2022

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 :

  • In the former cgal_bindings version working with CGAL 4.14.3, CDTP2.insert_constraint() calls Internal_Contrained_triangulation_2_Internal_Constrained_Delaunay_triangulation_2_Constrained_Delaunay_triangulation_plus_2.insert_constraint() which works.
  • With the last version of cgal-bindings and CGAL 5.4, CDTP2.insert_constraint() calls Constrained_Delaunay_triangulation_plus_2.insert_constraint() which raises the error above. We note that in the old bindings this method does not exist in CGAL_Triangulation_2.py.

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

Thanks in advance.
Jean.

@lrineau
Copy link
Member

lrineau commented Apr 6, 2022

@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
Copy link
Member

sloriot commented Apr 7, 2022

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
Copy link
Author

Jiunixo commented Apr 8, 2022

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
Copy link
Author

Jiunixo commented Apr 8, 2022

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 logilab@d423fe3 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
Copy link
Member

sloriot commented Apr 11, 2022

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
Copy link
Author

Jiunixo commented Sep 27, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants