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