The no-tower case was gated on there being no purge volume, but the SEMM flush matrix reads every configured slot and is nonzero even when only one filament is used, so the plater preview drew a tower the print would never contain.
Four files overlapped, and each resolution favours upstream where the two sides
had done the same work:
WipeTower's M104/M109 tool qualifier. Both sides bounds-checked the physical
extruder map lookup; upstream omits the T qualifier when the map cannot answer,
where this branch fell back to the logical index and so named a carriage that may
not be the one printing. Upstream's behaviour is what this branch documents
elsewhere, so its version is taken and the local helper is dropped.
get_extruders' mixed-slot switch. Upstream added the same concept to the CLI
overload as expand_mixed_slots, so the GUI overloads' parameter is renamed to
match rather than carrying two names for one idea.
GLCanvas3D's sequential-clearance branch gains upstream's
update_compacted_wipe_tower_clearance for the by-layer case.
The printer_agent re-sync in TabPrinter::reload_config was upstream's and their
preset-undo fix removed it, so it goes; the IMEX modes grid re-sync beside it
stays, since it spans three options and is not a Field.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
imex_wipe_tower_hull() open-coded "does this plate print a tower?" as one
filament and no forcing reason. normalize_fdm_2, which is what actually
clears enable_prime_tower before slicing, has a second arm the copy omitted,
and reads the mixed-filament flag project-wide where the copy read the
plate. Two cases went wrong in opposite directions.
A ByObject plate with several objects prints no tower and the scene draws
none, yet the copy validated one and could refuse the slice with "the prime
tower overlaps an area reserved for IDEX/IQEX parallel printing" - with
nothing on screen to move.
A plate using one plain slot while some other slot in the project is a blend
does print a tower, because normalize_fdm_2 keeps it for any mixed filament
in the project, and the copy skipped validation entirely, so the tower could
be placed in a carriage zone and sliced.
prime_tower_is_printed() in libslic3r states the rule once, and both the
gate and a test use it. The counts are the ones normalize_fdm_2 is handed:
filament slots as authored, so a mixed slot counts once, and distinct
objects rather than instances. filament_is_mixed is a project option, so it
is passed in rather than read from the print preset.
The test drives every combination the rule looks at and compares the verdict
against normalize_fdm_2 itself, so the two cannot drift again without
failing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Toolchange Cyclic Order
* Apply cyclic order to first layer
* Unit test
* Copilot fixes
---------
Co-authored-by: Rodrigo Faselli <162915171+RF47@users.noreply.github.com>
* fix: bounds-check the toolchange flush-volume and HRC per-filament lookups
GCode::set_extruder's toolchange flush-volume lookup and
GCodeProcessor::update_slice_warnings's HRC check index per-filament and
per-extruder arrays (flush_volumes_matrix, the filament map, the nozzle list)
by filament/extruder id. When a config leaves one of those arrays shorter than
the filament count (partial or legacy multi-extruder projects, minimal
configs), the reads run off the end: silent on a normal STL, a hard abort under
_GLIBCXX_ASSERTIONS.
Route both reads through bounds checks: the flush lookup falls back to no flush,
matching the existing unknown-old-filament branch beside it, and the HRC check
skips an unmapped filament, mirroring the required_nozzle_HRC guard on the line
above. When the arrays are sized to the filament count the values are unchanged,
so correctly-specified configs are unaffected.
* ci: retrigger checks
* fix: guard H2C per-filament array reads against short config arrays
The H2C tool-ordering, wipe-tower, and g-code export paths index per-filament
config arrays by filament/tool id. A config with fewer entries than the filament
count (partial or legacy projects, minimal test configs) makes these reads run
past the end of the vector: silent under a normal STL, but UB that aborts under
the flatpak build's bounds-checked STL (_GLIBCXX_ASSERTIONS).
Route the reads through the existing clamping accessors (get_at,
get_filament_category, is_in_same_extruder) and add a small clamp helper for
filament_change_length. The guards are no-ops when the arrays are sized to the
filament count, so correctly specified configs are unaffected.
* fix: size the grouping context's filament_info to the filament count
build_filament_group_context built model_info.filament_info by walking
filament_type, so a config whose filament_type is shorter than the filament
count produced a short vector. FilamentGroup indexes filament_info by filament
id, so clamping the individual reads only moved the out-of-bounds access
downstream. Loop to filament_nums and read all three fields through get_at,
and drop filament_ids entries past the filament count, since the grouping code
pairs filament_ids and filament_info by position.
Adds a regression test with four filaments and one-entry filament_type /
filament_is_support. Without the fix it throws bad_alloc from copying a garbage
std::string read past the end.
* fix: guard the carousel nozzle-change length reads too
The carousel branch added in b90ac13d86/b0dddb4648 reads
m_filaments_change_length by tool id without a bounds check, the same
pattern this branch already routed through filament_change_length_at
a few lines above in both plan_toolchange and plan_tower_new.
* fix: guard WipeTower per-filament array reads against short config arrays
The BambuStudio WipeTower sync reintroduced raw per-filament array
indexing that reads out of bounds when a config leaves an array shorter
than the filament count: m_physical_extruder_map in format_line_M104/M109
(indexed even when empty), and m_filament_categories in get_wall_skip_points
and get_wall_filament_for_all_layer. Silent on a normal STL, a hard abort
under the bounds-checked STL the Flatpak build uses.
Bounds-check the physical extruder map before indexing (omitting the T
token, as the existing -1 path already does), and route the two raw
m_filament_categories reads through the clamping get_filament_category()
accessor the surrounding code already uses. No change for correctly-sized
configs.
Every printer's config block lists belt_slice_rotation_angle (default 45), so the processor marked all G-code as belt G-code: imported flat G-code got the belt view on a belt printer, and the belt-only Z handling in the processor ran for non-belt prints whose config block precedes the body. Take the angle only from outside the config block, where only the belt header writes it.
Painted support/seam facets, support volumes, seam occlusion, MMU and fuzzy skin painting (top/bottom
and side facets) and the adaptive infill octree used trafo_centered(), or trafo() with a centre-offset
shift, while the layers were sliced with the belt rotation, remap and Z lift; they now share
PrintObject::trafo_sliced().
Merge origin/main (00429da739) into belt-printer.
Conflicts resolved:
- src/CMakeLists.txt: keep both wxInspector workarounds.
- GCodeProcessor.cpp: keep the belt compare_pos / z_for_height lines.
- PrintObjectSlice.cpp: the belt bbox-Z guard also covers main's
printable_region_ids bookkeeping.
- TreeSupport.cpp: the belt-floor check runs before main's PendingNode
queueing.
- Tab.hpp: keep the belt fields, drop the removed upload description
fields.
- tests/libslic3r/CMakeLists.txt: keep both test files.
Also included:
- eSUN PLA belt presets declare their own filament_id (OFkrxQC4) and
scripts/filament_id_snapshot.json is regenerated, as main's filament_id
check requires.
- Custom.json version bumped to 02.04.00.05 so the belt entries reach
existing installs.
- Fix the ambiguous WithinRel call in the belt apron width test, which
otherwise breaks the fff_print build.
Two independent leaks of filament on the belt purge prism, plus the
replan safety net the second one needs.
1. The early-truncation scan bounded itself with the prism's own
toolchanges. ToolOrdering covers the whole print and the prism is a
printed object in it, so the "last toolchange" the scan found was on
the prism's own top layers -- it runs past every model object by
design -- and the truncation cancelled nothing. Bound the scan at the
tallest non-prism object (support layers included; on a belt they can
top the object). On MCTEST5 that was 197 toolchanges over 39.4 mm of
tower that no swap ever needed.
2. On a layer with no toolchange, the prism's entire fill printed as
solid infill in its own filament. Drop the fills no toolchange
claimed, right after the purge marking and before
ensure_perimeters_infills_order() force-overrides whatever is left.
Perimeters stay so the bar keeps a continuous wall. An earlier version
of this deleted the entities and had to be reverted: psWipeTower can
rerun without regenerating infill, and a later tool ordering may claim
what this one did not. The entities are now stashed with their layer,
region and index and put back exactly, the same reversibility contract
layer truncation already had.
3. Both stashes go stale if an object step reruns: make_fills() clears
and regenerates fills over m_layers only, so a stale stash would put
old fills back next to new ones, and truncated layers would keep old
perimeters/fills. Undo the plan's edits at the top of Print::process()
whenever psWipeTower is not done. Every object-step invalidation also
invalidates psWipeTower, so that condition is exactly "some object
step may rerun"; when it is done nothing regenerates and the edits
must stay. This also covers a prism left behind after belt mode is
turned off, which previously stayed truncated forever.
WipingExtrusions::is_entity_overridden() becomes public so the prism can
tell claimed fills from unclaimed ones.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SsuY8Laiyh7q2zPVVKV3HZ
build: clear eleven single-site clang-cl warning categories
Each of these is the last site left in its category, and every one is the
compiler saying it cannot tell what the code meant. Nothing here changes
defined behavior.
- OrcaSlicer_app_msvc.cpp printed a DWORD with %d
- StackWalker.cpp ran delete[] through an LPVOID
- ToolOrdering.cpp used a bare ; as a deliberate skip loop's body
- WipeTower.cpp had finish_block_tcr = finish_block_tcr, so the branch that
reached it did nothing. Folding the condition into the enclosing if leaves
the other branch untouched
- GCodeProcessor.cpp had an else binding to the inner if while the outer if
carried no braces
- AmsMappingPopupUpdate.cpp wrote >= 1 || <= 3 where its own comment says &&
- CalibrationWizardPresetPage.cpp left max_decimal_length unset through a
pair of conditions that cover every value but not visibly so
- DevManager.cpp bound map elements to pair<K, V> rather than
pair<const K, V>, copying every one
- SyncAmsInfoDialog.cpp had extraneous parentheses around a comparison
- Http.cpp had if (speed > 0.01) speed = speed;. speed now starts at 0 as
well, because curl_easy_getinfo leaves the target untouched when it fails
and the value reaches Progress either way
- SnapmakerPrinterAgent.cpp truncated npos into an unsigned int, so the
!= npos guard was always true. A colour with no # still yields 0, because
the wrap produced 0 as well
Nine categories go to zero. -Wtautological-overlap-compare and
-Wsometimes-uninitialized reach zero when #15583 merges their second site.
Every edit makes the precedence the compiler already applies explicit. None
of them regroups an expression, so behavior is unchanged at all eight sites.
Strip parentheses and whitespace from the diff and the token stream matches.
GCodeProcessor.cpp:1472 tests == where the symmetric clause below tests !=,
which reads like a typo and is not one. A comment now explains why.
OrcaSlicer.cpp:4760 was the only judgment call. Its leading !is_seq_print is
bare while both operands are parenthesized, so the written form matches what
the compiler does. Kept rather than guessed at.
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.
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.
Both change emitted G-code, which is why they were kept out of the extraction
commit. Both are wrong only where the machine mapping is non-identity, which is
the definition of each bug.
1. Suppress lifts commanded through an unknown position.
_travel_to_z() emits full XYZ whenever the mapping must emit every axis, because
the mapping can make machine Z depend on logical X/Y, and it builds that point
from m_pos. At print start, and after any custom G-code that invalidates
position, m_pos.xy is the uninitialised origin; mapping (0, 0, z) through a
non-identity remap produces a real but wrong machine point -- for a reverse
mapping, build_vol_max, i.e. the far corner of the bed. The subsequent full-XYZ
move corrects the position, but the lift has already commanded a rapid across
the whole bed at travel speed.
Belt kinematics already guarded this; the Cartesian path did not. The guard is
now applied at all three lift sites through must_skip_lift_now(), not just the
one the extraction covered: travel_to_xyz()'s pending-lift branch,
lazy_lift(spiral_vase=true), and eager_lift(). The latter two also needed the
state fix -- both recorded m_lifted = target_lift regardless, so suppressing
only the emission would leave a later unlift() descending from a height that was
never commanded.
2. Never emit a G2/G3 arc a mapping cannot represent.
extrude_arc_to_xy() emitted G2/G3 with logical X/Y and I/J and never consulted
the mapping. There is no general fix by transforming the arc: a permutation
moves it out of the XY plane that I/J describes, a negation reverses handedness,
and the belt shear maps a circle to an ellipse that G2/G3 cannot express at all.
So supports_arc_moves() gates generation through the existing
GCode::should_disable_arc_fitting() hook, and BeltGCode's special-case override
is deleted -- belt now gets the same behaviour from the general rule instead of
its own exception.
supports_arc_moves() is m_remap_x == 0 && m_remap_y == 1, not !has_axis_remap():
an arc emits only X/Y/I/J, so a mapping that merely negates or reverses Z leaves
every emitted word untouched and keeps its arcs.
The fallback for an unrepresentable arc tessellates it into linear segments at a
0.005mm chord tolerance rather than substituting a single chord, and splits dE
proportionally across the segments. The capability check is hoisted above every
extrusion mutation: an earlier form ran it after filament()->extrude(dE) and so
extruded 2*dE on the fallback path.
Known limits of that fallback, since it is worth stating rather than discovering:
emitted relative E is conserved only to per-segment rounding (a radius-5
semicircle with dE=1.5 emits 1.50012 across 36 segments); the 0.005mm bound is a
logical-frame bound, about 0.00855mm in machine space under a 45-degree belt
shear; unequal endpoint radii and non-finite inputs are unchecked. Ordinary
export takes the original polyline when the mapping rejects arcs, so this path
is a fallback rather than the normal route.
Known gap, not claimed fixed: classic wipe towers have their own
enable_arc_fitting and their own G2/G3 emitter in GCode/WipeTower.cpp, which
should_disable_arc_fitting() does not govern. Belt printers are barred from
classic wipe towers; a remapped Cartesian printer is not.
Tests in tests/fff_print/test_gcodewriter.cpp: reverse-X remap with unknown and
with known position plus an identity control; eager_lift emitting nothing and
recording nothing; the arc-capability matrix including the Z-only cases; and the
tessellated fallback. E accounting is asserted through used_filament() rather
than E(), which resets per line in relative-E mode.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011jgzj1sf53KMLPweZ8yeUQ
BeltGCodeWriter subclassed GCodeWriter and overrode seven methods, five of them
by copying the base body and changing the transform. The base writer already
carried an axis remap and already branched at each of its seven
coordinate-emission decisions; the subclass did the same branching with a
different transform, and the two copies had begun to drift.
Replace the inheritance with a strategy object owned by GCodeWriter:
CartesianKinematics to_machine = the existing apply_axis_remap; today's base
behaviour, moved rather than changed.
BeltKinematics to_machine = MachineFrameTransform o axis_remap o
BeltBackTransform, plus a world_coordinates variant for
the PA calibration generators.
New: src/libslic3r/GCode/MachineKinematics.{hpp,cpp}, GCode/BeltKinematics.{hpp,cpp}
Deleted: src/libslic3r/BeltGCodeWriter.{hpp,cpp} (341 lines)
Points worth a reviewer's attention:
* The predicate is must_emit_all_axes(), not couples_axes(). The base returns
true for any non-identity remap, including pure permutations that do not
physically couple axes, so the question is "must every axis word be
emitted", not a statement about kinematics.
* Every per-site word-omission branch is preserved. The base deliberately
emits X/Y only, or Z only, or drops Z when its quantised value is unchanged.
The strategy changes which transform applies, never whether words are
omitted.
* set_kinematics() replays the configured remap and build volume onto a newly
installed strategy, because BeltGCode::init_belt_writer runs before
GCode.cpp calls set_axis_remap/set_build_volume_max.
* uses_pointwise_travel_speed() preserves a pre-existing divergence rather
than introducing one: the base travel_to_xyz emits the raw configured travel
speed in its final branch, ignoring the first-layer value computed at the
top, whereas the belt path used the first-layer-aware value throughout. Both
are kept. Unifying them changes feedrates and belongs in its own change.
* The [BELT-DEBUG] block is deleted; it rate-limited itself with a
function-local static thread_local in the hot emission path, and this is the
commit that would otherwise have moved it into shared code.
This commit is intended to preserve existing export output. That is reviewed by
construction -- each emission site keeps its own omission branch and each policy
divergence is preserved -- and is NOT verified against a G-code diff corpus.
Building that corpus is the outstanding work here.
Two API-equivalence exceptions, neither reachable by any caller today:
* Belt kinematics with no plane pointer installed, m_is_first_layer true,
initial and normal travel speeds differing, travel_to_xyz() reaching its
final branch: the old belt writer selected the initial-layer speed, the new
writer selects the normal travel speed. The pending-lift and XY-only
branches keep their previous selection.
* Belt kinematics installed without set_force_normal_lift(true) and a
non-normal lift requested: the old belt writer forced a normal lift, the new
writer can take the slope branch.
The PA-pattern generator reaches the writer through explicit travel_to_z() /
travel_to_xy(), not travel_to_xyz() or the lazy/eager lift paths, and normal
belt export installs both the plane and the forced-normal-lift policy, so
neither exception changes output produced today. They are recorded because a
future caller could reach them.
tests/fff_print/test_gcodewriter.cpp was also not compiling before this branch:
it called writer.to_machine_coords(), a method that existed only on
BeltGCodeWriter. It never surfaced because the build targets OrcaSlicer, not
all, and BUILD_TESTS defaults to OFF, so that translation unit was outside every
compile path. Fixed here; the existing 30-degree coordinate assertions are kept
verbatim as the best available regression net.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011jgzj1sf53KMLPweZ8yeUQ
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>
None of these came from the review; they were found auditing the branch for the
same class of leak review comment 6 identified.
physical_extruder_map was normalised through effective_physical_extruder_map on
every Print::apply(), so any printer with more than one nozzle and no authored map
got the identity [0,1,...,n-1] where the single-element {0} default belongs. The
key carries two readings: the IMEX paths index it by logical extruder and need one
entry per extruder, while the inherited BBL paths read it through the clamping
get_at(), for which {0} means "everything is physical 0". Deriving unconditionally
imposed the IMEX reading on profiles that mean the other one, changing the config
block line and the {first_tools} / {first_filaments} / {curr_physical_extruder_id}
placeholders for multi-nozzle non-IMEX printers. Gated on is_imex; every consumer
needing the per-extruder form is already IMEX-gated, and profiles with an authored
map of the right length are unaffected either way.
That gating unmasked a latent out-of-bounds read: WipeTower's M104/M109 emitters
index m_physical_extruder_map by tool with no bounds check, which reads past the
end of the single-element default on a multi-nozzle machine. Upstream's bug, from
the BambuStudio wipe tower sync, previously hidden because the map was being
widened for everyone. Now bounds-checked, falling back to the tool's own index --
the form GCodeProcessor already uses for the same map.
Preset::save() and get_preset_differed_for_save() carried a branch storing the
full vector whenever a child and its parent had different lengths. It was written
against a set_with_nil that threw on mismatched sizes; upstream #13035 replaced
that with a tolerant version that keeps the child vector verbatim and nil-marks
only the overlapping range, and that fix was already in the tree when this branch
was rebased. Left in, it defeated the delta encoding for every user printer preset
whose extruder count differs from its parent's: a 7-extruder profile inheriting a
single-extruder base wrote out all of its per-variant retraction keys as literals,
including ones identical to the parent, pinning them against future vendor updates
while the UI still reported the preset as inheriting. Removed; the two save loops
are now identical to upstream. Note this only affects new saves -- presets already
written keep their frozen values until re-saved.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* build: enable /Zc:lambda for MSVC
MSVC keeps its legacy lambda processor under /std:c++17, which rejects
reading a constexpr constant inside a lambda that does not capture it
(C3493). No other compiler requires that capture, and clang reports it as
an unused one, so the two cannot both be satisfied without the flag.
/Zc:lambda selects the conforming lambda parser that clang and GCC
already use. It is implied by /std:c++20 and /permissive-, so it is only
needed while we are on C++17. clang-cl is conforming already and does not
take the flag.
It requires VS2019 16.8, so build_release_vs.bat now says 16.8+.
* build: clear 237 unused lambda capture warnings
236 captures across 81 files, 142 of them `this`. Removing an unused
capture changes no behavior; clang does not report a capture whose type
has a non-trivial destructor, so nothing held only to extend an object's
lifetime is in this set.
Nine of them are the second half of the warning, "is not required to be
captured for this use", where the capture is a const or constexpr value
the body does read. Those depend on the /Zc:lambda change in the previous
commit. One of them, in FillRectilinear.cpp, had been worked around with
an #ifndef __APPLE__ guard around the capture list, which is now gone.
GUI_ObjectTableSettings.cpp captured its reset button only to read it
inside #ifdef __WXOSX_MAC__. That branch now takes the button from the
event it is already handling.
* build: fail configure on MSVC older than 19.28 instead of dropping /Zc:lambda
cl.exe answers an unrecognized /Zc: sub-option with warning D9002 and keeps
going, so on VS2019 before 16.8 the flag is silently ignored and the build
instead dies with C3493 in FillRectilinear.cpp, nowhere near the cause.
* fix: delete three locals that are now unused
Their only remaining use was the lambda capture this branch removed. The
Clang builds set -Wno-unused-variable, so the build never flagged them.
---------
Co-authored-by: Rodrigo Faselli <162915171+RF47@users.noreply.github.com>
PartPlate::store_to_3mf_structure read first_layer_time from the indirect cali_bboxes_data struct,
which the GUI populates at Plater.cpp:10600 but the CLI never writes to. The result was uninitialized
memory leaking into slice_info.config
Read directly from get_slice_result()->initial_layer_time, which is populated by
GCodeProcessor::finalize() in both code paths and matches the pattern already used a few lines
above for gcode_prediction.
Also default-initialize PlateBBoxData::first_layer_time to 0.0f as a defense against any other consumer
reading it without an explicit write.
* Normalize the junction direction vector over XYZE
calc_vmax_junction_deviation() treats the dot product of two jd_unit_vec as a
cosine, but the vectors were scaled by 1 / block.distance, which is the XYZ
length. On an extruding move the E component then pushes the 4D norm above 1 and
the dot product below -1, so the corner reads as straighter than it is and is
planned too fast -- the more so the higher the flow. Measured on a 6 degree
corner at scv 5: 86.9mm/s with no extrusion, 94.4mm/s at 0.029mm/mm, 150.0mm/s
at 0.1mm/mm.
Neither firmware does that. Marlin normalizes over XYZE for any extruding move
(planner.cpp: `if (... || esteps > 0) normalize_junction_vector(unit_vec)`) and
Klipper leaves E out of the cosine entirely, dotting only axes_r[0..2]
(toolhead.py::Move.calc_junction). Normalizing satisfies both: with E normalized
in, the cosine differs from the XYZ-only one by ~1e-5 at printing flow rates.
This is a deliberate divergence from PrusaSlicer, which still scales by
1 / distance -- it carries an older Marlin's behaviour.
Travel moves are unaffected, their vector was already unit length.
Reported by Copilot in review of #15304.
* Test that extrusion rate does not change corner planning
The junction deviation tests were all travel-only, which is exactly why the E
component of the junction vector went unchecked. Cover it: the same corner has
to be planned the same whether nothing, an ordinary 0.42 x 0.2 line, or a fat
large-nozzle line is extruded through it, on both Klipper and Marlin 2.
Reported by Copilot in review of #15304.
* Plan corners with junction deviation where the firmware uses it
The time estimator only ever had the classic per-axis jerk model, which limits a
corner by the largest single-axis component of the velocity change. That is
anisotropic: the same corner is allowed sqrt(2) more speed on a diagonal than on
an axis, which paints a four-lobed ripple around every circular wall in the
actual speed and actual flow views, worst on small parts whose walls are made of
short segments.
Klipper has no classic jerk at all and Marlin 2 has none while M205 J is in use;
both plan corners with junction deviation, which sees only the corner angle. Add
that model and use it for those machines:
- Klipper: derived from the square corner velocity, as the firmware does
(jd = scv^2 * (sqrt(2) - 1) / max_accel), reading the scv from
machine_max_jerk_x, where process_SET_VELOCITY_LIMIT() already stores
SQUARE_CORNER_VELOCITY.
- Marlin 2: machine_max_junction_deviation, which was already loaded into the
machine limits but never reached the planner.
- Every other flavor keeps the classic jerk path unchanged.
The model has no per-axis jerk floor, so this also drops the hard slow spot the
estimator drew at the start of every loop from machine_max_jerk_e.
Toolpaths are unaffected: on a full export the only lines that change are M73.
The junction deviation maths, including Marlin's JD_HANDLE_SMALL_SEGMENTS arc
approximation, is ported from PrusaSlicer's src/libslic3r/GCode/GCodeProcessor.cpp.
The Klipper mapping is not in PrusaSlicer, which ignores SET_VELOCITY_LIMIT.
* Add tests for junction deviation corner planning
Cover the three properties the change rests on:
- a right angle on Klipper is planned at exactly the square corner velocity,
the identity that makes the scv to junction deviation mapping correct, and a
shallow corner is planned far faster than per-axis jerk allows;
- junction deviation gives the same speed whatever the corner's orientation,
while classic jerk keeps its sqrt(2) spread, which is the four-lobed ripple;
- machines that do not plan with junction deviation are provably untouched,
including a Marlin 2 printer that has it disabled.
- Emit coincident belt_brim_by_layer bands even when the leading object layer
has no InstanceVisit (zero-extrusion lead-in / no coinciding support), so the
brim at first belt contact is no longer dropped.
- Register each coincident band's brim filament in ToolOrdering and emit each
band exactly once, in its brim-filament pass; emit ordinary-layer aprons in
the brim pass before object extrusion (correct tool, brim-first) instead of
with whatever tool was active.
- has_belt_brim(): inner-only brims need brim_width>0 (leading/extra produce no
inner geometry), fixing spurious prime-tower/spiral rejection; mirror in
wants_brim. Single-extruder/single-object output is unchanged except
previously-dropped bands now print.
reset() cleared the sibling machine_frame_transform_active but not
belt_tilt_angle/belt_z_origin/preslice_remap_*; a reused result carried
stale belt metadata into a subsequent normal print, flipping the store_z
branch and corrupting start-gcode preview Z for non-belt prints.
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.
Kris Austin