Fix fly-off arc handles and move Arc Properties to start/end/angle.

Fixes https://gitlab.com/kicad/code/kicad/issues/5791

include/tool/edit_points.h

@@ -184,6 +184,9 @@ public:
     bool IsHover() const { return m_isHover; }
     void SetHover( bool aHover = true ) { m_isHover = aHover; }
 
+    bool IsGridFree() const { return m_gridFree; }
+    void SetGridFree( bool aGridFree = true ) { m_gridFree = aGridFree; }
+
     bool operator==( const EDIT_POINT& aOther ) const
     {
         return m_position == aOther.m_position;
@@ -199,21 +202,16 @@ public:
     static const int HOVER_SIZE = 5;
 
 private:
-    ///> Position of EDIT_POINT
+    VECTOR2I  m_position;        // Position of EDIT_POINT
-    VECTOR2I  m_position;
 
-    ///> An optional item to a connected item.  Used to mimic polyLine behaviour
+    EDA_ITEM* m_connection;      // An optional item to a connected item.  Used to mimic
-    ///> with individual line segments.
+                                 // polyLine behaviour with individual line segments.
-    EDA_ITEM* m_connection;
+    bool      m_isActive;        // True if this point is being manipulated
+    bool      m_isHover;         // True if this point is being hovered over
+    bool      m_gridFree;        // True if this point should not be snapped to the grid.
 
     ///> Constraint for the point, NULL if none
     std::shared_ptr<EDIT_CONSTRAINT<EDIT_POINT> > m_constraint;
-
-    ///> True if this point is being manipulated
-    bool m_isActive;
-
-    ///> True if this point is being hovered over
-    bool m_isHover;
 };
 
 
@@ -235,8 +233,11 @@ public:
      */
     EDIT_LINE( EDIT_POINT& aOrigin, EDIT_POINT& aEnd ) :
         EDIT_POINT( aOrigin.GetPosition() + ( aEnd.GetPosition() - aOrigin.GetPosition() ) / 2 ),
-        m_origin( aOrigin ), m_end( aEnd )
+        m_origin( aOrigin ),
+        m_end( aEnd )
     {
+        // Don't snap the line center to the grid
+        SetGridFree();
     }
 
     ///> @copydoc EDIT_POINT::GetPosition()

libs/kimath/include/trigo.h

@@ -111,6 +111,7 @@ void RotatePoint( double *pX, double *pY, double cx, double cy, double angle );
 const VECTOR2I GetArcCenter( const VECTOR2I& aStart, const VECTOR2I& aMid, const VECTOR2I& aEnd );
 const VECTOR2D GetArcCenter( const VECTOR2D& aStart, const VECTOR2D& aMid, const VECTOR2D& aEnd );
 const wxPoint GetArcCenter( const wxPoint& aStart, const wxPoint& aMid, const wxPoint& aEnd );
