# HG changeset patch
# User Benjamin Golinvaux <bgo@osimis.io>
# Date 1567755498 -7200
# Node ID c20dbaab360c8d69777bfaa768a996184a3d7291
# Parent  0e21ecafcc237a0c06cd5c62874d0e9376a3af9d
Ability to cope with empty "Referenced SOP Instance UID" (dicom path (3006,0039)[i] / (0x3006, 0x0040)[0] / (0x3006, 0x0016)[0] / (0x0008, 0x1155)) + better logs + code formating

diff -r 0e21ecafcc23 -r c20dbaab360c Framework/Loaders/DicomStructureSetLoader.cpp
--- a/Framework/Loaders/DicomStructureSetLoader.cpp	Mon Sep 02 17:44:20 2019 +0200
+++ b/Framework/Loaders/DicomStructureSetLoader.cpp	Fri Sep 06 09:38:18 2019 +0200
@@ -64,10 +64,6 @@
 
     virtual void Handle(const OrthancRestApiCommand::SuccessMessage& message)
     {
-#if 0
-      if (logbgo115)
-        LOG(TRACE) << "DicomStructureSetLoader::AddReferencedInstance::Handle() (SUCCESS)";
-#endif
       Json::Value tags;
       message.ParseJsonBody(tags);
         
@@ -76,17 +72,6 @@
 
       DicomStructureSetLoader& loader = GetLoader<DicomStructureSetLoader>();
 
-#if 0
-      {
-        std::stringstream ss;
-        //DumpDicomMap(ss, dicom);
-        std::string dicomMapStr = ss.str();
-        if (logbgo115)
-          LOG(TRACE) << "  DicomStructureSetLoader::AddReferencedInstance::Handle() about to call AddReferencedSlice on dicom = " << dicomMapStr;
-      }
-#endif
-
-
       loader.content_->AddReferencedSlice(dicom);
 
       loader.countProcessedInstances_ ++;
@@ -150,24 +135,12 @@
       const std::string instanceId = lookup[0]["ID"].asString();
 
       {
-#if 0
-        if(logbgo115)
-          LOG(TRACE) << "DicomStructureSetLoader::LookupInstance::Handle() (SUCCESS)";
-#endif
         std::auto_ptr<OrthancRestApiCommand> command(new OrthancRestApiCommand);
         command->SetHttpHeader("Accept-Encoding", "gzip");
         std::string uri = "/instances/" + instanceId + "/tags";
         command->SetUri(uri);
         command->SetPayload(new AddReferencedInstance(loader, instanceId));
-#if 0
-        if (logbgo115)
-          LOG(TRACE) << "  DicomStructureSetLoader::LookupInstance::Handle() about to schedule request with AddReferencedInstance subsequent command on uri \"" << uri << "\"";
-#endif
         Schedule(command.release());
-#if 0
-        if (logbgo115)
-          LOG(TRACE) << "  DicomStructureSetLoader::LookupInstance::Handle() request+command scheduled";
-#endif
       }
     }
   };
@@ -195,27 +168,30 @@
         loader.content_.reset(new DicomStructureSet(dicom));
       }
 
+      // Some (admittedly invalid) Dicom files have empty values in the 
+      // 0008,1155 tag. We try our best to cope with this.
       std::set<std::string> instances;
+      std::set<std::string> nonEmptyInstances;
       loader.content_->GetReferencedInstances(instances);
+      for (std::set<std::string>::const_iterator
+        it = instances.begin(); it != instances.end(); ++it)
+      {
+        std::string instance = Orthanc::Toolbox::StripSpaces(*it);
+        if(instance != "")
+          nonEmptyInstances.insert(instance);
+      }
 
-      loader.countReferencedInstances_ = static_cast<unsigned int>(instances.size());
+      loader.countReferencedInstances_ = 
+        static_cast<unsigned int>(nonEmptyInstances.size());
 
       for (std::set<std::string>::const_iterator
-             it = instances.begin(); it != instances.end(); ++it)
+        it = nonEmptyInstances.begin(); it != nonEmptyInstances.end(); ++it)
       {
         std::auto_ptr<OrthancRestApiCommand> command(new OrthancRestApiCommand);
         command->SetUri("/tools/lookup");
         command->SetMethod(Orthanc::HttpMethod_Post);
         command->SetBody(*it);
-        
-        // The following headers have been commented out because
-        // they were causing issues in the reverse proxy in a dev scenario.
-        // They should NOT be required for POST requests
-        //command->SetHttpHeader("pragma", "no-cache");
-        //command->SetHttpHeader("cache-control", "no-cache");
-
         command->SetPayload(new LookupInstance(loader, *it));
-        //LOG(TRACE) << "About to schedule a /tools/lookup POST request. URI = " << command->GetUri() << " Body size = " << (*it).size() << " Body = " << (*it) << "\n";
         Schedule(command.release());
       }
     }
