Hi,
this is really good stuff, i can confirm that you've correctly
understood the various issues about which you were unsure.
Incidentally we had been talking about something like this with Rohit
recently.
There are so many things to do at the moment, could you please address
these points before we apply your patch, that would help a lot:
1) need to extend the quaternion unit tests to cover that. The file is
test/geo_quaternion.cpp. No need to adapt all of it, just append a few
sanity checks for Quaternion<Map...> at the end, e.g. build a
quaternion that maps the array of a plain quaternion, do some
arithmetic on both and check (VERIFY_IS_APPROX) that the results are
the same.
2) i've made a few comments inline below.
2009/10/21 Mathieu Gautier <mathieu.gautier@xxxxxx>:
> Hi,
>
> I want to implement a class which hold both a quaternion and a vector3 to
> discribe a position of a frame in respect to an other frame (Something
> similar to Frame in KDL). For some historic reason, the memory layout must
> be a continuous array of scalar (Scalar[7]), the 3 first scalars are the
> vector3 and the other are the quaternion. Besides I have to interface this
> structure with other libraries which return an array of 7 scalars. I also
> have to map array of 4 scalars into the Quaternion class.
OK. So indeed with the current quaternion class you were out of luck
because its array is 16-byte aligned.
>
> I though of something like that :
>
> template<typename _Scalars>
> struct Frame {
> _Scalars[7] data;
> Map<Matrix<3,1,_Scalars>> v;
> Map<Quaternion<_Scalars>> q;
> };
>
> and setting the Map accordingly. However, there's no such concept of Map for
> the Quaternion class or RotationBase.
Indeed.
>
> So, I modified the Quaternion class to use either a Matrix<Scalar,4,1> to
> store the quaternion coefficients or a Map<Matrix<Scalar,4,1>> to map an
> array of scalars. I added a argument to the template prototype of the
> quaternion class which I call StorageType which is either the Matrix or
> Map<Matrix>.
Good.
In the Quaternion constructor, you could add some sanity checks for that type:
EIGEN_STATIC_ASSERT_VECTOR_ONLY(StorageType)
EIGEN_STATIC_ASSERT(
int(StorageType::Flags)&DirectAccessBit,
THIS_METHOD_IS_ONLY_FOR_EXPRESSIONS_WITH_DIRECT_MEMORY_ACCESS_SUCH_AS_MAP_OR_PLAIN_MATRICES
)
> Since I have almost no experience with Eigen, I don't know if it's the
> better choice and if it could cause alignement issues. I have attached my
> modifications as a patch. This implementation may be incomplete.
That's good and there's no alignment issue, because the StorageType
would only claim to be aligned if you explicitly tell it that the
pointer is aligned.
Currently we have a little performance trouble: there's no way to tell
Map that the pointer is aligned, but see other new thread, this is
taken care of.
>
> So, to summarize, the Frame class would be:
>
> template<typename _Scalars>
> struct Frame {
> _Scalars[7] data;
> Map<Matrix<3,1,_Scalars>> v;
> QuaternionMap<_Scalars> q;
> };
>
> where v is a mapping of data and q a mapping of data+3.
Looks good.
Some more comments inline in the patch:
>
> --> -template<typename _Scalar> struct ei_traits<Quaternion<_Scalar> >
> +template<typename _Scalar, class StorageType> struct
> ei_traits<Quaternion<_Scalar, StorageType> >
> {
> typedef _Scalar Scalar;
> };
>
> -template<typename _Scalar>
> -class Quaternion : public RotationBase<Quaternion<_Scalar>,3>
> +template<typename _Scalar, class StorageType = Matrix<_Scalar, 4, 1>>
> +class Quaternion : public RotationBase<Quaternion<_Scalar, StorageType>,3>
Why make it templated in both the Scalar and StorageType? The Scalar
could be deduced from the storage type:
typedef typename StorageType::Scalar Scalar;
fewer template argument passing, is better.