From 3a67dceab21e520dc22b1bb0cf2ad382fbccfc2e Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Fri, 10 Oct 2025 13:09:46 -0700 Subject: [PATCH] Fix polygon point editor behavior to avoid hard limits When dragging a vertex, use the 45/90 limits as guides and display the full guidelines. When dragging a midpoint, do not use these limits. Keep the standard converging limit --- common/preview_items/construction_geom.cpp | 15 ++- common/tool/construction_manager.cpp | 2 +- common/tool/grid_helper.cpp | 4 + common/tool/point_editor_behavior.cpp | 6 +- include/tool/edit_constraints.h | 3 + include/tool/grid_helper.h | 2 + pcbnew/tools/pcb_point_editor.cpp | 122 ++++++++++++++++++++- 7 files changed, 146 insertions(+), 8 deletions(-) diff --git a/common/preview_items/construction_geom.cpp b/common/preview_items/construction_geom.cpp index 5604649392..da1b2a3317 100644 --- a/common/preview_items/construction_geom.cpp +++ b/common/preview_items/construction_geom.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -97,7 +98,7 @@ void CONSTRUCTION_GEOM::ViewDraw( int aLayer, VIEW* aView ) const for( const DRAWABLE_INFO& drawable : m_drawables ) { gal.SetStrokeColor( drawable.IsPersistent ? m_persistentColor : m_color ); - gal.SetLineWidth( drawable.LineWidth ); + gal.SetLineWidth( drawable.LineWidth / gal.GetWorldScale() ); std::visit( [&]( const auto& visited ) @@ -149,9 +150,19 @@ void CONSTRUCTION_GEOM::ViewDraw( int aLayer, VIEW* aView ) const for( const SNAP_GUIDE& guide : m_snapGuides ) { + const SEG& segment = guide.Segment; + + if( segment.A == segment.B ) + continue; + + std::optional clipped = KIGEOM::ClipLineToBox( LINE( segment ), viewport ); + + if( !clipped ) + continue; + gal.SetStrokeColor( guide.Color ); gal.SetLineWidth( guide.LineWidth ); - gal.DrawLine( guide.Segment.A, guide.Segment.B ); + gal.DrawLine( clipped->A, clipped->B ); } if( haveSnapLine ) diff --git a/common/tool/construction_manager.cpp b/common/tool/construction_manager.cpp index df6c17cc5a..7d70eadadb 100644 --- a/common/tool/construction_manager.cpp +++ b/common/tool/construction_manager.cpp @@ -681,7 +681,7 @@ void SNAP_MANAGER::UpdateSnapGuides() if( activeDirection && *activeDirection == static_cast( ii ) ) { - guide.LineWidth = 2; + guide.LineWidth = 5; guide.Color = m_snapGuideHighlightColor; } else diff --git a/common/tool/grid_helper.cpp b/common/tool/grid_helper.cpp index 42e7897b89..c1bdff3249 100644 --- a/common/tool/grid_helper.cpp +++ b/common/tool/grid_helper.cpp @@ -142,6 +142,10 @@ void GRID_HELPER::SetSnapLineOrigin( const VECTOR2I& aOrigin ) m_snapManager.GetSnapLineManager().SetSnapLineOrigin( aOrigin ); } +void GRID_HELPER::SetSnapLineEnd( const std::optional& aEnd ) +{ + m_snapManager.GetSnapLineManager().SetSnapLineEnd( aEnd ); +} void GRID_HELPER::ClearSnapLine() { diff --git a/common/tool/point_editor_behavior.cpp b/common/tool/point_editor_behavior.cpp index 5834ffc42a..b643750472 100644 --- a/common/tool/point_editor_behavior.cpp +++ b/common/tool/point_editor_behavior.cpp @@ -52,7 +52,7 @@ void POLYGON_POINT_EDIT_BEHAVIOR::BuildForPolyOutline( EDIT_POINTS& aPo else aPoints.AddLine( aPoints.Point( i ), aPoints.Point( i + 1 ) ); - aPoints.Line( i ).SetConstraint( new EC_PERPLINE( aPoints.Line( i ) ) ); + aPoints.Line( i ).SetConstraint( new EC_CONVERGING( aPoints.Line( i ), aPoints ) ); } // The last missing line, connecting the last and the first polygon point @@ -60,7 +60,7 @@ void POLYGON_POINT_EDIT_BEHAVIOR::BuildForPolyOutline( EDIT_POINTS& aPo aPoints.Point( aPoints.GetContourStartIdx( cornersCount - 1 ) ) ); aPoints.Line( aPoints.LinesSize() - 1 ) - .SetConstraint( new EC_PERPLINE( aPoints.Line( aPoints.LinesSize() - 1 ) ) ); + .SetConstraint( new EC_CONVERGING( aPoints.Line( aPoints.LinesSize() - 1 ), aPoints ) ); } @@ -94,7 +94,7 @@ void POLYGON_POINT_EDIT_BEHAVIOR::UpdateOutlineFromPoints( SHAPE_POLY_SET& aOu for( unsigned i = 0; i < aPoints.LinesSize(); ++i ) { if( !isModified( aEditedPoint, aPoints.Line( i ) ) ) - aPoints.Line( i ).SetConstraint( new EC_PERPLINE( aPoints.Line( i ) ) ); + aPoints.Line( i ).SetConstraint( new EC_CONVERGING( aPoints.Line( i ), aPoints ) ); } } diff --git a/include/tool/edit_constraints.h b/include/tool/edit_constraints.h index e2df7d3154..829193ef1b 100644 --- a/include/tool/edit_constraints.h +++ b/include/tool/edit_constraints.h @@ -194,6 +194,9 @@ public: /// @copydoc EDIT_CONSTRAINT::Apply() virtual void Apply( EDIT_POINT& aHandle, const GRID_HELPER& aGrid ) override; + const EDIT_POINT& GetConstrainer() const { return m_constrainer; } + VECTOR2I GetLineVector() const { return m_line; } + private: const EDIT_POINT& m_constrainer; ///< Point that imposes the constraint. VECTOR2I m_line; ///< Vector representing the constraining line. diff --git a/include/tool/grid_helper.h b/include/tool/grid_helper.h index 884372819c..659ffdb03a 100644 --- a/include/tool/grid_helper.h +++ b/include/tool/grid_helper.h @@ -25,6 +25,7 @@ #define GRID_HELPER_H #include +#include #include #include @@ -126,6 +127,7 @@ public: void SetSnapLine( bool aSnap ) { m_enableSnapLine = aSnap; } void SetSnapLineDirections( const std::vector& aDirections ); void SetSnapLineOrigin( const VECTOR2I& aOrigin ); + void SetSnapLineEnd( const std::optional& aEnd ); void ClearSnapLine(); std::optional SnapToConstructionLines( const VECTOR2I& aPoint, const VECTOR2I& aNearestGrid, diff --git a/pcbnew/tools/pcb_point_editor.cpp b/pcbnew/tools/pcb_point_editor.cpp index 25f709c54f..44db0f4174 100644 --- a/pcbnew/tools/pcb_point_editor.cpp +++ b/pcbnew/tools/pcb_point_editor.cpp @@ -70,6 +70,47 @@ using namespace std::placeholders; const unsigned int PCB_POINT_EDITOR::COORDS_PADDING = pcbIUScale.mmToIU( 20 ); +static void appendDirection( std::vector& aDirections, const VECTOR2I& aDirection ) +{ + if( aDirection.x != 0 || aDirection.y != 0 ) + aDirections.push_back( aDirection ); +} + +static std::vector getConstraintDirections( EDIT_CONSTRAINT* aConstraint ) +{ + std::vector directions; + + if( !aConstraint ) + return directions; + + if( dynamic_cast( aConstraint ) ) + { + appendDirection( directions, VECTOR2I( 1, 0 ) ); + appendDirection( directions, VECTOR2I( 0, 1 ) ); + } + else if( dynamic_cast( aConstraint ) ) + { + appendDirection( directions, VECTOR2I( 1, 0 ) ); + appendDirection( directions, VECTOR2I( 0, 1 ) ); + appendDirection( directions, VECTOR2I( 1, 1 ) ); + appendDirection( directions, VECTOR2I( 1, -1 ) ); + } + else if( dynamic_cast( aConstraint ) ) + { + appendDirection( directions, VECTOR2I( 0, 1 ) ); + } + else if( dynamic_cast( aConstraint ) ) + { + appendDirection( directions, VECTOR2I( 1, 0 ) ); + } + else if( EC_LINE* lineConstraint = dynamic_cast( aConstraint ) ) + { + appendDirection( directions, lineConstraint->GetLineVector() ); + } + + return directions; +} + // Few constants to avoid using bare numbers for point indices enum RECT_POINTS { @@ -2158,6 +2199,39 @@ int PCB_POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent ) updateEditedPoint( aEvent ); bool inDrag = false; bool isConstrained = false; + bool haveSnapLineDirections = false; + + auto updateSnapLineDirections = + [&]() + { + std::vector directions; + + if( inDrag && m_editedPoint ) + { + EDIT_CONSTRAINT* constraint = nullptr; + + if( m_altConstraint ) + constraint = m_altConstraint.get(); + else if( m_editedPoint->IsConstrained() ) + constraint = m_editedPoint->GetConstraint(); + + directions = getConstraintDirections( constraint ); + } + + if( directions.empty() ) + { + grid.SetSnapLineDirections( {} ); + grid.SetSnapLineEnd( std::nullopt ); + haveSnapLineDirections = false; + } + else + { + grid.SetSnapLineDirections( directions ); + grid.SetSnapLineOrigin( m_original.GetPosition() ); + grid.SetSnapLineEnd( std::nullopt ); + haveSnapLineDirections = true; + } + }; BOARD_COMMIT commit( editFrame ); @@ -2235,6 +2309,8 @@ int PCB_POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent ) clones.emplace_back( clone ); grid.AddConstructionItems( { clone }, false, true ); + + updateSnapLineDirections(); } bool need_constraint = Is45Limited() || Is90Limited(); @@ -2243,6 +2319,7 @@ int PCB_POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent ) { setAltConstraint( need_constraint ); isConstrained = need_constraint; + updateSnapLineDirections(); } // Keep point inside of limits with some padding @@ -2319,16 +2396,20 @@ int PCB_POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent ) } } + bool constraintSnapped = false; + // Apply 45 degree or other constraints if( !m_angleSnapActive && m_altConstraint ) { m_editedPoint->SetPosition( pos ); m_altConstraint->Apply( grid ); + constraintSnapped = true; } else if( !m_angleSnapActive && m_editedPoint->IsConstrained() ) { m_editedPoint->SetPosition( pos ); m_editedPoint->ApplyConstraint( grid ); + constraintSnapped = true; } else if( !m_angleSnapActive && m_editedPoint->GetGridConstraint() == SNAP_TO_GRID ) { @@ -2340,6 +2421,14 @@ int PCB_POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent ) m_editedPoint->SetPosition( pos ); } + if( haveSnapLineDirections ) + { + if( constraintSnapped ) + grid.SetSnapLineEnd( m_editedPoint->GetPosition() ); + else + grid.SetSnapLineEnd( std::nullopt ); + } + updateItem( commit ); getViewControls()->ForceCursorPosition( true, m_editedPoint->GetPosition() ); updatePoints(); @@ -2398,6 +2487,7 @@ int PCB_POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent ) getViewControls()->SetAutoPan( false ); setAltConstraint( false ); + updateSnapLineDirections(); if( m_editorBehavior ) m_editorBehavior->FinalizeItem( *m_editPoints, commit ); @@ -2428,6 +2518,7 @@ int PCB_POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent ) inDrag = false; frame()->UndoRedoBlock( false ); + updateSnapLineDirections(); m_toolMgr->PostAction( ACTIONS::reselectItem, item ); // FIXME: Needed for generators } @@ -2451,6 +2542,7 @@ int PCB_POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent ) inDrag = false; frame()->UndoRedoBlock( false ); + updateSnapLineDirections(); } // Only cancel point editor when activating a new tool @@ -2511,6 +2603,7 @@ int PCB_POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent ) } m_editedPoint = nullptr; + grid.SetSnapLineDirections( {} ); return 0; } @@ -2640,9 +2733,11 @@ void PCB_POINT_EDITOR::updatePoints() return; int editedIndex = -1; + bool editingLine = false; if( m_editedPoint ) { + // Check if we're editing a point (vertex) for( unsigned ii = 0; ii < m_editPoints->PointsSize(); ++ii ) { if( &m_editPoints->Point( ii ) == m_editedPoint ) @@ -2651,6 +2746,20 @@ void PCB_POINT_EDITOR::updatePoints() break; } } + + // If not found in points, check if we're editing a line (midpoint) + if( editedIndex == -1 ) + { + for( unsigned ii = 0; ii < m_editPoints->LinesSize(); ++ii ) + { + if( &m_editPoints->Line( ii ) == m_editedPoint ) + { + editedIndex = ii; + editingLine = true; + break; + } + } + } } if( !m_editorBehavior->UpdatePoints( *m_editPoints ) ) @@ -2660,10 +2769,19 @@ void PCB_POINT_EDITOR::updatePoints() getView()->Add( m_editPoints.get() ); } - if( editedIndex >= 0 && editedIndex < (int) m_editPoints->PointsSize()) - m_editedPoint = &m_editPoints->Point( editedIndex ); + if( editedIndex >= 0 ) + { + if( editingLine && editedIndex < (int) m_editPoints->LinesSize() ) + m_editedPoint = &m_editPoints->Line( editedIndex ); + else if( !editingLine && editedIndex < (int) m_editPoints->PointsSize() ) + m_editedPoint = &m_editPoints->Point( editedIndex ); + else + m_editedPoint = nullptr; + } else + { m_editedPoint = nullptr; + } getView()->Update( m_editPoints.get() );