
Hi all, First I would like to say that I appreciate all the hard work that has gone into the library. Implementing a fully generic geometry library is a very difficult undertaking. Having said that I don't think that GGL is at the level of maturity that I would expect from a Boost.Geometry library. During the course of my review I would constantly ask myself when using a given feature whether this is something that I would use in my professional work without hesitation, and my answer was often that it didn't seem quite ready for production use. I think with a few more iterations and more collaboration with the Boost community it could be. So my formal vote is not to accept at this time. - What is your evaluation of the design? The quality of the interface varies, but is in general OK. The use of tag-dispatching and trait specialization seems reasonable as does the strategy design. Other parts of the design seem a bit more ad-hoc. For example: template<typename DegreeOrRadian> struct geographic { typedef DegreeOrRadian units; }; Which seems to be a way to specify a unit type for angular coordinates. I'm not sure how such a construct extends into other unit types such as feet/meters on distance coordinates. It is perhaps not so relevant in the context of the operations required to be performed on a given unit type. I think a better design would have incorporated these nuances into a trait for the given coordinate. Given the availability of Boost.Units I'm sure something neat could be done on this idea. Further to this I found a type trait struct 'is_radian<T>' but none for 'is_degree' (or is_meter, is_feet). I think these artifacts will present issues if the geometry library were to be later abstracted to support units on the coordinate types. Dimensionally qualified quantities can be calculated for geometries for whose dimensions the quantity makes no sense (e.g. area( point ), length( point ).) These functions return default constructed instances of their corresponding return type as shown here: template<typename ReturnType, typename Geometry, typename Strategy> struct calculate_null { static inline ReturnType apply(Geometry const& , Strategy const&) { return ReturnType(); } }; In my test ReturnType was double. This would seem to silently compile and return garbage on these inputs. In practice on vs2008( VC9 ) these return values were 0 (with optimizations on) even though the return resolves to double();. I'm guessing other platforms won't be so lucky. The geometry concepts seem sound at a glance, but I wonder how they would fare in the light of extension. For example, the ggl::point_type< Geometry > meta-function would seem to presume that all geometries have an underlying point representation. This doesn't necessarily extend to lines or planes, and so may present issues later on when those types are added. Some of the default geometry types hold data by reference which makes their usage in containers a bit difficult (specifically the segment type.) Much of the nomenclature of functions and types in the library seems to come from the OGC (Open Geospatial Consortium) or so I assume. I don't think this consortium is widely recognized as a standard for geometry names in general. I'm not suggesting that there is a standard, but I think better names may have been chosen in light of more common usages in academic computational geometry literature. For example, we have polygon, but then linestring (rather than polyline). The term 'envelope' is used to refer to what I believe is known in the literature as an axis-aligned bounding box. This may come down to stylistic preference, but I would expect a Boost.Geometry to have a higher level of fidelity in the nomenclature. - What is your evaluation of the implementation? The implementation seems technically competent. The segment intersection algorithm matches the speed of what we use in our production code. The boolean operations seem fast. My tests of the area/length/centroid operations gave correct results. - What is your evaluation of the documentation? The documentation is not sufficient. I would suggest better tutorials for all common use cases. A simple query on the user newsgroup would probably give a good list of those most common cases. Overall, the documentation is probably the least polished part of the submission. - What is your evaluation of the potential usefulness of the library? Tremendously useful. - Did you try to use the library? With what compiler? Did you have any problems? Yes. VS 2008 (vc9). Yes, a few. The most troubling was a seg-fault while trying to synthesize a boolean difference operation. Some of the default geometry concept implementations were clunky to use in what I would consider a common use case. (As I said above the segment type isn't default constructible due to holding points by reference..). This latter issue is a bit trivial as I can write my own, but I figured it might speak to the library's maturity as rough edges like this tend to be fixed with wide use. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? In-depth study. - Are you knowledgeable about the problem domain? Yes. Over the course of my professional career I have worked on 2 geometry libraries. I think there is a lot of potential here and hope that work continues. Best of luck, Brandon