+const wxPoint GetArcCenter( VECTOR2I aStart, VECTOR2I aEnd, double aAngle );
 
 /**
  * Returns the subtended angle for a given arc

libs/kimath/src/trigo.cpp

@@ -338,6 +338,31 @@ void RotatePoint( double* pX, double* pY, double angle )
 }
 
 
+const wxPoint GetArcCenter( VECTOR2I aStart, VECTOR2I aEnd, double aAngle )
+{
+    if( aAngle < 0 )
+    {
+        std::swap( aStart, aEnd );
+        aAngle = abs( aAngle );
+    }
+
+    if( aAngle > 180 )
+    {
+        std::swap( aStart, aEnd );
+        aAngle = 360 - aAngle;
+    }
+
+    int chord = ( aStart - aEnd ).EuclideanNorm();
+    int r = ( chord / 2 ) / sin( aAngle * M_PI / 360.0 );
+
+    VECTOR2I vec = aEnd - aStart;
+    vec = vec.Resize( r );
+    vec = vec.Rotate( ( 180.0 - aAngle ) * M_PI / 360.0 );
+
+    return (wxPoint) ( aStart + vec );
+}
+
+
 const VECTOR2D GetArcCenter( const VECTOR2D& aStart, const VECTOR2D& aMid, const VECTOR2D& aEnd )
 {
     VECTOR2D center;

pcbnew/board_commit.cpp

@@ -283,6 +283,15 @@ void BOARD_COMMIT::Push( const wxString& aMessage, bool aCreateUndoEntry, bool a
 
                 connectivity->Update( boardItem );
                 view->Update( boardItem );
+
+                if( m_editModules )
+                {
+                    static_cast<MODULE*>( boardItem )->RunOnChildren( [&]( BOARD_ITEM* aChild )
+                                                                      {
+                                                                          view->Update( aChild );
+                                                                      });
+                }
+
                 board->OnItemChanged( boardItem );
 
                 // if no undo entry is needed, the copy would create a memory leak

pcbnew/dialogs/dialog_graphic_item_properties.cpp

@@ -178,9 +178,6 @@ bool DIALOG_GRAPHIC_ITEM_PROPERTIES::TransferDataToWindow()
 
     case S_ARC:
         SetTitle( _( "Arc Properties" ) );
-        m_startPointLabel->SetLabel( _( "Center" ) );
-        m_endPointLabel->SetLabel( _( "Start Point" ) );
-
         m_AngleValue = m_item->GetAngle() / 10.0;
         break;
 
@@ -206,7 +203,12 @@ bool DIALOG_GRAPHIC_ITEM_PROPERTIES::TransferDataToWindow()
         break;
     }
 
-    if( m_flipStartEnd )
+    if( m_item->GetShape() == S_ARC )
+    {
+        m_startX.SetValue( m_item->GetArcStart().x );
+        m_startY.SetValue( m_item->GetArcStart().y );
+    }
+    else if( m_flipStartEnd )
     {
         m_startX.SetValue( m_item->GetEnd().x );
         m_startY.SetValue( m_item->GetEnd().y );
@@ -217,10 +219,15 @@ bool DIALOG_GRAPHIC_ITEM_PROPERTIES::TransferDataToWindow()
         m_startY.SetValue( m_item->GetStart().y );
     }
 
-    if(  m_item->GetShape() == S_CIRCLE )
+    if( m_item->GetShape() == S_CIRCLE )
     {
         m_endX.SetValue( m_item->GetRadius() );
     }
+    else if( m_item->GetShape() == S_ARC )
+    {
+        m_endX.SetValue( m_item->GetArcEnd().x );
+        m_endY.SetValue( m_item->GetArcEnd().y );
+    }
     else if( m_flipStartEnd )
     {
         m_endX.SetValue( m_item->GetStart().x );
@@ -264,7 +271,11 @@ bool DIALOG_GRAPHIC_ITEM_PROPERTIES::TransferDataFromWindow()
     BOARD_COMMIT commit( m_parent );
     commit.Modify( m_item );
 
-    if( m_flipStartEnd )
+    if( m_item->GetShape() == S_ARC )
+    {
+        m_item->SetArcStart( wxPoint( m_startX.GetValue(), m_startY.GetValue() ) );
+    }
+    else if( m_flipStartEnd )
     {
         m_item->SetEndX( m_startX.GetValue() );
         m_item->SetEndY( m_startY.GetValue() );
@@ -279,6 +290,10 @@ bool DIALOG_GRAPHIC_ITEM_PROPERTIES::TransferDataFromWindow()
     {
         m_item->SetEnd( m_item->GetStart() + wxPoint( m_endX.GetValue(), 0 ) );
     }
+    else if( m_item->GetShape() == S_ARC )
+    {
+        m_item->SetArcEnd( wxPoint( m_endX.GetValue(), m_endY.GetValue() ) );
+    }
     else if( m_flipStartEnd )
     {
         m_item->SetStartX( m_endX.GetValue() );
@@ -298,9 +313,8 @@ bool DIALOG_GRAPHIC_ITEM_PROPERTIES::TransferDataFromWindow()
     }
 
     if( m_moduleItem )
-    {   // We are editing a footprint.
+    {
-        // Init the item coordinates relative to the footprint anchor,
+        // We are editing a footprint; init the item coordinates relative to the footprint anchor.
-        // that are coordinate references
         m_moduleItem->SetStart0( m_moduleItem->GetStart() );
         m_moduleItem->SetEnd0( m_moduleItem->GetEnd() );
 
@@ -315,13 +329,16 @@ bool DIALOG_GRAPHIC_ITEM_PROPERTIES::TransferDataFromWindow()
     m_item->SetLayer( ToLAYER_ID( layer ) );
 
     if( m_item->GetShape() == S_ARC )
-        m_item->SetAngle( m_AngleValue * 10.0 );
+    {
+        m_item->SetCenter( GetArcCenter( m_item->GetArcStart(), m_item->GetArcEnd(), m_AngleValue ) );
+        m_item->SetAngle( m_AngleValue * 10.0, false );
+    }
 
     m_item->RebuildBezierToSegmentsPointsList( m_item->GetWidth() );
 
     commit.Push( _( "Modify drawing properties" ) );
 
-    m_parent->SetMsgPanel( m_item );
+    m_parent->UpdateMsgPanel();
 
     return true;
 }

pcbnew/fp_shape.cpp

@@ -148,14 +148,18 @@ EDA_ITEM* FP_SHAPE::Clone() const
 }
 
 
-void FP_SHAPE::SetAngle( double aAngle )
+void FP_SHAPE::SetAngle( double aAngle, bool aUpdateEnd )
 {
     // Mark as depreciated.
     // m_Angle does not define the arc anymore
     // m_Angle must be >= -360 and <= +360 degrees
     m_Angle = NormalizeAngle360Max( aAngle );
-    m_ThirdPoint0 = m_End0;
+
-    RotatePoint( &m_ThirdPoint0, m_Start0, -m_Angle );
+    if( aUpdateEnd )
+    {
+        m_ThirdPoint0 = m_End0;
+        RotatePoint( &m_ThirdPoint0, m_Start0, -m_Angle );
+    }
 }
 
 

pcbnew/fp_shape.h

@@ -67,7 +67,7 @@ public:
         return false;
     }
 
-    void SetAngle( double aAngle ) override;
+    void SetAngle( double aAngle, bool aUpdateEnd = true ) override;
 
     /**
      * Move an edge of the footprint.

pcbnew/pcb_shape.cpp

@@ -431,14 +431,16 @@ double PCB_SHAPE::GetArcAngleEnd() const
 }
 
 
-void PCB_SHAPE::SetAngle( double aAngle )
+void PCB_SHAPE::SetAngle( double aAngle, bool aUpdateEnd )
 {
-    // Mark as depreciated.
-    // m_Angle does not define the arc anymore
     // m_Angle must be >= -360 and <= +360 degrees
     m_Angle = NormalizeAngle360Max( aAngle );
-    m_ThirdPoint = m_End;
+
-    RotatePoint( &m_ThirdPoint, m_Start, -m_Angle );
+    if( aUpdateEnd )
+    {
+        m_ThirdPoint = m_End;
+        RotatePoint( &m_ThirdPoint, m_Start, -m_Angle );
+    }
 }
 
 
@@ -1269,8 +1271,8 @@ static struct DRAWSEGMENT_DESC
         propMgr.AddProperty( new PROPERTY<PCB_SHAPE, int>( _( "Thickness" ),
                     &PCB_SHAPE::SetWidth, &PCB_SHAPE::GetWidth, PROPERTY_DISPLAY::DISTANCE ) );
         // TODO show certain properties depending on the shape
-        propMgr.AddProperty( new PROPERTY<PCB_SHAPE, double>( _( "Angle" ),
+        //propMgr.AddProperty( new PROPERTY<PCB_SHAPE, double>( _( "Angle" ),
-                    &PCB_SHAPE::SetAngle, &PCB_SHAPE::GetAngle, PROPERTY_DISPLAY::DECIDEGREE ) );
+        //            &PCB_SHAPE::SetAngle, &PCB_SHAPE::GetAngle, PROPERTY_DISPLAY::DECIDEGREE ) );
         // TODO or may have different names (arcs)
         // TODO type?
         propMgr.AddProperty( new PROPERTY<PCB_SHAPE, int>( _( "End X" ),

pcbnew/pcb_shape.h

@@ -104,7 +104,7 @@ public:
      * sets the angle for arcs, and normalizes it within the range 0 - 360 degrees.
      * @param aAngle is tenths of degrees, but will soon be degrees.
      */
