[IGSTK-Developers] Re: igstkTransfrom documentation update & Style problems

[Patrick Cheng]

Hi Luis and Ziv,

Making good sandwich is important! We need to be careful that we 'NEVER' 
set rotation and translation separately in our code, which will erase 
each other in this case if we follow Luis's solution.

I just did a quick search, at least in Tracker code, which deals with 
all the transform composition, we are using SetTranslationAndRotation() 
all the time.

I am wondering if we should provide
SetRotation() and SetTranslation() at all?

These methods can be misleading, and I am afraid not many people will 
read the documentation.

A name like:
SetRotationAndClearTranslation()
or
SetTranslationAndClearRotation()
Will be more explicit name.

Patrick



Luis Ibanez wrote:
> 
> Hi Ziv,
> 
> Again, the problem with setting them separately is that the time stamp 
> is shared.
> 
> The second setting will override the time stamp of the first setting and 
> the Transform
> will end up with a Translation component that does not match in time to 
> the Rotation
> component.
> 
> E.g. the Translation component may even have expired, and by setting 
> only the
> Rotation we will extend its validity time artificially.  It is like 
> making a sandwich with
> fresh cheese and an expired ham.... the cheese may be good, but the 
> total sandwich
> has gone bad anyways.
> 
> If the pivot calibration is computing both components separately, then 
> they should only
> be stored in an igstkTransform when both Translation and Rotation are 
> available, and that
> should be done using the SetRotationAndTranslation() method.
> 
> 
>    Luis
> 
> 
> ----------------------------------------------------------------------------
> On 8/6/07, *Ziv Yaniv* < zivy at isis.georgetown.edu 
> <mailto:zivy at isis.georgetown.edu>> wrote:
> 
>     Hi Luis,
> 
>     Setting rotation and translation separately without resetting the
>     transformation is reasonable behavior. Pivot calibration for example
>     only sets the translational part of a transformation. The user can
>     set the rotational part after they get the transformation from the
>     pivot calibration according to prior knowledge (what Patrick is
>     doing right now). Of course there are many ways to do this
>     (composing separate rotation and translation transformations to get
>     a single transform, or setting the transformation only when you have
>     both rotation and translation in hand).
> 
>     In any case I view this as an arbitrary choice and my only
>     preference is that the documentation and implementation match.
> 
> 
>                  Ziv
> 
> 
> 
> 
>     -----Original Message-----
>     From: "Luis Ibanez" < luis.ibanez at kitware.com
>     <mailto:luis.ibanez at kitware.com>>
>     Sent 8/6/2007 12:41:56 PM
>     To: "'Ziv Yaniv'" < zivy at isis.imac.georgetown.edu
>     <mailto:zivy at isis.imac.georgetown.edu>>, "IGSTK Developers"
>     <igstk-developers at public.kitware.com
>     <mailto:igstk-developers at public.kitware.com>>
>     Subject: igstkTransfrom documentation update & Style problems
> 
>     Hi Ziv,
> 
>     Thanks for updating the documentation of the Transform class:
> 
>     http://public.kitware.com/cgi-bin/viewcvs.cgi/Source/igstkTransform.h?root=IGSTK&sortby=date&r2=1.20&r1=1.19
>      <http://public.kitware.com/cgi-bin/viewcvs.cgi/Source/igstkTransform.h?root=IGSTK&sortby=date&r2=1.20&r1=1.19>
> 
>     Unfortunately they introduced several coding style violations.
>     http://public.kitware.com/IGSTK/Sites/insight.journal.kitware/KWStyle/20070806-0430-Nightly/BuildError.html
>      <http://public.kitware.com/IGSTK/Sites/insight.journal.kitware/KWStyle/20070806-0430-Nightly/BuildError.html>
> 
>     Also, I'm not sure that the right thing was to modify the documentation.
>     It seems that the SetRotation method actually has the bug of not setting
>     the translation to identity, and the SetTranslation() method has the bug
>     of not setting the rotation to identity.
> 
>     This should be necessary because the transform has a single TimeStamp,
>     and we assume that it is the same for both Translation and Rotation.
> 
>     I'll suggest that we put the documentation back, and fix the bug in
>     the implementation of the methods.
> 
>     Do you see a reason why not initialized Translation / Rotation may
>     bring any advantage ?
> 
> 
>         Thanks
> 
> 
>            Luis
> 
> 
> 
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> IGSTK-Developers mailing list
> IGSTK-Developers at public.kitware.com
> http://public.kitware.com/cgi-bin/mailman/listinfo/igstk-developers