From 552f053a557a760ed2438485ca69aa571eb8ecdb Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Thu, 23 Sep 2021 22:07:19 +0100 Subject: [PATCH] Remove pins from symbol hit-testing. Fixes https://gitlab.com/kicad/code/kicad/issues/8508 --- eeschema/autoplace_fields.cpp | 4 +- eeschema/cross-probing.cpp | 3 +- eeschema/lib_symbol.cpp | 12 +++- eeschema/lib_symbol.h | 2 +- eeschema/sch_edit_frame.cpp | 12 +--- .../cadstar/cadstar_sch_archive_loader.cpp | 2 +- eeschema/sch_symbol.cpp | 58 +++++++++---------- eeschema/sch_symbol.h | 11 +++- eeschema/tools/ee_selection.cpp | 2 +- 9 files changed, 55 insertions(+), 51 deletions(-) diff --git a/eeschema/autoplace_fields.cpp b/eeschema/autoplace_fields.cpp index 84e7217142..7592482f8c 100644 --- a/eeschema/autoplace_fields.cpp +++ b/eeschema/autoplace_fields.cpp @@ -129,7 +129,7 @@ public: m_align_to_grid = cfg->m_AutoplaceFields.align_to_grid; } - m_symbol_bbox = m_symbol->GetBodyBoundingBox(); + m_symbol_bbox = m_symbol->GetBodyAndPinsBoundingBox(); m_fbox_size = computeFBoxSize( /* aDynamic */ true ); m_is_power_symbol = !m_symbol->IsInNetlist(); @@ -294,7 +294,7 @@ protected: EDA_RECT item_box; if( SCH_SYMBOL* item_comp = dynamic_cast( item ) ) - item_box = item_comp->GetBodyBoundingBox(); + item_box = item_comp->GetBodyAndPinsBoundingBox(); else item_box = item->GetBoundingBox(); diff --git a/eeschema/cross-probing.cpp b/eeschema/cross-probing.cpp index 10643bb437..b30924ee35 100644 --- a/eeschema/cross-probing.cpp +++ b/eeschema/cross-probing.cpp @@ -144,8 +144,7 @@ SCH_ITEM* SCH_EDITOR_CONTROL::FindSymbolAndItem( const wxString& aReference, #endif // COMP_1_TO_1_RATIO #ifndef COMP_1_TO_1_RATIO // Do the scaled zoom - // Pass "false" to only include visible fields of symbol in bbox calculations. - EDA_RECT bbox = symbol->GetBoundingBox( false ); + EDA_RECT bbox = symbol->GetBoundingBox(); wxSize bbSize = bbox.Inflate( bbox.GetWidth() * 0.2f ).GetSize(); VECTOR2D screenSize = getView()->GetViewport().GetSize(); diff --git a/eeschema/lib_symbol.cpp b/eeschema/lib_symbol.cpp index dbc5089b17..8732870851 100644 --- a/eeschema/lib_symbol.cpp +++ b/eeschema/lib_symbol.cpp @@ -848,7 +848,7 @@ void LIB_SYMBOL::ViewGetLayers( int aLayers[], int& aCount ) const } -const EDA_RECT LIB_SYMBOL::GetBodyBoundingBox( int aUnit, int aConvert ) const +const EDA_RECT LIB_SYMBOL::GetBodyBoundingBox( int aUnit, int aConvert, bool aIncludePins ) const { EDA_RECT bbox; @@ -864,9 +864,17 @@ const EDA_RECT LIB_SYMBOL::GetBodyBoundingBox( int aUnit, int aConvert ) const continue; if( item.Type() == LIB_PIN_T ) - bbox.Merge( static_cast( item ).GetBoundingBox( false, true ) ); + { + if( aIncludePins ) + { + const LIB_PIN& pin = static_cast( item ); + bbox.Merge( pin.GetBoundingBox( false, true ) ); + } + } else + { bbox.Merge( item.GetBoundingBox() ); + } } return bbox; diff --git a/eeschema/lib_symbol.h b/eeschema/lib_symbol.h index 68a697e62a..acccf72421 100644 --- a/eeschema/lib_symbol.h +++ b/eeschema/lib_symbol.h @@ -215,7 +215,7 @@ public: * if aConvert == 0 Convert is non used * Fields are not taken in account **/ - const EDA_RECT GetBodyBoundingBox( int aUnit, int aConvert ) const; + const EDA_RECT GetBodyBoundingBox( int aUnit, int aConvert, bool aIncludePins ) const; const EDA_RECT GetBoundingBox() const override { diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index 976fca7508..15a85e3d27 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -1500,16 +1500,8 @@ const BOX2I SCH_EDIT_FRAME::GetDocumentExtents( bool aIncludeAllVisible ) const for( EDA_ITEM* item : GetScreen()->Items() ) { if( item != dsAsItem ) // Ignore the drawing-sheet itself - { - if( item->Type() == SCH_SYMBOL_T ) - { - // For symbols we need to get the bounding box without invisible text - SCH_SYMBOL* symbol = static_cast( item ); - bBoxItems.Merge( symbol->GetBoundingBox( false ) ); - } - else - bBoxItems.Merge( item->GetBoundingBox() ); - } + bBoxItems.Merge( item->GetBoundingBox() ); + bBoxDoc = bBoxItems; } } diff --git a/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.cpp b/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.cpp index ae828112eb..dc88caf011 100644 --- a/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.cpp +++ b/eeschema/sch_plugins/cadstar/cadstar_sch_archive_loader.cpp @@ -166,7 +166,7 @@ void CADSTAR_SCH_ARCHIVE_LOADER::Load( SCHEMATIC* aSchematic, SCH_SHEET* aRootSh if( item->Type() == SCH_SYMBOL_T ) { SCH_SYMBOL* comp = static_cast( item ); - bbox = comp->GetBodyBoundingBox(); + bbox = comp->GetBodyAndPinsBoundingBox(); for( const SCH_FIELD& field : comp->GetFields() ) { diff --git a/eeschema/sch_symbol.cpp b/eeschema/sch_symbol.cpp index 0818faff19..54ad495683 100644 --- a/eeschema/sch_symbol.cpp +++ b/eeschema/sch_symbol.cpp @@ -1298,14 +1298,14 @@ void SCH_SYMBOL::Show( int nestLevel, std::ostream& os ) const #endif -EDA_RECT SCH_SYMBOL::GetBodyBoundingBox() const +EDA_RECT SCH_SYMBOL::doGetBoundingBox( bool aIncludePins, bool aIncludeFields ) const { EDA_RECT bBox; if( m_part ) - bBox = m_part->GetBodyBoundingBox( m_unit, m_convert ); + bBox = m_part->GetBodyBoundingBox( m_unit, m_convert, aIncludePins ); else - bBox = dummy()->GetBodyBoundingBox( m_unit, m_convert ); + bBox = dummy()->GetBodyBoundingBox( m_unit, m_convert, aIncludePins ); int x0 = bBox.GetX(); int xm = bBox.GetRight(); @@ -1329,35 +1329,35 @@ EDA_RECT SCH_SYMBOL::GetBodyBoundingBox() const bBox.Normalize(); bBox.Offset( m_pos ); + + if( aIncludeFields ) + { + for( const SCH_FIELD& field : m_fields ) + { + if( field.IsVisible() ) + bBox.Merge( field.GetBoundingBox() ); + } + } + return bBox; } +EDA_RECT SCH_SYMBOL::GetBodyBoundingBox() const +{ + return doGetBoundingBox( false, false ); +} + + +EDA_RECT SCH_SYMBOL::GetBodyAndPinsBoundingBox() const +{ + return doGetBoundingBox( true, false ); +} + + const EDA_RECT SCH_SYMBOL::GetBoundingBox() const { - EDA_RECT bbox = GetBodyBoundingBox(); - - for( const SCH_FIELD& field : m_fields ) - { - if( field.IsVisible() ) - bbox.Merge( field.GetBoundingBox() ); - } - - return bbox; -} - - -const EDA_RECT SCH_SYMBOL::GetBoundingBox( bool aIncludeInvisibleText ) const -{ - EDA_RECT bbox = GetBodyBoundingBox(); - - for( const SCH_FIELD& field : m_fields ) - { - if( field.IsVisible() || aIncludeInvisibleText ) - bbox.Merge( field.GetBoundingBox() ); - } - - return bbox; + return doGetBoundingBox( true, true ); } @@ -1716,10 +1716,10 @@ bool SCH_SYMBOL::operator <( const SCH_ITEM& aItem ) const auto symbol = static_cast( &aItem ); - EDA_RECT rect = GetBodyBoundingBox(); + EDA_RECT rect = GetBodyAndPinsBoundingBox(); - if( rect.GetArea() != symbol->GetBodyBoundingBox().GetArea() ) - return rect.GetArea() < symbol->GetBodyBoundingBox().GetArea(); + if( rect.GetArea() != symbol->GetBodyAndPinsBoundingBox().GetArea() ) + return rect.GetArea() < symbol->GetBodyAndPinsBoundingBox().GetArea(); if( m_pos.x != symbol->m_pos.x ) return m_pos.x < symbol->m_pos.x; diff --git a/eeschema/sch_symbol.h b/eeschema/sch_symbol.h index 9db73be0c7..eb6baa8b25 100644 --- a/eeschema/sch_symbol.h +++ b/eeschema/sch_symbol.h @@ -315,13 +315,16 @@ public: const EDA_RECT GetBoundingBox() const override; - const EDA_RECT GetBoundingBox( bool aIncludeInvisibleText ) const; - /** - * Return a bounding box for the symbol body but not the fields. + * Return a bounding box for the symbol body but not the pins or fields. */ EDA_RECT GetBodyBoundingBox() const; + /** + * Return a bounding box for the symbol body and pins but not the fields. + */ + EDA_RECT GetBodyAndPinsBoundingBox() const; + //---------------------------------------------------------------- @@ -665,6 +668,8 @@ public: bool IsPointClickableAnchor( const wxPoint& aPos ) const override; private: + EDA_RECT doGetBoundingBox( bool aIncludePins, bool aIncludeFields ) const; + bool doIsConnected( const wxPoint& aPosition ) const override; void Init( const wxPoint& pos = wxPoint( 0, 0 ) ); diff --git a/eeschema/tools/ee_selection.cpp b/eeschema/tools/ee_selection.cpp index 6a20b79097..a4b65f347e 100644 --- a/eeschema/tools/ee_selection.cpp +++ b/eeschema/tools/ee_selection.cpp @@ -73,7 +73,7 @@ EDA_RECT EE_SELECTION::GetBoundingBox() const // so the exception is legit. try { - bbox.Merge( static_cast( item )->GetBoundingBox( false ) ); + bbox.Merge( static_cast( item )->GetBoundingBox() ); } catch( const boost::bad_pointer& exc ) {