From 996a5b85a6de3b96e07e1f10861f603144dfa5d1 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Thu, 4 Nov 2021 12:38:15 +0000 Subject: [PATCH] Overhaul IsJunctionNeeded logic to support all the different cases. Several clients were using this with different needs. The API is now more explicit (and there are more options under the hood). Fixes https://gitlab.com/kicad/code/kicad/issues/9518 --- eeschema/bus-wire-junction.cpp | 2 +- eeschema/sch_edit_frame.cpp | 2 +- eeschema/sch_line.cpp | 2 +- eeschema/sch_screen.cpp | 54 +++++++++++++++++++++-- eeschema/sch_screen.h | 29 ++++++++++-- eeschema/tools/sch_drawing_tools.cpp | 2 +- eeschema/tools/sch_edit_tool.cpp | 6 +-- eeschema/tools/sch_line_wire_bus_tool.cpp | 4 +- eeschema/tools/sch_move_tool.cpp | 2 +- 9 files changed, 86 insertions(+), 17 deletions(-) diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp index 69cae1cd8b..50bd7f4a79 100644 --- a/eeschema/bus-wire-junction.cpp +++ b/eeschema/bus-wire-junction.cpp @@ -170,7 +170,7 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( SCH_SCREEN* aScreen ) for( SCH_ITEM* item : aScreen->Items().OfType( SCH_JUNCTION_T ) ) { - if( !aScreen->IsJunctionNeeded( item->GetPosition() ) ) + if( !aScreen->IsExplicitJunction( item->GetPosition() ) ) remove_item( item ); else junctions.push_back( static_cast( item ) ); diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index 6254bd6cc4..ddada5a0bc 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -1214,7 +1214,7 @@ void SCH_EDIT_FRAME::AddItemToScreenAndUndoList( SCH_SCREEN* aScreen, SCH_ITEM* for( auto j = i + 1; j != pts.end(); j++ ) TrimWire( *i, *j ); - if( aScreen->IsJunctionNeeded( *i, true ) ) + if( aScreen->IsExplicitJunctionNeeded( *i ) ) AddJunction( aScreen, *i, true, false ); } diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp index 4d94751b7b..8d2e975f24 100644 --- a/eeschema/sch_line.cpp +++ b/eeschema/sch_line.cpp @@ -564,7 +564,7 @@ SCH_LINE* SCH_LINE::MergeOverlap( SCH_SCREEN* aScreen, SCH_LINE* aLine, bool aCh bool touching = leftmost_end == rightmost_start; - if( touching && aCheckJunctions && aScreen->IsJunctionNeeded( leftmost_end ) ) + if( touching && aCheckJunctions && aScreen->IsJunction( leftmost_end ) ) return nullptr; // Make a new segment that merges the 2 segments diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp index 6653baa4f2..f89b46e752 100644 --- a/eeschema/sch_screen.cpp +++ b/eeschema/sch_screen.cpp @@ -400,10 +400,55 @@ std::set SCH_SCREEN::MarkConnections( SCH_LINE* aSegment ) } -bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition, bool aNew ) const +bool SCH_SCREEN::IsJunction( const wxPoint& aPosition ) const +{ + bool hasExplicitJunction; + bool hasBusEntry; + bool isJunction = doIsJunction( aPosition, false, &hasExplicitJunction, &hasBusEntry ); + + return isJunction; +} + + +bool SCH_SCREEN::IsExplicitJunction( const wxPoint& aPosition ) const +{ + bool hasExplicitJunction; + bool hasBusEntry; + bool isJunction = doIsJunction( aPosition, false, &hasExplicitJunction, &hasBusEntry ); + + return isJunction && !hasBusEntry; +} + + +bool SCH_SCREEN::IsExplicitJunctionNeeded( const wxPoint& aPosition ) const +{ + bool hasExplicitJunction; + bool hasBusEntry; + bool isJunction = doIsJunction( aPosition, false, &hasExplicitJunction, &hasBusEntry ); + + return isJunction && !hasBusEntry && !hasExplicitJunction; +} + + +bool SCH_SCREEN::IsExplicitJunctionAllowed( const wxPoint& aPosition ) const +{ + bool hasExplicitJunction; + bool hasBusEntry; + bool isJunction = doIsJunction( aPosition, true, &hasExplicitJunction, &hasBusEntry ); + + return isJunction && !hasBusEntry; +} + + + +bool SCH_SCREEN::doIsJunction( const wxPoint& aPosition, bool aBreakCrossings, + bool* aHasExplicitJunctionDot, bool* aHasBusEntry ) const { enum layers { WIRES = 0, BUSES }; + *aHasExplicitJunctionDot = false; + *aHasBusEntry = false; + bool breakLines[ 2 ] = { false }; std::unordered_set exitAngles[ 2 ]; std::vector midPointLines[ 2 ]; @@ -419,8 +464,8 @@ bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition, bool aNew ) const switch( item->Type() ) { case SCH_JUNCTION_T: - if( aNew && item->HitTest( aPosition, -1 ) ) - return false; + if( item->HitTest( aPosition, -1 ) ) + *aHasExplicitJunctionDot = true; break; @@ -445,7 +490,7 @@ bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition, bool aNew ) const } else if( line->HitTest( aPosition, -1 ) ) { - if( aNew ) + if( aBreakCrossings ) breakLines[ layer ] = true; // Defer any line midpoints until we know whether or not we're breaking them @@ -461,6 +506,7 @@ bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition, bool aNew ) const exitAngles[ BUSES ].insert( uniqueAngle++ ); breakLines[ WIRES ] = true; exitAngles[ WIRES ].insert( uniqueAngle++ ); + *aHasBusEntry = true; } break; diff --git a/eeschema/sch_screen.h b/eeschema/sch_screen.h index 7e998becdc..f0ee458cef 100644 --- a/eeschema/sch_screen.h +++ b/eeschema/sch_screen.h @@ -310,19 +310,38 @@ public: size_t CountConnectedItems( const wxPoint& aPos, bool aTestJunctions ) const; /** - * Test if a junction is required for the items at \a aPosition on the screen. + * Test if a junction is required for the items at \a aPosition on the screen. Note that + * this coule be either an implied junction (bus entry) or an explicit junction (dot). * * A junction is required at \a aPosition if one of the following criteria is satisfied: * - One wire midpoint and one or more wire endpoints. * - Three or more wire endpoints. * - One wire midpoint and a symbol pin. * - Two or more wire endpoints and a symbol pin. + * - One bus midpoint or endpoint and a bus entry. * * @param[in] aPosition The position to test. - * @param aNew Checks if a _new_ junction is needed, i.e. there isn't one already * @return True if a junction is required at \a aPosition. */ - bool IsJunctionNeeded( const wxPoint& aPosition, bool aNew = false ) const; + bool IsJunction( const wxPoint& aPosition ) const; + + /** + * Indicates that a junction dot is necessary at the given location. See IsJunctionNeeded + * for more info. + */ + bool IsExplicitJunction( const wxPoint& aPosition ) const; + + /** + * Indicates that a junction dot is necessary at the given location, and does not yet exist. + * See IsJunctionNeeded for more info. + */ + bool IsExplicitJunctionNeeded( const wxPoint& aPosition ) const; + + /** + * Indicates that a juction dot may be placed at the given location. See IsJunctionNeeded + * for more info. + */ + bool IsExplicitJunctionAllowed( const wxPoint& aPosition ) const; /** * Test if \a aPosition is a connection point on \a aLayer. @@ -486,6 +505,10 @@ public: */ double m_LastZoomLevel; +private: + bool doIsJunction( const wxPoint& aPosition, bool aBreakCrossings, + bool* aHasExplicitJunctionDot, bool* aHasBusEntry ) const; + private: friend SCH_EDIT_FRAME; // Only to populate m_symbolInstances. friend SCH_SEXPR_PARSER; // Only to load instance information from schematic file. diff --git a/eeschema/tools/sch_drawing_tools.cpp b/eeschema/tools/sch_drawing_tools.cpp index f1a7a49bdb..43a266cebe 100644 --- a/eeschema/tools/sch_drawing_tools.cpp +++ b/eeschema/tools/sch_drawing_tools.cpp @@ -725,7 +725,7 @@ int SCH_DRAWING_TOOLS::SingleClickPlace( const TOOL_EVENT& aEvent ) { if( type == SCH_JUNCTION_T ) { - if( !m_frame->GetScreen()->IsJunctionNeeded( cursorPos, true ) ) + if( !m_frame->GetScreen()->IsExplicitJunctionAllowed( cursorPos ) ) { m_frame->ShowInfoBarError( _( "Junction location contains no joinable " "wires and/or pins." ) ); diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index 8dad9c3e24..6169d79d13 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -1018,14 +1018,14 @@ int SCH_EDIT_TOOL::DoDelete( const TOOL_EVENT& aEvent ) } } - for( auto point : pts ) + for( const wxPoint& point : pts ) { SCH_ITEM* junction = screen->GetItem( point, 0, SCH_JUNCTION_T ); if( !junction ) continue; - if( junction->HasFlag( STRUCT_DELETED ) || !screen->IsJunctionNeeded( point ) ) + if( junction->HasFlag( STRUCT_DELETED ) || !screen->IsExplicitJunction( point ) ) m_frame->DeleteJunction( junction, appendToUndo ); } @@ -1682,7 +1682,7 @@ int SCH_EDIT_TOOL::BreakWire( const TOOL_EVENT& aEvent ) if( !lines.empty() ) { - if( m_frame->GetScreen()->IsJunctionNeeded( cursorPos, true ) ) + if( m_frame->GetScreen()->IsExplicitJunctionNeeded( cursorPos ) ) m_frame->AddJunction( m_frame->GetScreen(), cursorPos, true, false ); m_frame->TestDanglingEnds(); diff --git a/eeschema/tools/sch_line_wire_bus_tool.cpp b/eeschema/tools/sch_line_wire_bus_tool.cpp index ec9498de38..dd825831c7 100644 --- a/eeschema/tools/sch_line_wire_bus_tool.cpp +++ b/eeschema/tools/sch_line_wire_bus_tool.cpp @@ -959,7 +959,7 @@ void SCH_LINE_WIRE_BUS_TOOL::finishSegments() for( const wxPoint& pt : new_ends ) { - if( m_frame->GetScreen()->IsJunctionNeeded( pt, true ) ) + if( m_frame->GetScreen()->IsExplicitJunctionNeeded( pt ) ) m_frame->AddJunction( m_frame->GetScreen(), pt, true, false ); } @@ -1024,7 +1024,7 @@ int SCH_LINE_WIRE_BUS_TOOL::AddJunctionsIfNeeded( const TOOL_EVENT& aEvent ) for( const wxPoint& point : pts ) { - if( m_frame->GetScreen()->IsJunctionNeeded( point, true ) ) + if( m_frame->GetScreen()->IsExplicitJunctionNeeded( point ) ) m_frame->AddJunction( m_frame->GetScreen(), point, true, false ); } diff --git a/eeschema/tools/sch_move_tool.cpp b/eeschema/tools/sch_move_tool.cpp index 80d6e2387f..f1d79a9df5 100644 --- a/eeschema/tools/sch_move_tool.cpp +++ b/eeschema/tools/sch_move_tool.cpp @@ -497,7 +497,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) // to denote the state. for( const DANGLING_END_ITEM& it : internalPoints ) { - if( m_frame->GetScreen()->IsJunctionNeeded( it.GetPosition(), true ) ) + if( m_frame->GetScreen()->IsExplicitJunctionNeeded( it.GetPosition()) ) m_frame->AddJunction( m_frame->GetScreen(), it.GetPosition(), true, false ); }