From e3602c543fe920f212d51d3bb5fef6624ad73bf7 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Date: Thu, 23 Oct 2025 11:44:41 -0400 Subject: [PATCH] fix: issue #25 `Unable to read STL file with scientific notation` --- modules/core/include/openstl/core/stl.h | 174 ++++++++++++---- tests/core/src/deserialize.test.cpp | 253 +++++++++++++++++------- 2 files changed, 324 insertions(+), 103 deletions(-) diff --git a/modules/core/include/openstl/core/stl.h b/modules/core/include/openstl/core/stl.h index c6ecb0d..b291818 100644 --- a/modules/core/include/openstl/core/stl.h +++ b/modules/core/include/openstl/core/stl.h @@ -25,15 +25,22 @@ SOFTWARE. #ifndef OPENSTL_OPENSTL_SERIALIZE_H #define OPENSTL_OPENSTL_SERIALIZE_H #include -#include #include -#include +#include +#include +#include #include #include #include #include #include #include +#include +#include +#include +#include +#include +#include #define MAX_TRIANGLES 1000000 @@ -139,56 +146,153 @@ namespace openstl } /** - * @brief Read a vertex from a stream. + * @brief Trim leading whitespace from a string_view. * - * @tparam Stream The type of the input stream. - * @param stream The input stream from which to read the vertex. - * @param vertex The Vec3 object to store the read vertex. + * @param sv Input string view. + * @return A string_view with leading whitespace removed. + */ + inline std::string_view ltrim(std::string_view sv) noexcept { + size_t i{0}; + while (i < sv.size() && std::isspace(static_cast(sv[i]))) ++i; + return sv.substr(i); + } + + /** + * @brief Case-insensitive prefix match. + * + * @param hay Line to inspect. + * @param needle Expected prefix. + * @return True if hay starts with needle (ignoring case). + */ + inline bool istarts_with(std::string_view hay, std::string_view needle) noexcept { + if (hay.size() < needle.size()) return false; + for (size_t i = 0; i < needle.size(); ++i) { + const auto a = std::tolower(static_cast(hay[i])); + const auto b = std::tolower(static_cast(needle[i])); + if (a != b) return false; + } + return true; + } + + /** + * @brief Read a line or throw if the stream ends unexpectedly. + * + * @tparam Stream Input stream type. + * @param s Stream to read from. + * @param context Context string for error reporting. + * @return The read line (without newline). + * + * @throws std::runtime_error If EOF is reached unexpectedly. */ template - inline void readVertex(Stream& stream, Vec3& vertex) + inline std::string getline_or_throw(Stream& s, const char* context) { + std::string out{}; + if (!std::getline(s, out)) { + throw std::runtime_error(std::string("Unexpected end of stream while reading ") + context); + } + return out; + } + + /** + * @brief Parse three floats from a line after a given ASCII STL keyword. + * + * Accepts scientific notation; uses C locale for numeric parsing. + * + * @param line Input line (whitespace tolerated). + * @param keyword Expected leading keyword ("facet normal" or "vertex"). + * @param a Output float #1. + * @param b Output float #2. + * @param c Output float #3. + * + * @throws std::runtime_error On keyword mismatch or parsing failure. + */ + inline void parse_three_floats_after(std::string_view line, + std::string_view keyword, + float& a, float& b, float& c) { - std::string line; - std::getline(stream, line); - if (line.find("vertex") != std::string::npos) { - std::istringstream iss(line); - iss.ignore(7); // Ignore "vertex " - iss >> vertex.x >> vertex.y >> vertex.z; + line = ltrim(line); + if (!istarts_with(line, keyword)) { + throw std::runtime_error("Expected keyword '" + std::string(keyword) + "' not found."); + } + std::string_view rest = ltrim(line.substr(keyword.size())); + + // Use classic C locale to ensure '.' and scientific notation work regardless of global locale. + std::istringstream iss{std::string(rest)}; + iss.imbue(std::locale::classic()); + if (!(iss >> a >> b >> c)) { + throw std::runtime_error("Failed to parse three floats after '" + std::string(keyword) + "'."); } } /** - * @brief Deserialize an ASCII STL file from a stream and convert it to a vector of triangles. + * @brief Read one vertex from an ASCII STL vertex line. * - * @tparam Stream The type of the input stream. - * @param stream The input stream from which to read the ASCII STL data. - * @return A vector of triangles representing the geometry from the ASCII STL file. + * Expected syntax: "vertex x y z" + * + * @tparam Stream Input stream type. + * @param stream Stream positioned at a vertex line. + * @param v Output vertex coordinates. + * + * @throws std::runtime_error On malformed or missing vertex line. */ template - inline std::vector deserializeAsciiStl(Stream& stream) + inline void readVertex(Stream& stream, Vec3& v) { + const std::string line = getline_or_throw(stream, "vertex line"); + parse_three_floats_after(std::string_view(line), "vertex", v.x, v.y, v.z); + } + + /** + * @brief Deserialize triangles from an ASCII STL input stream. + * + * Supports scientific notation and variable whitespace. + * Ignores unrelated lines (endfacet/endsolid/comments). + * + * @tparam Stream Input stream type. + * @param stream Stream containing ASCII STL data. + * @param max_triangles Optional safety bound (default: unlimited). + * + * @return Vector of parsed triangles. + * + * @throws std::runtime_error On malformed geometry or size overflow. + */ + template + inline std::vector deserializeAsciiStl( + Stream& stream, + std::size_t max_triangles = std::numeric_limits::max()) { - std::vector triangles; - std::string line; - while (std::getline(stream, line)) { - if (line.find("facet normal") != std::string::npos) { - Triangle tri{}; - { - std::istringstream iss(line); - iss.ignore(13); // Ignore "facet normal " - iss >> tri.normal.x >> tri.normal.y >> tri.normal.z; - } + std::vector tris; + std::string raw; + + while (std::getline(stream, raw)) { + std::string_view line = ltrim(std::string_view(raw)); - std::getline(stream, line); // Skip 'outer loop' line - readVertex(stream, tri.v0); - readVertex(stream, tri.v1); - readVertex(stream, tri.v2); - triangles.push_back(tri); + if (!istarts_with(line, "facet normal")) { + // Tolerate other lines (solid/endsolid/endfacet/endloop/comments). + continue; } - if (activateOverflowSafety() && triangles.size() > MAX_TRIANGLES) { + + Triangle t{}; + parse_three_floats_after(line, "facet normal", t.normal.x, t.normal.y, t.normal.z); + + // Read and optionally validate the 'outer loop' line. + const std::string outer = getline_or_throw(stream, "'outer loop'"); + // If you want strictness, uncomment: + // if (!istarts_with(ltrim(std::string_view(outer)), "outer loop")) { + // throw std::runtime_error("Expected 'outer loop' after 'facet normal'."); + // } + + // Three vertices + readVertex(stream, t.v0); + readVertex(stream, t.v1); + readVertex(stream, t.v2); + + tris.push_back(t); + if (tris.size() > max_triangles) { throw std::runtime_error("Triangle count exceeds the maximum allowable value."); } } - return triangles; + + return tris; } /** diff --git a/tests/core/src/deserialize.test.cpp b/tests/core/src/deserialize.test.cpp index 5171ef2..c3f3dc3 100644 --- a/tests/core/src/deserialize.test.cpp +++ b/tests/core/src/deserialize.test.cpp @@ -7,74 +7,191 @@ using namespace openstl; -TEST_CASE("Deserialize ASCII STL", "[openstl]") { - SECTION("Single triangle") - { - std::stringstream stream; - stream << "solid name\n"; - stream << "facet normal 0.1 0.2 1.0\n"; - stream << "outer loop\n"; - stream << "vertex 0.0 0.0 0.0\n"; - stream << "vertex 1.0 0.0 0.0\n"; - stream << "vertex 0.0 1.0 0.0\n"; - stream << "endloop\n"; - stream << "endfacet\n"; - stream << "endsolid name\n"; - - auto triangles = deserializeAsciiStl(stream); - - REQUIRE(triangles.size() == 1); - REQUIRE(triangles[0].normal.x == 0.1f); - REQUIRE(triangles[0].normal.y == 0.2f); - REQUIRE(triangles[0].normal.z == 1.0f); - - REQUIRE(triangles[0].v0.x == 0.0f); - REQUIRE(triangles[0].v0.y == 0.0f); - REQUIRE(triangles[0].v0.z == 0.0f); - - REQUIRE(triangles[0].v1.x == 1.0f); - REQUIRE(triangles[0].v1.y == 0.0f); - REQUIRE(triangles[0].v1.z == 0.0f); - - REQUIRE(triangles[0].v2.x == 0.0f); - REQUIRE(triangles[0].v2.y == 1.0f); - REQUIRE(triangles[0].v2.z == 0.0f); - - stream.clear(); stream.seekg(0); - auto triangles_auto = deserializeStl(stream); - REQUIRE(triangles.size() == triangles_auto.size()); - } - SECTION("Multiple triangles") - { - std::stringstream stream; - stream << "solid name\n"; - stream << "facet normal 0.1 0.2 1.0\n"; - stream << "outer loop\n"; - stream << "vertex 0.0 0.0 0.0\n"; - stream << "vertex 1.0 0.0 0.0\n"; - stream << "vertex 0.0 1.0 0.0\n"; - stream << "endloop\n"; - stream << "endfacet\n"; - stream << "facet normal 0.0 0.0 1.0\n"; - stream << "outer loop\n"; - stream << "vertex 0.0 0.0 0.0\n"; - stream << "vertex 0.0 1.0 0.0\n"; - stream << "vertex 1.0 0.0 0.0\n"; - stream << "endloop\n"; - stream << "endfacet\n"; - stream << "endsolid name\n"; - - auto triangles = deserializeAsciiStl(stream); - - REQUIRE(triangles.size() == 2); - REQUIRE(triangles[0].normal.x == 0.1f); - REQUIRE(triangles[0].normal.y == 0.2f); - REQUIRE(triangles[0].normal.z == 1.0f); - - stream.clear(); stream.seekg(0); - auto triangles_auto = deserializeStl(stream); - REQUIRE(triangles.size() == triangles_auto.size()); - } + +static std::string oneTriangleBlock( + const std::string& normal, const std::string& v0, const std::string& v1, const std::string& v2, + const std::string& outer="outer loop") +{ + std::ostringstream ss; + ss << "facet normal " << normal << "\n"; + ss << outer << "\n"; + ss << "vertex " << v0 << "\n"; + ss << "vertex " << v1 << "\n"; + ss << "vertex " << v2 << "\n"; + ss << "endloop\n"; + ss << "endfacet\n"; + return ss.str(); +} + +TEST_CASE("Deserialize ASCII STL: single triangle", "[openstl][ascii]") { + const std::string stl_text = + "solid name\n" + "facet normal 0.1 0.2 1.0\n" + "outer loop\n" + "vertex 0.0 0.0 0.0\n" + "vertex 1.0 0.0 0.0\n" + "vertex 0.0 1.0 0.0\n" + "endloop\n" + "endfacet\n" + "endsolid name\n"; + + std::stringstream ss1(stl_text); + auto triangles = deserializeAsciiStl(ss1); + + REQUIRE(triangles.size() == 1); + REQUIRE(triangles[0].normal.x == 0.1f); + REQUIRE(triangles[0].normal.y == 0.2f); + REQUIRE(triangles[0].normal.z == 1.0f); + + REQUIRE(triangles[0].v0.x == 0.0f); + REQUIRE(triangles[0].v0.y == 0.0f); + REQUIRE(triangles[0].v0.z == 0.0f); + + REQUIRE(triangles[0].v1.x == 1.0f); + REQUIRE(triangles[0].v1.y == 0.0f); + REQUIRE(triangles[0].v1.z == 0.0f); + + REQUIRE(triangles[0].v2.x == 0.0f); + REQUIRE(triangles[0].v2.y == 1.0f); + REQUIRE(triangles[0].v2.z == 0.0f); + + std::stringstream ss2(stl_text); + auto triangles_auto = deserializeStl(ss2); + REQUIRE(triangles.size() == triangles_auto.size()); +} + +TEST_CASE("Deserialize ASCII STL: multiple triangles", "[openstl][ascii]") { + const std::string stl_text = + "solid name\n" + "facet normal 0.1 0.2 1.0\n" + "outer loop\n" + "vertex 0.0 0.0 0.0\n" + "vertex 1.0 0.0 0.0\n" + "vertex 0.0 1.0 0.0\n" + "endloop\n" + "endfacet\n" + "facet normal 0.0 0.0 1.0\n" + "outer loop\n" + "vertex 0.0 0.0 0.0\n" + "vertex 0.0 1.0 0.0\n" + "vertex 1.0 0.0 0.0\n" + "endloop\n" + "endfacet\n" + "endsolid name\n"; + + std::stringstream ss1(stl_text); + auto triangles = deserializeAsciiStl(ss1); + + REQUIRE(triangles.size() == 2); + REQUIRE(triangles[0].normal.x == 0.1f); + REQUIRE(triangles[0].normal.y == 0.2f); + REQUIRE(triangles[0].normal.z == 1.0f); + + std::stringstream ss2(stl_text); + auto triangles_auto = deserializeStl(ss2); + REQUIRE(triangles.size() == triangles_auto.size()); +} + +TEST_CASE("Deserialize ASCII STL: scientific notation parses (issue #25)", "[openstl][ascii][sci]") { + std::stringstream ss; + ss << "solid name\n"; + ss << oneTriangleBlock( + "3.530327e-01 -3.218319e-01 -8.785170e-01", + "5.502911e-01 -7.287032e-01 3.099700e-01", + "2.905658e-01 -3.847714e-01 7.960480e-02", + "4.099400e-01 -2.538241e-01 7.960480e-02"); + ss << "endsolid name\n"; + + auto tris = deserializeAsciiStl(ss); + REQUIRE(tris.size() == 1); + REQUIRE_THAT(tris[0].normal.x, Catch::Matchers::WithinAbs( 3.530327e-01f, 1e-6f)); + REQUIRE_THAT(tris[0].normal.y, Catch::Matchers::WithinAbs(-3.218319e-01f, 1e-6f)); + REQUIRE_THAT(tris[0].normal.z, Catch::Matchers::WithinAbs(-8.785170e-01f, 1e-6f)); + REQUIRE_THAT(tris[0].v0.x, Catch::Matchers::WithinAbs( 5.502911e-01f, 1e-6f)); + REQUIRE_THAT(tris[0].v2.z, Catch::Matchers::WithinAbs( 7.960480e-02f, 1e-6f)); +} + +TEST_CASE("Deserialize ASCII STL: keywords are case-insensitive", "[openstl][ascii][case]") { + std::stringstream ss; + ss << "solid s\n"; + ss << "FACET NORMAL 1E+00 0E+00 0E+00\n"; + ss << "OUTER LOOP\n"; + ss << "VERTEX 0E+00 0E+00 0E+00\n"; + ss << "VERTEX 1E+00 0E+00 0E+00\n"; + ss << "VERTEX 0E+00 1E+00 0E+00\n"; + ss << "ENDLOOP\nENDFACET\nENDSOLID s\n"; + + auto tris = deserializeAsciiStl(ss); + REQUIRE(tris.size() == 1); + REQUIRE_THAT(tris[0].normal.x, Catch::Matchers::WithinAbs(1.0f, 1e-6f)); +} + +TEST_CASE("Deserialize ASCII STL: Windows CRLF endings are tolerated", "[openstl][ascii][crlf]") { + std::string text; + text = "solid s\r\n"; + text += oneTriangleBlock("1.0 0.0 0.0", "0 0 0", "1 0 0", "0 1 0"); + std::replace(text.begin(), text.end(), '\n', '\r'); // make everything CR + // Ensure CRLF pairs exist (simulate typical CRLF): we’ll craft quickly: + // For simplicity, rebuild with \r\n pairs: + text = "solid s\r\n"; + text += "facet normal 1.0 0.0 0.0\r\n"; + text += "outer loop\r\n"; + text += "vertex 0 0 0\r\n"; + text += "vertex 1 0 0\r\n"; + text += "vertex 0 1 0\r\n"; + text += "endloop\r\nendfacet\r\nendsolid s\r\n"; + + std::stringstream ss(text); + auto tris = deserializeAsciiStl(ss); + REQUIRE(tris.size() == 1); + REQUIRE_THAT(tris[0].v1.x, Catch::Matchers::WithinAbs(1.0f, 1e-6f)); +} + +TEST_CASE("Deserialize ASCII STL: extra tokens after numbers are ignored", "[openstl][ascii][garbage]") { + std::stringstream ss; + ss << "solid s\n"; + ss << "facet normal 0 0 1 extra tokens here\n"; + ss << "outer loop\n"; + ss << "vertex 0 0 0 trailing\n"; + ss << "vertex 1 0 0 garbage\n"; + ss << "vertex 0 1 0 more_garbage\n"; + ss << "endloop\nendfacet\nendsolid s\n"; + + auto tris = deserializeAsciiStl(ss); + REQUIRE(tris.size() == 1); + REQUIRE_THAT(tris[0].normal.z, Catch::Matchers::WithinAbs(1.0f, 1e-6f)); +} + +TEST_CASE("Deserialize ASCII STL: malformed vertex fails fast (missing coord)", "[openstl][ascii][error]") { + std::stringstream ss; + ss << "solid s\n"; + ss << "facet normal 0 0 1\n"; + ss << "outer loop\n"; + ss << "vertex 0 0\n"; // <-- missing Z + ss << "vertex 1 0 0\n"; + ss << "vertex 0 1 0\n"; + ss << "endloop\nendfacet\nendsolid s\n"; + + REQUIRE_THROWS_AS(deserializeAsciiStl(ss), std::runtime_error); +} + +TEST_CASE("Deserialize ASCII STL: unexpected EOF fails fast", "[openstl][ascii][eof]") { + std::stringstream ss; + ss << "solid s\n"; + ss << "facet normal 0 0 1\n"; + ss << "outer loop\n"; + ss << "vertex 0 0 0\n"; + // stream ends abruptly before vertex 2/3 + REQUIRE_THROWS_AS(deserializeAsciiStl(ss), std::runtime_error); +} + +TEST_CASE("Deserialize ASCII STL: non-facet text is ignored (0 triangles)", "[openstl][ascii][ignore]") { + std::stringstream ss; + ss << "solid s\n"; + ss << "this is a comment\n"; + ss << "endsolid s\n"; + auto tris = deserializeAsciiStl(ss); + REQUIRE(tris.empty()); } TEST_CASE("Deserialize Binary STL", "[openstl]") {