Re: [boost] Proposal/InterestCheck: Boost.Geom

Subject: Re: [boost] Proposal/InterestCheck: Boost.Geom
From: Simonson, Lucanus J (lucanus.j.simonson_at_[hidden])
Date: 2009-01-27 13:23:43

Anis Benyelloul wrote:
> By ``pretty'' I mean these kind of things:
> box<..> bx;
> bx.center() = point(0,0); // this will ``move'' the box so that it's
> 			  center is // at the origin
> bx.corner<xmin, ymin, zmin>()=bx.center() // this will stretch the
> 		      box so that // the corner is where the center was before
> 
This is very clever, does center() generate an object that casts to point, but caches a reference to the box and implements an assignment operator that modifies box?  What an interesting idea.

I'm a little concerned that such a thing might be a little too clever.  Without your comment I would assume that bx.center() returns a point by value, that the assignment of the origin point to that value modifies the temp copy and not the original box, and that the temp copy then goes out of scope.  If reading a code leads a developer who is unfamiliar with the details of the library to the wrong impression of what it does, I fear it may indicate an unintuitive interface that could be error prone.  Why not have two center() functions, one that returns a point by value, and one that accepts a point and returns that point by value.

point pt = bx.center(); //assigns copy of center of box to pt
bx.center(point(0,0)); //centers box on origin
Point pt2 = bx.center(get_some_point(from_some_object)); //centers box on a point and stores a copy of the point to pt2

This API for dealing with the center of a box follows the principle of least astonishment.

> Instead of ``pretty' you could say ``rich'' or ``richer than a strcut
> point {int x,y;};''
> 
Rich seems like a less subjective term than pretty, since it implies quantity.

> Exactly. So I'm not alone, glad to hear it :)

Actually, GUIs was the only other domain Dave A. could think of where my original proposal for rectilinear geometry library was applicable.  I knew you had to be out there somewhere.  I think there are many other domains that like to simplify their lives by using axis parallel geometries, but now I have arbitrary angle APIs in my library too, which covers everybody else.

>> Do you also intend people to wrap a point_type by casting it to
>> point<point_type>?
> 
> Yes. There is a geom::point_wrap() and a geom::box_wrap() non-member
> functions that do that. Maybe I should mention them earlier in the
> manual. 

My understanding is that doing so is not legal according to the C++ standard.  Someone please correct me if I am wrong.

struct A {};
struct B { A a; void bar(); };
void foo() {
        A a;
        B b;
        B& B_view_of_A = *((B*)&(a)); //result of cast is undefined, but generally works because B is the same in memory as A is most compilers
        B_view_of_A.bar();
}

I know that you want your dot operator syntax because it is how you are used to working with geometry objects, but you can provide a rich API without dot operator syntax

template <typename T>
boost::enable_if<is_box_like<T>::type, point>::type
center(T& obj);

This definition of center can accept an A directly without need of wrapping it.

>> It seems like you are allowing a point<point<point_type> > with this
>> construct.
> 
> Yes, this not explained until somewhere deep inside chapter 5 or so.
> But this is not very important from a user perspective. This mainly
> allows to cut down the number of necessary member overloads of
> geom::point<>. 

I had a similar problem with my wrappers in my original submission.  I got around it by making point conform to the default expectation of the point_traits so that you could recursively call traits until you got down to the base level.  Since there are no wrappers in the concepts based implementation I don't have such a problem anymore.

>>  Your traits seem a little heavy,
> 
> Yes, but it is necessary to allow type-specific optimizations (such
> as if you are packing the X and Y coordinates in the same machine
> word, geom::point<>::operator==() will be able to compare both
> coordinates with the same operation, instead of the generic x1==x2 &&
> y1==y2). 
> 
> On the other hand if you don't want to specialize everything (and are
> happy with the generic implementations) you can use
> point_xy_trait/point_xyz_traits which will specialize point_traits
> for you based on a subset of its features. 

I overlooked that you provide such helper traits, but I had that in mind as one solution.

It should not be necessary to use traits to provide type specific optimizations.  You can specialize any member function of a templated class, however, you cannot partially specialize such a function, so you have to explicitly specify all template parameters, which can be somewhat limiting.  

#include <iostream>
template <typename T>
struct A {
        void foo() { std::cout << "default\n"; }
};

template <>
void A<int>::foo() { std::cout << "optimized for int\n"; }

int main() {
        A<float> af;
        A<int> ai;
        af.foo();
        ai.foo();
        return 0;
}

Outputs:
default
optimized for int

This is somewhat more direct than providing for the ability to use a user defined trait for every behavior of a geometry object, which requires quite a lot of typing and indirection within the library.

I want to add that inlined indirection can be zero cost, but in practice the more layers of inlined functions you have the harder it gets for the compiler to optimize.  Eventually the compiler bails on inlining and will fail to inline where you think it ought to.  There is a huge improvement in the performance of my original library (which is similar to yours) when moving from gcc 3.x to gcc 4.x, primarily due to better inlining (we believe.)  This undermines the claim of zero cost abstraction, because if it got cheaper it clearly wasn't zero cost in the first place.  We did a lot of analysis, and in real code the compiler often failed to inline the simple accessor functions that it always inlined in toy test cases.  This led to a double digit performance loss.  By playing with pragmas and inline limit flags we recovered that performance, but I fear your faith in the compiler my be somewhat misplaced.

Regards,
Luke