With single_extruder_multi_material and purge_in_prime_tower, the flush
matrix replaces the prime volume and its average is non-zero for one
filament, so the estimate reports a tower. normalize_fdm_2 clears the tower
for that same plate, so none is printed: the estimate answers how big a
tower is, never whether there is one, and a caller reading a non-zero depth
as "a tower is printed" reserves space for, or blocks on, a phantom. The
test asserts that disagreement directly.
Type1 is the exception in the same case, because it decides from its
per-filament purge list and a lone filament is never changed to. The scene
reads the same estimate either way. Type2 is the default for every non-Bambu
printer, and so for every IDEX/IQEX one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nozzle_temperature is variant-expanded, so its length is columns rather than
filament slots. On the dynamic-nozzle path that makes it longer than the
slot count, and an out-of-slot index reached get_filament_config_index() and
came back as filament 0 - the clamp the bounds-checking was meant to remove.
Bound by the slot count instead; on the ordinary path the two are equal and
nothing changes. The is_extruder_used write gains the matching lower-bound
guard.
IMEXHelpers.hpp now states both halves of the rule its call sites follow.
Bound anything derived from the extruder map against the filament slot count
before using it as a filament id, not against the option about to be read.
And a miss is -1, which is a correct tool qualifier but matches no physical
head, so it cannot serve as a skip-the-primary sentinel: which head prints a
filament is answered by the filament and the map, never by a mode role,
since a primary-mode print may use any or all tools, one at a time.
The consequence is recorded there rather than left implicit. The two skip
sites skip nothing for a slot past the end of the map, so a plate with more
slots than nozzles double-writes the primary's pressure advance. It is
narrow and unreported, and a guard there would be a smaller change than
naming a head.
The header also records that RepRapFirmware sends an unqualified pressure
advance as M572 D0, naming drive 0 absolutely rather than the active tool.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The parallel printing options shipped to testers as is_ixex and ixex_*,
became is_imex and imex_* when the feature stopped being called iXex, and
the two clearance keys were renamed once more to say what they measure:
nozzle to carriage edge on the collision side, not the carriage's full
width. Nothing translated any of it, so loading an existing printer profile
dropped every one of these values - the keys are unknown and cleared.
is_ixex is the one that matters most, because without it the others migrate
into a feature that stays switched off, leaving settings that look
configured and do nothing.
Per-plate mode is persisted twice and only one path went through
handle_legacy. Plate metadata in a 3MF is matched by exact attribute name
and written with set_key_value, so a project saved between the per-plate
mode landing and the rename loaded every plate back on Primary and sliced
single-carriage with no warning. The loader now accepts the old attribute
name.
ixex_primary_col and ixex_primary_row are dropped rather than mapped: the
primary is a role inside the mode's active-tools string now, not a grid
coordinate, and they were never in an option list, so no saved file carries
them.
The test drives the full era-1 key list and asserts the enum values rather
than non-nullness, since a forward-compatible substitution would otherwise
hide a failed deserialize behind a default. handle_legacy had no test before
this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two range-for loops bound const std::string& to braced lists of string
literals, so each iteration constructed a temporary to bind to. GCC 16
reports it as -Wrange-loop-construct, which upstream's blanket -Werror turns
into a build failure; clang does not report it at all. Spell the
initializers the way the loop above them already does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings on the preceding commit, plus one defect it should have
caught.
- The IDEX/IQEX pressure-advance loop fed resolve_filament_for_head()'s
result straight into enable_pressure_advance and pressure_advance. That
result is bounded by physical_extruder_map, which holds one entry per
NOZZLE, while both options are indexed per filament SLOT. On a printer
with more nozzles than the project has filaments the two spaces diverge
and get_at() clamped the overflow onto filament 0, emitting its pressure
advance on a secondary carriage. The second-layer temperature loop bounds the
same lookup, but against nozzle_temperature, which is variant-expanded and so
is not the slot count either -- it is not the precedent it looks like.
IMEXHelpers.hpp states the rule
once, and a test pins the contract that makes the bound necessary:
resolve_filament_for_head() answers in nozzle space, so a non-negative
result is not by itself safe to use as a filament id.
- The header claimed every caller renders a -1 tool qualifier as "emit
none". RepRapFirmware substitutes the historical D0 instead, deliberately
and with its own comment in GCodeWriter. Say so, rather than leaving a
contract a future author would code against.
- A cross-reference pointed at a hard-coded line number that the preceding
commit had itself shifted by nine lines. Name the function instead.
- The multi-color rejection reasons reach the user through Print::validate()
as raw English, while the returns on either side of them use L(). Wrap
them and register IMEXHelpers.cpp for extraction. They also still said
"IMEX", the internal name, so they move to IDEX/IQEX with the rest of the
user-facing strings rather than shipping the internal one to translators.
- Trim the preceding commit's comments. One block explained the same
clamping hazard six times; the canonical explanation now lives in
IMEXHelpers.hpp and the call sites point at it. The mode grid carried
twelve lines of commentary and no code, most of it archaeology already in
the commit message, and one claim about the modes editor that was not
true. The ArrangeJob threading note stays: it documents an invariant that
cannot be recovered from the code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings on the IDEX/IQEX parallel printing code, all in paths the
feature owns.
- physical_extruder_map lookups used ConfigOptionVector::get_at(), which
clamps an out-of-range index to values.front() rather than reporting a
miss. The map holds one entry per nozzle while filament ids index slots,
and nothing caps the slot count at the nozzle count, so a project authored
with more filaments than the printer has extruders silently addressed the
primary's head: pressure advance pinned to the wrong carriage, and
skip-primary loops suppressing whichever head sat at pem[0]. Bounds-check
at all four sites and treat the miss as "no mapping" (-1). Covered by a new
imex_pem_tool_for test; the header note now warns against get_at here.
- IMEXFilamentPickerPopover leaked a top-level window per ghost click:
wxPopupTransientWindow::Dismiss() only hides, and never reaches OnDismiss().
Destroy from an OnDismiss() override and dismiss the picker through
DismissAndNotify(), which is the path a successful pick takes.
- ArrangeJob read PartPlate's IMEX zone cache from the worker thread, where
a cache miss rebuilds GLModel members with no GL context current while the
GUI thread may be painting them. Snapshot the zones in prepare(), on the
main thread, already converted to plate-local coordinates.
- The mode grid anchored its row window to the Primary's gantry row. A window
as tall as the grid can only start at row 0, so this drew tiles for tools
that do not exist and hid real ones. Render the whole grid instead; a
Primary outside it is a data problem the zone layout already reports.
- Build the mode tooltip from one format string rather than two catalog
fragments concatenated around a runtime value, so translators can move the
mode name within the sentence, and register IMEXModesCtrl.cpp for string
extraction.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Fix internal bridges over Hilbert Curve/Octagram Spiral sparse infill
For patterns with curved/turning anchor lines (Hilbert Curve, Octagram
Spiral), the bridge_over_infill algorithm produced incorrect results:
1. determine_bridging_angle: sampling curved anchor orientations
produced noise across all turning directions (0/90/180/270°)
instead of a single dominant one, yielding unstable bridge angles
with 180° spread. Fix: use the configured infill_direction + 90°
directly, bypassing the noisy sampling. The old blind +0.25*PI
(Hilbert) and +1/16*PI (Octagram) offsets are removed.
2. construct_anchored_polygon: curved Hilbert/Octagram anchors
intersected each vertical scan line many times at wildly different
Y positions, producing chaotic polygon sections — holes in random
places, bridges over air, rotated bridges. Fix: replace the curved
infill polylines with synthetic straight lines parallel to
infill_direction, spaced at the real infill line spacing
(flow_spacing / density). Lines are centered on the limiting_area
bbox center so that after rotation they span the full bridged_area.
Anchors are left at full bbox length (not clipped) to guarantee
every scan line finds an anchor.
Rectilinear and other straight-line patterns are unaffected.
Known limitation: some bridge edges may still terminate over air in
edge cases where the nearest synthetic anchor line is more than one
infill spacing away from the bridge boundary. This will be addressed
in a follow-up.
* fix: anchor internal bridges to actual sparse infill
Preserve real anchors across regions and align plane-path anchor origins with printed infill. Respect lower-layer rotation templates and model alignment, and sample curved bridge boundaries more finely.
Add regression coverage for anchor alignment, bridge angles and region isolation, with Orca comments explaining the geometry constraints. Verified 175 FFF tests before the comment-only follow-up; preserve CRLF in modified files.
* Fix internal bridge support contacts and separated infill origins
Restore anchor contact after bridge smoothing and share per-body pattern origins between anchors and printed infill. Recompute origins when preparation settings change.
Cover multiline counts 1, 2 and 3 and add regressions for printed bridge support, separated infill alignment and reslicing.
* Add explicit standard headers to PrintObject tests
* test: cover surface centering when infill settings change
Verify top and bottom Archimedean Chords and Octagram Spiral paths after switching centering modes or toggling separated infills. Compare reslicing against fresh slicing and document dependent infill invalidation.
* test: preserve directional surface infill when settings change
* perf: index layer islands for connected-body detection
* test: use public print pipeline for body centering checks
* build: clear 2 warnings - cast the NSTextField the class check already proved
mainframe_text_field is NSTextField* and was assigned a bare NSView*, which
Clang reports as -Wincompatible-pointer-types. Both assignments sit inside
if ([viewObject class] == [NSTextField self]), so the runtime type is already
guaranteed, and the line above the second one casts the same variable the same
way to call setTextColor. macOS only, since nothing else compiles this file.
* build: clear 6 warning categories from the clang-cl inventory
-Wmissing-braces (9). Aggregates whose first member is itself an aggregate.
GUID's fourth member is BYTE[8], so the trailing eight bytes take their own
braces. The others were reaching for zero-initialization with {0} and say {}
now. bbs_3mf's backup Task ends in an anonymous union, which needs braces of
its own; those braces initialize the union's first member rather than the one
named at the call site, so the RemoveBackup site says so in a comment.
-Wmacro-redefined (11). SendMultiMachinePage.hpp defines five names that
Preferences.hpp, PresetBundleDialog.hpp, ExportPresetBundleDialog.hpp and
TroubleshootDialog.hpp also define with different values, so the value in
force depended on include order. All nine of this file's DESIGN_ macros take
the SEND_ prefix it already uses for its own macros, values unchanged, so a
DESIGN_ name added elsewhere later cannot collide with it again. They read as
one page-local palette, a 900 to 400 gray ramp plus sizes, so the four with
no current readers stay: dropping them would leave gaps in a named scale. test_marchingsquares.cpp defines NOMINMAX,
which libslic3r already passes as a PUBLIC compile definition, so it takes
the #ifndef guard the other suites use.
-Wbraced-scalar-init (3). Two PushStyleVar calls resolve to the float
overload, so the braces were initializing a scalar. ConfigOptionFloatsNullable
already takes an initializer_list, so the inner braces did the same thing.
-Wmicrosoft-goto (2). Both gotos in copy_file_gui jump forward over the
initialization of size, dwRead and dwWrite, which only MSVC accepts. Those
declarations move up to join the others at the top of the function.
-Wunused-private-field (3). Every use of ColourPicker's m_clrData and
m_picker_widget is behind !defined(__linux__), so on Linux they are written
and never read; the members now carry the same guard. ParamsPanel's
m_size_move is read nowhere. Tab has its own, which is the one Tab.cpp uses.
-Wnonportable-include-path (2). BaseException.h asked for "stackwalker.h"
and the file on disk is StackWalker.h.
fix: report the real error when a Windows G-code export fails
copy_file built its failure message as "Error: " + errCode. Adding a DWORD
to a string literal is pointer arithmetic, not concatenation, so the pointer
lands errCode bytes into an 8-byte literal and runs past its end for any code
above 7. std::string then calls strlen on it and throws length_error, and the
catch(...) in BackgroundSlicingProcess::finalize_gcode replaces the diagnosis
with "Unknown error occurred during exporting G-code."
Every code a user is likely to hit is past the end: write-protected media is
19, no media 21, a full disk 112, and a destination held open by another
program 32. Codes 1 to 7 stay inside the literal and produce a truncated
message instead. So the "Maybe the SD card is write locked?" text has not
been reachable on Windows since this path was added in #2923.
Now that it is reachable, that guess only fits removable media, so it is
conditional on m_export_path_on_removable_media. The existing string is
untouched and keeps its 23 translations; the fixed-drive case adds one string.
The preview brim, the placement margin and the pre-generation validation
warning each decided on their own whether the tower has a Type2 cone
base, reading the wall type and cone angle three different ways. The
preview's read cast the preset's enum to ConfigOptionEnum<T>, which a
preset-shaped config never holds, so the cone base was never previewed.
estimate_wipe_tower_first_layer_outline now answers that question once,
beside the footprint estimate, from the config and the resolved planner;
all three sites take the outline from it. The libslic3r case reads the
outline off a preset-shaped config, where the old cast came back empty.
The clamps and validation work from estimates. Once the tower is
generated, _make_wipe_tower re-tests the exact first-layer footprint,
brim and cone base included, against the printable area and the
exclusion zone, so an off-plate tower fails with a clear error instead
of exporting unprintable G-code. The rectangle-wall mesh footprint
learns the Type2 cone base so that check and the post-generation
validation see the real outline.
Pre-generation, validation hard-checks the body plus an explicit brim
and warns on the estimated auto brim and cone base with the existing
"may collide" strings, so the user hears about a marginal position on
the first slice rather than only at generation time.
Two fff_print fixtures that print a tower at the default position move
it onto the 200 mm test bed, as the multifilament fixtures already do:
the shipped default y of 220 is off that bed, and the backstop now says
so instead of exporting the tower.
Smooth timelapse no longer charges a prime volume it does not purge. A
tower printed with no tool change is exactly the idle depth: the
stability minimum for Type2, the wrapping detection depth for Type1.
Charging a full prime_volume on top made the previewed and arranged
tower deeper than the one that is printed.
The Type2 half of "a tool change reserves a tower whatever the purge
volumes resolve to" arrives with the base commit; here it only has to
survive the planner split, since Type1 already reserves per filament.
The wipe tower filament only joins the tool ordering when there is a
tower to join, which is the has_wipe_tower() half of the guard
Print::extruders applies.
The shared estimate reserved every tower with one volume-per-purge rule
and the stability floor. Both planners do more: WipeTower (Type1) wipes
each filament's own prime volume in whole lines, one block per
adhesiveness category sized by its worst layer, rams the leaving
filament at every nozzle change, and squares a rib tower from the
planned depth; WipeTower2 (Type2) spaces its lines by
wipe_tower_extra_spacing, not the Type1-only infill gap, and its extra
flow cancels out of the depth. Both extend the ribs rather than the body
below the stability minimum, size every layer including a thinner first
one, and lay the brim in whole loops, WipeTower reporting half a spacing
of line width on top.
All of that now lives in estimate_wipe_tower_footprint, fed the planner
(resolve_wipe_tower_type mirrors Print::wipe_tower_type and the CLI's
Bambu Lab detection) and the filament ids rather than a count. Print
passes its own tool set; the PartPlate adapter derives the plate's ids
from the passed config and treats an explicit count as a floor, so the
CLI's count-only callers size per filament too. The placement clamp also
reserves a Type2 cone's base bulge, which the body box does not cover.
The planner-mirroring helpers sit beside the planners in WipeTower and
WipeTower2 so the two stay in sync; the libslic3r cases pin them to
footprints measured from generated G-code.
A raft is not a reason to reserve a tower. Print::apply runs
normalize_fdm_2, which clears enable_prime_tower for a plate that purges
one filament unless smooth timelapse or wrapping detection is on, so a
single-filament plate with a raft prints no tower at all and the estimate
was reserving bed area for one. Drop the input; need_wipe_tower is now
exactly the two exceptions normalize_fdm_2 honours, named there so the
next reason added has to be checked against it.
The GUI preview and the validation containment check each re-derived
"is a tower printed here" from the filament count instead of reading the
estimate, so both missed the towers printed with no tool change to purge
for. They now take the answer from the footprint, which is the drift this
shared estimate exists to remove. A tower that is not printed estimates to
zero, so its hull is degenerate and every check on it passes trivially -
the containment check needs no gate of its own.
WipeTowerData::width was written only by the pre-generation estimate and
left at zero for the whole post-generation life of the Print, while its
neighbour depth held the real value. Set it from the generator in both
branches.
The plate's height scan transformed every model part's full mesh per
instance on each scene reload, discarding all but the z extent. The
cached convex hull has the same z extent.
A plate loaded from a sliced .gcode.3mf holds no objects and its filaments
live in slice_filaments_info; the config-taking get_extruders overload
returned an empty list for it, which sized the tower for a placeholder two
filaments. It now answers the way the wx overload does, without reaching
the plater.
Also drop estimate_wipe_tower_size, which has no callers.
import_presets reduced each zip entry to a basename by stripping only
'/', so on Windows an entry named with '\' separators kept its
directory components and was extracted wherever they pointed. Strip
both separators, and reject any entry whose name still escapes the
extraction folder.
The preset name from the JSON and the bundle id from
bundle_structure.json were joined onto the preset directory unchecked
as well, which let either of them write outside it on every platform.
Both are now validated before anything is written.
The check is the is_path_within_root helper the 3MF importer already
had, moved to Utils so both importers share it. It treats '/' and '\'
as separators on every platform, so a bundle that would escape on one
OS is rejected on all of them.
* Make tree support deterministic without giving up its parallelism
* Break equal-distance ties in the tree support MST by coordinates
* test: cover the determinism this PR fixes
The MST unit tests here cover the tie-break, but the drop_nodes rework
has no test.
Adds two cases to the tree support suite. The thread-scheduling one
slices five configs twice each and compares the support point sequence,
which is what the node ordering moves. The MST tie one pins the branch
diameter and line width that carry Prim's equal-distance ties into the
toolpaths.
slice_with_tree_support takes an optional config list so the second case
can add the tree parameters it needs, and the double-slice comparison is
shared rather than written twice.
Both fail on main without this PR. The first passes from 60d1ceb580, the
second from e148865dd6.
---------
Co-authored-by: raistlin7447 <kris.austin@gmail.com>
* Fix fuzzy skin failing the slice: the minimum junction width was unscaled
* Unit Tests For Fuzzy Fix
* Cover ridged multifractal noise in the fuzzy skin width floor test
Its output is not bounded to [-1, 1], so it scales past the configured
thickness and drives the junction width negative. The floor has to hold
for any noise value, not just an in-range one.
Orca content-addresses every system filament, Bambu's included, but a printer,
its AMS and its vendor's cloud know only that vendor's own catalog ids. The
printer agent now translates between the two: outbound MQTT and FTP traffic, the
AMS mapping sent with a print job, and the ids written into a 3mf bound for the
printer all leave in the printer's own ids, while status messages, loaded
projects and SD-card prints arrive in Orca's. An id with no mapping passes
through unchanged, and an agent whose printers already speak Orca's ids
translates nothing at all.
Bambu's map is generated from BambuStudio's own shipped bundle; a missing or
unreadable file leaves every lookup an identity rather than taking the app down.
The profile check validates the map's shape, and profile CI now runs on the paths
that can change it. docs/HLSD/filament_id.md records the places the map
deliberately does not reach.
* Fix incorrect early exit for CLI mode no-support preventing parameters from being read
* Use PartPlate's m_height to allow CLI to perform proper BuildVolume check
* Add safeguard against extruder_pintable_heights and extruder_areas vector size mismatch
* Preserve printable_height precision in PartPlate/PartPlateList
* Fixed multiple BuildVolume warning issue, and keep check_outside diff minimal
# Description
<!--
> Please provide a summary of the changes made in this PR. Include
details such as:
> * What issue does this PR address or fix?
> * What new features or enhancements does this PR introduce?
> * Are there any breaking changes or dependencies that need to be
considered?
-->
Part 1 of 3 of the CLI-mode bug sweep, split out of #15452 per review
feedback there. This PR contains the crash fixes.
## Fixes
- **`--outputdir`/`--datadir` with a missing parent directory aborted**
via unguarded `create_directory`. Directories are now created
recursively, with a graceful early exit and a specific error message if
creation fails.
- **`--slice` + `--export-3mf` segfaulted on a from-scratch slice**:
`ConfigOptionVector::get_at()` on an empty vector is `.front()` of an
empty vector (UB). Guards added for `filament_color`/`filament_id` at
the CLI call site, and inside `DynamicPrintConfig::get_filament_type`
for `filament_type`/`filament_is_support`/`filament_id`. Only *empty*
vectors are treated as missing — the existing clamp-to-front behavior
for merely out-of-range indices is preserved, so GUI callers are
unaffected.
- **OOB heap write from stale `filament_self_index` on
`--load-filaments`** (fixes#14181): a 3MF carrying more
`filament_self_index` entries than loaded filaments wrote past the end
of `old_variant_counts`. The guard validates both bounds — entries `>
filament_count` *and* non-positive entries (`< 1`), since a single `0`
in an otherwise-valid array indexes `old_variant_counts[-1]`.
- **Wrong printable-area check** for non-rectangular beds: use the
printable area's bounding box instead of a naive vertex calculation
(fixes#15363).
- **`nozzle_height` and `align_center` were not read into the arrange
config** in CLI mode.
# Screenshots/Recordings/Graphs
<!--
> Please attach relevant screenshots to showcase the UI changes.
> Please attach images that can help explain the changes.
-->
## Tests
<!--
> Please describe the tests that you have conducted to verify the
changes made in this PR.
-->
- Repro'd each crash on CLI before the fix; all resolved after.
- `tests/libslic3r` suite passes; full binary builds clean on Linux.
- Added `get_filament_type` unit tests
<!--
> 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)
No IMEX code upstream, so nothing in this merge touches the feature. All 21 overlapping
files auto-resolved; verified every upstream addition is present in the merged tree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Reject invalid CLI argument values instead of silently accepting them
* Add read_cli accept/reject tests
* Update Option Type for LogFile argument
* Add read_cli vector option tests
* Accept common bool spellings on the CLI, cover --logfile in tests
* Add unit tests for truthy bool parsing
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>
