[IGSTK-Developers] Bug in igstkTracker with calibration transform

[David Gobbi]

Hi Julian,

I agree, changing the CylinderObject as you suggest is the best solution.

I don't like using the Tool CalibrationTransform to put the tip at (0, 
h/2, 0),
even as a quick fix.

 - David


Julien Jomier wrote:

> Ok, I think everything is working well now :)
>
> > Julien, what do you think of adding a protected method to
> > igstkSpatialObject
> > to allow subclasses to adjust the position/orientation of the object?
>
> I don't really like the idea of modifying the internal object because 
> we  don't really have to. Basically the CalibrationToolTransform 
> defines where the tracker his w.r.t to the origin of the object so you 
> don't have to add an offset somewhere else.
>
> I've started an application using IGSTK and the ultrasound probe 
> tracking and I've used the CylinderObject without any issues.
>
> Patrick, can you tell me which transform your are optimizing when you 
> do your registration? From what I've seen during the demo, the tool is 
> already calibrated so the CalibrationTransform should be set to 
> identity and you are optimizing the PatientTransform, which is 
> correct. However if you are using the CylinderSpatialObject you know 
> that the tip of the needle will be roughly h/2 (with h the height of 
> the needle) so you have to set the CalibrationTransform accordingly.
>
> That said, I think we should define a common coordinate frame for all 
> igstk objects. Meaning that by default the tip of the object will be 
> at (0,0,0) and the main orientation will be along the Z-Axis. What do 
> you think about changing the CylinderObject to follow this assumption?
>
> Julien
>
> David Gobbi wrote:
>
>> 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
>>>>
>>>>
>>>
>>
>>
>>
>