[IGSTK-Developers] Bug in igstkTracker with calibration transform

[David Gobbi]

Hi Patrick,

It goes to show that I need to spend more time in the lab working with the
tracking systems, to make sure that my code works the way it should.

At the T-con me an Luis talked about applying translations directly to
the spatial object, and we agreed that the safe way to do what you want
is to write a new spatial object class that places the origin at the end 
of the
cylinder instead of at the center, and that orients the cylinder along the
right axis.

Julien, what do you think of adding a protected method to igstkSpatialObject
to allow subclasses to adjust the position/orientation of the object?  I 
notice
that itkSpatialObject has an IndexToWorldTransform and an
ObjectToParentTransform, but I'm not sure what they are meant for.

 - David


Patrick Cheng wrote:

> It seems that we have reached a nice agreement here.
> To avoid the mis-calculation, we should offer that Transform 
> concatenation.
>
> I will go ahead to see if the orientation will look right or not.
>
> We still need to think about where to put the offset of the 
> SpatialObject's origin though.
>
> Patrick
>
> David Gobbi wrote:
>
>> 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
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>> _______________________________________________
>> IGSTK-Developers mailing list
>> IGSTK-Developers at public.kitware.com
>> http://public.kitware.com/cgi-bin/mailman/listinfo/igstk-developers
>>
>>
>