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

Symbol editor: fix inherited symbol editing bug. 

The library manager update part function was orphaning the root symbol
of derived symbols when the root symbol was edited.  Re-parent the
inherited symbols when updating a root symbol.

Update the mandatory field attributes whenever the parent is set for
derived parts.  This will show the fields with the correct attributes
in the symbol editor and viewer.

Using std::unique_ptr to hold the current symbols was deleting the
pointer on symbol selection changes causing the pointer in the library
manager buffer to be stale in some cases.  Revert back to using a
simple pointer and manual clean up as required.

Prevent derived symbols from being saved to a different library to
prevent orphaned symbols.

Fixes kicad/code/kicad#3649
This commit is contained in:
Wayne Stambaugh 2019-12-13 16:51:59 -05:00
parent e9323fff82
commit 3e431d0d39
8 changed files with 92 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
eeschema/class_libentry.cpp
View File

@ -96,9 +96,6 @@ LIB_PART::LIB_PART( const wxString& aName, LIB_PART* aParent, PART_LIB* aLibrary
m_showPinNumbers = true;
m_showPinNames = true;
if( aParent )
m_parent = aParent->SharedPtr();
// Add the MANDATORY_FIELDS in RAM only. These are assumed to be present
// when the field editors are invoked.
m_drawings[LIB_FIELD_T].reserve( 4 );
@ -107,8 +104,12 @@ LIB_PART::LIB_PART( const wxString& aName, LIB_PART* aParent, PART_LIB* aLibrary
m_drawings[LIB_FIELD_T].push_back( new LIB_FIELD( this, FOOTPRINT ) );
m_drawings[LIB_FIELD_T].push_back( new LIB_FIELD( this, DATASHEET ) );
SetLib( aLibrary );
SetName( aName );
if( aParent )
SetParent( aParent );
SetLib( aLibrary );
}
@ -120,7 +121,6 @@ LIB_PART::LIB_PART( const LIB_PART& aPart, PART_LIB* aLibrary ) :
m_library = aLibrary;
m_name = aPart.m_name;
m_parent = aPart.m_parent;
m_FootprintList = wxArrayString( aPart.m_FootprintList );
m_unitCount = aPart.m_unitCount;
m_unitsLocked = aPart.m_unitsLocked;
@ -143,6 +143,11 @@ LIB_PART::LIB_PART( const LIB_PART& aPart, PART_LIB* aLibrary ) :
newItem->SetParent( this );
m_drawings.push_back( newItem );
}
PART_SPTR parent = aPart.m_parent.lock();
if( parent )
SetParent( parent.get() );
}
@ -260,9 +265,37 @@ void LIB_PART::SetName( const wxString& aName )
void LIB_PART::SetParent( LIB_PART* aParent )
{
if( aParent )
{
m_parent = aParent->SharedPtr();
// Inherit the parent mandatory field attributes.
for( int id=0; id<MANDATORY_FIELDS; ++id )
{
LIB_FIELD* field = GetField( id );
// the MANDATORY_FIELDS are exactly that in RAM.
wxASSERT( field );
LIB_FIELD* parentField = aParent->GetField( id );
wxASSERT( parentField );
wxString name = field->GetText();
*field = *parentField;
if( id == VALUE )
field->SetText( name );
else if( id == DATASHEET && !GetDocFileName().IsEmpty() )
field->SetText( GetDocFileName() );
field->SetParent( this );
}
}
else
{
m_parent.reset();
}
}

5
eeschema/dialogs/dialog_edit_component_in_lib.cpp
View File

@ -153,8 +153,9 @@ bool DIALOG_EDIT_COMPONENT_IN_LIBRARY::TransferDataToWindow()
// Push a copy of each field into m_fields
m_libEntry->GetFields( *m_fields );
// Copy the data sheet field from the old alias document file name.
m_fields->at( DATASHEET ).SetText( m_libEntry->GetDocFileName() );
// Copy the data sheet field from the old alias document file name if it's not empty.
if( !m_libEntry->GetDocFileName().IsEmpty() )
m_fields->at( DATASHEET ).SetText( m_libEntry->GetDocFileName() );
// The Y axis for components in lib is from bottom to top while the screen axis is top
// to bottom: we must change the y coord sign for editing

12
eeschema/libedit/lib_edit_frame.cpp
View File