-    virtual void SetAngle( double aAngle );     // encapsulates the transition to degrees
+    virtual void SetAngle( double aAngle, bool aUpdateEnd = true );
     double GetAngle() const { return m_Angle; }
 
     void SetType( int aType )                       { m_Type = aType; }

pcbnew/tools/point_editor.cpp

@@ -170,6 +170,8 @@ public:
 
                 points->Point( ARC_MID ).SetConstraint( new EC_LINE( points->Point( ARC_MID ),
                                                                      points->Point( ARC_CENTER ) ) );
+
+                points->Point( ARC_MID ).SetGridFree();
                 break;
 
             case S_CIRCLE:
@@ -425,8 +427,6 @@ int POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent )
     m_refill = false;
     bool inDrag = false;
 
-    frame()->UndoRedoBlock( true );
-
     BOARD_COMMIT commit( editFrame );
     LSET snapLayers = item->GetLayerSet();
 
@@ -454,6 +454,8 @@ int POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent )
         {
             if( !inDrag )
             {
+                frame()->UndoRedoBlock( true );
+
                 commit.StageItems( selection, CHT_MODIFY );
 
                 controls->ForceCursorPosition( false );
@@ -466,8 +468,15 @@ int POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent )
             }
 
             //TODO: unify the constraints to solve simultaneously instead of sequentially
