Fix layer writing/reading for copper zones

* Always enumerate layers - never use the wildcards
* Keep fills on layers the zone is actually on when loading

Fixes https://gitlab.com/kicad/code/kicad/-/issues/19775

(cherry picked from commit 088e0e80a1)

pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp

@@ -1357,7 +1357,7 @@ void PCB_IO_KICAD_SEXPR::format( const FOOTPRINT* aFootprint, int aNestLevel ) c
 }
 
 
-void PCB_IO_KICAD_SEXPR::formatLayers( LSET aLayerMask, int aNestLevel ) const
+void PCB_IO_KICAD_SEXPR::formatLayers( LSET aLayerMask, int aNestLevel, bool aEnumerateLayers ) const
 {
     std::string output;
 
@@ -1379,53 +1379,55 @@ void PCB_IO_KICAD_SEXPR::formatLayers( LSET aLayerMask, int aNestLevel ) const
 
     LSET cu_mask = cu_all;
 
-    // output copper layers first, then non copper
+    if( !aEnumerateLayers )
+    {
+        // output copper layers first, then non copper
+        if( ( aLayerMask & cu_mask ) == cu_mask )
+        {
+            output += ' ' + m_out->Quotew( "*.Cu" );
+            aLayerMask &= ~cu_all;          // clear bits, so they are not output again below
+        }
+        else if( ( aLayerMask & cu_mask ) == fr_bk )
+        {
+            output += ' ' + m_out->Quotew( "F&B.Cu" );
+            aLayerMask &= ~fr_bk;
+        }
 
-    if( ( aLayerMask & cu_mask ) == cu_mask )
-    {
-        output += ' ' + m_out->Quotew( "*.Cu" );
-        aLayerMask &= ~cu_all;          // clear bits, so they are not output again below
-    }
-    else if( ( aLayerMask & cu_mask ) == fr_bk )
-    {
-        output += ' ' + m_out->Quotew( "F&B.Cu" );
-        aLayerMask &= ~fr_bk;
-    }
+        if( ( aLayerMask & adhes ) == adhes )
+        {
+            output += ' ' + m_out->Quotew( "*.Adhes" );
+            aLayerMask &= ~adhes;
+        }
 
-    if( ( aLayerMask & adhes ) == adhes )
-    {
-        output += ' ' + m_out->Quotew( "*.Adhes" );
-        aLayerMask &= ~adhes;
-    }
+        if( ( aLayerMask & paste ) == paste )
+        {
+            output += ' ' + m_out->Quotew( "*.Paste" );
+            aLayerMask &= ~paste;
+        }
 
-    if( ( aLayerMask & paste ) == paste )
-    {
-        output += ' ' + m_out->Quotew( "*.Paste" );
-        aLayerMask &= ~paste;
-    }
+        if( ( aLayerMask & silks ) == silks )
+        {
+            output += ' ' + m_out->Quotew( "*.SilkS" );
+            aLayerMask &= ~silks;
+        }
 
-    if( ( aLayerMask & silks ) == silks )
-    {
-        output += ' ' + m_out->Quotew( "*.SilkS" );
-        aLayerMask &= ~silks;
-    }
+        if( ( aLayerMask & mask ) == mask )
+        {
+            output += ' ' + m_out->Quotew( "*.Mask" );
+            aLayerMask &= ~mask;
+        }
 
-    if( ( aLayerMask & mask ) == mask )
-    {
-        output += ' ' + m_out->Quotew( "*.Mask" );
-        aLayerMask &= ~mask;
-    }
+        if( ( aLayerMask & crt_yd ) == crt_yd )
+        {
+            output += ' ' + m_out->Quotew( "*.CrtYd" );
+            aLayerMask &= ~crt_yd;
+        }
 
-    if( ( aLayerMask & crt_yd ) == crt_yd )
-    {
-        output += ' ' + m_out->Quotew( "*.CrtYd" );
-        aLayerMask &= ~crt_yd;
-    }
-
-    if( ( aLayerMask & fab ) == fab )
-    {
-        output += ' ' + m_out->Quotew( "*.Fab" );
-        aLayerMask &= ~fab;
+        if( ( aLayerMask & fab ) == fab )
+        {
+            output += ' ' + m_out->Quotew( "*.Fab" );
+            aLayerMask &= ~fab;
+        }
     }
 
     // output any individual layers not handled in wildcard combos above
@@ -2219,9 +2221,10 @@ void PCB_IO_KICAD_SEXPR::format( const ZONE* aZone, int aNestLevel ) const
     if( aZone->GetBoard() )
         layers &= aZone->GetBoard()->GetEnabledLayers();
 
+    // Always enumerate every layer for a zone on a copper layer
     if( layers.count() > 1 )
     {
-        formatLayers( layers );
+        formatLayers( layers, 0 /* nest level */, aZone->IsOnCopperLayer() );
     }
     else
     {

pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.h

@@ -426,7 +426,7 @@ private:
 
     void formatLayer( PCB_LAYER_ID aLayer, bool aIsKnockout = false ) const;
 
-    void formatLayers( LSET aLayerMask, int aNestLevel = 0 ) const;
+    void formatLayers( LSET aLayerMask, int aNestLevel = 0, bool aEnumerateLayers = false ) const;
 
     friend class FP_CACHE;
 

pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr_parser.cpp

@@ -1175,7 +1175,7 @@ BOARD* PCB_IO_KICAD_SEXPR_PARSER::parseBOARD_unchecked()
         {
             ZONE* z = static_cast<ZONE*>( zone );
 
-            z->SetLayerSet( z->GetLayerSet() & layers );
+            z->SetLayerSetAndRemoveUnusedFills( z->GetLayerSet() & layers );
         }
     }
 

pcbnew/zone.cpp

@@ -305,6 +305,29 @@ void ZONE::SetLayerSet( LSET aLayerSet )
 }
 
 
+void ZONE::SetLayerSetAndRemoveUnusedFills( LSET aLayerSet )
+{
+    if( aLayerSet.count() == 0 )
+        return;
+
+    if( m_layerSet != aLayerSet )
+    {
+        for( PCB_LAYER_ID layer : aLayerSet.Seq() )
+        {
+            // Only keep layers that are present in the new set
+            if( !aLayerSet.Contains( layer ) )
+            {
+                m_FilledPolysList[layer]  = std::make_shared<SHAPE_POLY_SET>();
+                m_filledPolysHash[layer]  = {};
+                m_insulatedIslands[layer] = {};
+            }
+        }
+    }
+
+    m_layerSet = aLayerSet;
+}
+
+
 void ZONE::ViewGetLayers( int aLayers[], int& aCount ) const
 {
     aCount = 0;

pcbnew/zone.h

@@ -128,6 +128,13 @@ public:
     void SetLayerSet( LSET aLayerSet ) override;
     virtual LSET GetLayerSet() const override { return m_layerSet; }
 
+    /**
+     * Set the zone to be on the aLayerSet layers and only remove the fill polygons
+     * from the unused layers, while keeping the fills on the layers in both the old
+     * and new layer sets.
+     */
+    void SetLayerSetAndRemoveUnusedFills( LSET aLayerSet );
+
     const wxString& GetZoneName() const { return m_zoneName; }
     void SetZoneName( const wxString& aName ) { m_zoneName = aName; }
 

qa/tests/pcbnew/CMakeLists.txt

@@ -64,6 +64,8 @@ set( QA_PCBNEW_SRCS
     pcb_io/cadstar/test_cadstar_footprints.cpp
     pcb_io/eagle/test_eagle_lbr_import.cpp
 
+    pcb_io/kicad_sexpr/test_kicad_sexpr.cpp
+
     group_saveload.cpp
 )
 

qa/tests/pcbnew/pcb_io/kicad_sexpr/test_kicad_sexpr.cpp

@@ -0,0 +1,103 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright The KiCad Developers, see AUTHORS.TXT for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include <filesystem>
+#include <string>
+
+#include <pcbnew_utils/board_test_utils.h>
+#include <pcbnew_utils/board_file_utils.h>
+#include <qa_utils/wx_utils/unit_test_utils.h>
+
+#include <pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.h>
+
+#include <board.h>
+#include <zone.h>
+
+
+struct KICAD_SEXPR_FIXTURE
+{
+    KICAD_SEXPR_FIXTURE() {}
+
+    PCB_IO_KICAD_SEXPR kicadPlugin;
+};
+
+
+/**
+ * Declares the struct as the Boost test fixture.
+ */
+BOOST_FIXTURE_TEST_SUITE( KiCadSexprIO, KICAD_SEXPR_FIXTURE )
+
+
+/**
+ * Compare all footprints declared in a *.lbr file with their KiCad reference footprint
+ */
+BOOST_AUTO_TEST_CASE( Issue19775_ZoneLayerWildcards )
+{
+    std::string dataPath = KI_TEST::GetPcbnewTestDataDir() + "plugins/kicad_sexpr/Issue19775_ZoneLayers/";
+
+    BOOST_TEST_CONTEXT( "Zone layers with wildcards" )
+    {
+        std::unique_ptr<BOARD> testBoard = std::make_unique<BOARD>();
+
+        kicadPlugin.LoadBoard( dataPath + "LayerWildcard.kicad_pcb", testBoard.get() );
+
+        // One zone in the file
+        BOOST_CHECK( testBoard->Zones().size() == 1 );
+
+        ZONE* z = testBoard->Zones()[0];
+
+        // On both front and back layers, with zone fill on both
+        BOOST_CHECK( z->GetLayerSet().Contains( F_Cu ) && z->GetLayerSet().Contains( B_Cu ) );
+        BOOST_CHECK( z->GetFilledPolysList( F_Cu )->TotalVertices() > 0 );
+        BOOST_CHECK( z->GetFilledPolysList( B_Cu )->TotalVertices() > 0 );
+    }
+
+    BOOST_TEST_CONTEXT( "Round trip layers" )
+    {
+        auto tmpBoard = std::filesystem::temp_directory_path() / "Issue19775_RoundTrip.kicad_pcb";
+
+        // Load and save the board from above to test how we write the zones into it
+        {
+            std::unique_ptr<BOARD> testBoard = std::make_unique<BOARD>();
+            kicadPlugin.LoadBoard( dataPath + "LayerEnumerate.kicad_pcb", testBoard.get() );
+            kicadPlugin.SaveBoard( tmpBoard.string(), testBoard.get() );
+        }
+
+        // Read the new board
+        std::unique_ptr<BOARD> testBoard = std::make_unique<BOARD>();
+        kicadPlugin.LoadBoard( tmpBoard.string(), testBoard.get() );
+
+        // One zone in the file
+        BOOST_CHECK( testBoard->Zones().size() == 1 );
+
+        ZONE* z = testBoard->Zones()[0];
+
+        // On both front and back layers, with zone fill on both
+        BOOST_CHECK( z->GetLayerSet().Contains( F_Cu ) && z->GetLayerSet().Contains( B_Cu ) );
+        BOOST_CHECK( z->GetFilledPolysList( F_Cu )->TotalVertices() > 0 );
+        BOOST_CHECK( z->GetFilledPolysList( B_Cu )->TotalVertices() > 0 );
+    }
+}
+
+
+BOOST_AUTO_TEST_SUITE_END()