Closes review comment 10.
`461c69c83e` settled this in April — IMEX internally, IDEX/IQEX as the user-facing label — but the
UI strings were never converted. Every translated string naming the feature now reads IDEX/IQEX:
41 occurrences across the printer and process option labels and tooltips, the modes editor, the
plate mode indicator, the pre-slice warnings, the placement refusals and the slicing errors. The
reviewer listed eight; the rest were in the same class.
Nothing else moves. The config keys keep the `imex_` spelling — `is_imex`, `imex_mode_names`,
`imex_parallel_mode` and the rest are on-disk format in existing printer presets and 3MF projects,
so renaming them would break every profile and project already saved. C++ identifiers, filenames,
comments and test names keep IMEX as well: it stays the internal name of the subsystem, which is
what covers the topology space (one gantry with 2-4 tools, 2x1 and 2x2 grids) that neither acronym
names on its own. Where a tooltip quotes a key, the key spelling is preserved and only the feature
word around it changed.
The `is_imex` tooltip is reworded rather than substituted: it already named the hardware families
parenthetically, so a literal replacement would have said IDEX/IQEX twice in one sentence.
No translation impact — no IMEX string had reached OrcaSlicer.pot or any catalogue, so there is
nothing to migrate. One test asserted on the old error text and now matches the new one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes review comments 13, 14 and 15, and adds the test for a shipped-profile
regression that nothing guarded.
- 14 and 15: compute_imex_slice_offset had eight tests on the calculation and none
on the result, which is the whole firmware-managed path. test_imex_slice_offset
now covers the derivation end (which config produces a non-zero offset, and that
it is plate-local rather than moving with the plate origin -- the bug that
shifted every plate after the first) and the consumption end (emitted
coordinates and first_layer_print_min/max both move by the derived amount).
The first_layer case also cross-checks the two consumers against each other: the
declared bounds must keep the same relationship to the emitted toolpaths in both
frames, which fails if exactly one of them is shifted. It deliberately does not
pin the size of that gap -- it is 2.225 mm here, set by the wall generator, the
same with no offset at all, and pinning it would fail on an unrelated change.
- 13: nothing exercised the imex_mode / imex_mode_index / imex_mode_gcode
placeholders or the {global} flow into machine_start_gcode that their ordering
exists to guarantee. Seven cases now do, including the ordering itself -- the
mode script declares a global and machine_start_gcode reads it back, so moving
the mode processing later leaves the variable undefined and fails the export --
plus the inert cases (Primary mode, and a printer with the table filled in but
is_imex off). All matching is whole-line, because the config block the exporter
appends repeats machine_start_gcode verbatim and would make substring checks
meaningless.
- New: GCodeWriter passes this->config.is_imex.value into the heater remap, and
nothing tested that it passes the flag rather than a constant. Hardcode true
there and the whole suite stays green while fdm_bbl_3dp_002_common, which ships
physical_extruder_map [1,0], starts sending filament 0's M104/M109 to heater 1.
The new case runs a two-nozzle non-IMEX printer with that map and asserts each
filament's temperature reaches only its own tool. It uses idle_temperature via
ooze prevention rather than nozzle_temperature: keys in
filament_options_with_variant are re-indexed per filament by variant slot at
apply time, and this harness pins nozzle_diameter to one value, so every filament
resolves to the same slot and the temperatures stop telling the heads apart.
Also fixes two weaknesses in tests added earlier in this branch: an assertion that
would have been prefix-satisfied by the very routing it was meant to exclude, and
a whole-file command comparison between two slices, which this slicer's output is
not stable enough to support.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes review comment 1.
imex_parallel_mode and imex_head_filament_map were streamed raw into XML attribute
values, while every other free-text attribute in the same writer goes through
xml_escape. Mode names are free text, so "PLA & ABS", a quote or a "<" made the
document malformed. The failure is not a bad value on reload: both load paths for
model_settings.config return false on an expat error, and m_is_bbl_3mf is set
before the second entry loop runs, so the whole project fails to open with
"Archive does not contain a valid model config".
Both attributes now use xml_escape_double_quotes_attribute_value(), which also
emits tab, CR and LF as numeric character references. That matters and plain
xml_escape would not do: XML normalises literal whitespace in attribute values on
read, so a tab in a mode name would come back as a space and silently rename the
mode. The read side needs no change -- it takes expat's already-decoded value with
no second unescape -- so this is a lossless round trip and a file written by the
new code still loads in an older build.
The round-trip test used "copy_mode", which exercised none of this; it now carries
&, <, a quote and a tab, and also pins that ' and > come back unmodified, since
both are legal raw inside a double-quoted value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes review comment 6.
The per-tool pressure advance work changed set_pressure_advance() for users who
are not using the feature. RepRapFirmware lost its D qualifier when no tool index
was supplied: upstream emits M572 D0 S<pa> unconditionally, and a bare M572
applies to whatever tool is currently selected and errors when there is none, so
PA started depending on tool-selection state for every RRF user. The D is back,
defaulting to 0, and D<tool> is reached only from the IMEX paths.
The same rewrite had also changed the comment separator from "<value>; Override"
to "<value> ; Override" on the Klipper, RRF, Marlin 2.x and Marlin Legacy
branches, so every non-IMEX print of those flavors carried a one-byte diff.
Restored. Upstream is internally inconsistent here -- BBL and Repetier do use the
spaced form -- and the point is to match it exactly rather than to tidy it.
Emitted output for all six flavors with no tool index is now byte-identical to
upstream. Verified on a real slice: a Klipper profile emits
"SET_PRESSURE_ADVANCE ADVANCE=0.02; Override pressure advance value", an exact
string match, with no EXTRUDER= qualifier. The tests were pinning the regressed
form and are inverted.
Also records at the imex key registrations why they are kept out of the g-code
config block, matching the house convention at the other banned keys.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes review comment 16, and fixes three defects the move exposed.
PartPlate::calc_imex_zones() was 392 lines deciding where every zone, collision
strip and safety margin sits, in the GUI layer, with no test coverage. The three
wxGetApp() calls that kept it there were all in its first 25 lines, fetching two
configs. The geometry now lives in compute_imex_zone_layout(); the wrapper fetches
the configs, calls it, and clips the returned rects to the bed outline for the
GLModels, which is the only part needing GUI types. libslic3r gained no wx
dependency: it takes DynamicPrintConfig directly, so the option lookups moved
verbatim rather than through a hand-written value struct that could drift.
The move is otherwise exact -- verified by a line-for-line diff of every
arithmetic expression against the original, and by running 15 scenarios through
the extracted code against hand-derived values. The only deletion is a lambda that
was never called.
Three fixes on top, each of which needed the code to be testable:
- An off-grid or absent Primary left pri_col/pri_row at their (0,0) initialisers
and built a layout from them, reporting the whole bed as the clear primary zone
and the whole bed as a blocked mirror zone at once; under
imex_firmware_managed_zones it shifted the slice by the bed centre. Guarding on
the resolved primary head covers both routes. Reachable only from a hand-edited
preset or a 3MF authored against another printer -- the editor pins Primary to
tool 0 -- but that is the same class the unresolved-mode fallback handles.
- imex_nozzle_clearance_x/y fell back to 0.0 where PrintConfig registers 30.0.
Both strip loops are gated on the value being positive, so the fallback emitted
no collision strips at all while the preview still drew 30 mm toolhead boxes.
- The collision-strip loop asked each mirror head for its own grid cell, but an
aggregated gantry's cell is pinned to the primary's column and expanded into a
full-width row strip. Where the representative's column differed from the
primary's, no boundary matched and the plate came back with no strips and no
margin bands -- an object flush against the shared boundary sliced without a
warning while the far carriage occupied it. Present since Span aggregation was
added in 4966d0fae8 and carried out of PartPlate verbatim. The flags now come
from the painted cells; an exhaustive sweep of the reachable grid, role and
layout space (1,630,720 configurations) shows the only behaviour change is the
missing strips appearing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes review comments 3, 5, 7, 8, 18 and 19, plus the library half of 20.
These share a file, so they share a commit; each is independent of the others.
- 3: ::isspace(char) is undefined for bytes above 0x7F because char is signed on
our targets. Three call sites now go through one strip_whitespace() using an
unsigned char cast. Line 308 parses imex_head_filament_map straight out of 3MF
metadata, so a non-ASCII byte reached it without passing through the UI.
- 5: an imex_head_filament_map override past the end of physical_extruder_map now
falls through to the printer's own routing instead of resolving to a wrong
filament. Bounded in resolve_filament_for_head, where the slot count is known,
rather than at the parse site, which has no count to check against; the parse
site also gains the absolute MAXIMUM_EXTRUDER_NUMBER cap its sibling already had.
- 7: imex_physical_heater_for's !is_imex early return is what keeps a stock BBL
profile (physical_extruder_map [1,0]) out of the heater remap, and had no test.
Six cases now cover it, pinning pass-through rather than get_at()'s clamp.
- 8: ImexRole::Span was missing from the imex_head_transform switch, so it warned
under -Wswitch. Identity is correct, not merely convenient: a Span tool prints
the primary's own zone through mid-print toolchanges and has no zone to be
translated into.
- 18: three positionally coupled string vectors were resolved by nine open-coded
lookups using three incompatible bounds idioms. None read out of bounds, but six
folded the guard into the match condition, so a ragged row did not stop the scan
and a later duplicate name could win. struct ImexMode + find_imex_mode() is now
the only resolution rule: the names array is the roster, first match wins, a
short sibling pads to empty and sets ragged, not-found is an explicit -1.
- 19: the letters P/C/M/S existed in three independent copies, one of which was the
writer of the on-disk format. kImexRoleTable is now the single source, read by
both parsers and the serializer. Adding a role was 14 edit sites with one
compiler-enforced; it is now the enum, the table entry, and four -Wswitch
switches. Verified by adding a fifth enumerator and recompiling: exactly four
warnings, nothing else.
- 20: imex_resolve_routing() extracts the mode/primary/routing chain that
Print::validate and the plater's warning collector each derived separately.
The three config keys keep their names, types and on-disk representation. This is
a read-side view only; presets and 3MF files are unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Revolve failed on a sketch whose profile was closed. Decoding the reported 3mf: four
entities forming a proper closed loop (joints open by 4.44e-06 mm, well inside
tolerance) plus one stray 1.82 mm Line at (-24.2, 80.3), inside the shaded region,
touching nothing.
The viewport's region_loops discards open chains ON PURPOSE — it exists to find
EXTRUDABLE regions — so the user saw one clean closed region. entities_to_wires kept
the stray as its own one-edge loop, so it returned two wires, and Revolve goes through
entities_to_wire which demands exactly one. Extrude would have failed one step later in
wires_to_face, because a one-edge open wire bounds no face. Same class as the tolerance
split fixed in 8b568b7b: the viewport and the kernel disagreeing about the sketch — this
time about what BELONGS to the profile.
entities_to_wires/entities_to_wire/build_sketch_wire take closed_only. It is not a
blanket rule: a SurfaceExtrude builds a sheet FROM an open profile and a Sweep PATH is
normally open, so all ten call sites are classified individually — true for the face
fallback, Extrude-taper, Revolve, the Sweep PROFILE and Loft profiles; false for
SurfaceExtrude/Revolve/Loft/Fill and the Sweep path.
A component counts as open when some welded node has DEGREE 1. The first attempt used
"the traversal did not return to its starting node", which regressed the bridged C
profile: a closed loop that also carries a second edge across the same two nodes has no
free endpoint, but its Eulerian walk ends elsewhere. Degree-1 is the property that
actually distinguishes a stray segment from a closed profile; the suite caught the
difference.
Behaviour change decided by Tommaso: a stray is IGNORED, not refused. The test that
required refusal dates from when ignoring meant falling through to a default rectangle —
geometry nobody drew. That fallback is gone, so ignoring now builds the circle the user
actually drew. Its assertion is updated with the reason.
The bridge round-trip test extruded an ENTIRELY open chain and "worked" only because
OCCT will make a face from an open wire. It gets a genuinely closed profile: the test is
about serialization, and deserialize_recipe recomputes, so the document has to be one
that legitimately builds.
Failures now say WHERE. sketch_open_ends reports free endpoints under the same weld
tolerance the wire build uses, and open_loop_message is shared by both throws, because
Extrude fails through build_sketch_face and Revolve through build_sketch_wire — enriching
only one would have left the commoner path the less informative one.
Kernel 66115 assertions / 608 cases green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011FbJKJAJxxkhDTs9XdZzKA
Tommaso asked the question that names the real defect: if the sketch was open, why was
the same sketch shaded closed and offered for extrude? Because the two halves used
different tolerances. region_loops shades a region closed at 1e-3 mm; connected_loop
chained at 1e-3; the kernel welded at 1e-4 and OCCT matched vertices at 1e-7. The
2.28e-5 mm gap in the reported sketch did not cause that disagreement, it only made it
visible — and fixing the gap alone would have left the contradiction in place, ready to
reappear anywhere in (1e-4, 1e-3].
kSketchJoinTol now lives in SketchEngine.hpp and is the only place the number exists.
region_loops, loop_report, connected_loop and entities_to_wires all read it through
sketch_join_tol(). The viewport cannot promise a region the kernel refuses to build.
The welding is optional, because a kernel that silently closes loops should let you say
no: "Auto-close sketch loops" in Preferences, default ON, no restart. OFF means only
exactly coincident endpoints join — and since both halves read the same value, the
viewport simply stops shading the region closed, so an open loop is visible rather than
welded behind your back. No separate UI needed for that; it falls out of sharing one
number.
Details that matter. The kernel defaults to auto-close ON independently of the GUI, so
headless and MCP callers behave like the viewport instead of inheriting an unset
preference. With the tolerance at 0 the comparisons become <=, because OFF must mean
exact, not broken. OCCT never receives a zero vertex tolerance — it is clamped to
Precision::Confusion.
The preference is pushed from EVERY entry that starts a sketch session, not just
begin(): a Constrain session enters through begin_constrain / begin_constrain_entities
and uses region_loops and connected_loop, so a single push site would have left those
sessions running on whatever the previous one set. begin_imported_transform is excluded
deliberately — it works on imported regions, not chained entities.
Tests: a loop with one joint open by 9e-4 mm, given out of traversal order, builds a
closed four-edge wire; with auto-close off the same loop yields no wire; and an exactly
closed loop still builds with auto-close off, proving OFF means exact. Kernel 66104
assertions / 606 cases green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011FbJKJAJxxkhDTs9XdZzKA
An extrude built a solid the user never drew: three sides of the handle plus the arc
that bulges outside the outline, with the bowl's second arc and the left edge missing.
Two faults met. entities_to_wires added edges in ENTITY-CREATION order, so a partial
wire rejects the next edge even when the sketch closes perfectly; and one joint of the
reported sketch is open by 2.28e-5 mm, wider than OCCT's 1e-7 vertex tolerance and
wider than this function's own EPS of 1e-6, so that edge was refused on geometry too.
Neither showed up, because BRepLib_MakeWire::Add DROPS a disconnected edge
(BRepLib_DisconnectedWire + NotDone) while every successful Add ends with
BRepLib_WireDone + Done() — overwriting the failure. `if (!wm.IsDone()) return {}`
was therefore asking only whether the LAST edge connected. Six edges in, four out,
IsDone() true.
Endpoints now weld into shared nodes at one tolerance (kSketchWeldTol) used by BOTH
the union-find grouping and the wire build — they disagreed before, which is how a
joint gets united into a loop and then refused by the builder. Each node becomes ONE
TopoDS_Vertex, so the builder matches on identity instead of proximity, with the
vertex tolerance widened because BRepLib_MakeEdge::Init projects a vertex onto the
curve within that tolerance and a welded node sits up to the weld gap off its
neighbour's curve. Members are then walked in traversal order. Finally the result is
counted: IsDone() alone is not evidence, edge_count == members.size() is.
Arc geometry is untouched — the midpoint from (start_angle+end_angle)/2 and the
solver's angle reflow both measured correct and were never part of this.
The regression case carries the reported sketch verbatim, open joint included. It
fails 4 == 6 without the fix, which was measured, not assumed. Kernel 66092
assertions / 604 cases green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011FbJKJAJxxkhDTs9XdZzKA
Five defects from ten minutes of real use, and the mode split behind the worst
of them. One commit because the changes overlap in the same functions; the
pieces are separable in the diff, not in the file.
CONSTRAINING NO LONGER NEEDS A COMMITTED SKETCH. A constraint could only be
applied by committing the sketch, selecting it in the feature tree, pressing the
padlock, and only then picking. While drawing, the CONSTRAIN toolbar was not even
on screen (set_ui_mode showed it in UiMode::Constrain alone) and apply_constraint
answered "Press Constrain on a sketch first" -- in a status line nobody looks at.
Draw two lines, press Parallel, get nothing: that is what a user reported as "the
UX is a mess", and they were right.
apply_constraint now takes the live session first, reading the picks from the
selection model the sketch tool already had (click, ctrl-click to extend,
double-click for the loop) and applying through try_add_constraints, which
already did append -> solve -> keep-or-rollback. The committed Constrain path
stays for editing an old sketch; it is no longer the only way in. The twenty
constraint buttons now show in Sketch mode as well.
The discriminator is is_sketching() && !is_constraining() &&
!is_constraining_entities(). Both begin_constrain and begin_constrain_entities
set m_active, so is_sketching() alone is true DURING a constrain session and the
new path would hijack the old one -- compiling perfectly and failing in
behaviour.
ONE PLANNER, NOT TWO. A second caller meant duplicating the logic that decides
whether a constraint is legal, which roles it binds and whether it needs a typed
value. That duplication is how today's Coincident bug survived: fixed in one
branch, alive in the next one down. plan_entity_constraint() now lives in the
kernel -- pure, no wx, no translation -- and both UI paths call it. DesignPanel
loses 304 lines and gains 155.
Being in the kernel makes it TESTABLE. The Parallel defect existed because a
constraint type met an entity type nobody had tried, and the only instrument was
a 13-minute GUI ladder. 19 new kernel cases cover the matrix: 264 -> 283 cases,
7648 -> 7867 assertions.
Parallel, Perpendicular and EqualLength gain the two-line guard they never had.
On non-lines they used to emit a def the solver silently dropped -- the sketch
reported itself constrained when it was not, the same class as Horizontal on a
Point. EqualLength on two rounds still promotes to EqualRadius first.
Symmetric is planned completely, including its axis pick: the plan carries a
VECTOR of defs because Symmetric on two lines is two constraints (P0/P0 and
P1/P1). A single def would have half-applied it -- one end pinned, one free,
looking correct until something moves.
A PLACED POINT SURVIVES THE COMMIT. Type::Point was created correctly and never
drawn once committed: both renderers skip it, correctly, since entity_polyline
gives a point nothing. What was missing is the vertex-marker path the live
session already used. rung_point passed throughout because it asserts the
document, and the point was always in the document -- the pixels lied.
ESC STOPS EATING AN UNSAVED SKETCH. The third press reached cancel_sketch(),
clearing m_entities with no warning and nothing to undo. live_sketch_has_work()
existed and was never consulted. The exit layer refuses once when there is work
and lets a second consecutive Esc through; the refusal re-arms on a button press,
never on mouse motion, or Esc could never exit while the hand moves.
TWO NEW RUNGS. D10 drives Parallel through the committed path -- it passes on
the PRE-fix binary, which is how we know the user's failure was the mode and not
the constraint. D11 is the acceptance for the collapse: draw, pick both, press
Parallel, no commit and no padlock. Ladder 126 -> 135 properties, all holding.
CON_BTN_SKETCH is measured, not derived: in Sketch mode the group renders after
the sketch toolbar, so the first button is at 677, not 449. Pitch 42, twenty
buttons, read off a screenshot. Deriving it by offset is how that table drifted
the last time.
Known limit, commented at the call site: a constraint added to a LIVE sketch is
not on the document undo stack, so Ctrl+Z will not take it back until the sketch
is committed.
snaporca-itp4, snaporca-oyhx, snaporca-l2vm
Format/3mf.cpp also saved and loaded it, but nothing calls its store_3mf and
its load_3mf only sees files fingerprinted as PrusaSlicer's, which never carry
a recipe. Its round-trip test only exercised that dead loop. The BBS backend,
which every save and load goes through, is untouched.
These dialogs treated a filament with no compatible_printers as compatible with
nothing, while the rest of the app treats it as compatible with everything, so
the entire Orca Filament Library was missing from the AMS material and
calibration filament lists. They now resolve compatibility the same way the
plater does, and a vendor profile still supersedes the library generic of the
same name.
# Description
This is an initial draft of the plugin audit workflow.
It focuses on the user experience and developer-facing permission
workflow. It does not yet include the complete implementation of every
operation that should be audited, such as the full filesystem,
networking, and process-spawning event coverage.
## User workflow
When a plugin is loaded:
1. The plugin’s register_capabilities() function is executed.
2. The plugin declares the permissions it requires.
3. OrcaSlicer displays a permission dialog listing the requested
resources.
4. If the user grants access:
- The permission is persisted in the plugin’s .install_state.json.
- Capability registration continues.
- The plugin is materialized and loaded.
5. If the user denies access:
- Plugin loading fails before capabilities are materialized.
- on_load() is not called.
- The plugin’s install state is marked with "enabled": false to prevent
repeated automatic load attempts.
At runtime, if a plugin accesses a resource that was not approved during
loading, the audit hook displays another permission dialog. For
filesystem requests, the dialog identifies the requested filepath.
- Granting access persists the permission and allows the operation.
- Denying access raises a Python PermissionError.
- The error propagates to the host, which records the failure and
unloads the plugin.
Host-side traceback logging is performed outside the plugin audit
context so that logging does not generate additional permission dialogs.
## Developer-facing API
Plugins can declare filesystem read permissions through the new API:
```python
import orca
AUDIT_PATH = __file__
@orca.plugin
class ExamplePackage(orca.base):
def register_capabilities(self):
orca.request_permissions(
fs_read=[AUDIT_PATH],
)
orca.register_capability(ExampleCapability)
```
orca.request_permissions() must be called from register_capabilities()
while the plugin is being loaded.
Currently supported permission:
orca.request_permissions(fs_read=[...])
The paths should be explicit filesystem paths that the plugin intends to
read. The host deduplicates repeated paths, presents the request after
registration completes, and persists granted paths in the plugin
install-state sidecar. This API is still experimental, and is by no
means the final implementation.
Support for additional permission categories, including filesystem write
access, networking, and process spawning, is reserved for subsequent
work.
# Screenshots/Recordings/Graphs
<!--
> Please attach relevant screenshots to showcase the UI changes.
> Please attach images that can help explain the changes.
-->
<img width="869" height="799" alt="image"
src="https://github.com/user-attachments/assets/8a5903cc-0cbb-45a8-b88a-706d6cba790f"
/>
## Tests
<!--
> Please describe the tests that you have conducted to verify the
changes made in this PR.
-->
<!--
> A guide for users on how to download the artifacts from this PR.
-->
[How to Download Pull Requests Artifacts for
Testing](https://www.orcaslicer.com/wiki/how_to_download_pr_artifacts)
update_values_from_multi_to_multi_2 iterates the destination PRINTER's variant
list while writing into a row taken from the destination PRINT preset. Those two
lengths are maintained independently -- print_extruder_variant against
printer_extruder_variant -- and Tab::load_current_preset() runs the migration
before the print preset is re-selected for the new printer. Opening a project
saved on a single-variant printer and switching to a seven-variant one therefore
wrote six elements past the end of a one-element vector. The corruption stays
silent until the next allocation, so the abort surfaces somewhere unrelated and
the backtrace points at innocent code.
Size the row to the variant count before indexing it. Every write is then in
range, and the result carries one value per destination variant, which is what
the callers consume. Pad with nil rather than a copied value: set_to_index()
skips nil entries, so a variant the object has no opinion about keeps tracking
the print preset instead of being pinned to another variant's number.
The same shape -- a count from one array indexing another -- appears twice more
in this file. update_values_from_multi_to_multi has three of these writes
protected only by assert(idx < old_count), and NDEBUG is defined for every
non-Debug configuration, so those guards are absent from shipping builds.
update_values_from_single_to_multi has the read half. Both are bounded here;
leaving them would fix one third of one defect.
Source reads are bounded too. is_nil(size_t) indexes values[idx] without
checking, so an index past the end was undefined behaviour on that side as well.
Where the row already matches the variant list -- every case that was not
corrupting the heap -- the resize is a no-op and the output is unchanged.
Fixes#15455
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Port of snaporca e635627b81. Parity OK: 17 files identical, 8 diverging at their
expected counts.
The last reference an industrial sketcher offers that this one did not: Onshape's
Use, SolidWorks' Convert Entities. Project already turned a body's 3D edges into
2D Lines, Circles and Arcs on a plane, but the result landed in its OWN feature,
so while drawing in one sketch you could not borrow an existing body's edge and
constrain to it.
No new geometry code: Project's per-edge conversion loop is factored into
project_edges_to_entities() and called from a second entry point that appends
into an EXISTING sketch with construction = true. The loop appears once now,
not twice.
Construction is what makes it cheap and safe: SketchEngine already skips
construction entities when building wires, so the references guide without being
built, and being otherwise ordinary entities every constraint from this epic --
Collinear, EqualRadius, the axis-projected distances, PointOnLine, Symmetric --
works against them for free.
Part of this refactors working code, so Project's behaviour identity is the
invariant; the existing [CadDocument][project] cases guard it and a new case
asserts a Project feature still emits construction == false.
project_edges_into_sketch returns the number of entities appended, or -1 on a bad
reference rather than throwing.
VERIFICATION LIMIT, as with the previous four commits: this fork's kernel suite
still cannot run (find_package(assimp) at configure time, snaporca-w80c). Shared
sources are byte-identical to snaporca's, where kernel is 7700 assertions / 270
cases and ALL LADDERS HELD 7/7.
Port of snaporca 2d36d28770. Parity OK: 17 files identical, 8 diverging at their
expected counts.
infer_axis_constraint returned only Horizontal or Vertical. On the
CAD-1000-hours corpus the top two transitions are sketch_dim -> sketch_draw
(5896) and back (5756): the signature of geometry that does not self-constrain
as it is drawn.
Every rule requires the relation to be ALREADY TRUE within tolerance, so nothing
the user drew is moved; parallel/perpendicular and tangent additionally require a
shared endpoint.
TWO LIMITS THE CORPUS RUNG FORCED, neither visible to the unit tests:
1. At most ONE constraint per rule per new entity, not one per PAIR, and no
one-at-a-time fallback for the relations batch. EqualRadius has no locality
restriction, so 200 equal holes produced ~20000 candidates; the rejected batch
then cost a solve per constraint and pinned the app at 95% of a core with the
MCP socket unresponsive.
2. Relations only for gesture-sized batches. "A scripted add is not a drawn
gesture" is already this file's rule at its bulk call site (snaporca-8xg1), and
EqualRadius also couples geometrically distant entities, merging independent
connected components and defeating the partitioning that makes large sketches
solvable (snaporca-yww4). With the cap alone geometry stayed correct (32/32
sheets clean) but seven of the largest timed out, including MPD681 -- the sheet
that call site's own comment names.
Also fixes the tolerance leak behind 2: the bulk path asks for exact inference
with ang_tol_rad = 0 but len_tol_frac kept its 0.01 default.
ALSO independent of this feature: run-kernel-tests.sh defaulted to
TAGS=[CadDocument] while four CAD test files carry their own tags and nothing
selected them (2624 assertions / 206 cases reported, 7648 / 264 actual). All 58
dark cases were passing; the coverage was never exercised.
VERIFICATION LIMIT, as with the previous three commits: this fork's kernel suite
still cannot run (find_package(assimp) at configure time, snaporca-w80c). Shared
sources are byte-identical to snaporca's, where kernel is 7651 assertions / 265
cases and ALL LADDERS HELD 7/7.
Port of snaporca 5b1294de59. Parity OK: 17 files identical, 8 diverging at their
expected counts.
Every industrial sketcher gives you the origin and the axes as references. Here
the origin was only a SNAP target and the axes did not exist, so Symmetric needed
a third picked ENTITY as its mirror axis: symmetry about the sketch's vertical
axis first required drawing a construction line.
Every constraint reference resolves through four lambdas in the solver
(valid/ptOf/primOf/coordOf), so teaching those about three negative sentinel
indices makes the origin and both axes available to EVERY constraint type at
once. No new SketchEntity type, no serialization change; -1 still means "unset".
The references live in G_FIXED and add no degrees of freedom, which a test
asserts via the reported DoF.
SymmetricAboutY / SymmetricAboutX are two buttons that need no third pick and no
construction line. Making the axes clickable in the viewport is deliberately left
out: that is canvas hit-testing work with its own risks.
Recorded in the tests because it will catch the next person: sys.dragged[] is
populated only during a drag, so a plain sketch_solve of an UNDER-constrained
system may move any free parameter -- solvespace runs Newton, it does not
minimise movement. PointOnLine onto an axis is one equation in two unknowns and
the point legitimately slides along it. Those tests pin the free direction
instead of asserting the other coordinate is untouched.
VERIFICATION LIMIT, as with the previous two commits: this fork's kernel suite
still cannot run (find_package(assimp) fails at configure, snaporca-w80c). The
shared sources are byte-identical to snaporca's, where kernel is 2624 assertions
/ 206 cases and ALL LADDERS HELD across all seven rungs.
Port of snaporca 9fa304c77a. Parity OK: 17 files identical, 8 diverging at their
expected counts.
The everyday dimension in SolidWorks and Onshape, and this kernel had no form of
it. Distance constrains the straight-line gap; LockX/LockY pin one point's
ABSOLUTE coordinate. Neither relates two points along an axis.
DistanceX/DistanceY emit SLVS_C_PROJ_PT_DISTANCE against two unit direction
lines built in the solver's G_FIXED group, so they add no degrees of freedom.
THE DIRECTION IS SIGNED, and getting it backwards is silent. libslvs defines a
LINE_SEGMENT's direction as point[0] - point[1] (entity.cpp) and
PROJ_PT_DISTANCE constrains (pB - pA).dot(dir) (constrainteq.cpp:234), so the
reference lines are built head-first to mean +X and +Y.
The same signedness was a real defect in the GUI: the inline editor was
pre-filled with |delta|, so when the closest endpoint pair ran right-to-left,
opening the dimension and accepting the number shown would flip the point to the
other side of its anchor. Opening a dimension and accepting its own value must
be a no-op. The refs are now ordered so the shown value is positive.
On the CAD-1000-hours corpus, dimensioning and constraining is 31.9% of all
observed CAD time -- the largest single class, 7.6x feature operations. This is
the item in the constraint epic that lands most directly on it.
VERIFICATION LIMIT, as with the previous commit: this fork's kernel suite still
cannot run (find_package(assimp) fails at configure time, snaporca-w80c). The
shared sources are byte-identical to snaporca's, where kernel is 2603 assertions
/ 200 cases and ALL LADDERS HELD across all seven rungs.
Port of snaporca 9ec6405e2d. Parity OK: 17 files identical, 8 diverging at their
expected counts (DesignPanel.cpp 32, test_slvs_constraints.cpp 3).
Measured on the CAD-1000-hours corpus: 51.5% of observed CAD time is 2D sketch
work, and dimensioning/constraining alone is 31.9% -- the largest single class.
Two constraints every industrial sketcher has were missing here.
EqualRadius fixes a dead end rather than adding a feature. Picking two circles
and pressing Equal emitted EqualLength, which maps to SLVS_C_EQUAL_LENGTH_LINES
and constrains nothing on a curve: a silent no-op with no error. Equal is now
one button with two meanings, as in Onshape and SolidWorks.
Collinear emits PARALLEL plus PT_LINE_DISTANCE=0 rather than PT_ON_LINE, whose
internal valP param this libslvs port leaves at 0, drifting an already-collinear
pair.
Both types are appended at the END of SketchConstraintType: cereal serializes it
positionally, so inserting elsewhere reinterprets every saved recipe.
VERIFICATION LIMIT, stated rather than implied: this fork's kernel suite could
NOT be run. scripts/CAD/run-kernel-tests.sh fails at CMake configure time on
find_package(assimp), before any source compiles -- a pre-existing deps gap
(snaporca-w80c), not this change. The shared sources are byte-identical to
snaporca's, where the full gate passed: kernel 2588/195 and ALL LADDERS HELD
across all seven rungs.
Also fixes two defects in this fork's scripts/CAD/run-all-checks.sh:
- `cd $(dirname $0)/..` landed in scripts/ instead of the repo root, so every
rung looked for itself under scripts/scripts/. Broken since the script moved
into scripts/CAD/; the three sibling scripts were fixed then and this was
missed, so the gate has not run since.
- C defaulted to snaporca-gui, the OTHER fork's rig container, so this fork's
gate would drive snaporca's app and report green about the wrong binary.
run-kernel-tests.sh:31 documents the identical defect being fixed once
already for the build volume; this is the third instance.
The recipe's 3MF entry moved from Metadata/SnapOrca_cad.bin to
Metadata/orca_cad.bin, so projects saved by earlier builds opened with an
empty Design tab. Both 3MF backends now read either name and write only the
new one; the recipe version advances to 6 to mark the move.
Requested by SoftFever on PR #15238: ten of these had accumulated loose in
scripts/ next to ~20 unrelated upstream ones, with names that only meant
something to whoever wrote them. They now sit in scripts/CAD/, mirroring the
src/libslic3r/CAD/ and src/slic3r/GUI/CAD/ split, and the verb in the name is
the role: build- produces a binary, start- brings something up, run- runs a
suite, check- asserts one thing against a live app.
kernel-test.sh -> CAD/run-kernel-tests.sh
ladder-all.sh -> CAD/run-all-checks.sh
sketch-ladder.py -> CAD/check-sketch-engine.py
ladder-corpus.py -> CAD/check-sketch-engine-corpus.py
gui-ladder.py -> CAD/check-gui-sketching.py
offer-ladder.py -> CAD/check-gui-context-menu.py
mcp-sketch-smoke.py -> CAD/check-mcp-sketch.py
rig-build.sh -> CAD/build-gui.sh
docker-iter-build.sh -> CAD/build-gui-incremental.sh
gui-session.sh -> CAD/start-headless-gui.sh
"Ladder" was the worst of them: it named the shape of the test (rungs of
increasing difficulty) rather than what the test proves, so nothing in the
directory listing told you which one needed a GPU and which was pure kernel.
Every reference rewritten -- the docs, the cross-calls between the scripts,
Dockerfile.deps, and the container-side /OrcaSlicer/scripts paths. The three
shell scripts resolve REPO relative to themselves and now sit one level
deeper, so that walk went from /.. to /../.. . The copies these push into a
container's /tmp were renamed to match, or the container would have kept the
old names alive.
Two runtime paths deliberately NOT renamed. /tmp/orca-rig-build.lock is a
cross-fork contract -- both forks take the same lock so two concurrent builds
serialise instead of OOMing the box, and renaming it on one side silently
removes that guard. /tmp/gui-session.log is a runtime artefact, not a script.
Added scripts/CAD/README.md: what each script proves, what it needs, and the
two constraints that have each cost a session (never build inside the GUI
container; a window manager is required or synthetic keys are ignored).
On CI, which was the other half of the request: the kernel suite is already
there and always has been. The cases are registered in
tests/libslic3r/CMakeLists.txt under if (SLIC3R_CAD), which defaults ON and no
workflow turns off, so they build into libslic3r_tests and run under ctest on
every platform via unit_tests.yml -- like any other unit test, needing no new
job. They have simply never been seen to run, because the workflows on this PR
are still awaiting maintainer approval. run-kernel-tests.sh is the local loop
over the same cases, and it is the only script here CI could run: the other
six need an OpenGL canvas and synthetic input.
Verified: scripts/CAD/run-kernel-tests.sh from its new location, all tests
passed, 2562 assertions in 190 test cases.
Validate mixed-filament definitions during the published material pass: definitions whose components reference slots that do not exist or hold other mixed filaments, or that carry fewer than two components, are reported through the shared skipped_keys channel instead of shipping a mix the GUI integrity check would only flag later.
Fix the slot-limit exhaustion report being silently dropped: it wrote to published_config->skipped_keys, which the pass's final move-assignment from the local vector clobbers. All rejections now go through the local.
Remove the unreachable persist branch from add_detached_preset: no caller passes save_to_project=false, so the parameter is gone and the copy is always project-embedded.
Tests: cover the exhaustion path, the new definition validation, the identity-tier matching matrix (including substitute reporting), the structural-key denylist, whole-vector size-mismatch skips, relocation payload degradation, the "(Published 2)" uniquify chain, mixed blend colours staying out of shared preset configs, and duplicate-slot last-wins. Also fix the legacy-3mf scenario passing vacuously behind an if-guarded assertion. All existing published/3mf tests pass unchanged.
A mixed filament is unsupported in a parallel mode outright, but the rule saying
so ran third. A plate carrying a blend plus any second filament tripped the
multi-color rule's used > 1 gate first and was told its active tools all sit on
one gantry -- a diagnosis of a multi-color print the user never configured,
whose remedy is to go rework the mode's tool roster. The blend was never
mentioned. Move the check ahead of both rules below it; being unsupported
regardless of routing or topology, it dominates them.
Nothing is masked that leads anywhere else: every branch of
imex_multicolor_block_reason is itself confined to non-primary modes, so the
mixed message's remedy -- switch this plate to Primary -- silences those too.
Say "Mixed filaments", not "Blended". Every other string in the app calls these
mixed, including the button that creates one and the sibling refusal for the
wipe tower filament, so the user had no way to connect the message to the
feature it names.
Three comments in the block were wrong, and two of them were newly wrong. The
routing rule's bounds-check note still said "Blended slots are out of range by
construction, but they never reach here -- the rule above returns first": the
rule above is now the multi-color one, which does not return first for a single
mixed filament, and out-of-range is not guaranteed at all. Mixed slots are kept
at the tail of the filament arrays by convention, not by enforcement --
PresetBundle::set_num_filaments grows filament_is_mixed with resize(), so
raising a printer's extruder count with a blend present lands physical slots
after the mixed one. The scan is position-agnostic and stays correct; only the
stated reason was wrong.
The same discovery makes the empty-routed_list guard live rather than the dead
code it was described as. Print::apply() normalises physical_extruder_map before
validate() runs, so an unauthored map is never the cause -- but a printer with
more filaments than logical extruders leaves the tail slots outside the map, and
raising the extruder count does exactly that.
The new test validates the plate twice. The first pass, with no blend, asserts
the multi-color rule is armed at all; without it the second proves nothing,
because the rule only fires here thanks to a degenerate fixture mode whose two
tools share a gantry. Give that mode a Span tool and the whole test would pass
under either ordering while appearing to guard it. It also pins err.object,
which the mixed path sets and the multi-color path leaves null -- a discriminator
that survives the next wording change. Verified by reverting the order: the test
fails on both the message and the object.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
