From 6fd898d1b3f2bb5173f8a7088809fec315960494 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Mon, 28 Feb 2022 00:17:55 +0000
Subject: [PATCH] Be more careful about non-visible elements in fields
 autoplacer.

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

(cherry picked from commit f241617478d8518e73f086cc8568530075e4f6a5)
---
 eeschema/autoplace_fields.cpp | 34 +++++++++++++++++++++++-----------
 eeschema/lib_symbol.cpp       |  4 ++--
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/eeschema/autoplace_fields.cpp b/eeschema/autoplace_fields.cpp
index 4675be4a51..c8134ce692 100644
--- a/eeschema/autoplace_fields.cpp
+++ b/eeschema/autoplace_fields.cpp
@@ -144,14 +144,14 @@ public:
      */
     void DoAutoplace( bool aManual )
     {
-        bool            force_wire_spacing = false;
+        bool            forceWireSpacing = false;
         SIDE_AND_NPINS  sideandpins = chooseSideForFields( aManual );
         SIDE            field_side = sideandpins.side;
         wxPoint         fbox_pos = fieldBoxPlacement( sideandpins );
         EDA_RECT        field_box( fbox_pos, m_fbox_size );
 
         if( aManual )
-            force_wire_spacing = fitFieldsBetweenWires( &field_box, field_side );
+            forceWireSpacing = fitFieldsBetweenWires( &field_box, field_side );
 
         // Move the fields
         int last_y_coord = field_box.GetTop();
@@ -160,6 +160,9 @@ public:
         {
             SCH_FIELD* field = m_fields[field_idx];
 
+            if( !field->IsVisible() )
+                continue;
+
             if( m_allow_rejustify )
             {
                 if( sideandpins.pins > 0 )
@@ -175,8 +178,8 @@ public:
                 }
             }
 
-            wxPoint pos( fieldHorizPlacement( field, field_box ),
-                         fieldVertPlacement( field, field_box, &last_y_coord, !force_wire_spacing ) );
+            wxPoint pos( fieldHPlacement( field, field_box ),
+                         fieldVPlacement( field, field_box, &last_y_coord, !forceWireSpacing ) );
 
             if( m_align_to_grid )
             {
@@ -201,7 +204,15 @@ protected:
         int max_field_width = 0;
         int total_height = 0;
 
+        std::vector<SCH_FIELD*> visibleFields;
+
         for( SCH_FIELD* field : m_fields )
+        {
+            if( field->IsVisible() )
+                visibleFields.push_back( field );
+        }
+
+        for( SCH_FIELD* field : visibleFields )
         {
             if( m_symbol->GetTransform().y1 )
                 field->SetTextAngle( TEXT_ANGLE_VERT );
@@ -215,7 +226,7 @@ protected:
             max_field_width = std::max( max_field_width, field_width );
 
             // Remove interline spacing from field_height for last line.
-            if( field == m_fields[ m_fields.size() - 1 ] )
+            if( field == visibleFields.back() )
                 field_height *= 0.62;
 
             if( !aDynamic )
@@ -322,6 +333,7 @@ protected:
             if( item_box.Intersects( aRect ) )
                 filtered.push_back( item );
         }
+
         return filtered;
     }
 
@@ -642,7 +654,7 @@ protected:
      *
      * @return Correct field horizontal position
      */
-    int fieldHorizPlacement( SCH_FIELD *aField, const EDA_RECT &aFieldBox )
+    int fieldHPlacement( SCH_FIELD *aField, const EDA_RECT &aFieldBox )
     {
         int field_hjust;
         int field_xcoord;
@@ -677,13 +689,13 @@ protected:
      *
      * @param aField - the field to place.
      * @param aFieldBox - box in which fields will be placed.
-     * @param aPosAccum - pointer to a position accumulator
+     * @param aAccumulatedPosition - pointer to a position accumulator
      * @param aDynamic - use dynamic spacing
      *
      * @return Correct field vertical position
      */
-    int fieldVertPlacement( SCH_FIELD *aField, const EDA_RECT &aFieldBox, int *aPosAccum,
-                            bool aDynamic )
+    int fieldVPlacement( SCH_FIELD *aField, const EDA_RECT &aFieldBox, int *aAccumulatedPosition,
+                         bool aDynamic )
     {
         int field_height;
         int padding;
@@ -704,9 +716,9 @@ protected:
             padding = FIELD_PADDING;
         }
 
-        int placement = *aPosAccum + padding / 2 + field_height / 2;
+        int placement = *aAccumulatedPosition + padding / 2 + field_height / 2;
 
-        *aPosAccum += padding + field_height;
+        *aAccumulatedPosition += padding + field_height;
 
         return placement;
     }
diff --git a/eeschema/lib_symbol.cpp b/eeschema/lib_symbol.cpp
index fa8bae5b65..2a9534fd3c 100644
--- a/eeschema/lib_symbol.cpp
+++ b/eeschema/lib_symbol.cpp
@@ -917,10 +917,10 @@ const EDA_RECT LIB_SYMBOL::GetBodyBoundingBox( int aUnit, int aConvert, bool aIn
         {
             const LIB_PIN& pin = static_cast<const LIB_PIN&>( item );
 
-            // Note: the roots of the pins are always inlcuded for symbols that don't have a
+            // Note: the roots of the pins are always included for symbols that don't have a
             // well-defined body.
 
-            if( aIncludePins )
+            if( aIncludePins && pin.IsVisible() )
                 bbox.Merge( pin.GetBoundingBox( false, true ) );
             else
                 bbox.Merge( pin.GetPinRoot() );