From f2f0868be98ac2e1c92b52e478536a5f63f92305 Mon Sep 17 00:00:00 2001 From: alves Date: Mon, 2 Feb 2026 18:15:25 +0800 Subject: [PATCH 1/8] feature update the download manager config. fix upload and print dialog may crash bug. --- ...ownloadManager.cpp => DownloadManager.cpp} | 44 +++++++++---------- ...ownloadManager.hpp => DownloadManager.hpp} | 40 ++++++++--------- src/slic3r/GUI/GUI_App.cpp | 6 +-- src/slic3r/GUI/GUI_App.hpp | 6 +-- src/slic3r/GUI/Plater.cpp | 42 ------------------ src/slic3r/GUI/SSWCP.cpp | 9 ++-- src/slic3r/GUI/WebPreprintDialog.cpp | 38 ++++++++++++++-- src/slic3r/GUI/WebPreprintDialog.hpp | 6 ++- 8 files changed, 93 insertions(+), 98 deletions(-) rename src/slic3r/GUI/{WCPDownloadManager.cpp => DownloadManager.cpp} (85%) rename src/slic3r/GUI/{WCPDownloadManager.hpp => DownloadManager.hpp} (66%) diff --git a/src/slic3r/GUI/WCPDownloadManager.cpp b/src/slic3r/GUI/DownloadManager.cpp similarity index 85% rename from src/slic3r/GUI/WCPDownloadManager.cpp rename to src/slic3r/GUI/DownloadManager.cpp index a9d521aac3..30391441e5 100644 --- a/src/slic3r/GUI/WCPDownloadManager.cpp +++ b/src/slic3r/GUI/DownloadManager.cpp @@ -6,7 +6,7 @@ namespace Slic3r { namespace GUI { -size_t WCPDownloadManager::start_download(const std::string& file_url, +size_t DownloadManager::start_download(const std::string& file_url, const std::string& file_name, std::shared_ptr wcp_instance) { @@ -23,8 +23,8 @@ size_t WCPDownloadManager::start_download(const std::string& file_url, std::string dest_path = dest_file.string(); // Create task - auto task = std::make_shared(task_id, file_url, file_name, dest_path, wcp_instance); - task->state = WCPDownloadState::Downloading; + auto task = std::make_shared(task_id, file_url, file_name, dest_path, wcp_instance); + task->state = SMDownloadState::Downloading; m_tasks[task_id] = task; @@ -36,7 +36,7 @@ size_t WCPDownloadManager::start_download(const std::string& file_url, // Step 2: Set progress callback http.on_progress([this, task](Http::Progress progress, bool& cancel) { - if (task->state == WCPDownloadState::Canceled) { + if (task->state == SMDownloadState::Canceled) { cancel = true; return; } @@ -87,7 +87,7 @@ size_t WCPDownloadManager::start_download(const std::string& file_url, file.write(body.c_str(), body.size()); file.close(); - task->state = WCPDownloadState::Completed; + task->state = SMDownloadState::Completed; task->percent = 100; send_complete_update(task, task->dest_path); cleanup_task(task->task_id); @@ -101,7 +101,7 @@ size_t WCPDownloadManager::start_download(const std::string& file_url, // Step 4: Set error callback http.on_error([this, task](std::string body, std::string error, unsigned status) { wxGetApp().CallAfter([this, task, error, status]() { - task->state = WCPDownloadState::Error; + task->state = SMDownloadState::Error; task->error_message = error; send_error_update(task, error); cleanup_task(task->task_id); @@ -112,7 +112,7 @@ size_t WCPDownloadManager::start_download(const std::string& file_url, task->http_object = http.perform(); } catch (std::exception& e) { - task->state = WCPDownloadState::Error; + task->state = SMDownloadState::Error; task->error_message = e.what(); send_error_update(task, e.what()); cleanup_task(task->task_id); @@ -122,7 +122,7 @@ size_t WCPDownloadManager::start_download(const std::string& file_url, return task_id; } -bool WCPDownloadManager::cancel_download(size_t task_id) { +bool DownloadManager::cancel_download(size_t task_id) { std::shared_ptr wcp_to_destroy; { @@ -134,8 +134,8 @@ bool WCPDownloadManager::cancel_download(size_t task_id) { } auto task = it->second; - if (task->state == WCPDownloadState::Downloading) { - task->state = WCPDownloadState::Canceled; + if (task->state == SMDownloadState::Downloading) { + task->state = SMDownloadState::Canceled; if (task->http_object) { task->http_object->cancel(); } @@ -158,41 +158,41 @@ bool WCPDownloadManager::cancel_download(size_t task_id) { return true; } -bool WCPDownloadManager::pause_download(size_t task_id) { +bool DownloadManager::pause_download(size_t task_id) { // Pause functionality can be implemented if needed // Current Http module may not support pause, need to implement resume from breakpoint std::lock_guard lock(m_tasks_mutex); auto it = m_tasks.find(task_id); - if (it != m_tasks.end() && it->second->state == WCPDownloadState::Downloading) { - it->second->state = WCPDownloadState::Paused; + if (it != m_tasks.end() && it->second->state == SMDownloadState::Downloading) { + it->second->state = SMDownloadState::Paused; // Note: Http module doesn't support pause directly, would need breakpoint resume return true; } return false; } -bool WCPDownloadManager::resume_download(size_t task_id) { +bool DownloadManager::resume_download(size_t task_id) { // Resume functionality can be implemented if needed // Would require breakpoint resume support in Http module std::lock_guard lock(m_tasks_mutex); auto it = m_tasks.find(task_id); - if (it != m_tasks.end() && it->second->state == WCPDownloadState::Paused) { + if (it != m_tasks.end() && it->second->state == SMDownloadState::Paused) { // Would need to restart download with range header return false; // Not implemented yet } return false; } -WCPDownloadState WCPDownloadManager::get_task_state(size_t task_id) { +SMDownloadState DownloadManager::get_task_state(size_t task_id) { std::lock_guard lock(m_tasks_mutex); auto it = m_tasks.find(task_id); if (it != m_tasks.end()) { return it->second->state; } - return WCPDownloadState::Error; + return SMDownloadState::Error; } -std::shared_ptr WCPDownloadManager::get_task(size_t task_id) { +std::shared_ptr DownloadManager::get_task(size_t task_id) { std::lock_guard lock(m_tasks_mutex); auto it = m_tasks.find(task_id); if (it != m_tasks.end()) { @@ -201,7 +201,7 @@ std::shared_ptr WCPDownloadManager::get_task(size_t task_id) { return nullptr; } -void WCPDownloadManager::send_progress_update(std::shared_ptr task, +void DownloadManager::send_progress_update(std::shared_ptr task, int percent, size_t downloaded, size_t total) { @@ -227,7 +227,7 @@ void WCPDownloadManager::send_progress_update(std::shared_ptr t } } -void WCPDownloadManager::send_complete_update(std::shared_ptr task, +void DownloadManager::send_complete_update(std::shared_ptr task, const std::string& file_path) { if (auto wcp = task->wcp_instance.lock()) { json complete_data; @@ -247,7 +247,7 @@ void WCPDownloadManager::send_complete_update(std::shared_ptr t } } -void WCPDownloadManager::send_error_update(std::shared_ptr task, +void DownloadManager::send_error_update(std::shared_ptr task, const std::string& error) { if (auto wcp = task->wcp_instance.lock()) { json error_data; @@ -265,7 +265,7 @@ void WCPDownloadManager::send_error_update(std::shared_ptr task } } -void WCPDownloadManager::cleanup_task(size_t task_id) { +void DownloadManager::cleanup_task(size_t task_id) { std::lock_guard lock(m_tasks_mutex); m_tasks.erase(task_id); m_last_percent.erase(task_id); diff --git a/src/slic3r/GUI/WCPDownloadManager.hpp b/src/slic3r/GUI/DownloadManager.hpp similarity index 66% rename from src/slic3r/GUI/WCPDownloadManager.hpp rename to src/slic3r/GUI/DownloadManager.hpp index fda468f777..ebc0568b6d 100644 --- a/src/slic3r/GUI/WCPDownloadManager.hpp +++ b/src/slic3r/GUI/DownloadManager.hpp @@ -15,7 +15,7 @@ namespace Slic3r { namespace GUI { // Download task state -enum class WCPDownloadState { +enum class SMDownloadState { Pending, Downloading, Paused, @@ -25,28 +25,28 @@ enum class WCPDownloadState { }; // Download task information -struct WCPDownloadTask { +struct DownloadTask { size_t task_id; std::string file_url; std::string file_name; std::string dest_path; std::weak_ptr wcp_instance; // Associated WCP instance Http::Ptr http_object; // HTTP object for cancellation - WCPDownloadState state; + SMDownloadState state; int percent; std::string error_message; - WCPDownloadTask(size_t id, const std::string& url, const std::string& name, + DownloadTask(size_t id, const std::string& url, const std::string& name, const std::string& path, std::shared_ptr instance) - : task_id(id), file_url(url), file_name(name), dest_path(path), - wcp_instance(instance), state(WCPDownloadState::Pending), percent(0) {} + : task_id(id), file_url(url), file_name(name), dest_path(path), wcp_instance(instance), state(SMDownloadState::Pending), percent(0) + {} }; -// WCP Download Manager -class WCPDownloadManager { +// Download Manager +class DownloadManager { public: - static WCPDownloadManager& getInstance() { - static WCPDownloadManager instance; + static DownloadManager& getInstance() { + static DownloadManager instance; return instance; } @@ -65,19 +65,19 @@ public: bool resume_download(size_t task_id); // Get task state - WCPDownloadState get_task_state(size_t task_id); + SMDownloadState get_task_state(size_t task_id); // Get task information - std::shared_ptr get_task(size_t task_id); + std::shared_ptr get_task(size_t task_id); private: - WCPDownloadManager() = default; - ~WCPDownloadManager() = default; - WCPDownloadManager(const WCPDownloadManager&) = delete; - WCPDownloadManager& operator=(const WCPDownloadManager&) = delete; + DownloadManager() = default; + ~DownloadManager() = default; + DownloadManager(const DownloadManager&) = delete; + DownloadManager& operator=(const DownloadManager&) = delete; std::mutex m_tasks_mutex; - std::unordered_map> m_tasks; + std::unordered_map> m_tasks; std::atomic m_next_task_id{1}; // Track last progress update for throttling @@ -85,14 +85,14 @@ private: std::unordered_map m_last_update; // Send progress update to WCP - void send_progress_update(std::shared_ptr task, int percent, + void send_progress_update(std::shared_ptr task, int percent, size_t downloaded, size_t total); // Send completion message to WCP - void send_complete_update(std::shared_ptr task, const std::string& file_path); + void send_complete_update(std::shared_ptr task, const std::string& file_path); // Send error message to WCP - void send_error_update(std::shared_ptr task, const std::string& error); + void send_error_update(std::shared_ptr task, const std::string& error); // Clean up completed task void cleanup_task(size_t task_id); diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index 47987430c0..a0a511887f 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -1066,7 +1066,7 @@ GUI_App::GUI_App() , m_imgui(new ImGuiWrapper()) , m_removable_drive_manager(std::make_unique()) , m_downloader(std::make_unique()) - , m_wcp_download_manager(&WCPDownloadManager::getInstance()) + , m_download_manager(&DownloadManager::getInstance()) , m_other_instance_message_handler(std::make_unique()) { //app config initializes early becasuse it is used in instance checking in Snapmaker_Orca.cpp @@ -6478,9 +6478,9 @@ Downloader* GUI_App::downloader() return m_downloader.get(); } -WCPDownloadManager* GUI_App::wcp_download_manager() +DownloadManager* GUI_App::download_manager() { - return m_wcp_download_manager; + return m_download_manager; } void GUI_App::load_url(wxString url) diff --git a/src/slic3r/GUI/GUI_App.hpp b/src/slic3r/GUI/GUI_App.hpp index 38a950676a..7221430fbd 100644 --- a/src/slic3r/GUI/GUI_App.hpp +++ b/src/slic3r/GUI/GUI_App.hpp @@ -89,7 +89,7 @@ class Plater; class ParamsPanel; class NotificationManager; class Downloader; -class WCPDownloadManager; +class DownloadManager; struct GUI_InitParams; class ParamsDialog; class HMSQuery; @@ -298,7 +298,7 @@ private: size_t m_instance_hash_int; std::unique_ptr m_downloader; - WCPDownloadManager* m_wcp_download_manager; + DownloadManager* m_download_manager; //BBS bool m_is_closing {false}; @@ -686,7 +686,7 @@ private: Model& model(); NotificationManager * notification_manager(); Downloader* downloader(); - WCPDownloadManager* wcp_download_manager(); + DownloadManager* download_manager(); std::string m_mall_model_download_url; diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 26f953ff49..905e8582f4 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -13685,18 +13685,6 @@ void Plater::send_gcode_legacy(int plate_idx, Export3mfProgressFn proFn, bool us islegal = (c_preset == connect_preset); - /* if (!islegal) { - MessageDialog msg_window(nullptr, - _L(" Your connected machine is ") + (connect_preset == "" ? "Unknown" : connect_preset) + _L("\nYour model's preset is ") + c_preset + _L("\nDo you want to continue?"), - L("machine check"), - wxICON_QUESTION | wxOK); - int res = msg_window.ShowModal(); - if (res != wxID_OK) { - return; - } - }*/ - - DynamicPrintConfig* physical_printer_config = &Slic3r::GUI::wxGetApp().preset_bundle->printers.get_edited_preset().config; if (! physical_printer_config || p->model.objects.empty()) return; @@ -13715,34 +13703,6 @@ void Plater::send_gcode_legacy(int plate_idx, Export3mfProgressFn proFn, bool us local_name.erase(std::remove(local_name.begin(), local_name.end(), '('), local_name.end()); local_name.erase(std::remove(local_name.begin(), local_name.end(), ')'), local_name.end()); - - /*if (wxGetApp().app_config->get("use_new_connect") == "true") { - upload_job = PrintHostJob(wxGetApp().get_host_config()); - } */ - - // if (local_name == "Snapmaker U1 0.4 nozzle" && devices.size() == 0) { - // MessageDialog msg_window(nullptr, _L("You don't have active machine, do you want to add one?"), _L("Info"), wxICON_QUESTION | wxOK | wxCANCEL); - - // int res = msg_window.ShowModal(); - // if (res == wxID_OK) { - // wxGetApp().mainframe->request_select_tab(MainFrame::TabPosition::tpMonitor); - // auto view = wxGetApp().mainframe->m_printer_view; - // if (view) { - // json msg; - // msg["head"] = json::object(); - // json payload = json::object(); - // payload["cmd"] = "devicepage_add_device"; - // payload["method"] = "call_flutter"; - // payload["params"] = json::object(); - // msg["payload"] = payload; - - // std::string str_msg = msg.dump(4, ' ', true); - // view->sendMessage(str_msg); - // } - // } - // return; - // } - if (wxGetApp().app_config->get("use_new_connect") == "true" || local_name == "Snapmaker U1 0.4 nozzle") { // 先不创建job，直接创建上传 / 上传下载对话框 // 获取默认文件名 @@ -13804,8 +13764,6 @@ void Plater::send_gcode_legacy(int plate_idx, Export3mfProgressFn proFn, bool us dialog->set_display_file_name(upload_job.upload_data.upload_path.string()); bool res = dialog->run(); - // wxGetApp().mainframe->m_printer_view->reload(); - if (dialog->is_finish()) { wxGetApp().mainframe->select_tab(MainFrame::TabPosition::tpMonitor); } diff --git a/src/slic3r/GUI/SSWCP.cpp b/src/slic3r/GUI/SSWCP.cpp index 33de5066f0..488072e504 100644 --- a/src/slic3r/GUI/SSWCP.cpp +++ b/src/slic3r/GUI/SSWCP.cpp @@ -3001,10 +3001,11 @@ void SSWCP_MachineOption_Instance::sw_FinishFilamentMapping() if (wxGetApp().get_web_preprint_dialog()) { WebPreprintDialog* dialog = dynamic_cast(wxGetApp().get_web_preprint_dialog()); if (dialog) { + // BBS: Use SafeEndModal to prevent duplicate EndModal calls if(dialog->is_finish()){ - dialog->EndModal(wxID_OK); + dialog->SafeEndModal(wxID_OK); }else{ - dialog->EndModal(wxID_CANCEL); + dialog->SafeEndModal(wxID_CANCEL); } } } @@ -4397,7 +4398,7 @@ void SSWCP_UserLogin_Instance::sw_DownloadFile() { } // Use WCP Download Manager - WCPDownloadManager* download_mgr = wxGetApp().wcp_download_manager(); + DownloadManager* download_mgr = wxGetApp().download_manager(); if (!download_mgr) { handle_general_fail(-1, "WCP Download Manager not available"); return; @@ -4432,7 +4433,7 @@ void SSWCP_UserLogin_Instance::sw_CancelDownload() { return; } - WCPDownloadManager* download_mgr = wxGetApp().wcp_download_manager(); + DownloadManager* download_mgr = wxGetApp().download_manager(); if (!download_mgr) { handle_general_fail(-1, "WCP Download Manager not available"); return; diff --git a/src/slic3r/GUI/WebPreprintDialog.cpp b/src/slic3r/GUI/WebPreprintDialog.cpp index 1ec1cfa56e..eb192a1f9c 100644 --- a/src/slic3r/GUI/WebPreprintDialog.cpp +++ b/src/slic3r/GUI/WebPreprintDialog.cpp @@ -96,6 +96,22 @@ void WebPreprintDialog::set_display_file_name(const std::string& filename) { void WebPreprintDialog::set_gcode_file_name(const std::string& filename) { m_gcode_file_name = filename; } +void WebPreprintDialog::set_finish(bool flag) +{ + m_finish = flag; + // BBS: Don't call EndModal here to avoid conflict with sw_FinishFilamentMapping() + // The external sw_FinishFilamentMapping() function will handle EndModal based on m_finish flag +} + +void WebPreprintDialog::SafeEndModal(int returnCode) +{ + // BBS: Prevent duplicate EndModal calls which can cause crashes + if (IsModal() && !m_modal_ended) { + m_modal_ended = true; + EndModal(returnCode); + } +} + void WebPreprintDialog::reload() { load_url(m_prePrint_url); @@ -123,8 +139,16 @@ bool WebPreprintDialog::run() } this->load_url(real_url); - if (this->ShowModal() == wxID_OK) { - return true; + + // BBS: Reset flags before showing modal + m_finish = false; + m_modal_ended = false; + + int result = this->ShowModal(); + + // BBS: Check finish flag to determine return value + if (result == wxID_OK || (result == wxID_CANCEL && m_finish)) { + return m_finish; } return false; } @@ -186,7 +210,15 @@ void WebPreprintDialog::OnClose(wxCloseEvent& evt) { auto noti_manager = wxGetApp().mainframe->plater()->get_notification_manager(); noti_manager->close_notification_of_type(NotificationType::PrintHostUpload); - evt.Skip(); + + // BBS: Use SafeEndModal to prevent duplicate EndModal calls + // This ensures consistency with sw_FinishFilamentMapping() and prevents crashes + SafeEndModal(wxID_CANCEL); + + // If not modal or already ended, skip the event + if (!IsModal() || m_modal_ended) { + evt.Skip(); + } } }} // namespace Slic3r::GUI \ No newline at end of file diff --git a/src/slic3r/GUI/WebPreprintDialog.hpp b/src/slic3r/GUI/WebPreprintDialog.hpp index c077d1204a..98ca1b7523 100644 --- a/src/slic3r/GUI/WebPreprintDialog.hpp +++ b/src/slic3r/GUI/WebPreprintDialog.hpp @@ -33,7 +33,10 @@ public: bool is_finish() { return m_finish; } - void set_finish(bool flag) { m_finish = flag; } + void set_finish(bool flag); + + // BBS: Safely end modal dialog, preventing duplicate EndModal calls + void SafeEndModal(int returnCode); private: void OnClose(wxCloseEvent& evt); @@ -53,6 +56,7 @@ private: bool m_switch_to_device = false; bool m_finish = false; + bool m_modal_ended = false; // BBS: Flag to prevent duplicate EndModal calls DECLARE_EVENT_TABLE() }; From 44127be8dad347ee5fd0d934478a4f239f154e0c Mon Sep 17 00:00:00 2001 From: alves Date: Mon, 2 Feb 2026 18:16:15 +0800 Subject: [PATCH 2/8] feature update download manager name. --- src/slic3r/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/slic3r/CMakeLists.txt b/src/slic3r/CMakeLists.txt index e74b6c66ed..8122c67092 100644 --- a/src/slic3r/CMakeLists.txt +++ b/src/slic3r/CMakeLists.txt @@ -504,8 +504,8 @@ set(SLIC3R_GUI_SOURCES GUI/SMPhysicalPrinterDialog.cpp GUI/SSWCP.cpp GUI/SSWCP.hpp - GUI/WCPDownloadManager.cpp - GUI/WCPDownloadManager.hpp + GUI/DownloadManager.cpp + GUI/DownloadManager.hpp GUI/WebPresetDialog.hpp GUI/WebPresetDialog.cpp GUI/WebSMUserLoginDialog.cpp From 3842444724985be039c727b753e04e5d860a7d99 Mon Sep 17 00:00:00 2001 From: alves Date: Tue, 3 Feb 2026 17:39:30 +0800 Subject: [PATCH 3/8] feature add download function for web and pc. --- src/slic3r/GUI/DownloadManager.cpp | 234 +++++++++++++++++++++++------ src/slic3r/GUI/DownloadManager.hpp | 148 +++++++++++++++--- src/slic3r/GUI/GUI_App.cpp | 2 +- src/slic3r/GUI/SSWCP.cpp | 57 +++++-- src/slic3r/GUI/SSWCP.hpp | 3 + 5 files changed, 364 insertions(+), 80 deletions(-) diff --git a/src/slic3r/GUI/DownloadManager.cpp b/src/slic3r/GUI/DownloadManager.cpp index 30391441e5..345ebf8dd0 100644 --- a/src/slic3r/GUI/DownloadManager.cpp +++ b/src/slic3r/GUI/DownloadManager.cpp @@ -1,20 +1,75 @@ -#include "WCPDownloadManager.hpp" +#include "DownloadManager.hpp" #include "GUI_App.hpp" #include #include #include +#include namespace Slic3r { namespace GUI { -size_t DownloadManager::start_download(const std::string& file_url, - const std::string& file_name, - std::shared_ptr wcp_instance) { - - std::lock_guard lock(m_tasks_mutex); +// ============================================================================ +// WCP Download Interface (for Web-to-PC communication) +// ============================================================================ +size_t DownloadManager::start_wcp_download(const std::string& file_url, + const std::string& file_name, + std::shared_ptr wcp_instance, + bool use_original_event_id) { + std::lock_guard lock(m_tasks_mutex); size_t task_id = m_next_task_id++; - // Get download path + auto downloadPath = wxGetApp().app_config->get("download_path"); + boost::filesystem::path dest_folder(downloadPath); + boost::filesystem::create_directories(dest_folder); + boost::filesystem::path dest_file = dest_folder / file_name; + std::string dest_path = dest_file.string(); + + auto task = std::make_shared(task_id, + file_url, + file_name, + dest_path, + wcp_instance, + use_original_event_id); + + task->state = DownloadTaskState::Downloading; + + m_tasks[task_id] = task; + start_download_impl(task); + return task_id; +} + +// ============================================================================ +// Internal Download Interface (for PC internal use) +// ============================================================================ +size_t DownloadManager::start_internal_download(const std::string& file_url, + const std::string& file_name, + const std::string& dest_path, + DownloadCallbacks callbacks) { + + std::lock_guard lock(m_tasks_mutex); + size_t task_id = m_next_task_id++; + + boost::filesystem::path dest_file_path(dest_path); + boost::filesystem::create_directories(dest_file_path.parent_path()); + + auto task = std::make_shared(task_id, + file_url, + file_name, + dest_path, + std::move(callbacks)); + + task->state = DownloadTaskState::Downloading; + + m_tasks[task_id] = task; + start_download_impl(task); + + return task_id; +} + +size_t DownloadManager::start_internal_download(const std::string& file_url, + const std::string& file_name, + DownloadCallbacks callbacks) { + // Get default download path auto downloadPath = wxGetApp().app_config->get("download_path"); boost::filesystem::path dest_folder(downloadPath); boost::filesystem::create_directories(dest_folder); @@ -22,26 +77,25 @@ size_t DownloadManager::start_download(const std::string& file_url, boost::filesystem::path dest_file = dest_folder / file_name; std::string dest_path = dest_file.string(); - // Create task - auto task = std::make_shared(task_id, file_url, file_name, dest_path, wcp_instance); - task->state = SMDownloadState::Downloading; + return start_internal_download(file_url, file_name, dest_path, std::move(callbacks)); +} + +void DownloadManager::start_download_impl(std::shared_ptr task) { - m_tasks[task_id] = task; - - // Start download wxGetApp().CallAfter([this, task]() { try { // Step 1: Create Http object Http http = Http::get(task->file_url); + http.timeout_max(0); + // Step 2: Set progress callback http.on_progress([this, task](Http::Progress progress, bool& cancel) { - if (task->state == SMDownloadState::Canceled) { + if (task->state == DownloadTaskState::Canceled) { cancel = true; return; } - // Calculate progress int percent = 0; if (progress.dltotal > 0) { percent = (int)(progress.dlnow * 100 / progress.dltotal); @@ -87,7 +141,7 @@ size_t DownloadManager::start_download(const std::string& file_url, file.write(body.c_str(), body.size()); file.close(); - task->state = SMDownloadState::Completed; + task->state = DownloadTaskState::Completed; task->percent = 100; send_complete_update(task, task->dest_path); cleanup_task(task->task_id); @@ -101,7 +155,7 @@ size_t DownloadManager::start_download(const std::string& file_url, // Step 4: Set error callback http.on_error([this, task](std::string body, std::string error, unsigned status) { wxGetApp().CallAfter([this, task, error, status]() { - task->state = SMDownloadState::Error; + task->state = DownloadTaskState::Error; task->error_message = error; send_error_update(task, error); cleanup_task(task->task_id); @@ -112,16 +166,15 @@ size_t DownloadManager::start_download(const std::string& file_url, task->http_object = http.perform(); } catch (std::exception& e) { - task->state = SMDownloadState::Error; + task->state = DownloadTaskState::Error; task->error_message = e.what(); send_error_update(task, e.what()); cleanup_task(task->task_id); } }); - - return task_id; } +//this function not currently in use bool DownloadManager::cancel_download(size_t task_id) { std::shared_ptr wcp_to_destroy; @@ -134,23 +187,32 @@ bool DownloadManager::cancel_download(size_t task_id) { } auto task = it->second; - if (task->state == SMDownloadState::Downloading) { - task->state = SMDownloadState::Canceled; + if (task->state == DownloadTaskState::Downloading) { + task->state = DownloadTaskState::Canceled; if (task->http_object) { task->http_object->cancel(); } - // Get WCP instance before cleanup (for destruction after lock release) - wcp_to_destroy = task->wcp_instance.lock(); + // Only for WCP downloads + if (task->is_wcp_download()) { + wcp_to_destroy = task->wcp_instance.lock(); + } else { + // For internal downloads, call error callback if available + if (task->callbacks.on_error) { + wxGetApp().CallAfter([task]() { + if (task->callbacks.on_error) { + task->callbacks.on_error(task->task_id, "Download canceled"); + } + }); + } + } cleanup_task(task_id); } else { return false; } } - - // Destroy WCP instance outside the lock to prevent deadlock - // This is the WCP instance from the original download request (sw_DownloadFile) + if (wcp_to_destroy) { wcp_to_destroy->finish_job(); } @@ -158,40 +220,38 @@ bool DownloadManager::cancel_download(size_t task_id) { return true; } +// this function not currently in use bool DownloadManager::pause_download(size_t task_id) { - // Pause functionality can be implemented if needed - // Current Http module may not support pause, need to implement resume from breakpoint std::lock_guard lock(m_tasks_mutex); auto it = m_tasks.find(task_id); - if (it != m_tasks.end() && it->second->state == SMDownloadState::Downloading) { - it->second->state = SMDownloadState::Paused; - // Note: Http module doesn't support pause directly, would need breakpoint resume + if (it != m_tasks.end() && it->second->state == DownloadTaskState::Downloading) { + it->second->state = DownloadTaskState::Paused; return true; } return false; } +// this function not currently in use bool DownloadManager::resume_download(size_t task_id) { - // Resume functionality can be implemented if needed - // Would require breakpoint resume support in Http module std::lock_guard lock(m_tasks_mutex); auto it = m_tasks.find(task_id); - if (it != m_tasks.end() && it->second->state == SMDownloadState::Paused) { - // Would need to restart download with range header - return false; // Not implemented yet + if (it != m_tasks.end() && it->second->state == DownloadTaskState::Paused) { + return false; } return false; } -SMDownloadState DownloadManager::get_task_state(size_t task_id) { +// this function not currently in use +DownloadTaskState DownloadManager::get_task_state(size_t task_id) { std::lock_guard lock(m_tasks_mutex); auto it = m_tasks.find(task_id); if (it != m_tasks.end()) { return it->second->state; } - return SMDownloadState::Error; + return DownloadTaskState::Error; } +// this function not currently in use std::shared_ptr DownloadManager::get_task(size_t task_id) { std::lock_guard lock(m_tasks_mutex); auto it = m_tasks.find(task_id); @@ -201,10 +261,39 @@ std::shared_ptr DownloadManager::get_task(size_t task_id) { return nullptr; } +// this function not currently in use +std::vector> DownloadManager::get_all_tasks() { + std::lock_guard lock(m_tasks_mutex); + std::vector> result; + result.reserve(m_tasks.size()); + for (const auto& pair : m_tasks) { + result.push_back(pair.second); + } + return result; +} + +// ============================================================================ +// Progress/Complete/Error Update Handlers +// ============================================================================ void DownloadManager::send_progress_update(std::shared_ptr task, int percent, size_t downloaded, size_t total) { + if (task->is_wcp_download()) { + send_wcp_progress_update(task, percent, downloaded, total); + } else { + call_internal_progress_callback(task, percent, downloaded, total); + } +} + +void DownloadManager::send_wcp_progress_update(std::shared_ptr task, + int percent, + size_t downloaded, + size_t total) { + if (!task->use_original_event_id) { + return; + } + if (auto wcp = task->wcp_instance.lock()) { json progress_data; progress_data["task_id"] = task->task_id; @@ -216,10 +305,14 @@ void DownloadManager::send_progress_update(std::shared_ptr task, wcp->m_res_data = progress_data; wcp->m_status = 0; wcp->m_msg = "Download progress"; - - // Use progress event ID + json header; - header["event_id"] = wcp->m_event_id + "_progress"; + if (task->use_original_event_id) { + header["event_id"] = wcp->m_event_id; + } else { + header["event_id"] = wcp->m_event_id + "_progress"; + } + header["command"] = "download_progress"; wcp->m_header = header; @@ -227,8 +320,31 @@ void DownloadManager::send_progress_update(std::shared_ptr task, } } +void DownloadManager::call_internal_progress_callback(std::shared_ptr task, + int percent, + size_t downloaded, + size_t total) { + if (task->callbacks.on_progress) { + task->callbacks.on_progress(task->task_id, percent, downloaded, total); + } +} + void DownloadManager::send_complete_update(std::shared_ptr task, const std::string& file_path) { + if (task->is_wcp_download()) { + send_wcp_complete_update(task, file_path); + } else { + call_internal_complete_callback(task, file_path); + } +} + +void DownloadManager::send_wcp_complete_update(std::shared_ptr task, + const std::string& file_path) { + + if (!task->use_original_event_id) { + return; + } + if (auto wcp = task->wcp_instance.lock()) { json complete_data; complete_data["task_id"] = task->task_id; @@ -240,15 +356,35 @@ void DownloadManager::send_complete_update(std::shared_ptr task, wcp->m_res_data = complete_data; wcp->m_status = 0; wcp->m_msg = "Download completed"; - wcp->send_to_js(); - // Release WCP instance to prevent memory leak + wcp->send_to_js(); wcp->finish_job(); } } +void DownloadManager::call_internal_complete_callback(std::shared_ptr task, + const std::string& file_path) { + if (task->callbacks.on_complete) { + task->callbacks.on_complete(task->task_id, file_path); + } +} + void DownloadManager::send_error_update(std::shared_ptr task, const std::string& error) { + if (task->is_wcp_download()) { + send_wcp_error_update(task, error); + } else { + call_internal_error_callback(task, error); + } +} + +void DownloadManager::send_wcp_error_update(std::shared_ptr task, + const std::string& error) { + + if (!task->use_original_event_id) { + return; + } + if (auto wcp = task->wcp_instance.lock()) { json error_data; error_data["task_id"] = task->task_id; @@ -258,13 +394,19 @@ void DownloadManager::send_error_update(std::shared_ptr task, wcp->m_res_data = error_data; wcp->m_status = -1; wcp->m_msg = error; + wcp->send_to_js(); - - // Release WCP instance to prevent memory leak wcp->finish_job(); } } +void DownloadManager::call_internal_error_callback(std::shared_ptr task, + const std::string& error) { + if (task->callbacks.on_error) { + task->callbacks.on_error(task->task_id, error); + } +} + void DownloadManager::cleanup_task(size_t task_id) { std::lock_guard lock(m_tasks_mutex); m_tasks.erase(task_id); diff --git a/src/slic3r/GUI/DownloadManager.hpp b/src/slic3r/GUI/DownloadManager.hpp index ebc0568b6d..c61ad574e6 100644 --- a/src/slic3r/GUI/DownloadManager.hpp +++ b/src/slic3r/GUI/DownloadManager.hpp @@ -1,5 +1,5 @@ -#ifndef slic3r_WCPDownloadManager_hpp_ -#define slic3r_WCPDownloadManager_hpp_ +#ifndef slic3r_DownloadManager_hpp_ +#define slic3r_DownloadManager_hpp_ #include #include @@ -7,6 +7,7 @@ #include #include #include +#include #include "../Utils/Http.hpp" #include "SSWCP.hpp" #include @@ -14,8 +15,8 @@ namespace Slic3r { namespace GUI { -// Download task state -enum class SMDownloadState { +// Download task state (renamed to avoid conflict with Downloader::DownloadState) +enum class DownloadTaskState { Pending, Downloading, Paused, @@ -24,25 +25,66 @@ enum class SMDownloadState { Canceled }; +// Download callback interface for internal downloads +struct DownloadCallbacks { + std::function on_progress; + std::function on_complete; + std::function on_error; + + DownloadCallbacks() = default; + DownloadCallbacks( + std::function progress, + std::function complete, + std::function error) + : on_progress(std::move(progress)) + , on_complete(std::move(complete)) + , on_error(std::move(error)) + {} +}; + // Download task information struct DownloadTask { size_t task_id; std::string file_url; std::string file_name; std::string dest_path; - std::weak_ptr wcp_instance; // Associated WCP instance - Http::Ptr http_object; // HTTP object for cancellation - SMDownloadState state; + + std::weak_ptr wcp_instance; + + DownloadCallbacks callbacks; + + Http::Ptr http_object; + DownloadTaskState state; int percent; std::string error_message; + bool auto_finish_job; + bool use_original_event_id; + + // Constructor for WCP downloads DownloadTask(size_t id, const std::string& url, const std::string& name, - const std::string& path, std::shared_ptr instance) - : task_id(id), file_url(url), file_name(name), dest_path(path), wcp_instance(instance), state(SMDownloadState::Pending), percent(0) + const std::string& path, std::shared_ptr instance, + bool use_original_event = false) + : task_id(id), file_url(url), file_name(name), dest_path(path) + , wcp_instance(instance), state(DownloadTaskState::Pending), percent(0) + , auto_finish_job(false), use_original_event_id(use_original_event) {} + + // Constructor for internal downloads + DownloadTask(size_t id, const std::string& url, const std::string& name, + const std::string& path, DownloadCallbacks cb) + : task_id(id), file_url(url), file_name(name), dest_path(path) + , callbacks(std::move(cb)), state(DownloadTaskState::Pending), percent(0) + , auto_finish_job(false), use_original_event_id(false) + {} + + // Check if this is a WCP download + bool is_wcp_download() const { + return !wcp_instance.expired(); + } }; -// Download Manager + class DownloadManager { public: static DownloadManager& getInstance() { @@ -50,11 +92,29 @@ public: return instance; } - // Start a download task - size_t start_download(const std::string& file_url, - const std::string& file_name, - std::shared_ptr wcp_instance); + // ============================================================================ + // WCP Download Interface (for Web-to-PC communication) + // ============================================================================ + size_t start_wcp_download(const std::string& file_url, + const std::string& file_name, + std::shared_ptr wcp_instance, + bool use_original_event_id = false); + // ============================================================================ + // Internal Download Interface (for PC internal use) + // ============================================================================ + size_t start_internal_download(const std::string& file_url, + const std::string& file_name, + const std::string& dest_path, + DownloadCallbacks callbacks); + + size_t start_internal_download(const std::string& file_url, + const std::string& file_name, + DownloadCallbacks callbacks); + + // ============================================================================ + // Common Interface (works for both WCP and internal downloads) + // ============================================================================ // Cancel a download task bool cancel_download(size_t task_id); @@ -65,11 +125,14 @@ public: bool resume_download(size_t task_id); // Get task state - SMDownloadState get_task_state(size_t task_id); + DownloadTaskState get_task_state(size_t task_id); // Get task information std::shared_ptr get_task(size_t task_id); + // Get all active tasks + std::vector> get_all_tasks(); + private: DownloadManager() = default; ~DownloadManager() = default; @@ -84,15 +147,54 @@ private: std::unordered_map m_last_percent; std::unordered_map m_last_update; - // Send progress update to WCP - void send_progress_update(std::shared_ptr task, int percent, - size_t downloaded, size_t total); + // ============================================================================ + // Internal Implementation + // ============================================================================ - // Send completion message to WCP - void send_complete_update(std::shared_ptr task, const std::string& file_path); + // Common download implementation (used by both WCP and internal downloads) + void start_download_impl(std::shared_ptr task); - // Send error message to WCP - void send_error_update(std::shared_ptr task, const std::string& error); + // Send progress update (handles both WCP and internal modes) + void send_progress_update(std::shared_ptr task, + int percent, + size_t downloaded, + size_t total); + + // Send completion message (handles both WCP and internal modes) + void send_complete_update(std::shared_ptr task, + const std::string& file_path); + + // Send error message (handles both WCP and internal modes) + void send_error_update(std::shared_ptr task, + const std::string& error); + + // WCP-specific: Send progress update via WCP instance + void send_wcp_progress_update(std::shared_ptr task, + int percent, + size_t downloaded, + size_t total); + + // WCP-specific: Send completion via WCP instance + void send_wcp_complete_update(std::shared_ptr task, + const std::string& file_path); + + // WCP-specific: Send error via WCP instance + void send_wcp_error_update(std::shared_ptr task, + const std::string& error); + + // Internal-specific: Call progress callback + void call_internal_progress_callback(std::shared_ptr task, + int percent, + size_t downloaded, + size_t total); + + // Internal-specific: Call complete callback + void call_internal_complete_callback(std::shared_ptr task, + const std::string& file_path); + + // Internal-specific: Call error callback + void call_internal_error_callback(std::shared_ptr task, + const std::string& error); // Clean up completed task void cleanup_task(size_t task_id); @@ -100,5 +202,5 @@ private: }} // namespace Slic3r::GUI -#endif // slic3r_WCPDownloadManager_hpp_ +#endif // slic3r_DownloadManager_hpp_ diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index a0a511887f..958630d939 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -13,7 +13,7 @@ #include "slic3r/GUI/WebPresetDialog.hpp" #include "slic3r/GUI/SSWCP.hpp" -#include "slic3r/GUI/WCPDownloadManager.hpp" +#include "slic3r/GUI/DownloadManager.hpp" #include "slic3r/Utils/PresetUpdater.hpp" #include "slic3r/Config/Version.hpp" diff --git a/src/slic3r/GUI/SSWCP.cpp b/src/slic3r/GUI/SSWCP.cpp index 488072e504..e8aac1accb 100644 --- a/src/slic3r/GUI/SSWCP.cpp +++ b/src/slic3r/GUI/SSWCP.cpp @@ -2,7 +2,7 @@ #include "SSWCP.hpp" #include "GUI_App.hpp" #include "MainFrame.hpp" -#include "WCPDownloadManager.hpp" +#include "DownloadManager.hpp" #include "nlohmann/json.hpp" #include "slic3r/GUI/Tab.hpp" #include "sentry_wrapper/SentryWrapper.hpp" @@ -4387,7 +4387,8 @@ void SSWCP_UserLogin_Instance::sw_GetUserUpdatePrivacy() } -void SSWCP_UserLogin_Instance::sw_DownloadFile() { +void SSWCP_UserLogin_Instance::sw_DownloadFile() +{ try { std::string fileName = m_param_data.count("file_name") ? m_param_data["file_name"].get() : ""; std::string fileUrl = m_param_data.count("file_url") ? m_param_data["file_url"].get() : ""; @@ -4397,17 +4398,55 @@ void SSWCP_UserLogin_Instance::sw_DownloadFile() { return; } - // Use WCP Download Manager + // Use Download Manager DownloadManager* download_mgr = wxGetApp().download_manager(); if (!download_mgr) { - handle_general_fail(-1, "WCP Download Manager not available"); + handle_general_fail(-1, "Download Manager not available"); return; } - // Start download task - size_t task_id = download_mgr->start_download(fileUrl, fileName, shared_from_this()); - + size_t task_id = download_mgr->start_wcp_download(fileUrl, + fileName, + shared_from_this(), + false); // use_original_event_id = false (sw_DownloadFile: finish immediately) + // Return task ID to Flutter + json response; + response["task_id"] = task_id; + response["file_name"] = fileName; + response["file_url"] = fileUrl; + m_res_data = response; + m_status = 0; + m_msg = "Download started"; + send_to_js(); + finish_job(); + + } catch (std::exception& e) { + handle_general_fail(-1, e.what()); + } +} + +void SSWCP_UserLogin_Instance::sw_DownloadFileEx() { + try { + std::string fileName = m_param_data.count("file_name") ? m_param_data["file_name"].get() : ""; + std::string fileUrl = m_param_data.count("file_url") ? m_param_data["file_url"].get() : ""; + + if (fileUrl.empty() || fileName.empty()) { + handle_general_fail(-1, "file_url and file_name are required"); + return; + } + + // Use Download Manager + DownloadManager* download_mgr = wxGetApp().download_manager(); + if (!download_mgr) { + handle_general_fail(-1, "Download Manager not available"); + return; + } + size_t task_id = download_mgr->start_wcp_download(fileUrl, + fileName, + shared_from_this(), + true); + json response; response["task_id"] = task_id; response["file_name"] = fileName; @@ -4416,9 +4455,7 @@ void SSWCP_UserLogin_Instance::sw_DownloadFile() { m_status = 0; m_msg = "Download started"; send_to_js(); - // Note: Do not call finish_job() here, as download is asynchronous - // The manager will send progress updates and completion/error messages via WCP - + } catch (std::exception& e) { handle_general_fail(-1, e.what()); } diff --git a/src/slic3r/GUI/SSWCP.hpp b/src/slic3r/GUI/SSWCP.hpp index fe40a45a42..2e7c1bdd5a 100644 --- a/src/slic3r/GUI/SSWCP.hpp +++ b/src/slic3r/GUI/SSWCP.hpp @@ -541,6 +541,9 @@ private: void sw_SubUserUpdatePrivacy(); void sw_DownloadFile(); + + void sw_DownloadFileEx(); + void sw_CancelDownload(); void sw_FileView(); From ae71ebd9085141d6e6646114d5425637ed2791be Mon Sep 17 00:00:00 2001 From: alves Date: Wed, 4 Feb 2026 15:19:52 +0800 Subject: [PATCH 4/8] feature add download dialog for download file. --- src/slic3r/CMakeLists.txt | 2 + src/slic3r/GUI/BBLStatusBarSend.cpp | 1 + src/slic3r/GUI/DownloadManager.cpp | 87 ++++- src/slic3r/GUI/DownloadManager.hpp | 4 + src/slic3r/GUI/GenericDownloadDialog.cpp | 423 +++++++++++++++++++++++ src/slic3r/GUI/GenericDownloadDialog.hpp | 113 ++++++ 6 files changed, 616 insertions(+), 14 deletions(-) create mode 100644 src/slic3r/GUI/GenericDownloadDialog.cpp create mode 100644 src/slic3r/GUI/GenericDownloadDialog.hpp diff --git a/src/slic3r/CMakeLists.txt b/src/slic3r/CMakeLists.txt index 8122c67092..41a2916983 100644 --- a/src/slic3r/CMakeLists.txt +++ b/src/slic3r/CMakeLists.txt @@ -506,6 +506,8 @@ set(SLIC3R_GUI_SOURCES GUI/SSWCP.hpp GUI/DownloadManager.cpp GUI/DownloadManager.hpp + GUI/GenericDownloadDialog.cpp + GUI/GenericDownloadDialog.hpp GUI/WebPresetDialog.hpp GUI/WebPresetDialog.cpp GUI/WebSMUserLoginDialog.cpp diff --git a/src/slic3r/GUI/BBLStatusBarSend.cpp b/src/slic3r/GUI/BBLStatusBarSend.cpp index 3976ae7bb1..71ab16ff08 100644 --- a/src/slic3r/GUI/BBLStatusBarSend.cpp +++ b/src/slic3r/GUI/BBLStatusBarSend.cpp @@ -57,6 +57,7 @@ BBLStatusBarSend::BBLStatusBarSend(wxWindow *parent, int id) m_cancelbutton->SetBorderColor(btn_bd_white); m_cancelbutton->SetTextColor(btn_txt_white); m_cancelbutton->SetCornerRadius(m_self->FromDIP(12)); + m_cancelbutton->SetCursor(wxCURSOR_HAND); m_cancelbutton->Bind(wxEVT_BUTTON, [this](wxCommandEvent &evt) { m_was_cancelled = true; diff --git a/src/slic3r/GUI/DownloadManager.cpp b/src/slic3r/GUI/DownloadManager.cpp index 345ebf8dd0..72faffd178 100644 --- a/src/slic3r/GUI/DownloadManager.cpp +++ b/src/slic3r/GUI/DownloadManager.cpp @@ -3,10 +3,37 @@ #include #include #include +#include #include namespace Slic3r { namespace GUI { +// ============================================================================ +// Helper Functions +// ============================================================================ + +std::string DownloadManager::get_unique_file_path(const boost::filesystem::path& file_path) +{ + if (!boost::filesystem::exists(file_path)) { + return file_path.string(); + } + + boost::filesystem::path parent_dir = file_path.parent_path(); + std::string filename = file_path.filename().string(); + std::string extension = file_path.extension().string(); + std::string name_without_ext = filename.substr(0, filename.size() - extension.size()); + + size_t version = 1; + boost::filesystem::path unique_path; + do { + std::string new_filename = name_without_ext + "(" + std::to_string(version) + ")" + extension; + unique_path = parent_dir / new_filename; + version++; + } while (boost::filesystem::exists(unique_path) && version < 10000); // Safety limit + + return unique_path.string(); +} + // ============================================================================ // WCP Download Interface (for Web-to-PC communication) // ============================================================================ @@ -22,11 +49,16 @@ size_t DownloadManager::start_wcp_download(const std::string& file_url, boost::filesystem::path dest_folder(downloadPath); boost::filesystem::create_directories(dest_folder); boost::filesystem::path dest_file = dest_folder / file_name; - std::string dest_path = dest_file.string(); + + // Generate unique file path if file already exists + std::string dest_path = get_unique_file_path(dest_file); + + // Update file_name if it was changed due to duplicate + std::string actual_file_name = boost::filesystem::path(dest_path).filename().string(); auto task = std::make_shared(task_id, file_url, - file_name, + actual_file_name, dest_path, wcp_instance, use_original_event_id); @@ -51,11 +83,14 @@ size_t DownloadManager::start_internal_download(const std::string& file_url, boost::filesystem::path dest_file_path(dest_path); boost::filesystem::create_directories(dest_file_path.parent_path()); + + // Generate unique file path if file already exists + std::string unique_dest_path = get_unique_file_path(dest_file_path); auto task = std::make_shared(task_id, file_url, file_name, - dest_path, + unique_dest_path, std::move(callbacks)); task->state = DownloadTaskState::Downloading; @@ -75,7 +110,9 @@ size_t DownloadManager::start_internal_download(const std::string& file_url, boost::filesystem::create_directories(dest_folder); boost::filesystem::path dest_file = dest_folder / file_name; - std::string dest_path = dest_file.string(); + + // Generate unique file path if file already exists + std::string dest_path = get_unique_file_path(dest_file); return start_internal_download(file_url, file_name, dest_path, std::move(callbacks)); } @@ -133,12 +170,22 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { // Save file boost::nowide::ofstream file(task->dest_path, std::ios::binary); if (!file.is_open()) { - send_error_update(task, "Failed to open file for writing"); + std::string error_msg = "Failed to open file for writing: " + task->dest_path; + BOOST_LOG_TRIVIAL(error) << "DownloadManager: " << error_msg; + send_error_update(task, error_msg); cleanup_task(task->task_id); return; } file.write(body.c_str(), body.size()); + if (file.fail()) { + std::string error_msg = "Failed to write file: " + task->dest_path; + BOOST_LOG_TRIVIAL(error) << "DownloadManager: " << error_msg << ", body size: " << body.size(); + file.close(); + send_error_update(task, error_msg); + cleanup_task(task->task_id); + return; + } file.close(); task->state = DownloadTaskState::Completed; @@ -146,7 +193,9 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { send_complete_update(task, task->dest_path); cleanup_task(task->task_id); } catch (std::exception& e) { - send_error_update(task, e.what()); + std::string error_msg = std::string("File write exception: ") + e.what(); + BOOST_LOG_TRIVIAL(error) << "DownloadManager: " << error_msg << ", file: " << task->dest_path; + send_error_update(task, error_msg); cleanup_task(task->task_id); } }); @@ -155,6 +204,11 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { // Step 4: Set error callback http.on_error([this, task](std::string body, std::string error, unsigned status) { wxGetApp().CallAfter([this, task, error, status]() { + std::string error_msg = boost::str(boost::format("HTTP error: %1% (status: %2%)") % error % status); + BOOST_LOG_TRIVIAL(error) << "DownloadManager: " << error_msg + << ", URL: " << task->file_url + << ", file: " << task->file_name + << ", dest: " << task->dest_path; task->state = DownloadTaskState::Error; task->error_message = error; send_error_update(task, error); @@ -166,6 +220,11 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { task->http_object = http.perform(); } catch (std::exception& e) { + std::string error_msg = std::string("Download exception: ") + e.what(); + BOOST_LOG_TRIVIAL(error) << "DownloadManager: " << error_msg + << ", URL: " << task->file_url + << ", file: " << task->file_name + << ", dest: " << task->dest_path; task->state = DownloadTaskState::Error; task->error_message = e.what(); send_error_update(task, e.what()); @@ -197,14 +256,14 @@ bool DownloadManager::cancel_download(size_t task_id) { if (task->is_wcp_download()) { wcp_to_destroy = task->wcp_instance.lock(); } else { - // For internal downloads, call error callback if available - if (task->callbacks.on_error) { - wxGetApp().CallAfter([task]() { - if (task->callbacks.on_error) { - task->callbacks.on_error(task->task_id, "Download canceled"); - } - }); - } + // For internal downloads, don't call error callback if task is being canceled + // during destruction (e.g., when dialog is closing). The callback may reference + // a destroyed dialog object, causing a crash. + // Note: We clear the callbacks before cleanup to prevent any delayed callbacks + // from accessing destroyed objects. + task->callbacks.on_error = nullptr; + task->callbacks.on_progress = nullptr; + task->callbacks.on_complete = nullptr; } cleanup_task(task_id); diff --git a/src/slic3r/GUI/DownloadManager.hpp b/src/slic3r/GUI/DownloadManager.hpp index c61ad574e6..22e993ae0f 100644 --- a/src/slic3r/GUI/DownloadManager.hpp +++ b/src/slic3r/GUI/DownloadManager.hpp @@ -198,6 +198,10 @@ private: // Clean up completed task void cleanup_task(size_t task_id); + + // Generate unique file path if file already exists + // Returns path like "file(1).zip", "file(2).zip" etc. + static std::string get_unique_file_path(const boost::filesystem::path& file_path); }; }} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/GenericDownloadDialog.cpp b/src/slic3r/GUI/GenericDownloadDialog.cpp new file mode 100644 index 0000000000..4607027942 --- /dev/null +++ b/src/slic3r/GUI/GenericDownloadDialog.cpp @@ -0,0 +1,423 @@ +#include "GenericDownloadDialog.hpp" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include +#include + +#include "libslic3r/libslic3r.h" +#include "libslic3r/Utils.hpp" +#include "GUI.hpp" +#include "I18N.hpp" +#include "wxExtensions.hpp" +#include "slic3r/GUI/MainFrame.hpp" +#include "GUI_App.hpp" +#include "slic3r/GUI/DownloadManager.hpp" + +namespace Slic3r { +namespace GUI { + +GenericDownloadDialog::GenericDownloadDialog(wxString title, + const std::string& file_url, + const std::string& file_name, + const std::string& dest_path, + wxWindow* parent) + : DPIDialog(parent ? parent : static_cast(wxGetApp().mainframe), + wxID_ANY, title, wxDefaultPosition, wxDefaultSize, wxCAPTION | wxCLOSE_BOX) + , m_title(title) + , m_file_url(file_url) + , m_file_name(file_name) + , m_dest_path(dest_path) +{ + std::string icon_path = (boost::format("%1%/images/Snapmaker_OrcaTitle.ico") % resources_dir()).str(); + SetIcon(wxIcon(encode_path(icon_path.c_str()), wxBITMAP_TYPE_ICO)); + + SetBackgroundColour(*wxWHITE); + setup_ui(); + + Bind(wxEVT_CLOSE_WINDOW, &GenericDownloadDialog::on_close, this); + wxGetApp().UpdateDlgDarkUI(this); + +} + +GenericDownloadDialog::~GenericDownloadDialog() +{ + m_is_destroying = true; + m_task_id = 0; +} + +void GenericDownloadDialog::setup_ui() +{ + wxBoxSizer *m_sizer_main = new wxBoxSizer(wxVERTICAL); + auto m_line_top = new wxPanel(this, wxID_ANY, wxDefaultPosition, wxSize(-1, 1)); + m_line_top->SetBackgroundColour(wxColour(166, 169, 170)); + m_sizer_main->Add(m_line_top, 0, wxEXPAND, 0); + + m_simplebook_status = new wxSimplebook(this); + m_simplebook_status->SetSize(wxSize(FromDIP(420), FromDIP(100))); + m_simplebook_status->SetMinSize(wxSize(FromDIP(420), FromDIP(100))); + m_simplebook_status->SetMaxSize(wxSize(FromDIP(420), FromDIP(250))); + + // Progress page + m_status_bar = std::make_shared(m_simplebook_status); + m_panel_download = m_status_bar->get_panel(); + m_panel_download->SetSize(wxSize(FromDIP(400), FromDIP(70))); + m_panel_download->SetMinSize(wxSize(FromDIP(400), FromDIP(70))); + m_panel_download->SetMaxSize(wxSize(FromDIP(400), FromDIP(70))); + + // Complete page + m_panel_complete = new wxPanel(m_simplebook_status, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL); + wxBoxSizer* sizer_complete = new wxBoxSizer(wxVERTICAL); + + m_complete_text = new wxStaticText(m_panel_complete, wxID_ANY, _L("Download completed successfully!"), + wxDefaultPosition, wxDefaultSize, 0); + m_complete_text->SetForegroundColour(*wxBLACK); + m_complete_text->Wrap(FromDIP(360)); + sizer_complete->Add(m_complete_text, 0, wxALIGN_CENTER | wxALL, 5); + + StateColor btn_close_bg(std::pair(wxColour(0x90, 0x90, 0x90), StateColor::Disabled), + std::pair(wxColour(206, 206, 206), StateColor::Pressed), + std::pair(wxColour(238, 238, 238), StateColor::Hovered), + std::pair(wxColour(231, 231, 231), StateColor::Normal)); + + StateColor btn_close_bd(std::pair(wxColour(255, 255, 254), StateColor::Disabled), + std::pair(wxColour(38, 46, 48), StateColor::Enabled)); + + StateColor btn_close_txt(std::pair(wxColour("#FFFFFE"), StateColor::Disabled), + std::pair(wxColour(36, 36, 36), StateColor::Normal)); + + m_close_button = new Button(m_panel_complete, _L("Close")); + m_close_button->SetSize(wxSize(FromDIP(80), FromDIP(28))); + m_close_button->SetMinSize(wxSize(FromDIP(80), FromDIP(28))); + m_close_button->SetMaxSize(wxSize(FromDIP(80), FromDIP(28))); + + m_close_button->SetBackgroundColour(*wxWHITE); + m_close_button->SetBackgroundColor(btn_close_bg); + m_close_button->SetBorderColor(btn_close_bd); + m_close_button->SetTextColor(btn_close_txt); + m_close_button->SetCornerRadius(FromDIP(12)); + m_close_button->SetCursor(wxCURSOR_HAND); + m_close_button->Bind(wxEVT_BUTTON, &GenericDownloadDialog::on_close_clicked, this); + + sizer_complete->Add(m_close_button, 0, wxALIGN_CENTER | wxALL, 5); + + m_panel_complete->SetSizer(sizer_complete); + m_panel_complete->Layout(); + sizer_complete->Fit(m_panel_complete); + + // Error page + m_panel_error = new wxPanel(m_simplebook_status, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL); + wxBoxSizer* sizer_error = new wxBoxSizer(wxVERTICAL); + + // Simple label for error message display + m_error_text = new wxStaticText(m_panel_error, wxID_ANY, wxEmptyString, + wxDefaultPosition, wxSize(FromDIP(380), -1), + wxALIGN_LEFT | wxST_ELLIPSIZE_END); + m_error_text->SetForegroundColour(*wxBLACK); + m_error_text->Wrap(FromDIP(380)); + sizer_error->Add(m_error_text, 0, wxEXPAND | wxLEFT | wxTOP | wxRIGHT, FromDIP(10)); + + // Button sizer aligned to right + wxBoxSizer* sizer_buttons = new wxBoxSizer(wxHORIZONTAL); + sizer_buttons->AddStretchSpacer(); + + StateColor btn_retry_bg(std::pair(wxColour(255, 255, 255), StateColor::Disabled), + std::pair(wxColour(206, 206, 206), StateColor::Pressed), + std::pair(wxColour(23, 99, 226), StateColor::Hovered), // Same as Normal + std::pair(wxColour(23, 99, 226), StateColor::Normal)); + + // Set border color to same as background to avoid corner color issues + StateColor btn_retry_bd(std::pair(wxColour(255, 255, 255), StateColor::Disabled), + std::pair(wxColour(23, 99, 226), StateColor::Enabled)); // Same as background + + StateColor btn_retry_txt(std::pair(wxColour("#FFFFFE"), StateColor::Disabled), + std::pair(wxColour(255, 255, 255), StateColor::Normal)); + + m_retry_button = new Button(m_panel_error, _L("Retry")); + m_retry_button->SetSize(wxSize(FromDIP(80), FromDIP(28))); + m_retry_button->SetMinSize(wxSize(FromDIP(80), FromDIP(28))); + m_retry_button->SetMaxSize(wxSize(FromDIP(80), FromDIP(28))); + // Set window background color to white to ensure rounded corners are white + m_retry_button->SetBackgroundColour(*wxWHITE); + m_retry_button->SetBackgroundColor(btn_retry_bg); + m_retry_button->SetBorderColor(btn_retry_bg); + m_retry_button->SetTextColor(btn_retry_txt); + m_retry_button->SetCornerRadius(FromDIP(12)); + m_retry_button->SetCursor(wxCURSOR_HAND); + m_retry_button->Bind(wxEVT_BUTTON, &GenericDownloadDialog::on_retry_clicked, this); + + // Setup StateColor for determine button (gray style) + StateColor btn_determine_bg(std::pair(wxColour(255, 255, 255), StateColor::Disabled), + std::pair(wxColour(206, 206, 206), StateColor::Pressed), + std::pair(wxColour(238, 238, 238), StateColor::Hovered), + std::pair(wxColour(231, 231, 231), StateColor::Normal)); + + StateColor btn_determine_bd(std::pair(wxColour(255, 255, 255), StateColor::Disabled), + std::pair(wxColour(38, 46, 48), StateColor::Enabled)); + + StateColor btn_determine_txt(std::pair(wxColour("#FFFFFE"), StateColor::Disabled), + std::pair(wxColour(36, 36, 36), StateColor::Normal)); + + m_error_close_button = new Button(m_panel_error, _L("Determine")); + m_error_close_button->SetSize(wxSize(FromDIP(80), FromDIP(28))); + m_error_close_button->SetMinSize(wxSize(FromDIP(80), FromDIP(28))); + m_error_close_button->SetMaxSize(wxSize(FromDIP(80), FromDIP(28))); + // Set window background color to white to ensure rounded corners are white + m_error_close_button->SetBackgroundColour(*wxWHITE); + m_error_close_button->SetBackgroundColor(btn_determine_bg); + m_error_close_button->SetBorderColor(btn_determine_bg); + m_error_close_button->SetTextColor(btn_determine_txt); + m_error_close_button->SetCornerRadius(FromDIP(12)); + m_error_close_button->SetCursor(wxCURSOR_HAND); + m_error_close_button->Bind(wxEVT_BUTTON, &GenericDownloadDialog::on_close_clicked, this); + sizer_buttons->Add(m_error_close_button, 0, 0); + sizer_buttons->AddSpacer(FromDIP(6)); + sizer_buttons->Add(m_retry_button, 0, 0); + + sizer_error->AddSpacer(FromDIP(40)); + sizer_error->Add(sizer_buttons, 0, wxALIGN_RIGHT | wxRIGHT, FromDIP(24)); + + + m_panel_error->SetSizer(sizer_error); + m_panel_error->Layout(); + sizer_error->Fit(m_panel_error); + + + m_sizer_main->Add(m_simplebook_status, 0, wxALL, FromDIP(16)); + + m_simplebook_status->AddPage(m_panel_download, wxEmptyString, true); + m_simplebook_status->AddPage(m_panel_complete, wxEmptyString, false); + m_simplebook_status->AddPage(m_panel_error, wxEmptyString, false); + + SetSizer(m_sizer_main); + Layout(); + Fit(); + CentreOnParent(); +} + +wxString GenericDownloadDialog::format_text(wxStaticText* st, wxString str, int warp) +{ + if (wxGetApp().app_config->get("language") != "zh_CN") { + return str; + } + + wxString out_txt = str; + wxString count_txt = ""; + + for (int i = 0; i < str.length(); i++) { + auto text_size = st->GetTextExtent(count_txt); + if (text_size.x < warp) { + count_txt += str[i]; + } else { + out_txt.insert(i - 1, '\n'); + count_txt = ""; + } + } + return out_txt; +} + +void GenericDownloadDialog::start_download() +{ + show_progress_page(); + m_status_bar->set_progress(0); + m_status_bar->set_status_text(_L("Preparing download...")); + m_status_bar->change_button_label(_L("Cancel")); + m_status_bar->set_cancel_callback_fina([this]() { + if (m_task_id > 0) { + DownloadManager::getInstance().cancel_download(m_task_id); + m_task_id = 0; // Mark as canceled to prevent duplicate cancellation + } + }); + + // Create download callbacks + DownloadCallbacks callbacks; + callbacks.on_progress = [this](size_t task_id, int percent, size_t downloaded, size_t total) { + on_download_progress(task_id, percent, downloaded, total); + }; + callbacks.on_complete = [this](size_t task_id, const std::string& file_path) { + on_download_complete(task_id, file_path); + }; + callbacks.on_error = [this](size_t task_id, const std::string& error) { + on_download_error(task_id, error); + }; + + // Start download + if (m_dest_path.empty()) { + m_task_id = DownloadManager::getInstance().start_internal_download( + m_file_url, m_file_name, std::move(callbacks)); + } else { + m_task_id = DownloadManager::getInstance().start_internal_download( + m_file_url, m_file_name, m_dest_path, std::move(callbacks)); + } +} + +int GenericDownloadDialog::ShowModal() +{ + start_download(); + return DPIDialog::ShowModal(); +} + +void GenericDownloadDialog::on_download_progress(size_t task_id, int percent, size_t downloaded, size_t total) +{ + wxGetApp().CallAfter([this, percent, downloaded, total]() { + // Check if dialog is being destroyed + if (m_is_destroying || IsBeingDeleted()) { + return; + } + + update_progress(percent); + + // Format status text + wxString status_text; + if (total > 0) { + double downloaded_mb = downloaded / (1024.0 * 1024.0); + double total_mb = total / (1024.0 * 1024.0); + status_text = wxString::Format(_L("Downloading: %.1f MB / %.1f MB (%d%%)"), + downloaded_mb, total_mb, percent); + } else { + double downloaded_mb = downloaded / (1024.0 * 1024.0); + status_text = wxString::Format(_L("Downloading: %.1f MB..."), downloaded_mb); + } + m_status_bar->set_status_text(status_text); + + // Call user callback if set + if (m_on_progress) { + m_on_progress(m_task_id, percent, downloaded, total); + } + }); +} + +void GenericDownloadDialog::on_download_complete(size_t task_id, const std::string& file_path) +{ + wxGetApp().CallAfter([this, file_path]() { + // Check if dialog is being destroyed + if (m_is_destroying || IsBeingDeleted()) { + return; + } + + m_download_success = true; + m_file_path = file_path; + show_complete_page(); + + // Mark task as completed - no need to cancel it + m_task_id = 0; + + // Call user callback if set + if (m_on_complete) { + m_on_complete(m_task_id, file_path); + } + }); +} + +void GenericDownloadDialog::on_download_error(size_t task_id, const std::string& error) +{ + // Log detailed error information for debugging + BOOST_LOG_TRIVIAL(error) << boost::format("GenericDownloadDialog: Download failed for file '%1%' from URL '%2%'. Error: %3%") + % m_file_name % m_file_url % error; + + wxGetApp().CallAfter([this, error]() { + // Check if dialog is being destroyed + if (m_is_destroying || IsBeingDeleted()) { + return; + } + + m_download_success = false; + m_error_message = error; + show_error_page(error); + + // Mark task as completed (failed) - no need to cancel it + m_task_id = 0; + + // Call user callback if set + if (m_on_error) { + m_on_error(m_task_id, error); + } + }); +} + +void GenericDownloadDialog::on_retry_clicked(wxCommandEvent& event) +{ + if (m_on_retry) { + m_on_retry(); + } + start_download(); + event.Skip(); +} + +void GenericDownloadDialog::on_close_clicked(wxCommandEvent& event) +{ + if (m_task_id > 0) { + DownloadManager::getInstance().cancel_download(m_task_id); + m_task_id = 0; + } + EndModal(m_download_success ? wxID_OK : wxID_CANCEL); + event.Skip(); +} + +void GenericDownloadDialog::on_close(wxCloseEvent& event) +{ + if (m_task_id > 0) { + DownloadManager::getInstance().cancel_download(m_task_id); + m_task_id = 0; + } + event.Skip(); +} + +void GenericDownloadDialog::show_progress_page() +{ + m_simplebook_status->SetSelection(0); + m_status_bar->set_progress(0); + m_status_bar->show_cancel_button(); +} + +void GenericDownloadDialog::show_complete_page() +{ + m_simplebook_status->SetSelection(1); + m_status_bar->hide_cancel_button(); +} + +void GenericDownloadDialog::show_error_page(const std::string& error_msg) +{ + m_simplebook_status->SetSelection(2); + + // Display simple error message: filename + "Download failed" + wxString filename = wxString::FromUTF8(m_file_name.c_str()); + wxString error_text = filename + " - Download failed"; + + // Set error text in simple label + m_error_text->SetLabel(error_text); + m_error_text->Wrap(FromDIP(380)); + + m_panel_error->Layout(); + m_simplebook_status->Layout(); + Layout(); + Fit(); + m_status_bar->hide_cancel_button(); +} + +void GenericDownloadDialog::update_progress(int percent, const wxString& status_text) +{ + m_status_bar->set_progress(percent); + if (!status_text.IsEmpty()) { + m_status_bar->set_status_text(status_text); + } +} + +void GenericDownloadDialog::on_dpi_changed(const wxRect &suggested_rect) +{ + // Handle DPI changes if needed +} + +}} // namespace Slic3r::GUI + diff --git a/src/slic3r/GUI/GenericDownloadDialog.hpp b/src/slic3r/GUI/GenericDownloadDialog.hpp new file mode 100644 index 0000000000..8ff71587c1 --- /dev/null +++ b/src/slic3r/GUI/GenericDownloadDialog.hpp @@ -0,0 +1,113 @@ +#ifndef slic3r_GenericDownloadDialog_hpp_ +#define slic3r_GenericDownloadDialog_hpp_ + +#include +#include +#include +#include + +#include "GUI_Utils.hpp" +#include +#include +#include "BBLStatusBar.hpp" +#include "BBLStatusBarSend.hpp" +#include "Jobs/Worker.hpp" +#include "slic3r/GUI/DownloadManager.hpp" +#include "Widgets/Button.hpp" + +class wxBoxSizer; +class wxPanel; +class wxStaticText; +class wxHyperlinkCtrl; + +namespace Slic3r { +namespace GUI { + +// Generic download dialog for custom download tasks with progress display +class GenericDownloadDialog : public DPIDialog +{ +public: + // Callback types + using DownloadCallback = std::function; + using CompleteCallback = std::function; + using ErrorCallback = std::function; + using RetryCallback = std::function; + + GenericDownloadDialog(wxString title, + const std::string& file_url, + const std::string& file_name, + const std::string& dest_path = "", + wxWindow* parent = nullptr); + ~GenericDownloadDialog(); + + // Start download + void start_download(); + + // Set callbacks (optional) + void set_on_progress(DownloadCallback callback) { m_on_progress = callback; } + void set_on_complete(CompleteCallback callback) { m_on_complete = callback; } + void set_on_error(ErrorCallback callback) { m_on_error = callback; } + void set_on_retry(RetryCallback callback) { m_on_retry = callback; } + + // Get download result + bool is_success() const { return m_download_success; } + std::string get_file_path() const { return m_file_path; } + std::string get_error_message() const { return m_error_message; } + + // Show modal and return result + int ShowModal() override; + +protected: + void on_close(wxCloseEvent& event); + void on_dpi_changed(const wxRect &suggested_rect) override; + wxString format_text(wxStaticText* st, wxString str, int warp); + + // Event handlers + void on_download_progress(size_t task_id, int percent, size_t downloaded, size_t total); + void on_download_complete(size_t task_id, const std::string& file_path); + void on_download_error(size_t task_id, const std::string& error); + void on_retry_clicked(wxCommandEvent& event); + void on_close_clicked(wxCommandEvent& event); + +private: + void setup_ui(); + void show_progress_page(); + void show_complete_page(); + void show_error_page(const std::string& error_msg); + void update_progress(int percent, const wxString& status_text = ""); + + wxString m_title; + std::string m_file_url; + std::string m_file_name; + std::string m_dest_path; + size_t m_task_id{0}; + bool m_download_success{false}; + std::string m_file_path; + std::string m_error_message; + + // Callbacks + DownloadCallback m_on_progress; + CompleteCallback m_on_complete; + ErrorCallback m_on_error; + RetryCallback m_on_retry; + + // UI components + wxSimplebook* m_simplebook_status{nullptr}; + std::shared_ptr m_status_bar; + wxPanel* m_panel_download{nullptr}; + wxPanel* m_panel_complete{nullptr}; + wxPanel* m_panel_error{nullptr}; + + wxStaticText* m_complete_text{nullptr}; + wxStaticText* m_error_text{nullptr}; + Button* m_retry_button{nullptr}; + Button* m_close_button{nullptr}; + Button* m_error_close_button{nullptr}; + + std::atomic m_is_destroying{false}; +}; + +}} // namespace Slic3r::GUI + +#endif // slic3r_GenericDownloadDialog_hpp_ + From 779e2716a8c005b51e2c69eb1385314497f1a0f2 Mon Sep 17 00:00:00 2001 From: alves Date: Wed, 4 Feb 2026 15:43:19 +0800 Subject: [PATCH 5/8] feature add download files and rename file(1),and check the dir and the file Complete path is valid. --- src/slic3r/GUI/DownloadManager.cpp | 86 ++++++++++++++++++++++++++---- src/slic3r/GUI/MainFrame.cpp | 13 ++++- 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/src/slic3r/GUI/DownloadManager.cpp b/src/slic3r/GUI/DownloadManager.cpp index 72faffd178..568776afa8 100644 --- a/src/slic3r/GUI/DownloadManager.cpp +++ b/src/slic3r/GUI/DownloadManager.cpp @@ -5,6 +5,7 @@ #include #include #include +#include namespace Slic3r { namespace GUI { @@ -14,24 +15,74 @@ namespace Slic3r { namespace GUI { std::string DownloadManager::get_unique_file_path(const boost::filesystem::path& file_path) { + // file_path should be the complete absolute path: directory + filename + std::string original_path = file_path.string(); + BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::get_unique_file_path: Checking path '%1%'") % original_path; + + // Check if file exists, if not return original path if (!boost::filesystem::exists(file_path)) { - return file_path.string(); + BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::get_unique_file_path: File does not exist, returning original path '%1%'") % original_path; + return original_path; } + BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::get_unique_file_path: File exists, generating unique name"); + boost::filesystem::path parent_dir = file_path.parent_path(); std::string filename = file_path.filename().string(); - std::string extension = file_path.extension().string(); - std::string name_without_ext = filename.substr(0, filename.size() - extension.size()); + // Properly extract extension (includes the dot, e.g., ".txt") + // For "file.txt", extension() returns ".txt" + // For "file", extension() returns "" + std::string extension = file_path.extension().string(); + + // Extract name without extension + // If extension is empty, name_without_ext is the full filename + std::string name_without_ext; + if (extension.empty()) { + name_without_ext = filename; + BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::get_unique_file_path: No extension found, filename='%1%'") % filename; + } else { + // Remove extension from filename (extension includes the dot) + // For "file.txt", filename="file.txt", extension=".txt", so name_without_ext="file" + name_without_ext = filename.substr(0, filename.size() - extension.size()); + BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::get_unique_file_path: filename='%1%', extension='%2%', name_without_ext='%3%'") + % filename % extension % name_without_ext; + } + + // Generate unique filename with Windows-style numbering: filename(1).ext, filename(2).ext, etc. size_t version = 1; boost::filesystem::path unique_path; do { - std::string new_filename = name_without_ext + "(" + std::to_string(version) + ")" + extension; + std::string new_filename; + if (extension.empty()) { + // No extension: filename(1), filename(2), etc. + new_filename = name_without_ext + "(" + std::to_string(version) + ")"; + } else { + // Has extension: filename(1).ext, filename(2).ext, etc. + new_filename = name_without_ext + "(" + std::to_string(version) + ")" + extension; + } unique_path = parent_dir / new_filename; + BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::get_unique_file_path: Trying version %1%: '%2%'") % version % unique_path.string(); version++; } while (boost::filesystem::exists(unique_path) && version < 10000); // Safety limit - return unique_path.string(); + if (version >= 10000) { + // If we hit the limit, log a warning and return a timestamp-based name + BOOST_LOG_TRIVIAL(warning) << boost::format("DownloadManager::get_unique_file_path: Too many duplicate files for '%1%', using timestamp-based name") + % original_path; + std::string timestamp = std::to_string(std::time(nullptr)); + std::string new_filename; + if (extension.empty()) { + new_filename = name_without_ext + "_" + timestamp; + } else { + new_filename = name_without_ext + "_" + timestamp + extension; + } + unique_path = parent_dir / new_filename; + } + + std::string result = unique_path.string(); + BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::get_unique_file_path: Final unique path: '%1%'") % result; + return result; } // ============================================================================ @@ -74,17 +125,34 @@ size_t DownloadManager::start_wcp_download(const std::string& file_url, // Internal Download Interface (for PC internal use) // ============================================================================ size_t DownloadManager::start_internal_download(const std::string& file_url, - const std::string& file_name, - const std::string& dest_path, - DownloadCallbacks callbacks) { + const std::string& file_name, + const std::string& dest_path, + DownloadCallbacks callbacks) { std::lock_guard lock(m_tasks_mutex); size_t task_id = m_next_task_id++; - boost::filesystem::path dest_file_path(dest_path); + boost::filesystem::path dest_path_obj(dest_path); + + // Check if dest_path is a directory or a complete file path + boost::filesystem::path dest_file_path; + if (boost::filesystem::is_directory(dest_path_obj) || dest_path_obj.filename().empty()) { + // dest_path is a directory, need to append file_name + BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::start_internal_download: dest_path '%1%' is a directory, appending file_name '%2%'") + % dest_path % file_name; + dest_file_path = dest_path_obj / file_name; + } else { + // dest_path is already a complete file path (directory + filename) + BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::start_internal_download: dest_path '%1%' is a complete file path") + % dest_path; + dest_file_path = dest_path_obj; + } + + // Create parent directory if it doesn't exist boost::filesystem::create_directories(dest_file_path.parent_path()); // Generate unique file path if file already exists + // dest_file_path should now be the complete absolute path: directory + filename std::string unique_dest_path = get_unique_file_path(dest_file_path); auto task = std::make_shared(task_id, diff --git a/src/slic3r/GUI/MainFrame.cpp b/src/slic3r/GUI/MainFrame.cpp index a6d4c42bb8..ca732fb3a6 100644 --- a/src/slic3r/GUI/MainFrame.cpp +++ b/src/slic3r/GUI/MainFrame.cpp @@ -75,7 +75,7 @@ #endif // _WIN32 #include #include "sentry_wrapper/SentryWrapper.hpp" - +#include "GenericDownloadDialog.hpp" #define UPDATE_BUSER true #define UPDATE_BUAUTO false @@ -2249,8 +2249,17 @@ static wxMenu* generate_help_menu() // //TODO // }); // Check New Version - append_menu_item(helpMenu, wxID_ANY, _L("Check for Update"), _L("Check for Update"), + append_menu_item( + helpMenu, wxID_ANY, _L("Check for Update"), _L("Check for Update"), [](wxCommandEvent&) { + std::string fileUrl = "https://public.resource.snapmaker.com/model/public/3mf/test_for_download.3mf"; + //std::string fileUrl = "https://github.com/Snapmaker/OrcaSlicer/releases/download/v2.2.1/Snapmaker_Orca_Windows_Installer_V2.2.1.exe"; + std::string filename = "test_for_download.3mf"; + std::string downloadPath = "C:/tmp"; + + GenericDownloadDialog dlg(_L("importing the model"), fileUrl, filename, downloadPath); + dlg.ShowModal(); + return; wxGetApp().check_new_version_sf(true, UPDATE_BUSER); }, "", nullptr, []() { return true; From 1f206dc8892a1b9bfef289089c02915c6a093a7f Mon Sep 17 00:00:00 2001 From: alves Date: Wed, 4 Feb 2026 16:35:07 +0800 Subject: [PATCH 6/8] feature add download file for wcp logic. --- src/slic3r/GUI/DownloadManager.cpp | 4 --- src/slic3r/GUI/GenericDownloadDialog.cpp | 5 +-- src/slic3r/GUI/MainFrame.cpp | 40 +++++++++++++++++++----- src/slic3r/GUI/MainFrame.hpp | 6 +++- src/slic3r/GUI/SSWCP.cpp | 17 ++-------- 5 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/slic3r/GUI/DownloadManager.cpp b/src/slic3r/GUI/DownloadManager.cpp index 568776afa8..3e203a960d 100644 --- a/src/slic3r/GUI/DownloadManager.cpp +++ b/src/slic3r/GUI/DownloadManager.cpp @@ -134,7 +134,6 @@ size_t DownloadManager::start_internal_download(const std::string& file_url, boost::filesystem::path dest_path_obj(dest_path); - // Check if dest_path is a directory or a complete file path boost::filesystem::path dest_file_path; if (boost::filesystem::is_directory(dest_path_obj) || dest_path_obj.filename().empty()) { // dest_path is a directory, need to append file_name @@ -148,11 +147,8 @@ size_t DownloadManager::start_internal_download(const std::string& file_url, dest_file_path = dest_path_obj; } - // Create parent directory if it doesn't exist boost::filesystem::create_directories(dest_file_path.parent_path()); - // Generate unique file path if file already exists - // dest_file_path should now be the complete absolute path: directory + filename std::string unique_dest_path = get_unique_file_path(dest_file_path); auto task = std::make_shared(task_id, diff --git a/src/slic3r/GUI/GenericDownloadDialog.cpp b/src/slic3r/GUI/GenericDownloadDialog.cpp index 4607027942..cc98f5b451 100644 --- a/src/slic3r/GUI/GenericDownloadDialog.cpp +++ b/src/slic3r/GUI/GenericDownloadDialog.cpp @@ -383,8 +383,9 @@ void GenericDownloadDialog::show_progress_page() void GenericDownloadDialog::show_complete_page() { - m_simplebook_status->SetSelection(1); - m_status_bar->hide_cancel_button(); + //m_simplebook_status->SetSelection(1); + //m_status_bar->hide_cancel_button(); + EndModal(wxID_OK); } void GenericDownloadDialog::show_error_page(const std::string& error_msg) diff --git a/src/slic3r/GUI/MainFrame.cpp b/src/slic3r/GUI/MainFrame.cpp index ca732fb3a6..a2771f4b5c 100644 --- a/src/slic3r/GUI/MainFrame.cpp +++ b/src/slic3r/GUI/MainFrame.cpp @@ -2252,14 +2252,6 @@ static wxMenu* generate_help_menu() append_menu_item( helpMenu, wxID_ANY, _L("Check for Update"), _L("Check for Update"), [](wxCommandEvent&) { - std::string fileUrl = "https://public.resource.snapmaker.com/model/public/3mf/test_for_download.3mf"; - //std::string fileUrl = "https://github.com/Snapmaker/OrcaSlicer/releases/download/v2.2.1/Snapmaker_Orca_Windows_Installer_V2.2.1.exe"; - std::string filename = "test_for_download.3mf"; - std::string downloadPath = "C:/tmp"; - - GenericDownloadDialog dlg(_L("importing the model"), fileUrl, filename, downloadPath); - dlg.ShowModal(); - return; wxGetApp().check_new_version_sf(true, UPDATE_BUSER); }, "", nullptr, []() { return true; @@ -4015,6 +4007,38 @@ void MainFrame::RunScript(wxString js) m_webview->RunScript(js); } +void MainFrame::downloadOpenProject(const std::string& fileUrl, const std::string& fileName, std::string completeFilePath) +{ + // std::string fileUrl = "https://public.resource.snapmaker.com/model/public/3mf/test_for_download.3mf"; + // std::string filename = "test_for_download.3mf"; + + GenericDownloadDialog dlg(_L("downloading the model"), fileUrl, fileName, completeFilePath); + dlg.ShowModal(); + + if (completeFilePath.empty()) { + auto downloadPath = wxGetApp().app_config->get("download_path"); + completeFilePath = downloadPath + "/" + fileName; + } + if (!boost::filesystem::exists(completeFilePath)) + { + BOOST_LOG_TRIVIAL(warning) << boost::format("the file '%1%' not exists") % completeFilePath; + return; + } + + // Auto-open project if it's a .3mf file + boost::filesystem::path path(completeFilePath); + std::string extension = boost::algorithm::to_lower_copy(path.extension().string()); + if (extension == ".3mf") { + BOOST_LOG_TRIVIAL(info) << boost::format("GenericDownloadDialog: Auto-opening project file '%1%'") % completeFilePath; + wxString wx_file_path = wxString::FromUTF8(completeFilePath.c_str()); + if (wxGetApp().can_load_project() && wxGetApp().mainframe && wxGetApp().mainframe->plater()) { + wxGetApp().mainframe->plater()->load_project(wx_file_path); + } + } + + +} + void MainFrame::technology_changed() { // update menu titles diff --git a/src/slic3r/GUI/MainFrame.hpp b/src/slic3r/GUI/MainFrame.hpp index 095f1f7ea1..661fe3ca4b 100644 --- a/src/slic3r/GUI/MainFrame.hpp +++ b/src/slic3r/GUI/MainFrame.hpp @@ -348,7 +348,11 @@ public: void load_printer_url(); bool is_printer_view() const; void refresh_plugin_tips(); - void RunScript(wxString js); + void RunScript(wxString js); + + void downloadOpenProject(const std::string& fileUrl, + const std::string& fileName, + std::string completeFilePath = ""); //SoftFever void show_device(bool bBBLPrinter); diff --git a/src/slic3r/GUI/SSWCP.cpp b/src/slic3r/GUI/SSWCP.cpp index e8aac1accb..3e7f33c33e 100644 --- a/src/slic3r/GUI/SSWCP.cpp +++ b/src/slic3r/GUI/SSWCP.cpp @@ -3187,14 +3187,10 @@ void SSWCP_MachineOption_Instance::sw_GetFileFilamentMapping() response["thumbnails"] = thumbnails; - - // file name response["filename"] = SSWCP::get_display_filename(); response["filepath"] = SSWCP::get_active_filename(); - - m_res_data = response; send_to_js(); finish_job(); @@ -4404,18 +4400,9 @@ void SSWCP_UserLogin_Instance::sw_DownloadFile() handle_general_fail(-1, "Download Manager not available"); return; } + + wxGetApp().mainframe->downloadOpenProject(fileUrl, fileName, ""); - size_t task_id = download_mgr->start_wcp_download(fileUrl, - fileName, - shared_from_this(), - false); // use_original_event_id = false (sw_DownloadFile: finish immediately) - - // Return task ID to Flutter - json response; - response["task_id"] = task_id; - response["file_name"] = fileName; - response["file_url"] = fileUrl; - m_res_data = response; m_status = 0; m_msg = "Download started"; send_to_js(); From dfb83e4a140c5436e3433a8086a4087b1e747785 Mon Sep 17 00:00:00 2001 From: alves Date: Wed, 4 Feb 2026 17:21:13 +0800 Subject: [PATCH 7/8] fix lock bug. --- src/slic3r/GUI/DownloadManager.cpp | 72 ++++++++++++++++++++++-- src/slic3r/GUI/GenericDownloadDialog.cpp | 11 +++- src/slic3r/GUI/MainFrame.cpp | 5 +- 3 files changed, 79 insertions(+), 9 deletions(-) diff --git a/src/slic3r/GUI/DownloadManager.cpp b/src/slic3r/GUI/DownloadManager.cpp index 3e203a960d..446b4b8a32 100644 --- a/src/slic3r/GUI/DownloadManager.cpp +++ b/src/slic3r/GUI/DownloadManager.cpp @@ -192,6 +192,16 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { // Step 2: Set progress callback http.on_progress([this, task](Http::Progress progress, bool& cancel) { + // Check if task is canceled or already cleaned up + { + std::lock_guard lock(m_tasks_mutex); + if (m_tasks.find(task->task_id) == m_tasks.end()) { + // Task has been cleaned up, cancel the download + cancel = true; + return; + } + } + if (task->state == DownloadTaskState::Canceled) { cancel = true; return; @@ -206,6 +216,13 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { // Throttle progress updates: update every 5% or every second std::lock_guard lock(m_tasks_mutex); + + // Double-check task still exists after acquiring lock + if (m_tasks.find(task->task_id) == m_tasks.end()) { + cancel = true; + return; + } + auto& last_pct = m_last_percent[task->task_id]; auto& last_upd = m_last_update[task->task_id]; @@ -222,7 +239,12 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { if (should_update) { last_upd = now; wxGetApp().CallAfter([this, task, percent, progress]() { - send_progress_update(task, percent, progress.dlnow, progress.dltotal); + // Check if task still exists before sending update + std::lock_guard lock(m_tasks_mutex); + if (m_tasks.find(task->task_id) != m_tasks.end() && + task->state != DownloadTaskState::Canceled) { + send_progress_update(task, percent, progress.dlnow, progress.dltotal); + } }); } }); @@ -230,6 +252,17 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { // Step 3: Set complete callback http.on_complete([this, task](std::string body, unsigned status) { wxGetApp().CallAfter([this, task, body]() { + // Check if task still exists and is not canceled + { + std::lock_guard lock(m_tasks_mutex); + if (m_tasks.find(task->task_id) == m_tasks.end() || + task->state == DownloadTaskState::Canceled) { + // Task has been canceled or cleaned up, ignore completion + BOOST_LOG_TRIVIAL(debug) << "DownloadManager: Ignoring complete callback for canceled/cleaned task " << task->task_id; + return; + } + } + try { // Save file boost::nowide::ofstream file(task->dest_path, std::ios::binary); @@ -268,6 +301,21 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { // Step 4: Set error callback http.on_error([this, task](std::string body, std::string error, unsigned status) { wxGetApp().CallAfter([this, task, error, status]() { + // Check if task still exists and is not canceled + { + std::lock_guard lock(m_tasks_mutex); + if (m_tasks.find(task->task_id) == m_tasks.end()) { + // Task has been cleaned up, ignore error callback + BOOST_LOG_TRIVIAL(debug) << "DownloadManager: Ignoring error callback for cleaned task " << task->task_id; + return; + } + if (task->state == DownloadTaskState::Canceled) { + // Task was canceled, ignore error callback (cancel already handled cleanup) + BOOST_LOG_TRIVIAL(debug) << "DownloadManager: Ignoring error callback for canceled task " << task->task_id; + return; + } + } + std::string error_msg = boost::str(boost::format("HTTP error: %1% (status: %2%)") % error % status); BOOST_LOG_TRIVIAL(error) << "DownloadManager: " << error_msg << ", URL: " << task->file_url @@ -330,7 +378,10 @@ bool DownloadManager::cancel_download(size_t task_id) { task->callbacks.on_complete = nullptr; } - cleanup_task(task_id); + // Cleanup task directly (already holding the lock, don't call cleanup_task) + m_tasks.erase(task_id); + m_last_percent.erase(task_id); + m_last_update.erase(task_id); } else { return false; } @@ -446,8 +497,11 @@ void DownloadManager::send_wcp_progress_update(std::shared_ptr tas void DownloadManager::call_internal_progress_callback(std::shared_ptr task, int percent, size_t downloaded, - size_t total) { - if (task->callbacks.on_progress) { + size_t total) { + // Only check if callback is still valid (cleared during cancellation) + // Don't check m_tasks because this is called from CallAfter which may execute + // after cleanup_task, but the callback should still be valid if not canceled + if (task->callbacks.on_progress && task->state != DownloadTaskState::Canceled) { task->callbacks.on_progress(task->task_id, percent, downloaded, total); } } @@ -487,7 +541,10 @@ void DownloadManager::send_wcp_complete_update(std::shared_ptr tas void DownloadManager::call_internal_complete_callback(std::shared_ptr task, const std::string& file_path) { - if (task->callbacks.on_complete) { + // Only check if callback is still valid (cleared during cancellation) + // Don't check m_tasks because cleanup_task may have been called, but the callback + // should still be valid if not canceled + if (task->callbacks.on_complete && task->state != DownloadTaskState::Canceled) { task->callbacks.on_complete(task->task_id, file_path); } } @@ -525,7 +582,10 @@ void DownloadManager::send_wcp_error_update(std::shared_ptr task, void DownloadManager::call_internal_error_callback(std::shared_ptr task, const std::string& error) { - if (task->callbacks.on_error) { + // Only check if callback is still valid (cleared during cancellation) + // Don't check m_tasks because cleanup_task may have been called, but the callback + // should still be valid if not canceled + if (task->callbacks.on_error && task->state != DownloadTaskState::Canceled) { task->callbacks.on_error(task->task_id, error); } } diff --git a/src/slic3r/GUI/GenericDownloadDialog.cpp b/src/slic3r/GUI/GenericDownloadDialog.cpp index cc98f5b451..301e9c0fba 100644 --- a/src/slic3r/GUI/GenericDownloadDialog.cpp +++ b/src/slic3r/GUI/GenericDownloadDialog.cpp @@ -53,8 +53,14 @@ GenericDownloadDialog::GenericDownloadDialog(wxString title, GenericDownloadDialog::~GenericDownloadDialog() { + // Set destroying flag first to prevent any callbacks from accessing this object m_is_destroying = true; - m_task_id = 0; + + // Cancel any active download before destruction + if (m_task_id > 0) { + DownloadManager::getInstance().cancel_download(m_task_id); + m_task_id = 0; + } } void GenericDownloadDialog::setup_ui() @@ -236,8 +242,9 @@ void GenericDownloadDialog::start_download() m_status_bar->set_cancel_callback_fina([this]() { if (m_task_id > 0) { DownloadManager::getInstance().cancel_download(m_task_id); - m_task_id = 0; // Mark as canceled to prevent duplicate cancellation + m_task_id = 0; } + EndModal(wxID_CANCEL); }); // Create download callbacks diff --git a/src/slic3r/GUI/MainFrame.cpp b/src/slic3r/GUI/MainFrame.cpp index a2771f4b5c..524f68fadb 100644 --- a/src/slic3r/GUI/MainFrame.cpp +++ b/src/slic3r/GUI/MainFrame.cpp @@ -4013,7 +4013,10 @@ void MainFrame::downloadOpenProject(const std::string& fileUrl, const std::strin // std::string filename = "test_for_download.3mf"; GenericDownloadDialog dlg(_L("downloading the model"), fileUrl, fileName, completeFilePath); - dlg.ShowModal(); + auto res = dlg.ShowModal(); + + if (res != wxID_OK) + return; if (completeFilePath.empty()) { auto downloadPath = wxGetApp().app_config->get("download_path"); From a609bc662626fd3c778a2a7a6ba057199f0ca56a Mon Sep 17 00:00:00 2001 From: alves Date: Wed, 4 Feb 2026 17:32:09 +0800 Subject: [PATCH 8/8] fix lock bug, and remove not work code. --- src/slic3r/GUI/DownloadManager.cpp | 59 +++++++----------------------- 1 file changed, 14 insertions(+), 45 deletions(-) diff --git a/src/slic3r/GUI/DownloadManager.cpp b/src/slic3r/GUI/DownloadManager.cpp index 446b4b8a32..41fd1f5fb3 100644 --- a/src/slic3r/GUI/DownloadManager.cpp +++ b/src/slic3r/GUI/DownloadManager.cpp @@ -29,21 +29,13 @@ std::string DownloadManager::get_unique_file_path(const boost::filesystem::path& boost::filesystem::path parent_dir = file_path.parent_path(); std::string filename = file_path.filename().string(); - - // Properly extract extension (includes the dot, e.g., ".txt") - // For "file.txt", extension() returns ".txt" - // For "file", extension() returns "" std::string extension = file_path.extension().string(); - - // Extract name without extension - // If extension is empty, name_without_ext is the full filename + std::string name_without_ext; if (extension.empty()) { name_without_ext = filename; BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::get_unique_file_path: No extension found, filename='%1%'") % filename; } else { - // Remove extension from filename (extension includes the dot) - // For "file.txt", filename="file.txt", extension=".txt", so name_without_ext="file" name_without_ext = filename.substr(0, filename.size() - extension.size()); BOOST_LOG_TRIVIAL(debug) << boost::format("DownloadManager::get_unique_file_path: filename='%1%', extension='%2%', name_without_ext='%3%'") % filename % extension % name_without_ext; @@ -252,15 +244,11 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { // Step 3: Set complete callback http.on_complete([this, task](std::string body, unsigned status) { wxGetApp().CallAfter([this, task, body]() { - // Check if task still exists and is not canceled - { - std::lock_guard lock(m_tasks_mutex); - if (m_tasks.find(task->task_id) == m_tasks.end() || - task->state == DownloadTaskState::Canceled) { - // Task has been canceled or cleaned up, ignore completion - BOOST_LOG_TRIVIAL(debug) << "DownloadManager: Ignoring complete callback for canceled/cleaned task " << task->task_id; - return; - } + // Check if task still exists and is not canceled (without lock to avoid deadlock with cleanup_task) + if (task->state == DownloadTaskState::Canceled) { + // Task has been canceled, ignore completion + BOOST_LOG_TRIVIAL(debug) << "DownloadManager: Ignoring complete callback for canceled task " << task->task_id; + return; } try { @@ -301,19 +289,11 @@ void DownloadManager::start_download_impl(std::shared_ptr task) { // Step 4: Set error callback http.on_error([this, task](std::string body, std::string error, unsigned status) { wxGetApp().CallAfter([this, task, error, status]() { - // Check if task still exists and is not canceled - { - std::lock_guard lock(m_tasks_mutex); - if (m_tasks.find(task->task_id) == m_tasks.end()) { - // Task has been cleaned up, ignore error callback - BOOST_LOG_TRIVIAL(debug) << "DownloadManager: Ignoring error callback for cleaned task " << task->task_id; - return; - } - if (task->state == DownloadTaskState::Canceled) { - // Task was canceled, ignore error callback (cancel already handled cleanup) - BOOST_LOG_TRIVIAL(debug) << "DownloadManager: Ignoring error callback for canceled task " << task->task_id; - return; - } + // Check if task was canceled (without lock to avoid deadlock with cleanup_task) + if (task->state == DownloadTaskState::Canceled) { + // Task was canceled, ignore error callback (cancel already handled cleanup) + BOOST_LOG_TRIVIAL(debug) << "DownloadManager: Ignoring error callback for canceled task " << task->task_id; + return; } std::string error_msg = boost::str(boost::format("HTTP error: %1% (status: %2%)") % error % status); @@ -368,11 +348,6 @@ bool DownloadManager::cancel_download(size_t task_id) { if (task->is_wcp_download()) { wcp_to_destroy = task->wcp_instance.lock(); } else { - // For internal downloads, don't call error callback if task is being canceled - // during destruction (e.g., when dialog is closing). The callback may reference - // a destroyed dialog object, causing a crash. - // Note: We clear the callbacks before cleanup to prevent any delayed callbacks - // from accessing destroyed objects. task->callbacks.on_error = nullptr; task->callbacks.on_progress = nullptr; task->callbacks.on_complete = nullptr; @@ -498,9 +473,7 @@ void DownloadManager::call_internal_progress_callback(std::shared_ptrcallbacks.on_progress && task->state != DownloadTaskState::Canceled) { task->callbacks.on_progress(task->task_id, percent, downloaded, total); } @@ -541,9 +514,7 @@ void DownloadManager::send_wcp_complete_update(std::shared_ptr tas void DownloadManager::call_internal_complete_callback(std::shared_ptr task, const std::string& file_path) { - // Only check if callback is still valid (cleared during cancellation) - // Don't check m_tasks because cleanup_task may have been called, but the callback - // should still be valid if not canceled + if (task->callbacks.on_complete && task->state != DownloadTaskState::Canceled) { task->callbacks.on_complete(task->task_id, file_path); } @@ -582,9 +553,7 @@ void DownloadManager::send_wcp_error_update(std::shared_ptr task, void DownloadManager::call_internal_error_callback(std::shared_ptr task, const std::string& error) { - // Only check if callback is still valid (cleared during cancellation) - // Don't check m_tasks because cleanup_task may have been called, but the callback - // should still be valid if not canceled + if (task->callbacks.on_error && task->state != DownloadTaskState::Canceled) { task->callbacks.on_error(task->task_id, error); }