haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Don't store document values in global PROPERTY_MANAGER. 

At best it leads to the wrong units being used if they're different
between (for instance) PCB Editor and Footprint Editor.  (And this
will only get worse if we ever to to a single binary.)

At worst it causes crashes when accessing freed ORIGIN_TRANSFORMS.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/16062
This commit is contained in:
Jeff Young 2023-11-10 18:36:02 +00:00
parent d41f4ec842
commit 9ef05fb762
5 changed files with 39 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
common/properties/pg_properties.cpp
View File

@ -123,13 +123,12 @@ wxPGProperty* PGPropertyFactory( const PROPERTY_BASE* aProperty, EDA_DRAW_FRAME*
switch( display )
{
case PROPERTY_DISPLAY::PT_SIZE:
ret = new PGPROPERTY_SIZE();
ret = new PGPROPERTY_SIZE( aFrame );
ret->SetEditor( PG_UNIT_EDITOR::BuildEditorName( aFrame ) );
break;
case PROPERTY_DISPLAY::PT_COORD:
ret = new PGPROPERTY_COORD();
static_cast<PGPROPERTY_COORD*>( ret )->SetCoordType( aProperty->CoordType() );
ret = new PGPROPERTY_COORD( aFrame, aProperty->CoordType() );
ret->SetEditor( PG_UNIT_EDITOR::BuildEditorName( aFrame ) );
break;
@ -209,8 +208,9 @@ wxPGProperty* PGPropertyFactory( const PROPERTY_BASE* aProperty, EDA_DRAW_FRAME*
}
PGPROPERTY_DISTANCE::PGPROPERTY_DISTANCE( const wxString& aRegEx,
PGPROPERTY_DISTANCE::PGPROPERTY_DISTANCE( EDA_DRAW_FRAME* aParentFrame, const wxString& aRegEx,
ORIGIN_TRANSFORMS::COORD_TYPES_T aCoordType ) :
m_parentFrame( aParentFrame ),
m_coordType( aCoordType )
{
m_regExValidator.reset( new REGEX_VALIDATOR( aRegEx ) );
@ -236,22 +236,21 @@ wxString PGPROPERTY_DISTANCE::DistanceToString( wxVariant& aVariant, int aArgFla
long distanceIU = aVariant.GetLong();
ORIGIN_TRANSFORMS* transforms = PROPERTY_MANAGER::Instance().GetTransforms();
const EDA_IU_SCALE* iuScale = PROPERTY_MANAGER::Instance().GetIuScale();
ORIGIN_TRANSFORMS& transforms = m_parentFrame->GetOriginTransforms();
const EDA_IU_SCALE& iuScale = m_parentFrame->GetIuScale();
if( transforms )
distanceIU = transforms->ToDisplay( static_cast<long long int>( distanceIU ), m_coordType );
distanceIU = transforms.ToDisplay( static_cast<long long int>( distanceIU ), m_coordType );
switch( PROPERTY_MANAGER::Instance().GetUnits() )
switch( m_parentFrame->GetUserUnits() )
{
case EDA_UNITS::INCHES:
return wxString::Format( wxS( "%g in" ), iuScale->IUToMils( distanceIU ) / 1000.0 );
return wxString::Format( wxS( "%g in" ), iuScale.IUToMils( distanceIU ) / 1000.0 );
case EDA_UNITS::MILS:
return wxString::Format( wxS( "%d mils" ), iuScale->IUToMils( distanceIU ) );
return wxString::Format( wxS( "%d mils" ), iuScale.IUToMils( distanceIU ) );
case EDA_UNITS::MILLIMETRES:
return wxString::Format( wxS( "%g mm" ), iuScale->IUTomm( distanceIU ) );
return wxString::Format( wxS( "%g mm" ), iuScale.IUTomm( distanceIU ) );
case EDA_UNITS::UNSCALED:
return wxString::Format( wxS( "%li" ), distanceIU );
@ -266,9 +265,9 @@ wxString PGPROPERTY_DISTANCE::DistanceToString( wxVariant& aVariant, int aArgFla
}
PGPROPERTY_SIZE::PGPROPERTY_SIZE( const wxString& aLabel, const wxString& aName,
long aValue )
: wxUIntProperty( aLabel, aName, aValue ), PGPROPERTY_DISTANCE( REGEX_UNSIGNED_DISTANCE )
PGPROPERTY_SIZE::PGPROPERTY_SIZE( EDA_DRAW_FRAME* aParentFrame ) :
wxUIntProperty( wxPG_LABEL, wxPG_LABEL, 0 ),
PGPROPERTY_DISTANCE( aParentFrame, REGEX_UNSIGNED_DISTANCE, ORIGIN_TRANSFORMS::NOT_A_COORD )
{
}
@ -279,10 +278,10 @@ wxValidator* PGPROPERTY_SIZE::DoGetValidator() const
}
PGPROPERTY_COORD::PGPROPERTY_COORD( const wxString& aLabel, const wxString& aName,
long aValue, ORIGIN_TRANSFORMS::COORD_TYPES_T aCoordType ) :
wxIntProperty( aLabel, aName, aValue ),
PGPROPERTY_DISTANCE( REGEX_SIGNED_DISTANCE, aCoordType )
PGPROPERTY_COORD::PGPROPERTY_COORD( EDA_DRAW_FRAME* aParentFrame,
ORIGIN_TRANSFORMS::COORD_TYPES_T aCoordType ) :
wxIntProperty( wxPG_LABEL, wxPG_LABEL, 0 ),
PGPROPERTY_DISTANCE( aParentFrame, REGEX_SIGNED_DISTANCE, aCoordType )
{
}

16
common/widgets/properties_panel.cpp
View File

@ -166,13 +166,10 @@ void PROPERTIES_PANEL::rebuildProperties( const SELECTION& aSelection )
wxCHECK( !types.empty(), /* void */ );
PROPERTY_MANAGER& propMgr = PROPERTY_MANAGER::Instance();
propMgr.SetUnits( m_frame->GetUserUnits() );
propMgr.SetIuScale( &m_frame->GetIuScale() );
propMgr.SetTransforms( &m_frame->GetOriginTransforms() );
PROPERTY_MANAGER& propMgr = PROPERTY_MANAGER::Instance();
std::set<PROPERTY_BASE*> commonProps;
const PROPERTY_LIST& allProperties = propMgr.GetProperties( *types.begin() );
const PROPERTY_LIST& allProperties = propMgr.GetProperties( *types.begin() );
copy( allProperties.begin(), allProperties.end(), inserter( commonProps, commonProps.begin() ) );
PROPERTY_DISPLAY_ORDER displayOrder = propMgr.GetDisplayOrder( *types.begin() );
@ -346,11 +343,8 @@ bool PROPERTIES_PANEL::extractValueAndWritability( const SELECTION& aSelection,
wxVariant& aValue, bool& aWritable )
{
PROPERTY_MANAGER& propMgr = PROPERTY_MANAGER::Instance();
propMgr.SetUnits( m_frame->GetUserUnits() );
propMgr.SetTransforms( &m_frame->GetOriginTransforms() );
bool different = false;
wxVariant commonVal;
bool different = false;
wxVariant commonVal;
aWritable = true;

30
include/properties/pg_properties.h
View File

@ -41,17 +41,18 @@ wxPGProperty* PGPropertyFactory( const PROPERTY_BASE* aProperty, EDA_DRAW_FRAME*
class PGPROPERTY_DISTANCE
{
public:
PGPROPERTY_DISTANCE( const wxString& aRegEx,
ORIGIN_TRANSFORMS::COORD_TYPES_T aCoordType = ORIGIN_TRANSFORMS::NOT_A_COORD );
PGPROPERTY_DISTANCE( EDA_DRAW_FRAME* aParentFrame, const wxString& aRegEx,
ORIGIN_TRANSFORMS::COORD_TYPES_T aCoordType );
virtual ~PGPROPERTY_DISTANCE() = 0;
void SetCoordType( ORIGIN_TRANSFORMS::COORD_TYPES_T aType ) { m_coordType = aType; }
ORIGIN_TRANSFORMS::COORD_TYPES_T CoordType() const { return m_coordType; }
protected:
bool StringToDistance( wxVariant& aVariant, const wxString& aText, int aArgFlags = 0 ) const;
wxString DistanceToString( wxVariant& aVariant, int aArgFlags = 0 ) const;
protected:
EDA_DRAW_FRAME* m_parentFrame;
std::unique_ptr<REGEX_VALIDATOR> m_regExValidator;
ORIGIN_TRANSFORMS::COORD_TYPES_T m_coordType;
};
@ -60,8 +61,7 @@ protected:
class PGPROPERTY_SIZE : public wxUIntProperty, public PGPROPERTY_DISTANCE
{
public:
PGPROPERTY_SIZE( const wxString& aLabel = wxPG_LABEL, const wxString& aName = wxPG_LABEL,
long aValue = 0 );
PGPROPERTY_SIZE( EDA_DRAW_FRAME* aParentFrame );
bool StringToValue( wxVariant& aVariant, const wxString& aText,
int aArgFlags = 0 ) const override
@ -81,9 +81,7 @@ public:
class PGPROPERTY_COORD : public wxIntProperty, public PGPROPERTY_DISTANCE
{
public:
PGPROPERTY_COORD( const wxString& aLabel = wxPG_LABEL, const wxString& aName = wxPG_LABEL,
long aValue = 0,
ORIGIN_TRANSFORMS::COORD_TYPES_T aCoordType = ORIGIN_TRANSFORMS::NOT_A_COORD );
PGPROPERTY_COORD( EDA_DRAW_FRAME* aParentFrame, ORIGIN_TRANSFORMS::COORD_TYPES_T aCoordType );
bool StringToValue( wxVariant& aVariant, const wxString& aText,
int aArgFlags = 0 ) const override
@ -104,9 +102,9 @@ public:
class PGPROPERTY_ANGLE : public wxFloatProperty
{
public:
PGPROPERTY_ANGLE( const wxString& aLabel = wxPG_LABEL, const wxString& aName = wxPG_LABEL,
double aValue = 0 )
: wxFloatProperty( aLabel, aName, aValue ), m_scale( 1.0 )
PGPROPERTY_ANGLE() :
wxFloatProperty( wxPG_LABEL, wxPG_LABEL, 0 ),
m_scale( 1.0 )
{
}
@ -134,9 +132,8 @@ protected:
class PGPROPERTY_COLORENUM : public wxEnumProperty
{
public:
PGPROPERTY_COLORENUM( const wxString& aLabel, const wxString& aName, wxPGChoices* aChoices,
int aValue = 0 ) :
wxEnumProperty( aLabel, aName, *aChoices, aValue ),
PGPROPERTY_COLORENUM( wxPGChoices* aChoices ) :
wxEnumProperty( wxPG_LABEL, wxPG_LABEL, *aChoices, 0 ),
m_colorFunc( []( int aDummy ) { return wxNullColour; } )
{
SetFlag( wxPG_PROP_CUSTOMIMAGE );
@ -164,9 +161,8 @@ protected:
class PGPROPERTY_STRING : public wxStringProperty
{
public:
PGPROPERTY_STRING( const wxString& aLabel = wxPG_LABEL, const wxString& aName = wxPG_LABEL,
const wxString& aValue = wxEmptyString ) :
wxStringProperty( aLabel, aName, aValue )
PGPROPERTY_STRING() :
wxStringProperty( wxPG_LABEL, wxPG_LABEL, wxEmptyString )
{}
virtual ~PGPROPERTY_STRING() = default;

29
include/properties/property_mgr.h
View File

@ -2,7 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2020 CERN
* Copyright (C) 2021 KiCad Developers, see AUTHORS.txt for contributors.
* Copyright (C) 2021-2023 KiCad Developers, see AUTHORS.txt for contributors.
*
* @author Tomasz Wlostowski <tomasz.wlostowski@cern.ch>
* @author Maciej Suminski <maciej.suminski@cern.ch>
@ -230,22 +230,6 @@ public:
*/
bool IsOfType( TYPE_ID aDerived, TYPE_ID aBase ) const;
EDA_UNITS GetUnits() const
{
return m_units;
}
void SetUnits( EDA_UNITS aUnits )
{
m_units = aUnits;
}
ORIGIN_TRANSFORMS* GetTransforms() const { return m_originTransforms; }
void SetTransforms( ORIGIN_TRANSFORMS* aTransforms ) { m_originTransforms = aTransforms; }
const EDA_IU_SCALE* GetIuScale() const { return m_iuScale; }
void SetIuScale( const EDA_IU_SCALE* aScale ) { m_iuScale = aScale; }
/**
* Rebuild the list of all registered properties. Needs to be called
* once before GetProperty()/GetProperties() are used.
@ -267,10 +251,7 @@ public:
private:
PROPERTY_MANAGER() :
m_dirty( false ),
m_units( EDA_UNITS::MILLIMETRES ),
m_originTransforms( nullptr ),
m_iuScale( &pcbIUScale )
m_dirty( false )
{
}
@ -343,12 +324,6 @@ private:
/// Flag indicating that the list of properties needs to be rebuild (RebuildProperties())
bool m_dirty;
EDA_UNITS m_units;
ORIGIN_TRANSFORMS* m_originTransforms;
const EDA_IU_SCALE* m_iuScale;
};

3
pcbnew/widgets/pcb_properties_panel.cpp
View File

@ -135,8 +135,7 @@ wxPGProperty* PCB_PROPERTIES_PANEL::createPGProperty( const PROPERTY_BASE* aProp
boardLayerIDs.push_back( canonicalLayers.GetValue( ii ) );
}
auto ret = new PGPROPERTY_COLORENUM( wxPG_LABEL, wxPG_LABEL,
new wxPGChoices( boardLayerNames, boardLayerIDs ) );
auto ret = new PGPROPERTY_COLORENUM( new wxPGChoices( boardLayerNames, boardLayerIDs ) );
ret->SetColorFunc(
[&]( int aValue ) -> wxColour