Eeschema: fix rescue multiple unit symbol bug.

The code that checked for pin conflicts to determine if a symbol needed
rescued did not check either the pin convert setting so it was possible
for a pin from the other convert on symbols that do not have identical
units to appear to not have a pin conflict.  Add tests for pin unit and
convert setting to prevent that from breaking the comparison.  This must
have always been broken.

Fix the symbol preview widget to prevent drawing all symbols on top of
each other (if we need to do this the code will have to be revised) and
also show the convert if valid.

Fix broken symbol cache library when saving alias symbols.

Fixes https://gitlab.com/kicad/code/kicad/issues/3879
This commit is contained in:
Wayne Stambaugh 2020-04-01 20:17:25 -04:00
parent 0d79ada3ec
commit e91f1f57dd
7 changed files with 87 additions and 39 deletions

View File

@ -697,6 +697,15 @@ bool LIB_PART::PinsConflictWith( LIB_PART& aOtherPart, bool aTestNums, bool aTes
for( LIB_PIN* eachOtherPin : otherPinList )
{
wxASSERT( eachOtherPin );
// Same unit?
if( eachThisPin->GetUnit() != eachOtherPin->GetUnit() )
continue;
// Same body stype?
if( eachThisPin->GetConvert() != eachOtherPin->GetConvert() )
continue;
// Same position?
if( eachThisPin->GetPosition() != eachOtherPin->GetPosition() )
continue;
@ -723,6 +732,7 @@ bool LIB_PART::PinsConflictWith( LIB_PART& aOtherPart, bool aTestNums, bool aTes
continue;
foundMatch = true;
break; // Match found so seach is complete.
}
if( !foundMatch )

View File

@ -1,7 +1,7 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2015-2019 KiCad Developers, see AUTHORS.txt for contributors.
* Copyright (C) 2015-2020 KiCad Developers, see AUTHORS.txt for contributors.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@ -243,8 +243,13 @@ void DIALOG_RESCUE_EACH::displayItemsInConflict()
else
{
RESCUE_CANDIDATE& selected_part = m_Rescuer->m_all_candidates[row];
m_previewOldWidget->DisplayPart( selected_part.GetCacheCandidate(), 0 );
m_previewNewWidget->DisplayPart( selected_part.GetLibCandidate(), 0 );
m_previewOldWidget->DisplayPart( selected_part.GetCacheCandidate(),
selected_part.GetUnit(),
selected_part.GetConvert() );
m_previewNewWidget->DisplayPart( selected_part.GetLibCandidate(),
selected_part.GetUnit(),
selected_part.GetConvert() );
}
}

View File

@ -3,7 +3,7 @@
*
* Copyright (C) 2004 Jean-Pierre Charras, jp.charras ar wanadoo.fr
* Copyright (C) 2008 Wayne Stambaugh <stambaughw@gmail.com>
* Copyright (C) 2004-2019 KiCad Developers, see AUTHORS.txt for contributors.
* Copyright (C) 2004-2020 KiCad Developers, see AUTHORS.txt for contributors.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@ -110,13 +110,13 @@ bool SCH_EDIT_FRAME::CreateArchiveLibrary( const wxString& aFileName )
if( part )
{
std::unique_ptr<LIB_PART> flattenedPart = part->Flatten();
// Use the full LIB_ID as the symbol name to prevent symbol name collisions.
wxString oldName = part->GetName();
part->SetName( component->GetLibId().GetUniStringLibId() );
flattenedPart->SetName( component->GetLibId().GetUniStringLibId() );
// AddPart() does first clone the part before adding.
archLib->AddPart( part );
part->SetName( oldName );
archLib->AddPart( flattenedPart.get() );
}
else
{

View File

@ -2,7 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2015 Chris Pavlina <pavlina.chris@gmail.com>
* Copyright (C) 2015-2019 KiCad Developers, see change_log.txt for contributors.
* Copyright (C) 2015-2020 KiCad Developers, see change_log.txt for contributors.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@ -119,11 +119,15 @@ static wxFileName GetRescueLibraryFileName()
RESCUE_CASE_CANDIDATE::RESCUE_CASE_CANDIDATE( const wxString& aRequestedName,
const wxString& aNewName,
LIB_PART* aLibCandidate )
LIB_PART* aLibCandidate,
int aUnit,
int aConvert )
{
m_requested_name = aRequestedName;
m_new_name = aNewName;
m_lib_candidate = aLibCandidate;
m_unit = aUnit;
m_convert = aConvert;
}
@ -164,7 +168,9 @@ void RESCUE_CASE_CANDIDATE::FindRescues( RESCUER& aRescuer,
continue;
RESCUE_CASE_CANDIDATE candidate( part_name, case_insensitive_matches[0]->GetName(),
case_insensitive_matches[0] );
case_insensitive_matches[0],
each_component->GetUnit(),
each_component->GetConvert() );
candidate_map[part_name] = candidate;
}
@ -207,12 +213,16 @@ bool RESCUE_CASE_CANDIDATE::PerformAction( RESCUER* aRescuer )
RESCUE_CACHE_CANDIDATE::RESCUE_CACHE_CANDIDATE( const wxString& aRequestedName,
const wxString& aNewName,
LIB_PART* aCacheCandidate,
LIB_PART* aLibCandidate )
LIB_PART* aLibCandidate,
int aUnit,
int aConvert )
{
m_requested_name = aRequestedName;
m_new_name = aNewName;
m_cache_candidate = aCacheCandidate;
m_lib_candidate = aLibCandidate;
m_unit = aUnit;
m_convert = aConvert;
}
@ -265,7 +275,9 @@ void RESCUE_CACHE_CANDIDATE::FindRescues( RESCUER& aRescuer,
// Check if the symbol has already been rescued.
wxString new_name = LIB_ID::FixIllegalChars( part_name, LIB_ID::ID_SCH );
RESCUE_CACHE_CANDIDATE candidate( part_name, new_name, cache_match, lib_match );
RESCUE_CACHE_CANDIDATE candidate( part_name, new_name, cache_match, lib_match,
each_component->GetUnit(),
each_component->GetConvert() );
candidate_map[part_name] = candidate;
}
@ -328,13 +340,17 @@ RESCUE_SYMBOL_LIB_TABLE_CANDIDATE::RESCUE_SYMBOL_LIB_TABLE_CANDIDATE(
const LIB_ID& aRequestedId,
const LIB_ID& aNewId,
LIB_PART* aCacheCandidate,
LIB_PART* aLibCandidate ) : RESCUE_CANDIDATE()
LIB_PART* aLibCandidate,
int aUnit,
int aConvert ) : RESCUE_CANDIDATE()
{
m_requested_id = aRequestedId;
m_requested_name = aRequestedId.Format();
m_new_id = aNewId;
m_lib_candidate = aLibCandidate;
m_cache_candidate = aCacheCandidate;
m_unit = aUnit;
m_convert = aConvert;
}
@ -420,7 +436,9 @@ void RESCUE_SYMBOL_LIB_TABLE_CANDIDATE::FindRescues(
libNickname.Replace( " ", "-" );
LIB_ID new_id( libNickname, new_name + "-" + part_id.GetLibNickname().wx_str() );
RESCUE_SYMBOL_LIB_TABLE_CANDIDATE candidate( part_id, new_id, cache_match, lib_match );
RESCUE_SYMBOL_LIB_TABLE_CANDIDATE candidate( part_id, new_id, cache_match, lib_match,
each_component->GetUnit(),
each_component->GetConvert() );
candidate_map[part_id] = candidate;
}

