BUG FIX: eeschema as segfaulting on the 'Insert' key because the m_itemToRepeat

was simply a pointer to an object on the display list.  At times this object
would disappear from the display list, in the test case because of a concatonation
of two wires, and if you then tried to clone the non-existent object you'd get a 
crash.  This was not merely a bug, but a naive design choice. IMO.
Now the item to repeat is cloned, so will never also be on the display list.
This commit is contained in:
Dick Hollenbeck 2013-08-05 16:02:41 -05:00
parent 7377cb541f
commit c0832a0342
13 changed files with 121 additions and 71 deletions

View File

@ -145,9 +145,11 @@ EDA_DRAW_PANEL::~EDA_DRAW_PANEL()
wxGetApp().GetSettings()->Write( ENBL_AUTO_PAN_KEY, m_enableAutoPan );
}
EDA_DRAW_FRAME* EDA_DRAW_PANEL::GetParent()
{
return ( EDA_DRAW_FRAME* ) wxWindow::GetParent();
wxWindow* mom = wxScrolledWindow::GetParent();
return (EDA_DRAW_FRAME*) mom;
}

View File

@ -162,7 +162,7 @@ void SCH_EDIT_FRAME::BeginSegment( wxDC* DC, int type )
}
m_canvas->SetMouseCapture( DrawSegment, AbortCreateNewLine );
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
}
else // A segment is in progress: terminates the current segment and add a new segment.
{
@ -239,7 +239,8 @@ void SCH_EDIT_FRAME::EndSegment( wxDC* DC )
return;
// Get the last non-null wire (this is the last created segment).
m_itemToRepeat = segment = (SCH_LINE*) s_wires.GetLast();
SetRepeatItem( segment = (SCH_LINE*) s_wires.GetLast() );
screen->SetCurItem( NULL );
m_canvas->EndMouseCapture( -1, -1, wxEmptyString, false );
@ -341,7 +342,7 @@ void SCH_EDIT_FRAME::DeleteCurrentSegment( wxDC* DC )
{
SCH_SCREEN* screen = GetScreen();
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
if( ( screen->GetCurItem() == NULL ) || !screen->GetCurItem()->IsNew() )
return;
@ -359,7 +360,7 @@ SCH_JUNCTION* SCH_EDIT_FRAME::AddJunction( wxDC* aDC, const wxPoint& aPosition,
{
SCH_JUNCTION* junction = new SCH_JUNCTION( aPosition );
m_itemToRepeat = junction;
SetRepeatItem( junction );
m_canvas->CrossHairOff( aDC ); // Erase schematic cursor
junction->Draw( m_canvas, aDC, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
@ -378,19 +379,19 @@ SCH_JUNCTION* SCH_EDIT_FRAME::AddJunction( wxDC* aDC, const wxPoint& aPosition,
SCH_NO_CONNECT* SCH_EDIT_FRAME::AddNoConnect( wxDC* aDC, const wxPoint& aPosition )
{
SCH_NO_CONNECT* NewNoConnect;
SCH_NO_CONNECT* no_connect = new SCH_NO_CONNECT( aPosition );
NewNoConnect = new SCH_NO_CONNECT( aPosition );
m_itemToRepeat = NewNoConnect;
SetRepeatItem( no_connect );
m_canvas->CrossHairOff( aDC ); // Erase schematic cursor
NewNoConnect->Draw( m_canvas, aDC, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
no_connect->Draw( m_canvas, aDC, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
m_canvas->CrossHairOn( aDC ); // Display schematic cursor
GetScreen()->Append( NewNoConnect );
GetScreen()->Append( no_connect );
OnModify();
SaveCopyInUndoList( NewNoConnect, UR_NEW );
return NewNoConnect;
SaveCopyInUndoList( no_connect, UR_NEW );
return no_connect;
}
@ -420,35 +421,46 @@ static void AbortCreateNewLine( EDA_DRAW_PANEL* Panel, wxDC* DC )
void SCH_EDIT_FRAME::RepeatDrawItem( wxDC* DC )
{
if( m_itemToRepeat == NULL )
SCH_ITEM* repeater = GetRepeatItem();
if( !repeater )
return;
m_itemToRepeat = (SCH_ITEM*) m_itemToRepeat->Clone();
//D( repeater>Show( 0, std::cout ); )
if( m_itemToRepeat->Type() == SCH_COMPONENT_T ) // If repeat component then put in move mode
// clone the repeater, move it, insert into display list, then save a copy
// via SetRepeatItem();
SCH_ITEM* my_clone = (SCH_ITEM*) repeater->Clone();
// If cloning a component then put into 'move' mode.
/* please tell me where components are snapshotted via SetRepeatItem()
if( my_clone->Type() == SCH_COMPONENT_T )
{
wxPoint pos = GetCrossHairPosition() -
( (SCH_COMPONENT*) m_itemToRepeat )->GetPosition();
( (SCH_COMPONENT*) my_clone )->GetPosition();
m_itemToRepeat->SetFlags( IS_NEW );
( (SCH_COMPONENT*) m_itemToRepeat )->SetTimeStamp( GetNewTimeStamp() );
m_itemToRepeat->Move( pos );
m_itemToRepeat->Draw( m_canvas, DC, wxPoint( 0, 0 ), g_XorMode );
MoveItem( m_itemToRepeat, DC );
return;
my_clone->SetFlags( IS_NEW );
( (SCH_COMPONENT*) my_clone )->SetTimeStamp( GetNewTimeStamp() );
my_clone->Move( pos );
my_clone->Draw( m_canvas, DC, wxPoint( 0, 0 ), g_XorMode );
MoveItem( my_clone, DC );
}
m_itemToRepeat->Move( wxPoint( g_RepeatStep.GetWidth(), g_RepeatStep.GetHeight() ) );
if( m_itemToRepeat->CanIncrementLabel() )
( (SCH_TEXT*) m_itemToRepeat )->IncrementLabel();
if( m_itemToRepeat )
else
*/
{
GetScreen()->Append( m_itemToRepeat );
my_clone->Move( wxPoint( g_RepeatStep.GetWidth(), g_RepeatStep.GetHeight() ) );
if( my_clone->CanIncrementLabel() )
( (SCH_TEXT*) my_clone )->IncrementLabel();
GetScreen()->Append( my_clone );
GetScreen()->TestDanglingEnds();
m_itemToRepeat->Draw( m_canvas, DC, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
SaveCopyInUndoList( m_itemToRepeat, UR_NEW );
m_itemToRepeat->ClearFlags();
my_clone->Draw( m_canvas, DC, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
SaveCopyInUndoList( my_clone, UR_NEW );
my_clone->ClearFlags();
}
// clone my_clone, now that it has been moved, thus saving new position.
SetRepeatItem( my_clone );
}

View File

@ -144,7 +144,7 @@ void SCH_EDIT_FRAME::MoveImage( SCH_BITMAP* aImageItem, wxDC* aDC )
m_canvas->SetMouseCapture( moveBitmap, abortMoveBitmap );
GetScreen()->SetCurItem( aImageItem );
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
SetUndoItem( aImageItem );

View File

@ -72,7 +72,7 @@ SCH_TEXT* SCH_EDIT_FRAME::CreateNewText( wxDC* aDC, int aType )
{
SCH_TEXT* textItem = NULL;
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
switch( aType )
{
@ -235,7 +235,7 @@ void SCH_EDIT_FRAME::OnConvertTextType( wxCommandEvent& aEvent )
}
}
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
OnModify();
newtext->Draw( m_canvas, &dc, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
m_canvas->CrossHairOn( &dc ); // redraw schematic cursor

View File

@ -199,7 +199,7 @@ SCH_COMPONENT* SCH_EDIT_FRAME::Load_Component( wxDC* aDC,
{
int unit = 1;
int convert = 1;
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
m_canvas->SetIgnoreMouseEvents( true );
wxString Name = SelectComponentFromLibrary( aLibname, aHistoryList, aUseLibBrowser,

View File

@ -268,7 +268,7 @@ void HIERARCHY_NAVIG_DLG::OnSelect( wxTreeEvent& event )
void SCH_EDIT_FRAME::DisplayCurrentSheet()
{
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
ClearMsgPanel();
SCH_SCREEN* screen = m_CurrentSheet->LastScreen();

View File

@ -385,7 +385,7 @@ void SCH_EDIT_FRAME::OnHotKey( wxDC* aDC, int aHotKey, const wxPoint& aPosition,
break;
case HK_REPEAT_LAST:
if( notBusy && m_itemToRepeat && ( m_itemToRepeat->GetFlags() == 0 ) )
if( notBusy && GetRepeatItem() && ( GetRepeatItem()->GetFlags() == 0 ) )
RepeatDrawItem( aDC );
break;

View File

@ -58,7 +58,7 @@ void SCH_EDIT_FRAME::OnLeftClick( wxDC* aDC, const wxPoint& aPosition )
if( ( GetToolId() == ID_NO_TOOL_SELECTED ) || ( item && item->GetFlags() ) )
{
m_canvas->SetAutoPanRequest( false );
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
if( item && item->GetFlags() )
{
@ -128,8 +128,9 @@ void SCH_EDIT_FRAME::OnLeftClick( wxDC* aDC, const wxPoint& aPosition )
{
if( false == GetScreen()->GetItem( gridPosition, 0, SCH_NO_CONNECT_T ) )
{
m_itemToRepeat = AddNoConnect( aDC, gridPosition );
GetScreen()->SetCurItem( m_itemToRepeat );
SCH_NO_CONNECT* no_connect = AddNoConnect( aDC, gridPosition );
SetRepeatItem( no_connect );
GetScreen()->SetCurItem( no_connect );
m_canvas->SetAutoPanRequest( true );
}
}
@ -137,7 +138,6 @@ void SCH_EDIT_FRAME::OnLeftClick( wxDC* aDC, const wxPoint& aPosition )
{
addCurrentItemToList( aDC );
}
break;
case ID_JUNCTION_BUTT:
@ -145,8 +145,9 @@ void SCH_EDIT_FRAME::OnLeftClick( wxDC* aDC, const wxPoint& aPosition )
{
if( false == GetScreen()->GetItem( gridPosition, 0, SCH_JUNCTION_T ) )
{
m_itemToRepeat = AddJunction( aDC, gridPosition, true );
GetScreen()->SetCurItem( m_itemToRepeat );
SCH_JUNCTION* junction = AddJunction( aDC, gridPosition, true );
SetRepeatItem( junction );
GetScreen()->SetCurItem( junction );
m_canvas->SetAutoPanRequest( true );
}
}
@ -154,7 +155,6 @@ void SCH_EDIT_FRAME::OnLeftClick( wxDC* aDC, const wxPoint& aPosition )
{
addCurrentItemToList( aDC );
}
break;
case ID_WIRETOBUS_ENTRY_BUTT:
@ -168,6 +168,7 @@ void SCH_EDIT_FRAME::OnLeftClick( wxDC* aDC, const wxPoint& aPosition )
addCurrentItemToList( aDC );
}
break;
case ID_BUSTOBUS_ENTRY_BUTT:
if( ( item == NULL ) || ( item->GetFlags() == 0 ) )
{

View File

@ -119,7 +119,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
{
case ID_HIERARCHY:
InstallHierarchyFrame( &dc, pos );
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
break;
case wxID_CUT:
@ -127,7 +127,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
break;
HandleBlockEndByPopUp( BLOCK_DELETE, &dc );
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
SetSheetNumberAndCount();
break;
@ -182,7 +182,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
m_canvas->MoveCursorToCrossHair();
DeleteConnection( id == ID_POPUP_SCH_DELETE_CONNECTION );
screen->SetCurItem( NULL );
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
screen->TestDanglingEnds( m_canvas, &dc );
m_canvas->Refresh();
break;
@ -222,7 +222,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
DeleteItem( item );
screen->SetCurItem( NULL );
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
screen->TestDanglingEnds( m_canvas, &dc );
SetSheetNumberAndCount();
OnModify();
@ -375,7 +375,7 @@ void SCH_EDIT_FRAME::Process_Special_Functions( wxCommandEvent& event )
// End switch ( id ) (Command execution)
if( GetToolId() == ID_NO_TOOL_SELECTED )
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
}
@ -445,7 +445,7 @@ void SCH_EDIT_FRAME::OnMoveItem( wxCommandEvent& aEvent )
}
if( GetToolId() == ID_NO_TOOL_SELECTED )
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
}
@ -561,7 +561,7 @@ void SCH_EDIT_FRAME::OnSelectTool( wxCommandEvent& aEvent )
break;
default:
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
}
// Simulate left click event if we got here from a hot key.
@ -695,7 +695,7 @@ void SCH_EDIT_FRAME::MoveItem( SCH_ITEM* aItem, wxDC* aDC )
{
wxCHECK_RET( aItem != NULL, wxT( "Cannot move invalid schematic item" ) );
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
if( !aItem->IsNew() )
{

View File

@ -177,7 +177,8 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( wxWindow* aParent, const wxString& aTitle,
const wxPoint& aPosition, const wxSize& aSize,
long aStyle ) :
SCH_BASE_FRAME( aParent, SCHEMATIC_FRAME_TYPE, aTitle, aPosition, aSize,
aStyle, SCH_EDIT_FRAME_NAME )
aStyle, SCH_EDIT_FRAME_NAME ),
m_item_to_repeat( 0 )
{
m_FrameName = SCH_EDIT_FRAME_NAME;
m_showAxis = false; // true to show axis
@ -207,8 +208,6 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( wxWindow* aParent, const wxString& aTitle,
icon.CopyFromBitmap( KiBitmap( icon_eeschema_xpm ) );
SetIcon( icon );
m_itemToRepeat = NULL;
/* Get config */
LoadSettings();
@ -268,6 +267,8 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( wxWindow* aParent, const wxString& aTitle,
SCH_EDIT_FRAME::~SCH_EDIT_FRAME()
{
delete m_item_to_repeat; // we own the cloned object, see this->SetRepeatItem()
SetScreen( NULL );
delete m_CurrentSheet; // a SCH_SHEET_PATH, on the heap.
delete m_undoItem;
@ -281,6 +282,27 @@ SCH_EDIT_FRAME::~SCH_EDIT_FRAME()
}
void SCH_EDIT_FRAME::SetRepeatItem( SCH_ITEM* aItem )
{
// we cannot store a pointer to an item in the display list here since
// that item may be deleted, such as part of a line concatonation or other.
// So simply always keep a copy of the object which is to be repeated.
SCH_ITEM* old = m_item_to_repeat;
SCH_ITEM* cur = aItem;
if( cur != old )
{
if( cur )
aItem = (SCH_ITEM*) cur->Clone();
m_item_to_repeat = aItem;
delete old;
}
}
void SCH_EDIT_FRAME::SetSheetNumberAndCount()
{
SCH_SCREEN* screen = GetScreen();

View File

@ -45,7 +45,7 @@ bool SCH_EDIT_FRAME::EditSheet( SCH_SHEET* aSheet, wxDC* aDC )
if( aSheet == NULL )
return false;
/* Get the new texts */
// Get the new texts
DIALOG_SCH_SHEET_PROPS dlg( this );
wxString units = GetUnitsLabel( g_UserUnit );
@ -277,11 +277,12 @@ static void MoveOrResizeSheet( EDA_DRAW_PANEL* aPanel, wxDC* aDC, const wxPoint&
}
/* Complete sheet move. */
// Complete sheet move.
static void ExitSheet( EDA_DRAW_PANEL* aPanel, wxDC* aDC )
{
SCH_SCREEN* screen = (SCH_SCREEN*) aPanel->GetScreen();
SCH_ITEM* item = screen->GetCurItem();
SCH_EDIT_FRAME* parent = ( SCH_EDIT_FRAME* ) aPanel->GetParent();
if( (item == NULL) || (item->Type() != SCH_SHEET_T) || (parent == NULL) )
@ -320,10 +321,10 @@ static void ExitSheet( EDA_DRAW_PANEL* aPanel, wxDC* aDC )
}
/* Create hierarchy sheet. */
// Create hierarchy sheet.
SCH_SHEET* SCH_EDIT_FRAME::CreateSheet( wxDC* aDC )
{
m_itemToRepeat = NULL;
SetRepeatItem( NULL );
SCH_SHEET* sheet = new SCH_SHEET( GetCrossHairPosition() );

View File

@ -123,7 +123,7 @@ public:
BASE_SCREEN* GetScreen();
virtual EDA_DRAW_FRAME* GetParent();
EDA_DRAW_FRAME* GetParent();
void OnPaint( wxPaintEvent& event );

View File

@ -134,7 +134,7 @@ private:
wxArrayString m_findStringHistoryList;
wxArrayString m_replaceStringHistoryList;
BLOCK_SELECTOR m_blockItems; ///< List of selected items.
SCH_ITEM* m_itemToRepeat; ///< Last item to insert by the repeat command.
SCH_ITEM* m_item_to_repeat; ///< Last item to insert by the repeat command.
int m_repeatLabelDelta; ///< Repeat label number increment step.
SCH_COLLECTOR m_collectedItems; ///< List of collected items.
SCH_FIND_COLLECTOR m_foundItems; ///< List of find/replace items.
@ -902,7 +902,8 @@ private:
void InstallHierarchyFrame( wxDC* DC, wxPoint& pos );
SCH_SHEET* CreateSheet( wxDC* DC );
void ReSizeSheet( SCH_SHEET* Sheet, wxDC* DC );
// Loads the cache library associated to the aFileName
/// Loads the cache library associated to the aFileName
bool LoadCacheLibrary( const wxString& aFileName );
public:
@ -1176,7 +1177,18 @@ public:
*/
void RepeatDrawItem( wxDC* DC );
void SetRepeatItem( SCH_ITEM* aItem ) { m_itemToRepeat = aItem; }
/**
* Function SetRepeatItem
* clones aItem and owns that clone in this container.
*/
void SetRepeatItem( SCH_ITEM* aItem );
/**
* Function GetRepeatItem
* returns the item which is to be repeated with the insert key. Such object
* is owned by this container, and must be cloned.
*/
SCH_ITEM* GetRepeatItem() const { return m_item_to_repeat; }
/**
* Function SetUndoItem