Commit Graph
6 Commits
Author SHA1 Message Date
Clifford GarwoodandClaude Opus 5 3c7b62bbca Offer the mode names this printer can carry in the name field
The name field becomes an editable ComboBox, the same pattern the sidebar uses
for parameters like sparse infill anchor length: predefined entries in a
drop-down, with the text still typeable. The mode table is authored for whatever
hardware the user has, so a closed list would be wrong -- nothing in the slicer
reads a mode's name except as the key a plate stores -- but the conventional
names are worth offering rather than leaving everyone to retype them.

What is offered follows the tool grid rather than a fixed list. Four carriages
get iq-copy and iq-mirror; multicolor needs a Span partner beside the primary
and a second gantry to copy the pair onto, so mc-copy and mc-mirror appear only
on a grid that can hold one. imex_resolve_routing() already refuses a multicolor
mode with no Span on the primary's gantry, and suggesting a name it would then
reject is worse than not suggesting it. Names a row already uses are dropped, so
the list only ever offers what is still free.

Two things about ComboBox matter when it is editable, which nothing else in the
tree does -- the other 78 call sites all pass wxCB_READONLY:

GetValue() returns the drop-down selection whenever there is one, so a name
typed after picking a suggestion would be silently discarded. Every read goes
through GetTextCtrl() instead. Field.cpp reconciles the same way for its own
open enums.

The constructor hands its value to TextInput as the LABEL -- the small
right-aligned slot a unit like "mm" occupies -- because a read-only combo hides
the text control and shows the label in its place. Left there, the name rendered
as a greyed echo beside the hint while the field itself sat empty. The combo is
built empty and the value written to the text control, and the selection handler
clears the label again afterwards, since SetSelection() writes there too.

The mode column widens to 176 to leave room for the drop-down arrow, with the
header spacer deriving from the same constant.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-10-01 18:37:53 -04:00
Clifford GarwoodandClaude Opus 5 dafc4ce39c Rework the parallel modes editor from the UI review
Addresses the interface notes on the IDEX/IQEX modes editor.

Add Mode moves from below the rows to the top of the panel, beside a "?" button
that now carries the overview text as its tooltip. At the bottom the button
shifted down the page every time a mode was added, so where it sat depended on
how many modes already existed. It is an Orca Button in the Confirm style, width
matched to the mode column it creates a row in, and the panel opens on the
legend rather than on a paragraph.

Remove moves out of the right-hand column, where it sat one icon away from
Edit -- a destructive control beside the one pressed most -- to under the name
field it deletes, and its icon becomes a boxed minus rather than an X, which
read as "close". Reset joins Edit in the right-hand column, which is now
top-aligned so the icons hold position regardless of row height. Tool tiles are
square at 24px, and the header spacer tracks that width so the column titles
stay over their columns when the grid changes shape.

The two text fields were landing on GTK's near-black default border, invisible
against the panel: measured 45,45,49 against a 43,43,43 background, where the
settings fields above use 74,74,81. wxTextCtrl cannot color its own border, so
each sits in a one pixel frame taking the color TextInput derives for the
theme, and carries wxBORDER_NONE so Windows and macOS do not draw a native edge
inside it. The G-code boxes also take the monospace face EditGCodeDialog uses.

Bed zone fills drop to roughly half opacity in the Standard theme. They cover
whole quadrants for a whole session, so at swatch saturation they dominate the
scene. The collision strip is dimmed less, since it marks where a head hits
something. The deuteranopia, tritanopia and high contrast themes keep their
alphas: those are chosen for discriminability, which is the opposite trade.

Also fixes the icon size never applying. All four ScalableButton call sites
passed eight arguments, so the size bound to use_default_disabled_bitmap and
bmp_px_cnt kept its default of 16. Both are passed now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-30 23:38:37 -04:00
Clifford GarwoodandClaude Opus 5 98544fd6bc Bound the parallel pressure-advance lookup to the filament count
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>
2026-09-18 01:32:26 -04:00
Clifford GarwoodandClaude Opus 5 e309029b28 Fix extruder-map bounds, popover lifetime, and arrange thread safety
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>
2026-09-18 01:32:26 -04:00
Clifford GarwoodandClaude Opus 5 5eac300d91 Use IDEX/IQEX in the user-facing strings
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>
2026-09-03 10:05:37 -04:00
Clifford GarwoodandClaude Opus 5 f75bdd6013 Move the IMEX modes editor into its own file and fix four defects in it
Closes review comments 21, 4 and 12, and the widget half of 19.

- 21: IMEXModesCtrl was 572 lines inside Tab.cpp. It now lives in
  IMEXModesCtrl.{hpp,cpp} next to IMEXFilamentPickerPopover, which was the
  precedent named in the comment. The move itself is exact -- member order,
  comments and every string literal unchanged -- and the class had no file-local
  dependencies in Tab.cpp, only its include list, so the new source states those
  explicitly.
- 4: a mode row with an empty Name was silently dropped on save, tools and G-code
  with it, and matches_config() compared against that same filtered output so the
  preset never went dirty and the row stayed on screen. Rows are now given a
  generated unique name instead of being discarded, and add_row() pre-fills one so
  the common path never produces a blank. Names are deliberately not translated:
  objects store a mode name in imex_parallel_mode and GCode.cpp matches it by
  string, so a localized name would break a project reopened in another language.
- 12: the editor had a third parser that read a bare token and an unknown role
  suffix as Primary, while parse_imex_active_tools reads both as Copy -- so the
  editor and the slicer could read one imex_mode_active_tools string two different
  ways. Deleted; the editor now uses the same two helpers the slicer does.
- 19: tile state was an int shadowing ImexRole, with the role letters duplicated in
  a second switch that wrote the on-disk format. The tile now holds
  optional<ImexRole>, with Inactive spelled as the absence of a role rather than a
  fifth integer, and the letters come from kImexRoleTable.

Four further changes, from testing rather than the review:

- Deleting a mode reported only the row's current name, so renaming a mode and then
  deleting it left every plate using it stranded on a name that no longer exists.
  Both the build-time and current names are now reported, minus any a surviving row
  still carries.
- The instruction text and colour legend were built once in the constructor and
  never rebuilt, so raising gantry count to 2 gave the tiles a Span role the legend
  never explained until the preset was saved and the page reopened. Both are
  rebuilt with the grid, and the per-role detail moved into legend tooltips so the
  panel no longer opens with a paragraph.
- The tile holding Primary is now read-only. Primary is tool 0 and moves only via
  Tool 0 Position; a click could previously demote the only Primary, leaving a mode
  that parses to no primary at all, which degrades the plate to an ordinary
  single-tool print with nothing in the editor showing what is wrong. A mode
  arriving without a Primary keeps every tile live so it can still be repaired.
- Names and G-code were read with ToStdString() (the ANSI codepage on Windows) and
  written with from_u8() (UTF-8). On a non-UTF-8 codepage a name like "Modus A"
  with a diaeresis was stored as invalid UTF-8, came back blank, and was then
  silently renamed by the auto-naming above. Every read is now into_u8() and every
  write from_u8(); EditGCodeDialog was affected in both directions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-03 00:56:26 -04:00