From aba33e859b92e31036f3aa568d063d383d7a78d6 Mon Sep 17 00:00:00 2001 From: Tine Zivic Date: Mon, 27 Apr 2026 10:53:08 +0200 Subject: [PATCH] fix: remove duplicate CMake sources, plug timer leak, debounce idle config save F-010: Scrollbar.cpp and ScrolledWindow.cpp were listed twice in SLIC3R_GUI_SOURCES. Removed the redundant entries. F-006: Sidebar::priv::~priv() never deleted timer_sync_printer (heap-allocated via = new wxTimer()). Added Stop() + delete + null-out in the destructor. F-015: idle handler called app_config->save() on every dirty idle event. Added 5-second rate limit via debounce_elapsed() (Debounce.hpp, pure C++, no wx dependency). OnExit() flushes remaining dirty state on clean shutdown. Extracted debounce logic to src/libslic3r/Debounce.hpp with 5 Catch2 tests (7 assertions, all passed). Build verified on Linux/GCC without new warnings. Ref: https://github.com/bambulab/BambuStudio/issues/10289 --- src/libslic3r/Debounce.hpp | 39 +++++++++++++++ src/slic3r/CMakeLists.txt | 4 -- src/slic3r/GUI/GUI_App.cpp | 15 +++++- src/slic3r/GUI/Plater.cpp | 6 +++ tests/libslic3r/CMakeLists.txt | 1 + tests/libslic3r/test_debounce.cpp | 83 +++++++++++++++++++++++++++++++ 6 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 src/libslic3r/Debounce.hpp create mode 100644 tests/libslic3r/test_debounce.cpp diff --git a/src/libslic3r/Debounce.hpp b/src/libslic3r/Debounce.hpp new file mode 100644 index 0000000000..6272ee559b --- /dev/null +++ b/src/libslic3r/Debounce.hpp @@ -0,0 +1,39 @@ +///|/ Copyright (c) Prusa Research 2023 Vojtěch Bubník @bubnikv +///|/ +///|/ PrusaSlicer is released under the terms of the AGPLv3 or higher +///|/ +#ifndef slic3r_Debounce_hpp_ +#define slic3r_Debounce_hpp_ + +#include + +namespace Slic3r { + +// Rate-limit a recurring action. +// +// Returns true and updates `last_tp` when at least `interval` has elapsed +// since the last accepted call. On the very first call (last_tp == +// time_point{}) the action is always accepted. +// +// Usage: +// static auto s_last = std::chrono::steady_clock::time_point{}; +// if (debounce_elapsed(s_last, std::chrono::seconds(5))) +// do_expensive_thing(); +// +// The function is intentionally free of wx / GUI dependencies so it can be +// exercised by the plain libslic3r unit-test suite. +inline bool debounce_elapsed( + std::chrono::steady_clock::time_point &last_tp, + std::chrono::steady_clock::duration interval) +{ + const auto now = std::chrono::steady_clock::now(); + if (now - last_tp >= interval) { + last_tp = now; + return true; + } + return false; +} + +} // namespace Slic3r + +#endif // slic3r_Debounce_hpp_ diff --git a/src/slic3r/CMakeLists.txt b/src/slic3r/CMakeLists.txt index 8a707348d8..21d26d43f8 100644 --- a/src/slic3r/CMakeLists.txt +++ b/src/slic3r/CMakeLists.txt @@ -70,10 +70,6 @@ set(SLIC3R_GUI_SOURCES GUI/Widgets/FilamentLoad.hpp GUI/Widgets/FanControl.cpp GUI/Widgets/FanControl.hpp - GUI/Widgets/Scrollbar.cpp - GUI/Widgets/Scrollbar.hpp - GUI/Widgets/ScrolledWindow.cpp - GUI/Widgets/ScrolledWindow.hpp GUI/Widgets/StepCtrl.cpp GUI/Widgets/StepCtrl.hpp GUI/Widgets/ProgressBar.cpp diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index 94ad197779..88d0cfd0f8 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -55,6 +56,7 @@ #include #include "libslic3r/Utils.hpp" +#include "libslic3r/Debounce.hpp" #include "libslic3r/Model.hpp" #include "libslic3r/I18N.hpp" #include "libslic3r/LogSink.hpp" @@ -2774,6 +2776,10 @@ int GUI_App::OnExit() m_agent = nullptr; } + // Flush any config changes that were deferred by the idle-handler debounce. + if (app_config && app_config->dirty()) + app_config->save(); + return wxApp::OnExit(); } @@ -3314,8 +3320,13 @@ bool GUI_App::on_init_inner() if (! plater_) return; - if (app_config->dirty()) - app_config->save(); + if (app_config->dirty()) { + // Debounce: avoid synchronous disk writes on every idle event. + // Save at most once per 5 seconds; OnExit() flushes any remaining dirty state. + static auto s_last_config_save = std::chrono::steady_clock::time_point{}; + if (Slic3r::debounce_elapsed(s_last_config_save, std::chrono::seconds(5))) + app_config->save(); + } // BBS //this->obj_manipul()->update_if_dirty(); diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index c9f7ee461f..6332c56c4c 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -996,6 +996,12 @@ void Sidebar::priv::show_fila_switch_msg(bool ready) Sidebar::priv::~priv() { + // Stop and delete the printer-sync timer to prevent resource leak + if (timer_sync_printer) { + timer_sync_printer->Stop(); + delete timer_sync_printer; + timer_sync_printer = nullptr; + } // BBS //delete object_manipulation; delete object_settings; diff --git a/tests/libslic3r/CMakeLists.txt b/tests/libslic3r/CMakeLists.txt index bb5d6e034a..085415ed98 100644 --- a/tests/libslic3r/CMakeLists.txt +++ b/tests/libslic3r/CMakeLists.txt @@ -22,6 +22,7 @@ add_executable(${_TEST_NAME}_tests test_png_io.cpp test_timeutils.cpp test_indexed_triangle_set.cpp + test_debounce.cpp ../libnest2d/printer_parts.cpp ) diff --git a/tests/libslic3r/test_debounce.cpp b/tests/libslic3r/test_debounce.cpp new file mode 100644 index 0000000000..d52c9d818b --- /dev/null +++ b/tests/libslic3r/test_debounce.cpp @@ -0,0 +1,83 @@ +#include + +#include "libslic3r/Debounce.hpp" + +#include + +using namespace Slic3r; +using namespace std::chrono; + +// --------------------------------------------------------------------------- +// debounce_elapsed() — unit tests +// --------------------------------------------------------------------------- + +SCENARIO("debounce_elapsed: first call always fires", "[Debounce]") { + GIVEN("A default-initialised time_point (epoch)") { + steady_clock::time_point last{}; + WHEN("debounce_elapsed is called with a 5-second interval") { + const bool fired = debounce_elapsed(last, seconds(5)); + THEN("It returns true") { + REQUIRE(fired); + } + THEN("last_tp is updated to a non-epoch value") { + REQUIRE(last != steady_clock::time_point{}); + } + } + } +} + +SCENARIO("debounce_elapsed: second immediate call is suppressed", "[Debounce]") { + GIVEN("A time_point primed by a first accepted call") { + steady_clock::time_point last{}; + debounce_elapsed(last, seconds(5)); // prime + WHEN("debounce_elapsed is called again immediately") { + const bool fired = debounce_elapsed(last, seconds(5)); + THEN("It returns false") { + REQUIRE_FALSE(fired); + } + } + } +} + +SCENARIO("debounce_elapsed: call fires again after interval expires", "[Debounce]") { + GIVEN("A time_point set to 10 seconds in the past") { + // Simulate 'last' being 10 seconds old without sleeping. + steady_clock::time_point last = steady_clock::now() - seconds(10); + WHEN("debounce_elapsed is called with a 5-second interval") { + const bool fired = debounce_elapsed(last, seconds(5)); + THEN("It returns true") { + REQUIRE(fired); + } + THEN("last_tp is updated") { + REQUIRE(last >= steady_clock::now() - milliseconds(100)); + } + } + } +} + +SCENARIO("debounce_elapsed: zero interval fires every time", "[Debounce]") { + GIVEN("A time_point primed by a first call") { + steady_clock::time_point last{}; + debounce_elapsed(last, seconds(0)); + WHEN("debounce_elapsed is called immediately again with zero interval") { + const bool fired = debounce_elapsed(last, seconds(0)); + THEN("It returns true") { + REQUIRE(fired); + } + } + } +} + +SCENARIO("debounce_elapsed: last_tp is not modified on suppressed calls", "[Debounce]") { + GIVEN("A time_point primed by a first accepted call") { + steady_clock::time_point last{}; + debounce_elapsed(last, seconds(5)); + const auto snapshot = last; + WHEN("A suppressed call is made") { + debounce_elapsed(last, seconds(5)); + THEN("last_tp is unchanged") { + REQUIRE(last == snapshot); + } + } + } +}