Subject: Re: [boost] Formal Review: Boost.Polygon starts today August 24, 2009
From: Thomas Klimpel (Thomas.Klimpel_at_[hidden])
Date: 2009-09-01 14:48:13
Barend Gehrels wrote:
> 1) the library supports many operations, but some basic polygon
> algorithms are missing:
> - convex hull
> - centroid
> - is_convex
I don't know whether convex hull should be considered as a basic polygon algorithm. I do agree that centroid is a basic polygon algorithm. I would even say that a "centroid" function makes more sense than the "center" function as currently implemented that returns the center of the bounding box of the polygon. An is_convex function could make sense, but I have concerns that it is not efficient enough. (It could be nice if an polygon knows its bounding box, whether it is convex, and whether it only has 45 degree angles.)
> 2) though some reviewers were satisfied about the performance, and the
> paper indicates it is well performing, our benchmark shows that it is in
> general slower or much slower than other libraries:
Who do you mean by "some reviewers"? The review by "Sebastian Redl" doesn't talk about performance, nor do most of the pre-reviews. My review mentioned performance two times:
-> But what is meant by "Performance of GTL could be improved by up to 10X with further work on the arbitrary-angle Booleans"??? <-
-> The benchmarks by the author also indicate that the library is very efficient. <-
and these statements should make it sufficiently clear that I haven't measured the performance myself. But you are basically right that the statement/possibility that the performance of the library could be improved by up to 10x is no reason for me to reject the library.
> The file transform.hpp contains a large number of enumarions for
> transforming polygons up/down/west/east etc. I think this is not
> necessary, and besides that "west" / "east" gives the impression of
> spherical coordinate systems while this library is cartesian only.
> Left/right should already be better, but why not two offset numbers in
> two dimensions?
I agree, if orientation_2d can be HORIZONTAL and VERTICAL, the directions should be LEFT, RIGHT for HORIZONTAL and UP, DOWN for VERTICAL. And if this creates confusion in the presence of orientation_3d (what is PROXIMAL???) and direction_3d, all 3d stuff should simply be removed from Boost.Polygon, because it doesn't fall into the scope of the library anyway.
> > - What is your evaluation of the potential usefulness of the library?
> I think most people using geometry / polygons are using floating point
> coordinates. As these are not supported in the algorithms where this
> library is specialized on (booleans), I think it is useful for a limited
> public (VSLI?)
It's also useful for VLSI :)
> I hope that my review was as objective as can be expected from an
> alternative library writer.
> It is to the other reviewers to decide that this is the case.
So I conclude from this (as being one of the other reviewers) that you don't vote against accepting Boost.Polygon as a Boost library. Or did you just miss the advice "Please always state in your review, whether you think the library should be accepted as a Boost library!"?