Construction geometry: lighter on the stack

Pass things as unqiue_ptrs. I don't think there's that much
on the stack, but certainly addinig big enough chunks to the batches can
upset the coroutines.

Also reinstate a ClearDrawables which went AWOL.pick

Relates-To: https://gitlab.com/kicad/code/kicad/-/issues/18835

common/tool/construction_manager.cpp

@@ -178,9 +178,9 @@ CONSTRUCTION_MANAGER::CONSTRUCTION_MANAGER( CONSTRUCTION_VIEW_HANDLER& aHelper )
     const std::chrono::milliseconds acceptanceTimeout(
             ADVANCED_CFG::GetCfg().m_ExtensionSnapTimeoutMs );
 
-    m_activationHelper = std::make_unique<ACTIVATION_HELPER<PENDING_BATCH>>(
+    m_activationHelper = std::make_unique<ACTIVATION_HELPER<std::unique_ptr<PENDING_BATCH>>>(
             acceptanceTimeout,
-            [this]( PENDING_BATCH&& aAccepted )
+            [this]( std::unique_ptr<PENDING_BATCH>&& aAccepted )
             {
                 acceptConstructionItems( std::move( aAccepted ) );
             } );
@@ -209,24 +209,20 @@ HashConstructionBatchSources( const CONSTRUCTION_MANAGER::CONSTRUCTION_ITEM_BATC
 }
 
 
-void CONSTRUCTION_MANAGER::ProposeConstructionItems( CONSTRUCTION_ITEM_BATCH aBatch,
-                                                     bool                    aIsPersistent )
+void CONSTRUCTION_MANAGER::ProposeConstructionItems(
+        std::unique_ptr<CONSTRUCTION_ITEM_BATCH> aBatch, bool aIsPersistent )
 {
-    if( aBatch.empty() )
+    if( aBatch->empty() )
     {
         // There's no point in proposing an empty batch
         // It would just clear existing construction items for nothing new
         return;
     }
 
-    const std::size_t hash = HashConstructionBatchSources( aBatch, aIsPersistent );
+    const std::size_t hash = HashConstructionBatchSources( *aBatch, aIsPersistent );
 
     m_activationHelper->ProposeActivation(
-            PENDING_BATCH{
-                    std::move( aBatch ),
-                    aIsPersistent,
-            },
-            hash );
+            std::make_unique<PENDING_BATCH>( std::move( *aBatch ), aIsPersistent ), hash );
 }
 
 
@@ -236,7 +232,7 @@ void CONSTRUCTION_MANAGER::CancelProposal()
 }
 
 
