BUG: update WriteCLI to fill a vector of strings, honor mutiple = false

Because Markups define multiple markups (with potentially multiple points) in one node,
it was writting multiple instances of a string + coordinates to the string stream passed
in to WriteCLI. This causes a problem for the parsing of the arguments in CLIs such as
FiducialRegistration which needed a separate string for each point.
With this change, the markup points are writen into a vector of strings, one string
for each point in each Markup.
Added the multipleFlag to be passed to WriteCLI, and if it's false, only write the
first selected markup to the output vector. If that markup has multiple points, it will
write all of those points.
Updated the Annotation control point to use the new WriteCLI signature, but because
of the way the points are written, it will only write the first point in a multi point
annotation (such as a ruler) if multiple is false, so it ignores it and the
CLI module logic takes care of ensuring that only the first child in an
annotation hierachy is written out.
Added a test for FiducialRegistration, updated the Markups node test 1 to test the single point flag.
Removed trailing white spaces.

Issue #1910
Issue #3345

Base/QTCLI/vtkSlicerCLIModuleLogic.cxx

@@ -1409,10 +1409,12 @@ void vtkSlicerCLIModuleLogic::ApplyTask(void *clientdata)
             }
           else if (markups && markups->IsA("vtkMRMLMarkupsNode"))
             {
-            std::ostringstream ss;
-            markups->WriteCLI(ss, prefix+flag, coordinateSystemFlag);
-            vtkDebugMacro("WriteCL markups output = " << ss.str());
-            commandLineAsString.push_back(ss.str());
+            int multipleFlag = 1;
+            if ((*pit).GetMultiple() == "false")
+              {
+              multipleFlag = 0;
+              }
+            markups->WriteCLI(commandLineAsString, prefix+flag, coordinateSystemFlag, multipleFlag);
             }
           else if (points)
             {
@@ -1426,6 +1428,11 @@ void vtkSlicerCLIModuleLogic::ApplyTask(void *clientdata)
               numChildren = col->GetNumberOfItems();
               }
             vtkDebugMacro("Displayable hierarchy has " << numChildren << " child nodes");
+            int multipleFlag = 1;
+            if ((*pit).GetMultiple() == "false")
+              {
+              multipleFlag = 0;
+              }
             for (unsigned int c = 0; c < numChildren; c++)
               {
               // the hierarchy nodes have a sorting index that's respected by
@@ -1443,10 +1450,12 @@ void vtkSlicerCLIModuleLogic::ApplyTask(void *clientdata)
                 if (displayableNode)
                   {
                   vtkDebugMacro("Found displayable node with id " << displayableNode->GetID());
-                  std::ostringstream ss;
-                  displayableNode->WriteCLI(ss, prefix+flag, coordinateSystemFlag);
-                  vtkDebugMacro("WriteCL output = " << ss.str());
-                  commandLineAsString.push_back(ss.str());
+                  displayableNode->WriteCLI(commandLineAsString, prefix+flag, coordinateSystemFlag);
+                  if (multipleFlag == 0)
+                    {
+                    // only write out the first child in the hierarchy
+                    break;
+                    }
                   }
                 }
               }

Libs/MRML/Core/vtkMRMLDisplayableNode.h

@@ -45,7 +45,7 @@ class VTK_MRML_EXPORT vtkMRMLDisplayableNode : public vtkMRMLStorableNode
 public:
   vtkTypeMacro(vtkMRMLDisplayableNode,vtkMRMLStorableNode);
   void PrintSelf(ostream& os, vtkIndent indent);
-  
+
   //--------------------------------------------------------------------------
   /// MRMLNode methods
   //--------------------------------------------------------------------------
@@ -54,32 +54,36 @@ class VTK_MRML_EXPORT vtkMRMLDisplayableNode : public vtkMRMLStorableNode
 
   virtual const char* GetNodeTagName() = 0;
 
-  /// 
+  ///
   /// Read node attributes from XML file
   virtual void ReadXMLAttributes( const char** atts);
-  
-  /// 
+
+  ///
   /// Write this node's information to a MRML file in XML format.
   virtual void WriteXML(ostream& of, int indent);
 
