From 56bf72cc5d3f68b349b30b8e1151564180e342c5 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Wed, 7 Aug 2019 19:23:59 +0100 Subject: [PATCH] OpenGL context must be saved/restored around a translation. Also includes a bunch of code cleanup. Fixes: lp:1838655 * https://bugs.launchpad.net/kicad/+bug/1838655 --- eeschema/connection_graph.cpp | 20 ++--- eeschema/dialogs/dialog_erc.cpp | 28 +++---- eeschema/erc.cpp | 126 +++++++++++--------------------- eeschema/sch_marker.cpp | 9 +-- eeschema/sch_painter.cpp | 10 ++- 5 files changed, 73 insertions(+), 120 deletions(-) diff --git a/eeschema/connection_graph.cpp b/eeschema/connection_graph.cpp index a77060972b..0003f8b6ff 100644 --- a/eeschema/connection_graph.cpp +++ b/eeschema/connection_graph.cpp @@ -2106,8 +2106,8 @@ bool CONNECTION_GRAPH::ercCheckNoConnects( const CONNECTION_SUBGRAPH* aSubgraph, wxPoint pos = pin->GetTransformedPosition(); msg.Printf( _( "Pin %s of component %s has a no-connect marker but is connected" ), - GetChars( pin->GetName() ), - GetChars( pin->GetParentComponent()->GetRef( &aSubgraph->m_sheet ) ) ); + pin->GetName(), + pin->GetParentComponent()->GetRef( &aSubgraph->m_sheet ) ); auto marker = new SCH_MARKER(); marker->SetTimeStamp( GetNewTimeStamp() ); @@ -2158,22 +2158,22 @@ bool CONNECTION_GRAPH::ercCheckNoConnects( const CONNECTION_SUBGRAPH* aSubgraph, pin = static_cast( item ); else has_other_connections = true; + break; default: if( item->IsConnectable() ) has_other_connections = true; + break; } } - // Check if invisible power pins connect to anything else - // Note this won't catch if a component has multiple invisible power - // pins but these don't connect to any other net; maybe that should be - // added as a further optional ERC check. + // Check if invisible power pins connect to anything else. + // Note this won't catch a component with multiple invisible power pins but these don't + // connect to any other net; maybe that should be added as a further optional ERC check? - if( pin && !has_other_connections && - pin->IsPowerConnection() && !pin->IsVisible() ) + if( pin && !has_other_connections && pin->IsPowerConnection() && !pin->IsVisible() ) { wxString name = pin->Connection( sheet )->Name(); wxString local_name = pin->Connection( sheet )->Name( true ); @@ -2192,8 +2192,8 @@ bool CONNECTION_GRAPH::ercCheckNoConnects( const CONNECTION_SUBGRAPH* aSubgraph, wxPoint pos = pin->GetTransformedPosition(); msg.Printf( _( "Pin %s of component %s is unconnected." ), - GetChars( pin->GetName() ), - GetChars( pin->GetParentComponent()->GetRef( &aSubgraph->m_sheet ) ) ); + pin->GetName(), + pin->GetParentComponent()->GetRef( &aSubgraph->m_sheet ) ); auto marker = new SCH_MARKER(); marker->SetTimeStamp( GetNewTimeStamp() ); diff --git a/eeschema/dialogs/dialog_erc.cpp b/eeschema/dialogs/dialog_erc.cpp index 1033c1d188..db344faa4c 100644 --- a/eeschema/dialogs/dialog_erc.cpp +++ b/eeschema/dialogs/dialog_erc.cpp @@ -508,9 +508,8 @@ void DIALOG_ERC::TestErc( REPORTER& aReporter ) // Erase all previous DRC markers. screens.DeleteAllMarkers( MARKER_BASE::MARKER_ERC ); - /* Test duplicate sheet names inside a given sheet, one cannot have sheets with - * duplicate names (file names can be duplicated). - */ + // Test duplicate sheet names inside a given sheet. While one can have multiple references + // to the same file, each must have a unique name. TestDuplicateSheetNames( true ); TestConflictingBusAliases(); @@ -519,8 +518,7 @@ void DIALOG_ERC::TestErc( REPORTER& aReporter ) m_parent->RecalculateConnections(); g_ConnectionGraph->RunERC( m_settings ); - /* Test is all units of each multiunit component have the same footprint assigned. - */ + // Test is all units of each multiunit component have the same footprint assigned. TestMultiunitFootprints( sheets ); std::unique_ptr objectsConnectedList( m_parent->BuildNetListBase() ); @@ -532,21 +530,15 @@ void DIALOG_ERC::TestErc( REPORTER& aReporter ) unsigned nextItemIdx = 0; int MinConn = NOC; - /* Check that a pin appears in only one net. This check is necessary - * because multi-unit components that have shared pins can be wired to - * different nets. - */ + // Check that a pin appears in only one net. This check is necessary because multi-unit + // components that have shared pins could be wired to different nets. std::unordered_map pin_to_net_map; - /* The netlist generated by SCH_EDIT_FRAME::BuildNetListBase is sorted - * by net number, which means we can group netlist items into ranges - * that live in the same net. The range from nextItem to the current - * item (exclusive) needs to be checked against the current item. The - * lastItem variable is used as a helper to pass the last item's number - * from one loop iteration to the next, which simplifies the initial - * pass. - */ - + // The netlist generated by SCH_EDIT_FRAME::BuildNetListBase is sorted by net number, which + // means we can group netlist items into ranges that live in the same net. The range from + // nextItem to the current item (exclusive) needs to be checked against the current item. + // The lastItem variable is used as a helper to pass the last item's number from one loop + // iteration to the next, which simplifies the initial pass. for( unsigned itemIdx = 0; itemIdx < objectsConnectedList->size(); itemIdx++ ) { auto item = objectsConnectedList->GetItem( itemIdx ); diff --git a/eeschema/erc.cpp b/eeschema/erc.cpp index 8c199c6272..911adac661 100644 --- a/eeschema/erc.cpp +++ b/eeschema/erc.cpp @@ -32,15 +32,12 @@ #include #include #include - -#include #include #include #include #include #include #include - #include @@ -227,65 +224,44 @@ int TestDuplicateSheetNames( bool aCreateMarker ) int TestConflictingBusAliases( bool aCreateMarker ) { - using std::pair; - using std::shared_ptr; - using std::vector; - - int err_count = 0; + wxString msg; + wxPoint dummyPos( 0, 0 ); + int err_count = 0; SCH_SCREENS screens; - vector< shared_ptr > aliases; - vector< pair< shared_ptr, shared_ptr > > conflicts; + std::vector< std::shared_ptr > aliases; for( auto screen = screens.GetFirst(); screen != NULL; screen = screens.GetNext() ) { - auto screen_aliases = screen->GetBusAliases(); + std::unordered_set< std::shared_ptr > screen_aliases = screen->GetBusAliases(); - for( auto alias : screen_aliases ) + for( const std::shared_ptr& alias : screen_aliases ) { - for( auto test : aliases ) + for( const std::shared_ptr& test : aliases ) { - if( alias->GetName() == test->GetName() && - alias->Members() != test->Members() ) + if( alias->GetName() == test->GetName() && alias->Members() != test->Members() ) { - conflicts.push_back( std::make_pair( alias, test ) ); + if( aCreateMarker ) + { + msg = wxString::Format( _( "Bus alias %s has conflicting definitions on" + "multiple sheets: %s and %s" ), + alias->GetName(), + alias->GetParent()->GetFileName(), + test->GetParent()->GetFileName() ); + + SCH_MARKER* marker = new SCH_MARKER(); + marker->SetData( ERCE_BUS_ALIAS_CONFLICT, dummyPos, msg, dummyPos ); + marker->SetMarkerType( MARKER_BASE::MARKER_ERC ); + marker->SetErrorLevel( MARKER_BASE::MARKER_SEVERITY_ERROR ); + + test->GetParent()->Append( marker ); + } + + ++err_count; } } } - aliases.insert( aliases.end(), - screen_aliases.begin(), screen_aliases.end() ); - } - - if( !conflicts.empty() ) - { - if( aCreateMarker ) - { - wxString msg; - - for( auto conflict : conflicts ) - { - auto marker = new SCH_MARKER(); - auto a1 = conflict.first; - auto a2 = conflict.second; - - msg.Printf( _( "Bus alias %s has conflicting definitions on multiple sheets: " ), - GetChars( a1->GetName() ) ); - - wxFileName f1 = a1->GetParent()->GetFileName(); - wxFileName f2 = a2->GetParent()->GetFileName(); - - msg << f1.GetFullName() << " and " << f2.GetFullName(); - - marker->SetData( ERCE_BUS_ALIAS_CONFLICT, wxPoint( 0, 0 ), - msg, wxPoint( 0, 0 ) ); - marker->SetMarkerType( MARKER_BASE::MARKER_ERC ); - marker->SetErrorLevel( MARKER_BASE::MARKER_SEVERITY_ERROR ); - - a2->GetParent()->Append( marker ); - - ++err_count; - } - } + aliases.insert( aliases.end(), screen_aliases.begin(), screen_aliases.end() ); } return err_count; @@ -313,7 +289,7 @@ int TestMultiunitFootprints( SCH_SHEET_LIST& aSheetList ) wxString fp; wxString unitName; - for( unsigned i = 0; i < component.second.GetCount(); ++i ) + for( int i = 0; i < component.second.GetCount(); ++i ) { SCH_COMPONENT* cmp = refList.GetItem( i ).GetComp(); SCH_SHEET_PATH sheetPath = refList.GetItem( i ).GetSheetPath(); @@ -327,7 +303,7 @@ int TestMultiunitFootprints( SCH_SHEET_LIST& aSheetList ) } } - for( unsigned i = 0; i < component.second.GetCount(); ++i ) + for( int i = 0; i < component.second.GetCount(); ++i ) { SCH_REFERENCE& ref = refList.GetItem( i ); SCH_COMPONENT* unit = ref.GetComp(); @@ -362,8 +338,7 @@ int TestMultiunitFootprints( SCH_SHEET_LIST& aSheetList ) } -void Diagnose( NETLIST_OBJECT* aNetItemRef, NETLIST_OBJECT* aNetItemTst, - int aMinConn, int aDiag ) +void Diagnose( NETLIST_OBJECT* aNetItemRef, NETLIST_OBJECT* aNetItemTst, int aMinConn, int aDiag ) { SCH_MARKER* marker = NULL; SCH_SCREEN* screen; @@ -403,10 +378,7 @@ void Diagnose( NETLIST_OBJECT* aNetItemRef, NETLIST_OBJECT* aNetItemTst, GetChars( GetText( ii ) ), GetChars( cmp_ref ), aNetItemRef->GetNet() ); - marker->SetData( ERCE_PIN_NOT_DRIVEN, - aNetItemRef->m_Start, - msg, - aNetItemRef->m_Start ); + marker->SetData( ERCE_PIN_NOT_DRIVEN, aNetItemRef->m_Start, msg, aNetItemRef->m_Start ); return; } } @@ -442,8 +414,7 @@ void Diagnose( NETLIST_OBJECT* aNetItemRef, NETLIST_OBJECT* aNetItemTst, } -void TestOthersItems( NETLIST_OBJECT_LIST* aList, - unsigned aNetItemRef, unsigned aNetStart, +void TestOthersItems( NETLIST_OBJECT_LIST* aList, unsigned aNetItemRef, unsigned aNetStart, int* aMinConnexion ) { unsigned netItemTst = aNetStart; @@ -597,17 +568,12 @@ int NETLIST_OBJECT_LIST::CountPinsInNet( unsigned aNetStart ) bool WriteDiagnosticERC( EDA_UNITS_T aUnits, const wxString& aFullFileName ) { - wxString msg; - wxFFile file( aFullFileName, wxT( "wt" ) ); if( !file.IsOpened() ) return false; - msg = _( "ERC report" ); - msg << wxT(" (") << DateAndTime() << wxT( ", " ) - << _( "Encoding UTF8" ) << wxT( " )\n" ); - + wxString msg = wxString::Format( _( "ERC report (%s, Encoding UTF8)\n" ), DateAndTime() ); int err_count = 0; int warn_count = 0; int total_count = 0; @@ -724,7 +690,7 @@ struct compare_paths }; // Helper functions to build the warning messages about Similar Labels: -static int countIndenticalLabels( std::vector& aList, NETLIST_OBJECT* aLabel ); +static int countIndenticalLabels( std::vector& aList, NETLIST_OBJECT* aRef ); static void SimilarLabelsDiagnose( NETLIST_OBJECT* aItemA, NETLIST_OBJECT* aItemB ); @@ -851,28 +817,23 @@ void NETLIST_OBJECT_LIST::TestforSimilarLabels() // Helper function: count the number of labels identical to aLabel // for global label: global labels in the full project // for local label: all labels in the current sheet -static int countIndenticalLabels( std::vector& aList, NETLIST_OBJECT* aLabel ) +static int countIndenticalLabels( std::vector& aList, NETLIST_OBJECT* aRef ) { int count = 0; - if( aLabel->IsLabelGlobal() ) + if( aRef->IsLabelGlobal() ) { - for( unsigned netItem = 0; netItem < aList.size(); ++netItem ) + for( auto i : aList) { - NETLIST_OBJECT* item = aList[netItem]; - - if( item->IsLabelGlobal() && item->m_Label == aLabel->m_Label ) + if( i->IsLabelGlobal() && i->m_Label == aRef->m_Label ) count++; } } else { - for( unsigned netItem = 0; netItem < aList.size(); ++netItem ) + for( auto i : aList) { - NETLIST_OBJECT* item = aList[netItem]; - - if( item->m_Label == aLabel->m_Label && - item->m_SheetPath.Path() == aLabel->m_SheetPath.Path() ) + if( i->m_Label == aRef->m_Label && i->m_SheetPath.Path() == aRef->m_SheetPath.Path() ) count++; } } @@ -892,18 +853,17 @@ static void SimilarLabelsDiagnose( NETLIST_OBJECT* aItemA, NETLIST_OBJECT* aItem SCH_SCREEN* screen = aItemA->m_SheetPath.LastScreen(); screen->Append( marker ); - wxString fmt = aItemA->IsLabelGlobal() ? - _( "Global label \"%s\" (sheet \"%s\") looks like:" ) : - _( "Local label \"%s\" (sheet \"%s\") looks like:" ); + wxString fmt = aItemA->IsLabelGlobal() ? _( "Global label '%s' (sheet '%s') looks like:" ) : + _( "Local label '%s' (sheet '%s') looks like:" ); wxString msg; - msg.Printf( fmt, GetChars( aItemA->m_Label ), GetChars( aItemA->m_SheetPath.PathHumanReadable() ) ); + msg.Printf( fmt, aItemA->m_Label, aItemA->m_SheetPath.PathHumanReadable() ); marker->SetData( aItemA->IsLabelGlobal() && aItemB->IsLabelGlobal() ? ERCE_SIMILAR_GLBL_LABELS : ERCE_SIMILAR_LABELS, aItemA->m_Start, msg, aItemA->m_Start ); fmt = aItemB->IsLabelGlobal() ? _( "Global label \"%s\" (sheet \"%s\")" ) : _( "Local label \"%s\" (sheet \"%s\")" ); - msg.Printf( fmt, GetChars( aItemB->m_Label ), GetChars( aItemB->m_SheetPath.PathHumanReadable() ) ); + msg.Printf( fmt, aItemB->m_Label, aItemB->m_SheetPath.PathHumanReadable() ); marker->SetAuxiliaryData( msg, aItemB->m_Start ); } diff --git a/eeschema/sch_marker.cpp b/eeschema/sch_marker.cpp index 62532c93e8..ea6ba063b0 100644 --- a/eeschema/sch_marker.cpp +++ b/eeschema/sch_marker.cpp @@ -30,7 +30,6 @@ #include #include -#include /// Factor to convert the maker unit shape to internal units: #define SCALING_FACTOR Millimeter2iu( 0.1 ) @@ -65,8 +64,7 @@ void SCH_MARKER::SwapData( SCH_ITEM* aItem ) void SCH_MARKER::Show( int nestLevel, std::ostream& os ) const { // for now, make it look like XML: - NestedSpace( nestLevel, os ) << '<' << GetClass().Lower().mb_str() - << GetPos() << "/>\n"; + NestedSpace( nestLevel, os ) << '<' << GetClass().Lower().mb_str() << GetPos() << "/>\n"; } #endif @@ -97,8 +95,9 @@ bool SCH_MARKER::Matches( wxFindReplaceData& aSearchData, void* aAuxData ) void SCH_MARKER::ViewGetLayers( int aLayers[], int& aCount ) const { - aCount = 1; - aLayers[0] = this->m_ErrorLevel == MARKER_SEVERITY_ERROR ? LAYER_ERC_ERR : LAYER_ERC_WARN; + aCount = 2; + aLayers[0] = this->m_ErrorLevel == MARKER_SEVERITY_ERROR ? LAYER_ERC_ERR : LAYER_ERC_WARN; + aLayers[1] = LAYER_SELECTION_SHADOWS; } diff --git a/eeschema/sch_painter.cpp b/eeschema/sch_painter.cpp index fe5f316361..40dbb733a2 100644 --- a/eeschema/sch_painter.cpp +++ b/eeschema/sch_painter.cpp @@ -1622,13 +1622,14 @@ void SCH_PAINTER::draw( SCH_MARKER *aMarker, int aLayer ) if( drawingShadows && !aMarker->IsSelected() ) return; - int layer = LAYER_ERC_WARN; - if( aMarker->GetErrorLevel() == MARKER_BASE::MARKER_SEVERITY_ERROR ) - layer = LAYER_ERC_ERR; + aLayer = LAYER_ERC_ERR; + else + aLayer = LAYER_ERC_WARN; - COLOR4D color = getRenderColor( aMarker, layer, drawingShadows ); + COLOR4D color = getRenderColor( aMarker, aLayer, drawingShadows ); + m_gal->Save(); m_gal->Translate( aMarker->GetPosition() ); m_gal->SetIsFill( !drawingShadows ); m_gal->SetFillColor( color ); @@ -1640,6 +1641,7 @@ void SCH_PAINTER::draw( SCH_MARKER *aMarker, int aLayer ) aMarker->ShapeToPolygon( polygon ); m_gal->DrawPolygon( polygon ); + m_gal->Restore(); }