Skip to content

fix: unify filament temp mixing blocking with plate error gating - #589

Open
iesteem wants to merge 5 commits into
mainfrom
fix_top_cover
Open

fix: unify filament temp mixing blocking with plate error gating#589
iesteem wants to merge 5 commits into
mainfrom
fix_top_cover

Conversation

@iesteem

@iesteem iesteem commented Jul 15, 2026

Copy link
Copy Markdown

Summary

Five commits hardening the filament temperature mixing (high/low temp materials on the same plate) blocking path, plus a related defensive fix.

Commit-by-commit

  1. fix: unify filament temp mixing blocking with plate error gating

    • Remove has_sliceable_plate_for_slice_all() hijack in the Preview-tab switch that triggered an unintended "slice all" instead of switching to preview.
    • Add m_filament_temp_blocked flag to PartPlate so can_slice() reflects the filament-temp-mixing state, consistent with m_apply_invalid for object-outside errors.
    • sync_filament_temp_mixing_notification() manages its own flag, independent of the background validation system.
    • Simplify EVT_GLVIEWTOOLBAR_PREVIEW to use can_slice() as a unified gate for both error types.
  2. fix: use on-demand checks instead of cached flag for filament temp mixing

    • Replace the per-plate cached m_filament_temp_blocked flag with on-demand is_plate_blocked_by_filament_temp_mixing() checks in the rendering and preview-switch paths.
    • Eliminates staleness risks identified during code review where non-current plates could retain stale blocked state after filament changes, undo/redo, or plate switching during slicing.
    • GLCanvas3D: on-demand red-exclamation check on plate thumbnails; Plater: on-demand check in EVT_GLVIEWTOOLBAR_PREVIEW.
  3. refactor: simplify filament slot collection in check_filament_temp_mixing

    • Merge duplicate key lists, rename used_slots for clarity, group logic into scoped blocks. No semantic change.
  4. refactor: rename STRING_EXCEPT_FILAMENTS_DIFFERENT_TEMP to STRING_EXCEPT_FILAMENTS_MIXING_TEMP

    • Better reflects that the check is about high/low temperature material mixing, not just "different" temperatures.
    • Touches PrintBase.hpp, Snapmaker_Orca.cpp (CLI), and Plater.cpp.
  5. feat: show per-filament high/low temp breakdown in mixing notifications

    • Plater::check_filament_temp_mixing now exposes a FilamentTempMixingDetail (high/low 1-based slot lists) via a new overload.
    • Notification text becomes dynamic; sync_filament_temp_mixing_notification caches the exact pushed strings to work around NotificationManager's exact-text close matching.
    • Added slice-all variants listing every conflicting plate.

Related defensive fix (bundled)

  • GCodeReader.cpp: parse_file_raw_internal() returns false when fopen fails, preventing fread(NULL) from triggering a CRT invalid-parameter crash.
  • Localization: zh_CN/Snapmaker_Orca_zh_CN.po updated with the new dynamic notification strings (Chinese only; other languages to be synced by CI / update-translation.yml).

Downstream effects

Red exclamation icon on plate thumbnails, disabled slice/export buttons, and blocked auto-slice on preview switch now work uniformly for both object-outside and filament-temp-mixing errors.


Reviewer Focus

Please prioritize the following (ordered by risk):

1. On-demand vs cached flag tradeoff (most important)

Commit 2 reverted the cached flag from commit 1 in favor of on-demand checks. Please confirm:

  • is_plate_blocked_by_filament_temp_mixing() is cheap enough to call on every plate-thumbnail render (GLCanvas3D::_render_imgui_select_plate_toolbar) — it walks model().objects and resolves presets each call. For projects with many plates / objects, is there measurable render-time impact?
  • All read sites that previously trusted m_filament_temp_blocked now go through the on-demand check. Are there any remaining readers of the (now-removed) flag that would silently compile to false?
  • Was the cached flag kept anywhere (e.g. for PartPlate::can_slice())? If can_slice() no longer reflects temp-mixing state, the "unified gating" promise of commit 1 may regress.

2. EVT_GLVIEWTOOLBAR_PREVIEW behavior change

Before: clicking Preview with a sliceable plate auto-fired slice_all.
After: blocking checks run first; if blocked, switch to Preview view without slicing.
Please confirm the non-blocking path still auto-slices as before, and that the workflow change is intentional and surfaced to users (release notes / changelog).

3. Notification close-then-push correctness

sync_filament_temp_mixing_notification() now caches filament_temp_mixing_last_error_text / filament_temp_mixing_last_warning_text for exact-text close. Please check:

  • All early-return paths (e.g. curr_plate == nullptr) — do they leak a stale notification by skipping the close step?
  • Across the three triggers (plate switch, filament change, bed-type change), the previous notification is always cleared before pushing the new one.
  • Slice-all mode caches a different text shape than single-plate mode; verify the close path matches the right cache when switching modes.

4. Slot-collection refactor (commit 3) is purely cosmetic

"No semantic change" is claimed. Please diff-check:

  • The merged key list covers exactly what the two original lists covered (wall_filament, sparse_infill_filament, solid_infill_filament, support_filament, support_interface_filament, wipe_tower_filament).
  • The uses_default_extruder branch + global extruder resolution order is unchanged.
  • The 1-based → 0-based slot conversion is preserved.

