BUG: Fix STLFileReader crash bug (#930)

- Original bug was caused by a malformed fmt::format statement with a mismatch '}' character. - Added more error checking to ensure we can safely recover from common STL reading errors - Updated the documentation to reflect the vendor list that does not write conformant STL Files. Signed-off-by: Michael Jackson <[email protected]>
BlueQuartzSoftware · Apr 28, 2024 · 7f055a5 · 7f055a5
1 parent c3d72cb
commit 7f055a5
Show file tree

Hide file tree

Showing 7 changed files with 241 additions and 26 deletions.
diff --git a/src/Plugins/SimplnxCore/docs/ReadStlFileFilter.md b/src/Plugins/SimplnxCore/docs/ReadStlFileFilter.md
@@ -12,14 +12,72 @@ This **Filter**  will read a binary STL File and create a **Triangle Geometry**
     UINT32     Number of triangles
 
     foreach triangle
-    REAL32[3]     Normal vector
-    REAL32[3]     Vertex 1
-    REAL32[3]     Vertex 2
-    REAL32[3]     Vertex 3
-    UINT16     Attribute byte count
+      REAL32[3]     Normal vector
+      REAL32[3]     Vertex 1
+      REAL32[3]     Vertex 2
+      REAL32[3]     Vertex 3
+      UINT16     Attribute byte count
     end
 
-**It is very important that the "Attribute byte Count" is correct as DREAM.3D follows the specification strictly.** If you are writing an STL file be sure that the value for the "Attribute byte count" is *zero* (0). If you chose to encode additional data into a section after each triangle then be sure that the "Attribute byte count" is set correctly. DREAM.3D will obey the value located in the "Attribute byte count".
+The filter will look for specific header information to try and determine the vendor of the STL file. Certain vendors do not write STL files that adhere to the file spec.
+
+## IMPORANT NOTES:
+
+**It is very important that the "Attribute byte Count" is correct as DREAM3D-NX follows the specification strictly.** If you are writing an STL file be sure that the value for the "Attribute byte count" is *zero* (0). If you chose to encode additional data into a section after each triangle then be sure that the "Attribute byte count" is set correctly. DREAM3D-NX will obey the value located in the "Attribute byte count".
+
+## Known Vendors who Write out of spec STL Files
+
+- Materialise Magics [https://www.materialise.com/en/industrial/software/magics-data-build-preparation](https://www.materialise.com/en/industrial/software/magics-data-build-preparation)
+
+    The filter looks in the header for "COLOR=" and "MATERIAL=" strings in the header.
+
+- Creaform VXelements [https://www.creaform3d.com/en/metrology-solutions/3d-applications-software-platforms](https://www.creaform3d.com/en/metrology-solutions/3d-applications-software-platforms)
+
+    The filter looks for "VXelements" in the header.
+
+## Code to convert
+
+If you find yourself in a situation where the STL File is non-conforming and is not made by one of the vendors above, this bit of Python
+code can clean up the file. This makes the absolute assumption that the **ONLY** thing wrong with the STL file is that the trailing UINT16 value for
+each triangle needs to be set to ZERO.
+
+        import struct
+
+        def modify_stl(input_file_path, output_file_path):
+            with open(input_file_path, 'rb') as input_file, open(output_file_path, 'wb') as output_file:
+                # Read and copy header
+                header = input_file.read(80)
+                output_file.write(header)
+                
+                # Read number of triangles
+                num_triangles = struct.unpack('<I', input_file.read(4))[0]
+                output_file.write(struct.pack('<I', num_triangles))
+                
+                # Define the format for one triangle (50 bytes total)
+                triangle_format = '<12fH'
+                triangle_size = struct.calcsize(triangle_format)
+                
+                # Process each triangle
+                for _ in range(num_triangles):
+                    # Read triangle data
+                    triangle_data = input_file.read(triangle_size)
+                    
+                    # Unpack and modify the last 2 bytes (attribute byte count)
+                    data = list(struct.unpack(triangle_format, triangle_data))
+                    data[-1] = 0  # Set the attribute byte count to zero
+                    
+                    # Repack and write the modified triangle data
+                    modified_triangle_data = struct.pack(triangle_format, *data)
+                    output_file.write(modified_triangle_data)
+
+        # Example usage
+        input_stl_path = '/path/to/input.stl'  # Specify the input file path
+        output_stl_path = '/path/to/input_FIXED.stl'  # Specify the output file path
+
+        modify_stl(input_stl_path, output_stl_path)
+
+
+
 
 % Auto generated parameter table will be inserted here
 

diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadStlFile.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadStlFile.cpp
@@ -8,6 +8,7 @@
 #include "simplnx/DataStructure/Geometry/TriangleGeom.hpp"
 #include "simplnx/Utilities/DataArrayUtilities.hpp"
 #include "simplnx/Utilities/ParallelDataAlgorithm.hpp"
+#include "simplnx/Utilities/StringUtilities.hpp"
 
 #include <cstdio>
 #include <utility>
@@ -92,10 +93,37 @@ std::array<float, 6> CreateMinMaxCoords()
           -std::numeric_limits<float>::max(), std::numeric_limits<float>::max(),  -std::numeric_limits<float>::max()};
 }
 
+bool IsMagicsFile(const std::string& stlHeaderStr)
+{
+  // Look for the tell-tale signs that the file was written from Magics Materialise
+  // If the file was written by Magics as a "Color STL" file then the 2byte int
+  // values between each triangle will be NON Zero which will screw up the reading.
+  // These NON Zero value do NOT indicate a length but is some sort of color
+  // value encoded into the file. Instead of being normal like everyone else and
+  // using the STL spec they went off and did their own thing.
+
+  static const std::string k_ColorHeader("COLOR=");
+  static const std::string k_MaterialHeader("MATERIAL=");
+  if(stlHeaderStr.find(k_ColorHeader) != std::string::npos && stlHeaderStr.find(k_MaterialHeader) != std::string::npos)
+  {
+    return true;
+  }
+  return false;
+}
+
+bool IsVxElementsFile(const std::string& stlHeader)
+{
+  // Look for the tell-tale signs that the file was written from VxElements Creaform
+  // Creaform STL files do not honor the last 2 bytes of the 50 byte Triangle struct
+  // as specified in the STL Binary File specification. If we detect this, then we
+  // ignore the 2 bytes are anything meaningful.
+  return nx::core::StringUtilities::contains(stlHeader, "VXelements");
+}
+
 } // End anonymous namespace
 
 ReadStlFile::ReadStlFile(DataStructure& data, fs::path stlFilePath, const DataPath& geometryPath, const DataPath& faceGroupPath, const DataPath& faceNormalsDataPath, bool scaleOutput,
-                         float32 scaleFactor, const std::atomic_bool& shouldCancel)
+                         float32 scaleFactor, const std::atomic_bool& shouldCancel, const IFilter::MessageHandler& mesgHandler)
 : m_DataStructure(data)
 , m_FilePath(std::move(stlFilePath))
 , m_GeometryDataPath(geometryPath)
