Skip to content

Commit

Permalink
BUG: Fix STLFileReader crash bug
Browse files Browse the repository at this point in the history
- 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 <mike.jackson@bluequartz.net>
  • Loading branch information
imikejackson committed Apr 28, 2024
1 parent c3d72cb commit cf5dca1
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 24 deletions.
70 changes: 64 additions & 6 deletions src/Plugins/SimplnxCore/docs/ReadStlFileFilter.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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)
Expand All @@ -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)
{
}

Expand All @@ -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");
Expand All @@ -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)
{
Expand All @@ -169,25 +195,59 @@ 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(pos >= stlFileSize)

Check failure on line 221 in src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadStlFile.cpp

View workflow job for this annotation

GitHub Actions / build (ubuntu-20.04, g++-9)

no match for ‘operator>=’ (operand types are ‘fpos_t’ {aka ‘_G_fpos64_t’} and ‘long unsigned int’)

Check failure on line 221 in src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadStlFile.cpp

View workflow job for this annotation

GitHub Actions / build (ubuntu-20.04, g++-10)

no match for ‘operator>=’ (operand types are ‘fpos_t’ and ‘long unsigned int’)

Check failure on line 221 in src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadStlFile.cpp

View workflow job for this annotation

GitHub Actions / build (ubuntu-20.04, clang++-10)

invalid operands to binary expression ('fpos_t' (aka '_G_fpos64_t') and 'unsigned long')
{
std::string msg =
fmt::format("Trying to read at file position {} >= file size {}. The STL File is probably corrupt or not written properly.\n The file header was '{}'", pos, stlFileSize, stlHeaderStr);
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];
Expand Down Expand Up @@ -260,6 +320,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]);
Expand All @@ -275,10 +337,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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down

0 comments on commit cf5dca1

Please sign in to comment.