Re: [boost] [Review] We're halfway through the GGL review

Subject: Re: [boost] [Review] We're halfway through the GGL review
From: Barend Gehrels (barend_at_[hidden])
Date: 2009-11-15 17:18:24

Hi Fabio,

>
> Maybe you can use them in one of the examples, to make things clearer.

Sure, that makes sense. We'll do that.

>
>>
>> Therefore, union and intersection work with output iterators. So:
>> union_inserter(p1, p2, std::back_inserter(ps1));
>
>     
> Hm, I tried:
>
>     #include <boost/ggl/ggl.hpp>
>     #include <boost/ggl/geometries/cartesian3d.hpp>
>     #include <boost/ggl/multi/geometries/multi_polygon.hpp>
>     #include <boost/ggl/algorithms/union.hpp>
>
>     ...
>
>     typedef multi_polygon<polygon_3d> polygon_set_3d;
>     polygon_set_3d ps;
>     
>     polygon_3d poly1;
>     polygon_3d poly2;
>     
>     union_inserter(poly1, poly2, std::back_inserter(ps));

There are two things:

- I forgot one thing, sorry, the inserter-versions always have their 
output type as a required template parameter. That one can, AFAIK, not 
be deduced from the output iterator. And it might be a different type of 
the input polygons. If someone knows if that is possible somehow, I 
would be happy to know. So it should be: union_inserter<polygon_2d>(p1, 
p2, std::back_inserter(ps1));

- You're using the 3d polygons here. The union would make a 2D 
union-operation then of your 3D polygons (so polygons with for all 
vertices a height). It would ignore the third dimension (because it is 
not a polyhedron union). I just tried it, to not give you again wrong 
information. But it is not working yet like this, a.o. because the 
functions convert, area and point-in-polygon which should support 3D 
then. Especially area implementation would probably be different for 3D. 
Though it is used as a helper-function in the union, so its usage could 
be avoided.

> Besides I would probably not call it union_ but geometric_union or 
> some such, but thats just personal preference.

Yes, but it is already in the namespace ggl. We thought that union_ 
conforms to boost/mpl conventions (not_, if_)

>
> For each class/function add a short usage example including all 
> relevant  includes (something short like the snipplet above)
> This makes it immediately clear how things work together.

That makes also sense, thanks.

Regards, Barend