View File

@ -2,7 +2,7 @@
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2015 Chris Pavlina <pavlina.chris@gmail.com>
* Copyright (C) 2015-2019 KiCad Developers, see change_log.txt for contributors.
* Copyright (C) 2015-2020 KiCad Developers, see change_log.txt for contributors.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@ -68,6 +68,8 @@ protected:
wxString m_requested_name;
wxString m_new_name;
LIB_PART* m_lib_candidate;
int m_unit;
int m_convert;
public:
virtual ~RESCUE_CANDIDATE() {}
@ -94,6 +96,10 @@ public:
*/
virtual LIB_PART* GetLibCandidate() const { return m_lib_candidate; }
int GetUnit() const { return m_unit; }
int GetConvert() const { return m_convert; }
/**
* Get a description of the action proposed, for displaying in the UI.
*/
@ -124,9 +130,11 @@ public:
* @param aRequestedName - the name the schematic asks for
* @param aNewName - the name we want to change it to
* @param aLibCandidate - the part that will give us
* @param aUnit The unit of the rescued symbol.
* @param aConvert The body style of the rescued symbol.
*/
RESCUE_CASE_CANDIDATE( const wxString& aRequestedName, const wxString& aNewName,
LIB_PART* aLibCandidate );
LIB_PART* aLibCandidate, int aUnit = 0, int aConvert = 0 );
RESCUE_CASE_CANDIDATE() { m_lib_candidate = NULL; }
@ -155,9 +163,12 @@ public:
* @param aNewName - the name we want to change it to
* @param aCacheCandidate - the part from the cache
* @param aLibCandidate - the part that would be loaded from the library
* @param aUnit The unit of the rescued symbol.
* @param aConvert The body style of the rescued symbol.
*/
RESCUE_CACHE_CANDIDATE( const wxString& aRequestedName, const wxString& aNewName,
LIB_PART* aCacheCandidate, LIB_PART* aLibCandidate );
LIB_PART* aCacheCandidate, LIB_PART* aLibCandidate,
int aUnit = 0, int aConvert = 0 );
RESCUE_CACHE_CANDIDATE();
@ -190,9 +201,12 @@ public:
* @param aNewName - the name we want to change it to
* @param aCacheCandidate - the part from the cache
* @param aLibCandidate - the part that would be loaded from the library
* @param aUnit The unit of the rescued symbol.
* @param aConvert The body style of the rescued symbol.
*/
RESCUE_SYMBOL_LIB_TABLE_CANDIDATE( const LIB_ID& aRequestedId, const LIB_ID& aNewId,
LIB_PART* aCacheCandidate, LIB_PART* aLibCandidate);
LIB_PART* aCacheCandidate, LIB_PART* aLibCandidate,
int aUnit = 0, int aConvert = 0 );
RESCUE_SYMBOL_LIB_TABLE_CANDIDATE();

