[IGSTK-Developers] Bug in igstkTracker with calibration transform

[David Gobbi]

Hi Julien,

I agree with you.  It certainly does look like the right-left ordering 
of the
rotation multiplication is wrong.  I have identified four places in the code
where a fix is needed, the fix is always as follows:

  rotation *= transform.GetRotation();   //   WRONG
  rotation = transform.GetRotation()*rotation; //  CORRECT

Go ahead and commit the fix.

To avoid this kind of thing in the future, we need a method to concatenate
our igstkTransforms:

  /** Set this transform to the concatenation of the two given transforms:
  * If you have two transforms and you wish to concatenate them, then
  *   t1->Concatenate(t2,t1)   implies   t1 = t2*t1
  void Transform::Concatenate(const Transform *t1, const Transform *t2);

If this makes sense, I can add it to igstkTransform.

 - David


Julien Jomier wrote:

> Hi David,
>
> Thanks for the explanation. I think I understand the way it works and 
> I agree with the design but I think the implementation is wrong.
> Let me try to explain.
>
> From the documentation:
>
>   T ' = P * R^-1 * T * C
>
>   where:
>   " T " is the original tool transform reported by the device,
>   " R^-1 " is the inverse of the transform for the reference tool,
>   " P " is the Patient transform (it specifies the position of
>         the reference with respect to patient coordinates), and
>   " T ' " is the transformation that is reported to the spatial objects
>   " C " is the tool calibration transform.
>
> So we are interested by this part of the equation here:
>
> T*C = RotationT * (RotationC+TranslationC)+ TranslationT
> T*C = RotationT*RotationC + RotationT*TranslationC + TranslationT (1)
>
> Looking at the code:
>
> // start with ToolCalibrationTransform
> rotation = toolCalibrationTransform.GetRotation();
> translation = toolCalibrationTransform.GetTranslation();
>
> // transform by the tracker's tool transform
> rotation *= transform.GetRotation();
> translation = transform.GetRotation().Transform(translation);
> translation += transform.GetTranslation();
>
> Which can be written as:
>
> T*C = RotationC*RotationT + RotationT*TranslationC + TranslationT (2)
>
> Therefore, comparing (1) and (2) the rotation is computed in the wrong 
> order or maybe I'm missing something really obvisous...
>
> And the m_PatientTransform is following the same implementation so 
> might be wrong as well.
>
> Let me know what you think,
>
> Julien
>
>
> David Gobbi wrote:
>
>> Hi Julien,
>>
>> I think that the tool calibration transform code is correct the way 
>> it is, but note that the
>> way the code is written, the tool calibration transform goes from the 
>> "calibrated tool"
>> coordinate system to the "raw tool" coordinate system.  This might be 
>> the opposite of
>> what you expect.
>>
>> There is a very good reason for having the calibration transform go 
>> in this direction.
>> You know how an igstkTransform defines a rotation followed by a 
>> translation? Since the tool tip is at (0,0,0) in the "calibrated" 
>> coord system, our "calibration transform"
>> applies a rotation about the tool tip, which is at (0,0,0) in the 
>> calibrated coord system,
>> followed by a translation to the "raw" tool tip location.
>>
>> This means that even if the rotation portion of the calibration 
>> transformation is incorrect,
>> the "tool tip" portion of the calibration will still give the correct 
>> tool tip location, since the
>> rotation part of the calibration occurs around the tool tip 
>> position.  If the calibration
>> transform was the other way around, then we would not get this nice 
>> independence of
>> translation from rotation.
>>
>> If you already had your calibration transformation going in this 
>> direction, email me and I
>> will take a close look at the code to make sure that there is no bug.
>>
>> - David
>>
>>
>> Julien Jomier wrote:
>>
>>> Hello,
>>>
>>> There is an important bug in the igstkTracker class preventing any 
>>> calibration matrix to be applied correctly.
>>>
>>> The problem is in the UpdateStatusSuccessProcessing() function.
>>>
>>> The calibration transform should be applied first (rotation 
>>> variable) and then the transform from the tracker (transform variable).
>>> However the code is as follow:
>>>
>>> rotation *= transform.GetRotation();
>>>
>>> whis is in the wrong order and should be:
>>>
>>> rotation = transform.GetRotation()*rotation;
>>>
>>> Let me know if I should fix it, or if someone else is currently on 
>>> the igstkTracker class right now or if I'm missing something...
>>>
>>> Julien
>>> _______________________________________________
>>> IGSTK-Developers mailing list
>>> IGSTK-Developers at public.kitware.com
>>> http://public.kitware.com/cgi-bin/mailman/listinfo/igstk-developers
>>>
>>
>>
>>
>