@@ -104,6 +132,7 @@ ReadStlFile::ReadStlFile(DataStructure& data, fs::path stlFilePath, const DataPa
 , m_ScaleOutput(scaleOutput)
 , m_ScaleFactor(scaleFactor)
 , m_ShouldCancel(shouldCancel)
+, m_MessageHandler(mesgHandler)
 {
 }
 
@@ -113,6 +142,8 @@ Result<> ReadStlFile::operator()()
 {
 
   m_MinMaxCoords = ::CreateMinMaxCoords();
+  std::error_code errorCode;
+  auto stlFileSize = std::filesystem::file_size(m_FilePath, errorCode);
 
   // Open File
   FILE* f = std::fopen(m_FilePath.string().c_str(), "rb");
@@ -138,13 +169,8 @@ Result<> ReadStlFile::operator()()
   // using the STL spec they went off and did their own thing.
   std::string stlHeaderStr(stlHeader.data(), nx::core::StlConstants::k_STL_HEADER_LENGTH);
 
-  bool magicsFile = false;
-  static const std::string k_ColorHeader("COLOR=");
-  static const std::string k_MaterialHeader("MATERIAL=");
-  if(stlHeaderStr.find(k_ColorHeader) != std::string::npos && stlHeaderStr.find(k_MaterialHeader) != std::string::npos)
-  {
-    magicsFile = true;
-  }
+  bool ignoreMetaSizeValue = (IsMagicsFile(stlHeaderStr) || IsVxElementsFile(stlHeaderStr) ? true : false);
+
   // Read the number of triangles in the file.
   if(std::fread(&triCount, sizeof(int32_t), 1, f) != 1)
   {
@@ -169,25 +195,69 @@ Result<> ReadStlFile::operator()()
   std::array<float, k_StlElementCount> fileVert = {0.0F};
   uint16_t attr = 0;
   std::vector<uint8_t> triangleAttributeBuffer(std::numeric_limits<uint16_t>::max()); // Just allocate a buffer of max UINT16 elements
+
+  fpos_t pos;
+  auto start = std::chrono::steady_clock::now();
+  int32_t progInt = 0;
+
   for(int32_t t = 0; t < triCount; ++t)
   {
+    progInt = static_cast<float>(t) / static_cast<float>(triCount) * 100.0f;
+
+    auto now = std::chrono::steady_clock::now();
+    // Only send updates every 1 second
+    if(std::chrono::duration_cast<std::chrono::milliseconds>(now - start).count() > 1000)
+    {
+      std::string message = fmt::format("Reading {}% Complete", progInt);
+      m_MessageHandler(nx::core::IFilter::ProgressMessage{nx::core::IFilter::Message::Type::Info, message, progInt});
+      start = std::chrono::steady_clock::now();
+    }
+    if(m_ShouldCancel)
+    {
+      return {};
+    }
+    // Get the current File Position
+    fgetpos(f, &pos);
+#if defined(__APPLE__) || defined(_WIN32)
+    if(pos >= stlFileSize)
+#else
+    if(pos.__pos >= stlFileSize)
+#endif
+    {
+      std::string msg = fmt::format(
+          "Trying to read at file position {} >= file size {}.\n  File Header: '{}'\n  Header Triangle Count: {}  Current Triangle: {}\n  The STL File does not conform to the STL file specification.",
+#if defined(__APPLE__) || defined(_WIN32)
+          pos,
+#else
+          pos.__pos,
+#endif
+          stlFileSize, stlHeaderStr, triCount, t);
+      return MakeErrorResult(nx::core::StlConstants::k_StlFileLengthError, msg);
+    }
+
+    // Read the Vertices and Normal (12 total float32 = 48 Bytes)
     size_t objsRead = std::fread(fileVert.data(), sizeof(float), k_StlElementCount, f); // Read the Triangle
     if(k_StlElementCount != objsRead)
     {
-      std::string msg = fmt::format("Error reading Triangle '{}}'. Object Count was {} and should have been {}", t, objsRead, k_StlElementCount);
+      std::string msg = fmt::format("Error reading Triangle '{}'. Object Count was {} and should have been {}", t, objsRead, k_StlElementCount);
       return MakeErrorResult(nx::core::StlConstants::k_TriangleParseError, msg);
     }
-
+    // Read the Uint16 value that is supposed to represent the number of bytes following that are file/vendor specific meta data
+    // Lots of writers/vendors do NOT set this properly which can cause problems.
     objsRead = std::fread(&attr, sizeof(uint16_t), 1, f); // Read the Triangle Attribute Data length
     if(objsRead != 1)
     {
-      std::string msg = fmt::format("Error reading Number of attributes for triangle '{}'. Object Count was {} and should have been 1", t, objsRead);
+      std::string msg = fmt::format("Error reading Number of attributes for triangle '{}'. uint16 count was {} and should have been 1", t, objsRead);
       return MakeErrorResult(nx::core::StlConstants::k_AttributeParseError, msg);
     }
-    if(attr > 0 && !magicsFile)
+    // If we are trying to follow along the STL Spec, skip the stated bytes unless
+    // we detected known Vendors that do not write proper STL Files.
+    if(attr > 0 && !ignoreMetaSizeValue)
     {
       std::ignore = std::fseek(f, static_cast<size_t>(attr), SEEK_CUR); // Skip past the Triangle Attribute data since we don't know how to read it anyways
     }
+
+    // Determine the Min/Max Coordinates
     if(fileVert[3] < m_MinMaxCoords[0])
     {
       m_MinMaxCoords[0] = fileVert[3];
@@ -260,6 +330,8 @@ Result<> ReadStlFile::operator()()
     {
       m_MinMaxCoords[5] = fileVert[11];
     }
+
+    // Write the data into the actual geometry
     faceNormals[3 * t + 0] = static_cast<double>(fileVert[0]);
     faceNormals[3 * t + 1] = static_cast<double>(fileVert[1]);
     faceNormals[3 * t + 2] = static_cast<double>(fileVert[2]);
@@ -275,10 +347,6 @@ Result<> ReadStlFile::operator()()
     triangles[t * 3] = 3 * t + 0;
     triangles[t * 3 + 1] = 3 * t + 1;
     triangles[t * 3 + 2] = 3 * t + 2;
-    if(m_ShouldCancel)
-    {
-      return {};
-    }
   }
 
   return eliminate_duplicate_nodes();

diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadStlFile.hpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadStlFile.hpp
@@ -23,7 +23,7 @@ class SIMPLNXCORE_EXPORT ReadStlFile
 {
 public:
   ReadStlFile(DataStructure& data, fs::path stlFilePath, const DataPath& geometryPath, const DataPath& faceGroupPath, const DataPath& faceNormalsDataPath, bool scaleOutput, float32 scaleFactor,
-              const std::atomic_bool& shouldCancel);
+              const std::atomic_bool& shouldCancel, const IFilter::MessageHandler& mesgHandler);
   ~ReadStlFile() noexcept;
 
   ReadStlFile(const ReadStlFile&) = delete;
@@ -57,5 +57,6 @@ class SIMPLNXCORE_EXPORT ReadStlFile
   const bool m_ScaleOutput = false;
   const float m_ScaleFactor = 1.0F;
   const std::atomic_bool& m_ShouldCancel;
+  const IFilter::MessageHandler& m_MessageHandler;
 };
 } // namespace nx::core
diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/ReadStlFileFilter.cpp b/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/ReadStlFileFilter.cpp
@@ -182,7 +182,7 @@ Result<> ReadStlFileFilter::executeImpl(DataStructure& data, const Arguments& fi
   auto scaleFactor = filterArgs.value<float32>(k_ScaleFactor);
 
   // The actual STL File Reading is placed in a separate class `ReadStlFile`
-  Result<> result = ReadStlFile(data, pStlFilePathValue, pTriangleGeometryPath, pFaceDataGroupPath, pFaceNormalsPath, scaleOutput, scaleFactor, shouldCancel)();
+  Result<> result = ReadStlFile(data, pStlFilePathValue, pTriangleGeometryPath, pFaceDataGroupPath, pFaceNormalsPath, scaleOutput, scaleFactor, shouldCancel, messageHandler)();
   return result;
 }
 

diff --git a/src/Plugins/SimplnxCore/src/SimplnxCore/utils/StlUtilities.hpp b/src/Plugins/SimplnxCore/src/SimplnxCore/utils/StlUtilities.hpp
@@ -19,6 +19,7 @@ inline constexpr int32_t k_StlHeaderParseError = -1104;
 inline constexpr int32_t k_TriangleCountParseError = -1105;
 inline constexpr int32_t k_TriangleParseError = -1106;
 inline constexpr int32_t k_AttributeParseError = -1107;
+inline constexpr int32_t k_StlFileLengthError = -1108;
 
 enum class StlFileType : int
 {

diff --git a/src/Plugins/SimplnxCore/test/CMakeLists.txt b/src/Plugins/SimplnxCore/test/CMakeLists.txt
@@ -230,6 +230,7 @@ if(EXISTS "${DREAM3D_DATA_DIR}" AND SIMPLNX_DOWNLOAD_TEST_FILES)
   download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME remove_flagged_triangles_test.tar.gz SHA512 cd5c6f3ea16a6d09e00e0c0bd0f941b27dca8a0beaeabb7262a2a6adaad83c829187c5d1aa433718123b628eaa839f016604c1134ced9f870723594b2df4be99)
   download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME generate_color_table_test.tar.gz SHA512 b5683c758964eb723267400b14047f8adb0d5365ee9ca93d1a6940e9b6ad198cd4739c1ca799eb787b7706e668dbc16ab8243642034cdba5b71d64c27e682d3f)
   download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME read_vtk_structured_points_test.tar.gz SHA512 e7a07a4e3901204c2562754cd71e0fdba1a46de2a5135bad2b6d66b40eefd0e63bed4dbe0ccd6ccadafb708ef63e20635d080aa3a35c172c4ced6986e0f75d5c)
+  download_test_data(DREAM3D_DATA_DIR ${DREAM3D_DATA_DIR} ARCHIVE_NAME ReadSTLFileTest.tar.gz SHA512 975587206625ffa183160308934e767347de55a34a16272cf5c121114efa286b3c6939e3c6a397e8728fdefe1771bc024bd4c9b409afdff0b76f2f56fcb9eb69)
 
 endif()