-            m_editedPoint->SetPosition( grid.BestSnapAnchor( evt->Position(),
+            if( m_editedPoint->IsGridFree() )
-                                                             snapLayers, { item } ) );
+            {
+                m_editedPoint->SetPosition( evt->Position() );
+            }
+            else
+            {
+                m_editedPoint->SetPosition( grid.BestSnapAnchor( evt->Position(), snapLayers,
+                                                                 { item } ) );
+            }
 
             // The alternative constraint limits to 45°
             bool enableAltConstraint = !!evt->Modifier( MD_CTRL );
@@ -477,10 +486,11 @@ int POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent )
             else
                 m_editedPoint->ApplyConstraint();
 
-            // Don't snap the line center to the grid
+            if( !m_editedPoint->IsGridFree() )
-            if( !dynamic_cast<EDIT_LINE*>( m_editedPoint) )
+            {
                 m_editedPoint->SetPosition( grid.BestSnapAnchor( m_editedPoint->GetPosition(),
                                                                  snapLayers, { item } ) );
+            }
 
             updateItem();
             updatePoints();
@@ -503,15 +513,23 @@ int POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent )
 
             commit.Push( _( "Drag a corner" ) );
             inDrag = false;
+            frame()->UndoRedoBlock( false );
+
             m_refill = true;
         }
 
         else if( evt->IsCancelInteractive() || evt->IsActivate() )
         {
             if( inDrag )      // Restore the last change
+            {
                 commit.Revert();
+                inDrag = false;
+                frame()->UndoRedoBlock( false );
+            }
             else if( evt->IsCancelInteractive() )
+            {
                 break;
+            }
 
             if( evt->IsActivate() && !evt->IsMoveTool() )
                 break;
@@ -529,7 +547,6 @@ int POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent )
         m_editPoints.reset();
     }
 
-    frame()->UndoRedoBlock( false );
     frame()->UpdateMsgPanel();
 
     return 0;
@@ -689,19 +706,13 @@ void POINT_EDITOR::editArcEndpointKeepTangent( PCB_SHAPE* aArc, VECTOR2I aCenter
 
         if( arcValid )
         {
-            aArc->SetAngle( newAngle );
+            aArc->SetAngle( newAngle, false );
             aArc->SetCenter( wxPoint( aCenter.x, aCenter.y ) );
 
             if( movingStart )
-            {
                 aArc->SetArcStart( wxPoint( aStart.x, aStart.y ) );
-                // Set angle computes the end point, so re-force it now.
-                aArc->SetArcEnd( wxPoint( aEnd.x, aEnd.y ) );
-            }
             else
-            {
                 aArc->SetArcEnd( wxPoint( aEnd.x, aEnd.y ) );
-            }
         }
     }
 }
