From 2261fc95e27cc96a2d86f43c185188a83af6e15c Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Tue, 15 Sep 2026 17:29:41 -0700 Subject: [PATCH] Cut each body by the later bodies that reach it, not by all of them (#177) splitBodiesForExport's multi-colour path subtracted every body against a running union of all the bodies after it. That union grows as the loop goes, so an n-body export did n booleans whose second operand reached the size of the whole model -- quadratic in body count, however little the parts actually touched. Only a later body that intersects this one can remove anything from it, and bounding boxes settle that in nanoseconds. Cutting against just those leaves the result identical, because subtracting a disjoint solid removes no volume, and leaves the work proportional to how much the model really overlaps rather than to how many parts it has. Measured on 100 coloured overlapping spheres, ~212,000 facets: bodies 3MF before 3MF after 10 1.714s 1.472s 25 2.599s 1.615s 50 4.103s 1.750s 100 6.582s 1.843s 3.6x at 100 bodies, and near-flat in body count where it had been quadratic. BelfrySCAD's Dalek model (119 bodies, 224k facets) goes 5.96s -> 2.07s. STL is untouched: it is not a multi-object format and never reaches this path. Identical output, checked rather than assumed. Every STL is byte-identical. Dalek's 3MF keeps all 6 objects and the same bounding boxes, with total volume 403426.1883 -> 403426.1882 and 48 fewer triangles out of 151,074 -- slivers the old cut against the big union left behind. The 100-sphere case matches object for object to 4e-15. The two new tests pin semantics, not speed, and both pass on the old code: they are there to stop a future rewrite getting this wrong. The obvious cheaper idea -- union same-coloured bodies first, then subtract per colour -- is wrong, because red/blue/red overlapping in that order must let the blue beat the first red; LaterWinsAcrossAnInterveningColour is exactly that case. --- pyproject.toml | 2 +- src/export.cpp | 51 ++++++++++++++++++++++------------ tests/test_export.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 19 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4c3effd..cdbd686 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "1.20.3" +version = "1.20.4" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/export.cpp b/src/export.cpp index c3bfe04..083a03f 100644 --- a/src/export.cpp +++ b/src/export.cpp @@ -362,9 +362,7 @@ void meshToArrays(const manifold::MeshGL& mesh, std::vector& verts, std:: tris = mesh.triVerts; } -bool boxesOverlap(const manifold::Manifold& a, const manifold::Manifold& b) { - const manifold::Box ba = a.BoundingBox(); - const manifold::Box bb = b.BoundingBox(); +bool boxesOverlap(const manifold::Box& ba, const manifold::Box& bb) { return !(ba.max.x < bb.min.x || bb.max.x < ba.min.x || ba.max.y < bb.min.y || bb.max.y < ba.min.y || ba.max.z < bb.min.z || bb.max.z < ba.min.z); } @@ -512,25 +510,42 @@ std::vector splitBodiesForExport(const std::vector& b c.key = keys[0]; claimedGroups.push_back(std::move(c)); } else { - // Reverse order + subtract-what-is-already-claimed is what makes - // the LATER body win: by the time an earlier one is reached, - // everything after it has already taken its volume. - std::optional claimed; + // Reverse order is what makes the LATER body win: by the time + // an earlier one is reached, everything after it has already + // taken its volume. + // + // Each body is cut by the later ones that actually REACH it, not + // by a running union of all of them. The running union was + // quadratic -- it grows to span the whole model, so every one of + // n subtractions faced O(n) geometry however little the parts + // really touched, and a 100-body 3MF spent 6.6s on it (#177). + // + // The result is identical: a later body whose bounding box is + // disjoint from this one cannot intersect it, and subtracting a + // disjoint solid removes no volume. Bounding boxes settle that in + // nanoseconds, so what is left is proportional to how much the + // model overlaps rather than to how many parts it has. + // + // Skipping the subtraction entirely when nothing overlaps is not + // just a shortcut: `A - disjoint B` returns A's volume but + // REORDERS its triangle list, which throws away any per-triangle + // colours A carried. Most models are mostly disjoint parts, so + // without this a two-tone body lost its colours the moment any + // other differently-coloured body existed. + std::vector boxes; + boxes.reserve(solids.size()); + for (const Solid& s : solids) boxes.push_back(s.man.BoundingBox()); + std::vector owned; + std::vector blockers; for (size_t n = solids.size(); n-- > 0;) { Solid& s = solids[n]; - manifold::Manifold piece = s.man; - if (claimed) { - // Skipping the subtraction when the bounding boxes - // cannot overlap is not just a shortcut: `A - disjoint - // B` returns A's volume but REORDERS its triangle list, - // which throws away any per-triangle colours A carried. - // Most models are mostly disjoint parts, so without this - // a two-tone body lost its colours the moment any other - // differently-coloured body existed. - if (boxesOverlap(s.man, *claimed)) piece = s.man - *claimed; + blockers.clear(); + for (size_t m = n + 1; m < solids.size(); ++m) { + if (boxesOverlap(boxes[n], boxes[m])) blockers.push_back(solids[m].man); } - claimed = claimed ? (*claimed + s.man) : s.man; + manifold::Manifold piece = s.man; + if (!blockers.empty()) piece = s.man - addAll(blockers); if (piece.IsEmpty()) continue; Claimed c; c.man = std::move(piece); diff --git a/tests/test_export.cpp b/tests/test_export.cpp index 9e1e397..0c56903 100644 --- a/tests/test_export.cpp +++ b/tests/test_export.cpp @@ -5,6 +5,7 @@ #include "test_helpers.hpp" #include +#include #include #include #include @@ -755,6 +756,69 @@ TEST(SplitColors, SingleMaterialGivesOneObject) { EXPECT_TRUE(objs[0].triColors.empty()); // no per-triangle colour to carry } +// Each body is cut only by the later bodies that actually reach it, rather +// than by a running union of all of them (#177). These pin the two things +// that rewrite must not change. + +TEST(SplitColors, AFarAwayBodyChangesNothing) { + // A body nowhere near the others cannot take volume from them, so adding + // one must leave every other object exactly as it was. The old code + // subtracted the union of ALL later bodies and leaned on a bounding-box + // test against that union to stay correct -- once the union spanned the + // model, the test stopped discriminating. + const std::string pair = + "color(\"red\") cube(10); color(\"blue\") translate([5,0,0]) cube(10);"; + std::vector without = evalToBodies(pair); + std::vector with = + evalToBodies(pair + " color(\"green\") translate([500,0,0]) cube(10);"); + + const std::vector a = splitBodiesForExport(without, nullptr, false, true); + const std::vector b = splitBodiesForExport(with, nullptr, false, true); + ASSERT_EQ(a.size(), 2u); + ASSERT_EQ(b.size(), 3u); + for (size_t i = 0; i < a.size(); ++i) { + EXPECT_EQ(a[i].tris.size(), b[i].tris.size()) << "object " << i << " was re-cut"; + EXPECT_EQ(a[i].verts.size(), b[i].verts.size()) << "object " << i << " was re-cut"; + } +} + +TEST(SplitColors, LaterWinsAcrossAnInterveningColour) { + // red, blue, red -- all overlapping, in that order. The later body wins, + // so the second red takes its volume whole, blue keeps only what that + // red does not cover, and the first red keeps only what neither does. + // + // Worth pinning because the obvious way to make this cheaper -- union + // the same-coloured bodies first, then subtract per colour -- gets it + // wrong: it would let the first red beat the blue that comes after it. + std::vector bodies = evalToBodies( + "color(\"red\") cube(10);" + "color(\"blue\") translate([5,0,0]) cube(10);" + "color(\"red\") translate([10,0,0]) cube(10);"); + const std::vector objs = splitBodiesForExport(bodies, nullptr, false, true); + ASSERT_EQ(objs.size(), 2u); // one per colour, the two reds merged + + const auto volumeOf = [](const ExportObject& o) { + double v = 0.0; + for (size_t t = 0; t + 2 < o.tris.size(); t += 3) { + const auto at = [&](size_t k) { + const size_t b = static_cast(o.tris[t + k]) * 3; + return std::array{o.verts[b], o.verts[b + 1], o.verts[b + 2]}; + }; + const std::array p = at(0), q = at(1), r = at(2); + v += (p[0] * (q[1] * r[2] - q[2] * r[1]) - p[1] * (q[0] * r[2] - q[2] * r[0]) + + p[2] * (q[0] * r[1] - q[1] * r[0])) / 6.0; + } + return v; + }; + + // Blue spans x 5..15 and the later red takes x 10..20, leaving x 5..10: + // 5 x 10 x 10. The reds keep everything else: 2000 - 500. + double red = 0.0, blue = 0.0; + for (const ExportObject& o : objs) (o.color[2] > o.color[0] ? blue : red) += volumeOf(o); + EXPECT_NEAR(blue, 500.0, 1e-6); + EXPECT_NEAR(red, 1500.0, 1e-6); +} + TEST(SplitColors, SingleMaterialWeldsTouchingColourRegions) { // Unioned, not concatenated: two touching cubes of different colours // must come out as one solid without the coincident interior faces