Properly set parent when changing operations list at once

CURA-12250
Ultimaker · Dec 10, 2024 · e53e941 · e53e941
1 parent 6a74ae2
commit e53e941
Show file tree

Hide file tree

Showing 11 changed files with 123 additions and 71 deletions.
diff --git a/include/operation_transformation/DirectTravelMoveGenerator.h b/include/operation_transformation/DirectTravelMoveGenerator.h
@@ -1,8 +1,7 @@
 // Copyright (c) 2024 UltiMaker
 // CuraEngine is released under the terms of the AGPLv3 or higher
 
-#ifndef PATHPROCESSING_DIRECTTRAVELMOVEGENERATOR_H
-#define PATHPROCESSING_DIRECTTRAVELMOVEGENERATOR_H
+#pragma once
 
 #include "operation_transformation/TravelMoveGenerator.h"
 
@@ -16,5 +15,3 @@ class DirectTravelMoveGenerator : public TravelMoveGenerator
 };
 
 } // namespace cura
-
-#endif // PATHPROCESSING_DIRECTTRAVELMOVEGENERATOR_H
diff --git a/include/operation_transformation/LayerPlanTravelMovesInserter.h b/include/operation_transformation/LayerPlanTravelMovesInserter.h
@@ -27,7 +27,7 @@ class LayerPlanTravelMovesInserter : public PrintOperationTransformer<LayerPlan>
 
     void appendTravelMovesRecursively(const std::shared_ptr<PrintOperation>& operation, const SpeedDerivatives& speed);
 
-    const std::shared_ptr<PrintOperation> makeTravelMove(const Point3LL& start_position, const Point3LL& end_position, const SpeedDerivatives& speed);
+    const std::shared_ptr<PrintOperation> makeTravelRoute(const Point3LL& start_position, const Point3LL& end_position, const SpeedDerivatives& speed);
 
 private:
     std::vector<std::shared_ptr<TravelMoveGenerator>> generators_;

diff --git a/include/print_operation/ContinuousExtruderMoveSequence.h b/include/print_operation/ContinuousExtruderMoveSequence.h
@@ -3,12 +3,8 @@
 
 #pragma once
 
-#include "GCodePathConfig.h"
-#include "SpaceFillType.h"
 #include "geometry/Point3LL.h"
 #include "print_operation/PrintOperationSequence.h"
