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