From f4160595b0163d8240f0d34f0cab6d558d7d541c Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Wed, 15 Jul 2026 00:12:43 +0200 Subject: [PATCH] Kill the remaining Design-tab UI freezes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit query_topology: indexing a body face-by-face was quadratic — face_by_index re-walks the explorer and edge_by_index rebuilds the whole indexed map on every single call. On a 15.7k-face / 25.6k-edge imported solid this blew past the MCP 15 s main-thread timeout with the UI frozen throughout. GeometryEngine gains faces_of()/edges_of(), which enumerate once in the very same order (ids stay interchangeable with the _by_index accessors, so fillet/up_to_face targets are unaffected). Measured on that body: 15 s timeout -> 0.46 s. Feature ops: every commit-time m_doc.recompute() (fillet, cut, shell, boolean, extrude, ...) now goes through recompute_guarded(), which runs the rebuild on a worker thread. Live-preview/drag paths stay inline on purpose — yielding inside a drag would be worse than the stall. run_off_ui_thread(): the progress dialog is now created only after 300 ms, so a fast op does not flash a dialog, while input stays blocked (wxWindowDisabler) for the whole operation since the worker owns the document. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BVzKmX6Y1aEteit1HTXG4Q --- src/libslic3r/GeometryEngine.cpp | 19 +++++++++ src/libslic3r/GeometryEngine.hpp | 6 +++ src/slic3r/GUI/DesignPanel.cpp | 69 +++++++++++++++++++++++--------- src/slic3r/GUI/DesignPanel.hpp | 3 ++ src/slic3r/GUI/McpControl.cpp | 14 +++++-- 5 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/libslic3r/GeometryEngine.cpp b/src/libslic3r/GeometryEngine.cpp index dede7cec50..311c24dfdb 100644 --- a/src/libslic3r/GeometryEngine.cpp +++ b/src/libslic3r/GeometryEngine.cpp @@ -499,6 +499,25 @@ TopoDS_Face GeometryEngine::face_by_index(const TopoDS_Shape& shape, int index) return TopoDS_Face(); } +std::vector GeometryEngine::faces_of(const TopoDS_Shape& shape) +{ + std::vector out; + for (TopExp_Explorer e(shape, TopAbs_FACE); e.More(); e.Next()) + out.push_back(TopoDS::Face(e.Current())); // same order as face_by_index + return out; +} + +std::vector GeometryEngine::edges_of(const TopoDS_Shape& shape) +{ + TopTools_IndexedMapOfShape map; + TopExp::MapShapes(shape, TopAbs_EDGE, map); // same order as edge_by_index + std::vector out; + out.reserve(map.Extent()); + for (int i = 1; i <= map.Extent(); ++i) + out.push_back(TopoDS::Edge(map(i))); + return out; +} + std::vector GeometryEngine::edges_of_face(const TopoDS_Face& face) { std::vector result; diff --git a/src/libslic3r/GeometryEngine.hpp b/src/libslic3r/GeometryEngine.hpp index f93feb7ec3..53a083f95d 100644 --- a/src/libslic3r/GeometryEngine.hpp +++ b/src/libslic3r/GeometryEngine.hpp @@ -118,6 +118,12 @@ public: // per-triangle face id, so a picked triangle's id maps back to a face here. static TopoDS_Face face_by_index(const TopoDS_Shape& shape, int index); // null if out of range static int face_count(const TopoDS_Shape& shape); + // Bulk enumeration in the SAME order as face_by_index / edge_by_index, so ids are + // interchangeable. Walking a body with the _by_index accessors is quadratic (each call + // rescans the shape — edge_by_index even rebuilds the whole indexed map), which cost + // ~15 s on a 4.7k-face imported solid; enumerate once instead. + static std::vector faces_of(const TopoDS_Shape& shape); + static std::vector edges_of(const TopoDS_Shape& shape); static std::vector edges_of_face(const TopoDS_Face& face); // Centre of mass (world) of a face — used to compute the extrude length for "up to face". static Vec3d face_centroid_world(const TopoDS_Face& face); diff --git a/src/slic3r/GUI/DesignPanel.cpp b/src/slic3r/GUI/DesignPanel.cpp index 28bcba4021..9f9b561784 100644 --- a/src/slic3r/GUI/DesignPanel.cpp +++ b/src/slic3r/GUI/DesignPanel.cpp @@ -28,6 +28,7 @@ #include #include #include +#include // wxWindowDisabler, wxMilliSleep #include #include @@ -2379,21 +2380,53 @@ void DesignPanel::on_import_svg() // to swallow them (OCCT throws Standard_Failure, which is not a std::exception). static void run_off_ui_thread(wxWindow* parent, const wxString& message, const std::function& work) { - wxProgressDialog dlg(_L("SnapOrca"), message, 100, parent, - wxPD_APP_MODAL | wxPD_AUTO_HIDE | wxPD_SMOOTH); std::atomic done{false}; std::thread worker([&work, &done]() { work(); done.store(true, std::memory_order_release); }); + + // Input stays blocked for the whole operation: the worker owns the document, so nothing + // in the UI may mutate it meanwhile. The dialog only appears if the work is actually slow — + // a fillet on a small body finishes in milliseconds and must not flash a dialog. + wxWindowDisabler disabler; + std::unique_ptr dlg; + int elapsed_ms = 0; while (!done.load(std::memory_order_acquire)) { - dlg.Pulse(); - wxYield(); // keep painting; app-modal blocks input to the rest of the UI + if (dlg == nullptr && elapsed_ms >= 300) + dlg = std::make_unique(_L("SnapOrca"), message, 100, parent, + wxPD_AUTO_HIDE | wxPD_SMOOTH); + if (dlg != nullptr) + dlg->Pulse(); + wxYield(); // keep the window painting instead of going unresponsive wxMilliSleep(30); + elapsed_ms += 30; } worker.join(); } +// Rebuild the document off the UI thread. Every feature op (fillet, cut, shell, boolean, ...) +// goes through recompute(), and on a heavy imported solid that is seconds of OCCT work — inline +// it freezes the window. OCCT throws Standard_Failure, which is not a std::exception and would +// terminate the process if it escaped the worker, so both are caught here. +bool DesignPanel::recompute_guarded(const wxString& message) +{ + bool ok = false; + run_off_ui_thread(this, message, [this, &ok]() { + try { + ok = m_doc.recompute(); + } catch (const Standard_Failure& e) { + const char* what = e.GetMessageString(); + m_doc.error = (what != nullptr && *what != '\0') ? what : "OCCT failure"; + ok = false; + } catch (const std::exception& e) { + m_doc.error = e.what(); + ok = false; + } + }); + return ok; +} + void DesignPanel::on_import_step() { wxFileDialog dlg(this, _L("Import STEP"), wxEmptyString, wxEmptyString, @@ -2537,7 +2570,7 @@ void DesignPanel::on_import_mesh() f.mode = BooleanMode::New; // its own coexisting body, like a STEP solid m_doc.features.push_back(f); - if (!m_doc.recompute()) { + if (!recompute_guarded(_L("Rebuilding model…"))) { fail(_L("Mesh import failed: ") + wxString::FromUTF8(m_doc.error)); return; } @@ -2749,7 +2782,7 @@ void DesignPanel::on_add_extrude() && m_doc.features[m_extrude_sketch_ref].import_on_face) f.target_body = m_doc.features[m_extrude_sketch_ref].import_face_body; } - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -2782,7 +2815,7 @@ void DesignPanel::on_add_dressup() if (didx >= 0 && didx < int(m_doc.features.size())) m_doc.features[didx].target_body = m_sel_solid_body; - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -2818,7 +2851,7 @@ void DesignPanel::on_add_hole() if (m_hole_on_face && hidx >= 0 && hidx < int(m_doc.features.size())) m_doc.features[hidx].target_body = m_hole_face_body; - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -2898,7 +2931,7 @@ void DesignPanel::on_add_thread() if (m_thread_on_face && tidx >= 0 && tidx < int(m_doc.features.size())) m_doc.features[tidx].target_body = m_thread_face_body; - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -2922,7 +2955,7 @@ void DesignPanel::on_add_revolve() m_revolve_axis->GetSelection(), m_revolve_flip->GetValue(), mode, "Revolve" + std::to_string(m_feature_counter)); - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -2952,7 +2985,7 @@ void DesignPanel::on_add_sweep() m_doc.add_sweep(m_sweep_profile_ref, path_ref, mode, "Sweep" + std::to_string(m_feature_counter)); - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -2980,7 +3013,7 @@ void DesignPanel::on_add_loft() m_doc.add_loft(refs, m_loft_ruled->GetValue(), mode, "Loft" + std::to_string(m_feature_counter)); - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -3003,7 +3036,7 @@ void DesignPanel::on_add_pattern() m_pattern_angle->GetValue(), target, "Pattern" + std::to_string(m_feature_counter)); - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -3053,7 +3086,7 @@ void DesignPanel::on_add_boolean() m_doc.add_boolean(op, m_bool_target->GetSelection(), m_bool_tool->GetSelection(), m_bool_keep->GetValue(), m_bool_tol->GetValue(), -1, -1, "Boolean" + std::to_string(m_feature_counter)); - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -3070,7 +3103,7 @@ void DesignPanel::on_add_cut() m_doc.add_cut(plane_from_choice(m_cut_plane->GetSelection()), m_cut_offset->GetValue(), /*flip*/ false, /*keep_upper*/ true, /*keep_lower*/ true, m_cut_target->GetSelection(), "Cut" + std::to_string(m_feature_counter)); - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -3165,7 +3198,7 @@ void DesignPanel::on_add_shell() m_doc.add_shell(m_shell_thickness->GetValue(), face, m_sel_solid_body, "Shell" + std::to_string(m_feature_counter)); - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -3190,7 +3223,7 @@ void DesignPanel::on_add_draft() m_doc.add_draft(m_draft_angle->GetValue(), m_sel_solid_face, m_sel_solid_body, "Draft" + std::to_string(m_feature_counter)); - if (!m_doc.recompute()) + if (!recompute_guarded(_L("Rebuilding model…"))) m_status->SetLabel(_L("Recompute error: ") + wxString::FromUTF8(m_doc.error)); else set_status_ok(); @@ -3638,7 +3671,7 @@ void DesignPanel::on_toggle_visibility() // solid to build), but that is a VALID state for hide — so clear the body // explicitly instead of letting after_tree_edit treat it as a rejected edit // (which would skip the overlay refresh, leaving hidden art on screen). - if (!m_doc.recompute()) { + if (!recompute_guarded(_L("Rebuilding model…"))) { m_doc.body = TopoDS_Shape(); m_doc.display_mesh = TriangleMesh{}; m_doc.error.clear(); diff --git a/src/slic3r/GUI/DesignPanel.hpp b/src/slic3r/GUI/DesignPanel.hpp index 967a37357d..502f5d6bcd 100644 --- a/src/slic3r/GUI/DesignPanel.hpp +++ b/src/slic3r/GUI/DesignPanel.hpp @@ -37,6 +37,9 @@ class DesignPanel : public wxPanel public: explicit DesignPanel(wxWindow* parent); void on_tab_shown(); // re-sync bed to the active printer when the Design tab is activated + // Rebuild off the UI thread (progress dialog only if it turns out to be slow), so a feature + // op on a heavy imported solid does not freeze the window. Returns m_doc.recompute()'s result. + bool recompute_guarded(const wxString& message); // MCP control hooks: let the external control server (McpControl.cpp) drive and // perceive the SAME kernel the GUI uses. Called only on the wx main thread. diff --git a/src/slic3r/GUI/McpControl.cpp b/src/slic3r/GUI/McpControl.cpp index f93dc7541d..4a646db78c 100644 --- a/src/slic3r/GUI/McpControl.cpp +++ b/src/slic3r/GUI/McpControl.cpp @@ -291,10 +291,16 @@ const TopoDS_Shape& body_shape(DesignPanel* panel, const json& params) json query_topology(DesignPanel* panel, const json& params) { const TopoDS_Shape& shape = body_shape(panel, params); + // Enumerate once. The _by_index accessors rescan the shape on every call (edge_by_index + // rebuilds the whole indexed map), so indexing a body face-by-face is quadratic: ~15 s on a + // 4.7k-face imported solid, on the UI thread. faces_of/edges_of keep the very same ids. + const std::vector all_faces = GeometryEngine::faces_of(shape); + const std::vector all_edges = GeometryEngine::edges_of(shape); + json faces = json::array(); - int nf = GeometryEngine::face_count(shape); + const int nf = int(all_faces.size()); for (int i = 0; i < nf; ++i) { - TopoDS_Face f = GeometryEngine::face_by_index(shape, i); + const TopoDS_Face& f = all_faces[i]; if (f.IsNull()) continue; json jf{{"id", i}, {"centroid", vec3(GeometryEngine::face_centroid_world(f))}, {"normal", vec3(GeometryEngine::face_normal_world(f))}, {"kind", "planar"}}; @@ -304,9 +310,9 @@ json query_topology(DesignPanel* panel, const json& params) faces.push_back(std::move(jf)); } json edges = json::array(); - int ne = GeometryEngine::edge_count(shape); + const int ne = int(all_edges.size()); for (int i = 0; i < ne; ++i) { - TopoDS_Edge e = GeometryEngine::edge_by_index(shape, i); + const TopoDS_Edge& e = all_edges[i]; if (e.IsNull()) continue; std::vector pts = GeometryEngine::sample_edge_world(e); if (pts.size() < 2) continue;