From fef6eaa8ba38d92164bf28849d21f8286c8b9ff1 Mon Sep 17 00:00:00 2001 From: Mikolaj Wielgus Date: Sat, 12 Nov 2022 03:09:26 +0100 Subject: [PATCH] Sim Model Editor: Don't use shared_ptr to hack around lifetimes --- eeschema/dialogs/dialog_sim_model.cpp | 33 ++++++++------ eeschema/sim/sim_property.cpp | 62 ++++++++++++++++----------- eeschema/sim/sim_property.h | 21 ++++----- 3 files changed, 66 insertions(+), 50 deletions(-) diff --git a/eeschema/dialogs/dialog_sim_model.cpp b/eeschema/dialogs/dialog_sim_model.cpp index f48f614db4..78751f811d 100644 --- a/eeschema/dialogs/dialog_sim_model.cpp +++ b/eeschema/dialogs/dialog_sim_model.cpp @@ -126,6 +126,18 @@ DIALOG_SIM_MODEL::DIALOG_SIM_MODEL( wxWindow* aParent, SCH_SYMBOL& aSymbol, template DIALOG_SIM_MODEL::~DIALOG_SIM_MODEL() { + // Disable all properties. This is necessary because some of their methods are called after + // destruction of DIALOG_SIM_MODEL, oddly. When disabled, they never access their models. + for( wxPropertyGridIterator it = m_paramGrid->GetIterator(); !it.AtEnd(); ++it ) + { + SIM_PROPERTY* prop = dynamic_cast( *it ); + + if( !prop ) + continue; + + prop->Disable(); + } + // Delete the GRID_TRICKS. m_pinAssignmentsGrid->PopEventHandler( true ); @@ -775,19 +787,18 @@ wxPGProperty* DIALOG_SIM_MODEL::newParamProperty( int aParamIndex ) const { case SIM_VALUE::TYPE_BOOL: // TODO. - prop = new SIM_BOOL_PROPERTY( paramDescription, param.info.name, m_library, - curModelSharedPtr(), aParamIndex ); + prop = new SIM_BOOL_PROPERTY( paramDescription, param.info.name, curModel(), aParamIndex ); prop->SetAttribute( wxPG_BOOL_USE_CHECKBOX, true ); break; case SIM_VALUE::TYPE_INT: - prop = new SIM_STRING_PROPERTY( paramDescription, param.info.name, m_library, - curModelSharedPtr(), aParamIndex, SIM_VALUE::TYPE_INT ); + prop = new SIM_STRING_PROPERTY( paramDescription, param.info.name, curModel(), + aParamIndex, SIM_VALUE::TYPE_INT ); break; case SIM_VALUE::TYPE_FLOAT: - prop = new SIM_STRING_PROPERTY( paramDescription, param.info.name, m_library, - curModelSharedPtr(), aParamIndex, SIM_VALUE::TYPE_FLOAT ); + prop = new SIM_STRING_PROPERTY( paramDescription, param.info.name, curModel(), + aParamIndex, SIM_VALUE::TYPE_FLOAT ); break; //case TYPE_COMPLEX: @@ -796,15 +807,13 @@ wxPGProperty* DIALOG_SIM_MODEL::newParamProperty( int aParamIndex ) const case SIM_VALUE::TYPE_STRING: if( param.info.enumValues.empty() ) { - prop = new SIM_STRING_PROPERTY( paramDescription, param.info.name, m_library, - curModelSharedPtr(), aParamIndex, - SIM_VALUE::TYPE_STRING ); + prop = new SIM_STRING_PROPERTY( paramDescription, param.info.name, curModel(), + aParamIndex, SIM_VALUE::TYPE_STRING ); } else { - prop = new SIM_ENUM_PROPERTY( paramDescription, param.info.name, m_library, - curModelSharedPtr(), aParamIndex, - SIM_VALUE::TYPE_STRING ); + prop = new SIM_ENUM_PROPERTY( paramDescription, param.info.name, curModel(), + aParamIndex, SIM_VALUE::TYPE_STRING ); } break; diff --git a/eeschema/sim/sim_property.cpp b/eeschema/sim/sim_property.cpp index 67177a2dd5..7e5580695e 100644 --- a/eeschema/sim/sim_property.cpp +++ b/eeschema/sim/sim_property.cpp @@ -251,25 +251,28 @@ wxTextEntry* SIM_STRING_VALIDATOR::getTextEntry() } -SIM_PROPERTY::SIM_PROPERTY( std::shared_ptr aLibrary, - std::shared_ptr aModel, - int aParamIndex ) - : m_library( std::move( aLibrary ) ), - m_model( std::move( aModel ) ), - m_paramIndex( aParamIndex ) +SIM_PROPERTY::SIM_PROPERTY( SIM_MODEL& aModel, int aParamIndex ) : + m_model( aModel ), + m_paramIndex( aParamIndex ), + m_disabled( false ) { } +void SIM_PROPERTY::Disable() +{ + m_disabled = true; +} + + SIM_BOOL_PROPERTY::SIM_BOOL_PROPERTY( const wxString& aLabel, const wxString& aName, - std::shared_ptr aLibrary, - std::shared_ptr aModel, + SIM_MODEL& aModel, int aParamIndex ) : wxBoolProperty( aLabel, aName ), - SIM_PROPERTY( aLibrary, aModel, aParamIndex ) + SIM_PROPERTY( aModel, aParamIndex ) { auto simValue = dynamic_cast*>( - m_model->GetParam( m_paramIndex ).value.get() ); + m_model.GetParam( m_paramIndex ).value.get() ); wxCHECK( simValue, /*void*/ ); @@ -287,26 +290,28 @@ void SIM_BOOL_PROPERTY::OnSetValue() { wxPGProperty::OnSetValue(); + if( m_disabled ) + return; + auto simValue = dynamic_cast*>( - m_model->GetParam( m_paramIndex ).value.get() ); + m_model.GetParam( m_paramIndex ).value.get() ); wxCHECK( simValue, /*void*/ ); - if( m_model->GetBaseModel() && *simValue == m_value.GetBool() ) - m_model->SetParamValue( m_paramIndex, "" ); + if( m_model.GetBaseModel() && *simValue == m_value.GetBool() ) + m_model.SetParamValue( m_paramIndex, "" ); else - m_model->SetParamValue( m_paramIndex, m_value.GetBool() ? "1" : "0" ); + m_model.SetParamValue( m_paramIndex, m_value.GetBool() ? "1" : "0" ); } SIM_STRING_PROPERTY::SIM_STRING_PROPERTY( const wxString& aLabel, const wxString& aName, - std::shared_ptr aLibrary, - std::shared_ptr aModel, + SIM_MODEL& aModel, int aParamIndex, SIM_VALUE::TYPE aValueType, SIM_VALUE_GRAMMAR::NOTATION aNotation ) : wxStringProperty( aLabel, aName ), - SIM_PROPERTY( aLibrary, aModel, aParamIndex ), + SIM_PROPERTY( aModel, aParamIndex ), m_valueType( aValueType ), m_notation( aNotation ) { @@ -323,15 +328,18 @@ wxValidator* SIM_STRING_PROPERTY::DoGetValidator() const bool SIM_STRING_PROPERTY::StringToValue( wxVariant& aVariant, const wxString& aText, int aArgFlags ) const { - wxString baseParamValue = m_model->GetBaseParam( m_paramIndex ).value->ToString(); + if( m_disabled ) + return false; + + wxString baseParamValue = m_model.GetBaseParam( m_paramIndex ).value->ToString(); aVariant = aText; // TODO: Don't use string comparison. - if( m_model->GetBaseModel() && ( aText == "" || aText == baseParamValue ) ) + if( m_model.GetBaseModel() && ( aText == "" || aText == baseParamValue ) ) { try { - m_model->SetParamValue( m_paramIndex, "" ); // Nullify. + m_model.SetParamValue( m_paramIndex, "" ); // Nullify. } catch( const IO_ERROR& ) { @@ -342,7 +350,7 @@ bool SIM_STRING_PROPERTY::StringToValue( wxVariant& aVariant, const wxString& aT } else { - m_model->SetParamValue( m_paramIndex, std::string( aText.ToUTF8() ) ); + m_model.SetParamValue( m_paramIndex, std::string( aText.ToUTF8() ) ); aVariant = GetParam().value->ToString(); } @@ -362,14 +370,13 @@ static wxArrayString convertStringsToWx( const std::vector& aString SIM_ENUM_PROPERTY::SIM_ENUM_PROPERTY( const wxString& aLabel, const wxString& aName, - std::shared_ptr aLibrary, - std::shared_ptr aModel, + SIM_MODEL& aModel, int aParamIndex, SIM_VALUE::TYPE aValueType, SIM_VALUE_GRAMMAR::NOTATION aNotation ) : wxEnumProperty( aLabel, aName, - convertStringsToWx( aModel->GetParam( aParamIndex ).info.enumValues ) ), - SIM_PROPERTY( aLibrary, aModel, aParamIndex ) + convertStringsToWx( aModel.GetParam( aParamIndex ).info.enumValues ) ), + SIM_PROPERTY( aModel, aParamIndex ) { auto it = std::find( GetParam().info.enumValues.begin(), GetParam().info.enumValues.end(), GetParam().value->ToString() ); @@ -381,6 +388,9 @@ SIM_ENUM_PROPERTY::SIM_ENUM_PROPERTY( const wxString& aLabel, const wxString& aN bool SIM_ENUM_PROPERTY::IntToValue( wxVariant& aVariant, int aNumber, int aArgFlags ) const { - m_model->SetParamValue( m_paramIndex, GetParam().info.enumValues.at( aNumber ) ); + if( m_disabled ) + return false; + + m_model.SetParamValue( m_paramIndex, GetParam().info.enumValues.at( aNumber ) ); return wxEnumProperty::IntToValue( aVariant, aNumber, aArgFlags ); } diff --git a/eeschema/sim/sim_property.h b/eeschema/sim/sim_property.h index 74ddab541a..e9450d5a49 100644 --- a/eeschema/sim/sim_property.h +++ b/eeschema/sim/sim_property.h @@ -82,16 +82,16 @@ private: class SIM_PROPERTY { public: - SIM_PROPERTY( std::shared_ptr aLibrary, std::shared_ptr aModel, - int aParamIndex ); + SIM_PROPERTY( SIM_MODEL& aModel, int aParamIndex ); - const SIM_MODEL::PARAM& GetParam() const { return m_model->GetParam( m_paramIndex ); } + void Disable(); + + const SIM_MODEL::PARAM& GetParam() const { return m_model.GetParam( m_paramIndex ); } protected: - std::shared_ptr m_library; // We hold a shared_ptr to SIM_LIBRARY to prevent its - // deallocation during this object's lifetime. - std::shared_ptr m_model; + SIM_MODEL& m_model; int m_paramIndex; + bool m_disabled; // If true, never access the models. }; @@ -99,8 +99,7 @@ class SIM_BOOL_PROPERTY : public wxBoolProperty, public SIM_PROPERTY { public: SIM_BOOL_PROPERTY( const wxString& aLabel, const wxString& aName, - std::shared_ptr aLibrary, - std::shared_ptr aModel, + SIM_MODEL& aModel, int aParamIndex ); wxValidator* DoGetValidator() const override; @@ -115,8 +114,7 @@ public: // We pass shared_ptrs because we need to make sure they are destroyed only after the last time // SIM_PROPERTY uses them. SIM_STRING_PROPERTY( const wxString& aLabel, const wxString& aName, - std::shared_ptr aLibrary, - std::shared_ptr aModel, + SIM_MODEL& aModel, int aParamIndex, SIM_VALUE::TYPE aValueType = SIM_VALUE::TYPE_FLOAT, SIM_VALUE_GRAMMAR::NOTATION aNotation = SIM_VALUE_GRAMMAR::NOTATION::SI ); @@ -136,8 +134,7 @@ class SIM_ENUM_PROPERTY : public wxEnumProperty, public SIM_PROPERTY { public: SIM_ENUM_PROPERTY( const wxString& aLabel, const wxString& aName, - std::shared_ptr aLibrary, - std::shared_ptr aModel, + SIM_MODEL& aModel, int aParamIndex, SIM_VALUE::TYPE aValueType = SIM_VALUE::TYPE_FLOAT, SIM_VALUE_GRAMMAR::NOTATION aNotation = SIM_VALUE_GRAMMAR::NOTATION::SI );