[IGSTK-Developers] Conclusion on igstkTransform issues - David&Patrick

[Luis Ibanez]

Hi Patrick,


You are right about the potential misuse of the long times.


We can remomve that risk when we add a specific type for
representing Time and TimeLapses, since that type could
define a constant that is considered to be an infinite time.


In practice we can take the NumericTraits::max() of the
type that we choose for internal representation of the time.
We could call that static value something like:


                Time::Forever()


In this way, the intent of the value will be clearer
to the readers of the code.



    Luis


-----------------------
Patrick Cheng wrote:
> Hi Luis,
> 
> I agree on statement 2) and 3).
> 
> But for 1), to be 'safe by design', we should not make any presumption 
> that an operation does no go over a "long period of time".
> 
> This "long duration" is easily being misused, and at least, it's not 
> consistent through out our code. some examples:
> 
> **********************************************************************
> [[[Super Long Period]]]
> igstkTrackerTool.cxx
> Line 31
>   m_ToolCalibrationTransform.SetToIdentity( 1e300 );
> 
> igstkMeshReader.cxx
> line91-92:
>   // Provide an identity transform with a long validity time.
>   const double validityTimeInMilliseconds = 1e30;
> 
> [[[Wrong]]]
> igstkPivotCalibration.cxx
> Line 149
> this->m_CalibrationTransform.SetTranslationAndRotation( translation, 
> quaternion, 0.1, 1000);
> Line 286
> this->m_CalibrationTransform.SetTranslation(translation, 0.1, 1000);
> 
> igstkPrincipalAxisCalibration.cxx
> Line 204
> this->m_CalibrationTransform.SetRotation( quaternion, 0.1, 1000);
> ************************************************************************
> 
> Luis Ibanez wrote:
> 
>>
>> Hi Patrick,
>>
>>
>> 1) The double use of Static and Dynamic transform will require
>>    SpatialObject to have both of them as member variables.
>>    Since SpatialObjects should be able to be "static" or "tracked".
>>
>>    What is the difficulty with having the "static" transforms be
>>    represented with a time stamp of long duration ?
>>
>>
>> 2) When composing transforms, the time stamp of the final transform
>>    must by the intersection among all the time stamps of the transforms
>>    involved on the composition.
>>
>>    We could add to the TimeStamp class, as service method that will
>>    compote the intersection between two TimeStamps. This operation
>>    is associative, so by applying it by pairs we can compose any
>>    number of TimeStamps.
>>
>>
>> 3) It is better to have a explicit "Compose" method, than having
>>    an operator overload.  The Compose method name could also make
>>    clearer whether we are pre-composing or post-composing the
>>    transform.
>>
>>
>>
>>
>>   Luis
>>
>>
>>
>> ---------------------
>> Patrick Cheng wrote:
>>
>>> Hi everybody,
>>>
>>> This is the conclusion of the discussion between david and me. 
>>> Welcome to comment on it.
>>>
>>> =======================================================================
>>>
>>> Problem 1. We need both static and dynamic transform. (Registration 
>>> and calibration transforms should be static, and tracker transforms 
>>> should be dynamic)
>>>
>>> Solution: Make subclasses of igstk::Transform,
>>> igstk::DynamicTransform and igstk::StaticTransform StaticTransform 
>>> which do not have time stamp. In base class we provide pure virtual 
>>> fuction:
>>> bool IsStatic()/IsDynamic();
>>>
>>>   Q? Do we some times have to switch the transform of a object from 
>>> dynamic to static or the other way around?
>>>
>>> =======================================================================
>>>
>>> Problem 2. We currently don't have a simple transform compose method, 
>>> and we are not taking care of time stamps in the current 
>>> implementation of the transform multiplication.
>>>
>>> e.g.
>>> Ta is static transform, Tb is dynamic, and Tc is dynamic.
>>> When we do T = Ta * Tb * Tc
>>> to get the final valid time stamp for T, we should first ignore Ta, 
>>> and pick up the earlier expiration time from Tb and Tc, and minus 
>>> current time, to get the valid time period for the T, and set a right 
>>> time stamp for it.
>>>
>>> Solution: Add a simple function or operator such as:
>>> transform = transform1.compose( transform2 )
>>> equals to:
>>> transform = transform1 * transform2
>>>
>>> This will avoid the wrong matrix composition, and make code simpler 
>>> and cleaner.
>>> Also this compose() method will calculate the correct time stamp 
>>> automatically according to predefine rules.
>>>
>>> =======================================================================
>>>
>>> David, I hope you still agree on these points.
>>>
>>>
>>
>>
>>
> 
>