Re: [Lib2geom-devel] Templates

[<jbc...@sw...>]

---- "Krzysztof Kosiński" <twe...@gm...> wrote: 
> 2014-06-16 12:22 GMT+02:00 Nathan Hurst <nj...@nj...>:
> > Yes, I thought about this when we started, and mental and I discussed
> > this at length.  Here are the reasons against:
> >
> > * as Krzysztof says, templates mean everything must be on the compile
> >   path for everything.
> > * More complex methods for everything
> > * Ugly error messages
> > * Minor performance cost
> > * Slower compiles (a big issue for me - the slower the compile the
> >   slower the development)
> 
> Slower compilation can be alleviated somewhat by -O0 and using clang.
> Your points 2 and 3 are the main problems, and furthermore they
> complicate the use of the library. The main advantage of 2Geom over
> more complex libraries such as CGAL is its ease of use.

With default template parameters (or aliases like std::string), I don't think the use of the library would change really.
The compiler error messages is a good point, but we already have that 'problem' for the templated stuff that we have.
 
> > I couldn't think of any other compelling reasons for (especially when you
> > consider that D2<T> does what you want).
> 
> The biggest reason "for" is less code duplication, which is why I
> created GenericInterval and GenericRect - fixing every minor bug and
> adding every method in both places was getting really tedious. On the
> other hand for Point we wouldn't deduplicate a lot of code, and on top
> of that we would have to write lots of template boilerplate.
> 
> 2014-06-16 20:29 GMT+02:00 Johan Engelen <jbc...@sw...>:
> > Yep. But, our current design could be changed to alleviate this issue
> > somewhat, by splitting out some functionality in different header files.
> > Things like Path::toPwSb(), Path::allNearestPoints, ... should not be member
> > functions.
> 
> I don't see why allNearestPoints should be a free function. It makes
> sense that is is a method, in keeping with the "ease of use" design
> goal.

One can have long debates about making something a member function or not.
I don't see an ease-of-use problem between:
  path1.allNearestPoints(...)
and
  all_nearest_points(path1, ...)

I only meant to say that we have the option to split files. Some of our class definitions are huge, because of (imho) adding too many class methods instead of putting those methods outside the class. I am guilty of this myself, but have changed opinion a bit recently.
Come to think of it, allNearestPoints might be generalized to a 2geom algorithm that works for all objects passed with an interface similar to Path (argument in favor of out-of-class function).

> >> * Minor performance cost
> >
> > You made a typo: minor performance boost. ;-)
> > (serious question: I'm curious to see an example where using templates makes
> > the code slower)
> 
> Multiple, almost identical instantiations of a template increase code
> size, which in turn reduces code density and makes worse use of the
> CPU instruction cache. So templates may not always improve
> performance.

This is what everybody says, but I'm not buying it. I think this only results in less performance for some pathological test cases.
I'd like to see a real case where templated code reduced performance.
In our case, I don't see a problem here.

> > If that were true, we would not need Point at all.
> > But you are right, Point<float> would not be worth it. Point<int> would be
> > much better; but mind caveats noted by Krzysz.
> > So I'll give up on this one for now.
> > (how about IntPoint<int>, <unsigned>, <short>, etc, Krzysz?)
> 
> I don't see much use for IntPoint<unsigned>, because it will lead to
> annoying signedness caveats (e.g. you have to be careful when
> subtracting unsigned points).
> 
> Same for IntPoint<short>, because it has low resolution and shorts are
> not faster than ints on any modern CPUs. The only case where it would
> be useful is when you have to process a large amount of points stored
> in memory, because then it would increase the number of them that fits
> in the cache.
> 
> There may be some use for IntPoint<long>, IntPoint<long long> or
> IntPoint<Bignum>, but I don't see any clear use cases right now.

Yeah I don't see much use either, but it's a little beside the point :)

I asked myself the question: should 2geom be in the standard library? Surely, many applications would benefit from a standard geometry library. Then I thought, it is not nearly as general as it could be.
Hope you all could ponder of this question a bit (2geom -> stdlib?). When Inkscape's release is done, I hope we can switch to C++11, triggering some motivation for revisiting large parts of 2geom's code, making sure the interfaces are homogeneous, etc.

regards,
  Johan