cleanup meta importer

vtkext/private/module/vtkF3DMetaImporter.cxx

@@ -33,7 +33,8 @@ void ClearColoringArrays(bool useCellData)
 
 void UpdateColoringVectors(vtkDataSet* dataset, bool useCellData)
 {
-  assert(dataset); // TODO enough ?
+  // XXX: This assumes importer do not import actors with an empty input
+  assert(dataset);
 
   // Recover all possible names
   std::set<std::string> arrayNames;
@@ -169,7 +170,6 @@ void vtkF3DMetaImporter::AddImporter(vtkSmartPointer<vtkImporter> importer)
   this->Modified();
 
   // Add a progress event observer
-  // TODO check this works
   vtkNew<vtkCallbackCommand> progressCallback;
   progressCallback->SetClientData(this);
   progressCallback->SetCallback(
@@ -183,6 +183,7 @@ void vtkF3DMetaImporter::AddImporter(vtkSmartPointer<vtkImporter> importer)
         if (self->Pimpl->Importers[i].first == caller)
         {
           // XXX: This does not consider that some importer may already have been updated
+          // or that some importers may take much longer than other.
           actualProgress = (i + progress) / self->Pimpl->Importers.size();
         }
       }
@@ -286,7 +287,9 @@ const std::vector<vtkF3DMetaImporter::VolumeStruct>& vtkF3DMetaImporter::GetVolu
 //----------------------------------------------------------------------------
 bool vtkF3DMetaImporter::Update()
 {
-  // XXX: This is the simplest way to implement coloring
+  // XXX: Doing this means that coloring information
+  // is recomputed from scratch when doing incremental loading
+  // This is the simplest way to implement coloring
   // and should not have too big of an overhead.
   this->Pimpl->ColoringInfoUpdated = false;
 
@@ -465,7 +468,6 @@ std::string vtkF3DMetaImporter::GetAnimationName(vtkIdType animationIndex)
       localAnimationIndex -= nAnim;
     }
   }
-  // TODO error ?
   return "";
 }
 
@@ -486,7 +488,6 @@ void vtkF3DMetaImporter::EnableAnimation(vtkIdType animationIndex)
       localAnimationIndex -= nAnim;
     }
   }
-  // TODO error ?
 }
 
 //----------------------------------------------------------------------------
@@ -506,7 +507,6 @@ void vtkF3DMetaImporter::DisableAnimation(vtkIdType animationIndex)
       localAnimationIndex -= nAnim;
     }
   }
-  // TODO error ?
 }
 
 //----------------------------------------------------------------------------
@@ -525,7 +525,6 @@ bool vtkF3DMetaImporter::IsAnimationEnabled(vtkIdType animationIndex)
       localAnimationIndex -= nAnim;
     }
   }
-  // TODO error ?
   return false;
 }
 
@@ -556,7 +555,6 @@ std::string vtkF3DMetaImporter::GetCameraName(vtkIdType camIndex)
       localCameraIndex -= nCam;
     }
   }
-  // TODO error ?
   return "";
 
 }
@@ -583,7 +581,6 @@ bool vtkF3DMetaImporter::GetTemporalInformation(vtkIdType animationIndex, double
       localAnimationIndex -= nAnim;
     }
   }
-  // TODO error ?
   return false;
 }
 

vtkext/private/module/vtkF3DMetaImporter.h

@@ -28,7 +28,10 @@ class vtkF3DMetaImporter : public vtkF3DImporter
   static vtkF3DMetaImporter* New();
   vtkTypeMacro(vtkF3DMetaImporter, vtkF3DImporter);
 
-  // TODO better way to do this ?
+  ///@{
+  /**
+   * Structs used to transfer actors information to the F3D renderer
+   */
   struct VolumeStruct
   {
     VolumeStruct()
@@ -56,6 +59,7 @@ class vtkF3DMetaImporter : public vtkF3DImporter
     vtkActor* Actor;
     vtkPolyDataMapper* Mapper;
   };