@@ -267,7 +243,7 @@
       {
         const Color& color = content_.GetStructureColor(i);
 
-        std::vector< std::vector<DicomStructureSet::PolygonPoint> > polygons;
+        std::vector< std::vector<DicomStructureSet::PolygonPoint2D> > polygons;
           
         if (content_.ProjectStructure(polygons, i, cuttingPlane))
         {
@@ -319,17 +295,10 @@
       command->SetHttpHeader("Accept-Encoding", "gzip");
 
       std::string uri = "/instances/" + instanceId + "/tags?ignore-length=3006-0050";
-#if 0
-      if (logbgo115)
-        LOG(TRACE) << "DicomStructureSetLoader::LoadInstance() instanceId = " << instanceId << " | uri = \"" << uri << "\"";
-#endif
+
       command->SetUri(uri);
       command->SetPayload(new LoadStructure(*this));
       Schedule(command.release());
-#if 0
-      if (logbgo115)
-        LOG(TRACE) << "DicomStructureSetLoader::LoadInstance() command (with LoadStructure) scheduled.";
-#endif
     }
   }
 
diff -r 0e21ecafcc23 -r c20dbaab360c Framework/Messages/ICallable.h
--- a/Framework/Messages/ICallable.h	Mon Sep 02 17:44:20 2019 +0200
+++ b/Framework/Messages/ICallable.h	Fri Sep 06 09:38:18 2019 +0200
@@ -92,7 +92,9 @@
       }
       else
       {
-        LOG(TRACE) << "The recorded fingerprint is " << observerFingerprint_ << " and the current fingerprint is " << currentFingerprint << " -- callable will be called.";
+        LOG(TRACE) << "The recorded fingerprint is " << observerFingerprint_
+          << " and the current fingerprint is " << currentFingerprint
+          << " -- callable will be called.";
         (observer_.*function_) (message);
       }
     }
diff -r 0e21ecafcc23 -r c20dbaab360c Framework/Toolbox/DicomStructureSet.cpp
--- a/Framework/Toolbox/DicomStructureSet.cpp	Mon Sep 02 17:44:20 2019 +0200
+++ b/Framework/Toolbox/DicomStructureSet.cpp	Fri Sep 06 09:38:18 2019 +0200
@@ -25,6 +25,7 @@
 
 #include <Core/Logging.h>
 #include <Core/OrthancException.h>
+#include <Core/Toolbox.h>
 #include <Plugins/Samples/Common/FullOrthancDataset.h>
 #include <Plugins/Samples/Common/DicomDatasetReader.h>
 
@@ -126,26 +127,66 @@
             LinearAlgebra::ParseVector(target, value));
   }
 
-
-  void DicomStructureSet::Polygon::CheckPoint(const Vector& v)
+  void DicomStructureSet::Polygon::CheckPointIsOnSlice(const Vector& v) const
   {
     if (hasSlice_)
     {
-      if (!LinearAlgebra::IsNear(GeometryToolbox::ProjectAlongNormal(v, geometry_.GetNormal()),
-                                 projectionAlongNormal_, 
-                                 sliceThickness_ / 2.0 /* in mm */))
+      double magnitude =
+        GeometryToolbox::ProjectAlongNormal(v, geometry_.GetNormal());
+      if(!LinearAlgebra::IsNear(
+        magnitude,
+        projectionAlongNormal_,
+        sliceThickness_ / 2.0 /* in mm */ ))
       {
-        LOG(ERROR) << "This RT-STRUCT contains a point that is off the slice of its instance";
-        throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat);          
+        LOG(ERROR) << "This RT-STRUCT contains a point that is off the "
+          << "slice of its instance | "
+          << "magnitude = " << magnitude << " | "
+          << "projectionAlongNormal_ = " << projectionAlongNormal_ << " | "
+          << "tolerance (sliceThickness_ / 2.0) = " << (sliceThickness_ / 2.0);
+
+        throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat);
       }
     }
   }
 