View File

@ -1,7 +1,7 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
* Copyright (C) 2018-2020 KiCad Developers, see AUTHORS.txt for contributors.
*
* This program is free software: you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
@ -58,7 +58,8 @@ SYMBOL_PREVIEW_WIDGET::SYMBOL_PREVIEW_WIDGET( wxWindow* aParent, KIWAY& aKiway,
// Do not display the grid: the look is not good for a small canvas area.
// But mainly, due to some strange bug I (JPC) was unable to fix, the grid creates
// strange artifacts on Windows when eeschema is run from Kicad manager (but not in stand alone...).
// strange artifacts on Windows when eeschema is run from Kicad manager (but not in
// stand alone...).
m_preview->GetGAL()->SetGridVisibility( false );
// Early initialization of the canvas background color,
@ -142,7 +143,7 @@ void SYMBOL_PREVIEW_WIDGET::fitOnDrawArea()
}
void SYMBOL_PREVIEW_WIDGET::DisplaySymbol( const LIB_ID& aSymbolID, int aUnit )
void SYMBOL_PREVIEW_WIDGET::DisplaySymbol( const LIB_ID& aSymbolID, int aUnit, int aConvert )
{
KIGFX::VIEW* view = m_preview->GetView();
auto settings = static_cast<KIGFX::SCH_RENDER_SETTINGS*>( view->GetPainter()->GetSettings() );
@ -177,18 +178,17 @@ void SYMBOL_PREVIEW_WIDGET::DisplaySymbol( const LIB_ID& aSymbolID, int aUnit )
// If unit isn't specified for a multi-unit part, pick the first. (Otherwise we'll
// draw all of them.)
if( m_previewItem->IsMulti() && aUnit == 0 )
aUnit = 1;
settings->m_ShowUnit = aUnit;
settings->m_ShowUnit = ( m_previewItem->IsMulti() && aUnit == 0 ) ? 1 : aUnit;
// For symbols having a De Morgan body style, use the first style
settings->m_ShowConvert = m_previewItem->HasConversion() ? 1 : 0;
settings->m_ShowConvert =
( m_previewItem->HasConversion() && aConvert == 0 ) ? 1 : aConvert;
view->Add( m_previewItem );
// Get the symbole size, in internal units
m_itemBBox = m_previewItem->GetUnitBoundingBox( aUnit, 0 );
m_itemBBox = m_previewItem->GetUnitBoundingBox( settings->m_ShowUnit,
settings->m_ShowConvert );
if( !m_preview->IsShown() )
{
@ -205,7 +205,7 @@ void SYMBOL_PREVIEW_WIDGET::DisplaySymbol( const LIB_ID& aSymbolID, int aUnit )
}
void SYMBOL_PREVIEW_WIDGET::DisplayPart( LIB_PART* aPart, int aUnit )
void SYMBOL_PREVIEW_WIDGET::DisplayPart( LIB_PART* aPart, int aUnit, int aConvert )
{
KIGFX::VIEW* view = m_preview->GetView();
@ -220,18 +220,19 @@ void SYMBOL_PREVIEW_WIDGET::DisplayPart( LIB_PART* aPart, int aUnit )
{
m_previewItem = new LIB_PART( *aPart );
// If unit isn't specified for a multi-unit part, pick the first. (Otherwise we'll
// draw all of them.)
if( m_previewItem->IsMulti() && aUnit == 0 )
aUnit = 1;
// For symbols having a De Morgan body style, use the first style
auto settings = static_cast<KIGFX::SCH_RENDER_SETTINGS*>( view->GetPainter()->GetSettings() );
settings->m_ShowConvert = m_previewItem->HasConversion() ? 1 : 0;
// If unit isn't specified for a multi-unit part, pick the first. (Otherwise we'll
// draw all of them.)
settings->m_ShowUnit = ( m_previewItem->IsMulti() && aUnit == 0 ) ? 1 : aUnit;
settings->m_ShowConvert =
( m_previewItem->HasConversion() && aConvert == 0 ) ? 1 : aConvert;
view->Add( m_previewItem );
// Get the symbole size, in internal units
m_itemBBox = aPart->GetUnitBoundingBox( aUnit, 0 );
m_itemBBox = aPart->GetUnitBoundingBox( settings->m_ShowUnit, settings->m_ShowConvert );
// Calculate the draw scale to fit the drawing area
fitOnDrawArea();

View File

@ -1,7 +1,7 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
* Copyright (C) 2018-2020 KiCad Developers, see AUTHORS.txt for contributors.
*
* This program is free software: you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
@ -56,9 +56,9 @@ public:
/**
* Set the currently displayed symbol.
*/
void DisplaySymbol( const LIB_ID& aSymbolID, int aUnit );
void DisplaySymbol( const LIB_ID& aSymbolID, int aUnit, int aConvert = 0 );
void DisplayPart( LIB_PART* aPart, int aUnit );
void DisplayPart( LIB_PART* aPart, int aUnit, int aConvert = 0 );
private:
void onSize( wxSizeEvent& aEvent );