mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-09-18 22:42:37 +00:00
920f0bd1263d5813b8e4ce2a584d63fb04ff3e13
30319
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Tommaso BianchiandClaude Opus 5
|
920f0bd126 |
CAD: keep the pick trace, stop paying for it when it is off (snaporca-txp8)
Ported from snaporca 3710d34568. See that commit for the full rationale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tommaso BianchiandClaude Opus 5
|
457610108e |
CAD tests: pin the sheet-body mass properties with the rig's own numbers (snaporca-lu27)
Ported from snaporca 35befd965c. See that commit for the full rationale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tommaso BianchiandClaude Opus 5
|
4c8c93b512 |
Design: keep the inline dimension frame mapped across queued fields (snaporca-p8uw)
Ported from snaporca e4e0e21581. See that commit for the full analysis. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tommaso Bianchi
|
d9abadc88f |
Revert "CAD: type a sketch dimension without clicking the field first"
This reverts commit
|
||
|
Tommaso BianchiandClaude Opus 5
|
ab15f386e4 |
CAD: type a sketch dimension without clicking the field first
On a Wayland session the in-canvas value field never took the keyboard focus, so after drawing a rectangle the first keystrokes went nowhere and the field had to be clicked before a number could be typed. open() already did Show, Raise, SetFocus on both frame and control, SelectAll, and re-asserted all of it in a CallAfter — none of it works here, and no amount of re-asserting would: under Wayland a client cannot focus itself, and mutter ignores gtk_window_present() without an activation token as focus-stealing prevention. The earlier fix recorded in this file (dropping wxFRAME_FLOAT_ON_PARENT, whose GTK _UTILITY_ hint made an xrdp session refuse focus) addressed a different compositor. Stop needing WM focus. The canvas keeps the focus and feeds the field: SketchInlineEditor::feed_key() types into the control directly — Enter commits, Esc cancels, Backspace/Delete edit, digits and '-' '.' ',' are accepted, and anything else is handed back so a stray letter cannot vanish into a numeric field. A m_fresh flag reproduces the SelectAll semantics the field already had, so the first digit replaces the prefill. It returns false when the control genuinely holds the focus, so X11 keeps wx's normal routing and no character is typed twice. The CHAR_HOOK gates on the editor's own is_open(), NOT on inline_busy(). inline_busy is a freeze flag for the sketch tool: cleared on commit, re-set only when the next queued field opens, with a CallAfter between them. Gating on it left a window where the field was on screen and the flag was false — typing worked for a rectangle's Width and not its Height. VERIFIED at the machine on behemoth: typing the first dimension directly, with no click, works. NOT yet confirmed: the Width -> Height handover; the is_open() gate is diagnosed from the handover code, not observed. The hook's SNAPORCA_KEYTRACE=1 switch logs each key with the focused widget if it needs chasing further. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tommaso BianchiandClaude Opus 5
|
614824ce89 |
CAD: right-click in a sketch offers verbs for what is selected, instead of for nothing
Select a line in a sketch, right-click, and every sketch verb was greyed: Trim,
Extend, Fillet, Chamfer, Offset, Mirror, the arrays, Constrain. The menu was
right and the selection was gone — two independent faults, each of which hid
the other.
FIRST, the Select-mode RightDown branch called clear_selection() before handing
the click back. Handing it back is correct: the m_right_consumed flag means "the
tool USED this right-click", and a plain right-click is not a gesture
terminator, so the offer should open. Clearing first is not: the offer describes
WHAT IS SELECTED, so wiping the selection guaranteed it could only ever describe
nothing. Deselection keeps its own gesture — left-click on empty space, a few
lines above in the same handler.
SECOND, offer_selection_kind() returned SkNone for every sketch state. The offer
table has always carried verbs for a selected line, arc, point or pair, but
nothing ever RETURNED those kinds, so fourteen rows were gated on selection bits
no code path could set. Classify the selection instead: SkLine / SkArc / SkPoint
/ Sk2Ent, via a first_selected_type() accessor on the tool and two forwarders on
the canvas.
Either fix alone measures as a failure — the classification is handed an empty
selection, or the preserved selection has no kind to match — which is why both
land together.
This is the second half of the report behind
|
||
|
Tommaso BianchiandClaude Opus 5
|
eb52972a8e |
CAD: the offer menu speaks one language, not two
On a non-English desktop the offer menu came out mixed: "Create / Add material /
Rimuovi / Fillet / chamfer / draft / Repeat / Transform / Reference / Modify",
and under Modify, "Elimina" beside "Constrain sketch".
Nothing was mistranslated. The row names went through a bare wxGetTranslation(),
which searches EVERY loaded catalogue — including wxWidgets' own wxstd. That
catalogue is loaded in the desktop's language whether or not the application has
one, and it happens to contain exactly two of our eight row names:
wxstd it: 'Remove' -> 'Rimuovi', 'Delete' -> 'Elimina'
Create, Add material, Repeat, Transform, Reference and Modify are not wx
vocabulary, so they stayed English. Two words in one language, six in another,
in the same menu — and the same trap is set for every other locale wx ships:
Supprimer, Löschen, Eliminar.
Name the domain: wxGetTranslation(s, SLIC3R_APP_KEY). These strings are now
translated by our own catalogue or not at all, which is consistent either way.
Left deliberately alone: the accelerator still renders as "Canc" rather than
"Del" on an Italian system. That is wx naming the physical key, and on an
Italian keyboard the key really is marked Canc — telling that user to press
"Del" would name a key they do not have.
Verified on the rig with LANG=it_IT: the menu now reads Remove and Delete, and
the submenu shows "Delete Canc" beside "Constrain sketch".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Tommaso BianchiandClaude Opus 5
|
3eb6e5d608 |
CAD: constraints are reachable from the offer menu, not only from a toolbar icon
A user evaluating the Design tab against Onshape reported that "adding constraints seems to be missing" — with nineteen constraint types and a solver shipped behind it (OrcaSlicer PR #15238, exussum12). They were not wrong about what they could see. The only ways in were an icon-only toolbar button whose tooltip you have to hover to read, and an offer row gated on sketch_mode with a sketch ENTITY selected, filed under "Reference". Right after finishing a sketch — the moment you want to constrain it — neither was in front of the user, so a shipped headline feature read as absent. Add a model-mode row: "Constrain sketch", offered under Modify when a sketch region is selected, routed through the new btn:constrain verb action. on_begin_constrain() also gains a fallback to m_sel_sketch_feat. The offer reaches it from a SkLoop selection, which carries no TREE selection, and the function read only tree_selection() — so the new row would have answered "Select a sketch in the tree first" about a sketch the user had visibly selected. It now adopts the region's owning sketch and syncs the tree to match. Verified on the rig: draw a rectangle, finish the sketch, click the region, right-click -> Modify -> "Constrain sketch" enters Constrain mode with "Pick 1-2 lines, then a constraint" and the Constraints (8) card listing the sketch's inferred constraints. That path did not exist before. Does NOT address the other half of the report: there is still no Pierce constraint, so a sweep profile cannot be tied to its path. Tracked separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tommaso BianchiandClaude Opus 5
|
9013f530fa |
CAD: a sheet body reports no volume, instead of a confident wrong one
mass_properties on an open shell returned volume 96000 with an inertia diagonal of [-4.2e7, -4.2e7, -6.9e7] for a 60x60x40 four-walled box — negative principal moments, which no real body can have. BRepGProp::VolumeProperties integrates the divergence theorem over whatever faces exist; on an open shell that is not a volume at all, and the old code hid the only obvious tell by taking std::abs() of the mass. "valid: true" then asserted the number was trustworthy. This matters because mass_properties is what an agent or a user reaches for to confirm a cut removed the right material. Silent nonsense there means the check passes on garbage. MassProps gains is_solid. For a sheet we compute surface area only — that stays exact — and report volume 0 with the inertia left zeroed. The MCP verb returns is_solid plus a note saying volume and inertia are not defined for an open shell; the GUI's Mass command says "sheet body — N cm² of surface, no volume" rather than quoting material that is not there. Verified on the rig: the sheet now returns volume 0.0, surface_area 9600.0 (exactly 4 x 60 x 40), is_solid false. The solid controls are unchanged and exact — a 60 mm cube reports 216000.0 and 21600.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tommaso BianchiandClaude Opus 5
|
1545fb7946 |
CAD: an armed Plane or Axis pick captures the face, instead of escalating to the body
Clicking a face that already happened to be selected, while a Plane or Axis pick was armed, read as a repeat pick: the click escalated to "whole body", the capture was lost, and the card's label stayed "(none)" with nothing on screen to explain it. On a cube it is easy to hit — the face under the cursor is often the one already selected from the previous step. The capture path in on_solid_picked already restores the flag for all three tools, and reset_plane_refs()/reset_axis_refs() restore it when a pick is abandoned — both were written as if the arm side disabled escalation. Only CoordSys actually did (that was snaporca-u0wd). Plane and Axis never had it. Verified on the rig: Midplane on a 60 mm cube now captures Face A (#5, top) and Face B (#3, side) on the FIRST click each, and the resulting plane renders as the 45-degree bisector between them, which is what a midplane of two perpendicular faces should be. Before this, the first pick escalated to the body and Face A stayed "(none)". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tommaso BianchiandClaude Opus 5
|
f6f2edb906 |
CAD: the Plane card refuses a method it cannot build, instead of quietly building another one
Every method in CadDocument's plane dispatch falls back to offset_angle_plane() when its references are missing. Picking Tangent and confirming with nothing selected therefore produced an OFFSET plane, announced as "Plane added — pick it as a sketch plane". The user asked for one construction and silently received a different one, with nothing on screen to reveal the substitution. Validate at the GUI boundary instead: Angle needs an edge, Midplane two faces (and not the same face twice — that yields a plane coincident with the face, which is well-defined and useless), Tangent a face, Two-edges two edges. Offset and Coincident are unchanged: both are meaningful with no reference, since they fall back to the base plane by design. on_add_plane() now returns false when it refuses, and confirm_tool() skips close_tool() in that case — a refusal that also threw away the picks the user had already made would be worse than the bug. The kernel keeps fallback_offset(): it must return something. It should just never be reachable from a user gesture without a warning. Verified on the Xvfb rig: Tangent with no pick refuses and creates no feature (it created one before), the card stays open with the type preserved, Midplane with no faces refuses with its own message, and Offset with no picks still creates a plane as it always did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tommaso Bianchi
|
9a5e9dfc36 |
CAD: the wheel scrolls the card panel, it does not edit the field under it
wxSpinCtrlDouble takes the mouse wheel whenever the pointer is over it. A card taller than the panel therefore could not be scrolled past without silently incrementing whatever field the cursor happened to be over — measured on the rig: eight notches over the Coord Sys card turned an X hint from 1,00 into 6,00 while the panel did not move at all. The same gesture over an Extrude distance, a Hole diameter or a mate Offset is a silent model change made by someone who believed they were navigating, and nothing on screen reports it. Every spin in this panel comes from one factory, so the guard goes there: an unfocused spin hands the wheel to its parent, and the scrolled cards panel gets it. A spin the user has deliberately focused still takes the wheel, which is the one case where editing is what was meant. Verified on the rig with an Extrude card: eight notches over an unfocused Distance leave it at 10,00; clicking into it first and scrolling takes it to 13,00. |
||
|
Tommaso Bianchi
|
03fb81020e |
CAD: picking a face for a Coord Sys must also make it a face-based frame
A mate between two connectors committed cleanly, recomputed without error, and moved nothing. apply_mate was reached and computed a translation of exactly (0, 0, 0). The connectors were the cause, not the mate. Picking a face stores coordsys_face and coordsys_body but left the Type combo on its default, Point (world) — and datum_frame ignores the pick entirely for that type, resolving the connector to coordsys_point, which is (0,0,0) unless the user typed otherwise. Two connectors built that way share one frame, so the mate between them is an identity transform: everything reports success and the assembly never moves. Capturing a face or an edge now switches the type to FaceAndDirection. Picking a face IS the choice of a face-based frame; asking for it twice, with no hint that the second half is required, is what made every mate a silent no-op. Verified on the rig, two bodies with a face-based connector each: before Extrude4 [-71.6, 402.6, 156.5] .. [-8.8, 412.6, 237.3] after Extrude4 [-31.4, -40.4, -10.0] .. [ 31.4, 40.4, -0.0] with the mate transform now (40.19, -196.88, 402.56) instead of (0, 0, 0). That is the first mate in this tree that assembles anything. Not the kernel: recompute applies mates exactly as preview does, proven by an A/B harness over all five kinds — the two paths give identical bounding boxes. |
||
|
Tommaso Bianchi
|
c45f84edf4 |
CAD: three Design-tab fixes — consumed holed loop, constraint rows, hover ghost
1. A consumed loop WITH HOLES was never dropped from the sketch overlay. sync_sketch_display compares each region's entity list against what a per-loop extrude stored, but rebuilt the candidate from the region's OWN entities while the extrude stores the region's entities PLUS every hole's (see selected_loop_entities). For a plate with one bore that is 4 against 5, so the match never fired and the extruded rectangle stayed drawn on top of the solid it had become. Adds region_entity_indices_with_holes, which returns the same order the extrude uses, and compares against that; a matching region now drops its holes' entities too. Hole-less regions are unaffected. This is also the artefact that made a correct plate-with-a-bore read as a plate with a plug in it during rig testing. 2. The Constraints card drew its header and its first row on top of each other. The rows and the delete buttons were parented to m_form rather than m_cards, so they were laid out in the wrong window's coordinate space and started at the card's top edge. Re-parented; nothing else about the card changed. 3. The mate hover ghost never appeared. The highlight handler asked for a repaint with request_repaint(), which only queues a Refresh — and a wxMenu popup runs its own modal loop, so the paint was not serviced until the menu closed, by which time the ghost had been dropped. Adds DesignCanvas::repaint_now(), which flushes the paint immediately, mirroring the m_status->Update() the status line in the same function already needed for the same reason. Delegated to opencode (DeepSeek V4 Pro) and reviewed by diff. Fix 1 needed an accessor on DesignSketchTool because region_loops/RegionLoop are private — that was outside the file list it was given, and it said so rather than working around it. Verified on the rig: a rectangle-plus-circle sketch extrudes to a plate with a bore and NO overlay left on top of it, and the Constraints card shows its header clear of eight readable rows. |
||
|
Tommaso Bianchi
|
9340d4c7dc |
CAD: the DoF readout comes back in Constrain mode
Leaving Sketch clears the "N degrees of freedom" line, which is right — it describes a sketch's constraint state and means nothing in Feature mode. But entering CONSTRAIN left it blank too, and Constrain is where the number is the whole point: the readout is fed only by a live solve, and no solve fires merely because the mode changed, so the line stayed empty until the user happened to change something. The solve callback now caches its last result and the labelling is split into apply_dof_status(), which set_ui_mode re-applies on entry to Constrain. Verified on the rig: a rectangle reports "4 degrees of freedom" in Sketch, and the same line is there after K enters Constrain. |
||
|
Tommaso Bianchi
|
1631963ba1 |
CAD: pick tolerances scale with the face, so a narrow face is reachable
The edge and vertex tolerances were fixed at 8 and 11 px. On a face that is barely wider than that on screen — a thin plate, or any part once you zoom out — every point on it lies within the edge budget, so the pick alternated edge and whole body and the FACE level could never be reached at all. That is not just awkward: a face pick is what gives a Coord Sys its owning body, and a mate needs one, so thin parts could not be assembled. Both tolerances are now capped at a third of the face's shorter on-screen side, measured from the edge samples the picker already walks. They only ever shrink, so a face with room keeps the full budget and nothing changes for ordinary geometry; a narrow one keeps its middle for itself. Verified on the rig on a 16.7 mm-wide plate at three zoom levels including one far enough out to make the face a thin sliver: the middle reports "face 5 selected" every time, while points near the rim still take the edge. |
||
|
Tommaso Bianchi
|
76d7dd6946 |
CAD: an armed face/edge pick must not lose its click to the body escalation
A card that asks for a face ("Click a solid FACE in the viewport") could not be
satisfied. Clicking the same sub-element twice deliberately escalates to the
whole body — right for free picking, wrong here: the user clicks the very face
the card is pointing at, the escalation turns it into a whole-body pick, and the
armed capture rejects it and leaves "(none)". Both orders failed, so a
face-based Coord Sys was reachable only by accident of ordering. That mattered
beyond the card: a mate needs a connector with an owning body, and a face or
edge pick is the only thing that sets one.
Adds DesignSketchTool::set_escalate_on_repick, off while the Plane, Axis or
CoordSys card has a pick armed and back on as soon as it is captured or
abandoned. While armed, clicking a face means "this face", which is what the
prompt already says.
Verified on the rig: arm Pick Face, click the face, and it reads "#5" on the
first click. With nothing armed the escalation still alternates whole <-> face
as before.
|
||
|
Tommaso Bianchi
|
d8103f794b |
CAD: a mate needs B to have a body, so stop offering one when it does not
The mate palette called all five kinds viable on connectors created as Point(world), which belong to no body. Clicking one built the Mate feature and the RECOMPUTE then failed with "mate: mate_cs_b has no associated body" — the refusal arrived one step too late, after the feature was already in the tree, and the user is told about it by an error line rather than by the palette that offered the thing. mate_options now checks the same two conditions the apply path throws on: B must have a body (B is the connector whose body moves; A is the fixed reference and needs none), and that body must still resolve. Either way all five kinds go non-viable with a reason, so the offer and the kernel cannot disagree. Verified on the rig: with two Point(world) connectors, every row is now dimmed and reads "connector B is not attached to a body — a mate moves B's body". That also exercises the dimmed-with-a-reason presentation for the first time, which until now had nothing to show because every kind was always viable. Tests: a new [mate] case covering no body, a body that no longer resolves, and the revival once B is given one. One existing case needed its setup widened rather than its assertion weakened: "an unrecorded fingerprint does not make a type non-viable" built two body-less connectors, so it was asserting a side effect of the old permissiveness instead of the property it is named for. Its connectors now have a body, leaving the missing fingerprint as the only variable. Full [CadDocument] suite: 2458 assertions in 185 cases. |
||
|
Tommaso Bianchi
|
08ef43fad8 |
CAD: the mate palette fired nothing — two menu handlers were shadowing it
Right-clicking a document with two coordinate systems builds the mate palette exactly as designed: a separator, a disabled "Mate: A -> B" header, then five rows in fixed order. Hovering a row showed no ghost and left the previous status message on screen; clicking one created nothing at all. The palette enumerated perfectly and fired nothing, which put the whole M8 mate epic out of reach from the UI. The mate handlers were never invoked. show_offer_menu binds two pairs of handlers to the SAME wxMenu: the mate pair (by mate_base, at base + 500) and the generic verb pair. wxWidgets pushes dynamic entries to the FRONT of the handler list, so the pair bound LAST runs FIRST for every id — and the generic pair was last. A mate id lands outside the verb table's range, so both generic lambdas returned early WITHOUT e.Skip(), which wx reads as "handled", and the mate handlers behind them never saw the event. Binds the two generic handlers to the verb id range [base, base + 499] so each half only sees its own ids regardless of bind order. Verified on the rig with a purpose-built assembly (two bodies 60 mm apart, a Coord Sys on each): before, clicking Fastened produced no feature and no status change; after, it produces a Mate feature. The verb rows keep working — the same run created both coordinate systems through Reference > Coord Sys. Note the mate then fails its recompute with "mate: mate_cs_b has no associated body", because a Point(world) coordinate system belongs to no body while the palette still advertises all five kinds as viable. That is a separate defect and is filed; this commit is about the palette being reachable at all. |
||
|
Tommaso Bianchi
|
7b953d564d |
CAD: a project whose model is a recipe is not an empty file
Reopening a saved CAD design ended on a modal "The file does not contain any geometry data." warning. A CAD project legitimately carries no mesh — the model lives in the feature tree (Metadata/SnapOrca_cad.bin) until Commit to Plate — so the warning is false for one, and it is the last thing the user sees after opening a design they spent an hour on. It reads as "your work is gone" at the exact moment the recipe HAS just loaded and the Design tab is about to rehydrate it, and the main window sits disabled behind the dialog until it is dismissed. Counts a non-empty model.cad_recipe as geometry. Verified on the rig: save without Commit to Plate, reopen, and the Design tab comes up with Sketch1 -> Extrude2 -> Body 1 editable, with no dialog at all. |
||
|
Tommaso Bianchi
|
5889f6640f |
CAD: extrude a sketch region with its holes, and stop crashing at startup
A rectangle with a circle inside it, drawn in ONE sketch, could not be extruded to a plate with a bore from the GUI. Five defects were in the way. Each was found by driving the app on a headless rig and measuring the result — the code reads correctly at every one of these points, which is why they survived. 1. Wire orientation (kernel). SketchEngine::wires_to_face added every hole as wires[i].Reversed(), which is only right when the sketch happens to wind both loops the same way. A circle drawn clockwise inside a counter-clockwise rectangle came out matching the outer boundary, OCCT swept it as a SECOND contour, and the prism was the plate with its bore filled and the disc's volume counted twice. Measured: bbox 67.17 x 219.67 x 10 with volume 152088 mm3 against a solid box of 147542 — a body larger than its own bounding box, which is the signature. Holes are now added as-is and ShapeFix_Face::FixOrientation() classifies them; that is winding-independent and is the idiom make_extrude_regions already used for imported glyphs, which is why holed TEXT always extruded correctly while a holed SKETCH never did. After the fix: 142996 mm3, implied bore radius 12.03 mm against the circle drawn. 2. The live-sketch click threw the picked region away. region_at() served only as a yes/no gate and on_face_selected() carried no argument, so Extrude fell back to whichever loop the resolver found first — clicking the material of a plate-with-a-hole extruded the disc. The region is now carried through, and DesignPanel also hands it to the tool with set_loop_pick(), AFTER open_tool() because that re-derives selection state, since extrude_uses_loop() reads selected_loop_entities() and that lives on the tool. 3. region_at() had no hole awareness and no innermost preference: it returned the first polygon containing the point. It now skips a region when the point lies inside one of that region's holes, and picks the smallest containing loop, so a click in the bore selects the disc and a click on the material selects the plate. 4. Startup segfault. DesignCanvas::request_repaint probed the GL backend via OpenGLManager::get_gl_info().get_renderer() before the canvas had initialised GL — glGetString with no context current and, before init_opengl(), no loaded function pointers. Anything that asked for a repaint while the panel was still being built landed there, with no window and nothing in the log. It now bails at the top on !is_initialized() and asks for a Refresh instead. Note the crash was in the PROBE, not in render(), which already guards itself. 5. A holed sketch on a plane whose normal points -Z came out as the full box PLUS a disc — 220274 mm3 where 163726 was due (192000 + 28274). wires_to_face took a SketchPlane parameter it never used and let OCCT infer a surface from the outer wire; when the inferred normal disagreed with the sketch's, the hole classification produced no hole. Every face is now built on the sketch's own gp_Pln. Also in this change, from the same rig session: - A right-click that only clears the sketch selection no longer reports itself as consumed, so it stops suppressing the offer menu. With any geometry in a live sketch there was no menu route left to add a second entity. - Escape no longer discards a live sketch that holds drawn geometry; it says so and keeps the work (live_sketch_has_work()). - The holed-region fill is an even-odd scanline instead of a keyhole bridge, so no corridor triangle leaks from the bore to the nearest corner. - Cyan is reserved for the selection: an unselected region no longer wears a shade one step off the selected one. - The origin planes follow the mode, so pressing Sketch on a document that already has a body offers them again instead of naming a plane you cannot see. Tests: three [holes] cases over add_extrude_entities asserting the plate-with-bore volume, solid and face counts on both a +Z and a -Z sketch plane, and the by-name refusal of two disjoint regions. Full [CadDocument] suite green. |
||
|
Tommaso Bianchi
|
e27a44e6ef | Merge remote-tracking branch 'prfork/cad-mainline' into cad-mainline | ||
|
Tommaso Bianchi
|
57b42bc059 |
Offer the origin planes while choosing a sketch plane, not only before the first body
Delete a sketch on a document that still has a body and you could not start a new one. Pressing Sketch said "click a face or a reference plane in the viewport" — while update_reference_planes had already called clear_base_pick, because a body existed. The instruction named something that was no longer there, and short of finding a face to click there was no way back into sketching at all. The planes are now offered when there is no solid yet OR while the UI is in Sketch mode, and set_ui_mode refreshes them so they appear the moment you press Sketch rather than at the next tree rebuild — which is not an event that pressing Sketch causes. Deliberately not always-on. m_dbp_active both RENDERS and picks, so leaving it set would float three translucent planes over every finished model. Tying them to the mode shows them exactly when they are the thing being chosen and takes them away again on Finish. Safe against stealing clicks: a base-plane pick is the last resort in on_mouse, firing only on a click that hit no geometry, so solids and committed sketches still win where they overlap. Found on the rig by Tommaso: "if i remove a sketch, i cannot create sketches anymore". |
||
|
SoftFever
|
2179f5f670 | fix build errors on Windows | ||
|
Tommaso Bianchi
|
81876a6ce6 |
Sketch regions understand holes, so the plate with the hole can be selected and extruded
A rectangle with a circle inside it extruded to a plain box. Tommaso reported it exactly right on the first attempt — "no intersection selectable, hence no plate with hole" — and it was a causal chain, not a guess. Two things were wrong and they compounded. region_loops() returned N independent filled polygons with no notion of nesting, so the only selectable things were the rectangle alone and the circle alone. The region a user actually wants — the bounded area WITH its hole — did not exist to be pointed at. Worse, the first polygon containing the click won, so clicking inside the circle selected the rectangle. And extrude_uses_loop() hands selected_loop_entities() to add_extrude_entities, which copied only that one loop's entities into the feature. So the circle never reached the kernel, build_sketch_face saw a single loop, and the multi-loop path added in 5c4ced91e7 never ran. Proven from his saved project: Sketch1 held 5 entities, Extrude2 held 4, and the committed mesh was 8 vertices — a box of 260.40 x 220.91 x 10.00. Eight vertices cannot describe a bore. A RegionLoop now carries the loops nested inside it. Containment is decided by testing one vertex, which is sufficient because loops in a well-formed sketch do not cross, and each loop is assigned to the SMALLEST loop containing it so a hole belongs to the region that actually bounds it. Picking respects holes: a click in the plate selects the plate, a click in the bore selects the disc. The selection hands over the region's own entities plus its holes', which is what finally reaches the kernel. The highlight lights the holes with their region, because it has to show what will be extruded. The status line said "Loop selected"; it now says "Region selected". What is selected is a bounded area that may contain holes, not a single closed curve — the old wording described the old, broken behaviour. snaporca-txp8, and it is what makes snaporca-88v reachable from the GUI at all: the kernel could build the holed face all along (verified on his own recipe: 2 closed loops, wires_to_face OK, area 74812.119 mm2), but nothing could ask it to. Reviewed and compiled (RC=0). NOT exercised — the rig check is the point. |
||
|
Tommaso Bianchi
|
b80f4e3036 |
Persist the CAD recipe on every save, not only on Commit to Plate
Modelling in the Design tab and pressing Ctrl+S saved a project with no feature history at all, and the app reported success. Found on the rig: a project saved after drawing a rectangle and a circle contained twelve archive entries, none of them Metadata/SnapOrca_cad.bin, and a 3dmodel.model with zero vertices. plater->model().cad_recipe was assigned in exactly one place — on_commit(), immediately after load_mesh_object. The 3MF exporter was never at fault: it faithfully wrote whatever the Model held, and on a save that had not gone through Commit to Plate that string had never been set. The recipe reached the Model only as a side effect of a different user action. It now tracks the document instead. sync_recipe_to_model() is called after a successful recompute, after tree edits (deletes, reorders and suppressions bypass recompute_guarded), and from on_commit, which delegates rather than repeating the rule. Only on success — a failed recompute leaves the document mid-edit, and persisting that would save a model the user never had. An empty document still clears it, so a non-CAD project carries no stale recipe. Doing it here rather than in the save path is deliberate: Ctrl+S, Save As, autosave and crash recovery all read model.cad_recipe, so keeping it current after each change makes every one of them correct at once, instead of teaching each save path to ask the Design tab. Commit to Plate means "send this to the slicer" — making saving depend on it was the bug, not the cure. Cost is one serialization per recompute, tens of KB against an OCCT rebuild that has just run. Why nothing caught it: the kernel round-trip tests serialize a CadDocument directly, and the 3MF tests exercise the exporter with a recipe already present. Neither can observe that the GUI never populates it, and every save in testing happened to follow a Commit to Plate. snaporca-vjk5. Reviewed and compiled (RC=0); persistence NOT yet confirmed on the rig — that check is the reason the issue stays open. |
||
|
SoftFever
|
6fd425505e | fix build error | ||
|
Tommaso Bianchi
|
56eebe3398 |
kernel-test: stop configuring the GUI, which the kernel suite never needed
This script builds only libslic3r_tests, which links libslic3r and no GUI code — but cmake still processed the whole if(SLIC3R_GUI) block and every find_package inside it, so the kernel suite silently depended on the GUI's dependency set. That came due the moment upstream added wxInspector as a REQUIRED find_package: the orcacad-deps image predates it, so configure died pointing at src/CMakeLists.txt:92 with nothing about the kernel having changed. Turning the block off is not a workaround for that one dependency — it is the suite finally declaring what it actually needs, so the next GUI-side dependency added upstream cannot break it either. Surfaced by taking SoftFever's merge of main into the PR branch. |
||
|
Tommaso Bianchi
|
358c331cc6 |
Merge SoftFever's main-into-cad-mainline update
He merged upstream main into the PR branch himself on 2026-08-13. Taking it into the local branch rather than force-pushing over it: the fork copy is what PR #15238 shows, and discarding a maintainer's merge to make my own push fast-forward would be both rude and a loss of 130 upstream commits. Brings the branch far closer to main than the 2026-07-24 merge-base the PR body describes, which is most of what snaporca-36u9 was filed for. |
||
|
Tommaso Bianchi
|
0e7fcb3daf |
Recipe v5: length-frame every feature, so the format stops orphaning projects
Every version bump so far has permanently orphaned every project saved before it. deserialize_recipe refused anything that was not exactly the current version, and with no migration path v2 and v3 projects are unopenable today — the 3MF still carries the mesh, so the user gets a frozen solid and no feature history, which is the whole point of the subsystem silently absent. The cause was the shape of the data, not the gate. save/load is one flat symmetric list of ~90 fields with no framing, so a reader has no way to know where a feature ends unless it agrees on every field. Each feature is now written as its own cereal stream behind a length prefix, and the same few lines handle both directions of mismatch. Older file, newer build: the sub-stream ends early, the read throws, and the fields already assigned are kept while the rest default — cereal assigns sequentially, so a mid-list throw leaves the earlier fields set, and that is what makes this work. Newer file, older build: the sub-stream holds more bytes than the reader knows; it reads what it knows and stops, and the outer stream is untouched because the length prefix was consumed in full. A field a project predates is not a corrupt project, so neither case is an error. v4 keeps its own pre-framing flat path and opens exactly as before — cad_recipe_v4.bin is untouched and now serves as the witness for that. v2 and v3 stay refused, by name: their field lists no longer exist in this code. This fixes the future, not the past, and the comment says so rather than implying otherwise. From here a new field only needs appending to save/load — no bump, no orphaned projects. That removes the cost that had blocked snaporca-44m and snaporca-dgv. The helix round-trip test was reading the blob back flat, reaching into the format instead of through it; framing necessarily breaks that, so it now goes through deserialize_recipe, which is a stronger assertion than it made before. Every field check it carried is unchanged. Tests: four new [CadDocument][recipe] cases, including the one the change exists for — a deliberately truncated feature blob must LOAD, keeping what it could read. Suite 177 -> 181 cases, 2366 -> 2411 assertions. snaporca-2txy. |
||
|
SoftFever
|
030e5f469e | Merge branch 'main' into cad-mainline | ||
|
Tommaso Bianchi
|
4b3ff99004 |
Project load: say WHY the CAD model could not be restored
deserialize_recipe distinguishes three cases that matter very differently to the person reading the message — saved by a NEWER build, saved by an OLDER one, or genuinely unreadable — and names the version in each. load_recipe threw all of that away and printed one generic sentence, so the user could not tell "update SnapOrca" from "your file is damaged", and had no way to find out. Same error-loss class as the 31 McpControl sites fixed in 1de72de9ed: the message existed, it was simply not passed on. The generic sentence stays as the fallback for the case where the kernel really has nothing to say. This does not make old projects loadable — that is snaporca-2txy, which the audit behind this change opened. It only stops the reason being withheld. snaporca-2txy (partial). Reviewed and compiled (RC=0), not exercised. |
||
|
Tommaso Bianchi
|
7245415af7 |
Sketch: a profile may hold more than one closed loop — a plate with a hole extrudes
entities_to_wire handled exactly two shapes of sketch: one lone Circle/Ellipse, or
any number of Line/Arc/EllipseArc/BSpline pushed into a single MakeWire. Everything
else fell off the end as a null wire, so a circle drawn inside a rectangle — the
most ordinary thing in this whole program — refused with "not supported yet". Two
separate closed polygons were quietly worse: both went into one MakeWire, which
does not mean "two loops" to OCCT.
entities_to_wires now returns one wire per loop. A Circle or Ellipse is a loop on
its own; chain entities are grouped by shared endpoints (union-find, 1e-6 in sketch
coordinates), and an open chain still comes back as a wire because a sweep path is
legitimately open. It is all-or-nothing: one loop that fails to build poisons the
whole result, because a partial profile would extrude a shape the user did not draw
— the failure 2e6a8f9e91 was written to stop.
entities_to_wire survives as a two-line wrapper returning the single wire when
there is exactly one loop and a null wire otherwise, so all nine of its call sites
keep their exact contract and Revolve/Sweep/Loft/Surface* are untouched. What a
holed profile means for each of those is a separate question.
wires_to_face takes the largest-area loop as the outer boundary and adds the rest
reversed, which is how OCCT is told a wire is a hole. Containment is CHECKED with
BRepClass_FaceClassifier, not assumed: a loop outside the largest one is a second
island, and one sketch producing several solids is a much bigger feature, so it is
refused by name ("two disjoint regions") rather than guessed at.
Only the Extrude case consumes the new face. Tapered extrudes of a holed profile
are refused — offsetting inner loops has to go the opposite way — and the guard
counts wires on the face already built rather than rebuilding every wire to ask how
many there are, which is also the more honest test: what matters is the profile
being extruded.
Tests: six new [CadDocument][sketchwire] cases, proved by VOLUME rather than by not
throwing — plate-with-hole, two holes, and two regression guards that a lone circle
and a lone polygon extrude exactly as before. Suite 177 cases / 2366 assertions.
No serialized field, recipe version untouched, golden fixtures unchanged.
snaporca-88v.
|
||
|
Tommaso Bianchi
|
ad8b5a73fe |
Hover pre-highlight: show what a click would take, before it is taken
Third and last piece of the selection model. The other two turned out to be built already — the rubber band is pick_bodies_in_rectangle and vertex picking is SolidSel::Vertex with its camera-facing square, both live — so this closes what the issue actually still described. Vertex beats edge beats face is a rule the user cannot see until after they have committed to a click. Showing the outcome under the pointer is what makes the precedence learnable at all, and is the charter's L5 read honestly: one click, one visible change means the change has to be predictable BEFORE the click, not only explicable after it. The resolution is now one function, resolve_solid_pick, const and writing only into its out-parameter. The click applies it and then runs its escalation unchanged; the hover applies nothing. Split this way the promise cannot drift from the act — a second implementation of "what is under the cursor" would eventually disagree with the first, and the disagreement would look like a picking bug rather than a duplication one. Rendering is likewise one function called twice. The pre-highlight draws first so the committed selection paints over it, and is suppressed entirely when the two are the same thing: two coats of the same colour reads as a rendering fault, and a promise about a click that would change nothing is not worth making. It is desaturated toward white rather than given its own hue — a distinct colour would read as a distinct KIND of selection, when it is the same selection one moment earlier. Two things that would have been silent bugs. The edge ribbon and the vertex square render with GL_BLEND off, so an alpha below 1 there is ignored; those two are quietened by a muted rgb and only the blended face fill takes the alpha multiplier. And the pre-highlight is cleared in clear_solid_selection, because it names a face by an index into a shape a recompute has just rebuilt — left behind, it would keep glowing on whatever now sits at that index, a real entity but not the one meant. Hover runs on plain motion only, with no button down and no band running: during a drag the pointer is doing something else and a promise about clicking would be a lie. It returns false so the event still reaches the camera — it asks for a repaint, it does not consume the gesture. snaporca-9xw. Reviewed and compiled (RC=0), not exercised. |
||
|
Tommaso Bianchi
|
498c7ff8d1 |
Pattern/Cut/Boolean: grey the button when there is no body, and say why
Tommaso reported the array controls as missing. They were not — Shift+N opens a Pattern card with every control correct — but the report was fair. With no body the button accepts the click, opens nothing, and writes its refusal somewhere other than where the click happened. From the user's seat that is indistinguishable from a dead button, and the icon is one unlabelled glyph among fourteen, which is how I mis-clicked it into Section view while reproducing this. A control that cannot act should look like it cannot act, before it is pressed. The three FEATURE buttons carrying a body-count guard — Pattern and Cut at one body, Boolean at two — are now greyed below their threshold with a tooltip naming what is missing. Only those three. The same guard shape also appears on rows INSIDE the flyouts, and those stay live: a drawer holds sketch-only entries too, so disabling the drawer would hide tools that are perfectly usable. The keyboard shortcuts keep running the guarded action rather than being gated — a key press has no greyed-out state to see, so the sentence is the only feedback there is. Re-evaluated in feed_bodies(), before its viewport early-return since this is about the toolbar and not the canvas, and once after the toolbar is built: an empty document is the state the bug was reported in and feed_bodies has not run yet on a fresh tab. snaporca-o9j. Reviewed and compiled (RC=0), not exercised. |
||
|
Tommaso Bianchi
|
13922a52b6 |
Design status: clear the DoF line on leaving sketch mode, and wrap the HUD chip
Two independent leftovers, both in the same status area. snaporca-752: the "N degrees of freedom" line described a sketch's constraint state and stayed on screen after Confirm, Cancel and the Escape downgrade, in Feature mode where it means nothing — visible in every Feature-mode screenshot of the 2026-07-27 sweep. Cleared in set_ui_mode rather than at those three exits, because that is the one place all of them pass through and a fourth exit added later would otherwise reintroduce it. Constrain mode keeps the readout: that is where the number is the whole point. snaporca-8cc: moving the status out of the panel and into the viewport HUD removed the clipping, but not the underlying problem. The chip is a top-level popup that Fit()s to its text, so a long sentence grew past the right edge of the canvas and hung over the window instead of being cut off inside it — the same silent length limit wearing a different hat. The label now wraps to the room actually available (canvas width minus the view-cube inset), which is what makes the earlier promise that "a sentence can be a sentence" true at 1366 as well as at 1920. SetLabel + Wrap + Fit are now one function called from both the text change and the placement. Wrap() rewrites the label it is handed, so it has to follow a fresh SetLabel every time, and the placement path runs on resize — a chip wrapped for the old width either overhangs a narrowed canvas or wastes a widened one. The left inset is one constant now because the wrap width and the anchor have to agree, or the chip wraps to a width it is not then given. snaporca-752, snaporca-8cc. Reviewed and compiled (RC=0), not exercised. |
||
|
Tommaso Bianchi
|
8737ff701e |
i18n: drop regenerated catalogues from the PR branch
The .pot, the Italian .po and list.txt are build product: 27,314 of the added lines in this branch were regenerated catalogues rather than code, and a reviewer running git diff --shortstat met that number before anything else. Restored to the merge-base so their diff is zero; they regenerate from source with scripts/run_gettext.sh whenever the maintainers want them refreshed. The Romanian catalogue goes with them, for a different reason: it is a complete new translation and deserves its own PR rather than riding along inside a CAD feature, where nobody qualified to review it would think to look. Nothing here changes what the Design tab does. The strings are still marked for translation in the sources; only the generated catalogues are out. |
||
|
Tommaso Bianchi
|
1a6252c88c |
Hole/Thread re-edit: restore the face latch from the feature, not from the last pick
m_hole_on_face and m_thread_on_face are cleared only by their tool's flyout and by their plane combobox, so after any on-face hole or thread the flag stays true for the rest of the session. load_feature_into_dialog restored the stored plane into the dropdown but never touched the latch, so re-editing from the feature tree ignored the plane it had just restored: hole_plane() returned the still-latched face plane, which may belong to a different face, a different body, or a body since rebuilt. Silent until snaporca-200 added the "On face" row, which then read as a confidently wrong answer rather than as nothing. The latch is now rebuilt from the stored feature, which is the only source that describes THIS hole. Not from the dropdown row: index_from_plane snaps an arbitrary face plane to the nearest XY/XZ/YZ, so driving the re-edit from the row would MOVE a hole drilled on a slanted or offset face — that was the reason the other candidate fix was rejected. is_base_plane() decides which of the two a stored plane is. It compares the origin as well as the axes (a plane parallel to XY but 12 mm up snaps to row 0 and would come back at z=0), and adds modeling_origin before comparing, because hole_plane() and thread_plane() add it to the dropdown plane before the feature stores it — a document with a shifted origin would otherwise mistake every dropdown hole for a face pick. Vector norms, not isApprox, which is relative to magnitude and useless against the zero origin. The face's (u,v) extent is not serialized, so m_hole_has_bounds is cleared: the gizmo's footprint clamp goes unbounded, which is honest, where another face's bounds are not. The label says which body the face belongs to instead of a face number the feature does not carry; "(none — uses Hole plane)" is the one thing that is definitely false there. snaporca-uif9. Reviewed and compiled (RC=0), not exercised. |
||
|
Tommaso Bianchi
|
6b3711fb08 |
Mate preview: hover a mate row and see the assembly move, commit nothing (G3)
refresh_preview() listed Tool::Mate among the features that produce no solid and cleared the ghost, with a comment saying a mate has no 3D ghost. The kernel never agreed: preview() routes a Mate candidate through apply_mate on a throwaway copy of the bodies, and build_candidate already filled the mate fields. That one early return was the whole of epic gap G3. A mate makes no NEW geometry but it MOVES a body, and the moved assembly is the ghost worth showing. Both the Mate card and the offer's mate palette now show it: hovering a palette row previews that kind, leaving the row drops it, and choosing one commits. Nothing is written to the document until the click. The committed bodies are hidden while the ghost is up — it is the whole assembly in its post-mate pose, not an added lump, so leaving them visible would draw the mated body twice and z-fight every other body against its own copy. Same reason Dressup and Draft hide them. Cleanup is after PopupMenu rather than on a close event: PopupMenu is modal, so by then the menu is gone and any command it raised has run. A flag distinguishes a ghost this menu put up from a preview that was already on screen. snaporca-b4sp. Reviewed and compiled (RC=0), not exercised. |
||
|
Tommaso Bianchi
|
8e15ad23e2 |
Mate palette: five types on the offer, dimmed with the reason, naming the pair
snaporca-lukg part B. The issue describes building a contextual viewport palette with a stable icon set, non-viable options dimmed and explained rather than hidden, and edge-aware placement. show_offer_menu() already does all three — its dead-row branch appends a disabled row with " — " and a reason, and wxMenu places itself against the screen edge. So this is not a new widget. It is one section added to that menu, fed by mate_options(). The header row names the pair: "Mate: A → B". That is epic gap G4 — the mate card is abstract dropdowns and never says which body moves. B is the connector on the body that MOVES, so B is the arrow's destination; the parameter order invites the opposite guess, which is why it is commented at the point of use. Five rows in one loop over the kernel's result, never reordered and never filtered. The palette addresses rows by position, so a shorter list would move every row below it — which is the whole argument for dimming instead of hiding. The pair comes from the Mate card's combos when that card is open, so the offer and the card cannot disagree about what they are acting on; otherwise the first two enabled connectors, which is defensible only because the header names them. An offer acting on an unnamed pair would be worse than no offer. REVIEW CATCH: the five type names arrived as an array indexed by kind, read as _L(table[i]). That compiles and is silently untranslatable — _L is a gettext macro and the extractor scans SOURCE for literals, so five strings would have shipped that are never in the catalogue. These names appear nowhere else in the tree, so that would have been their only occurrence. Now a switch of literal _L() calls. G3 INVESTIGATED, NOT BUILT, as specified. preview() DOES handle a Mate candidate: it copies the committed bodies to a temporary and routes the candidate through apply_mate on that copy, committing nothing, and build_candidate already fills the mate fields. The only blocker to a hover preview is refresh_preview()'s Tool::Mate early return, which clears the preview on the belief that a mate has no ghost. Nothing in the kernel refuses it. Filed rather than built. Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-lukg. |
||
|
Tommaso Bianchi
|
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.
|
||
|
Tommaso Bianchi
|
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. |
||
|
Tommaso Bianchi
|
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. |
||
|
Tommaso Bianchi
|
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. |
||
|
Tommaso Bianchi
|
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. |
||
|
Tommaso Bianchi
|
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.
|
||
|
Tommaso Bianchi
|
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. |
||
|
Tommaso Bianchi
|
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. |
||
|
Tommaso Bianchi
|
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. |
||
|
Tommaso Bianchi
|
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. |