+  bool DicomStructureSet::Polygon::IsPointOnSlice(const Vector& v) const
+  {
+    if (hasSlice_)
+    {
+      double magnitude = 
+        GeometryToolbox::ProjectAlongNormal(v, geometry_.GetNormal());
+      bool onSlice = LinearAlgebra::IsNear(
+        magnitude,
+        projectionAlongNormal_,
+        sliceThickness_ / 2.0 /* in mm */);
+      if (!onSlice)
+      {
+        LOG(WARNING) << "This RT-STRUCT contains a point that is off the "
+          << "slice of its instance | "
+          << "magnitude = " << magnitude << " | "
+          << "projectionAlongNormal_ = " << projectionAlongNormal_ << " | "
+          << "tolerance (sliceThickness_ / 2.0) = " << (sliceThickness_ / 2.0);
+      }
+      return onSlice;
+    }
+    else
+    {
+      return false;
+    }
+  }
 
   void DicomStructureSet::Polygon::AddPoint(const Vector& v)
   {
+#if 1
+    // BGO 2019-09-03
+    if (IsPointOnSlice(v))
+    {
+      points_.push_back(v);
+    }
+#else
     CheckPoint(v);
     points_.push_back(v);
+#endif 
   }
 
 
@@ -176,22 +217,21 @@
         
         for (Points::const_iterator it = points_.begin(); it != points_.end(); ++it)
         {
-          CheckPoint(*it);
-
-          double x, y;
-          geometry.ProjectPoint(x, y, *it);
-          extent_.AddPoint(x, y);
+          if (IsPointOnSlice(*it))
+          {
+            double x, y;
+            geometry.ProjectPoint(x, y, *it);
+            extent_.AddPoint(x, y);
+          }
         }
-
         return true;
       }
     }
   }
 
-
   bool DicomStructureSet::Polygon::IsOnSlice(const CoordinateSystem3D& slice) const
   {
-    bool isOpposite;
+    bool isOpposite = false;
 
     if (points_.empty() ||
         !hasSlice_ ||
@@ -205,15 +245,14 @@
     return (LinearAlgebra::IsNear(d, projectionAlongNormal_,
                                   sliceThickness_ / 2.0));
   }
-
-  
+    
   bool DicomStructureSet::Polygon::Project(double& x1,
                                            double& y1,
                                            double& x2,
                                            double& y2,
                                            const CoordinateSystem3D& slice) const
   {
-    // TODO Optimize this method using a sweep-line algorithm for polygons
+    // TODO: optimize this method using a sweep-line algorithm for polygons
     
     if (!hasSlice_ ||
         points_.size() <= 1)
@@ -432,6 +471,7 @@
                                                   DICOM_TAG_CONTOUR_SEQUENCE, 0,
                                                   DICOM_TAG_CONTOUR_IMAGE_SEQUENCE);
 
+      // (3006,0039)[i] / (0x3006, 0x0040)[0] / (0x3006, 0x0016)[0] / (0x0008, 0x1155)
       OrthancPlugins::DicomPath referencedInstancePath(DICOM_TAG_ROI_CONTOUR_SEQUENCE, i,
                                                        DICOM_TAG_CONTOUR_SEQUENCE, 0,
                                                        DICOM_TAG_CONTOUR_IMAGE_SEQUENCE, 0,
@@ -464,9 +504,9 @@
         size_t size;
 
         imageSequencePath.SetPrefixIndex(1, j);
-        if (!tags.GetSequenceSize(size, imageSequencePath) ||
-            size != 1)
+        if (!tags.GetSequenceSize(size, imageSequencePath) || size != 1)
         {
+          LOG(ERROR) << "The ContourImageSequence sequence (tag 3006,0016) must be present and contain one entry.";
           throw Orthanc::OrthancException(Orthanc::ErrorCode_NotImplemented);          
         }
 
@@ -483,6 +523,12 @@
           throw Orthanc::OrthancException(Orthanc::ErrorCode_BadFileFormat);          
         }
 
+        // seen in real world
+        if(Orthanc::Toolbox::StripSpaces(sopInstanceUid) == "") 
+        {
+          LOG(ERROR) << "WARNING. The following Dicom tag (Referenced SOP Instance UID) contains an empty value : // (3006,0039)[" << i << "] / (0x3006, 0x0040)[0] / (0x3006, 0x0016)[0] / (0x0008, 0x1155)";
+        }
+
         Polygon polygon(sopInstanceUid);
         polygon.Reserve(countPoints);
 
@@ -581,11 +627,8 @@
     {
       // This geometry is already known
       LOG(ERROR) << "DicomStructureSet::AddReferencedSlice(): (referencedSlices_.find(sopInstanceUid) != referencedSlices_.end()). sopInstanceUid = " << sopInstanceUid;
-      
-      // TODO: the following assertion has been disabled on 20190822 by BGO
-      // because it occurred from time to time. Since it wrecked havoc on the
-      
-      //throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+     
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
     }
     else
     {
@@ -664,9 +707,20 @@
       {
         if (!polygon->UpdateReferencedSlice(referencedSlices_))
         {
-          LOG(ERROR) << "DicomStructureSet::CheckReferencedSlices(): missing information about referenced instance: "
-                     << polygon->GetSopInstanceUid();
-          throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
+          std::string sopInstanceUid = polygon->GetSopInstanceUid();
+          if (Orthanc::Toolbox::StripSpaces(sopInstanceUid) == "")
+          {
+            LOG(ERROR) << "DicomStructureSet::CheckReferencedSlices(): "
+              << " missing information about referenced instance "
+              << "(sopInstanceUid is empty!)";
+          }
+          else
+          {
+            LOG(ERROR) << "DicomStructureSet::CheckReferencedSlices(): "
+              << " missing information about referenced instance "
+              << "(sopInstanceUid = " << sopInstanceUid << ")";
+          }
+          //throw Orthanc::OrthancException(Orthanc::ErrorCode_BadSequenceOfCalls);
         }
       }
     }
@@ -688,7 +742,7 @@
   }
 
   
