From c12cc19ef410d699747df6731f3d7ebf95e20e4b Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 6 Oct 2025 21:15:34 +0100 Subject: [PATCH] Rename "Expand Selection" menu item in selection tool. (We have a different "Expand Selection" command now, and this was never a great name for showing more choices anyway.) Also treat tablecells as opaque when guessing selection candidates. It's unlikely there's anything under them and keeping to their text bounds feels counter-intuitive. --- common/eda_shape.cpp | 40 ++++++--------------------- common/tool/selection_tool.cpp | 23 +++++++++------ eeschema/sch_tablecell.h | 5 ++++ eeschema/tools/sch_selection_tool.cpp | 16 ++--------- include/eda_shape.h | 15 +++++----- pcbnew/collectors.cpp | 28 +++++++++---------- pcbnew/pcb_tablecell.h | 5 ++++ pcbnew/tools/pcb_selection_tool.cpp | 20 ++++++++++---- 8 files changed, 71 insertions(+), 81 deletions(-) diff --git a/common/eda_shape.cpp b/common/eda_shape.cpp index 70614cc436..23b967e058 100644 --- a/common/eda_shape.cpp +++ b/common/eda_shape.cpp @@ -58,7 +58,6 @@ EDA_SHAPE::EDA_SHAPE( SHAPE_T aType, int aLineWidth, FILL_T aFill ) : m_rectangleHeight( 0 ), m_rectangleWidth( 0 ), m_cornerRadius( 0 ), - m_segmentLength( 0 ), m_editState( 0 ), m_proxyItem( false ) { @@ -78,7 +77,6 @@ EDA_SHAPE::EDA_SHAPE( const SHAPE& aShape ) : m_rectangleHeight( 0 ), m_rectangleWidth( 0 ), m_cornerRadius( 0 ), - m_segmentLength( 0 ), m_editState( 0 ), m_proxyItem( false ) { @@ -463,20 +461,6 @@ void EDA_SHAPE::SetCornerRadius( int aRadius ) } -void EDA_SHAPE::SetLength( const double& aLength ) -{ - switch( m_shape ) - { - case SHAPE_T::SEGMENT: - m_segmentLength = aLength; - break; - - default: - UNIMPLEMENTED_FOR( SHAPE_T_asString() ); - } -} - - void EDA_SHAPE::SetRectangleHeight( const int& aHeight ) { switch ( m_shape ) @@ -522,20 +506,6 @@ void EDA_SHAPE::SetRectangle( const long long int& aHeight, const long long int& } -void EDA_SHAPE::SetSegmentAngle( const EDA_ANGLE& aAngle ) -{ - switch( m_shape ) - { - case SHAPE_T::SEGMENT: - m_segmentAngle = aAngle; - break; - - default: - UNIMPLEMENTED_FOR( SHAPE_T_asString() ); - } -} - - bool EDA_SHAPE::IsClosed() const { switch( m_shape ) @@ -1836,11 +1806,17 @@ void EDA_SHAPE::SetPolyPoints( const std::vector& aPoints ) } -std::vector EDA_SHAPE::makeEffectiveShapes( bool aEdgeOnly, bool aLineChainOnly ) const +std::vector EDA_SHAPE::makeEffectiveShapes( bool aEdgeOnly, bool aLineChainOnly, bool aHittesting ) const { std::vector effectiveShapes; int width = GetEffectiveWidth(); - bool solidFill = ( IsSolidFill() || IsHatchedFill() || IsProxyItem() ) && !aEdgeOnly; + bool solidFill = IsSolidFill() + || IsHatchedFill() + || IsProxyItem() + || ( aHittesting && IsFilledForHitTesting() ); + + if( aEdgeOnly ) + solidFill = false; switch( m_shape ) { diff --git a/common/tool/selection_tool.cpp b/common/tool/selection_tool.cpp index 34e69530c4..bd0fdec7ec 100644 --- a/common/tool/selection_tool.cpp +++ b/common/tool/selection_tool.cpp @@ -261,18 +261,24 @@ bool SELECTION_TOOL::doSelectionMenu( COLLECTOR* aCollector ) EDA_ITEM* current = nullptr; SELECTION highlightGroup; bool selectAll = false; - bool expandSelection = false; + bool showMoreChoices = false; highlightGroup.SetLayer( LAYER_SELECT_OVERLAY ); getView()->Add( &highlightGroup ); do { - /// The user has requested the full, non-limited list of selection items - if( expandSelection ) + /// This must be the second time through the loop, and the user has requested the full, + /// non-limited list of selection items + if( showMoreChoices ) + { aCollector->Combine(); - expandSelection = false; + // prime event loop so we don't have to wait around for a mouse-moved + m_toolMgr->PrimeTool( { 0, 0 } ); + + showMoreChoices = false; + } int limit = std::min( 100, aCollector->GetCount() ); ACTION_MENU menu( true ); @@ -306,8 +312,8 @@ bool SELECTION_TOOL::doSelectionMenu( COLLECTOR* aCollector ) menu.AppendSeparator(); menu.Add( _( "Select &All\tA" ), limit + 1, BITMAPS::INVALID_BITMAP ); - if( !expandSelection && aCollector->HasAdditionalItems() ) - menu.Add( _( "&Expand Selection\tE" ), limit + 2, BITMAPS::INVALID_BITMAP ); + if( !showMoreChoices && aCollector->HasAdditionalItems() ) + menu.Add( _( "Show &More Choices...\tM" ), limit + 2, BITMAPS::INVALID_BITMAP ); if( aCollector->m_MenuTitle.Length() ) { @@ -387,7 +393,7 @@ bool SELECTION_TOOL::doSelectionMenu( COLLECTOR* aCollector ) { selectAll = false; current = nullptr; - expandSelection = true; + showMoreChoices = true; } // User has selected an item, so this one will be returned else if( id && ( *id > 0 ) && ( *id <= limit ) ) @@ -410,7 +416,8 @@ bool SELECTION_TOOL::doSelectionMenu( COLLECTOR* aCollector ) getView()->UpdateItems(); getEditFrame()->GetCanvas()->Refresh(); } - } while( expandSelection ); + } + while( showMoreChoices ); getView()->Remove( &highlightGroup ); diff --git a/eeschema/sch_tablecell.h b/eeschema/sch_tablecell.h index 3d76b973cd..ad5f96714d 100644 --- a/eeschema/sch_tablecell.h +++ b/eeschema/sch_tablecell.h @@ -73,6 +73,11 @@ public: int GetColumnWidth() const; void SetColumnWidth( int aWidth ); + bool IsFilledForHitTesting() const override + { + return true; + } + void Plot( PLOTTER* aPlotter, bool aBackground, const SCH_PLOT_OPTS& aPlotOpts, int aUnit, int aBodyStyle, const VECTOR2I& aOffset, bool aDimmed ) override; diff --git a/eeschema/tools/sch_selection_tool.cpp b/eeschema/tools/sch_selection_tool.cpp index df62aea0a1..84cae20b2a 100644 --- a/eeschema/tools/sch_selection_tool.cpp +++ b/eeschema/tools/sch_selection_tool.cpp @@ -1828,8 +1828,7 @@ void SCH_SELECTION_TOOL::GuessSelectionCandidates( SCH_COLLECTOR& collector, con } } - field->GetEffectiveTextShape( false, box, orient ) - ->Collide( poss, INT_MAX / 4, &dist ); + field->GetEffectiveTextShape( false, box, orient )->Collide( poss, INT_MAX / 4, &dist ); } else if( text ) { @@ -1837,18 +1836,9 @@ void SCH_SELECTION_TOOL::GuessSelectionCandidates( SCH_COLLECTOR& collector, con } else if( shape ) { - std::vector shapes = shape->MakeEffectiveShapes(); + auto shapes = std::make_shared( shape->MakeEffectiveShapesForHitTesting() ); - for( SHAPE* s : shapes ) - { - int shapeDist = dist; - s->Collide( poss, INT_MAX / 4, &shapeDist ); - - if( shapeDist < dist ) - dist = shapeDist; - - delete s; - } + shapes->Collide( poss, INT_MAX / 4, &dist ); // Filled shapes win hit tests anywhere inside them dominating = shape->IsFilledForHitTesting(); diff --git a/include/eda_shape.h b/include/eda_shape.h index 555d635584..a8005f77a0 100644 --- a/include/eda_shape.h +++ b/include/eda_shape.h @@ -381,9 +381,12 @@ public: return makeEffectiveShapes( aEdgeOnly ); } - void ShapeGetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector& aList ); + virtual std::vector MakeEffectiveShapesForHitTesting() const + { + return makeEffectiveShapes( false, false, true ); + } - void SetLength( const double& aLength ); + void ShapeGetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector& aList ); void SetRectangleHeight( const int& aHeight ); @@ -394,8 +397,6 @@ public: void SetCornerRadius( int aRadius ); int GetCornerRadius() const; - void SetSegmentAngle( const EDA_ANGLE& aAngle ); - bool IsClockwiseArc() const; /** @@ -483,7 +484,8 @@ protected: * lineChain rather than a closed polygon. */ // fixme: move to shape_compound - std::vector makeEffectiveShapes( bool aEdgeOnly, bool aLineChainOnly = false ) const; + std::vector makeEffectiveShapes( bool aEdgeOnly, bool aLineChainOnly = false, + bool aHittesting = false ) const; virtual int getMaxError() const { return 100; } @@ -501,9 +503,6 @@ protected: long long int m_rectangleWidth; int m_cornerRadius; - double m_segmentLength; - EDA_ANGLE m_segmentAngle; - VECTOR2I m_start; // Line start point or Circle center VECTOR2I m_end; // Line end point or Circle 3 o'clock point diff --git a/pcbnew/collectors.cpp b/pcbnew/collectors.cpp index 83210dcc25..41f24ca6fc 100644 --- a/pcbnew/collectors.cpp +++ b/pcbnew/collectors.cpp @@ -393,7 +393,7 @@ INSPECT_RESULT GENERAL_COLLECTOR::Inspect( EDA_ITEM* aTestItem, void* aTestData { if( zone->HitTestForCorner( m_refPos, accuracy * 2 ) || zone->HitTestForEdge( m_refPos, accuracy ) ) { - Append( aTestItem ); + Append( zone ); return INSPECT_RESULT::CONTINUE; } else if( !m_Guide->IgnoreZoneFills() ) @@ -402,7 +402,7 @@ INSPECT_RESULT GENERAL_COLLECTOR::Inspect( EDA_ITEM* aTestItem, void* aTestData { if( m_Guide->IsLayerVisible( layer ) && zone->HitTestFilledArea( layer, m_refPos ) ) { - Append( aTestItem ); + Append( zone ); return INSPECT_RESULT::CONTINUE; } } @@ -412,15 +412,15 @@ INSPECT_RESULT GENERAL_COLLECTOR::Inspect( EDA_ITEM* aTestItem, void* aTestData { if( footprint->HitTest( m_refPos, accuracy ) && footprint->HitTestAccurate( m_refPos, accuracy ) ) { - Append( aTestItem ); + Append( footprint ); return INSPECT_RESULT::CONTINUE; } } else if( pad || via ) { - if( aTestItem->HitTest( m_refPos, accuracy ) ) + if( boardItem->HitTest( m_refPos, accuracy ) ) { - Append( aTestItem ); + Append( boardItem ); return INSPECT_RESULT::CONTINUE; } } @@ -437,9 +437,9 @@ INSPECT_RESULT GENERAL_COLLECTOR::Inspect( EDA_ITEM* aTestItem, void* aTestData accuracy = KiROUND( accuracy * 1.5 ); } - if( aTestItem->HitTest( m_refPos, accuracy ) ) + if( boardItem->HitTest( m_refPos, accuracy ) ) { - Append( aTestItem ); + Append( boardItem ); return INSPECT_RESULT::CONTINUE; } } @@ -464,7 +464,7 @@ INSPECT_RESULT GENERAL_COLLECTOR::Inspect( EDA_ITEM* aTestItem, void* aTestData { if( zone->HitTestForCorner( m_refPos, accuracy * 2 ) || zone->HitTestForEdge( m_refPos, accuracy ) ) { - Append2nd( aTestItem ); + Append2nd( zone ); return INSPECT_RESULT::CONTINUE; } else if( !m_Guide->IgnoreZoneFills() ) @@ -473,7 +473,7 @@ INSPECT_RESULT GENERAL_COLLECTOR::Inspect( EDA_ITEM* aTestItem, void* aTestData { if( m_Guide->IsLayerVisible( layer ) && zone->HitTestFilledArea( layer, m_refPos ) ) { - Append2nd( aTestItem ); + Append2nd( zone ); return INSPECT_RESULT::CONTINUE; } } @@ -486,15 +486,15 @@ INSPECT_RESULT GENERAL_COLLECTOR::Inspect( EDA_ITEM* aTestItem, void* aTestData if( footprint->HitTest( m_refPos, accuracy ) && footprint->HitTestAccurate( m_refPos, accuracy ) ) { - Append2nd( aTestItem ); + Append2nd( footprint ); return INSPECT_RESULT::CONTINUE; } } else if( pad || via ) { - if( aTestItem->HitTest( m_refPos, accuracy ) ) + if( boardItem->HitTest( m_refPos, accuracy ) ) { - Append2nd( aTestItem ); + Append2nd( boardItem ); return INSPECT_RESULT::CONTINUE; } } @@ -507,9 +507,9 @@ INSPECT_RESULT GENERAL_COLLECTOR::Inspect( EDA_ITEM* aTestItem, void* aTestData accuracy = KiROUND( accuracy * 1.5 ); } - if( aTestItem->HitTest( m_refPos, accuracy ) ) + if( boardItem->HitTest( m_refPos, accuracy ) ) { - Append2nd( aTestItem ); + Append2nd( boardItem ); return INSPECT_RESULT::CONTINUE; } } diff --git a/pcbnew/pcb_tablecell.h b/pcbnew/pcb_tablecell.h index 2f377e826c..a1f91891c5 100644 --- a/pcbnew/pcb_tablecell.h +++ b/pcbnew/pcb_tablecell.h @@ -77,6 +77,11 @@ public: int GetColumnWidth() const; void SetColumnWidth( int aWidth ); + bool IsFilledForHitTesting() const override + { + return true; + } + wxString GetItemDescription( UNITS_PROVIDER* aUnitsProvider, bool aFull ) const override; void GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector& aList ) override; diff --git a/pcbnew/tools/pcb_selection_tool.cpp b/pcbnew/tools/pcb_selection_tool.cpp index bcf93cf721..e7d84c6ede 100644 --- a/pcbnew/tools/pcb_selection_tool.cpp +++ b/pcbnew/tools/pcb_selection_tool.cpp @@ -3545,7 +3545,6 @@ int PCB_SELECTION_TOOL::hitTestDistance( const VECTOR2I& aWhere, BOARD_ITEM* aIt } case PCB_TEXTBOX_T: - case PCB_TABLECELL_T: { PCB_TEXTBOX* textbox = static_cast( aItem ); @@ -3556,17 +3555,26 @@ int PCB_SELECTION_TOOL::hitTestDistance( const VECTOR2I& aWhere, BOARD_ITEM* aIt break; } + case PCB_TABLECELL_T: + { + PCB_TABLECELL* tablecell = static_cast( aItem ); + auto shape = std::make_shared( tablecell->MakeEffectiveShapesForHitTesting() ); + + shape->Collide( loc, aMaxDistance, &distance ); + + break; + } + case PCB_TABLE_T: { PCB_TABLE* table = static_cast( aItem ); + distance = aMaxDistance; for( PCB_TABLECELL* cell : table->GetCells() ) - { - // Add a bit of slop to text-shapes - if( cell->GetEffectiveTextShape()->Collide( loc, aMaxDistance, &distance ) ) - distance = std::clamp( distance - ( aMaxDistance / 2 ), 0, distance ); - } + distance = std::min( distance, hitTestDistance( aWhere, cell, aMaxDistance ) ); + // Tables should defer to their table cells. Never consider them exact. + distance = std::clamp( distance + ( aMaxDistance / 4 ), 0, aMaxDistance ); break; }