@ -105,6 +105,7 @@ LIB_EDIT_FRAME::LIB_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
SetShowElectricalType( true );
m_FrameSize = ConvertDialogToPixels( wxSize( 500, 350 ) ); // default in case of no prefs
m_my_part = nullptr;
m_treePane = nullptr;
m_libMgr = nullptr;
m_unit = 1;
@ -367,7 +368,10 @@ void LIB_EDIT_FRAME::SetCurPart( LIB_PART* aPart )
{
m_toolManager->RunAction( EE_ACTIONS::clearSelection, true );
m_my_part.reset( aPart );
if( m_my_part )
delete m_my_part;
m_my_part = aPart;
// select the current component in the tree widget
if( m_my_part )
@ -507,7 +511,7 @@ LIB_PART* LIB_EDIT_FRAME::getTargetPart() const
return alias;
}
return m_my_part.get();
return m_my_part;
}
@ -639,7 +643,7 @@ bool LIB_EDIT_FRAME::backupFile( const wxFileName& aOriginalFile, const wxString
void LIB_EDIT_FRAME::storeCurrentPart()
{
if( m_my_part && !GetCurLib().IsEmpty() && GetScreen()->IsModify() )
m_libMgr->UpdatePart( m_my_part.get(), GetCurLib() ); // UpdatePart() makes a copy
m_libMgr->UpdatePart( m_my_part, GetCurLib() ); // UpdatePart() makes a copy
}
@ -702,7 +706,7 @@ void LIB_EDIT_FRAME::RebuildView()
GetRenderSettings()->m_ShowUnit = m_unit;
GetRenderSettings()->m_ShowConvert = m_convert;
GetRenderSettings()->m_ShowDisabled = m_my_part && m_my_part->IsAlias();
GetCanvas()->DisplayComponent( m_my_part.get() );
GetCanvas()->DisplayComponent( m_my_part );
GetCanvas()->GetView()->HideWorksheet();
GetCanvas()->GetView()->ClearHiddenFlags();

4
eeschema/libedit/lib_edit_frame.h
View File

@ -49,7 +49,7 @@ class LIB_MANAGER;
*/
class LIB_EDIT_FRAME : public SCH_BASE_FRAME
{
std::unique_ptr< LIB_PART > m_my_part; // a part I own, it is not in any library, but a copy
LIB_PART* m_my_part; // a part I own, it is not in any library, but a copy
// could be.
wxComboBox* m_unitSelectBox; // a ComboBox to select a unit to edit (if the part
// has multiple units)
@ -149,7 +149,7 @@ public:
*
* This is a LIB_PART that I own, it is at best a copy of one in a library.
*/
LIB_PART* GetCurPart() { return m_my_part.get(); }
LIB_PART* GetCurPart() { return m_my_part; }
/**
* Take ownership of aPart and notes that it is the one currently being edited.

22
eeschema/libedit/lib_manager.cpp
View File

@ -390,6 +390,26 @@ bool LIB_MANAGER::UpdatePart( LIB_PART* aPart, const wxString& aLibrary )
if( partBuf )
{
if( partBuf->GetPart()->IsRoot() && libBuf.HasDerivedSymbols( aPart->GetName() ) )
{
// Reparent derived symbols.
for( auto entry : libBuf.GetBuffers() )
{
if( entry->GetPart()->IsRoot() )
continue;
if( entry->GetPart()->GetParent().lock() == partBuf->GetPart()->SharedPtr() )
{
if( !libBuf.DeleteBuffer( entry ) )
return false;
LIB_PART* reparentedPart = new LIB_PART( *entry->GetPart() );
reparentedPart->SetParent( partCopy );
libBuf.CreateBuffer( reparentedPart, new SCH_SCREEN( &m_frame.Kiway() ) );
}
}
}
libBuf.UpdateBuffer( partBuf, partCopy );
}
else // New entry
@ -761,7 +781,7 @@ LIB_MANAGER::LIB_BUFFER& LIB_MANAGER::getLibraryBuffer( const wxString& aLibrary
}
else if( !buf.GetPart( part->GetName() ) )
{
buf.CreateBuffer( new LIB_PART( *part, nullptr ), new SCH_SCREEN( &m_frame.Kiway() ) );
buf.CreateBuffer( new LIB_PART( *part ), new SCH_SCREEN( &m_frame.Kiway() ) );
}
}

18
eeschema/libedit/libedit.cpp
View File

@ -251,7 +251,7 @@ bool LIB_EDIT_FRAME::LoadOneLibraryPartAux( LIB_PART* aEntry, const wxString& aL
m_toolManager->RunAction( ACTIONS::zoomFitScreen, true );
updateTitle();
RebuildSymbolUnitsList();
SetShowDeMorgan( GetCurPart()->Flatten()->HasConversion() );
SetShowDeMorgan( GetCurPart()->HasConversion() );
SyncToolbars();
// Display the document information based on the entry selected just in
@ -464,6 +464,16 @@ void LIB_EDIT_FRAME::savePartAs()
return;
}
// @todo Either check the selecteced library to see if the parent symbol name is in
// the new library and/or copy the parent symbol as well. This is the lazy
// solution to ensure derived parts do not get orphaned.
if( part->IsAlias() && new_lib != old_lib )
{
DisplayError( this, _( "Derived symbols must be save in the same library\n"
"that the parent symbol exists." ) );
return;
}
wxString new_name = nameTextCtrl->GetValue();
new_name.Trim( true );
new_name.Trim( false );
@ -490,9 +500,7 @@ void LIB_EDIT_FRAME::savePartAs()
m_libMgr->UpdatePart( &new_part, new_lib );
SyncLibraries( false );
m_treePane->GetLibTree()->SelectLibId( LIB_ID( new_lib, new_part.GetName() ) );
if( isCurrentPart( old_lib_id ) )
LoadPart( new_name, new_lib, m_unit );
LoadPart( new_name, new_lib, m_unit );
}
}
@ -517,7 +525,7 @@ void LIB_EDIT_FRAME::UpdateAfterSymbolProperties( wxString* aOldName, wxArrayStr
m_my_part->SetName( *aOldName );
}
else
m_libMgr->UpdatePartAfterRename( m_my_part.get(), *aOldName, lib );
m_libMgr->UpdatePartAfterRename( m_my_part, *aOldName, lib );
}
// Reselect the renamed part

8
eeschema/libedit/libedit_undo_redo.cpp
View File

@ -75,7 +75,7 @@ void LIB_EDIT_FRAME::GetComponentFromRedoList()
// Store the current part in the undo buffer
PICKED_ITEMS_LIST* undoCommand = new PICKED_ITEMS_LIST();
LIB_PART* oldPart = m_my_part.get();
LIB_PART* oldPart = m_my_part;
oldPart->SetFlags( UR_TRANSIENT );
ITEM_PICKER undoWrapper( oldPart, undoRedoType );
undoCommand->PushItem( undoWrapper );
@ -85,7 +85,7 @@ void LIB_EDIT_FRAME::GetComponentFromRedoList()
// which calls delete <previous part>.
// <previous part> is now put in undo list and is owned by this list
// Just set the current part to the part which come from the redo list
m_my_part.reset( part );
m_my_part = part;
if( undoRedoType == UR_LIB_RENAME )
{
@ -123,7 +123,7 @@ void LIB_EDIT_FRAME::GetComponentFromUndoList()
// Store the current part in the redo buffer
PICKED_ITEMS_LIST* redoCommand = new PICKED_ITEMS_LIST();
LIB_PART* oldPart = m_my_part.get();
LIB_PART* oldPart = m_my_part;
oldPart->SetFlags( UR_TRANSIENT );
ITEM_PICKER redoWrapper( oldPart, undoRedoType );
redoCommand->PushItem( redoWrapper );
@ -133,7 +133,7 @@ void LIB_EDIT_FRAME::GetComponentFromUndoList()
// which calls delete <previous part>.
// <previous part> is now put in redo list and is owned by this list.
// Just set the current part to the part which come from the undo list
m_my_part.reset( part );
m_my_part = part;
if( undoRedoType == UR_LIB_RENAME )
{

6
eeschema/libedit/symbedit.cpp
View File

@ -109,7 +109,7 @@ void LIB_EDIT_FRAME::LoadOneSymbol()
wxCHECK_RET( alias, "Invalid symbol." );
SaveCopyInUndoList( m_my_part.get() );
SaveCopyInUndoList( m_my_part );
LIB_PART* first = alias;
LIB_ITEMS_CONTAINER& drawList = first->GetDrawItems();
@ -129,7 +129,7 @@ void LIB_EDIT_FRAME::LoadOneSymbol()
LIB_ITEM* newItem = (LIB_ITEM*) item.Clone();
newItem->SetParent( m_my_part.get() );
newItem->SetParent( m_my_part );
m_my_part->AddDrawItem( newItem );
item.ClearSelected();
}
@ -189,7 +189,7 @@ void LIB_EDIT_FRAME::SaveOneSymbol()
plugin->CreateSymbolLib( fn.GetFullPath(), &nodoc_props );
// The part gets flattened by the LIB_PART copy constructor.
LIB_PART* saved_part = new LIB_PART( *m_my_part.get() );
LIB_PART* saved_part = new LIB_PART( *m_my_part );
plugin->SaveSymbol( fn.GetFullPath(), saved_part, &nodoc_props );
}
catch( const IO_ERROR& ioe )