From 26877e24457bf67d772557ef1fb69a177990070e Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Thu, 10 Sep 2026 21:04:11 -0700 Subject: [PATCH] Cut faces: the cut green for uncoloured tools, and keepMinuendColor - An uncoloured subtrahend now paints the faces it exposes the reference's cut green (#9DCB51, what its preview shows and its colour-preserving render keeps) instead of the default geometry colour, so a cut through an uncoloured part reads as a cut. Recorded against the tool's runs before the merge; the merge looks even when every operand's colour agrees. - Evaluator::keepMinuendColor (bindings: keep_minuend_color=True) makes difference() paint cut faces with the minuend's colour instead -- the option OpenCSG cannot offer (openscad/openscad#4798). Each minuend part is differenced on its own, its cut-face runs re-minted under fresh IDs carrying that part's colour, then the parts are unioned. A union() built in this mode remembers what it merged (ColoredBody::mergedFrom, carried through transforms, color() and the cache restamp), so a union of coloured parts cuts each in its own colour. Cache keys are prefixed per mode. Off by default. BelfrySCAD #412. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Y9HJERiDZrsAb5tgmwdqU6 --- CLAUDE.md | 29 +++- bindings/module.cpp | 5 +- .../openscad_cpp_evaluator/colored_body.hpp | 18 +++ include/openscad_cpp_evaluator/evaluator.hpp | 16 ++ pyproject.toml | 2 +- python/openscad_cpp_evaluator/__init__.py | 10 +- src/builtins/booleans.cpp | 106 ++++++++++++ src/builtins/color.cpp | 40 ++--- src/builtins/transforms.cpp | 17 ++ src/csg_generate.cpp | 42 +++++ tests/test_multi_color_merge.cpp | 151 +++++++++++++++++- tests/test_python_bindings.py | 11 ++ 12 files changed, 413 insertions(+), 34 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e48bf88..2cc5dc2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -866,8 +866,33 @@ grep for `ponytail:`. And the invariant behind both: **`triColors` must index the body's CURRENT mesh**. The renderer masks its vertex arrays with it, so a stale length is an `IndexError` and a blank viewport rather than a wrong colour. Anything that re-meshes clears it — `simplify()`, `minkowski_difference()`, - and the ID-retag rebuild when the triangle count moves — and `bodyToDict` drops a mismatched array - as a backstop for whichever one gets missed next. + and `bodyToDict` drops a mismatched array as a backstop for whichever one gets missed next. The + ID-retag rebuild on a cache hit (`restampCachedIds`) used to clear it too when the triangle count + moved; it now **rebuilds it from the runs**, which survive the rebuild, and **re-records every + run's colour against the fresh IDs** from the cached body itself (its `color`, or per run what its + `triColors` carry) — this render's `idToColor` has never heard of an ID an earlier render minted, + and without it every cached operand looked uncoloured to the next merge: an edited three-colour + `difference()` lost the unchanged subtrahend's colour on the first re-render and went flat on the + next (BelfrySCAD #412). + **An uncoloured subtrahend paints the faces it exposes the cut green** (`kCutFaceColor`, + `colored_body.hpp`: #9DCB51, the reference's CGAL back-face green, which its preview shows and its + colour-preserving render keeps — measured in a 3MF export from 2026.02.01), recorded against the + tool's runs before the merge, and the merge is forced to look even when every operand's own colour + agrees. It used to fall to the default geometry colour, so a cut through an uncoloured part could + not be seen as a cut. A coloured tool still paints its cut with its own colour, as the reference + does. + **`Evaluator::keepMinuendColor`** (bindings: `Evaluator(keep_minuend_color=True)`) is the viewer + option the reference cannot offer (openscad/openscad#4798 — OpenCSG preview is pixels in a frame + buffer): `difference()` paints its cut faces with the **minuend's** colour instead. Since + (A ∪ B) − S = (A − S) ∪ (B − S), each minuend part is differenced on its own and its cut-face runs + are re-minted under fresh IDs carrying that part's colour (`finishKeepMinuend`, `booleans.cpp`; the + source node stays the tool's, so a click on a cut face still finds it), then the parts are unioned. + A `union()` built in this mode remembers what it merged (`ColoredBody::mergedFrom`, unmerged and + carried through `generateTransform`/`generateColor`/`restampCachedIds`), so `difference() { + union() { red; blue; } tool }` cuts red on the red side and blue on the blue side; any op that builds + a new body drops the record and that body is one part. Off by default — a viewer option, not a + language feature, so it is honest for every existing script — and cache keys are prefixed per + mode so the two never serve each other's bodies. Costs one boolean per minuend part. **2D cannot use any of that, and keeps colour geometrically instead** (`Part2d`, same file): a `CrossSection` is contours, not a mesh, so nothing in it remembers which child an edge came from and there is no provenance for `attachTriColors` to read back. So the 2D accumulator holds **one diff --git a/bindings/module.cpp b/bindings/module.cpp index 96c6010..2674028 100644 --- a/bindings/module.cpp +++ b/bindings/module.cpp @@ -477,7 +477,7 @@ nb::object coverageResultToPy(const std::optional& cr nb::object evaluate(const std::string& path, nb::dict viewportParams, std::shared_ptr manifoldCache, bool profile, - bool generate, bool strictCommas, bool coverage) { + bool generate, bool strictCommas, bool coverage, bool keepMinuendColor) { std::unordered_map vp = toViewportParams(viewportParams); std::vector bodies; @@ -501,6 +501,7 @@ nb::object evaluate(const std::string& path, nb::dict viewportParams, oscad::ParsedProgram program = oscad::getProgramFromFile(path); oscadeval::ResolvedUseScopes used = oscadeval::resolveUseScopes(program.nodes, path, logFn); oscadeval::Evaluator ev(logFn, nullptr, manifoldCache, oscadeval::DebugHooks{}, profile, coverage); + ev.keepMinuendColor = keepMinuendColor; ev.setUsedFileGlobals(used.usedFileGlobals); oscadeval::EvalContext ctx = oscadeval::EvalContext::makeRoot(used.rootScope.get()); bodies = oscadeval::toRenderableBodies(ev.evaluate(used.processedNodes, ctx, vp, generate)); @@ -955,7 +956,7 @@ NB_MODULE(_openscad_cpp_evaluator, m) { m.def("evaluate", &evaluate, nb::arg("path"), nb::arg("viewport_params"), nb::arg("manifold_cache") = nullptr, nb::arg("profile") = false, nb::arg("generate") = true, nb::arg("strict_commas") = false, - nb::arg("coverage") = false, + nb::arg("coverage") = false, nb::arg("keep_minuend_color") = false, "Evaluate a .scad file; return (bodies, echoes, id_to_node, csg_tree, profile_result, dyn, dyn_explicit, " "geometry, coverage_result).\n" "coverage=True records which statements, branch arms and bodies ran: coverage_result is a dict " diff --git a/include/openscad_cpp_evaluator/colored_body.hpp b/include/openscad_cpp_evaluator/colored_body.hpp index 1b51dd8..9370503 100644 --- a/include/openscad_cpp_evaluator/colored_body.hpp +++ b/include/openscad_cpp_evaluator/colored_body.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -28,6 +29,13 @@ enum class BodyRole { Normal, Highlight, Background, ShowOnly }; // Matches the reference's SceneRenderer._default_color. inline constexpr std::array kDefaultGeometryColor{0.9f, 0.85f, 0.1f, 1.0f}; +// The colour of a face a difference() exposed when the subtrahend that cut +// it carried no colour of its own: the reference's CGAL "back face" green, +// the same one its preview paints and its colour-preserving render (and +// 3MF export, measured on 2026.02.01) keeps. Distinct from the default +// geometry colour so a cut reads as a cut. +inline constexpr std::array kCutFaceColor{157.0f / 255.0f, 203.0f / 255.0f, 81.0f / 255.0f, 1.0f}; + struct ColoredBody { std::optional body; std::optional> color; // RGBA; nullopt = "no explicit color() -- follow the live theme" @@ -58,6 +66,16 @@ struct ColoredBody { BodyRole role = BodyRole::Normal; std::optional>> triColors; // per-triangle RGBA, multi-color CSG merges only + // The operands a union() merged into this body, kept unmerged, each as + // it was when merged (colour and transforms carried along since -- + // generateTransform and generateColor map over them). Set only under + // Evaluator::keepMinuendColor, where a difference() cuts each part on + // its own so its cut faces take THAT part's colour; nothing else reads + // it, and any other op that builds a new body from this one drops it. + // Shared, never mutated in place: an op that changes it makes a new + // vector. + std::shared_ptr> mergedFrom; + // Whether `body` is empty, once anything has asked (isEmptyBody, // csg_generate.cpp). Asking Manifold is not free: every accessor goes // through GetImpl(), which MATERIALIZES a lazy transform (a full copy of diff --git a/include/openscad_cpp_evaluator/evaluator.hpp b/include/openscad_cpp_evaluator/evaluator.hpp index 71c5270..4c29bd8 100644 --- a/include/openscad_cpp_evaluator/evaluator.hpp +++ b/include/openscad_cpp_evaluator/evaluator.hpp @@ -294,6 +294,22 @@ class Evaluator { std::size_t generatedNodeCount = 0; std::unordered_map>> idToColor; + // Record `rgba` against every run ID of `b`, so a later merge can still + // tell the body's triangles apart once the body itself is gone + // (attachTriColors looks runs up in idToColor). Cheap for a body that is + // still one original; a merged body pays a GetMeshGL(). + void recordRunColors(ColoredBody& b, const std::optional>& rgba); + + // difference() keeps the minuend's colour on the faces a subtrahend + // exposes, instead of the subtrahend's colour (or the cut green). Off + // by default: OpenSCAD paints cut faces with the cutter's colour, and + // that is what a script author sees there. A viewer option, not a + // language feature -- it is honest for every existing script. Costs one + // boolean per coloured minuend part instead of one per difference, + // since (A ∪ B) − S = (A − S) ∪ (B − S) is how each part's cut faces get + // that part's colour. Cached results are keyed apart per mode. + bool keepMinuendColor = false; + // Called by primitive-construction generate functions (cube, sphere, // cylinder, polyhedron) right after building a brand-new Manifold: // reads back its mesh's runOriginalID run(s) and records each against diff --git a/pyproject.toml b/pyproject.toml index 7966206..4b643d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "1.19.3" +version = "1.20.0" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/python/openscad_cpp_evaluator/__init__.py b/python/openscad_cpp_evaluator/__init__.py index d90dda4..6087b28 100644 --- a/python/openscad_cpp_evaluator/__init__.py +++ b/python/openscad_cpp_evaluator/__init__.py @@ -532,7 +532,8 @@ class Evaluator: """ def __init__(self, echo_fn=None, debug_hook=None, error_break_fn=None, return_hook=None, - manifold_cache=None, profile=False, fast_continue_signal=None, coverage=False): + manifold_cache=None, profile=False, fast_continue_signal=None, coverage=False, + keep_minuend_color=False): self._echo_fn = echo_fn self._debug_hook = debug_hook self._error_break_fn = error_break_fn @@ -541,6 +542,11 @@ def __init__(self, echo_fn=None, debug_hook=None, error_break_fn=None, return_ho self._profile = profile self._fast_continue_signal = fast_continue_signal self._coverage = coverage + # keep_minuend_color=True: difference() paints the faces a + # subtrahend exposes with the MINUEND's colour rather than the + # subtrahend's (or the cut green). A viewer option; see + # Evaluator::keepMinuendColor in evaluator.hpp. + self._keep_minuend_color = keep_minuend_color self.csg_tree = [] self.profile_result = None # coverage=True: after evaluate(), {"spans": [...], "files": [...], @@ -594,7 +600,7 @@ def evaluate(self, source_path: str, viewport_params: Optional[dict] = None, (body_dicts, echoes, id_spans, csg_tree, profile_result, dyn, dyn_explicit, geometry, coverage_result) = _ext.evaluate( source_path, vp, self._manifold_cache, self._profile, generate, - strict_commas, self._coverage) + strict_commas, self._coverage, self._keep_minuend_color) # The evaluated bodies, still on the C++ side. Stashed like # csg_tree/profile_result rather than returned, so # evaluate()'s own 2-tuple result is unchanged -- callers diff --git a/src/builtins/booleans.cpp b/src/builtins/booleans.cpp index d6dd7bc..673ebed 100644 --- a/src/builtins/booleans.cpp +++ b/src/builtins/booleans.cpp @@ -7,6 +7,7 @@ #include #include #include +#include namespace oscadeval { @@ -199,6 +200,61 @@ void attachTriColors(Evaluator& ev, ColoredBody& cb) { cb.triColors = std::move(triColors); } +// keepMinuendColor: the leaf parts of a minuend operand -- the operand +// itself, or what a union() merged it from, recursively. +void collectKeepParts(const ColoredBody& b, std::vector& out) { + if (b.mergedFrom) { + for (const ColoredBody& part : *b.mergedFrom) collectKeepParts(part, out); + } else if (b.body) { + out.push_back(b); + } +} + +// keepMinuendColor: one minuend part, differenced on its own. +struct KeepPart { + ColoredBody body; + std::vector ownIds; // run IDs the part was born with; anything else on it afterwards is a cut face +}; + +std::vector runIdsOf(const ColoredBody& b) { + const int original = b.body->OriginalID(); + if (original >= 0) return {static_cast(original)}; + return b.body->GetMeshGL().runOriginalID; +} + +// Union the per-part differences back into one body, first re-minting each +// part's cut-face runs (the subtrahend's IDs, shared by every part's result) +// under fresh IDs carrying THAT part's colour, so attachTriColors can tell +// one part's cut faces from another's. The source node stays the +// subtrahend's: clicking a cut face still finds the tool that made it. +manifold::Manifold finishKeepMinuend(Evaluator& ev, std::vector& parts) { + std::optional out; + for (KeepPart& part : parts) { + if (!part.body.body) continue; + manifold::MeshGL mesh = part.body.body->GetMeshGL(); + if (mesh.triVerts.empty()) continue; + std::unordered_map remap; + for (uint32_t& id : mesh.runOriginalID) { + if (std::find(part.ownIds.begin(), part.ownIds.end(), id) != part.ownIds.end()) continue; + auto found = remap.find(id); + if (found == remap.end()) { + const uint32_t fresh = manifold::Manifold::ReserveIDs(1); + auto node = ev.idToNode.find(id); + if (node != ev.idToNode.end()) ev.idToNode[fresh] = node->second; + // ponytail: a part that is itself a multi-colour merge + // gives its cut faces its first child's colour rather than + // the colour of whichever child the cut passed through. + ev.idToColor[fresh] = part.body.color; + found = remap.emplace(id, fresh).first; + } + id = found->second; + } + manifold::Manifold rebuilt(mesh); + out = out ? *out + rebuilt : rebuilt; + } + return out.value_or(manifold::Manifold()); +} + // One colour's worth of 2D result. // // 3D recovers per-child colour AFTER the merge, from Manifold's own @@ -286,6 +342,15 @@ std::vector generateCsg(Evaluator& ev, const CSGParams& params, con // only produce a uniformly coloured result. std::optional>> firstColor; bool mixedColors = false; + // keepMinuendColor: the minuend's parts, each differenced on its own + // (see Evaluator::keepMinuendColor). Not while measuring: a render() + // expression only wants the volume, which is the same either way. + const bool keeping = op == "difference" && ev.keepMinuendColor && !ev.measuring(); + std::vector keepParts; + // ... and a union() built under that mode remembers what it merged, so + // a difference() it is the minuend of can cut each part on its own. + const bool rememberParts = op == "union" && ev.keepMinuendColor && !ev.measuring(); + std::vector unionParts; size_t stmtIndex = 0; for (const Value& sizeVal : groupSizes) { @@ -338,6 +403,20 @@ std::vector generateCsg(Evaluator& ev, const CSGParams& params, con if (!firstColor) firstColor = c.color; else if (*firstColor != c.color) mixedColors = true; } + // A subtrahend with no colour of its own paints the faces it + // exposes the cut green, as the reference does, rather than the + // default geometry colour -- otherwise a cut through an uncoloured + // part is invisible as a cut. Recorded against its runs, which is + // all attachTriColors will have left after the merge; and the + // merge must then look, even when every operand's own colour + // agrees. + if (op == "difference" && res3d && !ev.measuring()) { + for (ColoredBody& c : bodies3d) { + if (c.color || c.triColors) continue; + ev.recordRunColors(c, kCutFaceColor); + mixedColors = true; + } + } for (const ColoredBody& c : split.foreground) { if (c.section) sections2d.push_back(c); } @@ -377,8 +456,27 @@ std::vector generateCsg(Evaluator& ev, const CSGParams& params, con cb.knownStatus = manifold::Manifold::Error::NoError; // every operand was cb.knownEmpty = grpEmpty; res3d = std::move(cb); + if (keeping) { + std::vector leaves; + for (const ColoredBody& c : bodies3d) collectKeepParts(c, leaves); + for (ColoredBody& leaf : leaves) { + std::vector ids = runIdsOf(leaf); + keepParts.push_back({std::move(leaf), std::move(ids)}); + } + } + if (rememberParts) { + for (const ColoredBody& c : bodies3d) collectKeepParts(c, unionParts); + } + } else if (keeping) { + // The whole-minuend result above is never evaluated (Manifold + // is lazy); finishKeepMinuend replaces it after the loop. + for (KeepPart& part : keepParts) part.body.body = *part.body.body - grp; + res3d->knownEmpty.reset(); } else if (op == "union") { res3d->body = *res3d->body + grp; + if (rememberParts) { + for (const ColoredBody& c : bodies3d) collectKeepParts(c, unionParts); + } if (known(res3d->knownEmpty, false) || known(grpEmpty, false)) res3d->knownEmpty = false; else if (known(res3d->knownEmpty, true) && known(grpEmpty, true)) res3d->knownEmpty = true; else res3d->knownEmpty.reset(); @@ -415,6 +513,14 @@ std::vector generateCsg(Evaluator& ev, const CSGParams& params, con } } + if (res3d && res3d->body && rememberParts && unionParts.size() > 1) { + res3d->mergedFrom = std::make_shared>(std::move(unionParts)); + } + if (res3d && res3d->body && keeping && !keepParts.empty()) { + res3d->body = finishKeepMinuend(ev, keepParts); + res3d->knownEmpty.reset(); + mixedColors = true; // attachTriColors still no-ops when every run agrees + } if (res3d && res3d->body && mixedColors) attachTriColors(ev, *res3d); std::vector result; diff --git a/src/builtins/color.cpp b/src/builtins/color.cpp index a29600f..7691450 100644 --- a/src/builtins/color.cpp +++ b/src/builtins/color.cpp @@ -52,31 +52,19 @@ CSGParams resolveColor(Evaluator& ev, const oscad::ModularCall& node, EvalContex namespace { -// Stamping cb.color is not enough: the colour has to be recorded against -// the body's RUN IDS too. -// -// attachTriColors (booleans.cpp) reads `idToColor` back AFTER a merge has -// thrown the individual bodies away -- run IDs are all it has left to go -// on -- and a run it cannot find looks uncoloured. tagGenerated() records -// the colour a body was BORN with, which covers `color("red") cube(10)` -// because the colour reaches the primitive through the context. It does -// not cover a body that already existed when colour was applied to it, -// which is every module whose geometry comes out of its own CSG: -// `union() { color("red") stroke(...); text(); }` had no red recorded for -// the stroke's runs at all, so every run looked identical, the merge kept -// the first child's colour, and the whole thing came out red. -void recordRunColors(Evaluator& ev, ColoredBody& b, const std::optional>& rgba) { - if (!b.body || bodyIsEmpty(b)) return; - // A body that is still one original knows its own ID without building - // a mesh -- the common case, and the cheap one. - const int original = b.body->OriginalID(); - if (original >= 0) { - ev.idToColor[static_cast(original)] = rgba; - return; +} // namespace + +namespace { +std::shared_ptr> recolorParts(const std::vector& parts, + const std::optional>& rgba) { + std::vector out; + for (ColoredBody part : parts) { + part.color = rgba; + if (part.mergedFrom) part.mergedFrom = recolorParts(*part.mergedFrom, rgba); + out.push_back(std::move(part)); } - for (uint32_t id : b.body->GetMeshGL().runOriginalID) ev.idToColor[id] = rgba; + return std::make_shared>(std::move(out)); } - } // namespace std::vector generateColor(Evaluator& ev, const CSGParams& params, @@ -85,7 +73,11 @@ std::vector generateColor(Evaluator& ev, const CSGParams& params, std::vector result; for (ColoredBody b : flattenCsgTree(children)) { b.color = rgba; - if (!ev.measuring()) recordRunColors(ev, b, rgba); + if (!ev.measuring()) ev.recordRunColors(b, rgba); + // color() over a union() colours every part it was merged from + // (see ColoredBody::mergedFrom) -- the parts' runs are the body's + // runs, so recording once above already covers them. + if (b.mergedFrom) b.mergedFrom = recolorParts(*b.mergedFrom, rgba); result.push_back(std::move(b)); } return result; diff --git a/src/builtins/transforms.cpp b/src/builtins/transforms.cpp index 85475aa..e01fcbb 100644 --- a/src/builtins/transforms.cpp +++ b/src/builtins/transforms.cpp @@ -349,6 +349,19 @@ CSGParams resolveTransform(Evaluator& ev, const oscad::ModularCall& node, EvalCo return std::move(result.params); } +namespace { +std::shared_ptr> transformParts(const std::vector& parts, + const std::string& name, const CallArgs& args) { + std::vector out; + for (ColoredBody part : parts) { + if (part.body) part.body = applyTransform3d(name, args, std::move(*part.body)); + if (part.mergedFrom) part.mergedFrom = transformParts(*part.mergedFrom, name, args); + out.push_back(std::move(part)); + } + return std::make_shared>(std::move(out)); +} +} // namespace + std::vector generateTransform(Evaluator&, const CSGParams& params, const std::vector>& children, const oscad::ASTNode&) { const std::string& name = std::get(params.at("name")); @@ -391,6 +404,10 @@ std::vector generateTransform(Evaluator&, const CSGParams& params, if (std::optional m = transformMatrix3d(name, args)) transformMeshInPlace(*b.rawMesh, *m); } + // The unmerged parts of a union move with it (see + // ColoredBody::mergedFrom). Recursion through this same + // function keeps their own parts moving too. + if (b.mergedFrom) b.mergedFrom = transformParts(*b.mergedFrom, name, args); } result.push_back(std::move(b)); } diff --git a/src/csg_generate.cpp b/src/csg_generate.cpp index a9f06be..5c50ac7 100644 --- a/src/csg_generate.cpp +++ b/src/csg_generate.cpp @@ -46,6 +46,11 @@ std::vector Evaluator::generateTreeImpl(const std::vector std::optional key; if (manifoldCache_ && !node.uncacheable) { key = cacheKey(node); + // A difference() built with keepMinuendColor colours its cut + // faces differently, so the two modes must never serve each + // other's bodies. Prefixing every key (not only differences) + // keeps the rule simple; the cache holds both. + if (keepMinuendColor) key->insert(0, "keepMinuendColor;"); // A key that serialized a function literal is not a pure // function of content -- it embeds a raw AST address that the // next parse can hand out again. Dropping the key here forces @@ -305,6 +310,26 @@ std::vector Evaluator::generatePartialTree() { return generateTreeImpl(flat); } +namespace { +std::shared_ptr> remapParts(const std::vector& parts, + const std::unordered_map& remap) { + std::vector out; + for (ColoredBody part : parts) { + if (part.mergedFrom) part.mergedFrom = remapParts(*part.mergedFrom, remap); + if (part.body && !part.body->IsEmpty()) { + manifold::MeshGL mesh = part.body->GetMeshGL(); + for (uint32_t& id : mesh.runOriginalID) { + auto found = remap.find(id); + if (found != remap.end()) id = found->second; + } + part.body = manifold::Manifold(mesh); + } + out.push_back(std::move(part)); + } + return std::make_shared>(std::move(out)); +} +} // namespace + void Evaluator::restampCachedIds(std::vector& bodies, const oscad::ASTNode& node, const oscad::ASTNode* producer) { // A cache hit hands back the geometry AND the originalIDs of whichever @@ -382,6 +407,10 @@ void Evaluator::restampCachedIds(std::vector& bodies, const oscad:: id = found->second; } cb.body = manifold::Manifold(mesh); + // The unmerged parts a union() remembered (ColoredBody::mergedFrom) + // share the merged body's IDs, so the same remap applies to them; + // a part left on the old IDs would look uncoloured next to it. + if (cb.mergedFrom) cb.mergedFrom = remapParts(*cb.mergedFrom, remap); // Rebuilding a Manifold from a mesh can clean it up (degenerate // triangles merged away), and triColors is indexed by triangle, so // the old array no longer lines up. The RUNS survive the rebuild, @@ -404,6 +433,19 @@ void Evaluator::restampCachedIds(std::vector& bodies, const oscad:: } } + +void Evaluator::recordRunColors(ColoredBody& b, const std::optional>& rgba) { + if (!b.body || bodyIsEmpty(b)) return; + // A body that is still one original knows its own ID without building + // a mesh -- the common case, and the cheap one. + const int original = b.body->OriginalID(); + if (original >= 0) { + idToColor[static_cast(original)] = rgba; + return; + } + for (uint32_t id : b.body->GetMeshGL().runOriginalID) idToColor[id] = rgba; +} + ColoredBody Evaluator::tagGenerated(manifold::Manifold body, const oscad::ASTNode& node, const Value& colorValue) { manifold::MeshGL mesh = body.GetMeshGL(); std::optional> color = valueToColor(colorValue); diff --git a/tests/test_multi_color_merge.cpp b/tests/test_multi_color_merge.cpp index 4540bff..749d83e 100644 --- a/tests/test_multi_color_merge.cpp +++ b/tests/test_multi_color_merge.cpp @@ -1,4 +1,5 @@ #include "openscad_cpp_evaluator/evaluator.hpp" +#include "openscad_cpp_evaluator/manifold_cache.hpp" #include "test_helpers.hpp" @@ -53,11 +54,13 @@ TEST(MultiColorCsgMerge, UnionOfMixedColorsSetsTriColors) { [](const auto& c) { return c[3] == 1.0f && !approxEqual(c, {0.0f, 1.0f, 1.0f, 0.5f}); })); } -TEST(MultiColorCsgMerge, DifferenceCutFaceGetsDefaultColor) { +TEST(MultiColorCsgMerge, DifferenceCutFaceGetsTheCutGreen) { // The cylinder tool has no explicit color() -- its newly-exposed cut // face (a fresh runOriginalID contributed by the subtraction tool) - // must fall back to the default geometry color, matching what real - // OpenSCAD shows for an uncolored modifier used only as a cutter. + // takes the cut green, which is what OpenSCAD 2026.02.01 paints there + // in preview and keeps in a colour-preserving render (3MF export + // measured: #9DCB51). It used to take the default geometry colour, + // so a cut through an uncoloured part could not be seen as a cut. Evaluated e = evalSrc("difference() {" " union() {" " color(\"lightgreen\") cube(10);" @@ -70,9 +73,151 @@ TEST(MultiColorCsgMerge, DifferenceCutFaceGetsDefaultColor) { const auto distinct = distinctColors(*e.bodies[0].triColors); EXPECT_EQ(distinct.size(), 3u); EXPECT_TRUE(std::any_of(distinct.begin(), distinct.end(), [](const auto& c) { return approxEqual(c, {0.0f, 1.0f, 1.0f, 0.5f}); })); + EXPECT_TRUE(std::any_of(distinct.begin(), distinct.end(), [](const auto& c) { return approxEqual(c, kCutFaceColor); })); +} + +TEST(MultiColorCsgMerge, UncolouredMinuendCutByUncolouredToolStillShowsTheCut) { + // Nothing is coloured, so the operands' own colours agree -- the merge + // must look anyway, or the cut vanishes into the default colour. + Evaluated e = evalSrc("difference() { cube(10); translate([5,5,-1]) cylinder(h=12, d=6, $fn=16); }"); + ASSERT_EQ(e.bodies.size(), 1u); + ASSERT_TRUE(e.bodies[0].triColors.has_value()); + const auto distinct = distinctColors(*e.bodies[0].triColors); + EXPECT_EQ(distinct.size(), 2u); + EXPECT_TRUE(std::any_of(distinct.begin(), distinct.end(), [](const auto& c) { return approxEqual(c, kCutFaceColor); })); EXPECT_TRUE(std::any_of(distinct.begin(), distinct.end(), [](const auto& c) { return approxEqual(c, kDefaultGeometryColor); })); } +TEST(MultiColorCsgMerge, ColouredToolStillPaintsItsCut) { + Evaluated e = evalSrc("difference() { color(\"orange\") cube(10); color(\"cyan\") translate([5,5,-1]) cylinder(h=12, d=6, $fn=16); }"); + const auto distinct = distinctColors(*e.bodies[0].triColors); + EXPECT_EQ(distinct.size(), 2u); + EXPECT_TRUE(std::any_of(distinct.begin(), distinct.end(), [](const auto& c) { return approxEqual(c, {0.0f, 1.0f, 1.0f, 1.0f}); })); +} + +namespace { +Evaluated evalKeepingMinuend(const std::string& code) { + Evaluated e{parseSrc(code), nullptr, Evaluator(), {}, {}}; + e.ev.keepMinuendColor = true; + e.scope = oscad::buildScopes(e.ast); + EvalContext ctx = EvalContext::makeRoot(e.scope.get()); + e.tree = e.ev.resolveTree(e.ast, ctx); + e.bodies = e.ev.generateTree(e.tree); + return e; +} +} // namespace + +TEST(KeepMinuendColor, SingleColouredMinuendStaysOneColour) { + // BelfrySCAD #412's enhancement: the pencil sharpened by a cone should + // still be pencil-coloured at the point. + Evaluated e = evalKeepingMinuend( + "difference() { color(\"orange\") cube(20, center=true); cylinder(30, r=8, center=true, $fn=24); }"); + ASSERT_EQ(e.bodies.size(), 1u); + // Every run resolves to orange, so no per-triangle array is needed. + EXPECT_FALSE(e.bodies[0].triColors.has_value()); + ASSERT_TRUE(e.bodies[0].color.has_value()); + EXPECT_TRUE(approxEqual(*e.bodies[0].color, {1.0f, 0.647058845f, 0.0f, 1.0f})); + // And the geometry is the same difference. + Evaluated plain = evalSrc("difference() { color(\"orange\") cube(20, center=true); cylinder(30, r=8, center=true, $fn=24); }"); + EXPECT_NEAR(e.bodies[0].body->Volume(), plain.bodies[0].body->Volume(), 1e-3); +} + +TEST(KeepMinuendColor, EachMinuendPartKeepsItsOwnColourOnItsCutFaces) { + // A red block at x<0 and a blue one at x>0, a coloured tool through + // both: the cut faces on the red side are red, on the blue side blue, + // and the tool's cyan appears nowhere. + Evaluated e = evalKeepingMinuend( + "difference() {" + " union() { color(\"red\") translate([-10,-5,-5]) cube(10); color(\"blue\") translate([0,-5,-5]) cube(10); }" + " color(\"cyan\") rotate([0,90,0]) cylinder(h=30, r=2, center=true, $fn=16);" + "}"); + ASSERT_EQ(e.bodies.size(), 1u); + ASSERT_TRUE(e.bodies[0].triColors.has_value()); + const auto distinct = distinctColors(*e.bodies[0].triColors); + EXPECT_EQ(distinct.size(), 2u); + const manifold::MeshGL mesh = e.bodies[0].body->GetMeshGL(); + const auto& tc = *e.bodies[0].triColors; + ASSERT_EQ(tc.size(), mesh.triVerts.size() / 3); + for (size_t t = 0; t < tc.size(); ++t) { + double cx = 0; + for (int k = 0; k < 3; ++k) cx += mesh.vertProperties[mesh.triVerts[3 * t + k] * mesh.numProp]; + cx /= 3; + const std::array want = cx < 0 ? std::array{1, 0, 0, 1} : std::array{0, 0, 1, 1}; + ASSERT_TRUE(approxEqual(tc[t], want)) << "triangle " << t << " at x=" << cx; + } +} + +TEST(KeepMinuendColor, PartsFollowTransformsAndColorOverTheUnion) { + // translate() over the union moves the remembered parts with it, so + // the per-part cut still lands on the right side; color() over the + // union recolours every part, so the cut is that colour and nothing + // else. + Evaluated moved = evalKeepingMinuend( + "difference() {" + " translate([0,0,20]) union() { color(\"red\") translate([-10,-5,-5]) cube(10); color(\"blue\") translate([0,-5,-5]) cube(10); }" + " translate([0,0,20]) color(\"cyan\") rotate([0,90,0]) cylinder(h=30, r=2, center=true, $fn=16);" + "}"); + ASSERT_TRUE(moved.bodies[0].triColors.has_value()); + EXPECT_EQ(distinctColors(*moved.bodies[0].triColors).size(), 2u); + const manifold::MeshGL mesh = moved.bodies[0].body->GetMeshGL(); + const auto& tc = *moved.bodies[0].triColors; + for (size_t t = 0; t < tc.size(); ++t) { + double cx = 0; + for (int k = 0; k < 3; ++k) cx += mesh.vertProperties[mesh.triVerts[3 * t + k] * mesh.numProp]; + cx /= 3; + const std::array want = cx < 0 ? std::array{1, 0, 0, 1} : std::array{0, 0, 1, 1}; + ASSERT_TRUE(approxEqual(tc[t], want)) << "triangle " << t << " at x=" << cx; + } + + Evaluated recoloured = evalKeepingMinuend( + "difference() {" + " color(\"green\") union() { color(\"red\") translate([-10,-5,-5]) cube(10); color(\"blue\") translate([0,-5,-5]) cube(10); }" + " color(\"cyan\") rotate([0,90,0]) cylinder(h=30, r=2, center=true, $fn=16);" + "}"); + EXPECT_FALSE(recoloured.bodies[0].triColors.has_value()); + ASSERT_TRUE(recoloured.bodies[0].color.has_value()); + EXPECT_TRUE(approxEqual(*recoloured.bodies[0].color, {0.0f, 128.0f / 255.0f, 0.0f, 1.0f})); +} + +TEST(KeepMinuendColor, CachedUnionMinuendStillCutsPerPart) { + // Edit only the tool: the union is a cache hit, restamped to fresh + // IDs, and its remembered parts must be restamped with it. + auto cache = std::make_shared(); + auto run = [&](const std::string& tool) { + Evaluated e{parseSrc("difference() {" + " union() { color(\"red\") translate([-10,-5,-5]) cube(10); color(\"blue\") translate([0,-5,-5]) cube(10); }" + " " + tool + " }"), + nullptr, Evaluator({}, nullptr, cache), {}, {}}; + e.ev.keepMinuendColor = true; + e.scope = oscad::buildScopes(e.ast); + EvalContext ctx = EvalContext::makeRoot(e.scope.get()); + e.tree = e.ev.resolveTree(e.ast, ctx); + e.bodies = e.ev.generateTree(e.tree); + return e; + }; + run("rotate([0,90,0]) cylinder(h=30, r=2, center=true, $fn=16);"); + Evaluated second = run("rotate([0,90,0]) cylinder(h=30, r=3, center=true, $fn=16);"); + ASSERT_TRUE(second.bodies[0].triColors.has_value()); + const auto distinct = distinctColors(*second.bodies[0].triColors); + EXPECT_EQ(distinct.size(), 2u); + EXPECT_TRUE(std::any_of(distinct.begin(), distinct.end(), [](const auto& c) { return approxEqual(c, {1, 0, 0, 1}); })); + EXPECT_TRUE(std::any_of(distinct.begin(), distinct.end(), [](const auto& c) { return approxEqual(c, {0, 0, 1, 1}); })); +} + +TEST(KeepMinuendColor, CachedResultsAreKeyedApartPerMode) { + auto cache = std::make_shared(); + const std::string src = "difference() { color(\"orange\") cube(10); translate([5,5,-1]) cylinder(h=12, d=6, $fn=16); }"; + Evaluated off = evalSrcWithCache(src, cache); + ASSERT_TRUE(off.bodies[0].triColors.has_value()); // orange + cut green + Evaluated on{parseSrc(src), nullptr, Evaluator({}, nullptr, cache), {}, {}}; + on.ev.keepMinuendColor = true; + on.scope = oscad::buildScopes(on.ast); + EvalContext ctx = EvalContext::makeRoot(on.scope.get()); + on.tree = on.ev.resolveTree(on.ast, ctx); + on.bodies = on.ev.generateTree(on.tree); + EXPECT_FALSE(on.bodies[0].triColors.has_value()); // all orange, not the cached green-cut body +} + TEST(MultiColorCsgMerge, UnionOfSameExplicitColorLeavesTriColorsUnset) { // Cheap-path guarantee: if every contributing color resolves to the // same value, triColors must stay unset (same single-buffer, diff --git a/tests/test_python_bindings.py b/tests/test_python_bindings.py index 3c77f49..73e940d 100644 --- a/tests/test_python_bindings.py +++ b/tests/test_python_bindings.py @@ -865,3 +865,14 @@ def test_coverage_reports_statements_arms_and_bodies(): assert Evaluator().coverage_result is None off = Evaluator(); off.evaluate(path, generate=False) assert off.coverage_result is None + + +def test_keep_minuend_color_paints_cut_faces_with_the_minuend(tmp_path): + src = tmp_path / "keep.scad" + src.write_text('difference() { color("orange") cube(20, center=true);' + ' color("cyan") cylinder(30, r=8, center=true, $fn=24); }') + plain, _ = Evaluator().evaluate(str(src), {}) + assert plain[0].tri_colors is not None # orange faces and cyan cut faces + kept, _ = Evaluator(keep_minuend_color=True).evaluate(str(src), {}) + assert kept[0].tri_colors is None # one colour: the minuend's + assert all(abs(a - b) < 1e-3 for a, b in zip(kept[0].color[:3], (1.0, 0.647, 0.0)))