The mate palette called all five kinds viable on connectors created as
Point(world), which belong to no body. Clicking one built the Mate feature and
the RECOMPUTE then failed with "mate: mate_cs_b has no associated body" — the
refusal arrived one step too late, after the feature was already in the tree,
and the user is told about it by an error line rather than by the palette that
offered the thing.
mate_options now checks the same two conditions the apply path throws on: B must
have a body (B is the connector whose body moves; A is the fixed reference and
needs none), and that body must still resolve. Either way all five kinds go
non-viable with a reason, so the offer and the kernel cannot disagree.
Verified on the rig: with two Point(world) connectors, every row is now dimmed
and reads "connector B is not attached to a body — a mate moves B's body". That
also exercises the dimmed-with-a-reason presentation for the first time, which
until now had nothing to show because every kind was always viable.
Tests: a new [mate] case covering no body, a body that no longer resolves, and
the revival once B is given one. One existing case needed its setup widened
rather than its assertion weakened: "an unrecorded fingerprint does not make a
type non-viable" built two body-less connectors, so it was asserting a side
effect of the old permissiveness instead of the property it is named for. Its
connectors now have a body, leaving the missing fingerprint as the only variable.
Full [CadDocument] suite: 2458 assertions in 185 cases.
A rectangle with a circle inside it, drawn in ONE sketch, could not be extruded
to a plate with a bore from the GUI. Five defects were in the way. Each was
found by driving the app on a headless rig and measuring the result — the code
reads correctly at every one of these points, which is why they survived.
1. Wire orientation (kernel). SketchEngine::wires_to_face added every hole as
wires[i].Reversed(), which is only right when the sketch happens to wind both
loops the same way. A circle drawn clockwise inside a counter-clockwise
rectangle came out matching the outer boundary, OCCT swept it as a SECOND
contour, and the prism was the plate with its bore filled and the disc's
volume counted twice. Measured: bbox 67.17 x 219.67 x 10 with volume
152088 mm3 against a solid box of 147542 — a body larger than its own
bounding box, which is the signature. Holes are now added as-is and
ShapeFix_Face::FixOrientation() classifies them; that is winding-independent
and is the idiom make_extrude_regions already used for imported glyphs, which
is why holed TEXT always extruded correctly while a holed SKETCH never did.
After the fix: 142996 mm3, implied bore radius 12.03 mm against the circle
drawn.
2. The live-sketch click threw the picked region away. region_at() served only
as a yes/no gate and on_face_selected() carried no argument, so Extrude fell
back to whichever loop the resolver found first — clicking the material of a
plate-with-a-hole extruded the disc. The region is now carried through, and
DesignPanel also hands it to the tool with set_loop_pick(), AFTER open_tool()
because that re-derives selection state, since extrude_uses_loop() reads
selected_loop_entities() and that lives on the tool.
3. region_at() had no hole awareness and no innermost preference: it returned
the first polygon containing the point. It now skips a region when the point
lies inside one of that region's holes, and picks the smallest containing
loop, so a click in the bore selects the disc and a click on the material
selects the plate.
4. Startup segfault. DesignCanvas::request_repaint probed the GL backend via
OpenGLManager::get_gl_info().get_renderer() before the canvas had initialised
GL — glGetString with no context current and, before init_opengl(), no loaded
function pointers. Anything that asked for a repaint while the panel was
still being built landed there, with no window and nothing in the log. It now
bails at the top on !is_initialized() and asks for a Refresh instead. Note
the crash was in the PROBE, not in render(), which already guards itself.
5. A holed sketch on a plane whose normal points -Z came out as the full box PLUS
a disc — 220274 mm3 where 163726 was due (192000 + 28274). wires_to_face took
a SketchPlane parameter it never used and let OCCT infer a surface from the
outer wire; when the inferred normal disagreed with the sketch's, the hole
classification produced no hole. Every face is now built on the sketch's own
gp_Pln.
Also in this change, from the same rig session:
- A right-click that only clears the sketch selection no longer reports itself
as consumed, so it stops suppressing the offer menu. With any geometry in a
live sketch there was no menu route left to add a second entity.
- Escape no longer discards a live sketch that holds drawn geometry; it says so
and keeps the work (live_sketch_has_work()).
- The holed-region fill is an even-odd scanline instead of a keyhole bridge, so
no corridor triangle leaks from the bore to the nearest corner.
- Cyan is reserved for the selection: an unselected region no longer wears a
shade one step off the selected one.
- The origin planes follow the mode, so pressing Sketch on a document that
already has a body offers them again instead of naming a plane you cannot see.
Tests: three [holes] cases over add_extrude_entities asserting the plate-with-bore
volume, solid and face counts on both a +Z and a -Z sketch plane, and the
by-name refusal of two disjoint regions. Full [CadDocument] suite green.
He merged upstream main into the PR branch himself on 2026-08-13. Taking it into
the local branch rather than force-pushing over it: the fork copy is what PR
#15238 shows, and discarding a maintainer's merge to make my own push
fast-forward would be both rude and a loss of 130 upstream commits.
Brings the branch far closer to main than the 2026-07-24 merge-base the PR body
describes, which is most of what snaporca-36u9 was filed for.
Every version bump so far has permanently orphaned every project saved before
it. deserialize_recipe refused anything that was not exactly the current
version, and with no migration path v2 and v3 projects are unopenable today —
the 3MF still carries the mesh, so the user gets a frozen solid and no feature
history, which is the whole point of the subsystem silently absent.
The cause was the shape of the data, not the gate. save/load is one flat
symmetric list of ~90 fields with no framing, so a reader has no way to know
where a feature ends unless it agrees on every field.
Each feature is now written as its own cereal stream behind a length prefix, and
the same few lines handle both directions of mismatch. Older file, newer build:
the sub-stream ends early, the read throws, and the fields already assigned are
kept while the rest default — cereal assigns sequentially, so a mid-list throw
leaves the earlier fields set, and that is what makes this work. Newer file,
older build: the sub-stream holds more bytes than the reader knows; it reads what
it knows and stops, and the outer stream is untouched because the length prefix
was consumed in full. A field a project predates is not a corrupt project, so
neither case is an error.
v4 keeps its own pre-framing flat path and opens exactly as before —
cad_recipe_v4.bin is untouched and now serves as the witness for that. v2 and v3
stay refused, by name: their field lists no longer exist in this code. This fixes
the future, not the past, and the comment says so rather than implying otherwise.
From here a new field only needs appending to save/load — no bump, no orphaned
projects. That removes the cost that had blocked snaporca-44m and snaporca-dgv.
The helix round-trip test was reading the blob back flat, reaching into the
format instead of through it; framing necessarily breaks that, so it now goes
through deserialize_recipe, which is a stronger assertion than it made before.
Every field check it carried is unchanged.
Tests: four new [CadDocument][recipe] cases, including the one the change exists
for — a deliberately truncated feature blob must LOAD, keeping what it could read.
Suite 177 -> 181 cases, 2366 -> 2411 assertions.
snaporca-2txy.
entities_to_wire handled exactly two shapes of sketch: one lone Circle/Ellipse, or
any number of Line/Arc/EllipseArc/BSpline pushed into a single MakeWire. Everything
else fell off the end as a null wire, so a circle drawn inside a rectangle — the
most ordinary thing in this whole program — refused with "not supported yet". Two
separate closed polygons were quietly worse: both went into one MakeWire, which
does not mean "two loops" to OCCT.
entities_to_wires now returns one wire per loop. A Circle or Ellipse is a loop on
its own; chain entities are grouped by shared endpoints (union-find, 1e-6 in sketch
coordinates), and an open chain still comes back as a wire because a sweep path is
legitimately open. It is all-or-nothing: one loop that fails to build poisons the
whole result, because a partial profile would extrude a shape the user did not draw
— the failure 2e6a8f9e91 was written to stop.
entities_to_wire survives as a two-line wrapper returning the single wire when
there is exactly one loop and a null wire otherwise, so all nine of its call sites
keep their exact contract and Revolve/Sweep/Loft/Surface* are untouched. What a
holed profile means for each of those is a separate question.
wires_to_face takes the largest-area loop as the outer boundary and adds the rest
reversed, which is how OCCT is told a wire is a hole. Containment is CHECKED with
BRepClass_FaceClassifier, not assumed: a loop outside the largest one is a second
island, and one sketch producing several solids is a much bigger feature, so it is
refused by name ("two disjoint regions") rather than guessed at.
Only the Extrude case consumes the new face. Tapered extrudes of a holed profile
are refused — offsetting inner loops has to go the opposite way — and the guard
counts wires on the face already built rather than rebuilding every wire to ask how
many there are, which is also the more honest test: what matters is the profile
being extruded.
Tests: six new [CadDocument][sketchwire] cases, proved by VOLUME rather than by not
throwing — plate-with-hole, two holes, and two regression guards that a lone circle
and a lone polygon extrude exactly as before. Suite 177 cases / 2366 assertions.
No serialized field, recipe version untouched, golden fixtures unchanged.
snaporca-88v.
snaporca-lukg wants a palette offering all five mate types with the non-viable
ones DIMMED AND EXPLAINED rather than hidden — its reasoning being that a menu
changing shape between invocations destroys the motor memory experts rely on.
That needs an answer this document could not give. This is that answer, and
nothing else: mate_options(cs_a, cs_b) returns five MateOption{kind, viable,
reason}, always five, always in kind order, never filtered.
The geometry test rides on the fingerprint added for snaporca-kqih, which is why
it costs no new serialized field: coordsys_face_kind already records the surface
type. Revolute and Cylindrical need a cylindrical face at both ends because they
need an axis to turn about; Planar needs flat faces; Fastened and Slider
constrain frames rather than surfaces, so no geometry test applies to them.
UNKNOWN IS PERMISSIVE. A fingerprint of -1 means PointWorld or a connector that
has not resolved yet, and it does NOT make a type non-viable. Refusing on missing
information is the false-alarm behaviour that gets a whole feature ignored — the
same reasoning already recorded on kqih for the drift warning, applied again
because it is the same trade.
Reasons name WHICH connector is the problem when only one is. "needs a
cylindrical face at both ends" tells the user what the rule is; "connector A is
on a flat face" tells them where to look, and the second half is the one that
saves the time.
The stability contract has its own test, asserting five entries in kind order
even for a completely invalid pair. That matters more than any individual
verdict: the palette addresses rows by position, so a shorter list would move
every row below it.
Golden fixture unchanged — this is a pure query. Suite 167 -> 171 cases,
2284 -> 2348 assertions, green. snaporca-lukg part A; the palette is part B.
kqih option (c). A FaceAndDirection connector stores a global face index, and an
upstream edit can renumber faces so the index silently names a different one. The
DANGLING case already threw; this is the in-range-but-wrong case, which nothing
detected.
Fingerprint the face on first resolve, compare afterwards, and report a mismatch
into mate_conflicts — the channel that already marks the tree row — never as an
error. A drift warning must not abort the recompute, because the alternative
makes a legitimate Draft on a mated face fatal.
WHAT THE FINGERPRINT IS, AND WHAT IT IS NOT. Surface type plus edge count. Not
centroid or area: legitimate parametric edits move and resize faces, which is the
entire point of the model, so either would fire on every dimension change. Not
the normal, which is the tempting one — Draft deliberately tilts a face and
Transform reorients a body, both legitimate. Type and edge count survive rigid
motion, tilting and resizing, and catch the case that actually happens: a planar
index sliding onto a fillet's cylindrical face after a dress-up inserts faces.
The accepted cost is that a slide between two planar 4-edge faces is invisible. A
partial detector that never cries wolf beats a total one that does, because a
false alarm on a valid connector teaches people to ignore the warning.
Connectors with no fingerprint record one on first recompute, so old recipes
self-heal and both writers (DesignPanel, McpControl) get it without changing.
THE VERSION BUMP IS THE IMPORTANT HALF. The task was specified with "do not
change the recipe version" — that was wrong, and the rule is written in the
header three lines above the constant: bump whenever save/load gains a field.
deserialize_recipe() gates on v == VERSION and then reads a FLAT symmetric field
list. A v3 blob under a v3 build that has grown two fields passes the gate and
reads two ints past the end of every connector, into the next feature's bytes.
That is silent corruption of a saved project, which is worse than any load error.
Now v4, and v3 gets the existing clean refusal.
cad_recipe_v3.bin is KEPT, unregenerated, with a test asserting it is refused and
that nothing half-read is left behind. It is the only artefact that can prove the
gate works, because it was written by an older build — regenerating it with
today's code would destroy the evidence, which the test says in as many words.
Suite 163 -> 167 cases, 2248 -> 2277 assertions, green. Fixture v4 34928 bytes.
snaporca-kqih.
Two tests that separate a hypothesis nobody had tested. The socket showed four
chamfers on a filleted rim removing 29.6 / 20.0 / 10.3 / 7.5 mm3, falling
steadily. That could be the chamfer maths degenerating on a filleted rim, or it
could be how the driver captured its edge ids. Those have completely different
fixes, so the first job was to find out which.
dressup_edge is a global index into TopExp::MapShapes(shape, TopAbs_EDGE),
resolved against the body AS IT STANDS at that feature's position, and every
dress-up rewrites that map. So the two usage patterns are:
ids re-read after each chamfer: 0.400, 0.397, 0.397, 0.395 mm3 (max/min 1.01)
four ids captured up-front: 0.400, 0.008, 0.397, 0.280 mm3 (max/min ~48)
The kernel chamfers uniformly when handed a fresh id. It degrades only when
handed ids snapshot against an earlier shape — and the second chamfer's stale id
landed on a nearly-consumed edge and cut two percent of what was asked. That is
the accumulating-drift signature the socket showed.
Conclusion: driver artefact. apply_chamfer and OCCT are not at fault.
The part that makes this worth a test rather than a note: IT DOES NOT THROW.
ok=1, error empty. A stale id still resolves to a valid edge — just the wrong
one — so nothing anywhere reports it. Silent wrong geometry, which is the class
this project does not tolerate, reachable by any caller that reads the scene once
and then issues several dress-ups.
Test 2 asserts the non-uniformity as CURRENT BEHAVIOUR and says so in the code:
it documents a defect, it does not bless one. When the driver contract is fixed
it should be rewritten, not deleted.
Suite 161 -> 163 cases, 2217 -> 2248 assertions, green. Tests only, no
production code. snaporca-rgbj.
Driving the control socket: hexagon prism, six vertical fillets, four chamfers
on the already-filleted rim, an M8 hole. Afterwards describe_scene reported
bodies=3 and error='' — entirely healthy — while body 2's TopoDS_Shape was null.
Only mass_properties on that one body revealed anything was wrong.
So a feature destroyed a body, recompute() returned true, and the document went
on advertising it. Any downstream consumer — slicing, STEP export, a mass
properties report — met a null shape with no warning. That is the silent
corruption class, which is the one class this project does not tolerate.
recompute() now scans the freshly built bodies for a null shape, names the body
and the feature that destroyed it, and returns false. Returning false rather than
just setting error is the point: it hands the caller its normal rollback path, so
the operation that destroyed the body is undone instead of committed.
The message says "an unidentified feature" when source_feature is -1. "feature 0"
would be a lie, and a message that exists to tell you where to look has to be
trusted.
TEST IS A POSITIVE CONTRACT, AND THE REASON MATTERS. The reported order was
driven headlessly first, as the better test: it does NOT reproduce. The dress-up
step throws "fillet radius too large", which is an already-loud already-caught
path, so recompute fails honestly and never nulls a body. No public-API sequence
found so far reaches the guard's branch without a GUI, and faking a null into
`bodies` after the fact would not exercise it — the guard runs on `built`, before
the swap. So the test asserts what can be asserted: a box + fillet recomputes
true, error is empty, and no body is null. The guard's own branch is defensive
and currently unexercised; that is stated here rather than implied by a green
suite.
Kernel suite: 2217 assertions in 161 test cases, all passing. No existing test
relied on a null body surviving a recompute, so hardening this broke nothing.
snaporca-5425 (part a). Part b — why the chamfer chain degenerates on an
already-filleted rim — is untouched and stays open.
tests/libslic3r/CMakeLists.txt added only test_caddocument.cpp under
SLIC3R_CAD. The other five shipped in the tree and were never compiled, so
49 TEST_CASE blocks looked like coverage and were not: sketch constraints,
sketch editing, sketch import, inference, and the libslvs constraint set.
They also still targeted Catch2 v2 — mainline is on v3, where the umbrella
header is catch2/catch_all.hpp and Approx lives in the Catch namespace rather
than at global scope. Both fixed; nothing else in the files changed.
Found by building the tree rather than reading it. Suite goes from 374 to 423
test cases, 54,424 to 54,620 assertions, all passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two enabled mates driving the same body is not an error today — the later one
just wins, and the earlier mate looks ignored with nothing said. A cycle in the
mate graph is worse: composition still produces a result, but an arbitrary,
order-dependent one.
recompute() now fills a mate_conflicts vector of (feature index, reason) before
the geometry pass, so it survives a throw further down. It catches a second mate
on the same target body, a mate positioning a body against itself, and a cycle,
via an iterative three-colour DFS over the body graph. Broken mates are skipped
silently — apply_mate() already errors on those.
Deliberately non-fatal: recompute() still returns true and error stays empty.
Deliberately not "over-constraint" — that word promises DOF analysis from a
solver this kernel does not have.
Port of snaporca ec4ffeb979. Kernel half of snaporca-bioq.
Suite: 2213 assertions / 160 cases green on this fork too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mate_cs_a/mate_cs_b are feature indices. remove_feature() remapped sketch_ref
through the deletion but not the mate connectors, and move_feature() swapped
sketch_ref but not the mate connectors. Deleting or reordering any feature
ahead of a connector slid both references onto whatever features landed on
those slots.
Nothing reported it. recompute() only rejects out-of-range and non-CoordSys
targets, and a shifted index normally lands on the assembly's other CoordSys —
an assembly carries at least two by construction. So the mate resolved against
the wrong frames and moved the wrong body, silently.
Extracted a remap lambda in remove_feature() and a swap_ref lambda in
move_feature(), applied to sketch_ref and both mate connectors.
Two tests, both confirmed red before the fix. [mate] tags green here:
395 assertions / 29 cases — the first end-to-end kernel compile of this fork.
Ported from snaporca; CadDocument.cpp is byte-identical across forks again.
Refs: snaporca-kqih
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to #14785. Routes the tests that still hand-rolled temp paths through
the shared helpers and unifies the temp guards.
- Add ScopedTemporaryDir and a shared ScopedTemporaryPath base under it and
ScopedTemporaryFile.
- Move test_3mf's round-trip .3mf output out of the TEST_DATA_DIR source tree
(a fixed-name leak) and test_toolordering's fixed-name temp .gcode (a sharding
collision) onto ScopedTemporaryFile.
- Move test_config, test_slicing_pipeline_bindings, the test_3mf backup dirs, and
test_preset_bundle_loading onto the guards.
- Make slic3rutils ScopedDataDir compose ScopedTemporaryDir; dedupe
test_network_versions' fixture and delete test_plugin_lifecycle's duplicate.
## Problem
`Toolchange temperature commands are unchanged when the wipe tower wait
is off`
(added in #15144) fails on both Linux runners and passes on Windows and
macOS.
It is the only failing test in the suite, and it has been failing on
main since
that PR merged.
| Job | Result |
| --- | --- |
| Windows x64 / Unit Tests | pass |
| Windows arm64 / Unit Tests | pass |
| macOS arm64 / Unit Tests | pass |
| Linux x86_64 / Unit Tests | **fail** |
| Linux aarch64 / Unit Tests | **fail** |
From the merge commit
([Linux
x86_64](https://github.com/OrcaSlicer/OrcaSlicer/actions/runs/31072382258/job/92531704095),
[Linux
aarch64](https://github.com/OrcaSlicer/OrcaSlicer/actions/runs/31072382258/job/92531704075)),
still reproducing on current main:
```
first difference at trace entry 29
main: M104 S240 T0 ; preheat T0 time: 31s lead 30.9s
branch: M104 S240 T0 ; preheat T0 time: 30s lead 30.3s
```
## Cause
Each preheat entry records the same quantity twice: `lead` at one
decimal, and
`time:` inside the command text as that value rounded to a whole second.
`split_lead` already compares `lead` with a 0.5s tolerance and explains
why the
estimate moves. `time:` sits in the exactly-compared command text, so it
never
got that tolerance — and being rounded, it flips on a drift far below
0.5s
(30.4 and 30.6 render as `30s` and `31s`). Entry 29 is the only entry in
the
163-entry golden whose lead rounds up; every other preheat sits at
30.0–30.4 and
rounds down, which is why it is the only one that fails.
The variation is per-toolchain, not run to run. Both Linux arches
produce
exactly `lead 30.3s`; Windows x64/arm64 and macOS arm64 all produce
exactly
`30.9s`. Repeated local runs are byte-identical. macOS arm64 passing
while Linux
aarch64 fails rules out the ISA — it is floating-point accumulation over
a few
thousand move durations under GCC vs Clang vs MSVC.
The mechanism makes it discrete rather than gradual: the backtrace parks
the
preheat at the first exported line at least `preheat_time` before the
tool
change, so `lead` is `preheat_time` plus the leftover of whichever move
that
landed on. A sub-tenth difference selects the neighbouring move and
`lead` steps
by that move's whole duration.
Entries 1–28 match exactly, including five earlier preheats whose leads
fall
inside the existing tolerance, so the toolpaths themselves are
identical. I also
reverted the two prime-tower commits that landed between the golden's
capture
point and now, rebuilt, and got a byte-identical trace — this is not
behavioural
drift.
That also rules out regenerating the golden: no single capture satisfies
all
three toolchains, and recapturing on Linux would turn the three
currently-green
runners red.
## Fix
Test-only.
- `lead` keeps a tolerance, widened to 1.5s (measured drift 0.6s; a
preheat
actually leaving its backtrace position would move by tens of seconds).
- `time:` is **not** compared across runs at all. Being a rounding of
`lead`, it
carries nothing the tolerance does not already cover, and comparing it
across
runs can only reproduce the flake. It is instead checked against its own
entry's `lead` — a correct rounding keeps `|time - lead| <= 0.5`.
That second point matters: simply tolerating `time:` numerically would
have made
the test blind to a real change, because drift and a wrong rounding both
move it
by 1. The self-consistency check keeps that coverage. I verified it by
changing
`(int) std::round(time_diffs[0])` to `(int) time_diffs[0]` in
`GCodeProcessor::export_lines` — the test fails with
`"time:" is not its entry's "lead" rounded to a whole second`, where a
plain
tolerance would have passed silently.
Everything else is still compared exactly: all M104/M109 values, tool
ids,
block markers, ordering, entry count, and the annotation text including
its
trailing `s`. The other 138 entries remain byte-exact.
No production code, no golden regeneration. The golden file and these
helpers
are used by this one test and nothing else, and the tolerance only
widens, so
Windows and macOS keep passing unchanged. A note is added to the
golden's header
so the next mismatch in those fields is not "fixed" by recapturing.
## How to verify
Before, on Linux:
```bash
git checkout main && ./build_linux.sh -t
ctest --test-dir build/tests -R "Toolchange temperature commands are unchanged" --output-on-failure
# fails at trace entry 29
```
After:
```bash
cmake --build build --config Release --target fff_print_tests
ctest --test-dir build/tests --output-on-failure # 463/463
```
The placement clamps and the tower-approach router both stood in the bed's
bounding box for the bed itself, so on a delta or hexagonal bed the prime tower
could be parked in a corner that does not exist and the nozzle could be routed
across it. Both now test the real printable outline, slicing reports a tower
that does not fit instead of printing it off the bed, and a tower parked near an
edge is routed along the clamped side rather than falling back to a straight
line across the tower.
Also fixes the placement validation rotating the tower hull by degrees read as
radians about the plate origin, and never rotating the generated tower footprint
at all.
Adds a printer option that picks up the new tool without a blocking temperature
wait, travels to the wipe tower, and waits there right before purging, parked
beside the tower so the ooze from the heat-up lands next to it rather than on the
model. The incoming filament's target is raised ahead of the tool change, so the
heat-up overlaps both the change itself and the travel to the tower.
Off by default, and only offered for multi-extruder printers using a Type 2 wipe
tower; the generic toolchanger profile enables it.
The wipe tower's "Delay after unloading" never happened on Klipper. It was
emitted as G4 S<seconds>, and Klipper's G4 reads only the P parameter, in
milliseconds, so the pause was silently skipped. The option now produces a
dwell Klipper actually performs.
Also corrects the planner flush rationale, which cited an extruder position
reset that Klipper resolves at parse time and does not need synchronized, and
adds end-to-end coverage that slices a two-filament print and checks the
emitted wipe tower G-code on both a Klipper and a non-Klipper flavor.
No change to any other firmware flavor's output, and no shipped profile sets a
non-zero delay, so no shipped profile's output moves either.
The wipe tower emitted G4 S0 to make the firmware finish its queued moves
before commands that must not take effect early. Klipper's G4 reads only the
P parameter, so that flush never happened there and a temperature change could
land seconds ahead of the moves it was meant to follow. Klipper now gets M400
instead, through one helper shared by both wipe tower implementations.
No change to any other firmware flavor's output, so no shipped profile or saved
project is affected.
test(libslic3r): replace the disabled convex_hull_2d test, closing #11269
The last "failing libslic3r test" from #11269 was the disabled
SCENARIO("2D convex hull of sinking object", "[3mf][.]") in test_3mf.cpp.
It checked ModelObject::convex_hull_2d for a sinking object against
PrusaSlicer's reference hull, but Orca's convex_hull_2d does not clip
geometry below the bed the way PrusaSlicer's its_convex_hull_2d_above does,
so the reference never matched. The test also wrote a debug mesh to a
hardcoded /tmp path and its comparison loop was inverted.
Remove it and add tests/libslic3r/test_model.cpp characterizing
convex_hull_2d on non-sinking transforms (identity and scale+offset),
where the projected footprint is unambiguous. Homed in a Model test file
since it exercises ModelObject, not 3MF.
* Sync WipeTower from BambuStudio(through ca1881761)
* Fix post-slice self-invalidation on custom multi-extruder printers
* Complete the rib wipe tower port in WipeTower2
The rib tower is now always square (prime_tower_width is ignored, as the
GUI already implies), carries the rib origin offset like the BBL tower so
the rib tips sit inside the configured position, clamps the rib length to
the tower diagonal, and extends the ribs for short towers.
* Use the squared rib tower size in arrange estimates
estimate_wipe_tower_polygon reserved the arrange footprint and clamped the
tower X position with the raw prime_tower_width, under-reserving space
whenever the rib wall squares the tower to a different width.
* Print the WipeTower2 shell with a non-support, non-soluble filament
Like the BBL tower: the layer's sparse infill, wall, and brim go to the
first toolchange to a non-support/non-soluble filament, or are printed
with the incoming filament before any toolchange. The minimal-purge
clamp now also covers toolchanges that get no finish-layer saving.
Output is unchanged when no support/soluble filament is used.
* Port the skip-points gap wall to WipeTower2
prime_tower_skip_points was stubbed for Type2 towers: the wall call
hard-coded skip_points=false, the gap cutter received an empty vector,
and append_tcr2 never routed the entry travel. Now the toolchange entry
positions are precomputed from the finalized plan, the wall is cut open
at each entry, and the entry travel approaches around the tower bounding
box through the opening when it starts outside the tower. The geometry
helpers are re-synced with the BBL versions (add_extra_point guards,
per-point side selection). The cone wall keeps its separate path, where
the option stays inert.
Behavior change: non-BBL towers now honor the (default-on) checkbox with
gap walls and routed entries; with the option off the output is
unchanged, and the BBL tower path is untouched.
* Route the in-place toolchange tower entry through the skip-point gap
On multi-tool printers without ramming the tool changes away from the
tower and the entry travel is the tcr's own positioning move, which went
straight across the printed wall. Append the avoid-perimeter path to the
change-filament gcode instead, so the head approaches around the tower
and enters through the wall opening (append_tcr parity).
* Iron the purge start out through the skip-point gap in WipeTower2
Port the BBL tower's entry line ironing: extrude the first 3 mm of the
purge, retract, drag the nozzle 1.5x back out through the wall gap at
F600, creep back at F240 and unretract, so the toolchange start blob
ends up in the gap instead of on the wall. Fires only when the purge
starts at the left-edge entry heading right (in-place toolchangers);
SEMM ram/cooling wipes start mid-box and the priming line has no wall,
so both keep their previous output.
* Reserve WipeTower2 toolchange depth to match the printed purge
The planner reserved ramming rows gated only on enable_filament_ramming and
sized them with the SEMM 0.25s time step, while toolchange_Unload rams on
(semm && enable_filament_ramming) || filament_multitool_ramming with the
multitool time step. Disabling multitool ramming therefore left ~3 unprinted
rows per toolchange as blank bands in the tower. Without ramming the first
wipe line also needs reserved depth of its own (it no longer rides the last
ramming row), plus the y_step/2 offset the wipe start inherits from the
ramming start position - otherwise the tightened boxes truncate the ordered
purge at the box edge.
* Tile WipeTower2 purge rows contiguously across toolchange blocks
Without ramming, each purge block reserved one wipe pitch more than its
rows occupy (ceil+1 rounding plus the ram-geometry start offset), and the
wipe began a full pitch inside the block, leaving a blank band of exactly
two pitches between adjacent blocks. Plan the block as whole wipe rows,
start the first row so the row lattice continues across the block
boundary, and fill the reserved box instead of stopping at the ordered
volume, mirroring how the BBL WipeTower keeps planned depth identical to
printed rows. Ram-printing toolchanges (SEMM with ramming enabled,
multitool ramming) are unchanged.
* Scrub the WipeTower2 toolchange entry with the BBL flat-ironing spiral
The entry scrub now matches the BBL tower's toolchange_wipe_new sequence:
after the ironing drag the retracted nozzle runs a dry expanding-square
spiral centred on the wall-gap entry point before resuming the purge row.
The spiral runs whenever the gap wall is on (disable per filament via
filament_tower_ironing_area = 0); WipeTower2 no longer reads
prime_tower_flat_ironing.
* Restart the WipeTower2 wipe at the box boundary after multitool ramming
With the gap wall on a multi-tool printer, quantize the ram band up to its
whole reserved rows (as the BBL tower does for the old-tool purge) and start
CP TOOLCHANGE WIPE at the left-edge boundary on a fresh row below it instead
of continuing from wherever the ram serpentine ended. The entry scrub then
runs at the wall gap on ram toolchanges too, and the wipe box is whole rows,
so it is filled completely like the no-ram case. SEMM and skip-points-off
behavior is unchanged.
* Move the WipeTower2 wall gap to the wipe start row for ram toolchanges
* code cleanup
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix typo
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Selecting a face and sketching on it is the most common gesture in solid
modelling, and it was impossible. The plane came from a combo holding
XY/XZ/YZ plus datums, and plane_from_choice had no face branch at all, so
the only route onto a face was to build a Coincident datum plane on it
first, confirm that, reopen the sketch and find the datum in the
dropdown. Three extra steps and a junk feature in the tree.
The fix is not another combo row. A new sketch now takes its plane from
what is SELECTED IN THE VIEWPORT: a picked planar face wins outright, and
only when nothing is picked does it fall back to the reference plane —
which is itself normally set by clicking one of the ghost planes in 3D,
not by opening the combo. The tool hint names the target ("Circle — click
center, then radius · on the picked face") so the choice is visible on the
geometry side rather than needing a control to read back.
CadDocument::plane_of_face is the shared derivation, so the sketch path
and the Coincident datum method cannot drift apart. It refuses
non-planar faces: face_normal_world evaluates at the mid parameter, which
on a cylinder or a fillet is a tangent plane at one arbitrary point —
fine for offsetting a datum, wrong as a sketch plane, and silently
sketching on a tangent is worse than declining.
Picking the face also CONSUMES it. Leaving the pick live meant the next
Extrude saw a selected face and push/pulled it instead of extruding the
sketch just drawn — the same trap the imported-art path already guards
against.
Verified on :10 end to end with no combo interaction: build a box, click
its top face twice to cycle whole -> face, press S then C, and the circle
is drawn in the plane of that face with its Radius tab on the geometry
(artifacts/shots/f3a2-03-face.png, f3a2-04-sketch-on-face.png,
f3a2-05-circle-drawn.png). Kernel side: 154 cases / 2125 assertions green
on both forks, including that a cylinder resolves exactly its two flat
caps and refuses the barrel.
Still side-panel-shaped and to be dealt with separately: the Plane combo
remains on the card and now merely displays a stale row when a face is
the real target. It should show the actual target or go away.
snaporca-3a2.
entities_to_wire handles exactly two shapes: one lone closed entity
(Circle/Ellipse), or a chain of open ones (Line/Arc/EllipseArc/BSpline).
Anything else -- a circle coexisting with a line, two circles -- returns a
null wire. build_sketch_wire answered that by falling through to its
legacy tail, which ends in a rectangle built from width/height. For an
entity sketch those fields are whatever they were initialised to, so the
extrude produced a box the user never drew, silently and with ok:true.
That is how the ellipse+stray-arc case in the P2 Tier-B.1 verification
turned into a default-rectangle solid.
Throw there instead. The legacy profile/shape paths below are still
reached by sketches that legitimately carry no entities at all, so the
enum and profile constructors are untouched -- only the case where
entities exist and cannot be turned into a wire now fails, which is
exactly the case that was fabricating geometry.
This does NOT implement the multi-loop support the issue asks for. Doing
that properly means deciding containment -- a circle inside a rectangle is
a hole, a circle beside it is a second region -- and make_extrude_regions
cannot be reused because it takes flattened Vec2d contours for imported
Text/SVG art and would discard the analytic circle. Guessing containment
would trade a visible failure for a wrong solid, which is the opposite of
the point. Left scoped on snaporca-88v.
Also converts the three float comparisons in the two test cases added this
session from Approx to WithinAbs/WithinRel, per tests/CLAUDE.md, which
rules Approx out for being asymmetric and double-only. The rest of the
file's pre-existing Approx uses are left alone.
153 cases / 2090 assertions green on both forks; no existing test depended
on the default-rectangle fallback.
snaporca-88v (partial: the silent-fallback half).
A boolean cut whose tool misses the target is a perfectly legal
operation: OCCT reports IsDone(), the shape comes back unchanged, and the
feature lands in the recipe reporting ok:true. Driving the MCP socket,
that produced two consecutive {ok: true, bodies: 1, error: ''} responses
for a hole that was never drilled -- same viewport, same 46939.11 mm3 --
and the tree grew two Hole features that will never cut anything. A
caller, an agent especially, has no signal at all that the thing it asked
for did not happen.
Measure the volume across the op in route_feature's in-place branch and
refuse the no-op. Only for removals: Hole, Thread, and Extrude / Revolve
/ Sweep / Loft in Cut mode. Everything else may legitimately leave the
volume alone -- a Transform certainly does. The tolerance is relative,
because an absolute epsilon is wrong across the mm-to-metre range of real
parts, and a cut that shaves a numerically invisible sliver is a miss
too. The existing rollback in the MCP actions already preserves the
reason, so a missed hole now answers ok:false with the error and undoes
the feature.
The confusion underneath was not itself a bug: hole's x/y are in the
sketch plane's frame, whose origin is describe_scene's modeling_origin,
and describe_tools documented them only as "number, unit mm". Passing the
world centre put the hole 135 mm clear of the solid. All six x/y params
on hole / hole_styled / hole_standard now say which frame they are in,
since the wrong guess was silent.
Regression test drives the reported failure directly: a hole at x=135 on
a 20x20x20 box is rejected and leaves the body untouched, the same hole
at the origin still removes exactly pi*4^2*20, and a cut-mode extrude
whose profile sits at x=200 is rejected too. 152 cases / 2083 assertions
green on both forks.
snaporca-daf.
Two P0s in the same gesture. Filleting a corner of a parametric rectangle
produced either nothing at all or a sharp corner with a stray arc floating
above it.
Trigger (snaporca-cq2): the only routes that ever reached confirm_op were
finishing the whole sketch and an unsignposted click on empty space.
set_tool() dropped a ready op, so typing a radius or dragging the arrow and
then touching any other tool threw the value away. Commit a ready op on tool
change (before m_mode is reassigned — op_ready() and confirm_op() both switch
on it), commit on Enter in the radius editor, and drop the pending op before
Esc's tool downgrade so Esc still cancels rather than applies.
Substitution (snaporca-pl5): libslvs writes its last Newton iterate into the
params whether or not it converged, and SketchSolver read them back
unconditionally, so every REJECTED solve deformed the sketch. The fillet
ladder tries a deliberately over-constrained rung first (a tangent on each
leg, against the legs' own H/V); it is correctly rejected, but its wreckage
then failed rungs 2 and 3, which solve cleanly on their own. The arc ended up
with no constraints at all, the rigid loop won, and the corner snapped shut.
Measured: from pristine geometry rung 1 gives result=INCONSISTENT with 3 bad
constraints, rung 2 gives dof=6 with the arc's radius intact.
Read the geometry back only on success. try_add_constraints then needs no
"restore" re-solve — the entities still hold the prior solved state.
Kernel suite 151 cases / 2072 assertions green on both forks; the GUI check
ran on the Snapmaker fork (9d72c4377a).
Ported from the Snapmaker fork. snaporca-pl5 snaporca-cq2
A filleted solid arrived on the plate as a broken model: the slicer reported
"8 non-manifold edges" on an 80x50x12 box with r=3 on all edges, and advised
repairing it in another CAD application -- the exact round trip the Design tab
exists to remove. The same box without the fillet committed cleanly.
Measured rather than guessed. Each of the 8 bad edges is degenerate, both
endpoints the same vertex:
open tri=145 edge=1 face=5 v59(3.000000 3.000000 0.000000) v59(3.000000 3.000000 0.000000)
open tri=538 edge=1 face=6 v87(3.000000 3.000000 12.000000) v87(...)
... one per corner, 8 corners
OCCT triangulates a degenerate surface parameterization with a triangle at the
pole; a corner sphere patch has exactly one. Its two pole nodes are distinct in
the per-face triangulation and collapse to a single vertex when the faces are
welded, leaving a zero-area triangle whose v->v edge can never pair with a
neighbour. its_face_neighbors counts it as open, and the field the object panel
prints as "non-manifold edges" is in fact stats.open_edges.
So the geometry was never wrong -- the B-rep volume matches the Steiner formula
for a box dilated by a ball to 0.016%. Only the bookkeeping was.
Dropping those triangles after the weld removes 8 of 3492 and takes open_edges
to 0. Zero area, so nothing about the shape changes. tri_face is compacted in
the same pass, since it must stay index-aligned with the triangle list that the
face picking and per-body colouring both index into.
Guarded by a new [CadDocument] case that asserts open_edges == 0, no degenerate
triangle survives, and both per-triangle maps still match the triangle count.
The existing suite only ever checked B-rep volumes and areas, which is why a
mesh defect this visible went unnoticed: 150 cases / 2049 assertions green on
both forks.
snaporca-agw
store_bbs_3mf reaches Model::get_backup_path(), which builds
temporary_dir() + "/orcaslicer_model/" + timestamp. temporary_dir() returns a file-static
that ONLY OrcaSlicer.cpp's startup sets, so in a test binary it is the empty string and the
backup path becomes "/orcaslicer_model/..." — absolute, at the filesystem root. An
unprivileged process cannot create that, so the save returned false and the scenario died
on REQUIRE(store_bbs_3mf(sp)).
This was the SINGLE failure in this fork's Unit Tests — 1 of 566, on Linux x86_64, Linux
aarch64 and macOS arm64 — from CI run 30191490709:
Failed to create backup path "/orcaslicer_model/Sun_Jul_26/08_49_41#5398#1":
boost::filesystem::create_directories: Permission denied [system:13]
It hid because that job had never run to completion on this branch before: every earlier
run was cancelled by the concurrency group first. It also passed on Windows x64, where the
drive-root path is writable, and it passes in the local build container, which runs as root.
Verified against the same defect in the Snapmaker fork by running the built binary as
uid 1000: permission denied before, 4 assertions passing after.
snaporca-vg8
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM
snaporca-tkz, the last quarantined test. Root cause read out of the vendored
source rather than guessed: slvs/constrainteq.cpp, Type::ARC_LINE_TANGENT does
ExprVector ap = SK.GetEntity(arc->point[other ? 2 : 1])->PointGetExprs();
so it dereferences the ARC'S ENDPOINTS. A full circle entity carries only
point[0], its centre. point[1] and point[2] are zero handles, FindById throws
"Cannot find handle", and the process ABORTS rather than failing the solve —
taking every later test in the binary with it. That is also the wrong equation
for a circle regardless: it only makes the line perpendicular to the radius at
an endpoint that does not exist.
CT::Tangent no longer hands a full circle to that constraint. For a circle it
emits PT_LINE_DISTANCE(centre, line) = radius, which is precisely what tangency
to a circle means. Arcs keep the ARC_LINE_TANGENT path they are built for.
One limitation, stated rather than buried: the slvs C API takes a constant
distance and offers no way to reference the circle's radius parameter, so the
radius is captured when the constraint is emitted. That is exact whenever the
radius is fixed or is simply not driven by another constraint in the same
solve, and re-solving restores tangency if something else moves it. Tying them
would need an auxiliary point constrained onto both the circle and the line.
With this and eeca6794e7, both quarantined tests are gone and the exclusion in
kernel-test.sh goes with them. A green run now means the whole CAD suite
passed, not "everything except the two we gave up on":
149 cases / 2043 assertions, no filters.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
snaporca-kzy was filed as "internal thread cuts too little material". It does
not. Measured on the test's own fixture, a 40x40x20 box:
plain Ø12 bore removes 2261 mm3
internal thread removes 2157 = 1571 (minor bore) + 586 (groove)
apply_thread bores at the MINOR radius (radius - depth = 5) and then carves the
groove out to radius + depth = 7. A tapped hole therefore keeps the crests
between turns and holds MORE material than a plain clearance hole at the
nominal radius — which is what every real tapped hole does. The test asserted
the opposite, so it was asking for something physically wrong and had been
quarantined for it since it was written.
One hypothesis discarded on the way: that the shortfall was a tessellation
artefact, since chords on a helical surface undercut a concave bore. Exact
BRepGProp::VolumeProperties agreed with the tessellated volume to within
2.5 mm3, so that was not it and is not offered as a hedge.
The reference is now the tap-drill bore the thread actually starts from (Ø10),
against which the groove's 586 mm3 is the meaningful quantity — that is what
"the thread cuts" means. Test re-tagged [CadDocument][thread], so CI covers the
thread path again instead of skipping it.
Also documented the (void)internal in make_thread_profile. It reads like a bug
and is not: the V is the same shape either way and the caller decides, fusing
it onto a shaft or cutting it out of a wall. Someone "fixing" it to point
inward for the internal case would make the groove sweep already-empty bore
space and cut nothing — the exact failure the old comment described.
Suite 148 cases / 2035 assertions, with this test now among them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by sweeping the index-space defect class deliberately rather than by
hitting it: that class produced 4 of the 8 defects found by hand yesterday, so
it was worth auditing every combo in the panel that maps a row selection onto
a document index.
Most of it came back clean — the loft sidecar vectors are consistent at all
four read sites, the sheet-body pickers go through the helper everywhere, mate
connectors carry client data. Six did not. A stored target_body indexes the
body list AS IT WAS just before that feature ran during replay, but Transform,
Mirror, Thicken, Rib, Project and DeleteFace all populated their combo from
the live m_doc.bodies. Boolean and Cut already replayed to the right slot.
The failure is concrete: model a body, Thicken it, then Cut something later in
the tree. A Cut replaces one body with two, so every index at or after it
shifts. Reopen the Thicken and the combo lists the post-cut bodies while
selecting the pre-cut index — showing, and on confirm re-targeting, a
different body than the feature actually used. A Boolean that consumes its
tool body shifts them the other way for the same result.
fill_body_choice() does the truncated replay populate_body_choices() already
did, for the single-combo tools. Six call sites, and 60 lines of duplicated
population loops go with them.
Visible change when testing: re-editing an early feature now lists FEWER
bodies, because it lists only those that existed then. That is correct — you
cannot target a body that did not exist yet — and it is what Boolean and Cut
have always done.
The new kernel test pins the invariant the GUI now leans on: a Cut turns one
body into two, and replaying to just before it yields the earlier, shorter
list. If body ordering after a split ever changes, that assumption fails
loudly here instead of silently in a dialog.
NOT click-tested — GUI wiring, compile-verified only. Filed as snaporca-oz7
and added to snaporca-cfi.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
datum_frame took a FaceAndDirection frame's Z from the face normal, which
follows the body, but its X from coordsys_x_hint, a world constant, whenever
no explicit edge reference was set. Spinning a body about its own face normal
therefore left the frame bit-identical: the connector could not encode that
rotation at all, so Fastened and Slider mates claimed to fix an orientation
the frame could not see.
X now comes from the face's own first usable edge, which rotates with the
body. The hint survives only as a last resort, for faces that offer no
in-plane direction — a full circular edge has coincident endpoints, and a
seam projects to nothing in-plane.
The new test spins a box 90 degrees about its top-face normal and asserts the
frame's X turned with it. Reverting just the X_tent derivation and rerunning
makes it fail with "1.0 is within 0.000001 of 0.0" — cos(angle) between the
before and after X is exactly 1, i.e. the frame did not move — and that is the
only failure in 2019 assertions, so the test discriminates this defect and
nothing else.
Note for anyone replaying an older document: a face-only connector's frame
can now differ from what that recipe produced before, so a mate built on one
may place its body differently. Nothing in the suite or the golden v3 fixture
changed, but the semantics did.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This fork's Unit Tests job failed on every single commit, because CI runs the
whole ctest suite including the two cases tagged [known-broken] that
scripts/kernel-test.sh has always excluded locally. A job that is red
unconditionally is worse than no job: it trains everyone to ignore it, so the
next genuine regression arrives invisible.
No workflow change was needed. scripts/run_unit_tests.sh already passes
-LE NotWorking, and tests/CMakeLists.txt registers Catch2 tags as ctest labels
via catch_discover_tests(ADD_TAGS_AS_LABELS) — so the exclusion upstream
already ships works as soon as the cases carry the tag. Verified against the
built test tree: 337 tests unfiltered, 335 with -LE NotWorking, i.e. exactly
these two dropped and nothing else.
The second cause recorded in the issue, the test-reporter step failing with
"Resource not accessible by integration: 403" on a fork, is already fixed
upstream: the Publish Test Results step now carries continue-on-error: true.
The comment these cases carried claimed CI kept the bugs visible by reporting
them forever. That is now false and was never a good mechanism anyway, so
visibility moves to the tracker: snaporca-tkz for the solver SIGABRT, and
snaporca-kzy, filed now, for the thread groove volume. Neither is fixed;
neither is forgotten.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rib guarded with `sk.type != CadFeatureType::Sketch`, while every other
sketch consumer — Extrude, SurfaceExtrude, SurfaceRevolve, the loft paths —
tests `!= Sketch && != Project`. A Project feature carries a plane and Line
entities, which is all a rib reads, so the guard blocked "project a body
edge, then rib along it" for no stated reason.
The picker in the Design tab offered Sketch features only, so it is widened
to match: a kernel that accepts Project refs and a GUI that never lists them
would have left the path unreachable anyway.
Worth recording for whoever hits this next: Rib also needs a sketch carrying
EXPLICIT entities. A parametric Rectangle sketch (add_sketch with
width/height) has an empty entities vector — build_sketch_wire synthesises
its profile on demand — so rib_entity 0 is out of range there and it fails
with "rib: bad entity". That is why Rib could not be driven headlessly at
all before this change; a Project feature is now the one programmatic way to
produce a ribbable line.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects found while driving the tools that Phase B wired but nobody had
exercised yet.
apply_project had no all-edges branch: with no face picked and no explicit
edge list it threw "no edges or face selected". That is precisely the state
the Project card opens in, and its label reads "(all edges)" — so the card's
default could never be confirmed. It now projects every edge of the source
body. Edges perpendicular to the target plane collapse to a point when
projected, so segments whose endpoints coincide are dropped instead of being
emitted as zero-length lines that would poison the sketch downstream.
The second defect is why the first one was invisible. 29 of the 31 rollback
sites in McpControl ran `if (!ok) doc.undo();`, and undo() recomputes the
restored feature list — which succeeds and clears doc.error. Every failing
command therefore reported `error: ""`. Yesterday's fix covered 2 sites and I
treated the file as done; it was not. All 31 now capture the reason before
the rollback and restore it after. Failures that read as `""` now read as
"rib: bad entity" / "surface-revolve: revolve failed".
Verified on the running GUI through the control socket: the Project call that
previously returned ok:false now returns ok:true, and failures carry a reason.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
axis_plane_a/b are filled by the GUI from populate_plane_choices(), whose rows
are XY / XZ / YZ followed by the datum planes, and the row is stored verbatim.
The kernel's base_plane() indexed datum_planes[ref] directly, so the two spaces
were off by three: picking XY resolved to datum plane 0, and picking the first
datum ran past the end and failed with "plane ref not found". The
PlaneIntersection axis type could not work from the GUI at all.
base_plane() now uses the encoding CadFeature::plane_base already uses — 0/1/2
are the base planes through the modeling origin, >=3 indexes datum_planes[ref-3]
— so there is one convention for plane references instead of two. That also
makes two base planes usable, which the previous code rejected as out of scope
even though XY x XZ is an ordinary way to define the X axis.
Removed the dead find_plane lambda directly above it. It was never called and
half-anticipated this exact offset ("if (ref >= 3) // base plane offset"),
which is presumably where the confusion started.
Tests: the existing parallel-planes case encoded the OLD convention, passing
axis_plane_a = 0 to mean "datum 0" — values the GUI cannot produce — so it is
re-based onto rows 3 and 4. Two new cases cover what the GUI actually emits:
base x base (XY x XZ -> X) and base x datum, the latter pinning the +3 offset.
Both verified to FAIL against the previous indexing, at test_caddocument.cpp
:2325 and :2346.
Found by auditing the remaining tools for the index-space defect class that had
already produced three bugs in the GUI; this is the first instance of it
crossing the GUI/kernel boundary.
Suite 143 cases / 1980 assertions (was 141/1972). GUI compiles clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tommaso Bianchi