WIP: refactoring of the multiframe conversion implementation

This is an intermediate commit that aims to improve organization of the conversion
functionality, motivated by the various standing issues (e.g., QIICR#217, QIICR#192 and QIICR#150)
that are difficult to address with the current level of disarray in the code.

Some of the ideas are the following:

 - introduce basic hierarchy of classes to allow reuse of the common
   functionality between PM and SEG converters (MultiFrameConverter parent for
   ParametricMapConverter and SegmentationImageConverter)
 - do not use static functions, instead make better use of converter classes
 - factor out containers to keep information about ImageVolume and DICOMFrame
   to simplify conversion and book keeping
 - improve consistency of naming and code style

My plan to proceed with the refactoring is to implement "shadow" functions in the
updated classes while keeping the original functions initially, and then phase out
the original functions.

The current commit is just an intermediate milestone to open this for feedback
and keep development history at least somewhat incremental.

apps/paramaps/itkimage2paramap.cxx

@@ -3,7 +3,7 @@
 
 // DCMQI includes
 #undef HAVE_SSTREAM // Avoid redefinition warning
-#include "dcmqi/ParaMapConverter.h"
+#include "dcmqi/ParametricMapConverter.h"
 #include "dcmqi/internal/VersionConfigure.h"
 
 
@@ -44,7 +44,7 @@ int main(int argc, char *argv[])
   std::string metadata( (std::istreambuf_iterator<char>(metainfoStream) ),
                         (std::istreambuf_iterator<char>()));
 
-  DcmDataset* result = dcmqi::ParaMapConverter::itkimage2paramap(parametricMapImage, dcmDatasets, metadata);
+  DcmDataset* result = dcmqi::ParametricMapConverter::itkimage2paramap(parametricMapImage, dcmDatasets, metadata);
 
   if (result == NULL) {
     return EXIT_FAILURE;

apps/paramaps/paramap2itkimage.cxx

@@ -3,7 +3,7 @@
 
 // DCMQI includes
 #undef HAVE_SSTREAM // Avoid redefinition warning
-#include "dcmqi/ParaMapConverter.h"
+#include "dcmqi/ParametricMapConverter.h"
 #include "dcmqi/internal/VersionConfigure.h"
 
 
@@ -25,7 +25,7 @@ int main(int argc, char *argv[])
   CHECK_COND(sliceFF.loadFile(inputFileName.c_str()));
   DcmDataset* dataset = sliceFF.getDataset();
 
-  pair <FloatImageType::Pointer, string> result =  dcmqi::ParaMapConverter::paramap2itkimage(dataset);
+  pair <FloatImageType::Pointer, string> result =  dcmqi::ParametricMapConverter::paramap2itkimage(dataset);
 
   string fileExtension = helper::getFileExtensionFromType(outputType);
 

apps/seg/itkimage2segimage.cxx

@@ -3,7 +3,7 @@
 
 // DCMQI includes
 #undef HAVE_SSTREAM // Avoid redefinition warning
-#include "dcmqi/ImageSEGConverter.h"
+#include "dcmqi/SegmentationImageConverter.h"
 #include "dcmqi/internal/VersionConfigure.h"
 
 typedef dcmqi::Helper helper;
@@ -55,7 +55,7 @@ int main(int argc, char *argv[])
   ifstream metainfoStream(metaDataFileName.c_str(), ios_base::binary);
   std::string metadata( (std::istreambuf_iterator<char>(metainfoStream) ),
                        (std::istreambuf_iterator<char>()));
-  DcmDataset* result = dcmqi::ImageSEGConverter::itkimage2dcmSegmentation(dcmDatasets, segmentations, metadata, skipEmptySlices);
+  DcmDataset* result = dcmqi::SegmentationImageConverter::itkimage2dcmSegmentation(dcmDatasets, segmentations, metadata, skipEmptySlices);
 
   if (result == NULL){
     return EXIT_FAILURE;

apps/seg/segimage2itkimage.cxx

@@ -3,7 +3,7 @@
 
 // DCMQI includes
 #undef HAVE_SSTREAM // Avoid redefinition warning
-#include "dcmqi/ImageSEGConverter.h"
+#include "dcmqi/SegmentationImageConverter.h"
 #include "dcmqi/internal/VersionConfigure.h"
 
 
@@ -24,7 +24,7 @@ int main(int argc, char *argv[])
   CHECK_COND(sliceFF.loadFile(inputSEGFileName.c_str()));
   DcmDataset* dataset = sliceFF.getDataset();
 
-  pair <map<unsigned,ShortImageType::Pointer>, string> result =  dcmqi::ImageSEGConverter::dcmSegmentation2itkimage(dataset);
+  pair <map<unsigned,ShortImageType::Pointer>, string> result =  dcmqi::SegmentationImageConverter::dcmSegmentation2itkimage(dataset);
 
   string outputPrefix = prefix.empty() ? "" : prefix + "-";
 

include/dcmqi/DICOMFrame.h

@@ -0,0 +1,36 @@
+//
+// Created by Andrey Fedorov on 3/9/17.
+//
+
+#ifndef DCMQI_DICOMFRAME_H
+#define DCMQI_DICOMFRAME_H
+
+#include <dcmtk/dcmdata/dcdatset.h>
+
+namespace dcmqi {
+
+  // Describe input/output frames for the purposes of sorting and associating with the
+  // slices of the ITK volume
+  class DICOMFrame {
+  public:
+    // distinguish between the frames from the legacy data and enhanced multiframe objects
+    enum {
+      LegacyInstanceFrame = 0,
+      EnhancedInstanceFrame
+    };
+
+    DICOMFrame(DcmDataset *dataset, int number) :
+        frameNumber(number),
+        frameDataset(dataset) {
+
+    };
+
+  private:
+    int frameType;
+    DcmDataset *frameDataset;
+    int frameNumber;
+    OFString originStr; // revisit this
+  };
+}
+
+#endif //DCMQI_DICOMFRAME_H

include/dcmqi/ImageVolume.h

@@ -0,0 +1,66 @@
+//
+// Created by Andrey Fedorov on 3/9/17.
+//
+
+#ifndef DCMQI_IMAGEVOLUME_H
+#define DCMQI_IMAGEVOLUME_H
+
+#include <vnl/vnl_vector.h>
+#include <dcmtk/dcmfg/fginterface.h>
+#include <dcmtk/dcmiod/modfloatingpointimagepixel.h>
+
+#include <itkImage.h>
+
+namespace dcmqi {
+
+  // Maintain properties of the image volume
+  // Attributes ara parallel to those of itkImageData, but limited to what we need for the task of conversion,
+  // and the class is not templated over the pixel type
+  class ImageVolume {
+  public:
+    // pixel types that are relevant for the types of objects we want to support
+    enum {
+      FLOAT32 = 0, // PM
+      FLOAT64,     // PM
+      UINT16       // PM or SEG
+    };
+
+    typedef IODFloatingPointImagePixelModule::value_type Float32PixelType;
+    typedef itk::Image<Float32PixelType, 3> Float32ITKImageType;
+
+    ImageVolume(){
+      rowDirection.set_size(3);
+      columnDirection.set_size(3);
+      sliceDirection.set_size(3);
+      origin.set_size(3);
+      spacing.set_size(3);
+      pixelData = NULL;
+    }
+
+    // while going from DICOM PM/SEG, we get volume information from FGInterface
+    int initializeFromDICOM(FGInterface&);
+    int initializeFromITK(Float32ITKImageType::Pointer);
+
+    // TODO - inherited class? or separate segments before passing to this one?
+    // int initializeFromSegment(FGInterface&, unsigned);
+
+  protected:
+    int initializeDirections(FGInterface &);
+    int initializeExtent(FGInterface &);
+    bool getDeclaredSpacing(FGInterface&);
+
+  private:
+
+    // use vnl_vector to simplify support of vector calculations
+    vnl_vector<double> rowDirection, columnDirection, sliceDirection;
+    vnl_vector<double> origin;
+    unsigned sliceExtent;
+    vnl_vector<double> spacing;
+    void* pixelData;
+    int pixelDataType;
+  };
+
+};
+
+
+#endif //DCMQI_IMAGEVOLUME_H

include/dcmqi/ConverterBase.h → include/dcmqi/MultiframeConverter.h

@@ -35,13 +35,16 @@
 
 using namespace std;
 
+// common type definitions
 typedef short ShortPixelType;
 typedef itk::Image<ShortPixelType, 3> ShortImageType;
 typedef itk::ImageFileReader<ShortImageType> ShortReaderType;
 
 namespace dcmqi {
 
-  class ConverterBase {
+  class MultiframeConverter {
+  public:
+    virtual int convert();
 
   protected:
     static IODGeneralEquipmentModule::EquipmentInfo getEquipmentInfo();
@@ -124,7 +127,7 @@ namespace dcmqi {
       */
 
       // Determine ordering of the frames, keep mapping from ImagePositionPatient string
-      //   to the distance, and keep track (just out of curiousity) how many frames overlap
+      //   to the distance, and keep track (just out of curiosity) how many frames overlap
       vnl_vector<double> refOrigin(3);
       {
         OFBool isPerFrame;

include/dcmqi/ParaMapConverter.h → include/dcmqi/ParametricMapConverter.h

@@ -18,8 +18,9 @@
 #include <itkMinimumMaximumImageCalculator.h>
 
 // DCMQI includes
-#include "dcmqi/ConverterBase.h"
+#include "dcmqi/MultiframeConverter.h"
 #include "dcmqi/JSONParametricMapMetaInformationHandler.h"
+#include "dcmqi/ImageVolume.h"
 
 typedef IODFloatingPointImagePixelModule::value_type FloatPixelType;
 typedef itk::Image<FloatPixelType, 3> FloatImageType;
@@ -28,22 +29,52 @@ typedef itk::MinimumMaximumImageCalculator<FloatImageType> MinMaxCalculatorType;
 
 using namespace std;
 
-
 namespace dcmqi {
 
-  class ParaMapConverter : public ConverterBase {
+  DcmDataset* itkimage2paramapReplacement(const FloatImageType::Pointer &parametricMapImage, vector<DcmDataset*> dcmDatasets,
+                                                 const string &metaData);
+  pair <FloatImageType::Pointer, string> paramap2itkimageReplacement(DcmDataset *pmapDataset);
+
+  class ParametricMapConverter : public MultiframeConverter {
 
   public:
-    static DcmDataset* itkimage2paramap(const FloatImageType::Pointer &parametricMapImage, vector<DcmDataset*> dcmDatasets,
-                                        const string &metaData);
+    // placeholder to replace static function going from ITK to DICOM
+    ParametricMapConverter(const FloatImageType::Pointer &parametricMapImage, vector<DcmDataset*> dcmDatasets,
+                           const string &metaData);
 
+    // placeholder to replace static function going from DICOM to ITK
+    ParametricMapConverter(DcmDataset*);
+
+
+    static DcmDataset* itkimage2paramap(const FloatImageType::Pointer &parametricMapImage, vector<DcmDataset*> dcmDatasets,
+                                 const string &metaData);
     static pair <FloatImageType::Pointer, string> paramap2itkimage(DcmDataset *pmapDataset);
+
+    // do the conversion
+    int convert();
+
+    // get the result
+    FloatImageType::Pointer getFloat32ITKImage() const;
+    string getMetaData() const;
+    DcmDataset* getDataset() const;
+
   protected:
+
+    ParametricMapConverter() : parametricMapVolume(NULL), parametricMapDataset(NULL) {};
+
     static OFCondition addFrame(DPMParametricMapIOD &map, const FloatImageType::Pointer &parametricMapImage,
                                 const JSONParametricMapMetaInformationHandler &metaInfo, const unsigned long frameNo, OFVector<FGBase*> perFrameGroups);
 
     static void populateMetaInformationFromDICOM(DcmDataset *pmapDataset,
                                                  JSONParametricMapMetaInformationHandler &metaInfo);
+  private:
+    // these are the items we convert to and from, essentially
+    ImageVolume* parametricMapVolume;
+    DcmDataset* parametricMapDataset;
+
+    // these are the items we will need in the process of conversion
+    vector<DcmDataset*> referencedDatasets;
+    string metaData;
   };
 
 }

include/dcmqi/SegmentVolume.h

@@ -0,0 +1,17 @@
+//
+// Created by Andrey Fedorov on 3/9/17.
+//
+
+#ifndef DCMQI_SEGMENTVOLUME_H
+#define DCMQI_SEGMENTVOLUME_H
+
+#include "dcmqi/ImageVolume.h"
+
+namespace  dcmqi {
+  class SegmentVolume : public ImageVolume {
+
+  };
+}
+
+
+#endif //DCMQI_SEGMENTVOLUME_H

include/dcmqi/ImageSEGConverter.h → include/dcmqi/SegmentationImageConverter.h

@@ -22,7 +22,7 @@
 #include <itkChangeInformationImageFilter.h>
 
 // DCMQI includes
-#include "dcmqi/ConverterBase.h"
+#include "dcmqi/MultiframeConverter.h"
 #include "dcmqi/JSONSegmentationMetaInformationHandler.h"
 
 using namespace std;
@@ -32,7 +32,7 @@ typedef itk::LabelImageToLabelMapFilter<ShortImageType> LabelToLabelMapFilterTyp
 
 namespace dcmqi {
 
-  class ImageSEGConverter : public ConverterBase {
+  class SegmentationImageConverter : public MultiframeConverter {
 
   public:
     static DcmDataset* itkimage2dcmSegmentation(vector<DcmDataset*> dcmDatasets,

libsrc/CMakeLists.txt

@@ -1,49 +1,51 @@
 
 #-----------------------------------------------------------------------------
-set(ADDITIONAL_SRCS)
+set(ADDITIONAL_SRCS
+  DICOMFrame.cpp
+  ImageVolume.cpp
+  SegmentVolume.cpp
+  MultiframeConverter.cpp
+  Helper.cpp
+  SegmentationImageConverter.cpp
+  ParametricMapConverter.cpp
+  SegmentAttributes.cpp
+  JSONMetaInformationHandlerBase.cpp
+  JSONParametricMapMetaInformationHandler.cpp
+  JSONSegmentationMetaInformationHandler.cpp
+  )
+
+if(DCMQI_BUILTIN_JSONCPP)
+  list(APPEND ADDITIONAL_SRCS
+    ${DCMQI_SOURCE_DIR}/jsoncpp/jsoncpp.cpp
+    )
+  set(JsonCpp_INCLUDE_DIR ${DCMQI_SOURCE_DIR}/jsoncpp)
+endif()
 
 set(INCLUDE_DIR ../include/dcmqi)
 
-set(HDRS
+set(ADDITIONAL_HDRS
   ${INCLUDE_DIR}/preproc.h
   ${INCLUDE_DIR}/QIICRConstants.h
   ${INCLUDE_DIR}/QIICRUIDs.h
-  ${INCLUDE_DIR}/ConverterBase.h
   ${INCLUDE_DIR}/Exceptions.h
   ${INCLUDE_DIR}/framesorter.h
-  ${INCLUDE_DIR}/ImageSEGConverter.h
-  ${INCLUDE_DIR}/ParaMapConverter
   ${INCLUDE_DIR}/Helper.h
   ${INCLUDE_DIR}/JSONMetaInformationHandlerBase.h
   ${INCLUDE_DIR}/JSONParametricMapMetaInformationHandler.h
   ${INCLUDE_DIR}/JSONSegmentationMetaInformationHandler.h
   ${INCLUDE_DIR}/SegmentAttributes.h
+  ${INCLUDE_DIR}/DicomFrame.h
+  ${INCLUDE_DIR}/ImageVolume.h
+  ${INCLUDE_DIR}/SegmentVolume.h
+  ${INCLUDE_DIR}/SegmentationImageConverter.h
+  ${INCLUDE_DIR}/ParametricMapConverter.h
+  ${INCLUDE_DIR}/MultiframeConverter.h
   )
 
-set(SRCS
-  ConverterBase.cpp
-  ImageSEGConverter.cpp
-  ParaMapConverter.cpp
-  Helper.cpp
-  JSONMetaInformationHandlerBase.cpp
-  JSONParametricMapMetaInformationHandler.cpp
-  JSONSegmentationMetaInformationHandler.cpp
-  SegmentAttributes.cpp
-  )
-
-
-if(DCMQI_BUILTIN_JSONCPP)
-  list(APPEND ADDITIONAL_SRCS
-    ${DCMQI_SOURCE_DIR}/jsoncpp/jsoncpp.cpp
-    )
-  set(JsonCpp_INCLUDE_DIR ${DCMQI_SOURCE_DIR}/jsoncpp)
-endif()
-
 set(lib_name dcmqi)
 
 add_library(${lib_name} STATIC
-  ${HDRS}
-  ${SRCS}
+  ${ADDITIONAL_HDRS}
   ${ADDITIONAL_SRCS}
   )
 

libsrc/DICOMFrame.cpp

@@ -0,0 +1,5 @@
+//
+// Created by Andrey Fedorov on 3/9/17.
+//
+
+#include "dcmqi/DICOMFrame.h"