-#include "settings/types/Ratio.h"
-#include "utils/Coord_t.h"
 
 namespace cura
 {

diff --git a/include/print_operation/PrintOperationSequence.h b/include/print_operation/PrintOperationSequence.h
@@ -1,12 +1,14 @@
 // Copyright (c) 2024 UltiMaker
 // CuraEngine is released under the terms of the AGPLv3 or higher
 
-#ifndef PATHPLANNING_PRINTOPERATIONSEQUENCE_H
-#define PATHPLANNING_PRINTOPERATIONSEQUENCE_H
+#pragma once
+
+#include <range/v3/algorithm/contains.hpp>
 
 #include "geometry/Point3LL.h"
 #include "operation_transformation/PrintOperationTransformer.h"
 #include "print_operation/PrintOperation.h"
+#include "print_operation/PrintOperationPtr.h"
 
 namespace cura
 {
@@ -56,41 +58,42 @@ class PrintOperationSequence : public PrintOperation, public std::enable_shared_
      * @return The first found operation, or a null ptr if none was found
      * @note This function can also be used to iterate over children by providing a search function that always returns false
      */
-    std::shared_ptr<PrintOperation> findOperation(
-        const std::function<bool(const std::shared_ptr<PrintOperation>&)>& search_function,
+    PrintOperationPtr findOperation(
+        const std::function<bool(const PrintOperationPtr&)>& search_function,
         const SearchOrder search_order = SearchOrder::Forward,
         const std::optional<size_t> max_depth = SearchDepth::DirectChildren) const;
 
     template<class OperationType>
     std::shared_ptr<OperationType>
         findOperationByType(const SearchOrder search_order = SearchOrder::Forward, const std::optional<size_t> max_depth = SearchDepth::DirectChildren) const;
 
-    const std::vector<std::shared_ptr<PrintOperation>>& getOperations() const noexcept;
-
-    std::vector<std::shared_ptr<PrintOperation>>& getOperations() noexcept;
+    const std::vector<PrintOperationPtr>& getOperations() const noexcept;
 
     template<class OperationType>
     std::vector<std::shared_ptr<OperationType>> getOperationsAs() noexcept;
 
-    void setOperations(std::vector<std::shared_ptr<PrintOperation>>& operations) noexcept;
+    // void setOperations(std::vector<PrintOperationPtr>& operations) noexcept;
+
+    template<class OperationType>
+    void setOperations(std::vector<std::shared_ptr<OperationType>>& operations) noexcept;
 
 protected:
-    void appendOperation(const std::shared_ptr<PrintOperation>& operation);
+    void appendOperation(const PrintOperationPtr& operation);
 
-    void removeOperation(const std::shared_ptr<PrintOperation>& operation);
+    void removeOperation(const PrintOperationPtr& operation);
 
     template<class ChildType>
     void applyProcessorToOperationsRecursively(PrintOperationTransformer<ChildType>& processor);
 
 private:
-    std::vector<std::shared_ptr<PrintOperation>> operations_;
+    std::vector<PrintOperationPtr> operations_;
 };
 
 template<class OperationType>
 std::shared_ptr<OperationType> PrintOperationSequence::findOperationByType(const SearchOrder search_order, const std::optional<size_t> max_depth) const
 {
-    std::shared_ptr<PrintOperation> found_operation = findOperation(
-        [](const std::shared_ptr<PrintOperation>& operation)
+    PrintOperationPtr found_operation = findOperation(
+        [](const PrintOperationPtr& operation)
         {
             return static_cast<bool>(std::dynamic_pointer_cast<OperationType>(operation));
         },
@@ -126,6 +129,32 @@ std::vector<std::shared_ptr<OperationType>> PrintOperationSequence::getOperation
     return result;
 }
 
+template<class OperationType>
+void PrintOperationSequence::setOperations(std::vector<std::shared_ptr<OperationType>>& operations) noexcept
+{
+    for (const PrintOperationPtr& removed_operation : operations_)
+    {
+        if (! ranges::contains(operations, removed_operation))
+        {
+            removed_operation->setParent({});
+        }
+    }
+
+    for (const PrintOperationPtr& added_operation : operations)
+    {
+        if (! ranges::contains(operations_, added_operation))
+        {
+            added_operation->setParent(weak_from_this());
+        }
+    }
+
+    operations_.resize(operations.size());
+    for (size_t index = 0; index < operations.size(); ++index)
+    {
+        operations_[index] = operations[index];
+    }
+}
+
 template<class ChildType>
 void PrintOperationSequence::applyProcessorToOperationsRecursively(PrintOperationTransformer<ChildType>& processor)
 {
@@ -144,5 +173,3 @@ void PrintOperationSequence::applyProcessorToOperationsRecursively(PrintOperatio
 }
 
 } // namespace cura
-
-#endif // PATHPLANNING_PRINTOPERATIONSEQUENCE_H
diff --git a/include/print_operation/TravelRoute.h b/include/print_operation/TravelRoute.h
@@ -4,12 +4,15 @@
 #ifndef PATHPLANNING_TRAVELROUTE_H
 #define PATHPLANNING_TRAVELROUTE_H
 
+#include "path_planning/SpeedDerivatives.h"
 #include "print_operation/ContinuousExtruderMoveSequence.h"
 
 namespace cura
 {
 
+enum class PrintFeatureType : unsigned char;
 class TravelMove;
+struct Velocity;
 
 class TravelRoute : public ContinuousExtruderMoveSequence
 {

diff --git a/src/operation_transformation/DirectTravelMoveGenerator.cpp b/src/operation_transformation/DirectTravelMoveGenerator.cpp
@@ -3,6 +3,7 @@
 
 #include "operation_transformation/DirectTravelMoveGenerator.h"
 
+#include "PrintFeatureType.h"
 #include "print_operation/TravelMove.h"
 #include "print_operation/TravelRoute.h"
 

diff --git a/src/operation_transformation/InsertOperationsProcessor.cpp b/src/operation_transformation/InsertOperationsProcessor.cpp
@@ -10,7 +10,7 @@ namespace cura
 
 void InsertOperationsProcessor::process(PrintOperationSequence* operation)
 {
-    std::vector<std::shared_ptr<PrintOperation>>& child_operations = operation->getOperations();
+    std::vector<std::shared_ptr<PrintOperation>> child_operations = operation->getOperations();
     for (size_t index_first = 0; index_first < child_operations.size(); ++index_first)
     {
         const std::shared_ptr<PrintOperation>& operation_first = child_operations[index_first];
@@ -35,6 +35,8 @@ void InsertOperationsProcessor::process(PrintOperationSequence* operation)
             }
         }
     }
+
+    operation->setOperations(child_operations);
 }
 
 } // namespace cura
diff --git a/src/operation_transformation/LayerPlanTravelMovesInserter.cpp b/src/operation_transformation/LayerPlanTravelMovesInserter.cpp
@@ -31,33 +31,39 @@ void LayerPlanTravelMovesInserter::process(LayerPlan* layer_plan)
 
 void LayerPlanTravelMovesInserter::appendTravelsMovesBetweenChildren(const std::shared_ptr<PrintOperationSequence>& sequence, const SpeedDerivatives& speed)
 {
-    std::vector<std::shared_ptr<PrintOperation>>& child_operations = sequence->getOperations();
-    for (size_t index_first = 0; index_first < child_operations.size(); ++index_first)
+    std::vector<std::shared_ptr<PrintOperation>> child_operations = sequence->getOperations();
+    for (size_t index_first = 0; index_first < child_operations.size() - 1; ++index_first)
     {
         const std::shared_ptr<PrintOperation>& operation_first = child_operations[index_first];
         std::optional<Point3LL> first_end_position = operation_first->findEndPosition();
-        if (first_end_position.has_value())
+        if (! first_end_position.has_value())
         {
-            for (size_t index_second = index_first + 1; index_second < child_operations.size(); ++index_second)
+            continue;
+        }
+
+        for (size_t index_second = index_first + 1; index_second < child_operations.size(); ++index_second)
+        {
+            const std::shared_ptr<PrintOperation>& operation_second = child_operations[index_second];
+            std::optional<Point3LL> second_start_position = operation_second->findStartPosition();
+            if (! second_start_position.has_value())
             {
-                const std::shared_ptr<PrintOperation>& operation_second = child_operations[index_second];
-                std::optional<Point3LL> second_start_position = operation_second->findStartPosition();
-                if (second_start_position.has_value())
-                {
-                    if (const std::shared_ptr<PrintOperation> travel_move = makeTravelMove(first_end_position.value(), second_start_position.value(), speed))
-                    {
-                        child_operations.insert(std::next(child_operations.begin(), index_second), travel_move);
-                        index_first = index_second;
-                    }
-                    else
-                    {
-                        index_first = index_second - 1;
-                    }
-                    break;
-                }
+                continue;
             }
+
+            if (const std::shared_ptr<PrintOperation> travel_move = makeTravelRoute(first_end_position.value(), second_start_position.value(), speed))
+            {
+                child_operations.insert(std::next(child_operations.begin(), index_second), travel_move);
+                index_first = index_second;
+            }
+            else
+            {
+                index_first = index_second - 1;
+            }
+            break;
         }
     }
+
+    sequence->setOperations(child_operations);
 }
 
 void LayerPlanTravelMovesInserter::appendTravelMovesRecursively(const std::shared_ptr<PrintOperation>& operation, const SpeedDerivatives& speed)
@@ -73,7 +79,7 @@ void LayerPlanTravelMovesInserter::appendTravelMovesRecursively(const std::share
     }
 }
 
-const std::shared_ptr<PrintOperation> LayerPlanTravelMovesInserter::makeTravelMove(const Point3LL& start_position, const Point3LL& end_position, const SpeedDerivatives& speed)
+const std::shared_ptr<PrintOperation> LayerPlanTravelMovesInserter::makeTravelRoute(const Point3LL& start_position, const Point3LL& end_position, const SpeedDerivatives& speed)
 {
     if (end_position != start_position)
     {

diff --git a/src/print_operation/ContinuousExtruderMoveSequence.cpp b/src/print_operation/ContinuousExtruderMoveSequence.cpp
@@ -28,7 +28,7 @@ void ContinuousExtruderMoveSequence::reorderToEndWith(const std::shared_ptr<Extr
 {
     if (closed_)
     {
-        std::vector<std::shared_ptr<PrintOperation>>& operations = getOperations();
+        std::vector<std::shared_ptr<PrintOperation>> operations = getOperations();
         if (operations.size() > 1)
         {
             auto iterator = std::find(operations.begin(), operations.end(), extruder_move);
@@ -44,6 +44,8 @@ void ContinuousExtruderMoveSequence::reorderToEndWith(const std::shared_ptr<Extr
             {
                 spdlog::warn("Unable to change sequence ordering because the given move is not part of the sequence");
             }
+
+            setOperations(operations);
         }
     }
     else
@@ -80,7 +82,8 @@ void ContinuousExtruderMoveSequence::reverse()
             }
         }
 
-        std::reverse(getOperations().begin(), getOperations().end());
+        std::reverse(operations.begin(), operations.end());
+        setOperations(operations);
     }
 }
 

diff --git a/src/print_operation/PrintOperation.cpp b/src/print_operation/PrintOperation.cpp
@@ -46,7 +46,8 @@ std::shared_ptr<PrintOperationSequence>
 
         return parent_ptr->findParent(search_function, warn_not_found);
     }
-    else if (warn_not_found)
+
+    if (warn_not_found)
     {
         spdlog::warn("Couldn't find appropriate parent");
     }