kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #20152
[PATCH] Clarify ownership semantics
SCREEN objects are typically owned by the DRAW_FRAME they are attached to.
SCH_SCREEN objects may additionally be shared between one DRAW_FRAME, and
multiple SCH_SHEET objects, so they are reference counted.
To make this distinction more obvious, and avoid potential errors, move the
pointer into the derived DRAW_FRAME, with an appropriate smart pointer
type. This also avoids a few C-style casts and replaces them by compile
time checks.
---
common/draw_frame.cpp | 6 +----
eeschema/class_sch_screen.h | 32 ++++++++++++++++++++------
eeschema/sch_base_frame.cpp | 5 ----
eeschema/sch_base_frame.h | 6 ++++-
eeschema/sch_screen.cpp | 2 ++
eeschema/sch_sheet.cpp | 46 ++++++++-----------------------------
eeschema/sch_sheet.h | 11 ++++-----
eeschema/sheet.cpp | 4 ++--
gerbview/gerbview_frame.h | 5 ++++
include/draw_frame.h | 6 +----
include/wxBasePcbFrame.h | 6 ++++-
pagelayout_editor/pl_editor_frame.h | 8 +++----
12 files changed, 64 insertions(+), 73 deletions(-)
diff --git a/common/draw_frame.cpp b/common/draw_frame.cpp
index 42936ef..c71cdaa 100644
--- a/common/draw_frame.cpp
+++ b/common/draw_frame.cpp
@@ -116,7 +116,6 @@ EDA_DRAW_FRAME::EDA_DRAW_FRAME( KIWAY* aKiway, wxWindow* aParent,
m_toolManager = NULL;
m_toolDispatcher = NULL;
m_messagePanel = NULL;
- m_currentScreen = NULL;
m_toolId = ID_NO_TOOL_SELECTED;
m_lastDrawToolId = ID_NO_TOOL_SELECTED;
m_showAxis = false; // true to draw axis.
@@ -189,9 +188,6 @@ EDA_DRAW_FRAME::~EDA_DRAW_FRAME()
delete m_toolDispatcher;
delete m_galCanvas;
- delete m_currentScreen;
- m_currentScreen = NULL;
-
m_auimgr.UnInit();
ReleaseFile();
@@ -528,7 +524,7 @@ wxPoint EDA_DRAW_FRAME::GetGridPosition( const wxPoint& aPosition ) const
{
wxPoint pos = aPosition;
- if( m_currentScreen != NULL && m_snapToGrid )
+ if( GetScreen() != NULL && m_snapToGrid )
pos = GetNearestGridPosition( aPosition );
return pos;
diff --git a/eeschema/class_sch_screen.h b/eeschema/class_sch_screen.h
index 67c6451..c05fe75 100644
--- a/eeschema/class_sch_screen.h
+++ b/eeschema/class_sch_screen.h
@@ -97,6 +97,15 @@ private:
*/
void addConnectedItemsToBlock( const wxPoint& aPosition );
+ ~SCH_SCREEN();
+
+ void DecRefCount();
+
+ void IncRefCount();
+
+ friend void intrusive_ptr_add_ref( SCH_SCREEN* );
+ friend void intrusive_ptr_release( SCH_SCREEN* );
+
public:
/**
@@ -104,7 +113,10 @@ public:
*/
SCH_SCREEN( KIWAY* aKiway );
- ~SCH_SCREEN();
+ bool IsShared() const
+ {
+ return m_refCount > 1;
+ }
virtual wxString GetClass() const
{
@@ -125,12 +137,6 @@ public:
//TITLE_BLOCK& GetTitleBlock() const { return (TITLE_BLOCK&) m_titles; }
void SetTitleBlock( const TITLE_BLOCK& aTitleBlock ) { m_titles = aTitleBlock; }
- void DecRefCount();
-
- void IncRefCount();
-
- int GetRefCount() const { return m_refCount; }
-
/**
* Function GetDrawItems().
* @return - A pointer to the first item in the linked list of draw items.
@@ -530,6 +536,18 @@ public:
};
+inline void intrusive_ptr_add_ref( SCH_SCREEN *a_screen )
+{
+ a_screen->IncRefCount();
+}
+
+
+inline void intrusive_ptr_release( SCH_SCREEN *a_screen )
+{
+ a_screen->DecRefCount();
+}
+
+
/**
* Class SCH_SCREENS
* is a container class that holds multiple SCH_SCREENs in a hierarchy.
diff --git a/eeschema/sch_base_frame.cpp b/eeschema/sch_base_frame.cpp
index 52898e8..fb5ed60 100644
--- a/eeschema/sch_base_frame.cpp
+++ b/eeschema/sch_base_frame.cpp
@@ -75,11 +75,6 @@ void SCH_BASE_FRAME::SetDrawBgColor( EDA_COLOR_T aColor)
}
-SCH_SCREEN* SCH_BASE_FRAME::GetScreen() const
-{
- return (SCH_SCREEN*) EDA_DRAW_FRAME::GetScreen();
-}
-
const wxString SCH_BASE_FRAME::GetZoomLevelIndicator() const
{
return EDA_DRAW_FRAME::GetZoomLevelIndicator();
diff --git a/eeschema/sch_base_frame.h b/eeschema/sch_base_frame.h
index c99e1a6..337f46b 100644
--- a/eeschema/sch_base_frame.h
+++ b/eeschema/sch_base_frame.h
@@ -27,6 +27,8 @@
#include <draw_frame.h>
#include <class_sch_screen.h>
+#include <boost/intrusive_ptr.hpp>
+
class PAGE_INFO;
class TITLE_BLOCK;
class LIB_VIEW_FRAME;
@@ -53,6 +55,7 @@ protected:
int m_repeatDeltaLabel; ///< the increment value of labels like bus members
///< when they are repeated
+ boost::intrusive_ptr<SCH_SCREEN> m_screen;
public:
SCH_BASE_FRAME( KIWAY* aKiway, wxWindow* aParent,
@@ -61,7 +64,8 @@ public:
const wxPoint& aPosition, const wxSize& aSize,
long aStyle, const wxString & aFrameName );
- SCH_SCREEN* GetScreen() const; // overload EDA_DRAW_FRAME
+ virtual SCH_SCREEN* GetScreen() const { return m_screen.get(); }
+ void SetScreen( SCH_SCREEN* a_screen ) { m_screen.reset( a_screen ); }
/**
* @return the increment value of the position of an item
diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp
index c2f8201..542e6c9 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -136,6 +136,8 @@ void SCH_SCREEN::DecRefCount()
wxCHECK_RET( m_refCount != 0,
wxT( "Screen reference count already zero. Bad programmer!" ) );
m_refCount--;
+ if( m_refCount == 0 )
+ delete this;
}
diff --git a/eeschema/sch_sheet.cpp b/eeschema/sch_sheet.cpp
index ddce303..7453c76 100644
--- a/eeschema/sch_sheet.cpp
+++ b/eeschema/sch_sheet.cpp
@@ -74,25 +74,12 @@ SCH_SHEET::SCH_SHEET( const SCH_SHEET& aSheet ) :
for( size_t i = 0; i < m_pins.size(); i++ )
m_pins[i].SetParent( this );
-
- if( m_screen )
- m_screen->IncRefCount();
}
SCH_SHEET::~SCH_SHEET()
{
-// wxLogDebug( wxT( "Destroying sheet " ) + m_name );
-
- // also, look at the associated sheet & its reference count
- // perhaps it should be deleted also.
- if( m_screen )
- {
- m_screen->DecRefCount();
-
- if( m_screen->GetRefCount() == 0 )
- delete m_screen;
- }
+ return;
}
@@ -104,33 +91,16 @@ EDA_ITEM* SCH_SHEET::Clone() const
void SCH_SHEET::SetScreen( SCH_SCREEN* aScreen )
{
- if( aScreen == m_screen )
- return;
-
- if( m_screen != NULL )
- {
- m_screen->DecRefCount();
-
- if( m_screen->GetRefCount() == 0 )
- {
- delete m_screen;
- m_screen = NULL;
- }
- }
-
m_screen = aScreen;
-
- if( m_screen )
- m_screen->IncRefCount();
}
-int SCH_SHEET::GetScreenCount() const
+bool SCH_SHEET::IsSharedScreen() const
{
if( m_screen == NULL )
return 0;
- return m_screen->GetRefCount();
+ return m_screen->IsShared();
}
@@ -702,10 +672,12 @@ bool SCH_SHEET::SearchHierarchy( const wxString& aFilename, SCH_SCREEN** aScreen
{
SCH_SHEET* sheet = (SCH_SHEET*) item;
- if( sheet->m_screen
- && sheet->m_screen->GetFileName().CmpNoCase( aFilename ) == 0 )
+ SCH_SCREEN* candidate_screen = sheet->GetScreen();
+
+ if( candidate_screen
+ && candidate_screen->GetFileName().CmpNoCase( aFilename ) == 0 )
{
- *aScreen = sheet->m_screen;
+ *aScreen = candidate_screen;
return true;
}
@@ -770,7 +742,7 @@ bool SCH_SHEET::Load( SCH_EDIT_FRAME* aFrame )
{
SetScreen( new SCH_SCREEN( &aFrame->Kiway() ) );
- success = aFrame->LoadOneEEFile( m_screen, m_fileName );
+ success = aFrame->LoadOneEEFile( m_screen.get(), m_fileName );
if( success )
{
diff --git a/eeschema/sch_sheet.h b/eeschema/sch_sheet.h
index 3304a0d..d3d0d34 100644
--- a/eeschema/sch_sheet.h
+++ b/eeschema/sch_sheet.h
@@ -31,6 +31,7 @@
#define SCH_SHEEET_H
#include <boost/ptr_container/ptr_vector.hpp>
+#include <boost/intrusive_ptr.hpp>
#include <boost/foreach.hpp>
#include <sch_text.h>
@@ -218,7 +219,7 @@ class SCH_SHEET : public SCH_ITEM
/// Screen that contains the physical data for the sheet. In complex hierarchies
/// multiple sheets can share a common screen.
- SCH_SCREEN* m_screen;
+ boost::intrusive_ptr<SCH_SCREEN> m_screen;
/// The list of sheet connection points.
SCH_SHEET_PINS m_pins;
@@ -282,7 +283,7 @@ public:
void SetFileNameSize( int aSize ) { m_fileNameSize = aSize; }
- SCH_SCREEN* GetScreen() { return m_screen; }
+ SCH_SCREEN* GetScreen() { return m_screen.get(); }
wxSize GetSize() { return m_size; }
@@ -303,11 +304,9 @@ public:
void SetScreen( SCH_SCREEN* aScreen );
/**
- * Function GetScreenCount
- * returns the number of times the associated screen for the sheet is being used. If
- * no screen is associated with the sheet, then zero is returned.
+ * Check whether the screen is shared
*/
- int GetScreenCount() const;
+ bool IsSharedScreen() const;
bool Save( FILE* aFile ) const;
diff --git a/eeschema/sheet.cpp b/eeschema/sheet.cpp
index d4fb0f2..6c98dc2 100644
--- a/eeschema/sheet.cpp
+++ b/eeschema/sheet.cpp
@@ -186,7 +186,7 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy )
}
else // Save to new file name.
{
- if( aSheet->GetScreenCount() > 1 )
+ if( aSheet->IsSharedScreen() )
{
msg += _( "This sheet uses shared data in a complex hierarchy.\n\n" );
msg += _( "Do you wish to convert it to a simple hierarchical sheet?" );
@@ -212,7 +212,7 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy )
// If the the associated screen is shared by more than one sheet, remove the
// screen and reload the file to a new screen. Failure to do this will trash
// the screen reference counting in complex hierarchies.
- if( aSheet->GetScreenCount() > 1 )
+ if( aSheet->IsSharedScreen() )
{
aSheet->SetScreen( NULL );
loadFromFile = true;
diff --git a/gerbview/gerbview_frame.h b/gerbview/gerbview_frame.h
index b33ac28..140fbfe 100644
--- a/gerbview/gerbview_frame.h
+++ b/gerbview/gerbview_frame.h
@@ -206,10 +206,15 @@ private:
// An array sting to store warning messages when reaging a gerber file.
wxArrayString m_Messages;
+ std::auto_ptr<GBR_SCREEN> m_screen;
+
public:
GERBVIEW_FRAME( KIWAY* aKiway, wxWindow* aParent );
~GERBVIEW_FRAME();
+ virtual GBR_SCREEN* GetScreen() const { return m_screen.get(); }
+ void SetScreen( GBR_SCREEN* a_screen ) { m_screen.reset( a_screen ); }
+
void OnCloseWindow( wxCloseEvent& Event );
bool OpenProjectFiles( const std::vector<wxString>& aFileSet, int aCtl ); // overload KIWAY_PLAYER
diff --git a/include/draw_frame.h b/include/draw_frame.h
index 6abdb05..3ca7b95 100644
--- a/include/draw_frame.h
+++ b/include/draw_frame.h
@@ -49,8 +49,6 @@ class EDA_DRAW_FRAME : public KIWAY_PLAYER
///< Id of active button on the vertical toolbar.
int m_toolId;
- BASE_SCREEN* m_currentScreen; ///< current used SCREEN
-
bool m_snapToGrid; ///< Indicates if cursor should be snapped to grid.
bool m_galCanvasActive; ///< whether to use new GAL engine
@@ -128,8 +126,6 @@ protected:
/// One-shot to avoid a recursive mouse event during hotkey movement
bool m_movingCursorWithKeyboard;
- void SetScreen( BASE_SCREEN* aScreen ) { m_currentScreen = aScreen; }
-
/**
* Function unitsChangeRefresh
* is called when when the units setting has changed to allow for any derived classes
@@ -301,7 +297,7 @@ public:
* derivatives. It is overloaded by derived classes to return
* SCH_SCREEN or PCB_SCREEN.
*/
- virtual BASE_SCREEN* GetScreen() const { return m_currentScreen; }
+ virtual BASE_SCREEN* GetScreen() const = 0;
/**
* Execute a remote command send via a socket to the application,
diff --git a/include/wxBasePcbFrame.h b/include/wxBasePcbFrame.h
index d860755..0ed7771 100644
--- a/include/wxBasePcbFrame.h
+++ b/include/wxBasePcbFrame.h
@@ -68,6 +68,9 @@ class TOOL_DISPATCHER;
*/
class PCB_BASE_FRAME : public EDA_DRAW_FRAME
{
+private:
+ std::auto_ptr<PCB_SCREEN> m_screen;
+
public:
DISPLAY_OPTIONS m_DisplayOptions;
EDA_UNITS_T m_UserGridUnit;
@@ -197,7 +200,8 @@ public:
virtual void SetToolID( int aId, int aCursor, const wxString& aToolMsg );
virtual void UpdateStatusBar();
- PCB_SCREEN* GetScreen() const { return (PCB_SCREEN*) EDA_DRAW_FRAME::GetScreen(); }
+ virtual PCB_SCREEN* GetScreen() const { return m_screen.get(); }
+ void SetScreen( PCB_SCREEN* a_screen ) { m_screen.reset( a_screen ); }
/**
* Function BestZoom
diff --git a/pagelayout_editor/pl_editor_frame.h b/pagelayout_editor/pl_editor_frame.h
index ba3b7ae..8622646 100644
--- a/pagelayout_editor/pl_editor_frame.h
+++ b/pagelayout_editor/pl_editor_frame.h
@@ -62,6 +62,8 @@ class PL_EDITOR_FRAME : public EDA_DRAW_FRAME
wxPoint m_grid_origin;
+ std::auto_ptr<PL_EDITOR_SCREEN> m_screen;
+
protected:
/// The last filename chosen to be proposed to the user
wxString m_lastFileName;
@@ -118,10 +120,8 @@ public:
*/
const wxString GetZoomLevelIndicator() const;
- PL_EDITOR_SCREEN* GetScreen() const // overload EDA_DRAW_FRAME
- {
- return (PL_EDITOR_SCREEN*) EDA_DRAW_FRAME::GetScreen();
- }
+ virtual PL_EDITOR_SCREEN* GetScreen() const { return m_screen.get(); }
+ void SetScreen( PL_EDITOR_SCREEN* a_screen ) { m_screen.reset( a_screen ); }
const wxPoint& GetAuxOrigin() const // overload EDA_DRAW_FRAME
{
References
