mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-09-27 19:01:02 +00:00
6b3642fa534ec355ebc3fe744324db1d4bb6ce00
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
6b3642fa53 |
Mate viability: which of the five apply, and why the others do not
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.
|
||
|
|
a3398c6609 |
MCP: let a caller find out its face/edge ids went stale
snaporca-rgbj measured the damage: four chamfers on a box remove 0.400/0.397/0.397/0.395 mm3 when each id is re-read, and 0.400/0.008/0.397/0.280 when the four ids are captured up front. The kernel is right in both runs — the second one asks for the wrong edges. Neither errors, because a stale id still resolves to a real edge, just not the one that was measured. That makes it an API problem rather than a script bug. Reading the scene once and then issuing several operations is the natural way to drive a socket, it is what every agent will write, and it produced silently wrong geometry with nothing anywhere reporting it. CadDocument::topo_generation is bumped where the bodies are replaced — the single line in recompute() where the face and edge maps actually change, so a feature type added later cannot forget to bump it, which a per-mutator counter would invite. describe_scene and query_topology return it. A caller may pass it back as "generation" on any call, and a mismatch is refused with a message that says what to do about it. Two deliberate choices: OPTIONAL, not mandatory. Every existing script keeps working unchanged; passing the generation is what buys the guarantee. Making it required would break every caller to fix a mistake only some of them make. CHECKED AT THE DISPATCHER, not in each handler. One site covers fillet, chamfer, shell, draft, coordsys, thicken, cut, project, delete_face and everything added after them. A per-handler check is a list that goes stale the first time someone adds a method in a hurry. Not serialized: an id means something only within the run that produced it, so persisting the counter would promise a stability the ids themselves do not have. No recipe version change. describe_tools now carries an id_lifetime note, because the guard only helps a caller who knows to ask for it. Kernel suite 167 cases / 2277 assertions green; libslic3r_gui builds. The guard itself is NOT exercised — it needs the socket, so it is on snaporca-bdco. snaporca-o1l2. |
||
|
|
9125e0b4f8 |
Connector face drift: warn without crying wolf — and bump the recipe version
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. |
||
|
|
13c702bed3 |
Chamfer drift is the driver's, not the kernel's — measured, not argued
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. |
||
|
|
488c94e957 |
SurfaceOffset and ThickenSurface: the arrow stands on a face, the tool still takes the sheet
These were the last tools from the charter audit with no handle at all, and the issue filed against them offered three options, all of which changed the tool. Reading on_add_surface_offset() dissolved the question instead. The premise was that a distance handle needs a frame, a sheet body has no single normal, and therefore the tool must start demanding a face. But the face was never needed for the OPERATION — only for the ARROW. Both tools still offset or thicken the entire sheet named in the combo. The picked face only says where to stand the handle. So the arrow appears whenever a face of that sheet is under selection, and its absence costs nothing: the card alone works exactly as before. Purely additive — no existing flow changes, and there is no new precondition for the user to learn. That is strictly better than any of (a) anchor on the first face and be wrong on a curved sheet, (b) sample a normal at the bbox centre and be arbitrary on a folded one, or (c) require a face pick and change what the tool demands. The arrow is refused when the picked face belongs to a DIFFERENT body than the sheet in the combo. An arrow standing on one body while the tool acts on another would name the wrong thing, which is worse than no arrow. ThickenSurface was not on the audit's list — it is a distinct tool from Thicken, with its own card and its own sheet-body combo, and it has exactly the same shape. Fixing one and not the other would have left the same gap under a different name. Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-9fel. |
||
|
|
416e7f7321 |
Mate conflicts: mark the row that carries them, and name the way out
detect_mate_conflicts() has been filling m_doc.mate_conflicts on every recompute
since the kernel half landed, and nothing read it. The diagnostics existed and
were invisible — a conflicting assembly looked exactly like a working one.
The tree row is where they go, because the tree is where the user is already
looking for which feature to change. Three states, in precedence order:
disabled -> dim. A SUPPRESSED mate is the user's answer to a conflict, so it
must read as suppressed rather than keep shouting about it.
conflict -> warn.
otherwise -> normal.
Selecting a marked row puts the reason on the status line — "Mate3 already
positions Body 2", the cycle, the self-mate — and names the eye as the way to
suppress it. A message that describes a problem with no action is a message that
gets ignored; the action here is already one click away on the row just selected.
Deliberately NOT a modal, and deliberately not treated as a document error. The
document still evaluates with a conflict present: the mate graph merely has more
than one answer for a body, and which one wins is the thing the user needs to
see. Blocking the loop to say so would interrupt without helping.
The dimming of non-involved bodies from the original UX proposal is still not
implemented, on purpose: under transform composition a failure mid-chain
propagates, so "not involved" is not a well-defined set, and dimming the wrong
bodies would hide the context needed to understand the conflict.
Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-bioq.
|
||
|
|
7311cb12cb |
Body-focus picking: fail open, and keep the combo and the viewport as one state
Two defects found by auditing the body-focus x-ray path, which shipped compiled but never exercised. Neither is reachable from the happy path its test plan walks, which is why compiling it proved nothing. 1. A STALE FOCUS KILLED THE VIEWPORT. The focus is a body INDEX held by the panel across recomputes, so it outlives the body it names: delete a body and the stored index can point past the end. body_pickable() then rejected EVERY body, because none of them equals an index that no longer exists — a viewport that silently accepts no clicks at all, with nothing on screen saying why. Out of range now means no restriction. Fail open, never dead. 2. THE COMBO AND THE FOCUS COULD DISAGREE. refresh_cs_body_choice() rebuilds the Body combo and, when the body list shrank, silently reset the selection to "(all)" — while the viewport stayed focused on the old index. Every other body kept its 25% alpha and picking stayed restricted to a body that might be gone. That is the exact mirror of the open_tool ordering bug this feature already fixed once: that one showed "Body N" over an opaque scene, this one shows "(all)" over a dimmed one. They are one state and are now written together. Guarded on CoordSys being the active tool, since it is the only card that owns this focus. In the edit path the function runs BEFORE open_tool with the previous tool still active, so the guard is false and the caller's explicit set_xray_focus still wins. Also confirmed while reading, since the header asserts it: set_solid_pick() does NOT touch m_pick_only_body, so the focus really does survive the mesh feed. That claim now has a check behind it rather than a comment. Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-bgvk. |
||
|
|
23585382ab |
Mate connector: a roll mark that survives a grazing view, and a quieter warning
Two open findings from the first rig judgement of the connector glyph. F4 — the quadrant collapses to a blob at grazing angles, which is exactly when the roll is hardest to read. Adds a radial tick along +X extending past the disc rim. As the disc flattens to a line the sector loses all its area, but a radial spoke keeps its length and its direction along the one axis that still projects. The alternative on the issue was to billboard the quadrant while the disc stayed in-plane. Rejected, and not on taste: at true grazing the view direction lies IN the connector's plane, so every in-plane direction projects onto the same screen line and the roll is geometrically unrecoverable. Billboarding would not recover it — it would face the camera and read as a definite orientation that is not the frame's. Degrading to a direction that can still be trusted beats drawing a confident lie. The tick is additive, so unlike billboarding it cannot make the non-grazing case worse; it still wants judging on the rig at a true grazing view before F4 is called closed. F5 — roll-undefined was a loud red: the strongest colour in the viewport spent on the least important connector, pulling the eye off the mate being made. It marks "this one could not be derived", not an error. Muted amber says look-here without shouting. No tick is drawn when the roll is undefined — a tick there would assert a direction that does not exist, which is the silent guess the hatched quadrant exists to avoid. Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-wgsc. |
||
|
|
e6a14b39c9 |
Rib: the thickness gets its handle, so the whole tool is draggable
Rib's depth already reused the Extrude arrow. Its thickness could not: the arrow points along the plane normal, and thickness is an offset either side of the rib line, IN the plane. Different direction, different handle. Two square handles at mid ± perp·half, plus the slab's actual footprint drawn as a thin closed rectangle — the footprint matters more than the dots, because what a rib thickness means is how wide that slab lands on the body, and until now there was no way to see it before committing. A drag on either handle sets the FULL thickness, twice the perpendicular distance from the line, because the slab is centred on the line and the handle sits at half. Both handles behave identically for the same reason, so they share one colour rather than pretending to be two different actions. A zero-length line has no direction to grow a slab perpendicular to, so the shared rib_frame() helper returns false and render and drag both draw nothing rather than dividing by zero. Non-Line entities clear the gizmo instead of guessing: the kernel is line-only and a gizmo that guesses would be lying about what Confirm will build. Unlike the helix callback this one goes through refresh_preview(), because Rib builds a real solid ghost that has to rebuild. The helix has none and skips it deliberately. Both gizmos coexist and resolve the sketch and entity the same way, so the depth arrow and the thickness handles can never disagree about which line they are on. Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-plew. |
||
|
|
b9d6b59f90 |
A feature that destroys a body must say so, not ship a phantom
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. |
||
|
|
e8306a6e9a |
Helix: draw the thing, then let the numbers be dragged
grep -i helix over the viewport code returned nothing at all. The tool was four coupled numbers and a Confirm button — you typed radius, pitch, height and taper blind and pressed OK to find out what you had made. So this is not only the charter's L2 failure; the tool had no visible state whatsoever while it was open. Adds a plane-anchored helix gizmo built on the datum-plane gizmo as its template, being the closest existing thing: also plane-anchored, also driven by a card while the sketch tool is inactive, also a render / hit-test / drag triad. It draws the live curve and the axis, and puts a handle on each of the three lengths: radius on the base circle, height at the top of the axis, pitch at the end of the first turn — which is exactly where one pitch of rise lands, so the handle means what it is standing on. Below one full turn the pitch handle moves to the end of the curve rather than floating off a curve that does not exist yet. Taper and handedness stay on the card. One is a shape modifier and the other a flag; L2 governs numbers you can point at. A drag reports the whole (radius, pitch, height) triple rather than one value, because pitch and height are coupled through the turn count and writing one alone would redraw a stale curve. The callback re-feeds the gizmo directly instead of going through refresh_preview(), since Helix takes the produces-no-solid early return and refresh_preview would rewrite the status line on every mouse move. REVIEW CATCH, fixed here: the first cut read taper as a fraction of the radius consumed over the turn count. It is an ANGLE IN DEGREES — helix_spine() builds a Geom_ConicalSurface of half-angle taper and takes the top radius as R+H*tan(taper), growing with the height risen. The wrong reading drew a preview that collapsed to a point for any non-zero taper while the committed feature was perfectly fine. A preview that lies is worse than no preview, which is what this commit replaced. Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-i3jc. |
||
|
|
a5bb41e340 |
Rib: the depth is the same arrow again
Rib's depth is a distance along the sketch plane normal, so it is the Extrude arrow for the fourth time — anchored at the midpoint of the line the rib is built on, because a rib's line IS its profile. This is half of Rib's L2 failure. The thickness is an in-plane offset either side of that line and no existing gizmo draws that; it needs a handle that does not exist yet, filed as snaporca-plew rather than left implied. One of two numbers draggable is strictly better than neither, and saying which half is missing is the point. Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-i3jc. |
||
|
|
4ed14eb0be |
SurfaceExtrude and Thicken: drag the distance instead of only typing it
Both tools produce exactly one number — a distance along a known normal — and neither had a handle for it. That is the same shape as the Extrude depth arrow, which was already written, already draggable and already had an editable label on the geometry. So this adds no gizmo: it points the existing one at two more tools. SurfaceExtrude anchors on its sketch's plane, at the profile centroid. Thicken anchors on the picked face, and reuses the face-as-profile recipe from the Extrude path verbatim — including the two things that path learned the hard way: look the face up on its OWNER body rather than the whole-document compound, and carry that body's display Move transform onto both the origin and the normal, or the arrow draws on the bed instead of on the face. The drag callback routes by active tool. `second` stays Extrude's alone: it is the two-sided pair, and the other two have a single distance each. SurfaceOffset is the third tool in this group and is deliberately NOT here. Its target is an arbitrary sheet body, which has no single normal to anchor an arrow on — that is a design decision, not typing, and it stays on the audit. Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-i3jc. |
||
|
|
b7397b48bd |
Transform: drag the body, the numbers follow
The Placement > Transform verb opened a card of spin controls — dx/dy/dz, an axis combo, an angle — with nothing on the geometry. The 3-axis drag gizmo the charter asks for already existed and was fully implemented (arrows, rotation rings, click-to-type per axis), reachable only from a small icon button in the tree card header. The prominent verb opened the form; the geometry-first control was hidden behind an icon. That was backwards. Transform now arms that same gizmo on the target body. The card stays as L2's typed half: the drag writes dx/dy/dz, the axis and the angle, and the pivot is seeded from the body's centroid so the parametric feature reproduces exactly what was dragged. Decomposition is exact for the interaction that matters — the gizmo's rings are per-world-axis, so a ring drag is an axial rotation. A pose composed from two rings is not axial and the card can only name one axis, so it reports the dominant one rather than refusing to answer. Three things this had to get right: - The gizmo bakes its drag into the display transform so the body follows the cursor, and the feature performs the same motion parametrically. Committing without reverting first would move the body twice. - tool_confirm() and tool_cancel() both tested moving_body() BEFORE the active tool, so with the gizmo armed Confirm would have dropped the gizmo and never created the feature. Both are now guarded on Tool::None. - close_tool() is the single revert point. Esc, Cancel and switching tools all pass through it, so a Transform that was never committed cannot leave the body displaced. Edit mode is untouched: re-seeding the gizmo from a stored feature is a separate problem, so editing an existing Transform still gets the card alone. Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-qtf4. |
||
|
|
3f62d4d58d |
Bodies: colour that survives selection, hide that toggles twice, Delete that acts
Three defects behind one report ("bodies cannot be moved or hidden/shown or
deleted, colour does not work"). They are unrelated to each other; only the
symptom was shared.
1. The Color tool wrote a per-body override that was correct end to end —
stored on CadBody, carried across recompute (CadDocument.cpp:3381), read
back by DesignCanvas::body_color() — and then overpainted every frame.
m_body_selected is a DOCUMENT-WIDE flag raised whenever a non-Sketch
feature row is selected, which is the resting state after any modelling
operation, and while it was true every body rendered gold. An explicit
colour now outranks the selection tint; unpainted bodies still tint, which
is all the tint was ever for.
2. The eye toggle re-selected the body row through m_tree, using item ids that
belong to m_parts. The row came back unselected, so the second press found
tree_body_selection() == -1 and fell through to the feature-level branch
instead of un-hiding. Hide worked exactly once. The sibling call in
refresh_parts() had it right.
3. The tree card's Delete button answered a selected body row with "select the
FEATURE that created this body" — an instruction the user cannot act on,
because the tree does not say which feature that is. on_delete_body()
already resolves CadBody::source_feature and confirms by name; it was
reachable only from the right-click offer. The button now routes to it.
Reviewed and compiled (libslic3r_gui, RC=0); not exercised — needs a session at
the machine to confirm all three in the viewport. snaporca-zjvg.
|
||
|
|
606026a920 |
Home: axonometric view, fitted
DesignCanvas::set_view() and fit_view() were both written and then never called from anywhere in the tree. The Design viewport has had no way back to a standard view since it existed: no key, no button, nothing but orbiting by hand until the model happens to drift into frame. That is worse than a missing convenience. A camera left pointing along the bed plane renders a scene that looks exactly like a failed renderer — geometry present, nothing visible — and an hour went into blaming the software GL stack before the real cause turned out to be two uncalled functions. Home rather than a letter: every letter A-Z is already a Shift+letter tool shortcut. Home is also the reset-the-view key most users arrive with. The dispatcher needed no change, it keys on the raw wx keycode. set_view() already does select_view + zoom_to_volumes, so this is fit and orient in one call. Doc row added to the View toggles table in docs/design_tab.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b305e8b154 |
tests: compile the five CAD test files that were never wired
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> |
||
|
|
d584d66003 |
Mate conflicts: name what silently wins, don't call it over-constraint
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> |
||
|
|
3eec65f2bd |
CoordSys: pick a body first, x-ray the rest
In an assembly the face you want for a mate connector is nearly always behind another body, and a click only ever returns the frontmost hit. Hiding the occluder from the Parts list works but means leaving the tool mid-pick. The CoordSys card now carries a Body chooser. Pick a body and every other one drops to 0.25 alpha AND stops catching clicks, so the wanted face is both visible and reachable in one gesture. "(all)" restores normal picking. Deliberately NOT hit cycling: repeated-click cycling was removed from solid picking as a charter L5/§10 violation (DesignSketchTool.cpp, "NO CYCLE"), and re-introducing it here would make "click a face" a multi-click gesture again. Mechanics: DesignSketchTool::set_pick_only_body() gates body_pickable(), which every pick path already consults; DesignCanvas::set_xray_focus() drives both it and the per-body alpha in reload(). The chooser stays a pick FILTER only -- coordsys_body still comes from the actual pick, so nothing in the kernel moves. Body focus follows the CoordSys card: open_tool() reads it back from the combo rather than clearing outright, because editing a CoordSys feature loads the card (and its body) before open_tool runs. snaporca-bgvk. NOT COMPILED: deps/build lacks OpenVDB so the GUI tree will not configure here; reviewed by diff only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8ff7ba440d |
CadDocument: reindex mate connectors on feature delete and reorder
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> |
||
|
|
0a2faedc32 |
Design: draw the mate connector, so its verse and polarity are visible
A mate connector was visible only to a program. resolve_datum_coordsys had exactly ONE consumer in the whole tree -- McpControl.cpp, the agent socket -- so the frame every mate is built on could not be seen at all, and the two questions a connector has to answer on sight had no answer in the viewport: which way does Z point (the VERSE), and which of the pair is anchored versus about to move (the POLARITY). The glyph is Onshape's proven core plus the part nobody ships. Disc for the XY plane, one gold quadrant for the roll -- the only in-glyph answer to "where is X", which matters because Fastened and Slider lock the clocking -- and a Z arrow drawn on +Z ONLY, never double-headed. Polarity is carried by the head: a filled cone travels, an open collar receives. Onshape, Fusion, Inventor and FreeCAD all draw both ends of a mate identically, which is why "which part moves?" is a standing complaint; nothing here invents new semantics, it just stops hiding them. Polarity is read from the committed Mate features, not only from the open card. A connector some mate drives must read as driven whenever it is on screen, or the glyph tells the truth only while a dialog happens to be open. The card, when open, still wins -- that is the live intent. Judged on the rig rather than in a mock, which changed three decisions: - Three RGB axis arms lose to one Z arrow. Rendered side by side (SNAPORCA_GLYPH=A selects the Onshape-style trio), the three heads are as large as the 22 px disc, they bury the quadrant, and at an oblique angle they pile into a smudge -- and the trio is indistinguishable from the move gizmo and the bed triad, which are already RGB arrow trios in this viewport. - Depth off floats, depth on tears. With GL_DEPTH_TEST off, connectors on faces pointing AWAY from the camera drew their discs over the solid, so the part looked covered in frames that were on its back. Turning depth on fixed that and immediately z-fought: the disc is exactly coplanar with its face and came out a broken dotted arc. Depth ON plus a 0.7*upp lift along Z buys both, and scaling the lift by upp keeps it sub-pixel instead of opening a visible gap on zoom-in. - Foreshortening degenerates an arrow into a dot when the axis points at the camera. It now draws a ring instead of silently vanishing, which is what a naive projection does. Everything is sized in screen pixels via upp = 1/zoom, like every other gizmo here: a connector is a symbol, not a part, so it must not shrink with the model. Research and the empirical findings are written up in DESIGN_MATE_CONNECTORS.md section 8b; rig images in artifacts/shots/g-0*.png, vendor reference glyphs in artifacts/glyphs/. Not fixed here, and recorded rather than papered over: the quadrant collapses to a blob at a grazing angle, which is exactly when the roll is hardest to read (F4); roll-undefined in red makes the least important connector the loudest thing on screen (F5); and a true grazing view, a curved face, and overlap with the move gizmo are still untested. Also surfaced while testing and unrelated to drawing: add_mate accepted a mate between two connectors on the SAME body and duly transformed the body relative to itself -- a concrete instance of the missing validation already filed as G6. Fork parity unchanged: DesignCanvas.cpp 16, DesignPanel.cpp 30, the other four files 0. |
||
|
|
24f4076bb5 |
Design: the Hole and Thread cards say which face they are holding
snaporca-200 asked which of the two models of "the card's face input" is right, because clicking empty canvas now clears the selection (snaporca-od0) and made them visibly disagree: Thicken / Shell / Draft read the LIVE selection and their label reverts to "(pick a solid face)", while Hole / Thread LATCH the face they were opened or picked on and keep it. The complaint was that Hole then drills a face you can no longer see selected. Taken to the rig, that turns out to be the wrong half of the story. With Hole open and its face picked, a click on empty canvas leaves the Ø6.0 ghost and its dimension gizmo drawn on that exact face — the card was never operating in secret, it was showing its target the strongest way a CAD tool can. Meanwhile Draft, whose behaviour was held up as the honest one, threw the pick away and had to be told the face again. So neither model replaces the other. They are different in kind: Thicken / Shell / Draft are operations whose operand IS the selected face, and Hole / Thread are placement tools with their own plane state that a pick merely seeds. The latch is also the kinder of the two now that empty clicks are a deliberate gesture — a stray one costs Thicken a pick and costs Hole nothing. What was genuinely missing is that nothing in those two cards NAMED the latched face, so after such a click the only words on screen were the viewport's "Nothing selected" over a ghost about to drill. Both cards now carry an "On face" row, the way the other three already do: Hole Face 5 | (none — uses Hole plane) Thread Face 1 | Edge 2 | (none — uses Thread plane) Thread names an edge when the cylinder came from a circular rim rather than a cylindrical face, which the code already distinguished internally and never said out loud. Verified on both rigs, every state driven through the GUI: face pick on open and on live pick, survival across a click on empty canvas, and the fallback after choosing XY/XZ/YZ from the plane dropdown. Thread's edge branch was exercised on a revolved tube's rim, its face branch on the same tube's outer wall. Filed while here, surfaced by the new row rather than caused by it — snaporca-uif9: re-editing a stored Hole/Thread from the feature tree restores f.plane into the dropdown but never clears m_hole_on_face, so the re-edit silently reuses the PREVIOUS card's latched face. Now visible by name instead of invisible. Fork parity unchanged: DesignPanel.cpp 30, DesignPanel.hpp 0. |
||
|
|
93395b7888 |
Design: escalate on the entity that was picked, and let the chip follow the window
Both found by Kimi reviewing the previous two commits, both then reproduced here before being touched. snaporca-97z. The re-pick escalation required m_solid_sel, m_sel_body, m_sel_face AND m_sel_edge to all match the previous pick. That looked stricter and was wrong: the edge branch sets only m_sel_edge and m_solid_sel, leaving m_sel_face as whichever face the ray happened to enter through — and a shared edge is entered through a different face depending on which side you view it from. So picking an edge and picking that same edge again from the other side compared equal edges, unequal faces, and refused the escalation the status line had just promised. Now the comparison is made at the level that was picked and nothing else. The edge id is already the stable global one from edge_index_of, so it identifies the edge without help from the face. Reproduced on the rig without needing to orbit, since two clicks 8px apart across an edge enter through different faces: pick -> sel=3 body=0 face=5 edge=3 ray -> body=0 face=0 re-pick -> escalated to whole body 0 pick -> sel=1 body=0 face=-1 edge=-1 Same edge, face 5 then face 0, escalation fires. The old condition could not. The frame-move case. The chip is anchored at an absolute screen position, and until now nothing told it the window had moved — only a resize, a status change or a tab switch re-placed it. Dragging the window by its title bar left it stranded where it was, verified on the rig by moving the frame and watching it stay put. wxEVT_MOVE on the top-level frame, alongside the ICONIZE and ACTIVATE binds from the previous commit. Two related cases are filed rather than bound, because the list of window-geometry events to chase is exactly what snaporca-lcq argues should stop: a layout change that translates the canvas without resizing it, and wxEVT_DPI_CHANGED. Not fixed, deliberately, and recorded on snaporca-97z: clicking the same FACE but landing within the vertex or edge tolerance resolves to a different kind and so does not escalate — that is the "smallest thing under the cursor" rule working as documented; and a vertex re-pick after moving the body compares stale world coordinates. Verified on both rigs. The cross-face edge case was exercised on orca_cad; DesignSketchTool.cpp is byte-identical across the forks, so snaporca inherits it, and its face-level escalation and empty-click clear were re-checked there directly. Fork parity unchanged: DesignSketchTool.cpp 0, DesignCanvas.cpp 16. |
||
|
|
6d1a4078ca |
Design: the status chip goes away with the window, not just with the page
Found by minimising the app on the rig with a face selected: the whole screen goes black and the chip is still drawn on the bare desktop. A wxPopupWindow is override-redirect — the window manager does not own it — so it neither iconises with its frame nor stacks behind other applications. IsShownOnScreen does not catch this either: an iconised frame still counts as shown, which is why the guard added for the tab case sails straight past it. So the frame has to say so itself: ICONIZE and ACTIVATE, both routed through the same show_status_hud the page change already uses. Restoring is safe — a popup cannot take focus, so our own Show() cannot re-trigger either event — and restoring while some other page is up still leaves the chip down, because show_status_hud(true) goes through place_status_hud's IsShownOnScreen guard. Verified on both rigs: chip up, minimise -> screen black and empty, restore -> chip back with its text and the face still selected. Restore while on Prepare -> chip stays down. This is the fourth defect from the same root, so snaporca-lcq now asks the question these binds keep deferring: whether the line should be canvas content, like the view cube and the round view buttons, rather than a window that has to be told about every way a window can stop being visible. Fork parity unchanged: DesignCanvas.cpp 16. |
||
|
|
9c3ce9b45e |
Design: clicking empty space lets go of the selection, and the status line follows its tab
Two things a click on nothing should already have done. snaporca-od0. A click that hit no geometry left the solid selection standing. A rubber band swept over empty space has always cleared it (pick_bodies_in_rectangle), and the two gestures cannot disagree about the same outcome. The visible cost was in the escalation that landed last commit: "click the face, click away, click the face again" arrived as the SECOND click on the same face and took the whole body, when the click away was the user letting go of it. Now the miss clears and says so. This gives up something real, deliberately: Thicken / Shell / Draft hold their input face in the panel's selection, so a stray click on empty canvas with one of those cards open hands that face back. Their handlers already write the "(pick a solid face)" placeholder and rebuild the ghost when the selection empties, so the card SAYS it lost the pick rather than confirming against a face the viewport has stopped highlighting. An orbit drag never reaches this branch — it exits at the 8px budget — so panning the view still does not deselect. snaporca-dlj. The status line is a wxPopupWindow, which is a TOP-LEVEL window: hiding the Design page does not hide it. Select a face, switch to Prepare, and the chip was still there reading "selected (whole body) — right-click for what applies to it" on a tab with no such selection and no such menu. Same cause, second symptom: a status update arriving while the page is hidden anchored against a client size that is not the size the page will have, and parked the chip on the tab bar. So: an IsShownOnScreen guard in place_status_hud, show_status_hud(bool) to take it down and bring it back with its text intact, driven from the page-changed handler. Verified on both rigs, not by reasoning about it: face 5 selected -> click bed -> "Nothing selected", tint gone -> click the same face -> face 5 again, NOT the body -> click it again with no click away -> whole body, so snaporca-gem is intact. Prepare -> chip gone; back to Design -> chip returns. KEYTRACE across the round trip shows shift+S then R still reaching the canvas (ui_mode 0 -> 1, Rectangle armed), which is the focus theft this popup replaced a wxFrame to avoid. Fork parity unchanged: DesignPanel.cpp 30, DesignCanvas.cpp 16, headers and DesignSketchTool.cpp 0. MainFrame.cpp is outside that set and was edited per fork. |
||
|
|
c32aa3f8ba |
Design: clicking the same face twice takes the body, and the status line moves onto the viewport
A click could point at a face, an edge or a vertex, but never at the body those
belong to: offer_selection_kind() can only return BodySolid when all three are
clear, which a viewport click never produces. The rubber band was the only door,
and the status line said "face 5 selected" while the user believed they had taken
the body. A second click on the SAME sub-element now escalates to it (snaporca-gem).
Not the pick cycle that was removed in bc2b741ce9 -- that one was silent and three
deep, so no click had a predictable meaning. Here the status line names the next
click before you make it, and a further click just takes the face under the cursor
again, which needs no teaching. Double-click is untouched: wx sends Down/Up/DClick/Up
and only the first Up carries a pending press, so a fast double-click still zooms to
fit and picks once.
The status line itself moved to the base of the viewport. In the side panel it was
clipped at ~73 characters with no warning and no wrap -- set_status()'s Wrap() never
took effect (snaporca-8cc) -- which silently length-limited every hint in the tab; the
first version of this change lost a clause to it. m_status is kept, hidden, as the
owner of the text and its colour, and the line is drawn in a bottom-left twin of the
readout HUD where there is a whole window's width.
Three defects found driving it on the rig, none of which the build could see:
* the HUD as a wxFrame took the WM's keyboard focus every time it was raised, and
the canvas then received NO key events -- every sketch shortcut silently dead.
Caught with SNAPORCA_KEYTRACE: shift+S logged a line, the following R logged
nothing. It is a wxPopupWindow now, which cannot be focused. SetFocus() on the
canvas does not fix it: focus was on another toplevel.
* zero vertical padding fits the popup tighter than the font's line box and clips
the glyphs; 6 (what the readout uses) reads as a two-line box. 3 is right.
* "has a caller chosen a colour?" compared the label's foreground against its
PARENT's, which differ by default, so every line counted as chosen and the
neutral text came out the panel's dark grey -- invisible on a dark chip. Compare
against the colour the label was created with, captured before any caller writes.
Verified on both rigs against fresh binaries: sketch -> extrude -> click face ->
click again -> whole body tinted, offer opens with the body rows live and Create /
Add material correctly greyed. Keyboard drives the whole sequence.
Filed and NOT fixed here: snaporca-od0 -- a bare-plate click does not deselect the
solid, so "click away, click back" escalates. Pre-existing; clearing there would also
drop the face the Thicken/Shell/Draft cards hold, which needs its own pass.
Refs: snaporca-gem, snaporca-8cc, snaporca-od0
|
||
|
|
7e5994b8cb |
Design: right-click a body row opens the offer, and taking a body always means the same thing
The third door onto the offer, after the viewport right-click and the Menu key. A body ROW is
an unambiguous body, so the offer reports BodySolid and the body verbs act on the row you can
see highlighted — the confirmation a face pick cannot give, since pointing at a face lights the
face and never the body the verb will change. The status line has been promising exactly this
("Body N selected — right-click for what applies to it") since before any handler existed on
that list; the product was advertising a gesture that did nothing.
WHAT THE RIG CAUGHT THAT THE BUILD DID NOT. The first version hung the state normalisation off
wxEVT_TREE_SEL_CHANGED. But SelectItem() on a row that is ALREADY selected fires no selection
event, so a stale vertex from an earlier viewport pick survived — and offer_selection_kind()
tests vertex FIRST, so right-clicking the body row served the VERTEX offer while the row sat
highlighted: Fillet/chamfer/draft greyed, Mirror standing where Repeat belongs, "vertex
selected" still in the status line and the cyan marker still on screen. The happy path (fresh
row, nothing else picked) looked perfect, which is why only the deliberate stale-state sequence
exposed it. Reading the code would not have shown it — SelectItem looks like it selects.
So the normalisation is no longer a selection handler. apply_body_row() is called
UNCONDITIONALLY by both doors, because taking a body from the list means the same state change
however it was asked for. It also clears m_sel_solid_vertex, which the original handler never
did — latent while nothing opened the offer from that list, and immediately fatal once
something did.
Verified on both rigs with the failing sequence itself: pick a vertex, then right-click the
already-selected row. Fillet/chamfer/draft enabled, Repeat back in place, status reads "Body 1
selected", vertex marker gone.
Does NOT touch the feature tree. That needs new selection kinds (offer_selection_kind has no
notion of "a feature is selected") plus verbs the atlas does not contain — Suppress, Rename,
Reorder, Roll back — and is filed separately.
|
||
|
|
6c59898ac0 |
Design: pointing at part of a body is pointing at the body
Tommaso: "i deleted a body using rubber band selection, but this is not intuitive as all the ux revolves around clicking". Correct on both counts, and a correction to what I said last round: the rubber band IS implemented and shipping (pick_bodies_in_rectangle, m_rubber, the drag branch in on_mouse). What is unbound is whole-body picking via CLICK; I read the comment about the click path and wrongly generalised it to the gesture as a whole. The handlers were never the problem either. Move, Mirror, Cut, Mass and Colour all resolve their target through selected_body_default() / m_sel_solid_body, and that is already set when you click a FACE — level >= 1 records the body. They would have worked from a click all along. The only thing keeping them out was the atlas gate: accepts listed body_solid and no face kind, so offer_selection_kind() returning FacePlanar filtered the rows away. This is therefore an atlas-only change, no handler edits. Cut, Split, Mirror, Transform, Mass and Colour now accept face/edge/vertex as well, matching what Delete Body already did. Edges and vertices are included deliberately, not just faces: a click resolves to a vertex, an edge or a face depending on where inside the pixel it lands, so accepting only faces would make Move vanish whenever you clicked near a corner — a flicker that reads as a bug and gets reported as "sometimes it works". NOT widened: Extrude on a face means push/pull THAT face, and Thicken consumes the face you point at. Both have genuine face-specific meaning, so widening them would change what they do rather than where they can be reached from. The rubber band keeps its job — it is still the only way to take a body without also naming one of its faces. It just stops being the only door. Verified on both rigs from a plain face click: Transform > Move opens with Body = Extrude2 (resolved from the face pick), Modify > Edit / Delete Face / Colour / Delete Body, and Reference > Mass. |
||
|
|
b2654ebd8a |
Design: a body knows what made it, so "Delete Body" can exist
Reported by Tommaso: select a body, and there is no Delete in the offer. Two independent faults stacked behind that. FIRST, clicking a body never selects the body. Whole-body picking is deliberately unbound (DesignSketchTool.cpp) pending the rubber band, so a viewport click only ever yields Face/Edge/Vertex. The offer therefore saw face_planar, and "delete" accepted body_solid but no face kind, so the row was filtered out entirely — while the status line read "Body 1 face 0 selected", which actively teaches the wrong model. SECOND, even selecting the body from the Bodies list, Delete refused in red: "Select the FEATURE that created this body". CadBody had no link back to its maker, so the offer was advertising a verb it could not perform — worse than the action:null rows fixed earlier this session, because this one is ENABLED and its refusal reads like user error. CadBody::source_feature fixes the second. It is stamped in ONE place, the recompute loop, and the rule is just "still unset?". That is sufficient because of an invariant worth stating: no feature ever replaces a whole CadBody. Every in-place op writes only .shape (boolean, cut, mirror-fuse, transform, dress-up — all 8 sites checked), so a body keeps the stamp it was born with; a consumed body is erased outright, taking its stamp with it; and the only bodies still at -1 are the ones the current feature just pushed. A feature type added later needs no change here as long as it keeps to that invariant. "Delete Body" fixes the first, sitting beside "Delete Face" in Modify and reachable by pointing at any face/edge/vertex. The two names cannot be confused, and "delete" gave up the body kinds so both can never appear for one selection. Deleting a body removes the feature that made it, which is a real edit to the recipe, so it asks first and NAMES the feature — a body vanishing from the viewport is not evidence of which feature went, and this is the one action here that cannot be eyeballed. Multi-body delete is NOT offered. bodies_2 was in the first draft of the verb; the handler deletes exactly one body, so a two-body selection would have silently deleted whichever was m_sel_solid_body. Caught before it reached a binary, at the cost of one rebuild. Verified on BOTH rigs, full round trip: click a face -> Modify > Delete Body -> "Delete Extrude2?" -> body gone, Sketch1 correctly left behind, panel falls back to the idle hint -> Undo -> Extrude2 and Body 1 restored. |
||
|
|
34eb4224a1 |
Design: fix a wrong issue ref in the Thicken comment
The previous commit cites snaporca-y7q, which does not exist — I wrote the ID from memory instead of reading it back from the bug I had just filed. The real one is snaporca-kgx, "Offer: Thicken (and peers) open with the picked face discarded". The comment is corrected here; the commit message above it cannot be, so this note is the pointer. |
||
|
|
9d47280a19 |
Design: a card opened from a face must use, and show, that face
snaporca-y7q. Thicken's opener cleared m_sel_solid_face outright. That was right when the
only door was a toolbar button — a button carries no selection, so pressing Thicken had to
clear and ask you to point at something. The offer inverted it: the verb is now invoked ON
a face, and the same line threw away the only thing the user had said. The card opened
reading "(pick a solid face)" over an immediate "thicken: face not found" — you pointed at
the face and were told none could be found.
Keep the pick when the body combo landed on the body it came from (the index is per-body,
and selected_body_default() returns exactly that body when it is valid).
Two neighbours had the mirror-image flaw, both invisible for the same reason — the value
was right and the ghost updated, so only the label lied:
- Thicken had NO live label update at all. Nothing outside the opener ever wrote
m_thicken_face_label, so while the card was open you could pick face after face and it
still read "(pick a solid face)".
- Shell and Draft wrote theirs ONLY from the pick handler, which runs while a card is
already open — so opened from a selection they showed the previous pick, or the
placeholder over a face they were about to use.
So the label is now written once in open_tool(), which every door goes through. The
edit-feature path already restores m_sel_solid_face from the stored feature BEFORE calling
open_tool, so it agrees rather than fights.
Verified on the snaporca rig: face 4 of an extruded plate, offer > Add material > Thicken
now opens "Face: Face 4" with "Preview — 24 triangles" and confirms to a real Body 2. Draft
opened from a face shows "Face 3" and previews the taper. This fork is code-identical here
bar the two permitted DropDown divergences; it still owes a build of its own (snaporca-5pl).
Project keeps its clear: there "(all edges)" is a legitimate default mode rather than a
failure, so changing it would alter behaviour with no reported problem behind it.
|
||
|
|
cfc2555c3a |
Design: a verb's address is data, so the toolbar widget can stop existing
snaporca-7ih's remaining half. Both flyout factories registered their verbs INSIDE the widget-building loop, so the ~40 retired tool buttons had to be constructed and then Hide()n: skipping construction would have deleted 42 offer verbs (26 fly:<family>#<row> + 16 Shift+keys) while their rows still rendered and did nothing when picked. Register first, build second. The addresses are pure data; the widget is one door onto them, not their owner. A family absent from kBarKeep now returns before any wxWindow is made. The keep-list stays a one-line data decision, not a structural one. And close the class of bug for good: the constructor now verifies, once, that every verb the atlas marks wired resolves to a real registration, logging each break and asserting in debug. Rows that render and do nothing have shipped three times (edit_feature and sk_move with action:null, then this) and are invisible from either side alone. Verified on the snaporca rig by walking the offer, not by reading the code — all four at-risk address kinds run with no widget behind them: fly:design_rect#2 drew an OBLIQUE rectangle (the third variant, not the family's first), key:S+E opened Extrude with its 10 mm gizmo, fly:material#4 opened Thicken. Hover hints, icons and nesting intact. This fork is code-identical here bar the two permitted DropDown divergences; it still owes a build of its own (snaporca-5pl). Two hints were wrong and are fixed: Cut said "Split the body with a plane", colliding with the Split verb one row away and pointing at a card for a value the canvas already offers as a draggable arrow; Split never said its plane comes from a picked face. Also, because it blocked the verification and will block the next one: gui-session.sh killed by full path while its own app_pid() matched by basename, so a differently-pathed instance survived, held the single-instance lock, and got reported as a healthy session — a Jul-30 binary nearly passed as this build. It now kills by basename and prints which binary is actually on screen. Traps 6 and 7 documented. |
||
|
|
96816f725c |
Design: every offer verb has a hint, shown on hover — and the status line wraps
Mirror of snaporca 2b3e890165 (DesignPanel.cpp applied as a patch; parity 30 / 16, shared files byte-identical). All 86 verbs now carry a hint: 55 extracted from the C++ tool definitions so the offer and the armed-tool hint cannot drift, 31 written by hand. One wxEVT_MENU_HIGHLIGHT binding shows the hovered verb's hint in the status line. The generator asserts that no wired verb lacks one. Also: all 200 status writes go through set_status(), which wraps instead of clipping at the panel edge; and the empty-document hint is called from on_tab_shown() as well, since after_tree_edit() never runs on a freshly opened tab. Verified on the rig. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
3037e55f44 |
Design: hints name the gesture that works, and an empty document says how to start
Mirror of snaporca 033347d062 (parity 30 / 16). Retiring the toolbar made ten hints untrue: each named an action whose door had moved to the offer, or a button no longer on the bar. They now name the gesture. An empty document blanked the status line entirely and now says how to start. Known and not fixed here: m_status does not wrap, so long hints clip; and the 86 offer verbs still have no per-verb hint of their own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
811719b7aa |
Design: Construction goes back on the sketch bar — a mode must show its state
Mirror of snaporca b3d4cf85af (parity 30 / 16). Hiding it with the drawing tools was wrong: Construction is a persistent MODE, not a tool — the Bed checkbox, not the Line button. Q and the offer's Construction row kept toggling a checkbox nobody could see, so you could not tell whether the next line would be construction geometry. Scoping unchanged and already correct: m_tb_sketch is shown only in UiMode::Sketch, so it appears exactly while a sketch is open or being edited. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
86f1f96c50 |
Design: the toolbar is chrome — every tool is reached from the offer
Mirror of snaporca 809aa9df87 (DesignPanel.cpp applied as a patch; parity 30 / 16, shared files byte-identical). fadd() and sadd() now gate what reaches the bar: file operations, Bed, Undo/Redo, Delete selected, Commit to Plate, Confirm/Cancel, plus Place on Face and Section view — the last two because they are chrome_only in the atlas and have no offer row to fall back on. The tool buttons are still built and then hidden, deliberately: their fly: addresses and Shift+key bindings are registered inside the widget-building loops, so not building them would silently drop 42 verbs from the offer while they still rendered. snaporca-7ih covers hoisting the registrations so the construction can go too. Four separators whose groups are now empty were dropped; they rendered as stray rules. Verified on the rig in both modes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
a535b0cb76 |
Design: the offer draws each verb's icon — set the bitmap BEFORE Append, not after
Mirror of snaporca bcab67f8ce (DesignPanel.cpp applied as a patch; parity 30 / 16, shared files byte-identical). tool_atlas.json now names an icon for 80 of 86 verbs, derived from the toolbar's own definitions rather than invented, and every one of the 54 distinct names was checked to exist in resources/images first. The first attempt drew nothing despite a green build: wxGTK builds the GtkMenuItem inside Append() and reads GetBitmap() there, so setting the bitmap on the returned item is a silent no-op. append_offer_item() constructs, sets, then appends — the same order Orca's own append_menu_item() uses. Verified on the rig. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
bc4fb3b680 |
Design: Text and SVG draw INTO the open sketch instead of beside it
Mirror of snaporca 1fb7786d9a (DesignPanel.cpp and DesignCanvas.cpp applied as patches; parity 30 / 16, shared files byte-identical). With a sketch open, Text/SVG outlines become ordinary Line entities via push_closed_lines() instead of a separate Sketch feature carrying rigid imported_regions — so the letters can be constrained, trimmed and extruded like anything drawn by hand. The buttons and offer actions arm Select first when in Sketch mode, since begin_sketch() does not run until a tool is armed. add_imported_regions() calls reset_autoedit(): without it the glyph contours entered the draw-then-edit queue and opened a Length field on the first segment, which freezes the canvas. Caught on the rig, not by reading. No sketch open: unchanged — a new Sketch feature, still dropped on a picked face. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
447c71a0d2 |
Design: Text and SVG join Create — they were excluded on a premise that is not true
Mirror of snaporca 6724ea27c5 (DesignPanel.cpp applied as a patch; parity 30, shared files byte-identical). chrome_only's rule is "acts on the DOCUMENT, not on a selection". Text and SVG both call add_imported_sketch(), which drops the art on a picked solid face via SketchPlane::from_face() — a selection-consuming profile creator, like Sketch. Now sk_text / sk_svg in the sketch half's Create row, where their toolbar buttons already sit. Verified on the rig: Create ends Point, Text, SVG. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
d5c5d5675e |
Design: a body tool acts on the body you picked, not on the first one
Mirror of snaporca e5e223a794 (DesignPanel.cpp applied as a patch; parity 30, hpp byte-identical). Every body combo opened on index 0, so picking a body and pressing Mirror acted on a different solid while the card showed that other body as the target. Nine sites now read the viewport selection; Boolean takes the picked body as target and a different one as tool, since defaulting both to the same body is a no-op. Verified functionally on the rig: picked the 20x20 body, mirrored, and the new body measures 20x20 — not the 80x50 one it would have used before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
273cf067e8 |
Design: record that Shell stays in Remove — decided, not overlooked
Mirror of snaporca e2ef3018cd. Data only, and only the `why` prose in two slots — the generated DesignOffer.hpp is byte-identical, so there is nothing to rebuild. Ratified 2026-08-01: the offer deliberately splits the toolbar's dressup family. Shell hollows a solid so it sits in Remove; Delete Face edits an existing solid so it sits in Modify. Recorded in both slots' `why` so either half explains the split. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
f6e6cd83c1 |
Design: the fillet row is named for the tools it actually holds
Mirror of snaporca 352cf1c259 (data only — tool_atlas.json + the regenerated DesignOffer.hpp; both byte-identical across the forks). "Fillet / chamfer / draft" rather than naming shell too: Shell is in Remove and Delete Face in Modify. Only the toolbar's dressup dropdown groups all five. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
0c2b643b0b |
Design: the card says which tool it is, and "Dress-up" stops being a word we use
Mirror of snaporca 3677937964 (DesignPanel.cpp applied as a patch; parity 30, shared files byte-identical). The card header read "Fillet 1" over a chamfer because the offer's Chamfer address opened the tool before setting the type, and open_tool() titles the card from that combo. Choose first, then open. "Dress-up" removed from the offer row (-> "Fillet / chamfer"), the card field (-> "Type", it was a label reading Dress-up whose value said Chamfer) and the toolbar tooltip. Verified on the rig: header "Chamfer 1", field "Type: Chamfer", row "Fillet / chamfer". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
3c998b8a62 |
Design: a tool's options come from the tool, not from a card on the left
Mirror of snaporca 6d5510734b (DesignPanel.cpp applied as a patch; parity re-checked at 30 lines, shared files byte-identical). Polygon's Sides/Circumscribed card is deleted — the choice is made in Create > Polygon, which names the counts and the two fits, because the side count cannot be recovered after drawing. Dress-up, Combine and Pattern were single verbs hiding several behind a combo and now name each one in the offer. Fixes fillet and chamfer both carrying key:S+F, which made the offer's Chamfer open a Fillet. Built green and verified on the rig: the Dress-up card opened from Chamfer reads Chamfer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
d1d61ce997 |
Design: every sketch tool has an address in the offer, not just its family
Mirror of snaporca 0c83f59f13 (DesignPanel.cpp applied as a patch, not copied, so this fork's 30 permitted divergent lines survive; parity re-checked at 30/16 with the shared files byte-identical). The sketch dropdown never registered "fly:<family>#<row>" addresses the way feat_dropdown does for model verbs, so the offer could name a family but only ever arm its first tool — Rectangle always gave a corner rectangle. Adds the registration, 14 atlas verbs (including the entire array family, which was absent, and rotate/scale), an action for the sk_move row that previously did nothing when picked, and a second submenu level so variants nest under their family instead of flattening 19 create tools. Built green and verified on the rig: Oblique rectangle arms oblique, not corner. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
fd1bc092d8 |
Design: slot Radius caption, keyboard offer, plane combo removal, mass props, docs
Mirror of snaporca ac85277bac..0e7cb3ec78 (six changes, applied as a patch to DesignPanel.cpp rather than copied, so this fork's 30 permitted divergent lines survive — parity re-checked afterwards: the five shared files are byte-identical, DesignCanvas.cpp and DesignPanel.cpp differ by exactly 16 and 30 lines). - The straight slot's inline field says Radius, which is what it sets. It stores the half-width and passed the typed number through unchanged, so 30 produced a 60 mm slot. - The offer opens from the keyboard (Menu, Shift+F10), anchored on the viewport rather than wherever the pointer happens to be. The card hint names the new route. - The sketch card's Plane combo is gone; the plane comes from the viewport. Also stops build_candidate collapsing a face plane to a base plane while editing. - Mass properties and the dead Edit row are wired into the offer; DesignOffer.hpp is regenerated from tool_atlas.json, verified by re-running the generator and diffing. - docs/rig_build_traps.md + scripts/rig-build.sh, which derives its fork identity from project() so it cannot be pointed at the other fork's image or volume. - docs/design_tab.md refreshed (44 commits stale) + a PR description, with this fork's own merge-base and diff shape rather than snaporca's. Built green in the deps container with the new script and verified on the rig: Menu and Shift+F10 both open the offer at the viewport centre with the pointer parked off-canvas, and the sketch card shows no Plane row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
6e9910303b |
Design: a sketch takes its floating chrome with it when it ends
Confirming a sketch while an inline value field was open left the field behind. The editor is a top-level frame, so it survived the session that owned it, and inline_busy stayed set with it — on_mouse_impl then returned true at its first branch for every later click and the viewport was simply dead. No refusal, no message: exactly the "click the geometry, nothing happens" the pick bugs above it were mistaken for. finish() and cancel() now call close_session_chrome(): dismiss the open field (keep-as-drawn, the same contract the polyline terminators already use), drop the queue of fields behind it, and clear the corner readout — which had the same defect for the same reason, sitting on 336.8° over a committed sketch because nothing redraws the HUD once the tool stops. Verified on the rig on the exact reported sequence: line on XZ, Return to accept the length, Confirm with the Angle field still open. The field goes, the sketch commits, and the next click reaches the pick (pick trace shows down/up consumed) and selects Sketch1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
1addba6ea0 |
Design: editing a quote UPDATES its dimension instead of appending a rival to it
Type a new length into a sketch quote and the number changed while the geometry
sat still, with the solver dropping to "Conflicting constraints" — a broken
constraint state the user never asked for, arrived at by doing the one thing the
status line invited.
An experiment separated the two candidate causes. Same gesture, one variable:
a line committed WITH a driving length refused the edit and conflicted; a line
committed with none (Esc keeps it as drawn) accepted it and visibly shrank. So
the value was never the problem and the solver was not wrong — it was being
handed two contradicting constraints and correctly declining to choose.
The quote-edit path appended unconditionally:
a.con = int(m_constraints.size());
m_constraints.push_back(constraint_for(a));
Accepting the length at draw time creates Distance(P0,P1) = 64.9. Clicking the
quote later creates a SECOND Distance on the same two points asking for 30. Over
-constrained by construction. A line with no dimension yet only ever gets one
constraint, which is exactly why this looked intermittent rather than total.
upsert_constraint() finds an existing constraint with the same type and operands,
overwrites its value and returns its index; it appends only when there is none.
Operand order is ignored — a Distance from A to B is the same constraint as B to
A, and so is an Angle. Returning the index matters as much as the update: it
keeps the annotation's `con` pointing at the constraint that is actually live, so
the NEXT edit is an update too rather than reverting to appending after one good
round. upsert_dimension() applies the same rule to the visible quote, which had
been stacking labels reading different values on the same pixel, and keeps the
existing label position so a placed quote does not teleport.
set_dimension_value already did the right thing through a.con. The machinery
existed; these two call sites never consulted it.
Verified on :11 on the exact failing case — draw a line, Return to lock the
length, Confirm, double-click to re-open, click the line, type 30 into the quote:
the line shrinks, the quote reads 30.0 mm, one label not two, and the solver
stays at "3 degrees of freedom" with no conflict.
NOT included: record_dimension_constraint() has the same unconditional push_back
in all six of its branches. It belongs to the legacy Constrain mode with
different selection semantics and I could not exercise it, so it is flagged
rather than changed blind.
Refs snaporca-e1p.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM
|
||
|
|
a1fdb9f217 |
Design: double-click a sketch stroke to edit it — the gesture belongs on the geometry
Selecting a committed sketch line lit the right tree row and then told the user to go and press Edit in the panel. That is the side-panel dependency this tab exists to remove, and it made "selectable" true while "editable from the geometry" stayed false. on_edit_feature already does the whole job — re-open the entities in the sketch UI with handles and live quotes — and was only ever reachable from a tree row. A double-click on a committed stroke now calls it for that feature. Double-click on empty space still fits the view, so nothing is taken away. The stroke hit test is now one hit_display_sketch() shared by the click and the double-click. Two copies of "what is under the pointer" drift, and a double-click acting on a different entity than the click before it is a miserable thing to chase. It also reports the entity index, which the tracer prints, so a pick that lands on the wrong stroke can be seen rather than inferred. Verified on :11 end to end: draw an open line, commit, double-click it. The tracer prints "double-click -> edit sketch feature 0 (entity 0)", the panel reads "Editing sketch — drag a handle or click a quote to edit", and a click inside the session selects the line with endpoint handles, live quotes and "1 selected — Delete removes them". NOT delivered by this commit, found while verifying it: typing a new value into a length quote is accepted and displayed but the geometry does not move and the solver drops to "Conflicting constraints". Filed separately — it lives in the constraint layer, not in selection, and nothing here touches it. Refs snaporca-e1p. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
c5404507c7 |
Design: an open sketch line can be clicked — region membership is not a licence to be pointed at
Reported from the rig: a committed sketch holding a single open line rendered on
the plate and could not be selected, so it could not be edited or deleted from
the viewport at all.
The viewport pick for committed sketches iterated region_loops(). That function
exists to find EXTRUDABLE regions and, as its own walk comment says, an open
chain "stalls" and is discarded. So for a sketch of open entities it returns an
empty list, the pick loop has nothing to iterate, and every click falls through
to bare plate. Not a tolerance problem and not a focus problem: there was no
candidate geometry to test against.
Whether a stroke bounds a closed region has nothing to do with whether the user
can point at it. The stroke test now covers every non-construction entity, and a
separate entity->region map preserves what a hit REPORTS, so a click inside a
closed loop still names that loop exactly as before. Region membership decides
the report, not whether the hit can happen.
The rest of the path was already written and simply unreachable: the panel's
handler has a region < 0 branch that selects the feature, highlights it in the
tree and says "Sketch selected — Extrude it, or Edit / Delete from the tree".
This makes existing behaviour reachable rather than adding new behaviour.
Verified on :11 with the pick tracer (SNAPORCA_PICK_TRACE=1), which is what
distinguished the two failure modes: before, the click reached the handler and
fell through to handle_solid_click ("no solid data"); after, it is consumed by
the display-sketch test and never reaches it, and the panel reads "Sketch
selected" with Sketch1 lit in the tree.
Construction geometry stays unpickable, matching region_loops' own filter. It is
the same class of bug — a construction line cannot be selected to delete it —
but including it risks construction stealing picks from real geometry, so it is
left as a separate decision.
Refs snaporca-e1p.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM
|
||
|
|
9f2b2bc511 |
Design: sketch means a tool — the offer works inside a sketch, and the app stops
contradicting itself about the plane Reported from the rig: pick a plane, press Sketch, and you are told to pick a plane. The app prescribed a sequence and then refused to acknowledge that you had followed it. Right-click did nothing, so there was no way to reach a drawing tool except the toolbar this tab exists to retire. act_sketch was two lines: set the mode, then print "Click a face or a reference plane in the viewport, then a sketch tool" — unconditionally, without ever asking whether a plane was already chosen. The plane was never lost; m_ref_plane held it and begin_sketch captures it when the first tool is armed. The sentence was simply false. It now asks. sketch_plane_target() is a companion to sketch_plane_from_selection that distinguishes "the user chose XZ" from "nothing chosen, falling back to XY" — a distinction m_ref_plane cannot express on its own, being always a valid index, so m_plane_picked carries it. With a target the readout names it and the offer opens on the Create row; without one the old prompt stands, because then it is true. The card above the status line was a local wxStaticText that nothing could update, so it went on asking for a plane two inches from a line saying the plane was chosen. It is a member now and the two are written together. Right-click inside a sketch was excluded wholesale so that it could end a polyline chain, abandon an anchor, exit a tool. That made every sketch row in the atlas unreachable. The honest test is not which mode we are in but whether the tool actually USED this right-click, and only the tool knows: on_mouse now wraps on_mouse_impl and records that once, for every terminator, instead of threading a flag through the twenty-odd sites that consume a RightDown. The canvas read-and-clears it on the matching release. Underneath all of it was one confusion — MODE versus SESSION — at four sites. begin_sketch does not run until the first tool is armed, so is_sketching() is false for exactly the interval between "press Sketch" and "pick a tool", which is precisely when the drawing tools must be on offer. The keyboard learned this once already (snaporca-0ud, whose comment states the rule) and I reintroduced it in offer_selection_kind and again in show_offer_menu, where the offer built from the FEATURE map and rendered nine rows that all refused the sketch selection. Both now call sketch_map_applies(), so they cannot drift apart again. The keyboard keeps its own split: its "sketching" gates undo and delete-last-entity, which genuinely need a live session. Verified on :11 against a fresh build. Pick XZ, press Sketch: card reads "Drawing on XZ", status reads "Sketching on XZ — pick a tool", offer opens with Create and Reference live and the six rows needing geometry greyed. Right-click while idle opens the offer. Right-click as a terminator does NOT — the chain ends, the line lands on XZ, its length field arms at 49.36 mm. That last one is the regression the blanket exclusion was buying and the reason this shape of fix was chosen over a mode test. Refs snaporca-6vs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
bb403b82cf |
Design: left-drag sweeps a rubber band, and it takes the whole body
The pick cycle died two commits ago and left no viewport route to a whole body at all: one click resolves vertex, edge or face, double-click is already zoom-to-fit, and the only way to take a body was the Bodies list — a geometry-first violation for as long as it stood. The rubber band is that route. Left-drag is the gesture, as asked. That button was orbit, so this canvas now maps the mouse the way every CAD the user already knows does: left selects, middle orbits, right pans. The change is a single flag on GLCanvas3D set only by DesignCanvas, so Prepare and Preview keep the mouse their users learned. Sketch mode inherits the same mapping, which is the consistent reading — Design is one modality, not two. Past an 8 px budget a press becomes a sweep, anchored at the ORIGINAL press point rather than at the frame where the threshold was crossed, so the first few pixels are not lost. Below the budget it is still a click and the existing vertex/edge/face pick runs untouched. Sampling is the display mesh's triangle vertices plus centroids — the same points the ray pick tests, already in world coordinates — and the body with the most samples inside wins, because the selection callback downstream carries one body. Crossing semantics: touching selects. Enclosed-only for left-to-right and crossing for right-to-left is the fuller CAD convention and is deferred, not forgotten; with one selectable body it would have bought nothing. Two defects fixed on the way, both found by exercising this: Right-drag pans, and every pan ended by popping the offer over wherever the camera stopped — the context menu arriving as the reward for moving the view. The offer is now the release of a STATIONARY right-click, at the same 8 px budget the pick uses. The selection handler wrote m_status twice. Only the later write ever reached the screen, so the earlier block had been dead since it was written, and its labels drifted out of step with the live ones unnoticed — including a vertex fix I made this morning in the branch that never renders. Deleted, with a note saying why, rather than left as two writers for the next person to pick the wrong one. Verified on :11 against a fresh build: click takes face 5; left-drag across the body reports "selected (whole body)" with the whole solid tinted and the camera unmoved; left-drag over empty space clears; stationary right-click opens the offer; right-drag pans with no menu; middle-drag orbits. Precedence re-checked after the deletion — face at 25 px from the corner, vertex from 10 px in. Refs snaporca-9xw. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
33f97d259b |
Design: vertex picking — a click near a corner takes the corner
Completes the precedence Tommaso asked for: vertex, then edge, then face, all from ONE click, all decided in screen pixels. Verified on :11 by sweeping into the right corner of a plate — x=1250 and 1290 report "face 5 selected", x=1308, 1318 and 1323 report "vertex selected" — and the cyan marker lands exactly on the corner in the render. The vertex tolerance (11 px) is deliberately LARGER than the edge one (8 px). A corner lies ON its edges, so equal radii would make vertices unreachable: every click near one would resolve to the edge underneath. Bigger-wins-first is what makes the smallest entity actually pickable. Vertices come from the sampled edge polylines' endpoints rather than a separate topology walk — every corner of a face is the end of one of its edges, so the data was already in hand. The highlight is a camera-facing square scaled by 1/zoom, the same trick the edge ribbon uses, so it reads as a constant dot at any zoom. This is why vertex picking did not ship with the previous commit: the Edge render path billboards a ribbon and degenerates on a two-point input, and a selection you cannot see is not a selection (L5). Better to add the primitive than to fake the feature. DesignPanel now distinguishes level 4: a picked corner sets neither face nor edge, because a corner is not its face, and the offer classifies it as OfferSel::Vertex — where Plane, Axis and Coord Sys already accept it. snaporca-9xw (rubber band still open — see the issue). |
||
|
|
b003d20e37 |
Design: kill the pick cycle — one click selects what is under the cursor
Tommaso, correctly: fix selection before building on it. I had taken the
whole→face→edge click cycle as terrain and hung the tool offer off it, when §10
of the charter already listed that cycle as an L5 violation. An offer can only
ever be as truthful as the selection beneath it, so this is the foundation and
it should have come first.
NOW: one click selects the SMALLEST thing under the pointer — the edge if the
cursor is within tolerance of one, otherwise the face. No repeat clicks, no
state, no memory of what was picked before. Verified by sweeping a column of
single clicks down a plate on :11: y=800..915 all report "face 5 selected", and
y=925/935 — within a few pixels of the front edge — report "edge 3 selected".
One gesture, one deterministic result, which is what L5 asks for.
TOLERANCE IS IN SCREEN PIXELS. The old edge step compared a ray-to-segment
distance in millimetres, so the same gesture meant different things at
different zoom levels. The pointer is a screen object; its tolerance has to be
one too. 8 px, measured against the edge polyline projected through the camera.
WHAT IS NOT HERE, AND WHY IT IS NOT FAKED. Whole-body selection has no viewport
gesture in this commit. Double-click is ALREADY zoom-to-fit, bound earlier in
the same on_mouse, and this pick runs on LeftUp where LeftDClick() can never be
true — so a double-click branch here would have been dead code that reads like
a working feature. I wrote one, found it unreachable, and deleted it rather
than leave it. The body gesture is the rubber band, which is its own piece of
work; until it lands bodies are selected from the Bodies list, and the hole is
named in a comment at the site instead of being left for someone to trip over.
Six status strings that promised the cycle ("click again for a face", "click
again for an edge", "click again to reset") are gone — they described a
behaviour that no longer exists, and a hint that lies is worse than none.
Both forks build. Parity: DesignSketchTool.cpp byte-identical, DesignPanel.cpp
30 divergent lines — the invariant exactly.
snaporca-6vs.
|
||
|
|
7114e316ea |
Design: the offer ships — right-click the geometry, get what applies to it
Row order is RATIFIED (charter 4.1, 2026-07-31) and this is the first working implementation of it: right-click in the Design viewport and a vertical list opens at the pointer with the eight families in their fixed order, the verbs that apply live, and the ones that do not disabled IN PLACE carrying their reason. THE MAP EXISTS ONCE. DesignOffer.hpp is GENERATED from docs/ux/tool_atlas.json by docs/ux/mockups/gen_offer_table.py — the same file the 113 mockups are drawn from. A drawing and the product therefore cannot drift apart, which is the only way row constancy survives contact with a codebase. Never hand-edit the header. NOTHING IS RE-IMPLEMENTED. Each row routes to the code that already runs that verb: "key:S+E" through m_keys_feature, "key:L" through m_keys_sketch, "fly:material#4" through the feature flyout's own action, "btn:colour" through the standalone button. The offer is a second door onto the same room, so the toolbar, the shortcuts and the menu cannot drift into three behaviours. The 8 verbs with kernel support but no GUI path show disabled, which is honest and matches section 10 of the charter. Right-click only fires the offer when the canvas is IDLE. Right-click already ends a polyline chain and finishes the move gizmo; taking those over would break two working interactions to add a third. Two things the running build corrected, both found by looking at screenshots: - THE REASON MUST BE TRUE FOR WHAT IS IN FRONT OF THE USER. Taking the first refusal in a family printed "Transform needs a body — add or import one first" on a document that HAS a body, because the real obstacle was that nothing was selected. Now the reason comes from a verb that accepts the current selection and fails only on document state; if no verb in the family accepts this selection at all, it says "select something first" or says nothing. A menu whose whole value is telling the truth cannot ship a lie. - Classification follows the level the pick cycle has REACHED, not the face the ray happened to hit, so the header cannot name a face while the whole body is lit. Sketching on the face you merely clicked is untouched — that path is sketch_plane_from_selection (snaporca-3a2). Verified on :11 end to end: nothing selected shows Sketch live with Shift+S and seven greyed rows each explaining itself; a selected solid shows Move directly with Shift+Y (one applicable verb, so no submenu and no extra click) and five families as submenus. Both forks compile and link. Fork parity re-checked after the port: DesignPanel.cpp 30 divergent lines, DesignCanvas.cpp 16, every other CAD file byte-identical — the invariant exactly. snaporca-96r. |
||
|
|
00948cf767 |
docs: the offer is a vertical list, and it opens on every machine
Folds the form-factor decision into the charter. 4.1 is rewritten around Tommaso's proposal — left-click selects, right-click opens a vertical list of icon / name / shortcut — and the radial is demoted to a "Rejected" subsection rather than deleted, because it is a good idea that loses on evidence and somebody will propose it again. The evidence is recorded with it: mean fill of 3.45 of 8, only two live slots on a fresh document, sketch Create needing nine addresses on an eight-slot ring, names that do not fit around a circle in translation, and a 380px disc over the model on a 1366x768 screen. The invariant survives the change of geometry, which is the useful proof: same eight families, same fixed order, nothing re-sorted or compacted. It is now stated as ROW constancy, and the one substantive gain is that unavailable verbs are disabled IN PLACE carrying their own reason, in strings the product already ships. An empty ring slot was mute; a greyed row teaches. On a first-run document the offer stops being a mostly-blank control and becomes a map of what the product does and what you must do first — which is the section 2 audience in one picture. Opening it is now a table rather than an assumption, because "right-click" is not a universal gesture: two-button mouse right-clicks, trackpads two-finger tap, a one-button Mac LONG-PRESSES or Ctrl-clicks, touch and pen long-press, and the keyboard uses the Menu key or Shift+F10. The long-press is explicitly an ADDITIONAL route — 6.2 forbids press-and-hold as a sole path and that stands, so the rule now names its own exception and closes it — and it must show that it is charging, or a user who lets go early concludes the product is broken (L5). Consequently: 6.2's keyboard bullet describes opening and walking the offer by key rather than by compass direction; the gate gains question 13 (every new pointer gesture declares its keyboard equivalent and what a one-button Mac, a trackpad and a touch screen do) and question 12 now asks whether tool_atlas.json was updated and the atlas regenerated; section 10 points at the rendered atlas and says the outstanding thing is ratifying row order, not drawing the map. snaporca-96r. |
||
|
|
c4990ee956 |
docs/ux: draw the offer as a vertical list too, and it wins
Tommaso was not sure about the ring and proposed a vertical list: left-click selects, right-click exposes icon / name / shortcut. Drawn, it is better, and the reasons are visible in the renders rather than arguable. THE DISABLED ROW CAN SPEAK. This is the one that decides it. A ring slot that does not apply is an empty circle: it says nothing, and on a fresh document six of the eight are empty. A list row that does not apply is greyed IN PLACE with its own name and its own reason — "Create a sketch, or pick a solid face, first", "Create a solid body to pattern first" — which are strings the product already ships and which tool_atlas.json already carries. The first-run picture stops being a mostly-empty ring and becomes a map of what the product does and what you must do first. For the audience section 2 puts first, that is the whole ballgame. THE OVERFLOW DISAPPEARS. Sketch Create needs nine addresses; a ring of eight pushed Polygon and Point behind a "More" slot. Nine rows is just nine rows. The one measured defect in the ring design is not a defect in this one. SHORTCUTS READ AS A COLUMN. Right-aligned in a list they stack into something the eye learns passively, which is exactly the graduation path 4.1 claims — and it is the mechanism by which the power user Tommaso describes stops opening the menu at all. Around a ring the same keys are eight loose chips. Also, unglamorously: long translated names fit, arrow keys and screen readers work natively where a radial needs special handling, and a 324px box costs the 1366x768 machine far less than a 380px disc over the model. What the ring keeps: equidistant targets and a future flick gesture. Since the brief is that power users live on the keyboard, that buys less than it looks. The invariant is untouched — same eight families, same fixed order, nothing re-sorted, nothing compacted. Only the geometry changed, which is the point: the map survived a change of form factor, so it was a real map. Both forms are now rendered side by side for the same states, and the atlas opens with the pairs. snaporca-96r. |
||
|
|
eb2fc986a4 |
docs/ux: the offer atlas — every tool, every state, drawn
The charter fixed the slot-constancy invariant but carried one hand-written eight-cell table as an illustration, and nothing of the offer exists in the product. Before any GUI code, the group needs the map itself: what verbs there are, what each one needs before it can be offered, and what the ring actually looks like in every situation a user can put it in. tool_atlas.json is the source of truth and it was extracted from the code, not from memory: verbs, shortcuts and the exact refusal strings from DesignPanel's six feat_dropdown call sites and the sk_key table, kernel coverage checked against CadFeatureType, headless coverage against McpControl's dispatch, and the selection kinds taken from the callbacks DesignCanvas actually exposes. It carries each verb's preconditions, so "why is that slot empty" has an answer already written in the product's own words. The eventual C++ table generates from this file too — the map exists once. gen_offer_mockups.py renders it: 20 selection kinds x 2 document states = 40 primary rings, plus 73 sub-rings, plus comparison sheets. 113 states, none of them hand-drawn, because a human drawing 113 rings is exactly how an address quietly changes. Everything is framed at 1366x768, the charter's own reach target, so L11 is tested in the mockups before it is tested in code. Three things the drawing found that the prose had not: - MEAN FILL IS 3.45 OF 8. The empty-slot rule is cheap in argument and expensive on screen; on a fresh document exactly two slots are live. That picture is the anti-clutter thesis made literal and it is the strongest image in the set. - SUB-RINGS MUST ANCHOR ON THEIR PARENT. Fanning them from north put Extrude at N, which is Create's address in the primary map, so the second level contradicted the first. Anchored, an address is two consistent strokes: Add material is NE and its first verb is NE again. - ONE FAMILY OVERFLOWS, AND ONLY ONE. Sketch-mode Create needs nine addresses on an eight-slot ring. That is the ninth-position pressure the charter predicted, arriving on schedule and measured rather than argued: either Point moves family, or the tail goes to a third level, or the ring is not eight. Every model-mode family fits. The generator refuses to wrap a tenth verb onto a first — silent collision is the one outcome worse than an ugly ring — and reports the overflow instead. Also fixed while looking at renders: the selection pill sat on top of the north slot's shortcut chip and hid it, and the scrim at 0.55 swallowed the very face the ring had been opened on, which is 4.1 failing inside its own mockup. Fork-neutral: the generator and everything it emits name no product, so both forks carry byte-identical copies. snaporca-2is. |
||
|
|
c3d286070e |
docs: the offer is a fixed address space, not a context menu
Tommaso's requirement, and it changes what the offer IS: a tool must sit in the
same physical position whatever you selected. Click a face, an edge or a text
and fillet is in fillet's place every time. Position becomes an address the hand
learns, and the eye stops being needed.
That kills the ordering rule this section had two commits ago. "Most-used first
for that kind of selection" is adaptive ordering, and adaptive ordering destroys
the one property that makes a spatial menu fast — worse, it destroys it exactly
for the user who has just started to learn the layout. Office 2000 shipped that
idea and withdrew it. So: NO adaptive ordering, ever, in any form.
The invariant, written to survive every future feature: every tool has exactly
one address; that address is identical in every selection type where the tool
appears; slots for inapplicable tools are left EMPTY rather than compacted; and
adding a tool never re-addresses an existing one. Empty slots are the price of
constancy and they are cheap — a compacted offer is denser and unlearnable, a
sparse one is memorised in a week. An empty slot also answers a question ("this
cannot be done to this thing") that a silently-inert tool does not.
Radial rather than a strip, reversing what I proposed last time and for a reason
that only appears once constancy is the requirement: a direction from the click
point is an absolute address that survives the offer opening anywhere on screen
and survives being clamped at a screen edge, while "third item down" does not.
Centre is a hole so the picked geometry stays visible, and it names what is
selected, so a mis-pick is caught before a verb is chosen.
Every slot carries its keyboard shortcut beside the icon and the word. This is
the graduation path and it is why power users never see a conflict: you reach
for the place, the place says "F", and one day your hand types F before the ring
finishes drawing. The offer is the mechanism by which a beginner stops needing
the offer — one interface at two speeds, no advanced mode in between.
Also here: a proposed eight-position compass map across face/edge/body/text
(create, add, remove, dress-up, repeat, transform, reference, modify) offered as
the group's first ratification, with families opening a secondary ring under the
same rule; arrow/numpad direction addressing so the spatial map works from the
keyboard; a gate question 12 that treats re-addressing an existing tool as a
breaking change to every user's muscle memory.
snaporca-2is.
|
||
|
|
d7583f0a2d |
docs: the grammar becomes object-driven, and commit stops being invisible
Two changes to section 4, both from Tommaso. FIRST: the selection does not merely feed the tool, it DETERMINES WHICH TOOLS EXIST. Point at a planar face and the product offers the small set of things a planar face can become; point at an edge and it offers fillet, chamfer and the sketch tools that can reference it. Nothing else, because nothing else is possible. This is the largest single thing available to us for a first-time user, and the reason is worth writing down: a beginner's difficulty is not operating a tool, it is not knowing which tools apply to what they are looking at. Sixty icons answer a question they cannot yet ask; a face that offers its own five verbs teaches the product by being used. It also deletes a whole class of failure — a tool that silently does nothing because the selection was wrong becomes unreachable. The offer is an accelerator, not a toll gate: toolbar and single-letter shortcuts keep working unchanged and consume the same selection, so an expert never looks at the offer and a beginner never needs the toolbar. Both routes land in the same place, which is how one interface serves all three audiences. SECOND: "click empty space to commit" is withdrawn. It was an invisible gesture with a destructive meaning — nothing on screen said it, and a stray click committed a feature still being adjusted. Exactly what L5 forbids. A pending feature now carries a confirm/cancel puck attached to its own geometry, beside its handles, with Enter/Escape mirroring it; empty space reverts to the safe meaning, clear the selection. The puck is an object in the scene, not a dialog: the camera orbits, the values stay editable, nothing is blocked (L4 intact). Two cases the rule has to get right or it damages the inner loop: continuous tools (line, rectangle, circle) still commit each entity on its own gesture — a tick per line would be miserable — and Enter/Escape end the tool rather than confirm an entity. And ambiguity resolves toward keeping work: starting another operation with a valid feature pending commits it rather than discarding it, because undo reaches everything and the recoverable direction is the right default. Section 10 gains the two honest consequences: today a selection offers nothing (the largest single item of new work this charter asks for) and committing is still the invisible empty-space click. snaporca-2is. |
||
|
|
610acfcd37 |
docs: reach is the first accessibility, and it gets a law
The charter had accessibility only in the assistive sense — keyboard, contrast, colour, targets — and said nothing about who can get through the door in the first place. That was the larger omission. The premise of an OSS CAD tool is that a kid on a school laptop, with no licence, no account, no fast machine and nobody to teach them, can open it and make a real thing; a tool that only the equipped can run reaches people who were already going to design something. So the fourteen-year-old is now the FIRST of three audiences, ahead of the maker and the mechanical designer, with an explicit rule that when audiences conflict the earlier one wins unless someone writes down why not. L11 states the floor: runs completely on a low-end laptop with integrated graphics at 1366x768, offline, no account, and no capability withheld behind a tier, a plugin or a cloud service. Section 6 splits into 6.1 reach and 6.2 the assistive floor: the reference machine, the small screen as the layout target rather than the stretch case, files that belong to the user, learnable with no documentation, plain language at the entry tier, and exploration that is never punished — undo reaches everything, nothing asks the user to be sure. Consequences elsewhere: the screen budget is set by the smallest screen we serve, not the reviewer's monitor; the PR gate gains a reach question; B6 joins the benchmark (the inner loop on the reference machine, offline, fresh install) and every task is measured there rather than on a workstation. The side-panel debt now fails L11 as well as L1 — on that screen the cards leave the model a strip. One role addition: the absent audience needs a seat. The kid cannot file an issue, so someone owns B5/B6 and the group watches real first-timers quarterly. Approachability is the one thing here that cannot be argued from principle. snaporca-2is. |
||
|
|
76690f66e9 |
docs: the UX charter names only this fork's product
The doctrine is shared but the document is not: each fork's copy now speaks about its own product only, so it reads as that project's own charter rather than as a note about a sibling repository. This is a deliberate divergence — the two copies must NOT be reconciled by a parity sweep. The CAD sources stay byte-identical; only this doc branches. snaporca-2is. |
||
|
|
8cc08845a8 |
docs: UX guidelines and charter for the Orca-CAD design group
The call with SoftFever settled that Orca-CAD is one of the branches to be implemented and that a design+dev group forms around it. A group without a written doctrine reviews by taste, and a CAD reviewed by taste becomes FreeCAD one locally-reasonable side panel at a time. So the doctrine is written first, as something a reviewer can FAIL a pull request against: ten laws each with its own test, the interaction grammar they compose into, the accessibility floor as a merge requirement, and a ten-question gate answered in every UI pull request. The position is Shapr3D's interaction economy, not its feature list — direct, gestural, almost no chrome, depth revealed by what you touch. Depth for mechanical designers arrives as progressive disclosure of tools that never move, in three tiers, non-modal, with assemblies and exploded views obeying the same point-then-act grammar as a beginner's extrude. The one thing neither Shapr3D nor FreeCAD has is that we live inside a slicer: plate, nozzle, material and build volume are known at design time, so print-domain failures are warnings on the geometry, not a report. Section 10 is an honest inventory: what already complies, and the six things that violate the laws today, none of them defended. The appendix keeps the anti-patterns we have already paid for, because each one is cheap to reintroduce. snaporca-2is. |
||
|
|
f14d31d956 |
scripts/gui-session.sh: bring the headless GUI up without clicking blind
The relaunch sequence was an ad-hoc pile of docker exec one-liners, and it had a real bug: it dismissed the first-run dialogs by computing the titlebar close box from `xdotool getwindowgeometry --shell` and clicking it. When the dialog had already closed, that eval left the geometry variables stale or empty, the click landed at a garbage coordinate, and it kept hitting the Sketch button in the toolbar underneath — so the app came up in sketch mode with a stray Sketch feature that then had to be cancelled by hand. Three times in one session. The fix is not a different mechanism. `xdotool windowclose` looks cleaner and KILLS THE APP: it destroys the GdkWindow out from under the dialog and the process dies with "GdkWindow unexpectedly destroyed", three GLib-GObject criticals and a segfault. Measured, not guessed — that is what the first version of this script did. Escape does not close the Setup Wizard either, which is why it needs handling at all. So the titlebar click stays, and what changes is that it refuses to click geometry it has not validated: the window id is re-resolved immediately before, all four geometry variables are unset first and must come back numeric, and the computed point must be inside the screen. Any of those failing logs why and clicks nothing. Two further honesty fixes in the status output, both caught by reading it rather than by it failing: app_pid skipped nothing, so with a container full of <defunct> instances it printed a dead pid as though the session were healthy — it now walks /proc/<pid>/stat and ignores zombies. And a container without x11vnc reported "vnc: DOWN" as if something had broken, when nothing was ever installed; it now says so, and does not try to start what is not there. Also replaces the fixed post-launch sleep with a wait for the main window, because cold starts under software GL vary by a lot, and adds --status for diagnosis: "no windows but the desktop is up" means the app died, "cannot connect at all" means the desktop did. That distinction cost real time to work out by hand. Verified on both containers: one run each, wizard closed on validated geometry, no stray sketch mode, live pid reported, and the app still up. snaporca-e1p adjacent (tooling, not the tab itself). |
||
|
|
3f8f46f93f |
Delete the sketch plane dropdown; the viewport decides
The plane combo is gone. A sketch takes its plane from what is picked in the viewport: a face on a solid, or one of the reference-plane ghosts, clicked in 3D. The card is now a single line of instruction instead of a control. The combo had become worse than redundant. Once a picked face could be the plane it displayed a row that CONTRADICTED the actual target — it still said XY while the sketch went onto the face — so the one place a user could look to confirm where they were drawing was the one place guaranteed to be wrong. What replaces it is state, not UI: m_ref_plane records which reference plane was last clicked in 3D (0/1/2 = XY/XZ/YZ, >=3 indexes the datums) and ref_plane_name() turns it into text for the on-geometry hint. Clicking a ghost plane while a session is live re-planes it immediately, which the combo's own handler used to do; that behaviour is kept, just driven from the geometry instead of the widget. A plane click also drops a stale face pick, so last pick wins in both directions. populate_plane_choices() stays — seven other pickers use it (Plane base, Axis A/B, Helix, Project, Mirror, Cut). Those are the next candidates, tracked on snaporca-e1p; this commit only removes the one that had become actively misleading. Also: tessellation now matches Orca's OWN STEP importer, linear deflection 0.003 instead of 0.01 (Format/STEP.hpp default; angular was already 0.5 rad and unchanged). The Design viewport was never using a different rendering technique — it hosts a real GLCanvas3D, builds a real Model/ModelVolume and goes through the same reload/GLVolume path and the same shaders as Prepare and Preview. What differed was the mesh handed to it: 3.3x coarser than anything else in the application, which is why a curved face read as faceted beside an imported part. Suite unaffected at 154 cases / 2125 assertions, so nothing depended on the old density. Verified on :10: the card shows no dropdown, one click on a face then a sketch tool still reports "on the picked face", and the circle is drawn in that face's plane (artifacts/shots/h3a2-02-sketch.png, h3a2-03-drawn.png). snaporca-e1p, snaporca-3a2. |
||
|
|
3c0843c68c |
Sketch on the face you clicked, not the one you clicked twice
The previous commit made a picked face the sketch plane and I verified it by clicking the face TWICE. That was the wrong test. handle_solid_click cycles whole -> face -> edge, and "First click on a (new) body/face selects the WHOLE solid; refine on repeat clicks" — so at level 1 m_sel_solid_face is -1, and one click on a face, which is what selecting a face means to anyone, still fell through to the plane combo. The fix was real and unreachable, which from the outside is indistinguishable from no fix at all. The face id was never missing. handle_solid_click resolves it by ray on the FIRST click and passes it to on_solid_selection_changed regardless of the cycle level; the panel simply discarded it whenever level < 2. Keep it in m_pick_face/m_pick_face_body and let a sketch use it, preferring an explicit face-level selection when there is one. Nothing about the cycle changes, so body operations that rely on whole-body selection are untouched. The new state is dropped wherever the existing picks are, so a stale face cannot come back: choosing a body from the Bodies list (an explicit choice with nothing pointed at), picking a committed sketch loop (last pick wins), undo/redo (recompute invalidates topology ids), and when a sketch consumes the face. Verified on :10 with ONE click, which is the flow that was broken: build a box, single-click its top face, S then C, and the hint reads "Circle — click center, then radius · on the picked face" with the circle drawn in that face's plane (artifacts/shots/g3a2-01-one-click.png, g3a2-02-sketch.png, g3a2-03-drawn.png). GUI-only, so the kernel suite is unaffected — plane_of_face and its 154 cases / 2125 assertions are unchanged from the previous commit. snaporca-3a2. |
||
|
|
6ce20d78c3 |
Sketch where the user pointed: a picked face is the sketch plane
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.
|
||
|
|
7f0a8c85ee |
An entity sketch that forms no wire fails, instead of extruding a default box
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). |
||
|
|
5f1811c2c5 |
A subtraction that removes nothing is an error, not a silent success
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.
|
||
|
|
5d05ca30ca |
Sketch shortcuts: pick the key map by mode, not by whether a session exists
All 17 single-letter sketch shortcuts were dead. The dispatch gate was circular: the sketch key map was consulted only when m_viewport->is_sketching() was already true, but is_sketching() is a whole-session flag whose only riser is begin_sketch(), called from select_tool() -- which is precisely what every sketch key closure calls. So the first letter after entering sketch mode fell through to the feature map, where the keys are Shift+letter, matched nothing, and did nothing. The mouse worked only because the toolbar flyout row reaches select_tool() directly, bypassing the gate. Which key MAP applies is a question about the mode. Split the flag: 'sketching' (live session) still drives undo/redo, Delete and the section-view branch, where a session genuinely has to exist; a new 'sketch_mode' (m_ui_mode == UiMode::Sketch alone) drives the map choice. Verified headless end to end, keyboard only: Shift+S, then R draws a 143.4 x 133.7 rectangle on XY reporting 4 degrees of freedom, then F and L arm Fillet and Line (artifacts/shots/0udb-01..03). Note the toolbar strip does NOT change when a tool is armed by keyboard -- the family buttons are flyouts and only show their own pressed state -- so the pixel diff on that strip, which is how this was originally measured, reads 0 for a tool that is live. The status line is the surface that actually reflects the armed tool. Also fixed, from snaporca-d9i's list: the plane-pick status line said "press Sketch to draw on it", naming a button that exists only in Feature mode. It is now mode-aware. Adds a SNAPORCA_KEYTRACE=1 trace in the CHAR_HOOK printing key, ui mode, is_sketching, in_text and the focused window's class. It is what separated "the fix does not work" from "the surface being measured never moves", and it costs a full GUI build to re-add, so it stays. snaporca-0ud, partial snaporca-d9i. |
||
|
|
347b83d887 |
Sketch fillet: never write a failed solve's geometry back, and commit the op
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 |
||
|
|
50577d66d0 |
Dress-up card: say whether Confirm will round the picked edge or the group
The card decides between a single picked edge and a whole face-group from a viewport pick it never mentioned. With no edge picked the user saw only the group combo and concluded per-edge rounding did not exist; with an edge picked the combo still read "All" — the opposite of what Confirm would do. Adds a Target row that names the actual target and greys the group combo out while an edge is picked, wired at the same three points Shell already uses: the selection-changed handler, the re-edit load, and open_tool. Ported from the Snapmaker fork (33771a0e95). snaporca-40d |
||
|
|
d5dee45acf |
Port the four DesignPanel fixes that never crossed from the Snapmaker fork
The CAD sources are meant to be byte-identical across the two forks, with exactly
two permitted divergences: DesignPanel.cpp's flyout plumbing (30 lines — mainline's
DropDown is Item-based where the other fork takes three parallel vectors) and
DesignCanvas.cpp's Bed3D::set_shape signature (16 lines). DesignPanel.cpp had drifted
to 105.
Nothing failed to announce this. The kernel suite does not compile the GUI, and all
four defects are interaction-level, so both forks stayed green while only one of them
had the fixes. The parity diff is what found it.
* combo_append_index and its five call sites. Orca's ComboBox keeps client data in
its own vector, so Append()'s clientData argument never reaches wxItemContainer
and m_clientDataItemsType stays wxClientData_None. GetClientData() opens with a
wxCHECK_MSG, which is an early return — so every read came back NULL and every
caller resolved it to index 0, silently, because 0 is a legal answer. Affects the
rib sketch picker, the sheet-body picker, both mate coordinate-system pickers and
the sweep path picker.
* Wrap(240) over the card labels. wxStaticText never wraps itself, so a one-sentence
hint sets its card's minimum width to the width of the whole sentence and every
control in that card is clipped at the sidebar's right edge.
* Shell and Draft face-label initialisation in open_tool. Both read the face from the
live pick at Confirm time, but only the pick handler wrote their labels, so picking
a face and then opening the card left the card describing one operation while
Confirm performed another.
* SurfaceOffset and ThickenSurface added to build_candidate's negative list. Both read
a sheet body from their own combo and both had it overwritten by the picked solid's
index. The list is a negative one, so a tool that picks its own body breaks by
omission — noted in a comment now.
Also drops a stray <cstdio> left behind by a debug probe.
Verified by building the GUI here for the first time since these landed: 461/461,
liblibslic3r_gui.a links. DesignPanel.cpp is back to exactly 30 divergent lines and
every other CAD file is byte-identical again.
snaporca-7xx snaporca-aqu snaporca-gu9 snaporca-c03 snaporca-5pl
|
||
|
|
faed169a01 |
Sketch dimensions: make Tab commit, instead of silently dropping what you typed
The inline editor special-cased Escape and Skip()ped every other key, so Tab fell through to wx's default navigation. Its popup frame holds exactly one control, so focus came straight back to that control with its text re-selected. Type 60, Tab, 40, Enter — expecting to fill two dimensions — and the 60 is gone: Tab neither committed it nor advanced, so the 40 just replaced the re-selected text. A re-selected field is pixel-identical to a freshly opened one, so nothing on screen says a number was dropped. Tab-to-next-dimension is what Onshape, SolidWorks and Fusion do, which is exactly why it is the key a user reaches for. Tab now calls do_commit(), the same path Enter takes; the caller's on_commit is already what walks to the next dimension. Verified by driving the GUI: 37 Tab 24 Enter now produces a 37.0 x 24.0 rectangle, where before it produced 24 and a mouse-derived value. snaporca-xah Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM |
||
|
|
695932ad87 |
Drop the degenerate triangle OCCT emits at every filleted corner
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
|
||
|
|
2c5ddd4102 |
OCCT link order: put TKFillet/TKOffset before their dependencies, not after
OCCT_LIBS is an explicit single-pass static link order — dependents first, TKernel
deliberately last. The CAD block appended its two extra toolkits to the END of that list,
which puts them after everything they depend on:
list(APPEND OCCT_LIBS TKFillet TKOffset)
TKOffset references BRepAlgo_Loop, and nm against the built deps prefix shows TKBool is the
only toolkit that defines it (TKTopAlgo, TKBO, TKPrim, TKFillet and TKOffset all define it
zero times). TKBool sits first in the list, so a single-pass linker has passed it long before
it reaches the appended TKOffset and will not go back:
libTKOffset.a(BRepOffset_MakeLoops.cxx.o): undefined reference to
BRepAlgo_Loop::BRepAlgo_Loop()
Only one configuration ever objected — the Snapmaker fork Flatpak (aarch64). Ordinary Linux,
macOS and Windows links resolve it regardless, and the mainline fork Flatpaks pass, so six
green platform legs said nothing about whether this list was correct.
Prepended via set() rather than list(PREPEND), which needs CMake 3.15 while this project
supports 3.13.
Worth knowing for later: TKFillet and TKOffset are mutually dependent, 20 symbols needed in
each direction, so a stricter single-pass link could still trip on that pair. It does not on
any current platform, so no --start-group or duplicate entry is added here; if something ever
complains about ChFi or BRepFill symbols, that cycle is the reason.
snaporca-2kj
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM
|
||
|
|
295c030309 |
DesignPanel.hpp: declare the three wx types it uses but never named
The header forward-declares a long list of wx types and omitted wxBoxSizer, wxTextCtrl and
wxListCtrl. All three are used as pointer members only (m_expr_text, m_var_list,
m_parts_hdr, m_hdr_tree_row), so a forward declaration is all they need — but there was
none. Every ordinary build compiled anyway because the wx/panel.h + wx/scrolwin.h chain
happens to pull the real headers in transitively.
The Snapmaker fork Flatpak build (aarch64) has a wx that does not, and it failed outright:
DesignPanel.hpp:511: error: 'wxTextCtrl' does not name a type; did you mean 'wxTreeCtrl'?
DesignPanel.hpp:521: error: 'wxListCtrl' does not name a type; did you mean 'wxFileCtrl'?
DesignPanel.hpp:663: error: 'wxBoxSizer' does not name a type; did you mean 'wxSizer'?
plus a cascade of "m_var_list / m_expr_text / m_parts_hdr was not declared in this scope".
Not an environment quirk: the header was simply not self-contained, which is exactly what
breaks a reviewer building in an unfamiliar configuration. The mainline fork Flatpaks passed
on both arches, so only that one manifest exposed it.
Audited the rest of the header afterwards: every other wx pointer type is either
forward-declared or genuinely included — only wxScrolledWindow and wxWindow are undeclared,
and both come from the real wx/scrolwin.h include.
snaporca-4dn
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyRwbuq6fjn3VV9U9UvhBM
|
||
|
|
c1b0484495 |
3mf test: give the BBS save a writable temp dir, instead of the filesystem root
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
|
||
|
|
5026dd11a6 |
Fix the solver abort on circle-line tangency; the CAD suite now runs complete
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>
|
||
|
|
f599ff0ff7 |
kernel-test.sh: one quarantined case now, not two
Follow-through from eeca6794e7. The header claimed two pre-existing failures are excluded and named both; the internal-thread case now runs like any other, so only the solver SIGABRT is left. A comment that lists a test which is no longer excluded sends the next reader looking for something that is not there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
df6ef85614 |
Un-quarantine the internal-thread test: the geometry was right, the test was not
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> |
||
|
|
a95e8ee701 |
Re-edit: list the bodies as of the feature's timeline slot, not the final ones
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> |
||
|
|
8f06b9dfd8 |
Design tab: make Cut re-editable, and stop misdescribing Import
on_edit_feature had two types falling into default: with "This feature type can't be edited yet". Boolean was already handled, so the follow-up note was stale on that point; the real gap was Cut and Import. Cut now re-edits like any other feature: plane, offset and target body are restored and the generic replace_feature path commits the change. The body list is rebuilt with populate_body_choices(m_edit_index) for the same reason Boolean does it — a Cut splits one body into two, so the live body list no longer matches the one this feature's target index was recorded against. Replaying to just before the feature makes the stored index land on the right entry. Import deliberately gets no dialog. An imported solid has no parameters to re-edit: its geometry is rigid data read from a file, not something rebuilt from numbers, and moving it is what the Transform feature already does. Building an "edit" for it would duplicate Transform behind a second name. So it now says that instead — the previous message implied a dialog was coming that should not. Imported 2D Text/SVG art is a different thing and stays re-editable; it arrives as a Sketch feature carrying imported_regions and is handled above. The default: arm is kept as a guard so a feature type added later announces itself rather than silently swallowing the click. NOT click-tested — GUI wiring, compile-verified only. Added to snaporca-cfi. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
9c199e4a38 |
Fix the Shellcheck CI job, red since the CAD kernel-test loop landed
The Shellcheck workflow has failed on every push and every scheduled run since 2026-07-24, on exactly one finding: SC2029 in scripts/kernel-test.sh, the file the CAD branch added. So the CAD work is what turned that job red, and a PR arriving with a red job is a bad way to open a conversation with a maintainer. Client-side expansion of $REMOTE is the intended behaviour — it is derived from $VOL locally and the remote has no such variable, exactly as the rsync destination two lines down relies on. So this is a disable with a reason, not a silencing: the note says why the warning does not apply. Verified by running the workflow's own command over all 24 matched scripts: exit 0, no findings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e2b921745d |
Add user documentation for the Design tab
docs/design_tab_upstream_portability.md explains the subsystem to a maintainer; nothing explained it to a user. This is that: what the tab is, how to get a first solid out of it, every tool grouped the way the toolbar groups them, and the keyboard shortcuts read out of the source rather than remembered. The limitations section is deliberate. Rib needing a sketch with an explicit open line, Surface Loft and Surface Fill having no hands-on verification, mates composing transforms instead of solving simultaneously, move-face and replace-face being absent, and the two quarantined kernel tests are all things a user would otherwise discover by hitting them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
f347afd22e |
Document the CAD subsystem's real dependency weight
Maintainers will ask what the Design tab costs before they will look at the diff, so measure it rather than assert it. The headline correction: the OCCT delta is THREE toolkits, not two. The comment in deps/OCCT/OCCT.cmake claimed "TKFillet + TKOffset (3.77 MiB, Windows only)". Walking OCCT's own adm/MODULES and each toolkit's EXTERNLIB shows ModelingAlgorithms holds twelve toolkits, that eight of them are built either way because DataExchange (the STEP path upstream already ships) depends on them, and that the true delta is TKFeat, TKFillet, TKOffset and TKXMesh — of which TKXMesh is never produced. So three archives are built: 7.40, 5.38 and 4.42 MiB. TKFeat is the interesting one. Nothing in the Design tab references it and it is absent from the TKFillet/TKOffset dependency closure, so it is built for nothing — OCCT's module flag is all-or-nothing per module. On static-link platforms that is build time and zero shipped bytes. Two numbers are deliberately absent, marked as absent, and not approximated: the Windows DLL delta needs a Windows build (snaporca-gix), and a clean-build time delta needs the deps prefix built twice on one machine. The old 3.77 MiB figure is withdrawn rather than reused — it covered two of the three toolkits. Also recorded: the vendored solver is 9,339 lines under GPLv3 with its LICENSE preserved, which combines into this AGPLv3 fork without difficulty (AGPLv3 §13), and it is live code driving every sketch constraint — not a carried corpse. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e2b58a17f7 |
Design i18n: extract the tab's strings at all, and translate them into Italian
DesignPanel.cpp was never listed in localization/i18n/list.txt, and xgettext
extracts only what that file names. All 934 of the Design tab's _L() calls
were therefore invisible to every translator in every language — not merely
untranslated, unextractable. Adding the one line is the actual fix; the rest
follows from it.
Regenerating the .pot brings the catalogue from 6007 to 6536 msgids. 549 of
the new ones are now translated into Italian: 5230 to 5779 translated
messages. Verified no existing work was destroyed — every msgid that survived
into the new .pot kept its translation, and the 36 that lost one are genuinely
gone from the sources. msgfmt --check-format passes, which matters here
because a large share of these carry %d / %s / %zu.
CAD terms follow Italian CAD convention rather than literal glosses: Fillet ->
Raccordo, Chamfer -> Smusso, Draft -> Sformo, Rib -> Nervatura, Mate ->
Accoppiamento, Shell -> Svuotamento, Pattern -> Serie, Sheet body -> Corpo
superficie. Strings identical in both languages are deliberately left
untranslated so gettext falls back to the msgid.
The long mixed-filament / Local-Z dithering tooltips are left untranslated on
purpose: slicer internals, outside a Design i18n task, and untranslated before
this commit too.
One string changed rather than translated. The Coord Sys hint read "Without an
edge the frame's rotation about its normal follows world X, not the body" —
that described the defect fixed in
|
||
|
|
1726e93760 |
Mate connectors: derive a face-only frame's X from the face, not from world
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> |
||
|
|
f13f2876f6 |
Tag the two known-broken tests [NotWorking] so CI stops being red on every commit
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> |
||
|
|
b25335e1b3 |
Rib: accept a Project feature as its sketch ref
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> |
||
|
|
1cb80f7f9f |
Project: implement "(all edges)"; stop discarding the failure reason
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> |
||
|
|
08e37296b9 |
Design tab: give every drawer entry a distinct icon
Six of the surface entries, both Thicken variants, Rib, Axis, Coord Sys, Mate and Delete Face all reused a sibling's glyph, so a drawer opened as a column of identical faces and the card that opened rarely matched the entry clicked. Fixed both halves: entry icons are now unique within their drawer, and each card header uses the icon of the entry that opens it. Two new glyphs, design_thicken and design_rib, are the only ones added — everywhere else an existing icon already carried the right meaning (design_revolve, design_offset, design_line, design_point, design_c_coincident, design_delete). The Surface drawer BUTTON deliberately keeps design_surface; only its entries may reuse the solid glyphs, because a menu row carries its own text label while two adjacent toolbar buttons do not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
77f2f4d4ad |
Design tab: datum and curve tools were rejected by the solid-preview check
refresh_preview() exempted only Sketch and Plane from the ghost-preview path. Axis, CoordSys, Helix and Project produce no solid either, so they fell through to it, found nothing, and reported "invalid: preview produced no geometry" — which also DISABLED Confirm, so all four tools were unusable rather than merely noisy. Guaranteed on an empty document; with a body present the ghost path finds something and masks it, which is why it survived until the tools were tried on a fresh project. All six non-solid tools are now exempt, each with its own ready message. The list is not a guess: recompute() skips Sketch, Helix, Plane, Axis and CoordSys outright and routes Project through apply_project(), which emits sketch entities and no solid — so the panel and the kernel now agree on exactly what is not a solid. Mate already had its own branch, since it needs Confirm gated on having two distinct CoordSys features. Introduced when Axis/CoordSys (batch 1) and Helix/Project (batch 3) were wired without extending this exemption. Confirmed fixed on hardware: Axis -> Plane Intersection with XY and XZ now resolves on an empty document, which also exercises 60b04feea1. Compiles clean; kernel untouched, suite unaffected at 143 cases / 1980 assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4d331099de |
CAD: Axis PlaneIntersection read its plane refs in the wrong index space
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>
|
||
|
|
0455b0bf96 |
Design tab: give the Surface drawer its own icon
The Surface drawer used design_extrude, the same face as the Add-material drawer, so the two buttons were indistinguishable in the feature bar. design_surface.svg is a draped patch — deliberately unlike design_plane (a flat parallelogram) and design_extrude (a box with an up-arrow) — with a faint interior rule so it reads as a skin rather than a solid face. Same visual language as the other 71: 24x24, no fill, #b6b6b6, stroke-width 0.85, round caps and joins. The five surface ENTRIES that also used design_extrude now use it too. That is not cosmetic tidying: a flyout button's face follows the last-picked entry (SetBitmap_(icon_names[i])), so changing only the drawer's default icon would have been undone the moment the user picked anything. Surface Loft keeps design_loft, which already suits it. The six rows still share one glyph between them, so they are told apart by label alone inside the flyout. Per-entry icons belong with snaporca-vrg (Draft/Shell reusing design_dressup), not here. Compiles clean; kernel untouched. Icon confirmed legible on hardware. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c565d6ba86 |
CAD: undo() never rolled back variables, so a bad one bricked the recipe
checkpoint() snapshotted `features` and undo() restored `features`, but
`variables` is a separate member of CadDocument. Every caller of the documented
checkpoint -> mutate -> recompute -> undo-on-failure pattern therefore failed to
roll a variable back: the bad value stayed in the document and every later
recompute failed, which is exactly the corruption the pattern exists to prevent.
Feature `expr` bindings were unaffected only because expr lives inside
CadFeature and rode along in the features snapshot — which is why the feature
side appeared to work.
This was a kernel gap, not a GUI one: McpControl::action_set_variable has the
same sequence and was equally broken.
The undo/redo stacks now hold a {features, variables} Snapshot. Nothing here is
serialized, so no recipe version change and no golden-fixture regeneration.
Two tests, both verified to FAIL against a faithful reproduction of the bug
(undo() leaving `variables` untouched) at test_caddocument.cpp:4420 and :4441:
one covers restoring a variable's previous value, the other covers removing a
variable that did not exist before the checkpoint. Worth recording that the
first mutation attempt was NOT faithful — it dropped the restore but kept
std::move(variables) into the redo stack, which empties the map as a side effect
and made the second test pass for the wrong reason. A mutation has to reproduce
the original defect, not merely break the code.
Second defect, same area: undo() calls recompute(), which succeeds and clears
doc.error, so the reason an edit was rejected was destroyed before anything
could display it. Six sites — four in DesignPanel, two in McpControl — now carry
the message across the rollback. on_remove_variable additionally asserted
"referenced by a feature expression" as fact; it now offers that as the likely
cause and appends the real error, since that diagnosis is wrong for any other
failure.
Suite 141 cases / 1972 assertions (was 139/1960). GUI compiles clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
33275f2c74 |
Design tab: sheet pickers targeted the wrong body; guard Delete Face's face list
The sheet-only pickers filtered correctly and then threw the filtering away. Their
rows are the SHEET bodies, but GetSelection() was passed straight through as an
index into m_doc.bodies. With a solid at 0 and a sheet at 1 — the normal order,
since you extrude a solid before making a surface — the single row is row 0 but
body 1, so Surface Offset and Thicken Surface targeted the SOLID. The kernel then
refused with "target is not a sheet", which reads as a kernel bug rather than a
picker bug, and the row's own label ("Body 2") disagreed with what was targeted.
Four sites per tool were wrong, including the re-edit path, which compared a body
index against the sheet-only row count and so restored the wrong row.
populate_sheet_body_choices() now carries the real body index in client data, and
two helpers make the row/body distinction hard to get wrong again:
sheet_choice_body() reads it back, select_sheet_choice() finds the row holding a
given body. No caller touches GetSelection()/SetSelection() on these pickers.
This is the third instance of the same index-space confusion in this file, after
the 0-based body labels in the interference report and the Rib sketch picker. The
kernel suite cannot catch any of them: the kernel receives whatever index the GUI
computed, and its own tests pass correct ones.
Delete Face was structurally right — its picker uses the all-bodies populate, so
its indices genuinely match, and accumulation appends with a running list. Two
gaps closed: clicking "Add picked face" with nothing picked was a silent no-op,
indistinguishable from a broken button, and the same face could be added twice,
putting a duplicate id into delete_faces that the defeaturing has no reason to
cope with. Re-adding is now a no-op with a message, not an error.
Both confirmed working on hardware. Kernel untouched: 139 cases / 1960 assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
8621f1168e |
Design tab: show/hide the printer bed; give Placement its own toolbar slot
Port of snaporca ca25352d74. Hand-applied rather than cherry-picked: unlike the
Phase B commits, which only touched DesignPanel.{cpp,hpp} and transfer verbatim,
this one reaches into GLCanvas3D and DesignCanvas, where the forks genuinely
differ — mainline passes m_show_world_axes to _render_bed where Snapmaker passes
a local show_axes, and the surrounding code sits ~90 lines further down. git am
refused, correctly; the six edits were applied against mainline's own context and
the DesignPanel half came across as a patch.
Bed toggle: a "Bed" checkbox in the document/view row, on by default, in that row
rather than in a card because a view option must stay reachable with no tool open.
It drives GLCanvas3D::m_show_bed (default true, so Prepare and Preview are
untouched) and gates _render_platelist as well as _render_bed — hiding the bed
while leaving its grid and outline floating would read as a rendering fault.
Bound to wxEVT_TOGGLEBUTTON, not wxEVT_CHECKBOX: Orca's CheckBox derives from
wxBitmapToggleButton, so a wxEVT_CHECKBOX handler never fires.
Also gives the Placement drawer its own "placement" toolbar slot. put("place")
already holds the Place-on-Face button, and put() formats slot item 0 as the
control and later items as its chevron, so sharing the slot bottom-aligned the
drawer's button like a chevron.
196/196 targets, 0 compile errors, orca-slicer links (165 MB). The build script
still exits non-zero at the AppImage bundling step on libpython3.12.so.1.0 —
that is snaporca-96t, packaging only, and does not affect the binary.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
d4904b8e2e |
Design tab: stop eight tool cards rendering at startup; regroup the drawers
The sidebar opened with eight tool cards stacked in it — Transform, Mirror,
Thicken, Rib, Project, Delete Face, Helix and Mate. A card added to the cards
sizer is visible until something hides it, and close_tool()'s hide-all only
runs on a tool SWITCH, so anything missing from the construction-time hide
block is on screen from the moment the tab opens. Those eight were wired into
close_tool() but never added here. All 37 cards are now hidden at startup.
Worth stating because it invalidates a check I ran while diagnosing this: every
card IS hidden somewhere in the file, so grepping for "hidden anywhere" says
nothing. The block that matters is the one in the constructor.
Second, the drawers mixed unrelated operations, and two group tooltips no
longer described their contents — Dress-up listed eight tools spanning three
different kinds of operation, and Add material still claimed to hold only
extrude/revolve/sweep/loft after Thicken and Rib were added to it.
One concept per drawer now:
Add material extrude, revolve, sweep, loft, thicken, rib
-> grows new solid material, whether from a profile, a face or
a line
Surface unchanged; already coherent
Datum / Curve plane, axis, coord sys, helix, PROJECT
-> reference geometry and derived curves. Project consumes a
body but PRODUCES sketch entities, so it is curve creation,
not a finishing operation
Placement TRANSFORM, MIRROR, MATE (new)
-> moves a body without changing its shape; a mate places one
body relative to another
Dress-up fillet/chamfer, draft, shell, delete face
-> finishing on the faces and edges of an existing solid
Hole / thread unchanged
The new drawer costs no toolbar width: the layout order already contained an
empty put("place") slot between "material" and "plane" with nothing registered
to it. Shift+Y and Shift+Z follow Transform and Mirror; every tool still
appears exactly once.
Compiles clean; kernel untouched, so the suite is unaffected at 139 cases /
1960 assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
6372b3b505 |
Design tab: M6 variables panel + per-feature expression bindings
M6 landed the kernel side as two plain public maps — CadDocument::variables and CadFeature::expr — reachable only through MCP's set_variable / set_feature_expr. Nothing in the GUI could create a variable, so the parametric layer was unreachable from the Design tab. Variables get a wxListCtrl (name, expression) with add/edit/remove below the feature tree, since they are document-scope and must not live in a card that only exists while a tool is open. Expression bindings get one generic row in the feature-edit path — a field-name combo plus an expression box — rather than an extra control on each of 32 cards. Every mutation copies McpControl's sequence exactly, and the rollback is the part that matters: checkpoint, mutate, recompute, undo() on failure. Without it one typo leaves the recipe permanently unrecomputable, since load() replays the whole list. Removing a variable a feature still references fails that recompute, so it reports the reference rather than a bare evaluation error. The field-name combo is deliberately editable: only 11 feature types get a curated field list, and free text is what makes the other 21 reachable. That is safe because assign_field() throws "unknown parameter: <name>" for anything it does not know, inside recompute()'s try block — so a wrong name gives a clear message and a rollback, never a silently dead binding. Two fixes on top of the generated wiring: - make_combo() passes wxCB_READONLY, under which Orca's ComboBox HIDES its text ctrl (ComboBox.cpp:51). There is no SetEditable() to undo that, so the field combo is constructed directly with style 0; that shows the ctrl with wxTE_PROCESS_ENTER and makes GetValue() return typed text. - the field-list helper had been made a file-static function taking DesignPanel::Tool, which required moving Tool out of private and into the public API. It is now a private static member instead: 32 values of internal card state should not be published to satisfy a signature. Compiles clean (0 errors); kernel suite unchanged at 139 cases / 1960 assertions. Phase B is complete on this fork — all 16 previously GUI-less tools plus the variables panel. Not yet exercised on a display. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |