Fix some determinism in saving

Teardrops don't get their own unique UUID saved in the file, so they can
end up being sorted differently on save.  Instead, we assign a synthetic
UUID to the teardrop based on its two connecting elements' UUIDs.

This also fixes the comparison function so that we don't end up mixing
around caches

common/kiid.cpp

@@ -286,6 +286,17 @@ void KIID::Increment()
 }
 
 
+KIID KIID::Combine( const KIID& aFirst, const KIID& aSecond )
+{
+    KIID result( 0 );
+
+    for( int i = 0; i < 16; ++i )
+        result.m_uuid.data[i] = aFirst.m_uuid.data[i] ^ aSecond.m_uuid.data[i];
+
+    return result;
+}
+
+
 void KIID::CreateNilUuids( bool aNil )
 {
     g_createNilUuids = aNil;

include/kiid.h

@@ -96,6 +96,18 @@ public:
      */
     void ConvertTimestampToUuid();
 
+    /**
+     * Creates a deterministic KIID from two input KIIDs by XORing their underlying UUIDs.
+     *
+     * This is useful for generating stable IDs for derived objects (like teardrops) that are
+     * created from the combination of two parent objects.
+     *
+     * @param aFirst the first KIID to combine
+     * @param aSecond the second KIID to combine
+     * @return a new KIID that is the XOR combination of the two inputs
+     */
+    static KIID Combine( const KIID& aFirst, const KIID& aSecond );
+
     /**
      * Generates a deterministic replacement for a given ID.
      *

pcbnew/board.cpp

@@ -3402,34 +3402,39 @@ bool BOARD::cmp_drawings::operator()( const BOARD_ITEM* aFirst, const BOARD_ITEM
     {
         const PCB_SHAPE* shape = static_cast<const PCB_SHAPE*>( aFirst );
         const PCB_SHAPE* other = static_cast<const PCB_SHAPE*>( aSecond );
-        return shape->Compare( other );
+        return shape->Compare( other ) < 0;
     }
     else if( aFirst->Type() == PCB_TEXT_T || aFirst->Type() == PCB_FIELD_T )
     {
         const PCB_TEXT* text = static_cast<const PCB_TEXT*>( aFirst );
         const PCB_TEXT* other = static_cast<const PCB_TEXT*>( aSecond );
-        return text->Compare( other );
+        return text->Compare( other ) < 0;
     }
     else if( aFirst->Type() == PCB_TEXTBOX_T )
     {
         const PCB_TEXTBOX* textbox = static_cast<const PCB_TEXTBOX*>( aFirst );
         const PCB_TEXTBOX* other = static_cast<const PCB_TEXTBOX*>( aSecond );
 
-        return textbox->PCB_SHAPE::Compare( other ) && textbox->EDA_TEXT::Compare( other );
+        int shapeCmp = textbox->PCB_SHAPE::Compare( other );
+
+        if( shapeCmp != 0 )
+            return shapeCmp < 0;
+
+        return textbox->EDA_TEXT::Compare( other ) < 0;
     }
     else if( aFirst->Type() == PCB_TABLE_T )
     {
         const PCB_TABLE* table = static_cast<const PCB_TABLE*>( aFirst );
         const PCB_TABLE* other = static_cast<const PCB_TABLE*>( aSecond );
 
-        return PCB_TABLE::Compare( table, other );
+        return PCB_TABLE::Compare( table, other ) < 0;
     }
     else if( aFirst->Type() == PCB_BARCODE_T )
     {
         const PCB_BARCODE* barcode = static_cast<const PCB_BARCODE*>( aFirst );
         const PCB_BARCODE* other = static_cast<const PCB_BARCODE*>( aSecond );
 
-        return PCB_BARCODE::Compare( barcode, other );
+        return PCB_BARCODE::Compare( barcode, other ) < 0;
     }
 
     return aFirst->m_Uuid < aSecond->m_Uuid;

pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp

@@ -794,12 +794,12 @@ void PCB_IO_KICAD_SEXPR::format( const BOARD* aBoard ) const
 {
     std::set<BOARD_ITEM*, BOARD_ITEM::ptr_cmp> sorted_footprints( aBoard->Footprints().begin(),
                                                                   aBoard->Footprints().end() );
-    std::set<BOARD_ITEM*, BOARD_ITEM::ptr_cmp> sorted_drawings( aBoard->Drawings().begin(),
+    std::set<BOARD_ITEM*, BOARD::cmp_drawings> sorted_drawings( aBoard->Drawings().begin(),
                                                                 aBoard->Drawings().end() );
     std::set<PCB_TRACK*, PCB_TRACK::cmp_tracks> sorted_tracks( aBoard->Tracks().begin(),
                                                                aBoard->Tracks().end() );
     std::set<PCB_POINT*, BOARD_ITEM::ptr_cmp> sorted_points( aBoard->Points().begin(),
-                                                               aBoard->Points().end() );
+                                                             aBoard->Points().end() );
     std::set<BOARD_ITEM*, BOARD_ITEM::ptr_cmp> sorted_zones( aBoard->Zones().begin(),
                                                              aBoard->Zones().end() );
     std::set<BOARD_ITEM*, BOARD_ITEM::ptr_cmp> sorted_groups( aBoard->Groups().begin(),

pcbnew/teardrop/teardrop.cpp

@@ -55,10 +55,15 @@ TEARDROP_MANAGER::TEARDROP_MANAGER( BOARD* aBoard, TOOL_MANAGER* aToolManager )
 
 
 ZONE* TEARDROP_MANAGER::createTeardrop( TEARDROP_VARIANT aTeardropVariant,
-                                        std::vector<VECTOR2I>& aPoints, PCB_TRACK* aTrack ) const
+                                        std::vector<VECTOR2I>& aPoints, PCB_TRACK* aTrack,
+                                        BOARD_ITEM* aCandidate ) const
 {
     ZONE* teardrop = new ZONE( m_board );
 
+    // Create a deterministic UUID from the track and candidate UUIDs so that teardrops
+    // maintain stable ordering in the output file across save/load cycles.
+    const_cast<KIID&>( teardrop->m_Uuid ) = KIID::Combine( aTrack->m_Uuid, aCandidate->m_Uuid );
+
     // teardrop settings are the last zone settings used by a zone dialog.
     // override them by default.
     ZONE_SETTINGS::GetDefaultSettings().ExportSetting( *teardrop, false );
@@ -94,10 +99,17 @@ ZONE* TEARDROP_MANAGER::createTeardrop( TEARDROP_VARIANT aTeardropVariant,
 
 
 ZONE* TEARDROP_MANAGER::createTeardropMask( TEARDROP_VARIANT aTeardropVariant,
-                                            std::vector<VECTOR2I>& aPoints, PCB_TRACK* aTrack ) const
+                                            std::vector<VECTOR2I>& aPoints, PCB_TRACK* aTrack,
+                                            BOARD_ITEM* aCandidate ) const
 {
     ZONE* teardrop = new ZONE( m_board );
 
+    // Create a deterministic UUID from the track and candidate UUIDs, then increment it
+    // to differentiate from the copper teardrop zone.
+    KIID maskUuid = KIID::Combine( aTrack->m_Uuid, aCandidate->m_Uuid );
+    maskUuid.Increment();
+    const_cast<KIID&>( teardrop->m_Uuid ) = maskUuid;
+
     teardrop->SetTeardropAreaType( aTeardropVariant == TD_TYPE_PADVIA ? TEARDROP_TYPE::TD_VIAPAD
                                                                       : TEARDROP_TYPE::TD_TRACKEND );
     teardrop->SetLayer( aTrack->GetLayer() == F_Cu ? F_Mask : B_Mask );
@@ -135,9 +147,9 @@ ZONE* TEARDROP_MANAGER::createTeardropMask( TEARDROP_VARIANT aTeardropVariant,
 void TEARDROP_MANAGER::createAndAddTeardropWithMask( BOARD_COMMIT& aCommit,
                                                      TEARDROP_VARIANT aTeardropVariant,
                                                      std::vector<VECTOR2I>& aPoints,
-                                                     PCB_TRACK* aTrack )
+                                                     PCB_TRACK* aTrack, BOARD_ITEM* aCandidate )
 {
-    ZONE* new_teardrop = createTeardrop( aTeardropVariant, aPoints, aTrack );
+    ZONE* new_teardrop = createTeardrop( aTeardropVariant, aPoints, aTrack, aCandidate );
     m_board->Add( new_teardrop, ADD_MODE::BULK_INSERT );
     m_createdTdList.push_back( new_teardrop );
 
@@ -145,7 +157,7 @@ void TEARDROP_MANAGER::createAndAddTeardropWithMask( BOARD_COMMIT& aCommit,
 
     if( aTrack->HasSolderMask() && IsExternalCopperLayer( aTrack->GetLayer() ) )
     {
-        ZONE* new_teardrop_mask = createTeardropMask( aTeardropVariant, aPoints, aTrack );
+        ZONE* new_teardrop_mask = createTeardropMask( aTeardropVariant, aPoints, aTrack, aCandidate );
         m_board->Add( new_teardrop_mask, ADD_MODE::BULK_INSERT );
         aCommit.Added( new_teardrop_mask );
     }
@@ -162,7 +174,7 @@ bool TEARDROP_MANAGER::tryCreateTrackTeardrop( BOARD_COMMIT& aCommit,
 
     if( computeTeardropPolygon( aParams, points, aTrack, aCandidate, aPos ) )
     {
-        createAndAddTeardropWithMask( aCommit, aTeardropVariant, points, aTrack );
+        createAndAddTeardropWithMask( aCommit, aTeardropVariant, points, aTrack, aCandidate );
         return true;
     }
 

pcbnew/teardrop/teardrop.h

@@ -203,12 +203,13 @@ private:
      * @param aPoints is the polygonal shape
      * @param aTrack is the track connected to the starting points of the teardrop
      * (mainly for net info)
+     * @param aCandidate is the pad/via/track that the teardrop connects to (used for UUID)
      */
-    ZONE* createTeardrop( TEARDROP_VARIANT aTeardropVariant,
-                          std::vector<VECTOR2I>& aPoints, PCB_TRACK* aTrack ) const;
+    ZONE* createTeardrop( TEARDROP_VARIANT aTeardropVariant, std::vector<VECTOR2I>& aPoints,
+                          PCB_TRACK* aTrack, BOARD_ITEM* aCandidate ) const;
 
-    ZONE* createTeardropMask( TEARDROP_VARIANT aTeardropVariant,
-                              std::vector<VECTOR2I>& aPoints, PCB_TRACK* aTrack ) const;
+    ZONE* createTeardropMask( TEARDROP_VARIANT aTeardropVariant, std::vector<VECTOR2I>& aPoints,
+                              PCB_TRACK* aTrack, BOARD_ITEM* aCandidate ) const;
 
     /**
      * Creates and adds a teardrop with optional mask to the board
@@ -216,9 +217,11 @@ private:
      * @param aTeardropVariant = variant of the teardrop( attached to a pad, or a track end )
      * @param aPoints is the polygonal shape
      * @param aTrack is the track connected to the starting points of the teardrop
+     * @param aCandidate is the pad/via/track that the teardrop connects to
      */
     void createAndAddTeardropWithMask( BOARD_COMMIT& aCommit, TEARDROP_VARIANT aTeardropVariant,
-                                       std::vector<VECTOR2I>& aPoints, PCB_TRACK* aTrack );
+                                       std::vector<VECTOR2I>& aPoints, PCB_TRACK* aTrack,
+                                       BOARD_ITEM* aCandidate );
 
     /**
      * Attempts to create a track-to-track teardrop

qa/tests/common/test_kiid.cpp

@@ -104,4 +104,70 @@ BOOST_AUTO_TEST_CASE( LegacyTimestamp )
 }
 
 
+BOOST_AUTO_TEST_CASE( CombineDeterministic )
+{
+    KIID a, b, c;
+
+    // Combining the same two KIIDs should always produce the same result
+    KIID combined1 = KIID::Combine( a, b );
+    KIID combined2 = KIID::Combine( a, b );
+    BOOST_CHECK_EQUAL( combined1.AsString(), combined2.AsString() );
+
+    // Combined result should be different from inputs
+    BOOST_CHECK( combined1 != a );
+    BOOST_CHECK( combined1 != b );
+
+    // Different inputs should produce different outputs
+    KIID combined3 = KIID::Combine( a, c );
+    BOOST_CHECK( combined1 != combined3 );
+
+    // Order matters for XOR with different values
+    KIID combined4 = KIID::Combine( b, a );
+    BOOST_CHECK_EQUAL( combined1.AsString(), combined4.AsString() );
+}
+
+
+BOOST_AUTO_TEST_CASE( CombineWithKnownValues )
+{
+    // Test with known UUID strings to verify XOR behavior
+    KIID a( "00000000-0000-0000-0000-000000000001" );
+    KIID b( "00000000-0000-0000-0000-000000000002" );
+
+    KIID combined = KIID::Combine( a, b );
+
+    // XOR of 0x01 and 0x02 = 0x03
+    BOOST_CHECK_EQUAL( combined.AsString(), "00000000-0000-0000-0000-000000000003" );
+
+    // Combining with self should give all zeros
+    KIID selfCombined = KIID::Combine( a, a );
+    BOOST_CHECK_EQUAL( selfCombined.AsString(), "00000000-0000-0000-0000-000000000000" );
+}
+
+
+BOOST_AUTO_TEST_CASE( CombineUniqueness )
+{
+    // Generate several random KIIDs and verify all combinations are unique.
+    // XOR is commutative, so Combine(a,b) == Combine(b,a). We only count
+    // unique unordered pairs, which gives n*(n-1)/2 combinations.
+    std::vector<KIID> ids;
+
+    for( int i = 0; i < 10; ++i )
+        ids.push_back( KIID() );
+
+    std::set<wxString> combinations;
+
+    for( size_t i = 0; i < ids.size(); ++i )
+    {
+        for( size_t j = i + 1; j < ids.size(); ++j )
+        {
+            KIID combined = KIID::Combine( ids[i], ids[j] );
+            combinations.insert( combined.AsString() );
+        }
+    }
+
+    // All 45 unique unordered pairs (10*9/2) should produce unique results
+    BOOST_CHECK_EQUAL( combinations.size(), 45 );
+}
+
+
 BOOST_AUTO_TEST_SUITE_END()

qa/tests/pcbnew/CMakeLists.txt

@@ -59,6 +59,7 @@ set( QA_PCBNEW_SRCS
     test_pcb_grid_helper.cpp
     test_save_load.cpp
     test_stacked_pin_netlist.cpp
+    test_teardrop_uuid.cpp
     test_tracks_cleaner.cpp
     test_triangulation.cpp
     test_multichannel.cpp

qa/tests/pcbnew/test_teardrop_uuid.cpp

@@ -0,0 +1,120 @@
+/*
+ * 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
+ */
+
+/**
+ * @file test_teardrop_uuid.cpp
+ * Test that deterministic UUID generation works correctly for teardrop scenarios.
+ * The core KIID::Combine function is tested in test_kiid.cpp. These tests verify
+ * the expected behavior when combining track and pad UUIDs as used by teardrops.
+ */
+
+#include <qa_utils/wx_utils/unit_test_utils.h>
+
+#include <kiid.h>
+
+
+BOOST_AUTO_TEST_SUITE( TeardropUUID )
+
+
+BOOST_AUTO_TEST_CASE( CombineTrackAndPadUuids )
+{
+    // Simulate the teardrop UUID generation: combine track and pad UUIDs
+    KIID trackUuid;
+    KIID padUuid;
+
+    KIID teardropUuid = KIID::Combine( trackUuid, padUuid );
+
+    // The combined UUID should be different from both inputs
+    BOOST_CHECK( teardropUuid != trackUuid );
+    BOOST_CHECK( teardropUuid != padUuid );
+
+    // The same inputs should always produce the same output
+    KIID teardropUuid2 = KIID::Combine( trackUuid, padUuid );
+    BOOST_CHECK_EQUAL( teardropUuid.AsString(), teardropUuid2.AsString() );
+}
+
+
+BOOST_AUTO_TEST_CASE( MaskUuidIsIncrementedFromTeardropUuid )
+{
+    // Teardrop masks use the combined UUID, then increment to differentiate
+    KIID trackUuid;
+    KIID padUuid;
+
+    KIID teardropUuid = KIID::Combine( trackUuid, padUuid );
+
+    KIID maskUuid = KIID::Combine( trackUuid, padUuid );
+    maskUuid.Increment();
+
+    // The mask UUID should be different from the teardrop UUID
+    BOOST_CHECK( maskUuid != teardropUuid );
+
+    // Both should be deterministic
+    KIID teardropUuid2 = KIID::Combine( trackUuid, padUuid );
+    KIID maskUuid2 = KIID::Combine( trackUuid, padUuid );
+    maskUuid2.Increment();
+
+    BOOST_CHECK_EQUAL( teardropUuid.AsString(), teardropUuid2.AsString() );
+    BOOST_CHECK_EQUAL( maskUuid.AsString(), maskUuid2.AsString() );
+}
+
+
+BOOST_AUTO_TEST_CASE( DifferentTracksProduceDifferentTeardropUuids )
+{
+    // Two tracks connecting to the same pad should produce different teardrop UUIDs
+    KIID track1Uuid;
+    KIID track2Uuid;
+    KIID padUuid;
+
+    KIID teardrop1Uuid = KIID::Combine( track1Uuid, padUuid );
+    KIID teardrop2Uuid = KIID::Combine( track2Uuid, padUuid );
+
+    // Different tracks should produce different teardrops
+    BOOST_CHECK( teardrop1Uuid != teardrop2Uuid );
+
+    // Each should still be deterministic
+    KIID teardrop1Uuid2 = KIID::Combine( track1Uuid, padUuid );
+    KIID teardrop2Uuid2 = KIID::Combine( track2Uuid, padUuid );
+
+    BOOST_CHECK_EQUAL( teardrop1Uuid.AsString(), teardrop1Uuid2.AsString() );
+    BOOST_CHECK_EQUAL( teardrop2Uuid.AsString(), teardrop2Uuid2.AsString() );
+}
+
+
+BOOST_AUTO_TEST_CASE( StableAcrossRegenerations )
+{
+    // Simulates regenerating teardrops: same track+pad should always give same UUID
+    KIID trackUuid( "12345678-1234-1234-1234-123456789012" );
+    KIID padUuid( "abcdef01-abcd-abcd-abcd-abcdef012345" );
+
+    // First generation
+    KIID teardropUuid1 = KIID::Combine( trackUuid, padUuid );
+
+    // Simulate regeneration by computing again
+    KIID teardropUuid2 = KIID::Combine( trackUuid, padUuid );
+
+    // Should be identical
+    BOOST_CHECK_EQUAL( teardropUuid1.AsString(), teardropUuid2.AsString() );
+}
+
+
+BOOST_AUTO_TEST_SUITE_END()