From 70a2b3a814d7c6efebee12f15e838ba57e8fbbca Mon Sep 17 00:00:00 2001 From: Lam Wei Lun Date: Fri, 11 Sep 2026 15:43:46 +0800 Subject: [PATCH] Code cleanup, dedup, update unit tests --- resources/web/dialog/SpeedDial/speeddial.js | 89 +++++- .../web/dialog/SpeedDial/speeddial.test.js | 21 ++ resources/web/dialog/SpeedDial/style.css | 21 -- src/slic3r/GUI/ActionRegistry.cpp | 4 +- src/slic3r/GUI/MainFrame.cpp | 4 +- src/slic3r/GUI/PluginsDialog.cpp | 78 ++--- src/slic3r/GUI/PluginsDialog.hpp | 289 ++++++++++-------- src/slic3r/GUI/Search.cpp | 25 +- tests/slic3rutils/test_action_source.cpp | 10 + 9 files changed, 316 insertions(+), 225 deletions(-) diff --git a/resources/web/dialog/SpeedDial/speeddial.js b/resources/web/dialog/SpeedDial/speeddial.js index 2045d42224..3dcbbd4fd1 100644 --- a/resources/web/dialog/SpeedDial/speeddial.js +++ b/resources/web/dialog/SpeedDial/speeddial.js @@ -1,6 +1,14 @@ // Speed Dial launcher page. Static-safe module: no DOM access at load time so a // node vm can exercise the pure helpers (searchActions / filterTabs / actionLabel / nextSel / -// commandList / commandSections / actionCategory / groupActions). +// commandList / commandSections / actionCategory / groupActions / favDigitFromEvent / spacerHeight). +// +// Cross-boundary contracts (keep in sync; the C++ side pins its half in tests): +// - favourite cap 10 -> ActionRegistry::kFavLimit (K_FAV_LIMIT here) +// - mode rank simple ConfigOptionMode order (MODE_RANK here) +// - action.mode token -> ActionRegistry::mode_key / SpeedDialDialog::mode_label +// - action.input "percent"/"tab" -> NativeCommands catalog (phases handled in activateEntry) +// - action.icon SVG base name -> AppAction::icon / resources/images/.svg +// - action list is frecency-sorted -> ActionRegistry::snapshot() // ---- state (populated by the C++ bridge via window.HandleStudio) ---- var ACTIONS = []; // [{id,title,source,group,kind,input,icon,mode}], already frecency-sorted by C++ @@ -45,7 +53,7 @@ function T(key, fallback) { var K_ROWS = 50; var ROW_H = 44; var renderEnd = 0; -var builtKey = ""; // phase|query|listLen - when it changes, rows are rebuilt from the first window +var builtKey = ""; // phase|query|total - when it changes, rows are rebuilt from the first window [0, K_ROWS) var spacerEl = null; // the trailing height spacer, always the last child of listEl // Section headers in the empty-query list ("Recent" then one per category). sectionStarts maps a flat @@ -200,6 +208,12 @@ function revealTarget(total, fromIndex, size) { return Math.min(total, Math.max(0, fromIndex) + size); } +// Pure: the bottom spacer's height - the un-rendered row tail plus any not-yet-rendered section +// headers, so the scrollbar reflects the full list and the last rows stay reachable. +function spacerHeight(total, rendered, totalSections, renderedSections) { + return Math.max(0, total - rendered) * ROW_H + Math.max(0, totalSections - renderedSections) * SECTION_H; +} + // buildKey: the command-list signature that decides whether rows must be rebuilt (new search / phase) // or just have their selection refreshed in place (arrow-nav / click). Cheap to compute. function buildKey() { return phase + "|" + (query || "").trim(); } @@ -232,6 +246,13 @@ function favIndexForDigit(d) { return -1; } +// Physical digit for a keydown event. Use e.code so macOS Option+digit (which composes to a symbol +// in e.key, e.g. Alt+1 -> "¡") still maps to the intended slot; fall back to e.key elsewhere. +function favDigitFromEvent(e) { + var m = /^(?:Digit|Numpad)([0-9])$/.exec((e && e.code) || ""); + return m ? m[1] : ((e && e.key) || ""); +} + function resultCountText(total, shown, query) { var n = total + " " + T("sd_actions", "actions"); return (query || "").trim() ? T("sd_showing", "Showing") + " " + shown + " " + T("sd_of", "of") + " " + n : n; @@ -319,10 +340,22 @@ function groupActions(list) { // by relevance; an empty query shows the recents first, then the rest grouped under category headers. // The empty-query result is cached on the action/recents array identities so scrolling doesn't regroup. var commandListCache = null; +// Typed-query result cache, keyed on the pool identity + query. searchActions populates the +// module-level matchIndex/searchNeedle; a hit restores both so a repeated call (keydown + input, +// or a scroll tick) skips the whole scan instead of recomputing it. +var searchCache = null; function commandList(actions, recents, query) { var all = actions || []; - if (shouldRenderActionList(query)) - return searchActions(all, query); + if (shouldRenderActionList(query)) { + if (searchCache && searchCache.actions === all && searchCache.query === query) { + matchIndex = searchCache.matchIndex; + searchNeedle = searchCache.needle; + return searchCache.list; + } + var found = searchActions(all, query); + searchCache = { actions: all, query: query, list: found, matchIndex: matchIndex, needle: searchNeedle }; + return found; + } var rec = recents || []; if (commandListCache && commandListCache.actions === all && commandListCache.recents === rec) return commandListCache.list; @@ -408,15 +441,35 @@ function modeFilterFromQuery(query) { return found; } +// Precomputed label parts for one action pool, keyed on the pool's array identity. The fold pass is +// O(N); doing it here instead of inside every actionLabel() call keeps row rendering O(rows), not O(rows*N). +var labelCache = null; +function labelParts(actions) { + if (labelCache && labelCache.actions === actions) + return labelCache; + var sig = {}, count = {}; + (actions || []).forEach(function (o) { + var s = foldLabel(o.title) + "|" + foldLabel(o.source || o.group || ""); + sig[o.id] = s; + count[s] = (count[s] || 0) + 1; + }); + labelCache = { actions: actions, sig: sig, count: count }; + return labelCache; +} + // Accessible label "Title from Pretty Source", disambiguated with the opaque action id when another // action shares the same title+source (case/separator-insensitive) - so two rows never read out identically. function actionLabel(action, actions) { var label = action.title + " from " + prettySource(action.source || action.group || ""); if (actions && actions.length) { - var mine = foldLabel(action.title) + "|" + foldLabel(action.source || action.group || ""); - var clash = actions.some(function (o) { - return o.id !== action.id && foldLabel(o.title) + "|" + foldLabel(o.source || o.group || "") === mine; - }); + var cache = labelParts(actions); + var mine = cache.sig[action.id]; + // mine is undefined only for an action outside the cached pool (e.g. a transient row); scan then. + var clash = mine !== undefined ? cache.count[mine] > 1 : + actions.some(function (o) { + return o.id !== action.id && foldLabel(o.title) + "|" + foldLabel(o.source || o.group || "") === + foldLabel(action.title) + "|" + foldLabel(action.source || action.group || ""); + }); if (clash) label += " (" + action.id + ")"; } @@ -788,13 +841,14 @@ function ensureSpacer() { // section headers reserve their own height too, so the last rows stay reachable. function setBottomSpacer(total) { ensureSpacer(); - var remainingRows = Math.max(0, total - renderEnd); - var remainingHeaders = Math.max(0, sectionTotal - sectionRendered); - spacerEl.style.height = (remainingRows * ROW_H + remainingHeaders * SECTION_H) + "px"; + spacerEl.style.height = spacerHeight(total, renderEnd, sectionTotal, sectionRendered) + "px"; } // Reveal rows up to `upto` (an exclusive index), appending without rebuilding the whole list. Used by // the scroll handler (viewport + overscan) and by arrow-nav that runs off the end of the current window. +// The window is append-only and row indices are absolute, so jumping the selection to the very last row +// (ArrowUp wrap with no fav bar) necessarily materializes the whole list; the label/search caches above +// keep that one-off cost linear rather than quadratic. function revealTo(list, upto) { var need = Math.min(list.length, upto); if (need <= renderEnd) @@ -893,8 +947,9 @@ function renderCommandsList() { updatePins(list); } -// A tab row: no pin/unpin (tabs aren't pinnable), placeholder tile (tabs have no pictogram). Uses -// tabTitle so pages added with an empty text (e.g. Home) still show a label. +// A tab row: no pin/unpin (tabs aren't pinnable), and a tile that shows the page icon when the +// notebook has one (plugin pages often don't). Uses tabTitle so pages added with an empty text +// (e.g. Home) still show a label. function renderTabRow(t, i) { var label = tabTitle(t); var shell = beginRow(t, i, true, label); @@ -911,6 +966,7 @@ function renderTabList() { var q = (query || "").trim(); var list = currentList(); listEl.innerHTML = ""; + spacerEl = null; // the tab list has no windowed spacer; rebuildCommandsList recreates it if (!list.length) { renderEmpty(q ? T("sd_no_tabs_match", "No tabs match") : T("sd_no_tabs", "No tabs")); @@ -990,7 +1046,10 @@ function flashHint(text) { hint.textContent = text; launcher.insertBefore(hint, launcher.firstChild); setTimeout(function () { - if (hint && hint.parentNode) hint.parentNode.removeChild(hint); + if (hint && hint.parentNode) { + hint.parentNode.removeChild(hint); + requestResize(); // reclaim the hint's height so the popup doesn't stay tall + } }, 2500); } @@ -1139,7 +1198,7 @@ function OnInit() { // Quick-launch a numbered favourite: Alt/Option + digit (0 = the 10th). Only in the // commands phase, where the pinned bar is shown. if (phase === "commands" && e.altKey && !e.ctrlKey && !e.metaKey) { - var slotIdx = favIndexForDigit(e.key); + var slotIdx = favIndexForDigit(favDigitFromEvent(e)); var favIds = currentVisibleFavs(); if (slotIdx >= 0 && slotIdx < favIds.length) { e.preventDefault(); diff --git a/resources/web/dialog/SpeedDial/speeddial.test.js b/resources/web/dialog/SpeedDial/speeddial.test.js index 521705d68f..85a0bcd064 100644 --- a/resources/web/dialog/SpeedDial/speeddial.test.js +++ b/resources/web/dialog/SpeedDial/speeddial.test.js @@ -216,6 +216,15 @@ assert.equal(ctx.favSlotForIndex(10), null, "index 10 is beyond the cap"); assert.equal(ctx.favSlotForIndex(-1), null, "negative index is not a slot"); assert.equal(ctx.K_FAV_LIMIT, 10, "the slot count matches the quick-launch cap"); +// favDigitFromEvent: prefer the physical code (so macOS Option+digit still maps even though e.key +// is the composed symbol), and fall back to e.key for keyboards/synthetic events without a code. +assert.equal(ctx.favDigitFromEvent({ code: "Digit1", key: "¡" }), "1", "Digit1 wins over a composed key"); +assert.equal(ctx.favDigitFromEvent({ code: "Digit0", key: "0" }), "0", "Digit0 is a physical digit"); +assert.equal(ctx.favDigitFromEvent({ code: "Numpad7", key: "7" }), "7", "numpad digits count"); +assert.equal(ctx.favDigitFromEvent({ code: "", key: "3" }), "3", "a missing code falls back to key"); +assert.equal(ctx.favDigitFromEvent({ key: "a" }), "a", "non-digit input is passed through (maps to -1)"); +assert.equal(ctx.favDigitFromEvent(null), "", "a null event yields no digit"); + // nextSel: arrow-nav wrapping. Down wraps at the list bottom to the first row; Up wraps at the // list top to the last row ONLY when there's no fav bar above (else it goes to the fav bar). assert.deepEqual(ctx.nextSel({ zone: "list", i: 2 }, "ArrowDown", 3, 0), { zone: "list", i: 0 }, @@ -245,6 +254,14 @@ assert.equal(ctx.revealTarget(100, -5, 50), 50, "negative start is clamped to th assert.equal(ctx.revealTarget(200, 50, 100), 150, "a scroll viewpoint reveals a window past the current rows"); assert.equal(ctx.revealTarget(10, 0, 50), 10, "a list shorter than one window stays fully materialized"); +// spacerHeight: un-rendered rows (44px) plus un-rendered section headers (30px), never negative. +assert.equal(ctx.spacerHeight(100, 50, 0, 0), 50 * 44, "the tail rows reserve their full height"); +assert.equal(ctx.spacerHeight(100, 100, 0, 0), 0, "a fully-rendered list needs no spacer"); +assert.equal(ctx.spacerHeight(100, 50, 3, 1), 50 * 44 + 2 * 30, "pending section headers reserve their height too"); +assert.equal(ctx.spacerHeight(10, 0, 2, 0), 10 * 44 + 2 * 30, "a short list still reserves its headers"); +assert.equal(ctx.spacerHeight(0, 0, 0, 0), 0, "an empty list has no spacer"); +assert.equal(ctx.spacerHeight(10, 20, 0, 5), 0, "over-rendered counters clamp to zero"); + // visibleFavourites: the quick-bar drops pins whose action no longer exists (plugin unloaded, // command removed) and collapses duplicate ids, keeping the persisted pin order. assert.deepEqual(ctx.visibleFavourites(["a", "b", "c"], [{ id: "a" }, { id: "b" }]), @@ -274,6 +291,10 @@ assert.equal(ctx.needsModeSwitch({ mode: "advanced" }, "advanced"), false, "an A assert.equal(ctx.needsModeSwitch({ mode: "develop" }, "develop"), false, "a Developer setting is not gated in Developer mode"); assert.equal(ctx.needsModeSwitch({}, "simple"), false, "a command with no mode is never gated"); +// MODE_RANK must match the C++ ConfigOptionMode order (comSimple < comAdvanced < comExpert < comDevelop). +assert.deepEqual(ctx.MODE_RANK, { simple: 0, advanced: 1, expert: 2, develop: 3 }, + "mode rank matches the C++ ConfigOptionMode order"); + // modeBadge: the tag text for gated settings, empty once the setting is available. assert.equal(ctx.modeBadge({ mode: "advanced" }, "simple"), "Advanced", "Advanced badge text"); assert.equal(ctx.modeBadge({ mode: "expert" }, "simple"), "Expert", "Expert badge text"); diff --git a/resources/web/dialog/SpeedDial/style.css b/resources/web/dialog/SpeedDial/style.css index 6aaca682d9..c9532d8dab 100644 --- a/resources/web/dialog/SpeedDial/style.css +++ b/resources/web/dialog/SpeedDial/style.css @@ -382,12 +382,6 @@ body { border-radius: 2px; } -.row-sc { - flex: 0 0 auto; - display: inline-flex; - gap: 3px; -} - /* Mode tag on settings above the user's current mode (Advanced/Expert/Developer). */ .row-mode { flex: 0 0 auto; @@ -400,21 +394,6 @@ body { white-space: nowrap; } -kbd { - display: inline-flex; - align-items: center; - justify-content: center; - min-width: 18px; - height: 18px; - padding: 0 5px; - font-family: ui-monospace, "Cascadia Mono", Consolas, monospace; - font-size: 11px; - color: var(--muted, var(--orca-muted, #6b7280)); - background: rgba(127, 127, 127, .12); - border: 1px solid var(--border, var(--orca-border, #ddd)); - border-radius: 4px; -} - .pin { flex: 0 0 auto; width: 24px; diff --git a/src/slic3r/GUI/ActionRegistry.cpp b/src/slic3r/GUI/ActionRegistry.cpp index b44d73ddc1..fb98b96a17 100644 --- a/src/slic3r/GUI/ActionRegistry.cpp +++ b/src/slic3r/GUI/ActionRegistry.cpp @@ -178,7 +178,7 @@ struct SettingAction : AppAction { std::string opt_key; Preset::Type type; - std::wstring category; // localized category, forwarded to jump_to_option + std::wstring category; // English category, forwarded to jump_to_option (it localizes) static std::string id_for(const std::string& opt_key, Preset::Type type) { return std::string(kSettingPrefix) + ":" + opt_key + ":" + std::to_string(int(type)); } @@ -556,7 +556,7 @@ void ActionRegistry::materialize_setting_actions() // title = the option leaf name (last label segment); group stays empty so the source path // (above) is the single display/search breadcrumb rather than being duplicated. auto action = std::make_unique(opt.opt_key(), opt.type, boost::nowide::narrow(label_w), std::string(), - opt.category_local, boost::nowide::narrow(path), opt.mode); + opt.category, boost::nowide::narrow(path), opt.mode); // Tile pictogram = the icon of the setting's own group header (e.g. Advanced -> param_advanced), // the one shown next to it in the page. Fall back to the page/category icon for groups diff --git a/src/slic3r/GUI/MainFrame.cpp b/src/slic3r/GUI/MainFrame.cpp index 4681a67c29..828c698ce1 100644 --- a/src/slic3r/GUI/MainFrame.cpp +++ b/src/slic3r/GUI/MainFrame.cpp @@ -737,7 +737,9 @@ DPIFrame(NULL, wxID_ANY, "", wxDefaultPosition, wxDefaultSize, BORDERLESS_FRAME_ evt.Skip(); // let the focused control keep Space return; } - wxGetApp().open_speed_dial(); + // Defer out of the native key-event stack: open_speed_dial() may create a WebView and + // run script, the same window work the codebase avoids doing on native callbacks. + this->CallAfter([this] { wxGetApp().open_speed_dial(); }); return; } if (evt.CmdDown() && evt.GetKeyCode() == 'R') { if (m_slice_enable) { wxGetApp().plater()->update(true, true); wxPostEvent(m_plater, SimpleEvent(EVT_GLTOOLBAR_SLICE_PLATE)); this->m_tabpanel->SelectPageByName(TAB_ID_PREVIEW); } return; } diff --git a/src/slic3r/GUI/PluginsDialog.cpp b/src/slic3r/GUI/PluginsDialog.cpp index 8bee3919b2..5b1e203c17 100644 --- a/src/slic3r/GUI/PluginsDialog.cpp +++ b/src/slic3r/GUI/PluginsDialog.cpp @@ -528,68 +528,37 @@ bool install_local_plugin_package(const boost::filesystem::path& package_file, w { struct Result { - std::mutex mutex; - bool ok = false; + std::mutex mutex; + bool ok = false; std::string error; }; auto state = std::make_shared(); - wxProgressDialog* progress = new wxProgressDialog(_L("Installing plugin"), _L("Installing plugin") + ": " + package_name, - 100, parent, wxPD_APP_MODAL | wxPD_AUTO_HIDE | wxPD_ELAPSED_TIME); - wxTimer* timer = new wxTimer(); - timer->Bind(wxEVT_TIMER, [progress](wxTimerEvent&) { - if (progress) - progress->Pulse(); - }); - timer->Start(100); - - // finished/loop live on the heap: the worker's completion callback is posted to the UI loop - // and can still fire after this stack frame is gone, so it must not reference locals. - struct WaitState - { - bool finished = false; - wxEventLoop loop; - }; - auto wait = std::make_shared(); - - std::thread([state, package_file, wait]() mutable { - std::string error; - bool ok = false; - try { - ok = PluginManager::instance().install_plugin(package_file, error); - } catch (const std::exception& ex) { - error = ex.what(); - } catch (...) { - error = "Unknown error"; - } - if (ok) { - // Reflect the new package in discovery/cloud metadata without blocking the caller. - try { refresh_plugin_metadata_blocking(kUseCurrentCloudMeta); } catch (...) {} - } - { + detail::run_wait_with_progress( + [state, package_file]() { + std::string error; + bool ok = false; + try { + ok = PluginManager::instance().install_plugin(package_file, error); + } catch (const std::exception& ex) { + error = ex.what(); + } catch (...) { + error = "Unknown error"; + } + if (ok) { + // Reflect the new package in discovery/cloud metadata without blocking the caller. + try { refresh_plugin_metadata_blocking(kUseCurrentCloudMeta); } catch (...) {} + } std::lock_guard lock(state->mutex); state->ok = ok; state->error = std::move(error); - } - if (!wxTheApp) - return; - wxTheApp->CallAfter([wait]() { - wait->finished = true; - if (wait->loop.IsRunning()) - wait->loop.Exit(); - }); - }).detach(); - - if (!wait->finished) - wait->loop.Run(); - - timer->Stop(); - delete timer; - progress->Destroy(); + }, + parent, _L("Installing plugin"), _L("Installing plugin") + ": " + package_name, 100, + wxPD_APP_MODAL | wxPD_AUTO_HIDE | wxPD_ELAPSED_TIME, /*alive=*/nullptr, /*restore=*/{}); std::lock_guard lock(state->mutex); - installed = state->ok; - error = std::move(state->error); + installed = state->ok; + error = std::move(state->error); } if (!installed) { @@ -980,6 +949,9 @@ bool PluginsDialog::install_plugin_package(const std::string& package_path) const boost::filesystem::path package_file(package_path); wxString message; const bool installed = install_local_plugin_package(package_file, this, message); + // The helper's overwrite prompt and progress dialog can push this webview behind; re-raise it + // once, after both have closed (the speed-dial path parents to the mainframe instead). + restore_z_order(); // The shared helper reports a user-cancelled overwrite with an empty message: stay silent. if (message.IsEmpty()) { diff --git a/src/slic3r/GUI/PluginsDialog.hpp b/src/slic3r/GUI/PluginsDialog.hpp index 2c4a5925d9..0aee7b2734 100644 --- a/src/slic3r/GUI/PluginsDialog.hpp +++ b/src/slic3r/GUI/PluginsDialog.hpp @@ -52,6 +52,168 @@ void open_plugin_hub(); // confirmation; on a user-cancelled overwrite it is empty; on failure it carries the reason. bool install_local_plugin_package(const boost::filesystem::path& package_file, wxWindow* parent, wxString& message); +namespace detail { + +// Shared worker + modal-progress machinery: pulse a progress dialog while `run` executes on a +// detached worker, then run `on_finish` back on the UI thread. `alive`, when non-null, gates both +// the pulse and `on_finish` so a worker outliving its dialog can't touch freed windows; pass null +// for a dialog-independent caller. `restore` runs after the progress dialog is destroyed and before +// `on_finish`, so a webview host can re-raise itself. `finish_after_dialog_destroyed` still calls +// `on_finish` (without touching the dialog) when the host died, so a waiting loop can exit. +template +void run_off_thread_with_progress(Run&& run, + OnFinish&& on_finish, + wxWindow* parent, + const wxString& title, + const wxString& message, + int maximum, + int style, + std::shared_ptr> alive, + bool finish_after_dialog_destroyed, + std::function restore) +{ + wxProgressDialog* progress = new wxProgressDialog(title, message, maximum, parent, style); + wxTimer* timer = new wxTimer(); + + timer->Bind(wxEVT_TIMER, [alive, progress, message](wxTimerEvent&) { + if ((!alive || alive->load(std::memory_order_acquire)) && progress) + progress->Pulse(message); + }); + + timer->Start(100); + + std::thread([alive, + progress, + timer, + run = std::forward(run), + on_finish = std::forward(on_finish), + finish_after_dialog_destroyed, + restore = std::move(restore)]() mutable { + try { + run(); + } catch (const std::exception& ex) { + BOOST_LOG_TRIVIAL(error) << "Plugin dialog worker failed: " << ex.what(); + } catch (...) { + BOOST_LOG_TRIVIAL(error) << "Plugin dialog worker failed with an unknown exception"; + } + + if (wxTheApp == nullptr) + return; + + wxTheApp->CallAfter([alive, + progress, + timer, + on_finish = std::move(on_finish), + finish_after_dialog_destroyed, + restore = std::move(restore)]() mutable { + timer->Stop(); + delete timer; + + if (!alive || alive->load(std::memory_order_acquire)) { + progress->Destroy(); + if (restore) + restore(); + on_finish(); + } else if (finish_after_dialog_destroyed) { + on_finish(); + } + }); + }).detach(); +} + +// Wait for a worker behind a progress dialog, returning its result (or rethrowing). The waiting +// loop stays responsive because it pumps the event loop the worker posts its completion into. +template +std::invoke_result_t&> run_wait_with_progress(Run&& run, + wxWindow* parent, + const wxString& title, + const wxString& message, + int maximum, + int style, + std::shared_ptr> alive, + std::function restore) +{ + using Result = std::invoke_result_t&>; + + bool finished = false; + wxEventLoop loop; + auto on_finish = [&finished, &loop]() { + finished = true; + if (loop.IsRunning()) + loop.Exit(); + }; + + if constexpr (std::is_void_v) { + struct WaitState + { + std::mutex mutex; + std::exception_ptr exception; + }; + + auto state = std::make_shared(); + run_off_thread_with_progress( + [run = std::forward(run), state]() mutable { + try { + run(); + } catch (...) { + std::lock_guard lock(state->mutex); + state->exception = std::current_exception(); + } + }, + on_finish, parent, title, message, maximum, style, std::move(alive), /*finish_after_dialog_destroyed=*/true, std::move(restore)); + + if (!finished) + loop.Run(); + + std::exception_ptr exception; + { + std::lock_guard lock(state->mutex); + exception = state->exception; + } + if (exception) + std::rethrow_exception(exception); + } else { + using StoredResult = std::decay_t; + struct WaitState + { + std::mutex mutex; + std::optional result; + std::exception_ptr exception; + }; + + auto state = std::make_shared(); + run_off_thread_with_progress( + [run = std::forward(run), state]() mutable { + try { + StoredResult result = run(); + std::lock_guard lock(state->mutex); + state->result.emplace(std::move(result)); + } catch (...) { + std::lock_guard lock(state->mutex); + state->exception = std::current_exception(); + } + }, + on_finish, parent, title, message, maximum, style, std::move(alive), /*finish_after_dialog_destroyed=*/true, std::move(restore)); + + if (!finished) + loop.Run(); + + std::optional result; + std::exception_ptr exception; + { + std::lock_guard lock(state->mutex); + if (state->result) + result.emplace(std::move(*state->result)); + exception = state->exception; + } + if (exception) + std::rethrow_exception(exception); + return std::move(*result); + } +} + +} // namespace detail + class PluginsDialog : public Slic3r::GUI::WebViewHostDialog { public: @@ -128,53 +290,8 @@ private: int style = wxPD_APP_MODAL | wxPD_AUTO_HIDE, bool finish_after_dialog_destroyed = false) { - const auto alive = m_alive; - wxProgressDialog* progress = new wxProgressDialog(title, message, maximum, this, style); - wxTimer* timer = new wxTimer(); - - timer->Bind(wxEVT_TIMER, [alive, progress, message](wxTimerEvent&) { - if (alive->load(std::memory_order_acquire) && progress) - progress->Pulse(message); - }); - - timer->Start(100); - - std::thread([this, - alive, - progress, - timer, - run = std::forward(run), - on_finish = std::forward(on_finish), - finish_after_dialog_destroyed]() mutable { - try { - run(); - } catch (const std::exception& ex) { - BOOST_LOG_TRIVIAL(error) << "Plugin dialog worker failed: " << ex.what(); - } catch (...) { - BOOST_LOG_TRIVIAL(error) << "Plugin dialog worker failed with an unknown exception"; - } - - if (wxTheApp == nullptr) - return; - - wxTheApp->CallAfter([this, - alive, - progress, - timer, - on_finish = std::move(on_finish), - finish_after_dialog_destroyed]() mutable { - timer->Stop(); - delete timer; - - if (alive->load(std::memory_order_acquire)) { - progress->Destroy(); - restore_z_order(); - on_finish(); - } else if (finish_after_dialog_destroyed) { - on_finish(); - } - }); - }).detach(); + detail::run_off_thread_with_progress(std::forward(run), std::forward(on_finish), this, title, message, maximum, style, + m_alive, finish_after_dialog_destroyed, [this] { restore_z_order(); }); } template @@ -184,83 +301,7 @@ private: int maximum = 100, int style = wxPD_APP_MODAL | wxPD_AUTO_HIDE) { - using Result = std::invoke_result_t&>; - - bool finished = false; - wxEventLoop loop; - auto on_finish = [&finished, &loop]() { - finished = true; - if (loop.IsRunning()) - loop.Exit(); - }; - - if constexpr (std::is_void_v) { - struct WaitState - { - std::mutex mutex; - std::exception_ptr exception; - }; - - auto state = std::make_shared(); - run_with_dialog( - [run = std::forward(run), state]() mutable { - try { - run(); - } catch (...) { - std::lock_guard lock(state->mutex); - state->exception = std::current_exception(); - } - }, - on_finish, title, message, maximum, style, true); - - if (!finished) - loop.Run(); - - std::exception_ptr exception; - { - std::lock_guard lock(state->mutex); - exception = state->exception; - } - if (exception) - std::rethrow_exception(exception); - } else { - using StoredResult = std::decay_t; - struct WaitState - { - std::mutex mutex; - std::optional result; - std::exception_ptr exception; - }; - - auto state = std::make_shared(); - run_with_dialog( - [run = std::forward(run), state]() mutable { - try { - StoredResult result = run(); - std::lock_guard lock(state->mutex); - state->result.emplace(std::move(result)); - } catch (...) { - std::lock_guard lock(state->mutex); - state->exception = std::current_exception(); - } - }, - on_finish, title, message, maximum, style, true); - - if (!finished) - loop.Run(); - - std::optional result; - std::exception_ptr exception; - { - std::lock_guard lock(state->mutex); - if (state->result) - result.emplace(std::move(*state->result)); - exception = state->exception; - } - if (exception) - std::rethrow_exception(exception); - return std::move(*result); - } + return detail::run_wait_with_progress(std::forward(run), this, title, message, maximum, style, m_alive, [this] { restore_z_order(); }); } std::function m_open_terminal_dlg_fn; diff --git a/src/slic3r/GUI/Search.cpp b/src/slic3r/GUI/Search.cpp index 21aa8b7350..16ec2051a9 100644 --- a/src/slic3r/GUI/Search.cpp +++ b/src/slic3r/GUI/Search.cpp @@ -143,17 +143,24 @@ void OptionsSearcher::append_options(DynamicPrintConfig *config, Preset::Type ty } } -inline void OptionsSearcher::sort_options() +void OptionsSearcher::sort_options() { - std::sort(options.begin(), options.end(), [](const Option &o1, const Option &o2) { return o1.label < o2.label; }); - Option * last = nullptr; - for (auto& opt : options) { - if (last && last->label == opt.label && last->group == opt.group && last->type == opt.type && last->category != opt.category) { - last->multi_category = true; - opt.multi_category = true; + // Both views are label-sorted and multi_category-marked. They are separate consumers (the sidebar + // search and the Speed Dial); keeping them in sync here prevents the all-modes view from silently + // diverging in order or flags. + auto sort_and_mark = [](std::vector