Modernized the code

Improved the readability of the code by using modern C++ practices such as list initialization, return statement with braces, use of `std::move` and `[[maybe_unused]]`, auto-lambda type deduction, and range-based for loop. Removed unnecessary if-else blocks and simplified the syntax. Contribute to CURA-9830
Ultimaker · May 3, 2024 · 96e4b5e · 96e4b5e
1 parent 4718d3d
commit 96e4b5e
Show file tree

Hide file tree

Showing 10 changed files with 220 additions and 258 deletions.
diff --git a/include/geometry/LinesSet.h b/include/geometry/LinesSet.h
@@ -37,11 +37,14 @@ class LinesSet
 
 public:
     // Required for some std calls as a container
-    typedef LineType value_type;
+    using value_type = LineType;
+    using iterator = typename std::vector<LineType>::iterator;
+    using const_iterator = typename std::vector<LineType>::const_iterator;
 
-public:
     /*! \brief Builds an empty set */
-    LinesSet() = default;
+    LinesSet() noexcept = default;
+
+    virtual ~LinesSet() = default;
 
     /*! \brief Creates a copy of the given lines set */
     LinesSet(const LinesSet& other) = default;
@@ -66,8 +69,16 @@ class LinesSet
      * \warning This constructor is actually only defined for a LinesSet containing OpenPolyline
      *          objects, because closed ones require an additional argument
      */
-    template<typename U = LineType, typename = typename std::enable_if<std::is_same<U, OpenPolyline>::value>::type>
-    LinesSet(ClipperLib::Paths&& paths);
+    template<typename U = LineType>
+        requires std::is_same_v<U, OpenPolyline>
+    LinesSet(ClipperLib::Paths&& paths)
+    {
+        reserve(paths.size());
+        for (ClipperLib::Path& path : paths)
+        {
+            lines_.emplace_back(std::move(path));
+        }
+    }
 
     const std::vector<LineType>& getLines() const
     {
@@ -84,22 +95,22 @@ class LinesSet
         lines_ = lines;
     }
 
-    std::vector<LineType>::const_iterator begin() const
+    const_iterator begin() const
     {
         return lines_.begin();
     }
 
-    std::vector<LineType>::iterator begin()
+    iterator begin()
     {
         return lines_.begin();
     }
 
-    std::vector<LineType>::const_iterator end() const
+    const_iterator end() const
     {
         return lines_.end();
     }
 
-    std::vector<LineType>::iterator end()
+    iterator end()
     {
         return lines_.end();
     }
@@ -126,15 +137,15 @@ class LinesSet
 
     /*!
      * \brief Pushes the given line at the end of the set
-     * \param checkNonEmpty Indicates whether we should check for the line to be non-empty before adding it
+     * \param check_non_empty Indicates whether we should check for the line to be non-empty before adding it
      */
-    void push_back(const LineType& line, CheckNonEmptyParam checkNonEmpty = CheckNonEmptyParam::EvenIfEmpty);
+    void push_back(const LineType& line, CheckNonEmptyParam check_non_empty = CheckNonEmptyParam::EvenIfEmpty);
 
     /*!
      * \brief Pushes the given line at the end of the set and takes ownership of the inner data
-     * \param checkNonEmpty Indicates whether we should check for the line to be non-empty before adding it
+     * \param check_non_empty Indicates whether we should check for the line to be non-empty before adding it
      */
-    void push_back(LineType&& line, CheckNonEmptyParam checkNonEmpty = CheckNonEmptyParam::EvenIfEmpty);
+    void push_back(LineType&& line, CheckNonEmptyParam check_non_empty = CheckNonEmptyParam::EvenIfEmpty);
 
     /*! \brief Pushes an entier set at the end and takes ownership of the inner data */
     template<class OtherLineType>
@@ -151,12 +162,12 @@ class LinesSet
         lines_.pop_back();
     }
 
-    size_t size() const
+    [[nodiscard]] size_t size() const
     {
         return lines_.size();
     }
 
-    bool empty() const
+    [[nodiscard]] bool empty() const
     {
         return lines_.empty();
     }
@@ -176,13 +187,12 @@ class LinesSet
         lines_.clear();
     }
 
-    template<typename... Args>
-    void emplace_back(Args&&... args)
+    void emplace_back(auto&&... args)
     {
-        lines_.emplace_back(args...);
+        lines_.emplace_back(std::forward<decltype(args)>(args)...);
     }
 
-    std::vector<LineType>::iterator erase(std::vector<LineType>::const_iterator first, std::vector<LineType>::const_iterator last)
+    iterator erase(const_iterator first, const_iterator last)
     {
         return lines_.erase(first, last);
     }
