From 0cf1a67e294a0017f4d9ba0383357ec527710869 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Fri, 4 Mar 2022 15:18:34 -0800 Subject: [PATCH] Cache read arc data for stability Arcs can be altered by the process of changing from on-disk representation to in-memory representation. Saving back to disk without modifying the arc should not modify the calculated values. This stores a copy of the on-disk representation that is only used to save back to disk in the event that the arc is not modified during editing. Fixes https://gitlab.com/kicad/code/kicad/issues/10442 (cherry picked from commit cd7141fd10645d9f686c8de5effb703bb3a8c189) --- common/eda_shape.cpp | 32 ++++++++++++++++--- .../sch_plugins/kicad/sch_sexpr_parser.cpp | 9 +++--- include/eda_shape.h | 21 ++++++++++++ pcbnew/fp_shape.cpp | 11 +++++++ pcbnew/fp_shape.h | 2 ++ 5 files changed, 66 insertions(+), 9 deletions(-) diff --git a/common/eda_shape.cpp b/common/eda_shape.cpp index b0250ed3cb..18753d19a8 100644 --- a/common/eda_shape.cpp +++ b/common/eda_shape.cpp @@ -436,6 +436,12 @@ void EDA_SHAPE::SetCenter( const wxPoint& aCenter ) wxPoint EDA_SHAPE::GetArcMid() const { + // If none of the input data have changed since we loaded the arc, + // keep the original mid point data to minimize churn + if( m_arcMidData.start == m_start && m_arcMidData.end == m_end + && m_arcMidData.center == m_arcCenter ) + return m_arcMidData.mid; + wxPoint mid = m_start; RotatePoint( &mid, m_arcCenter, -GetArcAngle() / 2.0 ); return mid; @@ -486,19 +492,35 @@ int EDA_SHAPE::GetRadius() const } +void EDA_SHAPE::SetCachedArcData( const wxPoint& aStart, const wxPoint& aMid, const wxPoint& aEnd, const wxPoint& aCenter ) +{ + m_arcMidData.start = aStart; + m_arcMidData.end = aEnd; + m_arcMidData.center = aCenter; + m_arcMidData.mid = aMid; +} + + void EDA_SHAPE::SetArcGeometry( const wxPoint& aStart, const wxPoint& aMid, const wxPoint& aEnd ) { + m_arcMidData = {}; m_start = aStart; m_end = aEnd; m_arcCenter = CalcArcCenter( aStart, aMid, aEnd ); + wxPoint new_mid = GetArcMid(); + m_endsSwapped = false; - /** - * If the input winding doesn't match our internal winding, the calculated midpoint will end up - * on the other side of the arc. In this case, we need to flip the start/end points and flag this - * change for the system + // Watch the ordering here. GetArcMid above needs to be called prior to initializing the + // m_arcMidData structure in order to ensure we get the calculated variant, not the cached + SetCachedArcData( aStart, aMid, aEnd, m_arcCenter ); + + /* + * If the input winding doesn't match our internal winding, the calculated midpoint will end + * up on the other side of the arc. In this case, we need to flip the start/end points and + * flag this change for the system. */ - wxPoint new_mid = GetArcMid(); + VECTOR2D dist( new_mid - aMid ); VECTOR2D dist2( new_mid - m_arcCenter ); diff --git a/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp b/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp index 469f8fe0dc..3a4ee13711 100644 --- a/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp +++ b/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp @@ -1013,15 +1013,15 @@ LIB_SHAPE* SCH_SEXPR_PARSER::parseArc() } } - arc->SetStart( startPoint ); - arc->SetEnd( endPoint ); - if( hasMidPoint ) { - arc->SetCenter( (wxPoint) CalcArcCenter( arc->GetStart(), midPoint, arc->GetEnd() ) ); + arc->SetArcGeometry( startPoint, midPoint, endPoint); } else if( hasAngles ) { + arc->SetStart( startPoint ); + arc->SetEnd( endPoint ); + /** * This accounts for an oddity in the old library format, where the symbol is overdefined. * The previous draw (based on wxwidgets) used start point and end point and always drew @@ -1035,6 +1035,7 @@ LIB_SHAPE* SCH_SEXPR_PARSER::parseArc() arc->SetStart( arc->GetEnd() ); arc->SetEnd( temp ); } + arc->SetCenter( center ); } else diff --git a/include/eda_shape.h b/include/eda_shape.h index eece8adc51..3f31fc2686 100644 --- a/include/eda_shape.h +++ b/include/eda_shape.h @@ -59,6 +59,15 @@ enum class FILL_T : int }; +// Holding struct to keep originating midpoint +struct ARC_MID +{ + wxPoint mid; + wxPoint start; + wxPoint end; + wxPoint center; +}; + class EDA_SHAPE { public: @@ -187,6 +196,17 @@ public: */ void SetArcGeometry( const wxPoint& aStart, const wxPoint& aMid, const wxPoint& aEnd ); + /** + * Set the data used for mid point caching. If the controlling points remain constant, then + * we keep the midpoint the same as it was when read in. This minimizes VCS churn + * + * @param aStart Cached start point + * @param aMid Cached mid point + * @param aEnd Cached end point + * @param aCenter Calculated center point using the preceeding three + */ + void SetCachedArcData( const wxPoint& aStart, const wxPoint& aMid, const wxPoint& aEnd, const wxPoint& aCenter ); + const std::vector& GetBezierPoints() const { return m_bezierPoints; } /** @@ -303,6 +323,7 @@ protected: wxPoint m_end; // Line end point or Circle 3 o'clock point wxPoint m_arcCenter; // Used only for Arcs: arc end point + ARC_MID m_arcMidData; // Used to store originating data wxPoint m_bezierC1; // Bezier Control Point 1 wxPoint m_bezierC2; // Bezier Control Point 2 diff --git a/pcbnew/fp_shape.cpp b/pcbnew/fp_shape.cpp index 16d2201c2f..6296ee39e6 100644 --- a/pcbnew/fp_shape.cpp +++ b/pcbnew/fp_shape.cpp @@ -179,6 +179,12 @@ void FP_SHAPE::SetCenter0( const wxPoint& aCenter ) wxPoint FP_SHAPE::GetArcMid0() const { + // If none of the input data have changed since we loaded the arc, + // keep the original mid point data to minimize churn + if( m_arcMidData_0.start == m_start && m_arcMidData_0.end == m_end + && m_arcMidData_0.center == m_arcCenter ) + return m_arcMidData_0.mid; + wxPoint mid0 = m_start0; RotatePoint( &mid0, m_arcCenter0, -GetArcAngle() / 2.0 ); return mid0; @@ -200,6 +206,11 @@ void FP_SHAPE::SetArcGeometry0( const wxPoint& aStart0, const wxPoint& aMid0, co m_start0 = aStart0; m_end0 = aEnd0; m_arcCenter0 = CalcArcCenter( aStart0, aMid0, aEnd0 ); + + m_arcMidData_0.center = m_arcCenter0; + m_arcMidData_0.end = m_end0; + m_arcMidData_0.mid = aMid0; + m_arcMidData_0.start = m_start0; } diff --git a/pcbnew/fp_shape.h b/pcbnew/fp_shape.h index 627c9786df..b4b6089696 100644 --- a/pcbnew/fp_shape.h +++ b/pcbnew/fp_shape.h @@ -165,6 +165,8 @@ protected: wxPoint m_arcCenter0; ///< Center of arc, relative to footprint origin, orient 0. wxPoint m_bezierC1_0; ///< Bezier Control Point 1, relative to footprint origin, orient 0. wxPoint m_bezierC2_0; ///< Bezier Control Point 2, relative to footprint origin, orient 0. + + ARC_MID m_arcMidData_0; ///< Originating Arc data, orient 0 }; #endif // FP_SHAPE_H