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.
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.
He merged upstream main into the PR branch himself on 2026-08-13. Taking it into
the local branch rather than force-pushing over it: the fork copy is what PR
#15238 shows, and discarding a maintainer's merge to make my own push
fast-forward would be both rude and a loss of 130 upstream commits.
Brings the branch far closer to main than the 2026-07-24 merge-base the PR body
describes, which is most of what snaporca-36u9 was filed for.
Every version bump so far has permanently orphaned every project saved before
it. deserialize_recipe refused anything that was not exactly the current
version, and with no migration path v2 and v3 projects are unopenable today —
the 3MF still carries the mesh, so the user gets a frozen solid and no feature
history, which is the whole point of the subsystem silently absent.
The cause was the shape of the data, not the gate. save/load is one flat
symmetric list of ~90 fields with no framing, so a reader has no way to know
where a feature ends unless it agrees on every field.
Each feature is now written as its own cereal stream behind a length prefix, and
the same few lines handle both directions of mismatch. Older file, newer build:
the sub-stream ends early, the read throws, and the fields already assigned are
kept while the rest default — cereal assigns sequentially, so a mid-list throw
leaves the earlier fields set, and that is what makes this work. Newer file,
older build: the sub-stream holds more bytes than the reader knows; it reads what
it knows and stops, and the outer stream is untouched because the length prefix
was consumed in full. A field a project predates is not a corrupt project, so
neither case is an error.
v4 keeps its own pre-framing flat path and opens exactly as before —
cad_recipe_v4.bin is untouched and now serves as the witness for that. v2 and v3
stay refused, by name: their field lists no longer exist in this code. This fixes
the future, not the past, and the comment says so rather than implying otherwise.
From here a new field only needs appending to save/load — no bump, no orphaned
projects. That removes the cost that had blocked snaporca-44m and snaporca-dgv.
The helix round-trip test was reading the blob back flat, reaching into the
format instead of through it; framing necessarily breaks that, so it now goes
through deserialize_recipe, which is a stronger assertion than it made before.
Every field check it carried is unchanged.
Tests: four new [CadDocument][recipe] cases, including the one the change exists
for — a deliberately truncated feature blob must LOAD, keeping what it could read.
Suite 177 -> 181 cases, 2366 -> 2411 assertions.
snaporca-2txy.
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.
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.
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 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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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>
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>
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>
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>
* Move Generic vendor above Bambu vendor in AMS material setting.
* Remove hardcoded sorted_names. Alphabetically sort Bambu with all vendors
* Fix sorting with case insensitive comparison
* Use arithmetic to get rank distance because priorities are stored in a vector. This lets us remove the <interator> include.
* Move currently active filaments added to the Prepare sidebar to the top of the AMS Material Selection combo box.
It is likely the user wants to set the material to the currently active filament.
* Reduce logging verbosity.
* Refactor current active preset filament finding to find nested preset inheritance.
* Initialize pointer to null before usage.
* Remove old commit code
* Remove new line
---------
Co-authored-by: Ioannis Giannakas <59056762+igiannakas@users.noreply.github.com>
Co-authored-by: yw4z <ywsyildiz@gmail.com>
* Fix detached copies of system presets
* Clarify detached preset compatibility
* Show unique preset state in save dialog
* Update SavePresetDialog.cpp
---------
Co-authored-by: yw4z <ywsyildiz@gmail.com>
* fix(GUI): honor "Ignore" when layer height exceeds the configured maximum
Entering a layer height above the printer's max_layer_height on the Print
Settings tab fired two guards in sequence. Tab::on_value_change prompts
"...Adjust to the set range automatically?" with an Adjust/Ignore choice, and
ConfigManipulation::update_print_fff_config then showed an OK-only "Too large
layer height. Reset to X" dialog whose result was never checked, so it always
reset the value. The second guard overrode the user's "Ignore", resetting the
layer height regardless.
Changes:
- Extract the shared Adjust/Ignore dialog into
ConfigManipulation::layer_height_out_of_range_dialog, reused by the tab
(Tab::on_value_change) and the per-object/part settings panels. The dialog now
names the value it will clamp to and reads correctly for too-low as well as
too-high.
- The tab/plate path is already covered by Tab::on_value_change, so the
duplicate reset is dropped from update_print_fff_config.
- The per-object/part panels have no on_value_change hook, so add
ConfigManipulation::check_object_layer_height and call it from the object
settings update paths, gated on the edited option (changed_opt_key ==
"layer_height"). It prompts once per layer-height edit and does not re-prompt
when unrelated object settings change after the user chose "Ignore".
Fixes#14214
* refactor(GUI): unify the layer-height range check across tab and object panels
Copilot review of #14369 noted that the per-object check only guarded the
max at extruder 0 and skipped the too-low case, diverging from the tab.
Move the whole range check into ConfigManipulation::check_layer_height,
used by both Tab::on_value_change and the per-object/part panels. It takes
the widest [min, max] window across the printer's extruders, offers
Adjust/Ignore in both directions, and resets a near-zero value. The tab's
inline block collapses to one call, dropping the duplicated limit logic.
* fix(GUI): only enforce layer-height limits that are actually set
max_layer_height defaults to 0 (unset), so the unconditional range check
offered to clamp any layer height to 0 on presets that don't define it.
Guard each branch (near-zero, too-high, too-low) so an unset limit disables
that direction; the slice-time nozzle-diameter check still applies.
Also run check_layer_height before update_print_fff_config in the object
panels so a near-zero per-object value prompts the same way the tab does,
with update_print_fff_config's fallback still covering the no-minimum case.
_update_select_plate_toolbar_stats_item(true) runs from
on_action_slice_all before the select-plate toolbar has necessarily been
initialized. m_all_plates_stats_item is only assigned in
_init_select_plate_toolbar, so slicing a multi-plate project shortly
after startup (before the Preview tab has rendered) leaves the pointer
null while show_stats_item is true, and the branch dereferences it,
crashing with SIGSEGV.
Every other dereference of this pointer already null-checks it. Add the
same check here so the all-plates stats item is left unselected until the
toolbar is initialized instead of crashing.
Fixes#15116
min_length_factor and is_top_or_bottom_layer had no default initializers, and the FillConcentric/FillConcentricInternal callers never set them, so WallToolPaths::removeSmallLines() thresholded on stack garbage. Which short extrusion lines it dropped then depended on memory layout, so concentric solid-infill output was nondeterministic between runs and across machines. Give every member a default, matching the adjacent FillParams. The perimeter path was already fine because it builds the struct via make_paths_params().
Fix Windows crash in Replace all with 3D file
Keep the replacement result message as wxString and substitute the volume
name directly.
On Windows, wxString::ToStdString() cannot encode the Unicode status icon
through the active ANSI code page and returns an empty string. Passing that
empty string to boost::format with a volume-name argument throws
boost::too_many_args and exits OrcaSlicer.
Prime towers reserved depth from the prime volume alone, ignoring the flush
matrix: rib-wall towers in both the engine and the preview, and rectangle and
cone towers in the preview, which never carried the flush-aware estimate the
engine already used. The preview also read the print preset, which does not
carry the printer- and filament-scope keys the estimate needs and so silently
fell back to defaults. On multi-nozzle printers the flush matrix, which holds
one block per nozzle, was additionally read as a single block. The tower could
come out too small for the purge it has to hold.
The flush-based estimate also skipped the height-based minimum depth that the
prime-volume one applies, so low-flush prints could estimate a tower shallower
than the one that actually gets built.
The placement clamps and the tower-approach router both stood in the bed's
bounding box for the bed itself, so on a delta or hexagonal bed the prime tower
could be parked in a corner that does not exist and the nozzle could be routed
across it. Both now test the real printable outline, slicing reports a tower
that does not fit instead of printing it off the bed, and a tower parked near an
edge is routed along the clamped side rather than falling back to a straight
line across the tower.
Also fixes the placement validation rotating the tower hull by degrees read as
radians about the plate origin, and never rotating the generated tower footprint
at all.
Adds a printer option that picks up the new tool without a blocking temperature
wait, travels to the wipe tower, and waits there right before purging, parked
beside the tower so the ooze from the heat-up lands next to it rather than on the
model. The incoming filament's target is raised ahead of the tool change, so the
heat-up overlaps both the change itself and the travel to the tower.
Off by default, and only offered for multi-extruder printers using a Type 2 wipe
tower; the generic toolchanger profile enables it.
* fixes: memcpy(...) writing to an object of type OrientParams with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
* review result: replaces anonymous namespace with static
Tommaso Bianchi