5. Enum rename impact

STRING_EXCEPT_FILAMENTS_DIFFERENT_TEMPSTRING_EXCEPT_FILAMENTS_MIXING_TEMP. Search the whole repo (including 3MF serialization, Sentry reporting, CLI) to confirm:

  • No missed call sites.
  • No persistence layer stores this enum by name (only by integer value = 2, which is preserved).

6. Per-filament breakdown formatting

The notification text is now dynamic and lists slots like [1] PLA White; [3] PETG Black. Please confirm:

  • Slot labels match what the user actually sees (preset name resolution, 1-based indexing).
  • Translation still works for the surrounding template — i.e. dynamic slot lists don't break gettext format strings (%1% etc.).
  • For very long slot lists (10+ filaments), does the notification render sanely (no truncation / layout break)?

7. Bundled GCodeReader.cpp fix

  • The fopen NULL check is a defensive fix unrelated to filament temp mixing — confirm it's intentional to bundle here (vs. a separate PR).
  • Does any Sentry crash fingerprint depend on this path crashing? If so, the fix will silence those reports.

8. i18n coverage

Only zh_CN.po is updated; the other 20 languages will fall back to English for the new dynamic strings. Is that acceptable for merge, or should we wait for update-translation.yml to sync?


Test Plan

  • Plate with both high-temp and low-temp filaments in use → ValidateError fires, slice button greyed out, red exclamation on plate thumbnail
  • Click Preview tab on a blocked plate → switches to Preview without auto-slicing; notification remains visible
  • Click Preview tab on a compatible plate → still auto-slices via slice_all (no regression)
  • Resolve the mixing conflict (swap a filament) → notification clears immediately, slice button re-enables, exclamation disappears (no stale state)
  • Undo/redo a filament swap → blocking state tracks correctly (verifies the on-demand check, not a stale cached flag)
  • Switch plates while slicing → non-current plates don't retain stale blocked state
  • Slice All with multiple conflicting plates → notification lists each plate with its high/low slot breakdown
  • Slice All with one blocked / one clean plate → blocked plate skipped, clean plate slices
  • Notification text in zh_CN renders correctly; English / other locales render with English fallback (no crash, no format-string breakage)
  • CLI slice on a mixing-conflict project → still reports the error via CLI_FILAMENTS_DIFFERENT_TEMP (rename is internal-only)
  • Loading a G-code file whose path is invalid → returns cleanly, no CRT crash

🤖 Generated with Claude Code

iesteem and others added 5 commits July 15, 2026 19:11
- Remove has_sliceable_plate_for_slice_all() hijack in Preview tab switch
  that triggered unintended "slice all" instead of switching to preview
- Add m_filament_temp_blocked flag to PartPlate so can_slice() reflects
  filament temp mixing state, consistent with m_apply_invalid for
  object-outside errors
- Let sync_filament_temp_mixing_notification() manage its own flag,
  independent of the background validation system
- Simplify EVT_GLVIEWTOOLBAR_PREVIEW to use can_slice() as unified gate
  for both error types

Downstream effects: red exclamation icon on plate thumbnails, disabled
slice/export buttons, and blocked auto-slice on preview switch now work
uniformly for both object-outside and filament-temp-mixing errors.
…xing

Replace the per-plate cached m_filament_temp_blocked flag with on-demand
is_plate_blocked_by_filament_temp_mixing() checks in the rendering and
preview-switch paths. This eliminates staleness risks identified during
code review where non-current plates could retain stale blocked state
after filament changes, undo/redo, or plate switching during slicing.

Changes:
- GLCanvas3D: add on-demand check for red exclamation on plate thumbnails
- Plater: add on-demand check in EVT_GLVIEWTOOLBAR_PREVIEW alongside
  can_slice(), keep sync_filament_temp_mixing_notification() for
  notification freshness
- PartPlate: revert cached flag; can_slice() unchanged

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…xing

Merge duplicate key lists, rename used_slots for clarity, and group logic
into scoped blocks. No semantic change.
…EPT_FILAMENTS_MIXING_TEMP

Better reflects that the check is about high/low temperature material mixing,
not just "different" temperatures.
Add per-filament high/low temperature breakdown to the filament temp
mixing warning/error notifications so users can see exactly which slots
are in conflict, both for single-plate and slice-all modes.

- Plater::check_filament_temp_mixing now exposes a FilamentTempMixingDetail
  (high/low 1-based slot lists) via a new overload
- Notification text becomes dynamic; sync_filament_temp_mixing_notification
  caches the exact pushed strings to work around NotificationManager's
  exact-text close matching
- confirm_filament_temp_mixing_before_slice / _before_slice_all use the
  same breakdown in their modal dialogs
- Safe translation helper tr_u8() avoids the dangling-pointer trap in
  I18N::translate_utf8() that crashed in non-English locales
- Add zh_CN translations for the new strings

Also fix a separate crash: GCodeReader::parse_file_raw_internal no longer
dereferences a NULL FILE* when fopen fails (e.g. loading a non-existent
gcode file at startup), and Plater::load_gcode now checks wxFileExists
up front to give a precise "does not exist" error instead of the vague
"does not contain valid G-code".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant