changeset 5816:3f10350b26da

DICOMWeb Json formatter: improve support for ill-formed DS values + DS values are now represented as strings instead of doubles
author Alain Mazy <am@orthanc.team>
date Wed, 25 Sep 2024 19:36:43 +0200
parents 122fd5f97d39
children 86cd21ef81fb
files NEWS OrthancFramework/Resources/CMake/OrthancFrameworkParameters.cmake OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h OrthancFramework/UnitTestsSources/DicomMapTests.cpp OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp
diffstat 6 files changed, 85 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Wed Sep 25 16:36:04 2024 +0200
+++ b/NEWS	Wed Sep 25 19:36:43 2024 +0200
@@ -4,8 +4,19 @@
 REST API
 -----------
 
+* API version upgraded to 26
 * Improved parsing of multiple numerical values in DICOM tags.
   https://discourse.orthanc-server.org/t/qido-includefield-with-sequences/4746/6
+* POSSIBLE BREAKING-CHANGES:
+  In DICOMWeb json, the "DS - Decimal String" values were represented by float numbers
+  and they are now represented as strings to avoid loss of precision when the decimal
+  number can not be exactly represented as a float.  This is based on the DICOMWeb
+  standard https://dicom.nema.org/medical/dicom/current/output/chtml/part18/sect_F.2.3.html
+  - DS values can be represented as String or Number in Json.
+  - For IS, DS, SV and UV, a JSON String representation can be used to preserve the original 
+    format during transformation of the representation, or if needed to avoid losing 
+    precision of a decimal string.
+  https://discourse.orthanc-server.org/t/dicomwebplugin-does-not-return-series-metadata-properly/5195
 
 
 Maintenance
@@ -26,12 +37,11 @@
   - added 2 metrics: orthanc_storage_cache_miss_count & orthanc_storage_cache_hit_count 
 * Upgraded dependencies for static builds:
   - curl 8.9.0
+  - boost 1.86.0
 * Added a new fallback when trying to decode a frame: transcode the file using the plugin
   before decoding the frame.  This solves some issues with JP2K Lossy compression:
   https://discourse.orthanc-server.org/t/decoding-displaying-jpeg2000-lossy-images/5117
 * Added a new warning that can be disabled in the configuration: W003_DecoderFailure
-* Upgraded dependencies for static builds:
-  - boost 1.86.0
 
 
 
--- a/OrthancFramework/Resources/CMake/OrthancFrameworkParameters.cmake	Wed Sep 25 16:36:04 2024 +0200
+++ b/OrthancFramework/Resources/CMake/OrthancFrameworkParameters.cmake	Wed Sep 25 19:36:43 2024 +0200
@@ -39,7 +39,7 @@
 # Version of the Orthanc API, can be retrieved from "/system" URI in
 # order to check whether new URI endpoints are available even if using
 # the mainline version of Orthanc
-set(ORTHANC_API_VERSION "24")
+set(ORTHANC_API_VERSION "26")
 
 
 #####################################################################
--- a/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp	Wed Sep 25 16:36:04 2024 +0200
+++ b/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp	Wed Sep 25 19:36:43 2024 +0200
@@ -28,6 +28,7 @@
 #include "../Logging.h"
 #include "../OrthancException.h"
 #include "../Toolbox.h"
+#include "../SerializationToolbox.h"
 #include "FromDcmtkBridge.h"
 
 #include <boost/math/special_functions/round.hpp>
@@ -341,6 +342,31 @@
     }
   }
 
