From acea1dae52aaabd48f98ff1f554636d677c8a5e4 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Wed, 7 Oct 2026 23:55:36 -0700 Subject: [PATCH] import(): weld an AMF object's vertices by exact position OpenSCAD (checked against 2026.02.01) treats an AMF object's vertices at exactly the same position as one vertex, so a closed object written as a triangle soup -- or with its vertex list duplicated, or a solid with a through-hole whose triangles never share indices -- imports as a closed solid. We took the indices as written, so every such file came in as an open surface ("N boundary edges, M unwelded vertices") that could not take part in CSG. Each object is now welded with STL's existing exact-match weldVertices (moved out of mesh_import.cpp's anonymous namespace). The weld is per object, never across objects, and never merges near-coincident vertices, matching OpenSCAD's import; its CSG-time repair of near-coincident vertices is a generic polyhedron/PolySet behaviour, not AMF's. Genuinely open meshes stay open, as in OpenSCAD. A corpus of 148 STL/OFF/OBJ/3MF/AMF files imports byte-identically before and after; of 30 hand-written and generated AMF cases only the 6 with duplicated positions changed, each to OpenSCAD's closed result. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 5 +- .../openscad_cpp_evaluator/mesh_import.hpp | 3 + src/import/mesh_import.cpp | 24 ++--- src/import/scene_import.cpp | 19 +++- tests/test_import_export.cpp | 93 +++++++++++++++++++ 5 files changed, 126 insertions(+), 18 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f388f43..fa7ed32 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1247,7 +1247,10 @@ grep for `ponytail:`. without `-1` counts), and X3D's `IndexedTriangleSet`/`TriangleSet` are read. X3D goes through a small real XML parser (comments, CDATA, DOCTYPE, entities); VRML through a tokenizer + generic node parser that steps over `PROTO`/`EXTERNPROTO`/`ROUTE` (VRML 1.0 is refused by name). AMF's - `unit` scales to mm, and a zipped AMF (`PK` header) is read through `readStoredZipEntryBySuffix`. + `unit` is ignored (OpenSCAD reads millimetres whatever it says), a zipped AMF (`PK` header) is read + through `readStoredZipEntryBySuffix`, and each AMF object's vertices are welded by exact position + (`weldVertices`, STL's), so a closed object written as a triangle soup imports closed, as in + OpenSCAD -- per object, never across objects, and never near-coincident vertices. What is not a mesh — primitives (Box, Sphere, ...), `Inline`, AMF constellations — lands in `LoadedMesh::warnings`, which both `import()` faces emit as `import: '': ...`. - `include/openscad_cpp_evaluator/import_builtin.hpp`, `include/openscad_cpp_evaluator/ diff --git a/include/openscad_cpp_evaluator/mesh_import.hpp b/include/openscad_cpp_evaluator/mesh_import.hpp index fe3ac8e..12cb10b 100644 --- a/include/openscad_cpp_evaluator/mesh_import.hpp +++ b/include/openscad_cpp_evaluator/mesh_import.hpp @@ -22,6 +22,9 @@ struct LoadedMesh { // "import: {what}" on parse/IO failure. Mirrors _load_stl/_load_obj/ // _load_off/_load_3mf exactly (including STL's exact-match vertex welding // and OBJ/OFF's fan triangulation of >3-gon faces). +// Merges vertices at exactly equal positions (STL's corners, an AMF +// object's duplicated vertices); every position keeps its first index. +LoadedMesh weldVertices(const std::vector>& verts, const std::vector>& tris); LoadedMesh loadStl(const std::string& path); LoadedMesh loadObj(const std::string& path); LoadedMesh loadOff(const std::string& path); diff --git a/src/import/mesh_import.cpp b/src/import/mesh_import.cpp index 99f17a3..17dbc41 100644 --- a/src/import/mesh_import.cpp +++ b/src/import/mesh_import.cpp @@ -57,6 +57,18 @@ std::vector splitWs(const std::string& s) { return out; } +std::string attrValue(std::string_view tag, std::string_view attrName) { + const std::string needle = std::string(attrName) + "=\""; + const size_t pos = tag.find(needle); + if (pos == std::string_view::npos) return "0"; + const size_t start = pos + needle.size(); + const size_t end = tag.find('"', start); + if (end == std::string_view::npos) return "0"; + return std::string(tag.substr(start, end - start)); +} + +} // namespace + // Exact-match vertex welding (STL has no shared-index concept; each // triangle carries its own private corner copies) -- matches // _weld_stl_vertices's np.unique(axis=0) exact-equality behavior. @@ -80,18 +92,6 @@ LoadedMesh weldVertices(const std::vector>& verts, const s return out; } -std::string attrValue(std::string_view tag, std::string_view attrName) { - const std::string needle = std::string(attrName) + "=\""; - const size_t pos = tag.find(needle); - if (pos == std::string_view::npos) return "0"; - const size_t start = pos + needle.size(); - const size_t end = tag.find('"', start); - if (end == std::string_view::npos) return "0"; - return std::string(tag.substr(start, end - start)); -} - -} // namespace - LoadedMesh loadStl(const std::string& path) { const std::vector bytes = readWholeFile(path); // OpenSCAD's wording for every file it cannot read as STL. diff --git a/src/import/scene_import.cpp b/src/import/scene_import.cpp index e650ce4..f902d77 100644 --- a/src/import/scene_import.cpp +++ b/src/import/scene_import.cpp @@ -653,13 +653,13 @@ LoadedMesh loadAmf(const std::string& path) { LoadedMesh out; for (std::string_view object : segments(xml, "object")) { const std::string_view verticesBlock = elementText(object, "vertices"); - const int base = static_cast(out.verts.size()); + LoadedMesh obj; for (std::string_view vertex : segments(verticesBlock, "vertex")) { const std::string_view c = elementText(vertex, "coordinates"); - out.verts.push_back({amfNumber(elementText(c, "x"), "x") * unit, amfNumber(elementText(c, "y"), "y") * unit, + obj.verts.push_back({amfNumber(elementText(c, "x"), "x") * unit, amfNumber(elementText(c, "y"), "y") * unit, amfNumber(elementText(c, "z"), "z") * unit}); } - const int count = static_cast(out.verts.size()) - base; + const int count = static_cast(obj.verts.size()); for (std::string_view volume : segments(object, "volume")) { for (std::string_view tri : segments(volume, "triangle")) { std::array t{}; @@ -669,11 +669,20 @@ LoadedMesh loadAmf(const std::string& path) { if (i < 0 || i >= count) throw std::runtime_error("AMF: triangle vertex " + std::to_string(i) + " is past its object's " + std::to_string(count) + " vertices"); - t[k] = base + i; + t[k] = i; } - out.tris.push_back(t); + obj.tris.push_back(t); } } + // OpenSCAD treats vertices at exactly the same position as one + // (checked, 2026.02.01): a closed object written as a triangle soup, + // or with its vertices listed twice, imports as a closed solid. + // Within each object only -- two open halves in separate objects + // stay open there too. Near-coincident vertices are not merged. + obj = weldVertices(obj.verts, obj.tris); + const int base = static_cast(out.verts.size()); + out.verts.insert(out.verts.end(), obj.verts.begin(), obj.verts.end()); + for (const auto& t : obj.tris) out.tris.push_back({base + t[0], base + t[1], base + t[2]}); } if (xml.find(" #include +#include #include #include #include @@ -267,6 +269,97 @@ TEST(ImportModuleContext, ZippedAmfImports) { EXPECT_NEAR(e.bodies[0].body->Volume(), 1.0, 1e-9); } +namespace { + +using Tri = std::array, 3>; + +// The unit cube at (x, y, z) as 12 outward triangles, two per face, the +// faces in -z, +z, -y, +x, +y, -x order. +std::vector cubeTris(double x, double y, double z) { + const int idx[12][3] = {{0, 3, 2}, {0, 2, 1}, {4, 5, 6}, {4, 6, 7}, {0, 1, 5}, {0, 5, 4}, + {1, 2, 6}, {1, 6, 5}, {2, 3, 7}, {2, 7, 6}, {3, 0, 4}, {3, 4, 7}}; + const double c[8][3] = {{0, 0, 0}, {1, 0, 0}, {1, 1, 0}, {0, 1, 0}, {0, 0, 1}, {1, 0, 1}, {1, 1, 1}, {0, 1, 1}}; + std::vector out; + for (const auto& t : idx) { + Tri tri; + for (int k = 0; k < 3; ++k) tri[k] = {c[t[k]][0] + x, c[t[k]][1] + y, c[t[k]][2] + z}; + out.push_back(tri); + } + return out; +} + +// One per entry, every triangle carrying its own three vertex +// copies: a triangle soup, as an exporter that never shares vertices writes. +std::string soupAmf(const std::vector>& objects) { + std::string amf = "\n"; + for (const auto& tris : objects) { + amf += ""; + for (const Tri& t : tris) + for (const auto& p : t) + amf += "" + std::to_string(p[0]) + "" + std::to_string(p[1]) + + "" + std::to_string(p[2]) + ""; + amf += ""; + for (size_t i = 0; i < tris.size(); ++i) + amf += "" + std::to_string(3 * i) + "" + std::to_string(3 * i + 1) + "" + + std::to_string(3 * i + 2) + ""; + amf += ""; + } + return amf + ""; +} + +} // namespace + +// OpenSCAD treats an AMF object's vertices at exactly the same position as +// one vertex (checked, 2026.02.01), so a closed object written as a soup +// imports as a closed solid. Without the weld it was an open surface with +// 36 boundary edges, drawn but unable to take part in CSG. +TEST(ImportModuleContext, AmfTriangleSoupWeldsIntoAClosedSolid) { + const Imported r = importText("soup.amf", soupAmf({cubeTris(0, 0, 0)})); + EXPECT_NEAR(r.volume, 1.0, 1e-9); + EXPECT_FALSE(anyContains(r.messages, "not a closed solid")) << ::testing::PrintToString(r.messages); +} + +// A 3x3x1 slab with a 1x1 hole through it (eight unit cubes around the +// middle, the faces between neighbours left out), as a soup: closed, genus 1. +TEST(ImportModuleContext, AmfSoupWithAThroughHoleWeldsClosed) { + std::map>, std::vector> faces; + std::map>, int> uses; + for (int i = 0; i < 3; ++i) + for (int j = 0; j < 3; ++j) { + if (i == 1 && j == 1) continue; + const std::vector c = cubeTris(i, j, 0); + for (size_t f = 0; f < c.size(); f += 2) { + std::vector> key{c[f][0], c[f][1], c[f][2], c[f + 1][0], c[f + 1][1], c[f + 1][2]}; + std::sort(key.begin(), key.end()); + key.erase(std::unique(key.begin(), key.end()), key.end()); + faces[key] = {c[f], c[f + 1]}; + ++uses[key]; + } + } + std::vector ring; + for (const auto& [key, tris] : faces) + if (uses[key] == 1) ring.insert(ring.end(), tris.begin(), tris.end()); + const auto path = tempPath("ring.amf"); + std::ofstream(path, std::ios::binary) << soupAmf({ring}); + std::vector messages; + Evaluated e = evalSrc("import(\"" + path.generic_string() + "\");", + [&](const std::string& m) { messages.push_back(m); }); + std::filesystem::remove(path); + ASSERT_EQ(e.bodies.size(), 1u); + ASSERT_TRUE(e.bodies[0].body); + EXPECT_NEAR(e.bodies[0].body->Volume(), 8.0, 1e-9); + EXPECT_EQ(e.bodies[0].body->Genus(), 1); + EXPECT_FALSE(anyContains(messages, "not a closed solid")) << ::testing::PrintToString(messages); +} + +// The weld is per object, as in OpenSCAD: a cube split into two objects, +// each half its triangles, is two open surfaces, not one closed cube. +TEST(ImportModuleContext, AmfWeldsWithinAnObjectOnly) { + const std::vector c = cubeTris(0, 0, 0); + const Imported r = importText("halves.amf", soupAmf({{c.begin(), c.begin() + 6}, {c.begin() + 6, c.end()}})); + EXPECT_TRUE(anyContains(r.messages, "not a closed solid")) << ::testing::PrintToString(r.messages); +} + TEST(ImportModuleContext, UnsupportedExtensionErrors) { const auto path = tempPath("unsupported.xyz"); std::ofstream(path) << "x";