yade-dev team mailing list archive

[Bruno Chareyre <bruno.chareyre@xxxxxxxxxxxxxxx>]

Hi Janek,
I think that was the right fix, thanks.
Note that yade is not using the periodic triangulation implemented in CGAL
[1] so there is no question on that point afaik.
Bruno

[1] that's because yade included periodicity years before cgal. I actually
implemented a periodicity based on cgal's non-periodic triangulation. Cgal
devs made periodic regular triangulation available more recently -
triggered by me for a part - but it's still not used in yade. Hopefully it
will be used one day but the refactoring it implies is a bit daunting.



On Fri, 4 Jan 2019 at 19:46, Janek Kozicki <janek_listy@xxxxx> wrote:

> It appears that this function move_point(…) had long been deprecated.
> I guess that's got something to do with previous modifications where we
> had to use weighted points differently.
>
> I propose to modify this line into calling move(…) instead.
>
> This isn't change like the previous one where the C++ template
> structure was changed and I had to dwell deeply into code to recover
> the original implementation. This is a "small" change of the code
> which I did not write, so I suppose that Bruno I will need you to
> certify this.
>
> Let's see the code in old version:
> /usr/include/CGAL/Regular_triangulation_3.h
>
> #ifndef CGAL_NO_DEPRECATED_CODE
> template < class Gt, class Tds, class Lds >
> typename Delaunay_triangulation_3<Gt,Tds,Default,Lds>::Vertex_handle
> Delaunay_triangulation_3<Gt,Tds,Default,Lds>::
> move_point(Vertex_handle v, const Point & p)
> {
>     CGAL_triangulation_precondition(! is_infinite(v));
>     CGAL_triangulation_expensive_precondition(is_vertex(v));
>
>     // Dummy implementation for a start.
>
>     // Remember an incident vertex to restart
>     // the point location after the removal.
>     Cell_handle c = v->cell();
>     Vertex_handle old_neighbor = c->vertex(c->index(v) == 0 ? 1 : 0);
>     CGAL_triangulation_assertion(old_neighbor != v);
>
>     remove(v);
>
>     if (dimension() <= 0)
>         return insert(p);
>     return insert(p, old_neighbor->cell());
> }
> #endif
>
>
> And the code in new version:
> /usr/include/CGAL/Regular_triangulation_3.h
>
> template <class Gt, class Tds, class Lds >
> typename Regular_triangulation_3<Gt,Tds,Lds>::Vertex_handle
> Regular_triangulation_3<Gt,Tds,Lds>::
> move(Vertex_handle v, const Weighted_point& p)
> {
>   CGAL_triangulation_precondition(!is_infinite(v));
>   if(v->point() == p)
>     return v;
>
>   Self tmp;
>   Vertex_remover<Self> remover(tmp);
>   Vertex_inserter<Self> inserter(*this);
>   return Tr_Base::move(v,p,remover,inserter);
> }
>
> Which calls tr_Base::move(…) from file:
> /usr/include/CGAL/Triangulation_3.h
>
> template < class Gt, class Tds, class Lds >
> template < class VertexRemover, class VertexInserter >
> typename Triangulation_3<Gt,Tds,Lds>::Vertex_handle
> Triangulation_3<Gt,Tds,Lds>::
> move(Vertex_handle v, const Point& p,
>      VertexRemover& remover, VertexInserter& inserter)
> {
>   CGAL_assertion(remover.hidden_points_begin() ==
> remover.hidden_points_end());
>   CGAL_triangulation_precondition(!is_infinite(v));
>
>   if(v->point() == p)
>     return v;
>
>   Vertex_handle w = move_if_no_collision(v,p,remover,inserter);
>   if(w != v)
>   {
>     remove(v, remover);
>     return w;
>   }
>
>   return v;
> }
>
> Also let's look at the documentation for move_point:
>
>
> https://doc.cgal.org/latest/Periodic_3_triangulation_3/classCGAL_1_1Periodic__3__Delaunay__triangulation__3.html#aa06932079eb7cf6eee8c5c9998088e34
>
> It says that it is just a little more optimized regular move. So it
> may be not surprise that those codes are not exactly the same. Maybe
> move_point became obsolete, because they removed the inefficiency
> that was present in older versions.
>
> Note that this function was NOT removed from periodic triangulations.
> This may be important since yade works in periodic too. So perhaps in
> Tesselation.ipp there should be a separate move(…) function for
> periodic cases, to use the old move_point function again when it is
> possible. Bruno?
>
> The compilation passed all tests and checks. But do those tests cover
> the call of _Tesselation<TT>::VertexHandle _Tesselation<TT>::move(…) ?
>
> To encourage discussion I am making a small git commit with one line
> changed ;)
>
> best regards
> Janek Kozicki
>
> _______________________________________________
> Mailing list: https://launchpad.net/~yade-dev
> Post to     : yade-dev@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~yade-dev
> More help   : https://help.launchpad.net/ListHelp
>