Re: [eigen] Bounding Volume Hierarchies

[Ilya Baran <baran37@xxxxxxxxx>]

Hello,

> Actually, I think a BVH is a bit too far to Eigen's objectives, and at
> a first glance I don't think it should be included in Eigen.
> Otherwise, by extension, we'll soon add a physics engine, a raytracer,
> etc. I think we have to find a limit.
>
> On the other hand, I have the project to write a spatial data
> structure library based on Eigen, and so your contribution would
> perfectly fit :) Since it is not started and yet and I don't have much
> time for that, I'm currently ok to commit your patch in Eigen's tree
> because:
> 1 - the module is in unsupported so no big deal, we can still see
> later whether it is worth creating a separate project or not,
> 2 - the contribution looks good, and so it's worth it making it
> available to most people right away,
> 3 - I know one KDE application which needs a BVH: Step.

Yes, this basically mirrors my thinking that I didn't articulate in
the previous email: I agree that BVH doesn't properly fit into Eigen's
scope as a linear algebra library (but does the geometry module,
really?  would other numerical methods, like solvers, integration,
ODE's etc?).  However, at this stage, the BVH is not sufficiently
fancy to warrant the overhead of a separate library, so I think
Eigen/unsupported is a good home for now, especially since with
Eigen's architecture you don't pay for what you don't use.

> Regarding the changes in AxisAlignedBox, I'm not very fan of using
> operator & and | for the intersection and union, and I would suggest
> explicit names.

I prefer reading "(a | b) & (c | d)" to
"(a.union(b)).intersection(c.union(d))", but it's not a strong
preference (and, of course, your call at that).  For explicit names,
I'd suggest "union" and "intersection" for operator| and operator& and
"unite" and "intersect" for operator|= and operator&= (rather than the
current "extend" and "clamp").

Thanks,

   -Ilya