PCB_FIELD: bug fixes

3d-viewer/3d_canvas/create_3Dgraphic_brd_items.cpp

@@ -165,11 +165,11 @@ void BOARD_ADAPTER::addFootprintShapes( const FOOTPRINT* aFootprint, CONTAINER_2
 {
     KIGFX::GAL_DISPLAY_OPTIONS empty_opts;
 
-    if( aFootprint->Reference().GetLayer() == aLayerId && aFootprint->Reference().IsVisible() )
+    for( PCB_FIELD* field : aFootprint->GetFields() )
-        addText( &aFootprint->Reference(), aContainer, &aFootprint->Reference() );
+    {
-
+        if( field->GetLayer() == aLayerId && field->IsVisible() )
-    if( aFootprint->Value().GetLayer() == aLayerId && aFootprint->Value().IsVisible() )
+            addText( field, aContainer, field );
-        addText( &aFootprint->Value(), aContainer, &aFootprint->Value() );
+    }
 
     for( BOARD_ITEM* item : aFootprint->GraphicalItems() )
     {

3d-viewer/3d_canvas/create_layer_items.cpp

@@ -599,7 +599,6 @@ void BOARD_ADAPTER::createLayers( REPORTER* aStatusReporter )
                 addShape( static_cast<PCB_SHAPE*>( item ), layerContainer, item );
                 break;
 
-            case PCB_FIELD_T:
             case PCB_TEXT_T:
                 addText( static_cast<PCB_TEXT*>( item ), layerContainer, item );
                 break;
@@ -646,7 +645,6 @@ void BOARD_ADAPTER::createLayers( REPORTER* aStatusReporter )
                     item->TransformShapeToPolygon( *layerPoly, layer, 0, maxError, ERROR_INSIDE );
                     break;
 
-                case PCB_FIELD_T:
                 case PCB_TEXT_T:
                 {
                     PCB_TEXT* text = static_cast<PCB_TEXT*>( item );
@@ -920,7 +918,6 @@ void BOARD_ADAPTER::createLayers( REPORTER* aStatusReporter )
                 addShape( static_cast<PCB_SHAPE*>( item ), layerContainer, item );
                 break;
 
-            case PCB_FIELD_T:
             case PCB_TEXT_T:
                 addText( static_cast<PCB_TEXT*>( item ), layerContainer, item );
                 break;
@@ -957,7 +954,6 @@ void BOARD_ADAPTER::createLayers( REPORTER* aStatusReporter )
                     item->TransformShapeToPolygon( *layerPoly, layer, 0, maxError, ERROR_INSIDE );
                     break;
 
-                case PCB_FIELD_T:
                 case PCB_TEXT_T:
                 {
                     PCB_TEXT* text = static_cast<PCB_TEXT*>( item );

include/core/typeinfo.h

@@ -429,6 +429,7 @@ constexpr bool IsPcbnewType( const KICAD_T aType )
     case PCB_PAD_T:
     case PCB_SHAPE_T:
     case PCB_BITMAP_T:
+    case PCB_FIELD_T:
     case PCB_TEXT_T:
     case PCB_TEXTBOX_T:
     case PCB_TRACE_T:

pcbnew/board_commit.cpp

@@ -431,6 +431,7 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags )
 
             switch( boardItem->Type() )
             {
+            case PCB_FIELD_T:
             case PCB_TEXT_T:
             case PCB_PAD_T:
             case PCB_SHAPE_T:            // a shape (normally not on copper layers)

pcbnew/collectors.cpp

@@ -180,6 +180,7 @@ INSPECT_RESULT GENERAL_COLLECTOR::Inspect( EDA_ITEM* testItem, void* testData )
         breakhere++;
         break;
 
+    case PCB_FIELD_T:
     case PCB_TEXT_T:
         breakhere++;
         break;
@@ -296,16 +297,6 @@ INSPECT_RESULT GENERAL_COLLECTOR::Inspect( EDA_ITEM* testItem, void* testData )
 
             if( m_Guide->IgnoreFPTextOnFront() && IsFrontLayer( layer ) )
                 return INSPECT_RESULT::CONTINUE;
-
-            /*
-             * The three text types have different criteria: reference and value have their own
-             * ignore flags; user text instead follows their layer visibility. Checking this here
-             * is simpler than later (when layer visibility is checked for other entities)
-             *
-             * Note: we fallthrough from PCB_FIELD_T above, hence the type check.
-             */
-            if( text->Type() == PCB_TEXT_T && !m_Guide->IsLayerVisible( layer ) )
-                return INSPECT_RESULT::CONTINUE;
         }
 
         break;

pcbnew/drc/drc_test_provider.cpp

@@ -249,20 +249,15 @@ int DRC_TEST_PROVIDER::forEachGeometryItem( const std::vector<KICAD_T>& aTypes,
     {
         if( typeMask[PCB_FIELD_T] )
         {
-            if( ( footprint->Reference().GetLayerSet() & aLayers ).any() )
+            for( PCB_FIELD* field : footprint->GetFields() )
             {
-                if( !aFunc( &footprint->Reference() ) )
+                if( ( field->GetLayerSet() & aLayers ).any() )
-                    return n;
+                {
+                    if( !aFunc( field ) )
+                        return n;
 
-                n++;
+                    n++;
-            }
+                }
-
-            if( ( footprint->Value().GetLayerSet() & aLayers ).any() )
-            {
-                if( !aFunc( &footprint->Value() ) )
-                    return n;
-
-                n++;
             }
         }
 

pcbnew/exporters/export_gencad.cpp

@@ -720,6 +720,9 @@ static size_t hashFootprint( const FOOTPRINT* aFootprint )
     constexpr int flags = HASH_FLAGS::HASH_POS | HASH_FLAGS::REL_COORD
                             | HASH_FLAGS::HASH_ROT | HASH_FLAGS::HASH_LAYER;
 
+    for( PCB_FIELD* i : aFootprint->Fields() )
+        ret += hash_fp_item( i, flags );
+
     for( BOARD_ITEM* i : aFootprint->GraphicalItems() )
         ret += hash_fp_item( i, flags );
 

pcbnew/footprint.cpp

@@ -954,6 +954,7 @@ const BOX2I FOOTPRINT::GetBoundingBox() const
 
 const BOX2I FOOTPRINT::GetBoundingBox( bool aIncludeText, bool aIncludeInvisibleText ) const
 {
+    std::vector<PCB_TEXT*> texts;
     const BOARD* board = GetBoard();
     bool         isFPEdit = board && board->IsFootprintHolder();
 
@@ -991,7 +992,10 @@ const BOX2I FOOTPRINT::GetBoundingBox( bool aIncludeText, bool aIncludeInvisible
 
         // Handle text separately
         if( item->Type() == PCB_TEXT_T )
+        {
+            texts.push_back( static_cast<PCB_TEXT*>( item ) );
             continue;
+        }
 
         // Treat dimension objects as text
         if( !aIncludeText && BaseType( item->Type() ) == PCB_DIMENSION_T )
@@ -1000,6 +1004,9 @@ const BOX2I FOOTPRINT::GetBoundingBox( bool aIncludeText, bool aIncludeInvisible
         bbox.Merge( item->GetBoundingBox() );
     }
 
+    for( PCB_FIELD* field : m_fields )
+        texts.push_back( field );
+
     for( PAD* pad : m_pads )
         bbox.Merge( pad->GetBoundingBox() );
 
@@ -1011,15 +1018,14 @@ const BOX2I FOOTPRINT::GetBoundingBox( bool aIncludeText, bool aIncludeInvisible
     // Groups do not contribute to the rect, only their members
     if( aIncludeText || noDrawItems )
     {
-        for( BOARD_ITEM* item : m_drawings )
+        // Only PCB_TEXT and PCB_FIELD items are independently selectable;
+        // PCB_TEXTBOX items go in with other graphic items above.
+        for( PCB_TEXT* text : texts )
         {
-            if( !isFPEdit && m_privateLayers.test( item->GetLayer() ) )
+            if( !isFPEdit && m_privateLayers.test( text->GetLayer() ) )
                 continue;
 
-            // Only PCB_TEXT items are independently selectable; PCB_TEXTBOX items go in with
+            bbox.Merge( text->GetBoundingBox() );
-            // other graphic items above.
-            if( item->Type() == PCB_TEXT_T )
-                bbox.Merge( item->GetBoundingBox() );
         }
 
         // This can be further optimized when aIncludeInvisibleText is true, but currently
@@ -1310,18 +1316,14 @@ bool FOOTPRINT::HitTest( const BOX2I& aRect, bool aContained, int aAccuracy ) co
                 return true;
         }
 
-        for( PCB_FIELD* field : m_fields )
-        {
-            if( field->HitTest( arect, false, 0 ) )
-                return true;
-        }
-
         for( ZONE* zone : m_zones )
         {
             if( zone->HitTest( arect, false, 0 ) )
                 return true;
         }
 
+        // PCB fields are selectable on their own, so they don't get tested
+
         for( BOARD_ITEM* item : m_drawings )
         {
             // Text items are selectable on their own, and are therefore excluded from this
@@ -1474,11 +1476,11 @@ INSPECT_RESULT FOOTPRINT::Visit( INSPECTOR inspector, void* testData,
             break;
 
         case PCB_FIELD_T:
-            if( inspector( &Reference(), testData ) == INSPECT_RESULT::QUIT )
+            if( IterateForward<PCB_FIELD*>( m_fields, inspector, testData, { scanType } )
-                return INSPECT_RESULT::QUIT;
+                == INSPECT_RESULT::QUIT )
-
+            {
-            if( inspector( &Value(), testData ) == INSPECT_RESULT::QUIT )
                 return INSPECT_RESULT::QUIT;
+            }
 
             break;
 

pcbnew/pcb_field.cpp

@@ -128,6 +128,10 @@ wxString PCB_FIELD::GetItemDescription( UNITS_PROVIDER* aUnitsProvider ) const
         return wxString::Format( _( "Datasheet '%s' of %s" ),
                                  KIUI::EllipsizeMenuText( GetText() ),
                                  GetParentFootprint()->GetReference() );
+    default:
+        return wxString::Format( _( "Field '%s' of %s" ),
+                                 KIUI::EllipsizeMenuText( GetText() ),
+                                 GetParentFootprint()->GetReference() );
     }
 
     // Can't get here, but gcc doesn't seem to know that....

pcbnew/plot_brditems_plotter.cpp

@@ -278,7 +278,7 @@ void BRDITEMS_PLOTTER::PlotFootprintTextItems( const FOOTPRINT* aFootprint )
     const PCB_TEXT* textItem = &aFootprint->Reference();
     PCB_LAYER_ID    textLayer = textItem->GetLayer();
 
-    // Reference and value are specific items, not in graphic items list
+    // Reference and value have special controls for forcing their plotting
     if( GetPlotReference() && m_layerMask[textLayer]
         && ( textItem->IsVisible() || GetPlotInvisibleText() ) )
     {
@@ -294,17 +294,32 @@ void BRDITEMS_PLOTTER::PlotFootprintTextItems( const FOOTPRINT* aFootprint )
         PlotText( textItem, textLayer, textItem->IsKnockout() );
     }
 
-    for( const BOARD_ITEM* item : aFootprint->GraphicalItems() )
+
+    std::vector<PCB_TEXT*> texts;
+
+    // Skip the reference and value texts that are handled specially
+    for( PCB_FIELD* field : aFootprint->Fields() )
+    {
+        if( field->IsReference() || field->IsValue() )
+            continue;
+
+        texts.push_back( field );
+    }
+
+    for( BOARD_ITEM* item : aFootprint->GraphicalItems() )
     {
         textItem = dyn_cast<const PCB_TEXT*>( item );
 
-        if( !textItem )
+        if( textItem )
+            texts.push_back( static_cast<PCB_TEXT*>( item ) );
+    }
+
+    for( const PCB_TEXT* text : texts )
+    {
+        if( !text->IsVisible() )
             continue;
 
-        if( !textItem->IsVisible() )
+        textLayer = text->GetLayer();
-            continue;
-
-        textLayer = textItem->GetLayer();
 
         if( textLayer == Edge_Cuts || textLayer >= PCB_LAYER_ID_COUNT )
             continue;
@@ -312,13 +327,13 @@ void BRDITEMS_PLOTTER::PlotFootprintTextItems( const FOOTPRINT* aFootprint )
         if( !m_layerMask[textLayer] || aFootprint->GetPrivateLayers().test( textLayer ) )
             continue;
 
-        if( textItem->GetText() == wxT( "${REFERENCE}" ) && !GetPlotReference() )
+        if( text->GetText() == wxT( "${REFERENCE}" ) && !GetPlotReference() )
             continue;
 
-        if( textItem->GetText() == wxT( "${VALUE}" ) && !GetPlotValue() )
+        if( text->GetText() == wxT( "${VALUE}" ) && !GetPlotValue() )
             continue;
 
-        PlotText( textItem, textLayer, textItem->IsKnockout() );
+        PlotText( text, textLayer, textItem->IsKnockout() );
     }
 }
 
@@ -519,7 +534,6 @@ void BRDITEMS_PLOTTER::PlotFootprintGraphicItems( const FOOTPRINT* aFootprint )
             break;
         }
 
-        case PCB_FIELD_T:
         case PCB_TEXT_T:
             // Plotted in PlotFootprintTextItems()
             break;

pcbnew/plugins/kicad/pcb_parser.cpp

@@ -3837,13 +3837,14 @@ FOOTPRINT* PCB_PARSER::parseFOOTPRINT_unchecked( wxArrayString* aInitialComments
                 field = new PCB_FIELD( footprint.get(), footprint->GetFieldCount(), pName );
 
                 field->SetText( pValue );
-                field->SetVisible( false );
                 field->SetLayer( footprint->GetLayer() == F_Cu ? F_Fab : B_Fab );
                 field->StyleFromSettings( m_board->GetDesignSettings() );
 
                 footprint->AddField( field );
             }
 
+            field->SetVisible( true );
+
             parsePCB_TEXT_effects( field );
         }
             break;

qa/pcbnew_utils/board_test_utils.cpp

@@ -153,7 +153,6 @@ struct kitest_cmp_drawings
             const PCB_TEXT* textA = static_cast<const PCB_TEXT*>( itemA );
             const PCB_TEXT* textB = static_cast<const PCB_TEXT*>( itemB );
 
-            TEST( textA->GetType(), textB->GetType() );
             TEST_PT( textA->GetPosition(), textB->GetPosition() );
             TEST( textA->GetTextAngle(), textB->GetTextAngle() );
         }
@@ -325,8 +324,6 @@ void CheckFpText( const PCB_TEXT* expected, const PCB_TEXT* text )
     {
         CHECK_ENUM_CLASS_EQUAL( expected->Type(), text->Type() );
 
-        CHECK_ENUM_CLASS_EQUAL( expected->GetType(), text->GetType() );
-
         BOOST_CHECK_EQUAL( expected->IsLocked(), text->IsLocked() );
 
         BOOST_CHECK_EQUAL( expected->GetText(), text->GetText() );

qa/tests/pcbnew/test_board_item.cpp

@@ -75,6 +75,7 @@ public:
         {
         case PCB_FOOTPRINT_T:         return new FOOTPRINT( &m_board );
         case PCB_PAD_T:               return new PAD( &m_footprint );
+        case PCB_FIELD_T:             return new PCB_FIELD( &m_footprint, m_footprint.GetFieldCount() );
         case PCB_SHAPE_T:             return new PCB_SHAPE( &m_board );
         case PCB_TEXT_T:              return new PCB_TEXT( &m_board );
         case PCB_TEXTBOX_T:           return new PCB_TEXTBOX( &m_board );