From fff428dcd660618e6af8a258a650af278c6abfc8 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Tue, 3 Mar 2026 11:27:30 -0800 Subject: [PATCH] Add THROTTLE class and DrainPendingEvents Five call sites independently reimplemented the same time-gated throttle pattern. This consolidates them all into a single THROTTLE class backed by steady_clock. Three call sites duplicated the wxWidgets YieldFor workaround for timer-event backlog (wxWidgets issue #26192). This replaces them with a shared DrainPendingEvents free function. --- common/widgets/app_progress_dialog.cpp | 7 +- common/widgets/wx_progress_reporters.cpp | 10 +-- eeschema/symbol_editor/symbol_edit_frame.cpp | 21 ++++-- eeschema/symbol_tree_model_adapter.cpp | 2 - .../symbol_tree_synchronizing_adapter.cpp | 15 ++--- include/core/throttle.h | 64 +++++++++++++++++++ include/widgets/wx_event_utils.h | 44 +++++++++++++ pcbnew/dialogs/dialog_drc.cpp | 19 ++---- pcbnew/dialogs/dialog_drc.h | 9 +-- pcbnew/fp_tree_synchronizing_adapter.cpp | 2 - pcbnew/pcb_io/allegro/allegro_builder.cpp | 16 ++--- .../pcb_io/allegro/convert/allegro_parser.cpp | 16 ++--- qa/tests/common/CMakeLists.txt | 1 + qa/tests/common/test_throttle.cpp | 59 +++++++++++++++++ 14 files changed, 213 insertions(+), 72 deletions(-) create mode 100644 include/core/throttle.h create mode 100644 include/widgets/wx_event_utils.h create mode 100644 qa/tests/common/test_throttle.cpp diff --git a/common/widgets/app_progress_dialog.cpp b/common/widgets/app_progress_dialog.cpp index 1ce4449a4b..df12626baf 100644 --- a/common/widgets/app_progress_dialog.cpp +++ b/common/widgets/app_progress_dialog.cpp @@ -24,6 +24,7 @@ #include #include #include +#include APP_PROGRESS_DIALOG::APP_PROGRESS_DIALOG( const wxString& aTitle, const wxString& aMessage, int aMaximum, wxWindow* aParent, @@ -54,5 +55,9 @@ bool APP_PROGRESS_DIALOG::Update( int aValue, const wxString& aNewMsg, bool* aSk m_appProgressIndicator.SetValue( aValue ); } - return APP_PROGRESS_DIALOG_BASE::Update( aValue, aNewMsg, aSkip ); + bool diag = APP_PROGRESS_DIALOG_BASE::Update( aValue, aNewMsg, aSkip ); + + DrainPendingEvents(); + + return diag; } diff --git a/common/widgets/wx_progress_reporters.cpp b/common/widgets/wx_progress_reporters.cpp index c374aee218..bcd97cccbe 100644 --- a/common/widgets/wx_progress_reporters.cpp +++ b/common/widgets/wx_progress_reporters.cpp @@ -24,8 +24,8 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -#include #include +#include #include @@ -96,10 +96,7 @@ bool WX_PROGRESS_REPORTER::updateUI() // Returns false when cancelled (if it's a cancellable dialog) bool diag = WX_PROGRESS_REPORTER_BASE::Update( cur, message ); - // Prevent wx from queuing timer events and slowing down - // See https://github.com/wxWidgets/wxWidgets/issues/26192 - if( wxEventLoopBase* loop = wxEventLoopBase::GetActive() ) - loop->YieldFor( wxEVT_CATEGORY_TIMER ); + DrainPendingEvents(); return diag; } @@ -122,8 +119,7 @@ bool GAUGE_PROGRESS_REPORTER::updateUI() wxGauge::SetValue( cur ); - if( wxEventLoopBase* loop = wxEventLoopBase::GetActive() ) - loop->YieldFor( wxEVT_CATEGORY_UI ); + DrainPendingEvents( wxEVT_CATEGORY_UI ); return true; // No cancel button on a wxGauge } diff --git a/eeschema/symbol_editor/symbol_edit_frame.cpp b/eeschema/symbol_editor/symbol_edit_frame.cpp index e280e008ff..fa907801de 100644 --- a/eeschema/symbol_editor/symbol_edit_frame.cpp +++ b/eeschema/symbol_editor/symbol_edit_frame.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -1348,18 +1349,28 @@ void SYMBOL_EDIT_FRAME::SyncLibraries( bool aShowProgress, bool aPreloadCancelle APP_PROGRESS_DIALOG progressDlg( _( "Loading Symbol Libraries" ), wxEmptyString, m_libMgr->GetAdapter()->GetLibrariesCount(), this ); + THROTTLE progressThrottle( std::chrono::milliseconds( 350 ) ); + int pendingProgress = 0; + bool callbackFired = false; + wxString pendingLibName; + m_libMgr->Sync( aForceRefresh, [&]( int progress, int max, const wxString& libName ) { - progressDlg.Update( progress, wxString::Format( _( "Loading library '%s'..." ), libName ) ); + pendingProgress = progress; + pendingLibName = libName; + callbackFired = true; + + if( progressThrottle.Ready() ) + progressDlg.Update( progress, wxString::Format( _( "Loading library '%s'..." ), libName ) ); } ); + + if( callbackFired ) + progressDlg.Update( pendingProgress, wxString::Format( _( "Loading library '%s'..." ), pendingLibName ) ); } else if( !aPreloadCancelled ) { - m_libMgr->Sync( aForceRefresh, - [&]( int progress, int max, const wxString& libName ) - { - } ); + m_libMgr->Sync( aForceRefresh, [&]( int progress, int max, const wxString& libName ) {} ); } if( m_treePane ) diff --git a/eeschema/symbol_tree_model_adapter.cpp b/eeschema/symbol_tree_model_adapter.cpp index 789d651f9a..d43de53f73 100644 --- a/eeschema/symbol_tree_model_adapter.cpp +++ b/eeschema/symbol_tree_model_adapter.cpp @@ -37,8 +37,6 @@ #include #include -#define PROGRESS_INTERVAL_MILLIS 33 // 30 FPS refresh rate - wxObjectDataPtr SYMBOL_TREE_MODEL_ADAPTER::Create( SCH_BASE_FRAME* aParent, SYMBOL_LIBRARY_ADAPTER* aManager ) diff --git a/eeschema/symbol_tree_synchronizing_adapter.cpp b/eeschema/symbol_tree_synchronizing_adapter.cpp index f3c72ec696..f98031f11d 100644 --- a/eeschema/symbol_tree_synchronizing_adapter.cpp +++ b/eeschema/symbol_tree_synchronizing_adapter.cpp @@ -27,6 +27,7 @@ #include +#include #include #include #include @@ -70,12 +71,10 @@ bool SYMBOL_TREE_SYNCHRONIZING_ADAPTER::IsContainer( const wxDataViewItem& aItem } -#define PROGRESS_INTERVAL_MILLIS 120 - void SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Sync( const wxString& aForceRefresh, std::function aProgressCallback ) { - wxLongLong nextUpdate = wxGetUTCTimeMillis() + (PROGRESS_INTERVAL_MILLIS / 2); + THROTTLE progressThrottle( std::chrono::milliseconds( 120 ) ); m_lastSyncHash = m_libMgr->GetHash(); int i = 0, max = GetLibrariesCount(); @@ -87,11 +86,8 @@ void SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Sync( const wxString& aForceRefresh, { const wxString& name = it->get()->m_Name; - if( wxGetUTCTimeMillis() > nextUpdate ) - { + if( progressThrottle.Ready() ) aProgressCallback( i, max, name ); - nextUpdate = wxGetUTCTimeMillis() + PROGRESS_INTERVAL_MILLIS; - } // Check the table row directly rather than adapter->HasLibrary(), which requires the // library to be fully loaded. After table reloads (e.g. adding a new library), all @@ -137,11 +133,8 @@ void SYMBOL_TREE_SYNCHRONIZING_ADAPTER::Sync( const wxString& aForceRefresh, if( m_libHashes.count( libName ) == 0 ) { - if( wxGetUTCTimeMillis() > nextUpdate ) - { + if( progressThrottle.Ready() ) aProgressCallback( i++, max, libName ); - nextUpdate = wxGetUTCTimeMillis() + PROGRESS_INTERVAL_MILLIS; - } auto optRow = adapter->GetRow( libName ); wxCHECK2( optRow.has_value(), continue ); diff --git a/include/core/throttle.h b/include/core/throttle.h new file mode 100644 index 0000000000..4b487117c0 --- /dev/null +++ b/include/core/throttle.h @@ -0,0 +1,64 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright The KiCad Developers, see AUTHORS.txt for contributors. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#pragma once + +#include + +/** + * Rate-limiter that fires at most once per interval. + * + * The first call to Ready() always returns true so callers can emit an + * initial update immediately. + */ +class THROTTLE +{ +public: + explicit THROTTLE( std::chrono::milliseconds aInterval ) : + m_interval( aInterval ), + m_last( std::chrono::steady_clock::time_point::min() ) + { + } + + /** + * @return true when at least \a m_interval has elapsed since the previous + * true return (or since construction). Resets the internal clock + * on a true return. + */ + bool Ready() + { + auto now = std::chrono::steady_clock::now(); + + if( now - m_last >= m_interval ) + { + m_last = now; + return true; + } + + return false; + } + +private: + std::chrono::milliseconds m_interval; + std::chrono::steady_clock::time_point m_last; +}; diff --git a/include/widgets/wx_event_utils.h b/include/widgets/wx_event_utils.h new file mode 100644 index 0000000000..d5f9965263 --- /dev/null +++ b/include/widgets/wx_event_utils.h @@ -0,0 +1,44 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright The KiCad Developers, see AUTHORS.txt for contributors. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#pragma once + +#include + +/** + * Drain pending events of the given categories from the active event loop. + * + * During long-running operations that call progress-dialog Update() frequently, + * wxWidgets can accumulate a backlog of timer events that makes the UI sluggish + * after the operation completes. Calling this after each Update() prevents the + * backlog from building up. + * + * See https://github.com/wxWidgets/wxWidgets/issues/26192 + * + * @param aCategories Event categories to yield for (default wxEVT_CATEGORY_TIMER). + */ +inline void DrainPendingEvents( long aCategories = wxEVT_CATEGORY_TIMER ) +{ + if( wxEventLoopBase* loop = wxEventLoopBase::GetActive() ) + loop->YieldFor( aCategories ); +} diff --git a/pcbnew/dialogs/dialog_drc.cpp b/pcbnew/dialogs/dialog_drc.cpp index 86a3c5f138..0794cb2a25 100644 --- a/pcbnew/dialogs/dialog_drc.cpp +++ b/pcbnew/dialogs/dialog_drc.cpp @@ -24,6 +24,7 @@ */ #include +#include #include #include #include @@ -81,8 +82,8 @@ DIALOG_DRC::DIALOG_DRC( PCB_EDIT_FRAME* aEditorFrame, wxWindow* aParent ) : m_markersTreeModel( nullptr ), m_unconnectedTreeModel( nullptr ), m_fpWarningsTreeModel( nullptr ), - m_lastUpdateUi( std::chrono::steady_clock::now() ), - m_lastYieldUi( std::chrono::steady_clock::now() ) + m_updateThrottle( std::chrono::milliseconds( 100 ) ), + m_yieldThrottle( std::chrono::milliseconds( 2000 ) ) { SetName( DIALOG_DRC_WINDOW_NAME ); // Set a window name to be able to find it KIPLATFORM::UI::SetFloatLevel( this ); @@ -228,27 +229,17 @@ bool DIALOG_DRC::updateUI() m_gauge->SetValue( newValue ); } - std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); - // Repaint the dialog at ~10Hz using Update() which processes only pending expose/ // draw events without entering the full platform event loop. - if( std::chrono::duration_cast( now - m_lastUpdateUi ).count() - > 100 ) - { + if( m_updateThrottle.Ready() ) Update(); - m_lastUpdateUi = now; - } // Yield to the event loop infrequently so the cancel button remains functional. // On some Linux systems with glycin-enabled gdk-pixbuf (2.44+), entering the GTK event // loop triggers heavyweight sandbox process spawning that can add hundreds of milliseconds // per call, so we keep this interval long. - if( std::chrono::duration_cast( now - m_lastYieldUi ).count() - > 2000 ) - { + if( m_yieldThrottle.Ready() ) Pgm().App().SafeYieldFor( this, wxEVT_CATEGORY_NATIVE_EVENTS ); - m_lastYieldUi = now; - } return !m_cancelled; } diff --git a/pcbnew/dialogs/dialog_drc.h b/pcbnew/dialogs/dialog_drc.h index 6ae3b53743..e6b684f0be 100644 --- a/pcbnew/dialogs/dialog_drc.h +++ b/pcbnew/dialogs/dialog_drc.h @@ -26,7 +26,7 @@ #pragma once -#include +#include #include #include #include @@ -136,10 +136,7 @@ private: RC_TREE_MODEL* m_unconnectedTreeModel; // wx reference-counted ptr RC_TREE_MODEL* m_fpWarningsTreeModel; // wx reference-counted ptr - /// Used to throttle visual refresh in updateUi() - std::chrono::steady_clock::time_point m_lastUpdateUi; - - /// Used to throttle event loop yields in updateUi() - std::chrono::steady_clock::time_point m_lastYieldUi; + THROTTLE m_updateThrottle; + THROTTLE m_yieldThrottle; }; diff --git a/pcbnew/fp_tree_synchronizing_adapter.cpp b/pcbnew/fp_tree_synchronizing_adapter.cpp index 00f7d5c30d..15bc07b37c 100644 --- a/pcbnew/fp_tree_synchronizing_adapter.cpp +++ b/pcbnew/fp_tree_synchronizing_adapter.cpp @@ -71,8 +71,6 @@ bool FP_TREE_SYNCHRONIZING_ADAPTER::IsContainer( const wxDataViewItem& aItem ) c } -#define PROGRESS_INTERVAL_MILLIS 33 // 30 FPS refresh rate - void FP_TREE_SYNCHRONIZING_ADAPTER::Sync( FOOTPRINT_LIBRARY_ADAPTER* aLibs ) { m_libs = aLibs; diff --git a/pcbnew/pcb_io/allegro/allegro_builder.cpp b/pcbnew/pcb_io/allegro/allegro_builder.cpp index 20dc8b3a0d..9b3593a6fc 100644 --- a/pcbnew/pcb_io/allegro/allegro_builder.cpp +++ b/pcbnew/pcb_io/allegro/allegro_builder.cpp @@ -24,7 +24,6 @@ #include "allegro_builder.h" -#include #include #include #include @@ -36,6 +35,7 @@ #include #include +#include #include #include @@ -4585,7 +4585,7 @@ bool BOARD_BUILDER::BuildBoard() const LL_WALKER fpWalker( m_brdDb.m_Header->m_LL_0x2B, m_brdDb ); std::vector bulkAddedItems; - auto lastRefresh = std::chrono::steady_clock::now(); + THROTTLE refreshThrottle( std::chrono::milliseconds( 100 ) ); for( const BLOCK_BASE* fpContainer : fpWalker ) { @@ -4626,16 +4626,8 @@ bool BOARD_BUILDER::BuildBoard() } } - if( m_progressReporter ) - { - auto now = std::chrono::steady_clock::now(); - - if( now - lastRefresh >= std::chrono::milliseconds( 100 ) ) - { - m_progressReporter->KeepRefreshing(); - lastRefresh = now; - } - } + if( m_progressReporter && refreshThrottle.Ready() ) + m_progressReporter->KeepRefreshing(); } } } diff --git a/pcbnew/pcb_io/allegro/convert/allegro_parser.cpp b/pcbnew/pcb_io/allegro/convert/allegro_parser.cpp index e98556be68..e7039741db 100644 --- a/pcbnew/pcb_io/allegro/convert/allegro_parser.cpp +++ b/pcbnew/pcb_io/allegro/convert/allegro_parser.cpp @@ -25,7 +25,6 @@ #include "convert/allegro_parser.h" #include -#include #include #include @@ -33,6 +32,7 @@ #include #include +#include #include #include @@ -2475,7 +2475,7 @@ void ALLEGRO::PARSER::readObjects( BRD_DB& aBoard ) BLOCK_PARSER blockParser( m_stream, ver, aBoard.m_Header->Get_0x27_End() ); - auto lastRefresh = std::chrono::steady_clock::now(); + THROTTLE refreshThrottle( std::chrono::milliseconds( 100 ) ); while( true ) { @@ -2577,16 +2577,8 @@ void ALLEGRO::PARSER::readObjects( BRD_DB& aBoard ) { m_progressReporter->AdvanceProgress(); - if( ( aBoard.GetObjectCount() & 0x3F ) == 0 ) - { - auto now = std::chrono::steady_clock::now(); - - if( now - lastRefresh >= std::chrono::milliseconds( 100 ) ) - { - m_progressReporter->KeepRefreshing(); - lastRefresh = now; - } - } + if( ( aBoard.GetObjectCount() & 0x3F ) == 0 && refreshThrottle.Ready() ) + m_progressReporter->KeepRefreshing(); } } } diff --git a/qa/tests/common/CMakeLists.txt b/qa/tests/common/CMakeLists.txt index e518b4e2ae..5a0da3ad99 100644 --- a/qa/tests/common/CMakeLists.txt +++ b/qa/tests/common/CMakeLists.txt @@ -95,6 +95,7 @@ set( QA_COMMON_SRCS io/pads/test_pads_common.cpp io/pads/test_pads_unit_converter.cpp + test_throttle.cpp view/test_zoom_controller.cpp test_netname_validator.cpp diff --git a/qa/tests/common/test_throttle.cpp b/qa/tests/common/test_throttle.cpp new file mode 100644 index 0000000000..5af26c6ad0 --- /dev/null +++ b/qa/tests/common/test_throttle.cpp @@ -0,0 +1,59 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright The KiCad Developers, see AUTHORS.txt for contributors. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include + +#include +#include + + +BOOST_AUTO_TEST_SUITE( Throttle ) + + +BOOST_AUTO_TEST_CASE( FirstCallAlwaysReady ) +{ + THROTTLE t( std::chrono::milliseconds( 500 ) ); + BOOST_CHECK( t.Ready() ); +} + + +BOOST_AUTO_TEST_CASE( SecondCallRejectsWithinInterval ) +{ + THROTTLE t( std::chrono::milliseconds( 200 ) ); + BOOST_CHECK( t.Ready() ); + BOOST_CHECK( !t.Ready() ); +} + + +BOOST_AUTO_TEST_CASE( ReadyAgainAfterInterval ) +{ + THROTTLE t( std::chrono::milliseconds( 50 ) ); + BOOST_CHECK( t.Ready() ); + + std::this_thread::sleep_for( std::chrono::milliseconds( 60 ) ); + + BOOST_CHECK( t.Ready() ); +} + + +BOOST_AUTO_TEST_SUITE_END()