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

[Frank Lindseth]

Interesting sandwich-analogy Luis...
Suggestion: Do A) now and B) when needed.
(in the spirit of IGSTK: do not add anything unless needed...)

-  Frank


On Aug 6, 2007, at 10:03 PM, Luis Ibanez wrote:

>
> Hi Patrick,
>
> Thanks for checking on the usage of these methods.
>
> I agree with both options.
>
> We should:
>
> A) Remove the SetRotation()/SetTranslation() methods
>
>                 OR
>
> B) Rename them to explicitly mention that they clear the
>      other component.
>
>
> I'm slightly incline towards (B).
>
>
> Ziv: Do you have a preference between these two options ?
>
>
>
>      Luis
>
>
>
> ---------------------------------------------------------------------- 
> --------------
> On 8/6/07, Patrick Cheng <cheng at isis.georgetown.edu> wrote:
> 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
>
>
>
> _______________________________________________
> IGSTK-Developers mailing list
> IGSTK-Developers at public.kitware.com
> http://public.kitware.com/cgi-bin/mailman/listinfo/igstk-developers



------------------------------
Frank Lindseth
Research Scientist (PhD)

SINTEF Health Research
Dept. Medical Technology
N-7465 Trondheim, Norway
Location: Olav Kyrres gt. 9, 4th floor, Trondheim

E-mail: Frank.Lindseth at sintef.no
Telephone: +47 928 09 372
Telefax: +47 930 70 800


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/igstk-developers/attachments/20070807/a42f669f/attachment-0001.html>