<br>Hi Patrick,<br><br>Thanks for checking on the usage of these methods.<br><br>I agree with both options.<br><br>We should:<br><br>A) Remove the SetRotation()/SetTranslation() methods<br><br>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; OR<br><br>B) Rename them to explicitly mention that they clear the
<br>&nbsp;&nbsp; &nbsp; other component.<br><br><br>I&#39;m slightly incline towards (B).<br><br><br>Ziv: Do you have a preference between these two options ?<br><br><br><br>&nbsp;&nbsp;&nbsp;&nbsp; Luis<br><br><br><br>------------------------------------------------------------------------------------
<br><div><span class="gmail_quote">On 8/6/07, <b class="gmail_sendername">Patrick Cheng</b> &lt;<a href="mailto:cheng@isis.georgetown.edu">cheng@isis.georgetown.edu</a>&gt; wrote:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi Luis and Ziv,<br><br>Making good sandwich is important! We need to be careful that we &#39;NEVER&#39;<br>set rotation and translation separately in our code, which will erase<br>each other in this case if we follow Luis&#39;s solution.
<br><br>I just did a quick search, at least in Tracker code, which deals with<br>all the transform composition, we are using SetTranslationAndRotation()<br>all the time.<br><br>I am wondering if we should provide<br>SetRotation() and SetTranslation() at all?
<br><br>These methods can be misleading, and I am afraid not many people will<br>read the documentation.<br><br>A name like:<br>SetRotationAndClearTranslation()<br>or<br>SetTranslationAndClearRotation()<br>Will be more explicit name.
<br><br>Patrick<br><br><br><br>Luis Ibanez wrote:<br>&gt;<br>&gt; Hi Ziv,<br>&gt;<br>&gt; Again, the problem with setting them separately is that the time stamp<br>&gt; is shared.<br>&gt;<br>&gt; The second setting will override the time stamp of the first setting and
<br>&gt; the Transform<br>&gt; will end up with a Translation component that does not match in time to<br>&gt; the Rotation<br>&gt; component.<br>&gt;<br>&gt; E.g. the Translation component may even have expired, and by setting
<br>&gt; only the<br>&gt; Rotation we will extend its validity time artificially.&nbsp;&nbsp;It is like<br>&gt; making a sandwich with<br>&gt; fresh cheese and an expired ham.... the cheese may be good, but the<br>&gt; total sandwich
<br>&gt; has gone bad anyways.<br>&gt;<br>&gt; If the pivot calibration is computing both components separately, then<br>&gt; they should only<br>&gt; be stored in an igstkTransform when both Translation and Rotation are<br>
&gt; available, and that<br>&gt; should be done using the SetRotationAndTranslation() method.<br>&gt;<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp;Luis<br>&gt;<br>&gt;<br>&gt; ----------------------------------------------------------------------------
<br>&gt; On 8/6/07, *Ziv Yaniv* &lt; <a href="mailto:zivy@isis.georgetown.edu">zivy@isis.georgetown.edu</a><br>&gt; &lt;mailto:<a href="mailto:zivy@isis.georgetown.edu">zivy@isis.georgetown.edu</a>&gt;&gt; wrote:<br>&gt;<br>
&gt;&nbsp;&nbsp;&nbsp;&nbsp; Hi Luis,<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; Setting rotation and translation separately without resetting the<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; transformation is reasonable behavior. Pivot calibration for example<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; only sets the translational part of a transformation. The user can
<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; set the rotational part after they get the transformation from the<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; pivot calibration according to prior knowledge (what Patrick is<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; doing right now). Of course there are many ways to do this
<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; (composing separate rotation and translation transformations to get<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; a single transform, or setting the transformation only when you have<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; both rotation and translation in hand).<br>&gt;
<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; In any case I view this as an arbitrary choice and my only<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; preference is that the documentation and implementation match.<br>&gt;<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;Ziv<br>&gt;<br>&gt;<br>&gt;<br>&gt;
<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; -----Original Message-----<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; From: &quot;Luis Ibanez&quot; &lt; <a href="mailto:luis.ibanez@kitware.com">luis.ibanez@kitware.com</a><br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; &lt;mailto:<a href="mailto:luis.ibanez@kitware.com">luis.ibanez@kitware.com
</a>&gt;&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; Sent 8/6/2007 12:41:56 PM<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; To: &quot;&#39;Ziv Yaniv&#39;&quot; &lt; <a href="mailto:zivy@isis.imac.georgetown.edu">zivy@isis.imac.georgetown.edu</a><br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; &lt;mailto:<a href="mailto:zivy@isis.imac.georgetown.edu">
zivy@isis.imac.georgetown.edu</a>&gt;&gt;, &quot;IGSTK Developers&quot;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; &lt;<a href="mailto:igstk-developers@public.kitware.com">igstk-developers@public.kitware.com</a><br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; &lt;mailto:<a href="mailto:igstk-developers@public.kitware.com">
igstk-developers@public.kitware.com</a>&gt;&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; Subject: igstkTransfrom documentation update &amp; Style problems<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; Hi Ziv,<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; Thanks for updating the documentation of the Transform class:
<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; <a href="http://public.kitware.com/cgi-bin/viewcvs.cgi/Source/igstkTransform.h?root=IGSTK&amp;sortby=date&amp;r2=1.20&amp;r1=1.19">http://public.kitware.com/cgi-bin/viewcvs.cgi/Source/igstkTransform.h?root=IGSTK&amp;sortby=date&amp;r2=1.20&amp;r1=1.19
</a><br>&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&lt;<a href="http://public.kitware.com/cgi-bin/viewcvs.cgi/Source/igstkTransform.h?root=IGSTK&amp;sortby=date&amp;r2=1.20&amp;r1=1.19">http://public.kitware.com/cgi-bin/viewcvs.cgi/Source/igstkTransform.h?root=IGSTK&amp;sortby=date&amp;r2=1.20&amp;r1=1.19
</a>&gt;<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; Unfortunately they introduced several coding style violations.<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; <a href="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</a><br>&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&lt;<a href="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</a>&gt;<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; Also, I&#39;m not sure that the right thing was to modify the documentation.<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; It seems that the SetRotation method actually has the bug of not setting
<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; the translation to identity, and the SetTranslation() method has the bug<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; of not setting the rotation to identity.<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; This should be necessary because the transform has a single TimeStamp,
<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; and we assume that it is the same for both Translation and Rotation.<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; I&#39;ll suggest that we put the documentation back, and fix the bug in<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; the implementation of the methods.<br>
&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; Do you see a reason why not initialized Translation / Rotation may<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp; bring any advantage ?<br>&gt;<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Thanks<br>&gt;<br>&gt;<br>&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;Luis<br>&gt;<br>&gt;<br>&gt;<br>
&gt;<br>&gt;<br>&gt; ------------------------------------------------------------------------<br>&gt;<br>&gt; _______________________________________________<br>&gt; IGSTK-Developers mailing list<br>&gt; <a href="mailto:IGSTK-Developers@public.kitware.com">
IGSTK-Developers@public.kitware.com</a><br>&gt; <a href="http://public.kitware.com/cgi-bin/mailman/listinfo/igstk-developers">http://public.kitware.com/cgi-bin/mailman/listinfo/igstk-developers</a><br><br><br></blockquote>
</div><br>