Harden #5070 fix + disable dead change_part_type() via #if 0 (#13281)

* 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.
This commit is contained in:
Damir Galeev
2026-04-23 16:12:18 +03:00
committed by GitHub
parent b2ce9f649f
commit ff3ad5ed98
2 changed files with 29 additions and 0 deletions

View File

@@ -29,6 +29,7 @@
#include <unordered_map>
#include <functional>
#include <boost/algorithm/string.hpp>
#include <boost/log/trivial.hpp>
#include <wx/progdlg.h>
#include <wx/listbook.h>
#include <wx/numformatter.h>
@@ -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; });

View File

@@ -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();