From ff3ad5ed98e99f1c4b9cba0917f625b3728fd915 Mon Sep 17 00:00:00 2001 From: Damir Galeev <49319928+DImagine@users.noreply.github.com> Date: Thu, 23 Apr 2026 16:12:18 +0300 Subject: [PATCH] Harden #5070 fix + disable dead change_part_type() via #if 0 (#13281) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Restore SVG/text→Support crash guard (#5070), disable change_part_type() The crash fix from #5070 prevented converting SVG/text modifiers to Support Blocker / Support Enforcer by hiding those entries in the old SingleChoiceDialog opened from ObjectList::change_part_type(). When the "Change type" UI was reworked into a submenu that calls ObjectList::set_volume_type() directly, the guard was left behind in the unused change_part_type() path -- the submenu path had no protection, silently reintroducing the #5070 crash on the submenu route (ModelVolume::set_type() does not clear text_configuration / emboss_shape, so the stale emboss state stays attached to a support volume). - Guard set_volume_type(): reject SUPPORT_BLOCKER / SUPPORT_ENFORCER when any selected volume is SVG or text, showing the same error as before. - Wrap ObjectList::change_part_type() in #if 0 instead of deleting it: the function is unused since the submenu rework, but keeping it in commented-out form preserves traceability with upstream Bambu Studio (avoids merge conflicts on each BBL sync). Suggested by RF47, confirmed by ianalexis. * Log instead of showing user error; action guard is defense-in-depth only Per review feedback from RF47: with the UI-side guard in #13120, the set_volume_type() action-time check is a pure safety net -- a healthy UI should never let the user reach it. Showing a modal error dialog would therefore either confuse a user who did nothing wrong (if the UI guard is bypassed by a future refactor/plugin/shortcut) or be dead code at runtime. Log the event for developers instead, keep the early return so the crash is still prevented. --- src/slic3r/GUI/GUI_ObjectList.cpp | 27 +++++++++++++++++++++++++++ src/slic3r/GUI/GUI_ObjectList.hpp | 2 ++ 2 files changed, 29 insertions(+) diff --git a/src/slic3r/GUI/GUI_ObjectList.cpp b/src/slic3r/GUI/GUI_ObjectList.cpp index c0a790961b..da0326704a 100644 --- a/src/slic3r/GUI/GUI_ObjectList.cpp +++ b/src/slic3r/GUI/GUI_ObjectList.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -5377,6 +5378,11 @@ ModelVolume* ObjectList::get_selected_model_volume() return (*m_objects)[obj_idx]->volumes[vol_idx]; } +// ORCA: kept as dead code (not called by any active path). Preserved in #if 0 +// form for traceability with upstream Bambu Studio -- removing outright would +// produce merge conflicts on every BBL sync. The active "Change Type" UI goes +// through the submenu in GUI_Factories.cpp -> ObjectList::set_volume_type(). +#if 0 void ObjectList::change_part_type() { wxDataViewItemArray selections; @@ -5573,6 +5579,7 @@ void ObjectList::change_part_type() return; } +#endif ModelVolumeType ObjectList::get_selected_volume_type() { @@ -5646,6 +5653,26 @@ void ObjectList::set_volume_type(ModelVolumeType new_type) return; } + // Defense-in-depth safety net against issue #5070: SVG/text volumes carry emboss metadata + // (text_configuration, emboss_shape) that only makes sense for Part / Negative Part / Modifier. + // ModelVolume::set_type() does not clear that metadata, so converting such volumes to + // Support Blocker / Support Enforcer leaves stale emboss state attached to a support volume + // and historically crashed (originally fixed in the now-disabled change_part_type() by hiding + // Support entries in the old choice dialog; the UI-side guard for the current submenu lives + // in MenuFactory::append_menu_item_change_type, see #13120). + // This block must never be reachable under a healthy UI; if it ever logs, the UI guard has + // been bypassed (new entry point, refactor, plugin, etc.) and should be investigated. + if (new_type == ModelVolumeType::SUPPORT_BLOCKER || new_type == ModelVolumeType::SUPPORT_ENFORCER) { + const bool has_text_or_svg = std::any_of(volumes.begin(), volumes.end(), + [](const VolumeSelection& sel) { return sel.volume->is_svg() || sel.volume->is_text(); }); + if (has_text_or_svg) { + BOOST_LOG_TRIVIAL(error) << __FUNCTION__ + << ": blocked attempt to set SUPPORT_BLOCKER/ENFORCER on SVG/text volume; " + << "UI guard should have prevented this -- possible regression in the Change Type menu"; + return; + } + } + const bool any_diff = std::any_of(volumes.begin(), volumes.end(), [new_type](const VolumeSelection& sel) { return sel.volume->type() != new_type; }); diff --git a/src/slic3r/GUI/GUI_ObjectList.hpp b/src/slic3r/GUI/GUI_ObjectList.hpp index 8fe85dc7f6..ed0dfabd11 100644 --- a/src/slic3r/GUI/GUI_ObjectList.hpp +++ b/src/slic3r/GUI/GUI_ObjectList.hpp @@ -413,7 +413,9 @@ public: bool fix_cut_selection(wxDataViewItemArray &sels); ModelVolume* get_selected_model_volume(); +#if 0 // ORCA: disabled alongside definition in GUI_ObjectList.cpp (see #if 0 block there) void change_part_type(); +#endif void set_volume_type(ModelVolumeType new_type); ModelVolumeType get_selected_volume_type();