-  /// Write this node's information to a string for passing to a CLI, precede
-  /// each datum with the prefix if not an empty string
-  // coordinateSystemFlag = 0 for RAS, 1 for LPS, 2 for IJK
-  virtual void WriteCLI(std::ostringstream& vtkNotUsed(ss), std::string vtkNotUsed(prefix), int vtkNotUsed(coordinateSystemFlag) = 0) {};
+  /// Write this node's information to a vector of strings for passing to a CLI,
+  /// precede each datum with the prefix if not an empty string
+  /// coordinateSystemFlag = 0 for RAS, 1 for LPS, 2 for IJK
+  /// multipleFlag = 1 for the whole list, 0 for the first in the list
+  virtual void WriteCLI(std::vector<std::string>& vtkNotUsed(commandLine),
+			std::string vtkNotUsed(prefix),
+			int vtkNotUsed(coordinateSystemFlag) = 0,
+			int vtkNotUsed(multipleFlag) = 1) {};
 
-  /// 
+  ///
   /// Copy the node's attributes to this object
   virtual void Copy(vtkMRMLNode *node);
 
-  /// 
+  ///
   /// Convenience method that sets the first display node ID.
   /// \sa SetAndObserverNthDisplayNodeID(int, const char*)
   inline void SetAndObserveDisplayNodeID(const char *displayNodeID)
   {
     this->SetAndObserveNodeReferenceID(this->GetDisplayNodeReferenceRole(), displayNodeID);
   }
 
-  /// 
+  ///
   /// Convenience method that adds a display node ID at the end of the list.
   /// \sa SetAndObserverNthDisplayNodeID(int, const char*)
   inline void AddAndObserveDisplayNodeID(const char *displayNodeID)
@@ -102,7 +106,7 @@ class VTK_MRML_EXPORT vtkMRMLDisplayableNode : public vtkMRMLStorableNode
     this->RemoveAllNodeReferenceIDs(this->GetDisplayNodeReferenceRole());
   }
 
-  /// 
+  ///
   /// Set and observe the Nth display node ID in the list.
   /// If n is larger than the number of display nodes, the display node ID
   /// is added at the end of the list. If DisplayNodeID is 0, the node ID is
@@ -154,7 +158,7 @@ class VTK_MRML_EXPORT vtkMRMLDisplayableNode : public vtkMRMLStorableNode
     return this->GetNthDisplayNodeID(0);
   }
 
-  /// 
+  ///
   /// Get associated display MRML node. Can be 0 in temporary states; e.g. if
   /// the displayable node has no scene, or if the associated display is not
   /// yet into the scene.
@@ -181,12 +185,12 @@ class VTK_MRML_EXPORT vtkMRMLDisplayableNode : public vtkMRMLStorableNode
   /// const std::vector<vtkMRMLDisplayNode*>& GetDisplayNodes();
   /// \sa GetNumberOfDisplayNodes, GetNthDisplayNode
 
-  /// 
+  ///
   /// alternative method to propagate events generated in Display nodes
-  virtual void ProcessMRMLEvents ( vtkObject * /*caller*/, 
-                                   unsigned long /*event*/, 
+  virtual void ProcessMRMLEvents ( vtkObject * /*caller*/,
+                                   unsigned long /*event*/,
                                    void * /*callData*/ );
-  
+
   /// DisplayModifiedEvent is fired when:
   ///  - a new display node is observed
   ///  - a display node is not longer observed
@@ -229,7 +233,7 @@ class VTK_MRML_EXPORT vtkMRMLDisplayableNode : public vtkMRMLStorableNode
   virtual const char* GetDisplayNodeReferenceMRMLAttributeName();
 
   ///
-  /// Called when a node reference ID is added (list size increased). 
+  /// Called when a node reference ID is added (list size increased).
   virtual void OnNodeReferenceAdded(vtkMRMLNodeReference *reference)
   {
     Superclass::OnNodeReferenceAdded(reference);
@@ -240,15 +244,15 @@ class VTK_MRML_EXPORT vtkMRMLDisplayableNode : public vtkMRMLStorableNode
   }
 
   ///
-  /// Called when a node reference ID is modified. 
+  /// Called when a node reference ID is modified.
   virtual void OnNodeReferenceModified(vtkMRMLNodeReference *reference)
   {
     Superclass::OnNodeReferenceModified(reference);
     this->InvokeEvent(vtkMRMLDisplayableNode::DisplayModifiedEvent, reference->ReferencedNode);
   }
 
   ///
-  /// Called after a node reference ID is removed (list size decreased). 
+  /// Called after a node reference ID is removed (list size decreased).
   virtual void OnNodeReferenceRemoved(vtkMRMLNodeReference *reference)
   {
     Superclass::OnNodeReferenceRemoved(reference);

Modules/CLI/FiducialRegistration/FiducialRegistration.cxx

@@ -88,11 +88,11 @@ int main(int argc, char* argv[])
 {
   PARSE_ARGS;
 
+
   double invalidRMS = -1;
-
-
+
     // Checking conditions.
-  
+
   if( fixedLandmarks.size() <= 0 || movingLandmarks.size() <= 0 ||
     fixedLandmarks.size() != movingLandmarks.size() )
   {
@@ -115,33 +115,33 @@ int main(int argc, char* argv[])
 
   if( transformType != "Translation" && fixedLandmarks.size() < 3 )
   {
-    std::cerr << "At least 3 fiducual points must be specified for Rigid or Similarity transforms"
-      << std::endl;
+    std::cerr << "At least 3 fixed landmark fiducial points must be specified "
+	      << "for Rigid or Similarity transforms, have " << fixedLandmarks.size()
+	      << std::endl;
     if (outputMessage == "")
     {
-      outputMessage = "At least 3 fiducual points must be specified for Rigid or Similarity transforms";
+      outputMessage = "At least 3 fixed landmark fiducial points must be specified for Rigid or Similarity transforms";
       rms = invalidRMS;
     }
   }
-
-
+
     // Return if conditions not met.
 
   if ( rms == invalidRMS )
   {
     // Write out the return parameters in "name = value" form
     std::ofstream rts;
     rts.open(returnParameterFile.c_str() );
-    rts << "rms = " << rms << std::endl; 
+    rts << "rms = " << rms << std::endl;
     rts << "outputMessage = " << outputMessage <<std::endl;
     rts.close();
 
     return EXIT_SUCCESS;
   }
 
-  
+
   // only calculate if the above conditions hold
-  
+
   typedef  std::vector<itk::Point<double, 3> > PointList;
 
   PointList fixedPoints(fixedLandmarks.size() );
@@ -181,7 +181,7 @@ int main(int argc, char* argv[])
 
 
     // Handle different transform types.
-  
+
   if( transformType == "Translation" )
   {
     // Clear out the computed rotaitoin if we only requested translation
@@ -214,38 +214,38 @@ int main(int argc, char* argv[])
     std::cerr << "Unsupported transform type: " << transformType << std::endl;
     return EXIT_FAILURE;
   }
-  
-  
+
+
     // Convert into an affine transform for saving to Slicer.
 
   typedef itk::AffineTransform<double, 3> AffineTransform;
   AffineTransform::Pointer fixedToMovingT = itk::AffineTransform<double, 3>::New();
-  
+
   fixedToMovingT->SetCenter( transform->GetCenter() );
   fixedToMovingT->SetMatrix( transform->GetMatrix() );
   fixedToMovingT->SetTranslation( transform->GetTranslation() );
-  
-  
+
+
     // Compute RMS error in the target coordinate system.
-  
+
   AffineTransform::Pointer movingToFixedT = AffineTransform::New();
   fixedToMovingT->GetInverse( movingToFixedT );
-  
+
   typedef InitializerType::LandmarkPointContainer LandmarkPointContainerType;
 
   typedef LandmarkPointContainerType::const_iterator PointsContainerConstIterator;
   PointsContainerConstIterator mitr = movingPoints.begin();
   PointsContainerConstIterator fitr = fixedPoints.begin();
-  
+
   SimilarityTransformType::OutputVectorType                 errortr;
   SimilarityTransformType::OutputVectorType::RealValueType  sum;
   InitializerType::LandmarkPointType                        movingPointInFixed;
   int                                                       counter;
-  
+
   sum = itk::NumericTraits< SimilarityTransformType::OutputVectorType::RealValueType >::ZeroValue();
   counter = itk::NumericTraits< int >::ZeroValue();
-  
-  while( mitr != movingPoints.end() ) 
+
+  while( mitr != movingPoints.end() )
   {
     movingPointInFixed = movingToFixedT->TransformPoint( *mitr );
     errortr = *fitr - movingPointInFixed;
@@ -256,23 +256,23 @@ int main(int argc, char* argv[])
   }
 
   rms = sqrt( sum / counter );
-  
-  
+
+
   itk::TransformFileWriter::Pointer twriter = itk::TransformFileWriter::New();
   twriter->SetInput( fixedToMovingT );
   twriter->SetFileName( saveTransform );
 
   twriter->Update();
 
-  
+
   outputMessage = "Success";
-  
-  
+
+
     // Write out the return parameters in "name = value" form
-  
+
   std::ofstream rts;
   rts.open(returnParameterFile.c_str() );
-  rts << "rms = " << rms << std::endl; 
+  rts << "rms = " << rms << std::endl;
   rts << "outputMessage = " << outputMessage <<std::endl;
   rts.close();
 

Modules/CLI/FiducialRegistration/Testing/CMakeLists.txt

@@ -1,2 +1,25 @@
 # See http://www.na-mic.org/Bug/view.php?id=3340
 # message(WARNING "warning: Module ${MODULE_NAME} doesn't have any test !")
+
+#-----------------------------------------------------------------------------
+set(CLP ${MODULE_NAME})
+
+#-----------------------------------------------------------------------------
+set(TEMP ${Slicer_BINARY_DIR}/Testing/Temporary)
+
+#-----------------------------------------------------------------------------
+add_executable(${CLP}Test ${CLP}Test.cxx)
+target_link_libraries(${CLP}Test  ${CLP}Lib)
+set_target_properties(${CLP}Test PROPERTIES LABELS ${CLP})
+
+set(testname ${CLP}Test)
+add_test(NAME ${testname} COMMAND ${SEM_LAUNCH_COMMAND} $<TARGET_FILE:${CLP}Test>
+  ModuleEntryPoint
+ --returnparameterfile ${TEMP}/FiducialRegistration.params
+ --fixedLandmarks 37.5615,82.3156,0 --fixedLandmarks -26.373,81.5164,0 --fixedLandmarks -2.39754,-21.5779,0
+ --movingLandmarks 39.959,59.9385,30 --movingLandmarks -24.7746,62.3361,30 --movingLandmarks 0.79918,-52.7459,33.75
+ --saveTransform ${TEMP}/FiducialRegistration_vtkMRMLLinearTransformNodeE.txt
+ --transformType Rigid
+)
+
+set_property(TEST ${testname} PROPERTY LABELS ${CLP})

Modules/CLI/FiducialRegistration/Testing/FiducialRegistrationTest.cxx

@@ -0,0 +1,15 @@
+#include "itkTestMain.h"
+
+#ifdef WIN32
+#define MODULE_IMPORT __declspec(dllimport)
+#else
+#define MODULE_IMPORT
+#endif
+
+// Comment copied from ThesholdTest.cxx; This will be linked against the ModuleEntryPoint in RealignLib
+extern "C" MODULE_IMPORT int ModuleEntryPoint(int, char * []);
+
+void RegisterTests()
+{
+  StringToTestFunctionMap["ModuleEntryPoint"] = ModuleEntryPoint;
+}

Modules/Loadable/Annotations/MRML/vtkMRMLAnnotationControlPointsNode.cxx

@@ -91,22 +91,29 @@ void vtkMRMLAnnotationControlPointsNode::WriteXML(ostream& of, int nIndent)
     }
 
   of << indent << " ctrlPtsNumberingScheme=\"" << this->NumberingScheme << "\"";
-
 }
 
 //----------------------------------------------------------------------------
-void vtkMRMLAnnotationControlPointsNode::WriteCLI(std::ostringstream& ss, std::string prefix, int coordinateSystem)
+void vtkMRMLAnnotationControlPointsNode::
+WriteCLI(std::vector<std::string>& commandLine, std::string prefix,
+	 int coordinateSystem, int multipleFlag)
 {
-  Superclass::WriteCLI(ss, prefix, coordinateSystem);
+  Superclass::WriteCLI(commandLine, prefix, coordinateSystem, multipleFlag);
+
+  // Ignoring multipleFlag, because by convention there is only one annotation
+  // per node, so if there's a 6 point ROI, it needs to have all of it's
+  // points written out. The multiple flag is managed at the CLI module logic
+  // level
 
   if (this->GetPoints())
     {
     vtkPoints *points = this->GetPoints();
     int n = points->GetNumberOfPoints();
 
-    for (int i = 0; i < n; i++ ) 
+    for (int i = 0; i < n; i++ )
       {
       double* ptr = points->GetPoint(i);
+      std::stringstream ss;
       if (prefix.compare("") != 0)
         {
         ss << prefix << " ";
@@ -125,10 +132,7 @@ void vtkMRMLAnnotationControlPointsNode::WriteCLI(std::ostringstream& ss, std::s
         lps[2] = ptr[2];
         ss << lps[0] << "," <<  lps[1] << "," <<  lps[2] ;
         }
-      if (i < n-1)
-        {
-          ss << " ";
-        }
+      commandLine.push_back(ss.str());
       }
     }
 }