+  ///@}
 
   /**
    * A struct containing information about possible coloring
@@ -72,9 +76,13 @@ class vtkF3DMetaImporter : public vtkF3DImporter
   };
 
   /**
-   * TODO
+   * Clear all importers and internal structures
    */
   void Clear();
+
+  /**
+   * Add an importer to update when importer all actors
+   */
   void AddImporter(vtkSmartPointer<vtkImporter> importer);
 
   /**
@@ -84,7 +92,7 @@ class vtkF3DMetaImporter : public vtkF3DImporter
   const vtkBoundingBox& GetGeometryBoundingBox();
 
   /**
-   * TODO
+   * Get a meta data description of all imported data
    */
   std::string GetMetaDataDescription() const;
 
@@ -106,24 +114,56 @@ class vtkF3DMetaImporter : public vtkF3DImporter
    */
   int FindIndexForColoring(bool useCellData, const std::string& arrayName);
 
-  // TODO better API ?
+  ///@{
+  /**
+   * API to recover information about all imported actors, point sprites and volume if any
+   */
   const std::vector<GeometryStruct>& GetGeometryActorsAndMappers();
   const std::vector<PointSpritesStruct>& GetPointSpritesActorsAndMappers();
   const std::vector<VolumeStruct>& GetVolumePropsAndMappers();
+  ///@}
 
-  // XXX: This is NOT an override, this is hiding
+  /**
+   * XXX: HIDE the vtkImporter::Update method and declare our own
+   * Import each of of the add importers into the first renderer of the render window.
+   * Importers that have already been imported will be skipped
+   * Also handles camera index if specified
+   * After import, create point sprites actors for all importers, and volume props
+   * for generic importer if compatible.
+   */
   bool Update();
 
+  /**
+   * Concatenate individual importers output description into one and return it
+   */
   std::string GetOutputsDescription() override;
+
+  ///@{
+  /**
+   * Implement vtkImporter animation API by adding animations for each individual importers one after the other
+   * No input checking on animationIndex
+   */
   vtkIdType GetNumberOfAnimations() override;
-  std::string GetAnimationName(vtkIdType vtkNotUsed(animationIndex)) override;
-  void EnableAnimation(vtkIdType vtkNotUsed(animationIndex)) override;
-  void DisableAnimation(vtkIdType vtkNotUsed(animationIndex)) override;
-  bool IsAnimationEnabled(vtkIdType vtkNotUsed(animationIndex)) override;
-  vtkIdType GetNumberOfCameras() override;
-  std::string GetCameraName(vtkIdType vtkNotUsed(camIndex)) override;
-  void SetCamera(vtkIdType vtkNotUsed(camIndex)) override;
+  std::string GetAnimationName(vtkIdType animationIndex) override;
+  void EnableAnimation(vtkIdType animationIndex) override;
+  void DisableAnimation(vtkIdType animationIndex) override;
+  bool IsAnimationEnabled(vtkIdType animationIndex) override;
   bool GetTemporalInformation(vtkIdType animationIndex, double frameRate, int& nbTimeSteps, double timeRange[2], vtkDoubleArray* timeSteps) override;
+  ///@}
+
+  ///@{
+  /**
+   * Implement vtkImporter camera API by adding cameras for each individual importers one after the other
+   * No input checking on camIndex
+   */
+  vtkIdType GetNumberOfCameras() override;
+  std::string GetCameraName(vtkIdType camIndex) override;
+  void SetCamera(vtkIdType camIndex) override;
+  ///@}
+
+  /**
+   * Update each individual importer at the provided value
+   */
   bool UpdateAtTimeValue(double timeValue) override;
 
 protected:
@@ -134,6 +174,10 @@ class vtkF3DMetaImporter : public vtkF3DImporter
   vtkF3DMetaImporter(const vtkF3DMetaImporter&) = delete;
   void operator=(const vtkF3DMetaImporter&) = delete;
 
+  /**
+   * Recover coloring information from each individual importer
+   * and store result in internal fields
+   */
   void UpdateInfoForColoring();
 
   struct Internals;