Don't generate duplicate IDs in line/wire/bus tool.

Also cleans up existing duplicate IDs when reading.

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

common/kiid.cpp

@@ -38,21 +38,23 @@
 #include <wx/log.h>
 
 // boost:mt19937 is not thread-safe
-static std::mutex rng_mutex;
+static std::mutex                                           rng_mutex;
 
 // Create only once, as seeding is *very* expensive
-static boost::mt19937 rng;
+static boost::mt19937                                       rng;
 static boost::uuids::basic_random_generator<boost::mt19937> randomGenerator( rng );
 
-// These don't have the same performance penalty, but might as well be consistent
+// These don't have the same performance penalty, but we might as well be consistent
-static boost::uuids::string_generator stringGenerator;
+static boost::uuids::string_generator                       stringGenerator;
-static boost::uuids::nil_generator    nilGenerator;
+static boost::uuids::nil_generator                          nilGenerator;
+
 
 // Global nil reference
 KIID niluuid( 0 );
 
+
 // When true, always create nil uuids for performance, when valid ones aren't needed
-static bool createNilUuids = false;
+static bool g_createNilUuids = false;
 
 
 // For static initialization
@@ -72,7 +74,7 @@ KIID::KIID()
     {
 #endif
 
-        if( createNilUuids )
+        if( g_createNilUuids )
         {
             m_uuid = nilGenerator();
         }
@@ -247,9 +249,24 @@ void KIID::ConvertTimestampToUuid()
 }
 
 
+void KIID::Increment()
+{
+    // This obviously destroys uniform distribution, but it can be useful when a
+    // deterministic replacement for a duplicate ID is required.
+
+    for( int i = 15; i >= 0; --i )
+    {
+        m_uuid.data[i]++;
+
+        if( m_uuid.data[i] != 0 )
+            break;
+    }
+}
+
+
 void KIID::CreateNilUuids( bool aNil )
 {
-    createNilUuids = aNil;
+    g_createNilUuids = aNil;
 }
 
 

eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp

@@ -94,6 +94,19 @@ void SCH_SEXPR_PARSER::checkpoint()
 }
 
 
