[vtk-developers] vtkVector class -- proposed rewrite

[David Lonie]

Hi list,

I'd like to ask for opinions on the following patch:

http://github.com/dlonie/VTK/commit/6fd3b1c50244174d2ecb2e8bf4ae40996b0f61d6

A bit of background:
While working on the vtkChemistry kit, I wanted to extend the
vtkVector class to perform basic vector arithmetic (dot products,
vector addition/subtraction, scalar multiplications, ... ). The Eigen
linear algebra library has a similar approach to vectors as vtkVector,
where the actual vector class is templated so that different sizes and
types (float, double, int, etc) can be used. Eigen will typedef
convenience class for these types, e.g. Vector3f (three floats),
Vector4i (four ints), etc, but we cannot do this in VTK because AIUI,
this would break the scripting language wrappers. So instead of
typedef'ing, we create concrete classes that inherit from the
templated classes.

A neat feature of this approach is that the vectors only take up the
same amount of memory as an array of equal length, and most (if not
all) functions are inlined by the compiler, making the vtkVector
objects very efficient for both storage and performance.

The problem:
The existing vtkVector class had few math functions, and handling
return types was difficult: implementing the math functions in
vtkVector meant returning a vtkVector<> class, so if we defined, say,
operator+, in the base vtkVector class we couldn't do something like:

vtkVector3d v1 (1.0, 2.0, 3.0); // A vector of three doubles
vtkVector3d v2 (4.0, 5.0, 6.0); // Another
vtkVector3d v3 = v1 + v2; // Error!

The error occurs because if operator+ is implemented in the vtkVector
base class with a vtkVector return type, the result cannot be
implicitly casted up to the derived vtkVector3d type. So these
functions must be implemented in the derived classes so that they can
return the appropriate types. Since I want to implement concrete
classes for (at least) unsigned int, int, float, and double types with
sizes 2, 3, and 4, maintaining such a system could be a nightmare --
fixing a single bug in the derived math functions could require fixing
at least 12 implementations!

Proposed solution:
The new vtkVector header (
http://github.com/dlonie/VTK/blob/6fd3b1c50244174d2ecb2e8bf4ae40996b0f61d6/Common/vtkVector.h
) is structured as followed:

#ifndef __vtkVector_concrete_generating

#ifndef __vtkVector_h
#define __vtkVector_h

(vtkVector class definition)

#define __vtkVector_concrete_generating

(Declarations and reinclusions of vtkVector.h)

#undef __vtkVector_concrete_generating
#endif // __vtkVector_h
#endif // ifndef __vtkVector_concrete_generating

#ifdef __vtkVector_concrete_generating

(generic, macro'd implementation of a single derived class)

#endif // ifndef__vtkVector_concrete_generating

It starts by declaring and implementing a templated vtkVector class
with some internal functions that operate on arrays. When included by
an external file, the preprocessor will read this class as usual. The
tricky bit is after the base class definition.

The generic, macro'd implementation of the base class uses compiler
macros for the name, size, and type of the vector class. The
"Declarations and reinclusions" section defines these macros and then
does '#include "vtkVector.h"'. Since __vtkVector_concrete_generating
is defined at this point, these reinclusions skip to the macro'd
implementation, which is expanded in the preprocessor to form the
actual concrete, derived class (e.g. vtkVector3d, vtkVector2u, etc).
This is done for each of the desired concrete classes.

I've been testing this out, and aside from outfoxing QtCreator's
auto-completion feature, this approach has worked well and is easy to
maintain. However, this approach is admittedly a bit bizarre, and I
want to get some feedback before I submit the patch. Particularly,
will this break the documentation and wrapping parsers, or are they
smart enough to follow all of the inclusions / definitions correctly?
Is this approach acceptable per VTK's standards and conventions?

Thanks,

Dave