From d7368588db04b57f69d65cef93992fd338645ffc Mon Sep 17 00:00:00 2001
From: jean-pierre charras <jp.charras@wanadoo.fr>
Date: Thu, 1 Dec 2022 16:01:26 +0100
Subject: [PATCH] stackup manager: fix incorrect handling of colors for
 multi-layer dielectrics The dialog allows a color selection for each
 dielectric layer. However for a "dielectric" defined by more than one layer,
 the color was handled only for the first layer, and not for the other
 sub-layers.

---
 .../board_stackup_manager/board_stackup.cpp   | 28 +++++++++++++--
 pcbnew/board_stackup_manager/board_stackup.h  |  6 ++--
 .../panel_board_stackup.cpp                   | 36 +++++++++----------
 pcbnew/exporters/gerber_jobfile_writer.cpp    |  4 +--
 pcbnew/plugins/kicad/pcb_parser.cpp           |  2 +-
 5 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/pcbnew/board_stackup_manager/board_stackup.cpp b/pcbnew/board_stackup_manager/board_stackup.cpp
index 4d4c3c09c6..735b847f73 100644
--- a/pcbnew/board_stackup_manager/board_stackup.cpp
+++ b/pcbnew/board_stackup_manager/board_stackup.cpp
@@ -48,6 +48,7 @@ BOARD_STACKUP_ITEM::BOARD_STACKUP_ITEM( BOARD_STACKUP_ITEM_TYPE aType )
 
     case BS_ITEM_TYPE_DIELECTRIC:
         m_TypeName = KEY_CORE;          // or prepreg
+        SetColor( NotSpecifiedPrm() );
         SetMaterial( wxT( "FR4" ) );    // or other dielectric name
         SetLossTangent( 0.02 );         // for FR4
         SetEpsilonR( 4.5 );             // for FR4
@@ -59,7 +60,7 @@ BOARD_STACKUP_ITEM::BOARD_STACKUP_ITEM( BOARD_STACKUP_ITEM_TYPE aType )
 
     case BS_ITEM_TYPE_SOLDERMASK:
         m_TypeName = wxT( "soldermask" );
-        m_Color = NotSpecifiedPrm();
+        SetColor( NotSpecifiedPrm() );
         SetMaterial( NotSpecifiedPrm() ); // or other solder mask material name
         SetThickness( GetMaskDefaultThickness() );
         SetEpsilonR( DEFAULT_EPSILON_R_SOLDERMASK );
@@ -67,7 +68,7 @@ BOARD_STACKUP_ITEM::BOARD_STACKUP_ITEM( BOARD_STACKUP_ITEM_TYPE aType )
 
     case BS_ITEM_TYPE_SILKSCREEN:
         m_TypeName = wxT( "silkscreen" );
-        m_Color = NotSpecifiedPrm();
+        SetColor( NotSpecifiedPrm() );
         SetMaterial( NotSpecifiedPrm() ); // or other silkscreen material name
         SetEpsilonR( DEFAULT_EPSILON_R_SILKSCREEN );
         break;
@@ -87,7 +88,6 @@ BOARD_STACKUP_ITEM::BOARD_STACKUP_ITEM( const BOARD_STACKUP_ITEM& aOther )
     m_DielectricPrmsList = aOther.m_DielectricPrmsList;
     m_TypeName = aOther.m_TypeName;
     m_LayerName = aOther.m_LayerName;
-    m_Color = aOther.GetColor();
 }
 
 
@@ -131,6 +131,13 @@ int BOARD_STACKUP_ITEM::GetMaskDefaultThickness()
 
 
 // Getters:
