Gencad export: fix an issue in hash_eda calculation, that can generate the same hash for 2 different footprints.

Therefore, for some footprints, the right footprint was not associated to the component.
The hash calculation was using a XOR to combine 2 sub hash values.
This is not a strong way to combine these hash values.
They are now added. Looks better to identify similar and different footprints.

Fixes: lp:1847575
https://bugs.launchpad.net/kicad/+bug/1847575
This commit is contained in:
jean-pierre charras 2019-10-10 18:44:33 +02:00
parent cecfeea947
commit 967b9d4a97
4 changed files with 63 additions and 59 deletions

View File

@ -39,7 +39,7 @@ static inline size_t hash_board_item( const BOARD_ITEM* aItem, int aFlags )
size_t ret = 0; size_t ret = 0;
if( aFlags & LAYER ) if( aFlags & LAYER )
ret ^= hash<unsigned long long>{}( aItem->GetLayerSet().to_ullong() ); ret = hash<unsigned long long>{}( aItem->GetLayerSet().to_ullong() );
return ret; return ret;
} }
@ -55,57 +55,57 @@ size_t hash_eda( const EDA_ITEM* aItem, int aFlags )
{ {
const MODULE* module = static_cast<const MODULE*>( aItem ); const MODULE* module = static_cast<const MODULE*>( aItem );
ret ^= hash_board_item( module, aFlags ); ret += hash_board_item( module, aFlags );
if( aFlags & POSITION ) if( aFlags & POSITION )
{ {
ret ^= hash<int>{}( module->GetPosition().x ); ret += hash<int>{}( module->GetPosition().x );
ret ^= hash<int>{}( module->GetPosition().y ); ret += hash<int>{}( module->GetPosition().y );
} }
if( aFlags & ROTATION ) if( aFlags & ROTATION )
ret ^= hash<double>{}( module->GetOrientation() ); ret += hash<double>{}( module->GetOrientation() );
for( auto i : module->GraphicalItems() ) for( auto i : module->GraphicalItems() )
ret ^= hash_eda( i, aFlags ); ret += hash_eda( i, aFlags );
for( auto i : module->Pads() ) for( auto i : module->Pads() )
ret ^= hash_eda( static_cast<EDA_ITEM*>( i ), aFlags ); ret += hash_eda( static_cast<EDA_ITEM*>( i ), aFlags );
} }
break; break;
case PCB_PAD_T: case PCB_PAD_T:
{ {
const D_PAD* pad = static_cast<const D_PAD*>( aItem ); const D_PAD* pad = static_cast<const D_PAD*>( aItem );
ret ^= hash_board_item( pad, aFlags ); ret += hash_board_item( pad, aFlags );
ret ^= hash<int>{}( pad->GetShape() << 16 ); ret += hash<int>{}( pad->GetShape() << 16 );
ret ^= hash<int>{}( pad->GetDrillShape() << 18 ); ret += hash<int>{}( pad->GetDrillShape() << 18 );
ret ^= hash<int>{}( pad->GetSize().x << 8 ); ret += hash<int>{}( pad->GetSize().x << 8 );
ret ^= hash<int>{}( pad->GetSize().y << 9 ); ret += hash<int>{}( pad->GetSize().y << 9 );
ret ^= hash<int>{}( pad->GetOffset().x << 6 ); ret += hash<int>{}( pad->GetOffset().x << 6 );
ret ^= hash<int>{}( pad->GetOffset().y << 7 ); ret += hash<int>{}( pad->GetOffset().y << 7 );
ret ^= hash<int>{}( pad->GetDelta().x << 4 ); ret += hash<int>{}( pad->GetDelta().x << 4 );
ret ^= hash<int>{}( pad->GetDelta().y << 5 ); ret += hash<int>{}( pad->GetDelta().y << 5 );
if( aFlags & POSITION ) if( aFlags & POSITION )
{ {
if( aFlags & REL_COORD ) if( aFlags & REL_COORD )
{ {
ret ^= hash<int>{}( pad->GetPos0().x ); ret += hash<int>{}( pad->GetPos0().x );
ret ^= hash<int>{}( pad->GetPos0().y ); ret += hash<int>{}( pad->GetPos0().y );
} }
else else
{ {
ret ^= hash<int>{}( pad->GetPosition().x ); ret += hash<int>{}( pad->GetPosition().x );
ret ^= hash<int>{}( pad->GetPosition().y ); ret += hash<int>{}( pad->GetPosition().y );
} }
} }
if( aFlags & ROTATION ) if( aFlags & ROTATION )
ret ^= hash<double>{}( pad->GetOrientation() ); ret += hash<double>{}( pad->GetOrientation() );
if( aFlags & NET ) if( aFlags & NET )
ret ^= hash<int>{}( pad->GetNetCode() << 6 ); ret += hash<int>{}( pad->GetNetCode() << 6 );
} }
break; break;
@ -119,64 +119,64 @@ size_t hash_eda( const EDA_ITEM* aItem, int aFlags )
if( !( aFlags & VALUE ) && text->GetType() == TEXTE_MODULE::TEXT_is_VALUE ) if( !( aFlags & VALUE ) && text->GetType() == TEXTE_MODULE::TEXT_is_VALUE )
break; break;
ret ^= hash_board_item( text, aFlags ); ret += hash_board_item( text, aFlags );
ret ^= hash<string>{}( text->GetText().ToStdString() ); ret += hash<string>{}( text->GetText().ToStdString() );
ret ^= hash<bool>{}( text->IsItalic() ); ret += hash<bool>{}( text->IsItalic() );
ret ^= hash<bool>{}( text->IsBold() ); ret += hash<bool>{}( text->IsBold() );
ret ^= hash<bool>{}( text->IsMirrored() ); ret += hash<bool>{}( text->IsMirrored() );
ret ^= hash<int>{}( text->GetTextWidth() ); ret += hash<int>{}( text->GetTextWidth() );
ret ^= hash<int>{}( text->GetTextHeight() ); ret += hash<int>{}( text->GetTextHeight() );
ret ^= hash<int>{}( text->GetHorizJustify() ); ret += hash<int>{}( text->GetHorizJustify() );
ret ^= hash<int>{}( text->GetVertJustify() ); ret += hash<int>{}( text->GetVertJustify() );
if( aFlags & POSITION ) if( aFlags & POSITION )
{ {
if( aFlags & REL_COORD ) if( aFlags & REL_COORD )
{ {
ret ^= hash<int>{}( text->GetPos0().x ); ret += hash<int>{}( text->GetPos0().x );
ret ^= hash<int>{}( text->GetPos0().y ); ret += hash<int>{}( text->GetPos0().y );
} }
else else
{ {
ret ^= hash<int>{}( text->GetPosition().x ); ret += hash<int>{}( text->GetPosition().x );
ret ^= hash<int>{}( text->GetPosition().y ); ret += hash<int>{}( text->GetPosition().y );
} }
} }
if( aFlags & ROTATION ) if( aFlags & ROTATION )
ret ^= hash<double>{}( text->GetTextAngle() ); ret += hash<double>{}( text->GetTextAngle() );
} }
break; break;
case PCB_MODULE_EDGE_T: case PCB_MODULE_EDGE_T:
{ {
const EDGE_MODULE* segment = static_cast<const EDGE_MODULE*>( aItem ); const EDGE_MODULE* segment = static_cast<const EDGE_MODULE*>( aItem );
ret ^= hash_board_item( segment, aFlags ); ret += hash_board_item( segment, aFlags );
ret ^= hash<int>{}( segment->GetType() ); ret += hash<int>{}( segment->GetType() );
ret ^= hash<int>{}( segment->GetShape() ); ret += hash<int>{}( segment->GetShape() );
ret ^= hash<int>{}( segment->GetWidth() ); ret += hash<int>{}( segment->GetWidth() );
ret ^= hash<int>{}( segment->GetRadius() ); ret += hash<int>{}( segment->GetRadius() );
if( aFlags & POSITION ) if( aFlags & POSITION )
{ {
if( aFlags & REL_COORD ) if( aFlags & REL_COORD )
{ {
ret ^= hash<int>{}( segment->GetStart0().x ); ret += hash<int>{}( segment->GetStart0().x );
ret ^= hash<int>{}( segment->GetStart0().y ); ret += hash<int>{}( segment->GetStart0().y );
ret ^= hash<int>{}( segment->GetEnd0().x ); ret += hash<int>{}( segment->GetEnd0().x );
ret ^= hash<int>{}( segment->GetEnd0().y ); ret += hash<int>{}( segment->GetEnd0().y );
} }
else else
{ {
ret ^= hash<int>{}( segment->GetStart().x ); ret += hash<int>{}( segment->GetStart().x );
ret ^= hash<int>{}( segment->GetStart().y ); ret += hash<int>{}( segment->GetStart().y );
ret ^= hash<int>{}( segment->GetEnd().x ); ret += hash<int>{}( segment->GetEnd().x );
ret ^= hash<int>{}( segment->GetEnd().y ); ret += hash<int>{}( segment->GetEnd().y );
} }
} }
if( aFlags & ROTATION ) if( aFlags & ROTATION )
ret ^= hash<double>{}( segment->GetAngle() ); ret += hash<double>{}( segment->GetAngle() );
} }
break; break;

