Pcbnew: FOOTPRINT_PREVIEW_PANEL passes reference to local

Previously, the GAL_DISPLAY_OPTIONS object in FOOTPRINT_PREVIEW_PANEL::New
was passed by reference in the ctor, down to EDA_DRAW_PANEL_GAL, which stored
it as a reference. The object in New() then goes out of scope, so the
referencing panel outlives the options.

Fix this by making a copy in a std::unique_ptr of the options, and giving
ownership to the panel.

There is another issue here: when the Pcbnew options are copies, the
OBSERVABLE subscriber list is copied too. This means if the panel called
NotifyChanged() on the options, Pcbnew would get updates, even though a copy
of the options changed. However, the panel doesn't change the options or
notify, so it's OK for now.

(cherry picked from commit 17e88d0944)
This commit is contained in:
John Beard 2019-04-11 12:06:50 +01:00
parent 4dae82054e
commit 4e2d900775
2 changed files with 29 additions and 12 deletions

View File

@ -245,10 +245,11 @@ public:
FOOTPRINT_PREVIEW_PANEL::FOOTPRINT_PREVIEW_PANEL( KIWAY* aKiway, wxWindow* aParent, FOOTPRINT_PREVIEW_PANEL::FOOTPRINT_PREVIEW_PANEL( KIWAY* aKiway, wxWindow* aParent,
KIGFX::GAL_DISPLAY_OPTIONS& aOpts, std::unique_ptr<KIGFX::GAL_DISPLAY_OPTIONS> aOpts,
GAL_TYPE aGalType ) GAL_TYPE aGalType )
: PCB_DRAW_PANEL_GAL ( aParent, -1, wxPoint( 0, 0 ), wxSize(200, 200), aOpts, aGalType ), : PCB_DRAW_PANEL_GAL ( aParent, -1, wxPoint( 0, 0 ), wxSize(200, 200), *aOpts, aGalType ),
KIWAY_HOLDER( aKiway ), KIWAY_HOLDER( aKiway ),
m_DisplayOptions( std::move( aOpts ) ),
m_footprintDisplayed( true ) m_footprintDisplayed( true )
{ {
m_iface = std::make_shared<FP_THREAD_IFACE>(); m_iface = std::make_shared<FP_THREAD_IFACE>();
@ -369,7 +370,6 @@ wxWindow* FOOTPRINT_PREVIEW_PANEL::GetWindow()
FOOTPRINT_PREVIEW_PANEL* FOOTPRINT_PREVIEW_PANEL::New( KIWAY* aKiway, wxWindow* aParent ) FOOTPRINT_PREVIEW_PANEL* FOOTPRINT_PREVIEW_PANEL::New( KIWAY* aKiway, wxWindow* aParent )
{ {
PCB_EDIT_FRAME* pcbnew = static_cast<PCB_EDIT_FRAME*>( aKiway->Player( FRAME_PCB, false ) ); PCB_EDIT_FRAME* pcbnew = static_cast<PCB_EDIT_FRAME*>( aKiway->Player( FRAME_PCB, false ) );
KIGFX::GAL_DISPLAY_OPTIONS gal_opts;
wxConfigBase* cfg = Kiface().KifaceSettings(); wxConfigBase* cfg = Kiface().KifaceSettings();
wxConfigBase* commonCfg = Pgm().CommonSettings(); wxConfigBase* commonCfg = Pgm().CommonSettings();
bool btemp; bool btemp;
@ -379,20 +379,29 @@ FOOTPRINT_PREVIEW_PANEL* FOOTPRINT_PREVIEW_PANEL::New( KIWAY* aKiway, wxWindow*
// Fetch grid & display settings from PCBNew if it's running; otherwise fetch them // Fetch grid & display settings from PCBNew if it's running; otherwise fetch them
// from PCBNew's config settings. // from PCBNew's config settings.
// We need a copy with a lifetime that matches the panel
std::unique_ptr<KIGFX::GAL_DISPLAY_OPTIONS> gal_opts;
if( pcbnew ) if( pcbnew )
{ {
gal_opts = pcbnew->GetGalDisplayOptions(); // Copy the existing Pcbnew options
// REVIEW: This also copies the current subscription list of the options
// to the new options. This is probably not what is intended, but because
// this widget doesn't change the options it should be OK.
gal_opts = std::make_unique<KIGFX::GAL_DISPLAY_OPTIONS>( pcbnew->GetGalDisplayOptions() );
} }
else else
{ {
gal_opts.ReadConfig( cfg, wxString( PCB_EDIT_FRAME_NAME ) + GAL_DISPLAY_OPTIONS_KEY ); // Make and populate a new one from config
gal_opts = std::make_unique<KIGFX::GAL_DISPLAY_OPTIONS>();
gal_opts->ReadConfig( cfg, wxString( PCB_EDIT_FRAME_NAME ) + GAL_DISPLAY_OPTIONS_KEY );
commonCfg->Read( GAL_ANTIALIASING_MODE_KEY, &itemp, (int) KIGFX::OPENGL_ANTIALIASING_MODE::NONE ); commonCfg->Read( GAL_ANTIALIASING_MODE_KEY, &itemp, (int) KIGFX::OPENGL_ANTIALIASING_MODE::NONE );
gal_opts.gl_antialiasing_mode = (KIGFX::OPENGL_ANTIALIASING_MODE) itemp; gal_opts->gl_antialiasing_mode = (KIGFX::OPENGL_ANTIALIASING_MODE) itemp;
commonCfg->Read( CAIRO_ANTIALIASING_MODE_KEY, &itemp, (int) KIGFX::CAIRO_ANTIALIASING_MODE::NONE ); commonCfg->Read( CAIRO_ANTIALIASING_MODE_KEY, &itemp, (int) KIGFX::CAIRO_ANTIALIASING_MODE::NONE );
gal_opts.cairo_antialiasing_mode = (KIGFX::CAIRO_ANTIALIASING_MODE) itemp; gal_opts->cairo_antialiasing_mode = (KIGFX::CAIRO_ANTIALIASING_MODE) itemp;
} }
#ifdef __WXMAC__ #ifdef __WXMAC__
@ -404,7 +413,7 @@ FOOTPRINT_PREVIEW_PANEL* FOOTPRINT_PREVIEW_PANEL::New( KIWAY* aKiway, wxWindow*
cfg->ReadLong( CanvasTypeKeyBase, EDA_DRAW_PANEL_GAL::GAL_TYPE_CAIRO ); cfg->ReadLong( CanvasTypeKeyBase, EDA_DRAW_PANEL_GAL::GAL_TYPE_CAIRO );
#endif #endif
auto panel = new FOOTPRINT_PREVIEW_PANEL( aKiway, aParent, gal_opts, canvasType ); auto panel = new FOOTPRINT_PREVIEW_PANEL( aKiway, aParent, std::move( gal_opts ), canvasType );
if( pcbnew ) if( pcbnew )
{ {

View File

@ -77,9 +77,16 @@ private:
FOOTPRINT_STATUS status; FOOTPRINT_STATUS status;
}; };
FOOTPRINT_PREVIEW_PANEL( /**
KIWAY* aKiway, wxWindow* aParent, * Create a new panel
KIGFX::GAL_DISPLAY_OPTIONS& aOpts, GAL_TYPE aGalType ); *
* @param aKiway the connected KIWAY
* @param aParent the owning WX window
* @param aOpts the GAL options (ownership is assumed)
* @param aGalType the displayed GAL type
*/
FOOTPRINT_PREVIEW_PANEL( KIWAY* aKiway, wxWindow* aParent,
std::unique_ptr<KIGFX::GAL_DISPLAY_OPTIONS> aOpts, GAL_TYPE aGalType );
virtual CACHE_ENTRY CacheAndReturn ( LIB_ID const& aFPID ); virtual CACHE_ENTRY CacheAndReturn ( LIB_ID const& aFPID );
@ -92,6 +99,7 @@ private:
std::shared_ptr<FP_THREAD_IFACE> m_iface; std::shared_ptr<FP_THREAD_IFACE> m_iface;
FOOTPRINT_STATUS_HANDLER m_handler; FOOTPRINT_STATUS_HANDLER m_handler;
std::unique_ptr<BOARD> m_dummyBoard; std::unique_ptr<BOARD> m_dummyBoard;
std::unique_ptr<KIGFX::GAL_DISPLAY_OPTIONS> m_DisplayOptions;
std::unique_ptr<COLORS_DESIGN_SETTINGS> m_colorsSettings; std::unique_ptr<COLORS_DESIGN_SETTINGS> m_colorsSettings;
LIB_ID m_currentFPID; LIB_ID m_currentFPID;