@@ -193,7 +203,7 @@ class LinesSet
         return *this;
     }
 
-    LinesSet& operator=(LinesSet&& other)
+    LinesSet& operator=(LinesSet&& other) noexcept
     {
         lines_ = std::move(other.lines_);
         return *this;

diff --git a/include/geometry/Shape.h b/include/geometry/Shape.h
@@ -30,7 +30,6 @@ class Shape : public LinesSet<Polygon>
     // Clipper expects and returns implicitely closed polygons
     static constexpr bool clipper_explicitely_closed_ = false;
 
-public:
     /*! \brief Constructor of an empty shape */
     Shape() = default;
 
@@ -49,38 +48,38 @@ class Shape : public LinesSet<Polygon>
      */
     explicit Shape(ClipperLib::Paths&& paths, bool explicitely_closed = clipper_explicitely_closed_);
 
-    Shape& operator=(const Shape& other);
+    Shape& operator=(const Shape& other) = default;
+
+    Shape& operator=(Shape&& other) noexcept = default;
 
-    Shape& operator=(Shape&& other);
+    ~Shape() override = default;
 
     void emplace_back(ClipperLib::Paths&& paths, bool explicitely_closed = clipper_explicitely_closed_);
 
     void emplace_back(ClipperLib::Path&& path, bool explicitely_closed = clipper_explicitely_closed_);
 
-    template<typename... Args>
-    void emplace_back(Args&&... args)
+    void emplace_back(auto&&... args)
     {
-        LinesSet::emplace_back(args...);
+        LinesSet<Polygon>::emplace_back(std::forward<decltype(args)>(args)...);
     }
 
+    [[nodiscard]] Shape difference(const Shape& other) const;
 
-    Shape difference(const Shape& other) const;
-
-    Shape unionPolygons(const Shape& other, ClipperLib::PolyFillType fill_type = ClipperLib::pftNonZero) const;
+    [[nodiscard]] Shape unionPolygons(const Shape& other, ClipperLib::PolyFillType fill_type = ClipperLib::pftNonZero) const;
 
     /*!
      * Union all polygons with each other (When polygons.add(polygon) has been called for overlapping polygons)
      */
-    Shape unionPolygons() const;
+    [[nodiscard]] Shape unionPolygons() const;
 
-    Shape intersection(const Shape& other) const;
+    [[nodiscard]] Shape intersection(const Shape& other) const;
 
     /*!
      *  @brief Overridden definition of LinesSet<Polygon>::offset()
      *  @note The behavior of this method is exactly the same, but it just exists because it allows
      *        for a performance optimization
      */
-    Shape offset(coord_t distance, ClipperLib::JoinType join_type = ClipperLib::jtMiter, double miter_limit = 1.2) const;
+    [[nodiscard]] Shape offset(coord_t distance, ClipperLib::JoinType join_type = ClipperLib::jtMiter, double miter_limit = 1.2) const;
 
     /*!
      * Intersect polylines with the area covered by the shape.
@@ -96,9 +95,9 @@ class Shape : public LinesSet<Polygon>
     template<class LineType>
     OpenLinesSet intersection(const LinesSet<LineType>& polylines, bool restitch = true, const coord_t max_stitch_distance = 10_mu) const;
 
-    Shape xorPolygons(const Shape& other, ClipperLib::PolyFillType pft = ClipperLib::pftEvenOdd) const;
+    [[nodiscard]] Shape xorPolygons(const Shape& other, ClipperLib::PolyFillType pft = ClipperLib::pftEvenOdd) const;
 
-    Shape execute(ClipperLib::PolyFillType pft = ClipperLib::pftEvenOdd) const;
+    [[nodiscard]] Shape execute(ClipperLib::PolyFillType pft = ClipperLib::pftEvenOdd) const;
 
     /*!
      * Check if we are inside the polygon.
@@ -112,7 +111,7 @@ class Shape : public LinesSet<Polygon>
      * \param border_result What to return when the point is exactly on the border
      * \return Whether the point \p p is inside this polygon (or \p border_result when it is on the border)
      */
-    bool inside(const Point2LL& p, bool border_result = false) const;
+    [[nodiscard]] bool inside(const Point2LL& p, bool border_result = false) const;
 
     /*!
      * Find the polygon inside which point \p p resides.
@@ -127,15 +126,15 @@ class Shape : public LinesSet<Polygon>
      * \param border_result Whether a point exactly on a polygon counts as inside
      * \return The index of the polygon inside which the point \p p resides
      */
-    size_t findInside(const Point2LL& p, bool border_result = false) const;
+    [[nodiscard]] size_t findInside(const Point2LL& p, bool border_result = false) const;
 
     /*!
      * \brief Approximates the convex hull of the polygons.
      * \p extra_outset Extra offset outward
      * \return the convex hull (approximately)
      *
      */
-    Shape approxConvexHull(int extra_outset = 0) const;
+    [[nodiscard]] Shape approxConvexHull(int extra_outset = 0) const;
 
     /*! \brief Make each of the polygons convex */
     void makeConvex();
@@ -145,7 +144,7 @@ class Shape : public LinesSet<Polygon>
      *
      * \return The area in square micron
      */
-    double area() const;
+    [[nodiscard]] double area() const;
 
     /*!
      * Smooth out small perpendicular segments
@@ -158,7 +157,7 @@ class Shape : public LinesSet<Polygon>
      * \param remove_length The length of the largest segment removed
      * \return The smoothed polygon
      */
-    Shape smooth(int remove_length) const;
+    [[nodiscard]] Shape smooth(int remove_length) const;
 
     /*!
      * Smooth out sharp inner corners, by taking a shortcut which bypasses the corner
@@ -167,9 +166,9 @@ class Shape : public LinesSet<Polygon>
      * \param shortcut_length The desired length of the shortcut line segment introduced (shorter shortcuts may be unavoidable)
      * \return The resulting polygons
      */
-    Shape smooth_outward(const AngleDegrees angle, int shortcut_length) const;
+    [[nodiscard]] Shape smoothOutward(const AngleDegrees angle, int shortcut_length) const;
 
-    Shape smooth2(int remove_length, int min_area) const; //!< removes points connected to small lines
+    [[nodiscard]] Shape smooth2(int remove_length, int min_area) const; //!< removes points connected to small lines
 
     void removeColinearEdges(const AngleRadians max_deviation_angle = AngleRadians(0.0005));
 
@@ -178,28 +177,28 @@ class Shape : public LinesSet<Polygon>
      * Exclude holes and parts within holes.
      * \return the resulting polygons.
      */
-    Shape getOutsidePolygons() const;
+    [[nodiscard]] Shape getOutsidePolygons() const;
 
     /*!
      * Split up the polygons into groups according to the even-odd rule.
      * Each SingleShape in the result has an outline as first polygon, whereas the rest are holes.
      */
-    std::vector<SingleShape> splitIntoParts(bool unionAll = false) const;
+    [[nodiscard]] std::vector<SingleShape> splitIntoParts(bool union_all = false) const;
 
     /*!
      * Sort the polygons into bins where each bin has polygons which are contained within one of the polygons in the previous bin.
      *
      * \warning When polygons are crossing each other the result is undefined.
      */
-    std::vector<Shape> sortByNesting() const;
+    [[nodiscard]] std::vector<Shape> sortByNesting() const;
 
     /*!
      * Split up the polygons into groups according to the even-odd rule.
      * Each vector in the result has the index to an outline as first index, whereas the rest are indices to holes.
      *
      * \warning Note that this function reorders the polygons!
      */
-    PartsView splitIntoPartsView(bool unionAll = false);
+    PartsView splitIntoPartsView(bool union_all = false);
 
     /*!
      * Removes polygons with area smaller than \p min_area_size (note that min_area_size is in mm^2, not in micron^2).
@@ -212,9 +211,9 @@ class Shape : public LinesSet<Polygon>
      * Removes the same polygons from this set (and also empty polygons).
      * Shape are considered the same if all points lie within [same_distance] of their counterparts.
      */
-    Shape removePolygon(const Shape& to_be_removed, int same_distance = 0) const;
+    [[nodiscard]] Shape removePolygon(const Shape& to_be_removed, int same_distance = 0) const;
 
-    Shape processEvenOdd(ClipperLib::PolyFillType poly_fill_type = ClipperLib::PolyFillType::pftEvenOdd) const;
+    [[nodiscard]] Shape processEvenOdd(ClipperLib::PolyFillType poly_fill_type = ClipperLib::PolyFillType::pftEvenOdd) const;
 
     /*!
      * Ensure the polygon is manifold, by removing small areas where the polygon touches itself.
@@ -231,7 +230,7 @@ class Shape : public LinesSet<Polygon>
 
     void applyMatrix(const Point3Matrix& matrix);
 
-    Shape offsetMulti(const std::vector<coord_t>& offset_dists) const;
+    [[nodiscard]] Shape offsetMulti(const std::vector<coord_t>& offset_dists) const;
 
     /*!
      * @brief Remove self-intersections from the polygons
@@ -242,7 +241,7 @@ class Shape : public LinesSet<Polygon>
      *
      * @return Polygons - the cleaned polygons
      */
-    Shape removeNearSelfIntersections() const;
+    [[nodiscard]] Shape removeNearSelfIntersections() const;
 
     /*!
      * \brief Simplify the polygon lines using ClipperLib::SimplifyPolygons
@@ -271,10 +270,10 @@ class Shape : public LinesSet<Polygon>
      * \param remove_holes Whether to remove empty holes or everything but the empty holes
      * \param ret Where to store polygons which are not empty holes
      */
-    void removeEmptyHoles_processPolyTreeNode(const ClipperLib::PolyNode& node, const bool remove_holes, Shape& ret) const;
-    void splitIntoParts_processPolyTreeNode(ClipperLib::PolyNode* node, std::vector<SingleShape>& ret) const;
-    void sortByNesting_processPolyTreeNode(ClipperLib::PolyNode* node, const size_t nesting_idx, std::vector<Shape>& ret) const;
-    void splitIntoPartsView_processPolyTreeNode(PartsView& partsView, Shape& reordered, ClipperLib::PolyNode* node) const;
+    void removeEmptyHolesProcessPolyTreeNode(const ClipperLib::PolyNode& node, const bool remove_holes, Shape& ret) const;
+    void splitIntoPartsProcessPolyTreeNode(ClipperLib::PolyNode* node, std::vector<SingleShape>& ret) const;
+    void sortByNestingProcessPolyTreeNode(ClipperLib::PolyNode* node, const size_t nesting_idx, std::vector<Shape>& ret) const;
+    void splitIntoPartsViewProcessPolyTreeNode(PartsView& parts_view, Shape& reordered, ClipperLib::PolyNode* node) const;
 };
 
 } // namespace cura