@@ -876,19 +887,13 @@ void POINT_EDITOR::editArcEndpointKeepCenter( PCB_SHAPE* aArc, VECTOR2I aCenter,
     else if( !clockwise && newAngle > 0.0 )
         newAngle -= 3600.0;
 
-    aArc->SetAngle( newAngle );
+    aArc->SetAngle( newAngle, false );
-    aArc->SetCenter( wxPoint( aCenter.x, aCenter.y ) );
+    aArc->SetCenter( (wxPoint) aCenter );
 
     if( movingStart )
-    {
+        aArc->SetArcStart( (wxPoint) aStart );
-        aArc->SetArcStart( wxPoint( aStart.x, aStart.y ) );
-        // Set angle computes the end point, so re-force it now.
-        aArc->SetArcEnd( wxPoint( aEnd.x, aEnd.y ) );
-    }
     else
-    {
+        aArc->SetArcEnd( (wxPoint) aEnd );
-        aArc->SetArcEnd( wxPoint( aEnd.x, aEnd.y ) );
-    }
 }
 
 
@@ -945,8 +950,8 @@ void POINT_EDITOR::editArcMidKeepCenter( PCB_SHAPE* aArc, VECTOR2I aCenter, VECT
     aStart = aStart + aCenter;
     aEnd   = aEnd + aCenter;
 
-    aArc->SetArcStart( wxPoint( aStart.x, aStart.y ) );
+    aArc->SetArcStart( (wxPoint) aStart );
-    aArc->SetArcEnd( wxPoint( aEnd.x, aEnd.y ) );
+    aArc->SetArcEnd( (wxPoint) aEnd );
 }
 
 
@@ -962,7 +967,7 @@ void POINT_EDITOR::editArcMidKeepEnpoints( PCB_SHAPE* aArc, VECTOR2I aCenter, VE
     // Find the new center
     aCenter = GetArcCenter( aStart, aMid, aEnd );
 
-    aArc->SetCenter( wxPoint( aCenter.x, aCenter.y ) );
+    aArc->SetCenter( (wxPoint) aCenter );
 
     // Check if the new arc is CW or CCW
     VECTOR2D startLine = aStart - aCenter;
@@ -994,12 +999,11 @@ void POINT_EDITOR::editArcMidKeepEnpoints( PCB_SHAPE* aArc, VECTOR2I aCenter, VE
         // Cancel Everything
         // If the accuracy is low, we can't draw precisely the arc.
         // It may happen when the radius is *high*
-        aArc->SetCenter( wxPoint( oldCenter.x, oldCenter.y ) );
+        aArc->SetCenter( (wxPoint) oldCenter );
     }
     else
     {
-        aArc->SetAngle( newAngle );
+        aArc->SetAngle( newAngle, false );
-        aArc->SetArcEnd( wxPoint( aEnd.x, aEnd.y ) );
     }
 
     // Now, update the edit point position
@@ -1087,8 +1091,6 @@ void POINT_EDITOR::updateItem() const
             VECTOR2I start = m_editPoints->Point( ARC_START ).GetPosition();
             VECTOR2I end = m_editPoints->Point( ARC_END ).GetPosition();
 
-            const VECTOR2I& cursorPos = getViewControls()->GetCursorPosition();
-
             if( isModified( m_editPoints->Point( ARC_CENTER ) ) )
             {
                 wxPoint moveVector = wxPoint( center.x, center.y ) - shape->GetCenter();
@@ -1096,26 +1098,22 @@ void POINT_EDITOR::updateItem() const
             }
             else if( isModified( m_editPoints->Point( ARC_MID ) ) )
             {
+                const VECTOR2I& cursorPos = getViewControls()->GetCursorPosition( false );
+
                 if( m_altEditMethod )
-                {
                     editArcMidKeepCenter( shape, center, start, mid, end, cursorPos );
-                }
                 else
-                {
                     editArcMidKeepEnpoints( shape, center, start, mid, end, cursorPos );
-                }
             }
             else if( isModified( m_editPoints->Point( ARC_START ) )
                      || isModified( m_editPoints->Point( ARC_END ) ) )
             {
+                const VECTOR2I& cursorPos = getViewControls()->GetCursorPosition();
+
                 if( m_altEditMethod )
-                {
                     editArcEndpointKeepCenter( shape, center, start, mid, end, cursorPos );
-                }
                 else
-                {
                     editArcEndpointKeepTangent( shape, center, start, mid, end, cursorPos );
-                }
             }
         }
             break;