From b070ecf0c73290df3749abd20151a64c53fb3c86 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 26 Dec 2022 10:48:17 +0000 Subject: [PATCH] Don't run simulation while dragging tuning slider. It has all manner of issues with wxWidgets event processing. The primary one that Kicad stumbles over is that there is no kill-focus when the mouse leaves the thumb if the sim is running, so subsequent mouse-up events (even if in another window) generate thumb scrolls. Another one that gets us is scroll events being generated when the window is fronted, but this may be debugger-specific. Fixes https://gitlab.com/kicad/code/kicad/issues/11366 --- eeschema/widgets/tuner_slider.cpp | 35 ++++++++---------------- eeschema/widgets/tuner_slider.h | 7 +---- eeschema/widgets/tuner_slider_base.cpp | 38 ++++++++++++++------------ eeschema/widgets/tuner_slider_base.fbp | 7 +++-- eeschema/widgets/tuner_slider_base.h | 1 + 5 files changed, 38 insertions(+), 50 deletions(-) diff --git a/eeschema/widgets/tuner_slider.cpp b/eeschema/widgets/tuner_slider.cpp index 44d0b72066..3b92dd3582 100644 --- a/eeschema/widgets/tuner_slider.cpp +++ b/eeschema/widgets/tuner_slider.cpp @@ -43,17 +43,13 @@ TUNER_SLIDER::TUNER_SLIDER( SIM_PLOT_FRAME* aFrame, wxWindow* aParent, m_min( 0.0 ), m_max( 0.0 ), m_value( 0.0 ), - m_changed( false ), m_frame ( aFrame ) { wxString ref = aSymbol->GetRef( &m_sheetPath ); const SPICE_ITEM* item = aFrame->GetExporter()->FindItem( std::string( ref.ToUTF8() ) ); if( !item ) - { - throw KI_PARAM_ERROR( wxString::Format( _( "Could not find Spice item with reference '%s'" ), - ref ) ); - } + throw KI_PARAM_ERROR( wxString::Format( _( "%s not found" ), ref ) ); m_name->SetLabel( ref ); m_closeBtn->SetBitmap( KiBitmap( BITMAPS::small_trash ) ); @@ -95,9 +91,6 @@ TUNER_SLIDER::TUNER_SLIDER( SIM_PLOT_FRAME* aFrame, wxWindow* aParent, updateValueText(); updateSlider(); - m_simTimer.SetOwner( this ); - Connect( wxEVT_TIMER, wxTimerEventHandler( TUNER_SLIDER::onSimTimer ), nullptr, this ); - Layout(); } @@ -156,16 +149,15 @@ bool TUNER_SLIDER::SetMax( const SPICE_VALUE& aVal ) void TUNER_SLIDER::updateComponentValue() { - // Start simulation in 100 ms, if the value does not change meanwhile - m_simTimer.StartOnce( 100 ); + wxQueueEvent( m_frame, new wxCommandEvent( EVT_SIM_UPDATE ) ); } void TUNER_SLIDER::updateSlider() { wxASSERT( m_max >= m_value && m_value >= m_min ); - SPICE_VALUE value = ( m_value - m_min ) / ( m_max - m_min ); - m_slider->SetValue( KiROUND( value.ToDouble() * 100.0 ) ); + double value = ( ( m_value - m_min ) / ( m_max - m_min ) ).ToDouble(); + m_slider->SetValue( KiROUND( value * 100.0 ) ); } @@ -204,7 +196,6 @@ void TUNER_SLIDER::updateValue() { SPICE_VALUE newCur( m_valueText->GetValue() ); SetValue( newCur ); - m_changed = true; } catch( const KI_PARAM_ERROR& ) { @@ -241,12 +232,18 @@ void TUNER_SLIDER::onSave( wxCommandEvent& event ) } +void TUNER_SLIDER::onSliderScroll( wxScrollEvent& event ) +{ + m_value = m_min + ( m_max - m_min ) * SPICE_VALUE( m_slider->GetValue() / 100.0 ); + updateValueText(); +} + + void TUNER_SLIDER::onSliderChanged( wxScrollEvent& event ) { m_value = m_min + ( m_max - m_min ) * SPICE_VALUE( m_slider->GetValue() / 100.0 ); updateValueText(); updateComponentValue(); - m_changed = true; } @@ -288,13 +285,3 @@ void TUNER_SLIDER::onMinTextEnter( wxCommandEvent& event ) { updateMin(); } - - -void TUNER_SLIDER::onSimTimer( wxTimerEvent& event ) -{ - if( m_changed ) - { - wxQueueEvent( m_frame, new wxCommandEvent( EVT_SIM_UPDATE ) ); - m_changed = false; - } -} diff --git a/eeschema/widgets/tuner_slider.h b/eeschema/widgets/tuner_slider.h index deb0ef5fc8..623b25208b 100644 --- a/eeschema/widgets/tuner_slider.h +++ b/eeschema/widgets/tuner_slider.h @@ -87,6 +87,7 @@ private: void onClose( wxCommandEvent& event ) override; void onSave( wxCommandEvent& event ) override; + void onSliderScroll( wxScrollEvent& event ) override; void onSliderChanged( wxScrollEvent& event ) override; void onMaxKillFocus( wxFocusEvent& event ) override; @@ -97,18 +98,12 @@ private: void onValueTextEnter( wxCommandEvent& event ) override; void onMinTextEnter( wxCommandEvent& event ) override; - void onSimTimer( wxTimerEvent& event ); - - ///< Timer that restarts the simulation after the slider value has changed - wxTimer m_simTimer; - KIID m_symbol; SCH_SHEET_PATH m_sheetPath; SPICE_VALUE m_min; SPICE_VALUE m_max; SPICE_VALUE m_value; - bool m_changed; SIM_PLOT_FRAME* m_frame; }; diff --git a/eeschema/widgets/tuner_slider_base.cpp b/eeschema/widgets/tuner_slider_base.cpp index 00009ba6fe..e063093a4e 100644 --- a/eeschema/widgets/tuner_slider_base.cpp +++ b/eeschema/widgets/tuner_slider_base.cpp @@ -25,6 +25,8 @@ TUNER_SLIDER_BASE::TUNER_SLIDER_BASE( wxWindow* parent, wxWindowID id, const wxP m_name = new wxStaticText( m_panel1, wxID_ANY, _("Name"), wxDefaultPosition, wxDefaultSize, 0 ); m_name->Wrap( -1 ); + m_name->SetFont( wxFont( wxNORMAL_FONT->GetPointSize(), wxFONTFAMILY_DEFAULT, wxFONTSTYLE_NORMAL, wxFONTWEIGHT_BOLD, false, wxEmptyString ) ); + bSizerUpper->Add( m_name, 0, wxEXPAND|wxTOP|wxRIGHT|wxLEFT, 5 ); @@ -88,7 +90,7 @@ TUNER_SLIDER_BASE::TUNER_SLIDER_BASE( wxWindow* parent, wxWindowID id, const wxP m_panel1->SetSizer( bSizer6 ); m_panel1->Layout(); bSizer6->Fit( m_panel1 ); - bSizerMain->Add( m_panel1, 1, wxEXPAND|wxRIGHT, 5 ); + bSizerMain->Add( m_panel1, 1, wxEXPAND|wxRIGHT, 8 ); this->SetSizer( bSizerMain ); @@ -96,15 +98,16 @@ TUNER_SLIDER_BASE::TUNER_SLIDER_BASE( wxWindow* parent, wxWindowID id, const wxP bSizerMain->Fit( this ); // Connect Events - m_slider->Connect( wxEVT_SCROLL_TOP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Connect( wxEVT_SCROLL_BOTTOM, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Connect( wxEVT_SCROLL_LINEUP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Connect( wxEVT_SCROLL_LINEDOWN, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Connect( wxEVT_SCROLL_PAGEUP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Connect( wxEVT_SCROLL_PAGEDOWN, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Connect( wxEVT_SCROLL_THUMBTRACK, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); + m_slider->Connect( wxEVT_SCROLL_TOP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Connect( wxEVT_SCROLL_BOTTOM, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Connect( wxEVT_SCROLL_LINEUP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Connect( wxEVT_SCROLL_LINEDOWN, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Connect( wxEVT_SCROLL_PAGEUP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Connect( wxEVT_SCROLL_PAGEDOWN, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Connect( wxEVT_SCROLL_THUMBTRACK, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Connect( wxEVT_SCROLL_THUMBRELEASE, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Connect( wxEVT_SCROLL_CHANGED, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); m_slider->Connect( wxEVT_SCROLL_THUMBRELEASE, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Connect( wxEVT_SCROLL_CHANGED, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); m_maxText->Connect( wxEVT_KILL_FOCUS, wxFocusEventHandler( TUNER_SLIDER_BASE::onMaxKillFocus ), NULL, this ); m_maxText->Connect( wxEVT_COMMAND_TEXT_ENTER, wxCommandEventHandler( TUNER_SLIDER_BASE::onMaxTextEnter ), NULL, this ); m_valueText->Connect( wxEVT_KILL_FOCUS, wxFocusEventHandler( TUNER_SLIDER_BASE::onValueKillFocus ), NULL, this ); @@ -118,15 +121,16 @@ TUNER_SLIDER_BASE::TUNER_SLIDER_BASE( wxWindow* parent, wxWindowID id, const wxP TUNER_SLIDER_BASE::~TUNER_SLIDER_BASE() { // Disconnect Events - m_slider->Disconnect( wxEVT_SCROLL_TOP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Disconnect( wxEVT_SCROLL_BOTTOM, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Disconnect( wxEVT_SCROLL_LINEUP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Disconnect( wxEVT_SCROLL_LINEDOWN, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Disconnect( wxEVT_SCROLL_PAGEUP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Disconnect( wxEVT_SCROLL_PAGEDOWN, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Disconnect( wxEVT_SCROLL_THUMBTRACK, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); + m_slider->Disconnect( wxEVT_SCROLL_TOP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Disconnect( wxEVT_SCROLL_BOTTOM, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Disconnect( wxEVT_SCROLL_LINEUP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Disconnect( wxEVT_SCROLL_LINEDOWN, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Disconnect( wxEVT_SCROLL_PAGEUP, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Disconnect( wxEVT_SCROLL_PAGEDOWN, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Disconnect( wxEVT_SCROLL_THUMBTRACK, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Disconnect( wxEVT_SCROLL_THUMBRELEASE, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); + m_slider->Disconnect( wxEVT_SCROLL_CHANGED, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderScroll ), NULL, this ); m_slider->Disconnect( wxEVT_SCROLL_THUMBRELEASE, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); - m_slider->Disconnect( wxEVT_SCROLL_CHANGED, wxScrollEventHandler( TUNER_SLIDER_BASE::onSliderChanged ), NULL, this ); m_maxText->Disconnect( wxEVT_KILL_FOCUS, wxFocusEventHandler( TUNER_SLIDER_BASE::onMaxKillFocus ), NULL, this ); m_maxText->Disconnect( wxEVT_COMMAND_TEXT_ENTER, wxCommandEventHandler( TUNER_SLIDER_BASE::onMaxTextEnter ), NULL, this ); m_valueText->Disconnect( wxEVT_KILL_FOCUS, wxFocusEventHandler( TUNER_SLIDER_BASE::onValueKillFocus ), NULL, this ); diff --git a/eeschema/widgets/tuner_slider_base.fbp b/eeschema/widgets/tuner_slider_base.fbp index e948854bc6..19cf6ba4b2 100644 --- a/eeschema/widgets/tuner_slider_base.fbp +++ b/eeschema/widgets/tuner_slider_base.fbp @@ -58,7 +58,7 @@ wxVERTICAL none - 5 + 8 wxEXPAND|wxRIGHT 1 @@ -154,7 +154,7 @@ 1 1 - + ,90,700,-1,70,0 0 0 wxID_ANY @@ -328,7 +328,8 @@ - onSliderChanged + onSliderScroll + onSliderChanged diff --git a/eeschema/widgets/tuner_slider_base.h b/eeschema/widgets/tuner_slider_base.h index 38a382863a..17ade65a71 100644 --- a/eeschema/widgets/tuner_slider_base.h +++ b/eeschema/widgets/tuner_slider_base.h @@ -51,6 +51,7 @@ class TUNER_SLIDER_BASE : public wxPanel STD_BITMAP_BUTTON* m_closeBtn; // Virtual event handlers, override them in your derived class + virtual void onSliderScroll( wxScrollEvent& event ) { event.Skip(); } virtual void onSliderChanged( wxScrollEvent& event ) { event.Skip(); } virtual void onMaxKillFocus( wxFocusEvent& event ) { event.Skip(); } virtual void onMaxTextEnter( wxCommandEvent& event ) { event.Skip(); }