diff --git a/include/geometry/SingleShape.h b/include/geometry/SingleShape.h
@@ -21,6 +21,10 @@ class Polygon;
 class SingleShape : public Shape
 {
 public:
+    SingleShape() = default;
+
+    explicit SingleShape(Shape&& shape) : Shape{ shape } {};
+
     Polygon& outerPolygon();
 
     const Polygon& outerPolygon() const;

diff --git a/include/settings/EnumSettings.h b/include/settings/EnumSettings.h
@@ -275,7 +275,7 @@ enum class BrimLocation
 /*!
  * Convenience binary operator to allow testing brim location easily, like (actual_location & BrimLocation::OUTSIDE)
  */
-static int operator&(BrimLocation location1, BrimLocation location2)
+[[maybe_unused]] static int operator&(BrimLocation location1, BrimLocation location2)
 {
     return static_cast<int>(location1) & static_cast<int>(location2);
 }

diff --git a/src/FffGcodeWriter.cpp b/src/FffGcodeWriter.cpp
@@ -998,7 +998,7 @@ void FffGcodeWriter::processRaft(const SliceDataStorage& storage)
             raft_outline_path = raft_outline_path.difference(storage.primeTower.getOuterPoly(layer_nr));
         }
 
-        for (const Shape raft_island : raft_outline_path.splitIntoParts())
+        for (const Shape& raft_island : raft_outline_path.splitIntoParts())
         {
             Infill infill_comp(
                 EFillMethod::ZIG_ZAG,
@@ -2689,7 +2689,7 @@ bool FffGcodeWriter::processInsets(
             gcode_layer.setOverhangMask(overhang_region);
         }
 
-        const auto roofing_mask = [&]() -> Shape
+        const auto roofing_mask_fn = [&]() -> Shape
         {
             const size_t roofing_layer_count = std::min(mesh.settings.get<size_t>("roofing_layer_count"), mesh.settings.get<size_t>("top_layers"));
 
@@ -2711,7 +2711,7 @@ bool FffGcodeWriter::processInsets(
             return roofing_mask;
         }();
 
-        gcode_layer.setRoofingMask(roofing_mask);
+        gcode_layer.setRoofingMask(roofing_mask_fn);
     }
     else
     {

diff --git a/src/WallsComputation.cpp b/src/WallsComputation.cpp
@@ -85,7 +85,7 @@ void WallsComputation::generateWalls(SliceLayerPart* part, SectionType section_t
         part->inner_area = wall_tool_paths.getInnerContour();
     }
 
-    part->outline = SingleShape({ Simplify(settings_).polygon(part->outline) });
+    part->outline = SingleShape{ Simplify(settings_).polygon(part->outline) };
     part->print_outline = part->outline;
 }