+KIID SCH_SEXPR_PARSER::parseKIID()
+{
+    KIID id( FromUTF8() );
+
+    while( m_uuids.count( id ) )
+        id.Increment();
+
+    m_uuids.insert( id );
+
+    return id;
+}
+
+
 bool SCH_SEXPR_PARSER::parseBool()
 {
     T token = NextTok();
@@ -1838,7 +1851,7 @@ SCH_SHEET_PIN* SCH_SEXPR_PARSER::parseSchSheetPin( SCH_SHEET* aSheet )
 
         case T_uuid:
             NeedSYMBOL();
-            const_cast<KIID&>( sheetPin->m_Uuid ) = KIID( FromUTF8() );
+            const_cast<KIID&>( sheetPin->m_Uuid ) = parseKIID();
             NeedRIGHT();
             break;
 
@@ -2050,7 +2063,7 @@ void SCH_SEXPR_PARSER::ParseSchematic( SCH_SHEET* aSheet, bool aIsCopyableOnly,
         {
         case T_uuid:
             NeedSYMBOL();
-            screen->m_uuid = KIID( FromUTF8() );
+            screen->m_uuid = parseKIID();
             NeedRIGHT();
             break;
 
@@ -2308,7 +2321,7 @@ SCH_SYMBOL* SCH_SEXPR_PARSER::parseSchematicSymbol()
 
         case T_uuid:
             NeedSYMBOL();
-            const_cast<KIID&>( symbol->m_Uuid ) = KIID( FromUTF8() );
+            const_cast<KIID&>( symbol->m_Uuid ) = parseKIID();
             NeedRIGHT();
             break;
 
@@ -2395,7 +2408,7 @@ SCH_SYMBOL* SCH_SEXPR_PARSER::parseSchematicSymbol()
                     // First version to write out pin uuids accidentally wrote out the symbol's
                     // uuid for each pin, so ignore uuids coming from that version.
                     if( m_requiredVersion >= 20210126 )
-                        uuid = KIID( FromUTF8() );
+                        uuid = parseKIID();
 
                     NeedRIGHT();
                     break;
@@ -2460,7 +2473,7 @@ SCH_BITMAP* SCH_SEXPR_PARSER::parseImage()
 
         case T_uuid:
             NeedSYMBOL();
-            const_cast<KIID&>( bitmap->m_Uuid ) = KIID( FromUTF8() );
+            const_cast<KIID&>( bitmap->m_Uuid ) = parseKIID();
             NeedRIGHT();
             break;
 
@@ -2560,7 +2573,7 @@ SCH_SHEET* SCH_SEXPR_PARSER::parseSheet()
 
         case T_uuid:
             NeedSYMBOL();
-            const_cast<KIID&>( sheet->m_Uuid ) = KIID( FromUTF8() );
+            const_cast<KIID&>( sheet->m_Uuid ) = parseKIID();
             NeedRIGHT();
             break;
 
@@ -2652,7 +2665,7 @@ SCH_JUNCTION* SCH_SEXPR_PARSER::parseJunction()
 
         case T_uuid:
             NeedSYMBOL();
-            const_cast<KIID&>( junction->m_Uuid ) = KIID( FromUTF8() );
+            const_cast<KIID&>( junction->m_Uuid ) = parseKIID();
             NeedRIGHT();
             break;
 
@@ -2689,7 +2702,7 @@ SCH_NO_CONNECT* SCH_SEXPR_PARSER::parseNoConnect()
 
         case T_uuid:
             NeedSYMBOL();
-            const_cast<KIID&>( no_connect->m_Uuid ) = KIID( FromUTF8() );
+            const_cast<KIID&>( no_connect->m_Uuid ) = parseKIID();
             NeedRIGHT();
             break;
 
@@ -2743,7 +2756,7 @@ SCH_BUS_WIRE_ENTRY* SCH_SEXPR_PARSER::parseBusEntry()
 
         case T_uuid:
             NeedSYMBOL();
-            const_cast<KIID&>( busEntry->m_Uuid ) = KIID( FromUTF8() );
+            const_cast<KIID&>( busEntry->m_Uuid ) = parseKIID();
             NeedRIGHT();
             break;
 
@@ -2808,7 +2821,7 @@ SCH_LINE* SCH_SEXPR_PARSER::parseLine()
 
         case T_uuid:
             NeedSYMBOL();
-            const_cast<KIID&>( line->m_Uuid ) = KIID( FromUTF8() );
+            const_cast<KIID&>( line->m_Uuid ) = parseKIID();
             NeedRIGHT();
             break;
 
@@ -2934,7 +2947,7 @@ SCH_TEXT* SCH_SEXPR_PARSER::parseSchText()
 
         case T_uuid:
             NeedSYMBOL();
-            const_cast<KIID&>( text->m_Uuid ) = KIID( FromUTF8() );
+            const_cast<KIID&>( text->m_Uuid ) = parseKIID();
             NeedRIGHT();
             break;
 

eeschema/sch_plugins/kicad/sch_sexpr_parser.h

@@ -81,6 +81,8 @@ class SCH_SEXPR_PARSER : public SCHEMATIC_LEXER
     int m_convert;          ///< The current body style being parsed.
     wxString m_symbolName;  ///< The current symbol name.
 
+    std::set<KIID>     m_uuids;
+
     PROGRESS_REPORTER* m_progressReporter;  // optional; may be nullptr
     const LINE_READER* m_lineReader;        // for progress reporting
     unsigned           m_lastProgressLine;
@@ -88,6 +90,8 @@ class SCH_SEXPR_PARSER : public SCHEMATIC_LEXER
 
     void checkpoint();
 
+    KIID parseKIID();
+
     void parseHeader( TSCHEMATIC_T::T aHeaderType, int aFileVersion );
 
     inline long parseHex()

eeschema/tools/sch_line_wire_bus_tool.cpp

@@ -651,7 +651,7 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const std::string& aTool, int aType,
                     segment->SetEndPoint( cursorPos );
 
                     // Create a new segment, and chain it after the current segment.
-                    segment = new SCH_LINE( *segment );
+                    segment = static_cast<SCH_LINE*>( segment->Duplicate() );
                     segment->SetFlags( IS_NEW | IS_MOVING );
                     segment->SetStartPoint( cursorPos );
                     m_wires.push_back( segment );
@@ -813,7 +813,7 @@ SCH_LINE* SCH_LINE_WIRE_BUS_TOOL::startSegments( int aType, const VECTOR2D& aPos
     // horizontal and vertical lines only switch is on.
     if( m_frame->eeconfig()->m_Drawing.hv_lines_only )
     {
-        segment = new SCH_LINE( *segment );
+        segment = static_cast<SCH_LINE*>( segment->Duplicate() );
         segment->SetFlags( IS_NEW | IS_MOVING );
         m_wires.push_back( segment );
 

include/kiid.h

@@ -59,8 +59,16 @@ public:
     wxString AsString() const;
     wxString AsLegacyTimestampString() const;
 
+    /**
+     * Returns true if a string has the correct formatting to be a KIID.
+     */
     static bool SniffTest( const wxString& aCandidate );
 
+    /**
+     * A performance optimization which disables/enables the generation of pseudo-random UUIDs.
+     *
+     * NB: uses a global.  Not thread safe!
+     */
     static void CreateNilUuids( bool aNil = true );
 
     /**
@@ -81,6 +89,14 @@ public:
      */
     void ConvertTimestampToUuid();
 
+    /**
+     * Generates a deterministic replacement for a given ID.
+     *
+     * NB: destroys uniform distribution!  But it's the only thing we have when a deterministic
+     * replacement for a duplicate ID is required.
+     */
+    void Increment();
+
     bool operator==( KIID const& rhs ) const
     {
         return m_uuid == rhs.m_uuid;