Re: [Boost-users] [boost::polygon::transform.hpp] Concatenation of transformations fails.

Subject: Re: [Boost-users] [boost::polygon::transform.hpp] Concatenation of transformations fails.
From: Simonson, Lucanus J (lucanus.j.simonson_at_[hidden])
Date: 2011-08-25 14:05:28

Dimitris Dakopoulos wrote:
> Hello to all,
> 
> I want to apply to geometrical transformations (in this case
> translations) on a point.
> 
> Point3D trans1(1,0,0);
> Point3D trans2(2,0,0);
> 
> Point2D p(100,100);
> 
> gtl::transformation<int> T1( trans1 );
> gtl::transformation<int> T2( trans2 );
> 
> gtl::transform( p, T1 );
> gtl::transform( p, T2 );
> 
> --- ... and the point's coordinates are (97,100) as expected.
> 
> --- When I try to concatenate the transformations:
> 
> Point3D trans1(1,0,0);
> Point3D trans2(2,0,0);
> 
> Point2D p(100,100);
> 
> gtl::transformation<int> T1( trans1 );
> gtl::transformation<int> T2( trans2 );
> 
> gtl::transform( p, T1 + T2 );
> 
> --- ... and I get random numbers.
> 
> I checked the boost/polygon/detail/transform_detail.hpp
> 
>  template <typename coordinate_type>
>  inline const transformation<coordinate_type>&
> transformation<coordinate_type>::operator+=(const
> transformation<coordinate_type>& tr){
>    //apply the inverse transformation of this to the translation
>    point of that //and convolve it with this translation point
>    coordinate_type x, y, z;
>    transformation<coordinate_type> inv = inverse();
>    inv.transform(x, y, z);
>    p_.set(HORIZONTAL, p_.get(HORIZONTAL) + x);
>    p_.set(VERTICAL, p_.get(VERTICAL) + y);
>    p_.set(PROXIMAL, p_.get(PROXIMAL) + z);
>    //concatenate axis transforms
>    atr_ += tr.atr_;
>    return *this;
>  }
> 
> ... and I noticed that the coordinate_type x, y, z; is not initialized
> and then the transformation is applied: inv.transform(x,y,z); which of
> course results to something random.
> 
> Any ideas?
> 
> Thank you for checking.
> 
> Dimitris

Here is the fix.  This function was working before I changed the interface to all my geometry data objects when I "boostified" the API in 2007.  Apparrently when I went through and fixed all the code that was broken by that interface change I didn't properly test this function.  Oddly, the comment was also wrong because it didn't match the code.  It would have been correct if I applied the inverse of the axis_transformation in (*this), but was redundant to convolve the translation points given that applying the full inverse transformation to the translation point does that already.  I tested several combinations of axis_transformation and translation points and everything seems to be working properly with the fixed code below:

  template <typename coordinate_type>
  inline const transformation<coordinate_type>& transformation<coordinate_type>::operator+=(const transformation<coordinate_type>& tr){
    //apply the inverse transformation of this to the translation point of that
    coordinate_type x=tr.p_.get(HORIZONTAL), y=tr.p_.get(VERTICAL), z=tr.p_.get(PROXIMAL);
    transformation<coordinate_type> inv = inverse();
    inv.transform(x, y, z);
    p_.set(HORIZONTAL, x);
    p_.set(VERTICAL, y);
    p_.set(PROXIMAL, z);
    //concatenate axis transforms
    atr_ += tr.atr_;
    return *this;
  }
  
Sorry for any inconvenience.  This fix should go out in the next boost release.  You'll note that I'm still using point_3d_data in the interface of transform, even though it is undocumented.  All the transform() functions that take a geometry and a tranformation object expect the transformation object to have a transform member function:

  void transform(coordinate_type& x, coordinate_type& y) const;

You can define your own transformation object types that satisfy this requirement and you don't need to use mine if you don't want to.  Specifically, mine are just axis transformations with translation.  For a general transformation you would need to declare your own that probably stores the 2d transformation transformation matrix.  To some extent I think of my transformation object as example code since it isn't documented.

Regards,
Luke