View File

@ -46,7 +46,7 @@ enum HASH_FLAGS
ALL = 0xff ALL = 0xff
}; };
/* /**
* Calculates hash of an EDA_ITEM. * Calculates hash of an EDA_ITEM.
* @param aItem is the item for which the hash will be computed. * @param aItem is the item for which the hash will be computed.
* @return Hash value. * @return Hash value.

View File

@ -40,12 +40,11 @@ DIALOG_GENCAD_EXPORT_OPTIONS::DIALOG_GENCAD_EXPORT_OPTIONS( PCB_EDIT_FRAME* aPar
: DIALOG_SHIM( aParent, wxID_ANY, _( "Export to GenCAD settings" ), wxDefaultPosition, : DIALOG_SHIM( aParent, wxID_ANY, _( "Export to GenCAD settings" ), wxDefaultPosition,
wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER ) wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER )
{ {
// Create widgets
SetSizeHints( wxSize( 500, 200 ), wxDefaultSize );
wxBoxSizer* m_mainSizer= new wxBoxSizer( wxVERTICAL ); wxBoxSizer* m_mainSizer= new wxBoxSizer( wxVERTICAL );
m_filePicker = new wxFilePickerCtrl( this, wxID_ANY, aPath, // Ctreate the file picker. The path will be set later, when the widget size
// is set to.
m_filePicker = new wxFilePickerCtrl( this, wxID_ANY, "",
_("Select a GenCAD export filename"), _("Select a GenCAD export filename"),
GencadFileWildcard(), GencadFileWildcard(),
wxDefaultPosition, wxSize( -1,-1 ), wxDefaultPosition, wxSize( -1,-1 ),
@ -60,8 +59,13 @@ DIALOG_GENCAD_EXPORT_OPTIONS::DIALOG_GENCAD_EXPORT_OPTIONS( PCB_EDIT_FRAME* aPar
m_mainSizer->Add( stdButtons, 0, wxEXPAND | wxLEFT | wxRIGHT | wxBOTTOM, 5 ); m_mainSizer->Add( stdButtons, 0, wxEXPAND | wxLEFT | wxRIGHT | wxBOTTOM, 5 );
SetSizer( m_mainSizer ); SetSizer( m_mainSizer );
Layout();
m_mainSizer->Fit( this ); // Now all widgets have the size fixed, call FinishDialogSettings
FinishDialogSettings();
// Set the path in m_filePicker, now the size is set
// (otherwize the text is truncated)
m_filePicker->SetPath( aPath );
Centre( wxBOTH ); Centre( wxBOTH );
} }

View File

@ -712,10 +712,10 @@ static size_t hashModule( const MODULE* aModule )
for( auto i : aModule->GraphicalItems() ) for( auto i : aModule->GraphicalItems() )
ret ^= hash_eda( i, flags ); ret += hash_eda( i, flags );
for( auto i : aModule->Pads() ) for( auto i : aModule->Pads() )
ret ^= hash_eda( i, flags ); ret += hash_eda( i, flags );
return ret; return ret;
} }