-void CONSTRUCTION_MANAGER::acceptConstructionItems( PENDING_BATCH&& aAcceptedBatch )
+void CONSTRUCTION_MANAGER::acceptConstructionItems( std::unique_ptr<PENDING_BATCH> aAcceptedBatch )
 {
     const auto getInvolved = [&]( const CONSTRUCTION_ITEM_BATCH& aBatchToAdd )
     {
@@ -256,15 +252,15 @@ void CONSTRUCTION_MANAGER::acceptConstructionItems( PENDING_BATCH&& aAcceptedBat
     {
         std::lock_guard<std::mutex> lock( m_batchesMutex );
 
-        if( aAcceptedBatch.IsPersistent )
+        if( aAcceptedBatch->IsPersistent )
         {
             // We only keep one previous persistent batch for the moment
-            m_persistentConstructionBatch = std::move( aAcceptedBatch.Batch );
+            m_persistentConstructionBatch = std::move( aAcceptedBatch->Batch );
         }
         else
         {
             bool anyNewItems = false;
-            for( CONSTRUCTION_ITEM& item : aAcceptedBatch.Batch )
+            for( CONSTRUCTION_ITEM& item : aAcceptedBatch->Batch )
             {
                 if( m_involvedItems.count( item.Item ) == 0 )
                 {
@@ -288,7 +284,7 @@ void CONSTRUCTION_MANAGER::acceptConstructionItems( PENDING_BATCH&& aAcceptedBat
                 m_temporaryConstructionBatches.pop_front();
             }
 
-            m_temporaryConstructionBatches.emplace_back( std::move( aAcceptedBatch.Batch ) );
+            m_temporaryConstructionBatches.emplace_back( std::move( aAcceptedBatch->Batch ) );
         }
 
         m_involvedItems.clear();
@@ -308,6 +304,7 @@ void CONSTRUCTION_MANAGER::acceptConstructionItems( PENDING_BATCH&& aAcceptedBat
     }
 
     KIGFX::CONSTRUCTION_GEOM& geom = m_viewHandler.GetViewItem();
+    geom.ClearDrawables();
 
     const auto addDrawables =
             [&]( const std::vector<CONSTRUCTION_ITEM_BATCH>& aBatches, bool aIsPersistent )

include/tool/construction_manager.h

@@ -25,6 +25,7 @@
 #pragma once
 
 #include <deque>
+#include <memory>
 #include <mutex>
 #include <vector>
 
@@ -170,7 +171,8 @@ public:
      *                      (and it will replace any previous persistent batch).
      *                      If false, the batch is temporary and may be pushed out by other batches.
      */
-    void ProposeConstructionItems( CONSTRUCTION_ITEM_BATCH aBatch, bool aIsPersistent );
+    void ProposeConstructionItems( std::unique_ptr<CONSTRUCTION_ITEM_BATCH> aBatch,
+                                   bool                                     aIsPersistent );
 
     /**
      * Cancel outstanding proposals for new geometry.
@@ -191,7 +193,7 @@ public:
 private:
     struct PENDING_BATCH;
 
-    void acceptConstructionItems( PENDING_BATCH&& aAcceptedBatchHash );
+    void acceptConstructionItems( std::unique_ptr<PENDING_BATCH> aAcceptedBatchHash );
 
     CONSTRUCTION_VIEW_HANDLER& m_viewHandler;
 
@@ -206,7 +208,7 @@ private:
     // Set of all items for which construction geometry has been added
     std::set<EDA_ITEM*> m_involvedItems;
 
-    std::unique_ptr<ACTIVATION_HELPER<PENDING_BATCH>> m_activationHelper;
+    std::unique_ptr<ACTIVATION_HELPER<std::unique_ptr<PENDING_BATCH>>> m_activationHelper;
 
     // Protects the persistent and temporary construction batches
     mutable std::mutex m_batchesMutex;

pcbnew/tools/pcb_grid_helper.cpp

@@ -184,7 +184,7 @@ void PCB_GRID_HELPER::AddConstructionItems( std::vector<BOARD_ITEM*> aItems, boo
     // For all the elements that get drawn construction geometry,
     // add something suitable to the construction helper.
     // This can be nothing.
-    CONSTRUCTION_MANAGER::CONSTRUCTION_ITEM_BATCH constructionItemsBatch;
+    auto constructionItemsBatch = std::make_unique<CONSTRUCTION_MANAGER::CONSTRUCTION_ITEM_BATCH>();
 
     std::vector<VECTOR2I> referenceOnlyPoints;
 
@@ -273,7 +273,7 @@ void PCB_GRID_HELPER::AddConstructionItems( std::vector<BOARD_ITEM*> aItems, boo
         // At this point, constructionDrawables can be empty, which is fine
         // (it means there's no additional construction geometry to draw, but
         // the item is still going to be proposed for activation)
-        constructionItemsBatch.emplace_back( CONSTRUCTION_MANAGER::CONSTRUCTION_ITEM{
+        constructionItemsBatch->emplace_back( CONSTRUCTION_MANAGER::CONSTRUCTION_ITEM{
                 CONSTRUCTION_MANAGER::SOURCE::FROM_ITEMS,
                 item,
                 std::move( constructionDrawables ),