+wxString BOARD_STACKUP_ITEM::GetColor( int aDielectricSubLayer ) const
+{
+    wxASSERT( aDielectricSubLayer >= 0 && aDielectricSubLayer < GetSublayersCount() );
+
+    return m_DielectricPrmsList[aDielectricSubLayer].m_Color;
+}
+
 int BOARD_STACKUP_ITEM::GetThickness( int aDielectricSubLayer ) const
 {
     wxASSERT( aDielectricSubLayer >= 0 && aDielectricSubLayer < GetSublayersCount() );
@@ -172,6 +179,15 @@ wxString BOARD_STACKUP_ITEM::GetMaterial( int aDielectricSubLayer ) const
 
 
 // Setters:
+void BOARD_STACKUP_ITEM::SetColor(  const wxString& aColorName , int aDielectricSubLayer )
+{
+    wxASSERT( aDielectricSubLayer >= 0 && aDielectricSubLayer < GetSublayersCount() );
+
+    if( aDielectricSubLayer >= 0 && aDielectricSubLayer < GetSublayersCount() )
+        m_DielectricPrmsList[aDielectricSubLayer].m_Color = aColorName;
+}
+
+
 void BOARD_STACKUP_ITEM::SetThickness( int aThickness, int aDielectricSubLayer )
 {
     wxASSERT( aDielectricSubLayer >= 0 && aDielectricSubLayer < GetSublayersCount() );
@@ -647,6 +663,12 @@ void BOARD_STACKUP::FormatBoardStackup( OUTPUTFORMATTER* aFormatter,
                 aFormatter->Print( nest_level+1, "addsublayer" );
             }
 
+            if( item->IsColorEditable() && IsPrmSpecified( item->GetColor( idx ) ) )
+            {
+                aFormatter->Print( 0, " (color %s)",
+                                   aFormatter->Quotew( item->GetColor( idx ) ).c_str() );
+            }
+
             if( item->IsThicknessEditable() )
             {
                 if( item->GetType() == BS_ITEM_TYPE_DIELECTRIC && item->IsThicknessLocked( idx ) )
diff --git a/pcbnew/board_stackup_manager/board_stackup.h b/pcbnew/board_stackup_manager/board_stackup.h
index f407ab5354..294f41cc77 100644
--- a/pcbnew/board_stackup_manager/board_stackup.h
+++ b/pcbnew/board_stackup_manager/board_stackup.h
@@ -78,6 +78,7 @@ private:
                             /// (for impedance controlled purposes), unused for other layers
     double m_EpsilonR;      /// For dielectric (and solder mask) the dielectric constant
     double m_LossTangent;   /// For dielectric (and solder mask) the dielectric loss
+    wxString m_Color;       /// mainly for silkscreen and solder mask
 };
 
 
@@ -151,11 +152,11 @@ public:
 
     BOARD_STACKUP_ITEM_TYPE GetType() const { return m_Type; }
     PCB_LAYER_ID GetBrdLayerId() const { return m_LayerId; }
-    wxString GetColor() const { return m_Color; }
     wxString GetLayerName() const { return m_LayerName; }
     wxString GetTypeName() const { return m_TypeName; }
     int GetDielectricLayerId() const { return m_DielectricLayerId; }
 
+    wxString GetColor( int aDielectricSubLayer = 0 ) const;
     int GetThickness( int aDielectricSubLayer = 0 ) const;
     bool IsThicknessLocked( int aDielectricSubLayer = 0 ) const;
     double GetEpsilonR( int aDielectricSubLayer = 0 ) const;
@@ -165,11 +166,11 @@ public:
     // Setters:
     void SetEnabled( bool aEnable) { m_enabled = aEnable; }
     void SetBrdLayerId( PCB_LAYER_ID aBrdLayerId ) { m_LayerId = aBrdLayerId; }
-    void SetColor( const wxString& aColorName ){ m_Color = aColorName; }
     void SetLayerName( const wxString& aName ) { m_LayerName = aName; }
     void SetTypeName( const wxString& aName ) { m_TypeName = aName; }
     void SetDielectricLayerId( int aLayerId ) { m_DielectricLayerId = aLayerId; }
 
+    void SetColor( const wxString& aColorName, int aDielectricSubLayer = 0 );
     void SetThickness( int aThickness, int aDielectricSubLayer = 0 );
     void SetThicknessLocked( bool aLocked, int aDielectricSubLayer = 0 );
     void SetEpsilonR( double aEpsilon, int aDielectricSubLayer = 0 );
@@ -180,7 +181,6 @@ private:
     BOARD_STACKUP_ITEM_TYPE m_Type;
     wxString m_LayerName;   /// name of layer as shown in layer manager. Useful to create reports
     wxString m_TypeName;    /// type name of layer (copper, silk screen, core, prepreg ...)
-    wxString m_Color;       /// mainly for silkscreen and solder mask
     PCB_LAYER_ID m_LayerId; /// the layer id (F.Cu to B.Cu, F.Silk, B.silk, F.Mask, B.Mask)
                             /// and UNDEFINED_LAYER (-1) for dielectric layers that are not
                             /// really layers for the board editor
diff --git a/pcbnew/board_stackup_manager/panel_board_stackup.cpp b/pcbnew/board_stackup_manager/panel_board_stackup.cpp
index 7cd689c37e..ba8eb33723 100644
--- a/pcbnew/board_stackup_manager/panel_board_stackup.cpp
+++ b/pcbnew/board_stackup_manager/panel_board_stackup.cpp
@@ -631,9 +631,9 @@ void PANEL_SETUP_BOARD_STACKUP::synchronizeWithBoard( bool aFullSync )
             auto bm_combo = dynamic_cast<wxBitmapComboBox*>( ui_row_item.m_ColorCtrl );
             int  selected = 0;  // The "not specified" item
 
-            if( item->GetColor().StartsWith( wxT( "#" ) ) )  // User defined color
+            if( item->GetColor( sub_item ).StartsWith( wxT( "#" ) ) )  // User defined color
             {
-                COLOR4D custom_color( item->GetColor() );
+                COLOR4D custom_color( item->GetColor( sub_item ) );
 
                 ui_row_item.m_UserColor = custom_color;
 
@@ -641,7 +641,7 @@ void PANEL_SETUP_BOARD_STACKUP::synchronizeWithBoard( bool aFullSync )
 
                 if( bm_combo )      // Update user color shown in the wxBitmapComboBox
                 {
-                    bm_combo->SetString( selected, item->GetColor() );
+                    bm_combo->SetString( selected, item->GetColor( sub_item ) );
                     wxBitmap layerbmp( m_colorSwatchesSize.x, m_colorSwatchesSize.y );
                     LAYER_SELECTOR::DrawColorSwatch( layerbmp, COLOR4D(), custom_color );
                     bm_combo->SetItemBitmap( selected, layerbmp );
@@ -655,7 +655,7 @@ void PANEL_SETUP_BOARD_STACKUP::synchronizeWithBoard( bool aFullSync )
                     // translated.
                     for( int ii = 0; ii < GetStandardColorCount( item->GetType()); ii++ )
                     {
-                        if( GetStandardColorName( item->GetType(), ii ) == item->GetColor() )
+                        if( GetStandardColorName( item->GetType(), ii ) == item->GetColor( sub_item ) )
                         {
                             selected = ii;
                             break;
@@ -888,9 +888,9 @@ BOARD_STACKUP_ROW_UI_ITEM PANEL_SETUP_BOARD_STACKUP::createRowData( int aRow,
 
     if( item->IsColorEditable() )
     {
-        if( item->GetColor().StartsWith( wxT( "#" ) ) )  // User defined color
+        if( item->GetColor( aSublayerIdx ).StartsWith( wxT( "#" ) ) )  // User defined color
         {
-            ui_row_item.m_UserColor = COLOR4D( item->GetColor() ).ToColour();
+            ui_row_item.m_UserColor = COLOR4D( item->GetColor( aSublayerIdx ) ).ToColour();
         }
         else
             ui_row_item.m_UserColor = GetDefaultUserColor( item->GetType() );
@@ -900,17 +900,17 @@ BOARD_STACKUP_ROW_UI_ITEM PANEL_SETUP_BOARD_STACKUP::createRowData( int aRow,
 
         m_fgGridSizer->Add( bm_combo, 1, wxLEFT|wxRIGHT|wxALIGN_CENTER_VERTICAL|wxEXPAND, 2 );
 
-        if( item->GetColor().StartsWith( wxT( "#" ) ) )
+        if( item->GetColor( aSublayerIdx ).StartsWith( wxT( "#" ) ) )
         {
             selected = GetColorUserDefinedListIdx( item->GetType() );
-            bm_combo->SetString( selected, item->GetColor() );
+            bm_combo->SetString( selected, item->GetColor( aSublayerIdx ) );
         }
         else
         {
             // Note: don't use bm_combo->FindString() because the combo strings are translated.
             for( int ii = 0; ii < GetStandardColorCount( item->GetType()); ii++ )
             {
-                if( GetStandardColorName( item->GetType(), ii ) == item->GetColor() )
+                if( GetStandardColorName( item->GetType(), ii ) == item->GetColor( aSublayerIdx ) )
                 {
                     selected = ii;
                     break;
@@ -1203,7 +1203,7 @@ bool PANEL_SETUP_BOARD_STACKUP::transferDataFromUIToStackup()
             }
         }
 
-        if( sub_item == 0 && item->IsColorEditable() )
+        if( item->IsColorEditable() )
         {
             wxBitmapComboBox* choice = dynamic_cast<wxBitmapComboBox*>( ui_item.m_ColorCtrl );
 
@@ -1212,9 +1212,9 @@ bool PANEL_SETUP_BOARD_STACKUP::transferDataFromUIToStackup()
                 int idx = choice->GetSelection();
 
                 if( IsCustomColorIdx( item->GetType(), idx ) )
-                    item->SetColor( ui_item.m_UserColor.ToHexString() );
+                    item->SetColor( ui_item.m_UserColor.ToHexString(), sub_item );
                 else
-                    item->SetColor( GetStandardColorName( item->GetType(), idx ) );
+                    item->SetColor( GetStandardColorName( item->GetType(), idx ), sub_item );
             }
         }
 
@@ -1444,25 +1444,25 @@ void PANEL_SETUP_BOARD_STACKUP::onMaterialChange( wxCommandEvent& event )
     textCtrl->ChangeValue( item->GetMaterial( sub_item ) );
 
     if( item->GetType() == BS_ITEM_TYPE_DIELECTRIC
-            && !item->GetColor().StartsWith( "#" ) /* User defined color */ )
+            && !item->GetColor( sub_item ).StartsWith( "#" ) /* User defined color */ )
     {
         if( substrate.m_Name.IsSameAs( "PTFE" )
               || substrate.m_Name.IsSameAs( "Teflon" ) )
         {
-            item->SetColor( "PTFE natural" );
+            item->SetColor( "PTFE natural", sub_item );
         }
         else if( substrate.m_Name.IsSameAs( "Polyimide" )
               || substrate.m_Name.IsSameAs( "Kapton" ) )
         {
-            item->SetColor( "Polyimide" );
+            item->SetColor( "Polyimide", sub_item );
         }
         else if( substrate.m_Name.IsSameAs( "Al" ) )
         {
-            item->SetColor( "Aluminum" );
+            item->SetColor( "Aluminum", sub_item );
         }
         else
         {
-            item->SetColor( "FR4 natural" );
+            item->SetColor( "FR4 natural", sub_item );
         }
     }
 
@@ -1470,7 +1470,7 @@ void PANEL_SETUP_BOARD_STACKUP::onMaterialChange( wxCommandEvent& event )
 
     for( int ii = 0; ii < GetStandardColorCount( item->GetType()); ii++ )
     {
-        if( GetStandardColorName( item->GetType(), ii ) == item->GetColor() )
+        if( GetStandardColorName( item->GetType(), ii ) == item->GetColor( sub_item ) )
         {
             picker->SetSelection( ii );
             break;
diff --git a/pcbnew/exporters/gerber_jobfile_writer.cpp b/pcbnew/exporters/gerber_jobfile_writer.cpp
index 19c48450f0..4819d411b1 100644
--- a/pcbnew/exporters/gerber_jobfile_writer.cpp
+++ b/pcbnew/exporters/gerber_jobfile_writer.cpp
@@ -656,9 +656,9 @@ void GERBER_JOBFILE_WRITER::addJSONMaterialStackup()
 
             if( item->IsColorEditable() && uptodate )
             {
-                if( IsPrmSpecified( item->GetColor() ) )
+                if( IsPrmSpecified( item->GetColor( sub_idx ) ) )
                 {
-                    wxString colorName = item->GetColor();
+                    wxString colorName = item->GetColor( sub_idx );
 
                     if( colorName.StartsWith( wxT( "#" ) ) ) // This is a user defined color.
                     {
diff --git a/pcbnew/plugins/kicad/pcb_parser.cpp b/pcbnew/plugins/kicad/pcb_parser.cpp
index fe88e726b5..fb162bf1ba 100644
--- a/pcbnew/plugins/kicad/pcb_parser.cpp
+++ b/pcbnew/plugins/kicad/pcb_parser.cpp
@@ -1570,7 +1570,7 @@ void PCB_PARSER::parseBoardStackup()
                                          wx_color.Alpha() );
                         }
 
-                        item->SetColor( name );
+                        item->SetColor( name, sublayer_idx );
                         NeedRIGHT();
                         break;