+  Json::Value DicomWebJsonVisitor::FormatDecimalString(double value, const std::string& originalString)
+  {
+    try
+    {
+      long long a = boost::math::llround<double>(value);
+
+      double d = fabs(value - static_cast<double>(a));
+
+      if (d <= std::numeric_limits<double>::epsilon() * 100.0)
+      {
+        return FormatInteger(a);  // if the decimal number is an integer, you can represent it as an integer  
+      }
+      else
+      {
+        return Json::Value(originalString);  // keep the original string to avoid rounding errors e.g, transforming "0.143" into 0.14299999999999
+      }
+    }
+    catch (boost::math::rounding_error&)
+    {
+      // Can occur if "long long" is too small to receive this value
+      // (e.g. infinity)
+      return Json::Value(originalString);
+    }
+  }
+
   DicomWebJsonVisitor::DicomWebJsonVisitor() :
     formatter_(NULL)
   {
@@ -677,14 +703,33 @@
                 case ValueRepresentation_DecimalString:
                 {
                   std::string t = Toolbox::StripSpaces(tokens[i]);
+                  boost::replace_all(t, ",", "."); // some invalid files uses "," instead of "."
+                  
+                  // remove invalid/useless trailing decimal separator
+                  if (t.size() > 0 && t[t.size()-1] == '.')
+                  {
+                    t.resize(t.size() -1);
+                  }
+
                   if (t.empty())
                   {
                     node[KEY_VALUE].append(Json::nullValue);
                   }
                   else
                   {
-                    double tmp = boost::lexical_cast<double>(t);
-                    node[KEY_VALUE].append(FormatDouble(tmp));
+                    // https://dicom.nema.org/medical/dicom/current/output/chtml/part18/sect_F.2.3.html
+                    // DS values can be represented as String or Number in Json.
+                    // For IS, DS, SV and UV, a JSON String representation can be used to preserve the original format during transformation of the representation, or if needed to avoid losing precision of a decimal string.
+                    // Since 1.12.5, always use the string repesentation.  Before, decimal numbers were represented as double which led to loss of precision (e.g: 0.143 represented as 0.1429999999)
+                    double tmp;
+                    if (SerializationToolbox::ParseDouble(tmp, t)) // make sure that the string contains a valid decimal number
+                    {
+                      node[KEY_VALUE].append(t);
+                    }
+                    else
+                    {
+                      throw boost::bad_lexical_cast();
+                    }
                   }
 
                   break;
--- a/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h	Wed Sep 25 16:36:04 2024 +0200
+++ b/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h	Wed Sep 25 19:36:43 2024 +0200
@@ -74,6 +74,8 @@
 
     static Json::Value FormatDouble(double value);
 
+    static Json::Value FormatDecimalString(double value, const std::string& originalString);
+
   public:
     DicomWebJsonVisitor();
 
--- a/OrthancFramework/UnitTestsSources/DicomMapTests.cpp	Wed Sep 25 16:36:04 2024 +0200
+++ b/OrthancFramework/UnitTestsSources/DicomMapTests.cpp	Wed Sep 25 19:36:43 2024 +0200
@@ -804,6 +804,7 @@
   dicom.ReplacePlainString(DICOM_TAG_PATIENT_NAME, "SB1^SB2^SB3^SB4^SB5");
   dicom.ReplacePlainString(DICOM_TAG_IMAGE_ORIENTATION_PATIENT, "1\\2.3\\4");
   dicom.ReplacePlainString(DICOM_TAG_IMAGE_POSITION_PATIENT, "");
+  dicom.ReplacePlainString(DICOM_TAG_PIXEL_SPACING, "0,143\\0,143");  // seen in https://discourse.orthanc-server.org/t/dicomwebplugin-does-not-return-series-metadata-properly/5195
 
   DicomWebJsonVisitor visitor;
   dicom.Apply(visitor);
@@ -815,10 +816,10 @@
     ASSERT_EQ(EnumerationToString(ValueRepresentation_DecimalString), tag["vr"].asString());
     ASSERT_EQ(2u, tag.getMemberNames().size());
     ASSERT_EQ(3u, value.size());
-    ASSERT_EQ(Json::realValue, value[1].type());
-    ASSERT_FLOAT_EQ(1.0f, value[0].asFloat());
-    ASSERT_FLOAT_EQ(2.3f, value[1].asFloat());
-    ASSERT_FLOAT_EQ(4.0f, value[2].asFloat());
+    ASSERT_EQ(Json::stringValue, value[1].type());  // since Orthanc 1.12.5, this is now stored as a string
+    ASSERT_EQ("1", value[0].asString());
+    ASSERT_EQ("2.3", value[1].asString());
+    ASSERT_EQ("4", value[2].asString());
   }
 
   {
@@ -827,13 +828,23 @@
     ASSERT_EQ(1u, tag.getMemberNames().size());
   }
 
+  {
+    const Json::Value& tag = visitor.GetResult() ["00280030"];  // PixelSpacing
+    const Json::Value& value = tag["Value"];
+
+    ASSERT_EQ(EnumerationToString(ValueRepresentation_DecimalString), tag["vr"].asString());
+    ASSERT_EQ(2u, value.size());
+    ASSERT_EQ("0.143", value[0].asString());
+    ASSERT_EQ("0.143", value[1].asString());
+  }
+
   std::string xml;
   visitor.FormatXml(xml);
 
   {
     DicomMap m;
     m.FromDicomWeb(visitor.GetResult());
-    ASSERT_EQ(3u, m.GetSize());
+    ASSERT_EQ(4u, m.GetSize());
 
     std::string s;
     ASSERT_TRUE(m.LookupStringValue(s, DICOM_TAG_PATIENT_NAME, false));
@@ -869,12 +880,12 @@
     ASSERT_EQ(EnumerationToString(ValueRepresentation_DecimalString), tag["vr"].asString());
     ASSERT_EQ(2u, tag.getMemberNames().size());
     ASSERT_EQ(4u, value.size());
-    ASSERT_EQ(Json::realValue, value[0].type());
+    ASSERT_EQ(Json::stringValue, value[0].type());
     ASSERT_EQ(Json::nullValue, value[1].type());
     ASSERT_EQ(Json::nullValue, value[2].type());
-    ASSERT_EQ(Json::realValue, value[3].type());
-    ASSERT_FLOAT_EQ(1.5f, value[0].asFloat());
-    ASSERT_FLOAT_EQ(2.5f, value[3].asFloat());
+    ASSERT_EQ(Json::stringValue, value[3].type());
+    ASSERT_EQ("1.5", value[0].asString());
+    ASSERT_EQ("2.5", value[3].asString());
   }
 
   std::string xml;
@@ -912,8 +923,8 @@
   target.FromDicomWeb(visitor.GetResult());
 
   ASSERT_EQ("DS", visitor.GetResult() ["00280030"]["vr"].asString());
-  ASSERT_FLOAT_EQ(1.5f, visitor.GetResult() ["00280030"]["Value"][0].asFloat());
-  ASSERT_FLOAT_EQ(1.3f, visitor.GetResult() ["00280030"]["Value"][1].asFloat());
+  ASSERT_EQ("1.5", visitor.GetResult() ["00280030"]["Value"][0].asString());
+  ASSERT_EQ("1.3", visitor.GetResult() ["00280030"]["Value"][1].asString());
 
   std::string s;
   ASSERT_TRUE(target.LookupStringValue(s, DICOM_TAG_PIXEL_SPACING, false));
--- a/OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp	Wed Sep 25 16:36:04 2024 +0200
+++ b/OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp	Wed Sep 25 19:36:43 2024 +0200
@@ -2043,7 +2043,7 @@
   ASSERT_EQ("DA", visitor.GetResult() ["00080012"]["vr"].asString());
   ASSERT_EQ("DA", visitor.GetResult() ["00080012"]["Value"][0].asString());
   ASSERT_EQ("DS", visitor.GetResult() ["00101020"]["vr"].asString());
-  ASSERT_FLOAT_EQ(42.0f, visitor.GetResult() ["00101020"]["Value"][0].asFloat());
+  ASSERT_EQ("42", visitor.GetResult() ["00101020"]["Value"][0].asString());
   ASSERT_EQ("DT", visitor.GetResult() ["0008002A"]["vr"].asString());
   ASSERT_EQ("DT", visitor.GetResult() ["0008002A"]["Value"][0].asString());
   ASSERT_EQ("FL", visitor.GetResult() ["00109431"]["vr"].asString());