From 4e2d9007759faa0e35addf5f24e16f773b4e63f2 Mon Sep 17 00:00:00 2001 From: John Beard Date: Thu, 11 Apr 2019 12:06:50 +0100 Subject: [PATCH] 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 17e88d09442ed73a6ce89e9dd2ba48b83d203ac1) --- pcbnew/footprint_preview_panel.cpp | 25 +++++++++++++++++-------- pcbnew/footprint_preview_panel.h | 16 ++++++++++++---- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/pcbnew/footprint_preview_panel.cpp b/pcbnew/footprint_preview_panel.cpp index 4a871870bd..ddd8112041 100644 --- a/pcbnew/footprint_preview_panel.cpp +++ b/pcbnew/footprint_preview_panel.cpp @@ -245,10 +245,11 @@ public: FOOTPRINT_PREVIEW_PANEL::FOOTPRINT_PREVIEW_PANEL( KIWAY* aKiway, wxWindow* aParent, - KIGFX::GAL_DISPLAY_OPTIONS& aOpts, + std::unique_ptr aOpts, 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 ), + m_DisplayOptions( std::move( aOpts ) ), m_footprintDisplayed( true ) { m_iface = std::make_shared(); @@ -369,7 +370,6 @@ wxWindow* FOOTPRINT_PREVIEW_PANEL::GetWindow() FOOTPRINT_PREVIEW_PANEL* FOOTPRINT_PREVIEW_PANEL::New( KIWAY* aKiway, wxWindow* aParent ) { PCB_EDIT_FRAME* pcbnew = static_cast( aKiway->Player( FRAME_PCB, false ) ); - KIGFX::GAL_DISPLAY_OPTIONS gal_opts; wxConfigBase* cfg = Kiface().KifaceSettings(); wxConfigBase* commonCfg = Pgm().CommonSettings(); 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 // from PCBNew's config settings. + // We need a copy with a lifetime that matches the panel + std::unique_ptr gal_opts; 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( pcbnew->GetGalDisplayOptions() ); } 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(); + + 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 ); - 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 ); - gal_opts.cairo_antialiasing_mode = (KIGFX::CAIRO_ANTIALIASING_MODE) itemp; + gal_opts->cairo_antialiasing_mode = (KIGFX::CAIRO_ANTIALIASING_MODE) itemp; } #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 ); #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 ) { diff --git a/pcbnew/footprint_preview_panel.h b/pcbnew/footprint_preview_panel.h index b86d52ea0a..356719b2f8 100644 --- a/pcbnew/footprint_preview_panel.h +++ b/pcbnew/footprint_preview_panel.h @@ -77,9 +77,16 @@ private: FOOTPRINT_STATUS status; }; - FOOTPRINT_PREVIEW_PANEL( - KIWAY* aKiway, wxWindow* aParent, - KIGFX::GAL_DISPLAY_OPTIONS& aOpts, GAL_TYPE aGalType ); + /** + * Create a new panel + * + * @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 aOpts, GAL_TYPE aGalType ); virtual CACHE_ENTRY CacheAndReturn ( LIB_ID const& aFPID ); @@ -92,7 +99,8 @@ private: std::shared_ptr m_iface; FOOTPRINT_STATUS_HANDLER m_handler; std::unique_ptr m_dummyBoard; - std::unique_ptr m_colorsSettings; + std::unique_ptr m_DisplayOptions; + std::unique_ptr m_colorsSettings; LIB_ID m_currentFPID; bool m_footprintDisplayed;