diff --git a/src/slic3r/GUI/DesignPanel.cpp b/src/slic3r/GUI/DesignPanel.cpp index 2f7a49963b..85104c683e 100644 --- a/src/slic3r/GUI/DesignPanel.cpp +++ b/src/slic3r/GUI/DesignPanel.cpp @@ -4693,6 +4693,30 @@ void DesignPanel::populate_body_choices(int as_of_feature) fill(m_cut_target, 0); // Cut tool: default to the first body } +void DesignPanel::fill_body_choice(ComboBox* c, int as_of_feature, int want) +{ + if (!c) return; + // Same replay as populate_body_choices, for the single-combo tools. A stored target_body + // indexes the body list AS IT WAS just before that feature ran; listing the final bodies + // instead makes the saved index select whatever now sits at that position, which is a + // different body as soon as a later Cut splits one (indices shift up) or a later Boolean + // consumes its tool body (indices shift down). + const std::vector* src = &m_doc.bodies; + std::vector as_of; + if (as_of_feature >= 0 && as_of_feature <= int(m_doc.features.size())) { + CadDocument tmp = m_doc; + tmp.features.resize(as_of_feature); + if (tmp.recompute() && !tmp.bodies.empty()) { as_of = tmp.bodies; src = &as_of; } + } + c->Clear(); + for (size_t i = 0; i < src->size(); ++i) { + const std::string& n = (*src)[i].name; + c->Append(n.empty() ? wxString::Format(_L("Body %zu"), i + 1) : wxString::FromUTF8(n)); + } + if (want >= 0 && want < int(c->GetCount())) c->SetSelection(want); + else if (c->GetCount() > 0) c->SetSelection(0); +} + void DesignPanel::on_add_boolean() { if (m_doc.bodies.size() < 2) { @@ -7006,16 +7030,7 @@ void DesignPanel::load_feature_into_dialog(const CadFeature& f) select_sheet_choice(m_surf_thicken_body, f.target_body); break; case CadFeatureType::Transform: { - { - m_xf_body->Clear(); - for (size_t i = 0; i < m_doc.bodies.size(); ++i) { - const std::string& n = m_doc.bodies[i].name; - m_xf_body->Append(n.empty() ? wxString::Format(_L("Body %zu"), i + 1) : wxString::FromUTF8(n)); - } - if (f.target_body >= 0 && f.target_body < int(m_xf_body->GetCount())) - m_xf_body->SetSelection(f.target_body); - else if (m_xf_body->GetCount() > 0) m_xf_body->SetSelection(0); - } + fill_body_choice(m_xf_body, m_edit_index, f.target_body); m_xf_dx->SetValue(f.xf_translate.x()); m_xf_dy->SetValue(f.xf_translate.y()); m_xf_dz->SetValue(f.xf_translate.z()); @@ -7029,32 +7044,14 @@ void DesignPanel::load_feature_into_dialog(const CadFeature& f) break; } case CadFeatureType::Mirror: { - { - m_mirror_body->Clear(); - for (size_t i = 0; i < m_doc.bodies.size(); ++i) { - const std::string& n = m_doc.bodies[i].name; - m_mirror_body->Append(n.empty() ? wxString::Format(_L("Body %zu"), i + 1) : wxString::FromUTF8(n)); - } - if (f.target_body >= 0 && f.target_body < int(m_mirror_body->GetCount())) - m_mirror_body->SetSelection(f.target_body); - else if (m_mirror_body->GetCount() > 0) m_mirror_body->SetSelection(0); - } + fill_body_choice(m_mirror_body, m_edit_index, f.target_body); populate_plane_choices(m_mirror_plane); m_mirror_plane->SetSelection(index_from_plane(f.plane)); m_mirror_keep->SetValue(f.mirror_keep_original); break; } case CadFeatureType::Thicken: { - { - m_thicken_body->Clear(); - for (size_t i = 0; i < m_doc.bodies.size(); ++i) { - const std::string& n = m_doc.bodies[i].name; - m_thicken_body->Append(n.empty() ? wxString::Format(_L("Body %zu"), i + 1) : wxString::FromUTF8(n)); - } - if (f.target_body >= 0 && f.target_body < int(m_thicken_body->GetCount())) - m_thicken_body->SetSelection(f.target_body); - else if (m_thicken_body->GetCount() > 0) m_thicken_body->SetSelection(0); - } + fill_body_choice(m_thicken_body, m_edit_index, f.target_body); m_sel_solid_face = f.thicken_face; m_thicken_face_label->SetLabel(f.thicken_face >= 0 ? wxString::Format(_L("Face %d"), f.thicken_face) @@ -7064,16 +7061,7 @@ void DesignPanel::load_feature_into_dialog(const CadFeature& f) break; } case CadFeatureType::Rib: { - { - m_rib_body->Clear(); - for (size_t i = 0; i < m_doc.bodies.size(); ++i) { - const std::string& n = m_doc.bodies[i].name; - m_rib_body->Append(n.empty() ? wxString::Format(_L("Body %zu"), i + 1) : wxString::FromUTF8(n)); - } - if (f.target_body >= 0 && f.target_body < int(m_rib_body->GetCount())) - m_rib_body->SetSelection(f.target_body); - else if (m_rib_body->GetCount() > 0) m_rib_body->SetSelection(0); - } + fill_body_choice(m_rib_body, m_edit_index, f.target_body); { m_rib_sketch->Clear(); int pre_sel = wxNOT_FOUND; @@ -7093,16 +7081,7 @@ void DesignPanel::load_feature_into_dialog(const CadFeature& f) break; } case CadFeatureType::Project: { - { - m_proj_source_body->Clear(); - for (size_t i = 0; i < m_doc.bodies.size(); ++i) { - const std::string& n = m_doc.bodies[i].name; - m_proj_source_body->Append(n.empty() ? wxString::Format(_L("Body %zu"), i + 1) : wxString::FromUTF8(n)); - } - if (f.project_source_body >= 0 && f.project_source_body < int(m_proj_source_body->GetCount())) - m_proj_source_body->SetSelection(f.project_source_body); - else if (m_proj_source_body->GetCount() > 0) m_proj_source_body->SetSelection(0); - } + fill_body_choice(m_proj_source_body, m_edit_index, f.project_source_body); populate_plane_choices(m_proj_plane); m_proj_plane->SetSelection(index_from_plane(f.plane)); m_sel_solid_face = f.project_face; @@ -7112,16 +7091,7 @@ void DesignPanel::load_feature_into_dialog(const CadFeature& f) break; } case CadFeatureType::DeleteFace: { - { - m_del_face_body->Clear(); - for (size_t i = 0; i < m_doc.bodies.size(); ++i) { - const std::string& n = m_doc.bodies[i].name; - m_del_face_body->Append(n.empty() ? wxString::Format(_L("Body %zu"), i + 1) : wxString::FromUTF8(n)); - } - if (f.target_body >= 0 && f.target_body < int(m_del_face_body->GetCount())) - m_del_face_body->SetSelection(f.target_body); - else if (m_del_face_body->GetCount() > 0) m_del_face_body->SetSelection(0); - } + fill_body_choice(m_del_face_body, m_edit_index, f.target_body); m_del_faces = f.delete_faces; { wxString s; diff --git a/src/slic3r/GUI/DesignPanel.hpp b/src/slic3r/GUI/DesignPanel.hpp index c95985e048..1e78fd1403 100644 --- a/src/slic3r/GUI/DesignPanel.hpp +++ b/src/slic3r/GUI/DesignPanel.hpp @@ -122,6 +122,10 @@ private: // >= 0 = the bodies as they existed just before that feature index (Boolean re-edit, so a // consumed tool body still appears and its saved selection round-trips). void populate_body_choices(int as_of_feature = -1); + // Fill `c` with the bodies as they existed just before `as_of_feature` and select + // `want`. Re-editing any feature that stores a body index needs this: the index was + // recorded against the body list at that point in the timeline, not the final one. + void fill_body_choice(ComboBox* c, int as_of_feature, int want); void populate_sheet_body_choices(ComboBox* c) const; // bodies where is_sheet_shape() is true // Rows of a sheet-filtered picker are not body indices; go through these two, never // GetSelection()/SetSelection() directly. diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp index b9b94d8041..ca2a4e3204 100644 --- a/tests/libslic3r/test_caddocument.cpp +++ b/tests/libslic3r/test_caddocument.cpp @@ -1408,6 +1408,42 @@ TEST_CASE("draft tapers a solid face about the body base", "[CadDocument]") REQUIRE_FALSE(bad.recompute()); } +TEST_CASE("a split renumbers the bodies a later feature indexes", "[CadDocument][cut]") +{ + // Pins the invariant the Design tab's re-edit path depends on (snaporca-oz7): a stored + // target_body indexes the body list AS IT WAS when that feature ran, and a Cut placed + // later in the tree changes that list. If this test ever fails, the GUI's + // fill_body_choice() replay-to-timeline-slot assumption needs revisiting with it. + using Catch::Matchers::WithinRel; + + CadDocument doc; + int sk = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 20, 20, 10, "Box"); + doc.add_extrude(sk, 20.0, false, BooleanMode::New, "Ext"); + REQUIRE(doc.recompute()); + REQUIRE(doc.bodies.size() == 1); + + // Cut the single body in half, keeping BOTH pieces — what the Cut card always does. + doc.add_cut(SketchPlane::XY(), 10.0, false, true, true, 0, "Split"); + REQUIRE(doc.recompute()); + REQUIRE(doc.error.empty()); + REQUIRE(doc.bodies.size() == 2); // one body became two + + // Body index 1 did not exist before the Cut. A feature recorded BEFORE the Cut could + // never have referred to it, which is exactly why re-edit must list the earlier set. + const double half = 20.0 * 20.0 * 10.0; + double v0 = double(SketchEngine::tessellate(doc.bodies[0].shape).volume()); + double v1 = double(SketchEngine::tessellate(doc.bodies[1].shape).volume()); + CHECK_THAT(v0 + v1, WithinRel(2.0 * half, 1e-3)); + + // Replaying to just before the Cut yields the pre-split list — the one a feature sitting + // there indexes into. This is the operation fill_body_choice() performs. + CadDocument as_of = doc; + as_of.features.resize(as_of.features.size() - 1); // drop the Cut + REQUIRE(as_of.recompute()); + REQUIRE(as_of.bodies.size() == 1); + CHECK(as_of.bodies.size() < doc.bodies.size()); +} + TEST_CASE("cut splits a body with a plane", "[cut]") { using Catch::Matchers::WithinRel;