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>
This commit is contained in:
Clifford Garwood
2026-09-18 01:32:26 -04:00
co-authored by Claude Opus 5
parent b17414b160
commit e309029b28
11 changed files with 170 additions and 54 deletions
+12 -8
View File
@@ -408,20 +408,24 @@ void IMEXModesCtrl::add_row(const std::string& name,
// raw_row = flip_y ? (n_rows-1-row) : row
// raw_col = flip_x ? (n_cols-1-col) : col
//
// row_start: anchor displayed rows to the gantry row containing the Primary
// assignment, so reducing gantry count keeps the meaningful row visible.
int primary_gantry_row = 0;
for (const auto& [idx, role] : tool_roles) {
if (role == ImexRole::Primary) { primary_gantry_row = idx / m_n_cols; break; }
}
int row_start = primary_gantry_row; // display rows [row_start .. row_start+m_n_rows-1]
// The displayed window is always the whole grid: m_n_rows is the gantry count, so valid tool
// indices are 0 .. m_n_rows*m_n_cols-1 and rows 0 .. m_n_rows-1. A window m_n_rows tall can
// only lie entirely inside the grid when it starts at row 0.
//
// This used to anchor the window's first row to the Primary's gantry row, to "keep the
// meaningful row visible" when the gantry count shrank. That cannot work: shifting a
// full-height window forward walks it off the end, so it drew tiles for tools that do not
// exist and hid real ones, and clicking a phantom tile wrote a tool index that
// compute_imex_zone_layout() (IMEXZones.cpp:76) then discards. A Primary sitting outside the
// current grid is a data problem -- only reachable from a hand-authored mode string, since
// the editor will not move Primary off tool 0 -- and the zone layout already answers it by
// producing no zones at all. Showing the real grid is the honest rendering of that state.
bool flip_x = (m_layout == 1 || m_layout == 3);
bool flip_y = (m_layout == 2 || m_layout == 3);
for (int row = m_n_rows - 1; row >= 0; --row) {
for (int col = 0; col < m_n_cols; ++col) {
int raw_row = flip_y ? (m_n_rows - 1 - row) : row;
raw_row += row_start; // anchor to Primary's gantry row
int raw_col = flip_x ? (m_n_cols - 1 - col) : col;
int tool_idx = raw_row * m_n_cols + raw_col;
std::optional<ImexRole> role; // nullopt == Inactive