-  bool DicomStructureSet::ProjectStructure(std::vector< std::vector<PolygonPoint> >& polygons,
+  bool DicomStructureSet::ProjectStructure(std::vector< std::vector<PolygonPoint2D> >& polygons,
                                            const Structure& structure,
                                            const CoordinateSystem3D& slice) const
   {
@@ -706,7 +760,7 @@
       {
         if (polygon->IsOnSlice(slice))
         {
-          polygons.push_back(std::vector<PolygonPoint>());
+          polygons.push_back(std::vector<PolygonPoint2D>());
           
           for (Points::const_iterator p = polygon->GetPoints().begin();
                p != polygon->GetPoints().end(); ++p)
@@ -762,7 +816,7 @@
         double x1, y1, x2, y2;
         if (polygon->Project(x1, y1, x2, y2, slice))
         {
-          std::vector<PolygonPoint> p(4);
+          std::vector<PolygonPoint2D> p(4);
           p[0] = std::make_pair(x1, y1);
           p[1] = std::make_pair(x2, y1);
           p[2] = std::make_pair(x2, y2);
diff -r 0e21ecafcc23 -r c20dbaab360c Framework/Toolbox/DicomStructureSet.h
--- a/Framework/Toolbox/DicomStructureSet.h	Mon Sep 02 17:44:20 2019 +0200
+++ b/Framework/Toolbox/DicomStructureSet.h	Fri Sep 06 09:38:18 2019 +0200
@@ -34,7 +34,7 @@
   class DicomStructureSet : public boost::noncopyable
   {
   public:
-    typedef std::pair<double, double> PolygonPoint;
+    typedef std::pair<double, double> PolygonPoint2D;
     
   private:
     struct ReferencedSlice
@@ -72,7 +72,8 @@
       Points              points_;
       Extent2D            extent_;
 
-      void CheckPoint(const Vector& v);
+      void CheckPointIsOnSlice(const Vector& v) const;
+      bool IsPointOnSlice(const Vector& v) const;
 
     public:
       Polygon(const std::string& sopInstanceUid) :
@@ -135,7 +136,7 @@
 
     Structure& GetStructure(size_t index);
   
-    bool ProjectStructure(std::vector< std::vector<PolygonPoint> >& polygons,
+    bool ProjectStructure(std::vector< std::vector<PolygonPoint2D> >& polygons,
                           const Structure& structure,
                           const CoordinateSystem3D& slice) const;
 
@@ -174,7 +175,7 @@
 
     Vector GetNormal() const;
 
-    bool ProjectStructure(std::vector< std::vector<PolygonPoint> >& polygons,
+    bool ProjectStructure(std::vector< std::vector<PolygonPoint2D> >& polygons,
                           size_t index,
                           const CoordinateSystem3D& slice) const
     {