Cura 10728 skirt height for skirt outline #1904

saumyaj3 · 2023-07-14T11:33:40Z

Description

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Test A
Test B

Test Configuration:

Operating System:

Checklist:

My code follows the style guidelines of this project as described in UltiMaker Meta
I have read the Contribution guide
I have commented my code, particularly in hard-to-understand areas
I have uploaded any files required to test this change

CURA-10728

github-actions · 2023-07-14T11:43:37Z

Unit Test Results

26 tests ±0 26 ✔️ ±0 16s ⏱️ +3s
  1 suites ±0   0 💤 ±0
  1 files ±0   0 ❌ ±0

Results for commit 4169359. ± Comparison against base commit 14bb2f2.

♻️ This comment has been updated with latest results.

include/utils/actions/smooth.h

This will fix an incorrect format causing test failure on 32-bit architectures.

If user tries to give skirt height more than the model_height. CURA-10728

casperlamboo · 2023-07-19T08:23:25Z

src/SkirtBrim.cpp

+            skirt_height = std::min(std::max(skirt_height, extruder.settings.get<int>("skirt_height")), static_cast<int>(storage.print_layer_count));
+        }


I have a slight preference for something like this:

Suggested change

skirt_height = std::min(std::max(skirt_height, extruder.settings.get<int>("skirt_height")), static_cast<int>(storage.print_layer_count));

}

skirt_height = std::max(skirt_height, extruder.settings.get<int>("skirt_height"));

}

skirt_height = std::min(skirt_height, static_cast<int>(storage.print_layer_count));

It is more readable in my opinion as you sequence the operations more. First you calculate the max skirt height of the various extrudes, then you cap this height to the layer count.

casperlamboo · 2023-07-19T08:36:55Z

src/SkirtBrim.cpp

-                (
-                    storage.getLayerOutlines(i_layer, include_support, include_prime_tower, external_only, extruder_nr)
-                );
+            skirt_height = std::min(std::max(skirt_height, extruder.settings.get<int>("skirt_height")), static_cast<int>(storage.print_layer_count));


From the documentation:

* \param extruder_nr The extruder for which to get the outlines. -1 to include outliens for all extruders

We should thus only include the skirt height of all extruders when this variable is set to -1. Taking some inspiration from this function

CuraEngine/src/sliceDataStorage.cpp

Line 285 in 14bb2f2

if (include_support && (extruder_nr == -1 || extruder_nr == int(mesh_group_settings.get<ExtruderTrain&>("adhesion_extruder_nr").extruder_nr)))

I would suggest to add something like

Suggested change

skirt_height = std::min(std::max(skirt_height, extruder.settings.get<int>("skirt_height")), static_cast<int>(storage.print_layer_count));

if (extruder_nr == -1 || extruder_nr == extruder.extruder_nr)

{

skirt_height = std::min(std::max(skirt_height, extruder.settings.get<int>("skirt_height")), static_cast<int>(storage.print_layer_count));

}

casperlamboo · 2023-07-19T08:37:12Z

src/SkirtBrim.cpp

+        for (int i_layer = layer_nr; i_layer < skirt_height; ++i_layer)
+        {
+            for (const auto& extruder : Application::getInstance().current_slice->scene.extruders)
+            {
+                first_layer_outline =
+                    first_layer_outline.unionPolygons
+                    (
+                        storage.getLayerOutlines(i_layer, include_support, include_prime_tower, external_only, extruder.extruder_nr)
+                    );
+            }
        }


I'm not sure if this is needed; we have an extruder_nr parameter, the documentation for this parameter reads

* \param extruder_nr The extruder for which to get the outlines. -1 to include outliens for all extruders

So we should only include the layer_outline for all extruders if extruder_nr is set to -1. When executing storage.getLayerOutlines with this -1 argument should already return the outline of all extruders.

CURA-10728

github-actions

⚠️ clang-tidy found issue(s) with the introduced code (1/10)

github-actions · 2023-07-20T13:34:23Z

include/utils/actions/smooth.h

+        const utils::integral auto max_resolution,
+        const utils::floating_point auto AB_magnitude,
+        const utils::floating_point auto BC_magnitude,
+        const utils::floating_point auto CD_magnitude) const noexcept
    {
        if (BC_magnitude > max_resolution / 10) // TODO: make dedicated front-end setting for this
        {


⚠️ readability-identifier-naming ⚠️
invalid case style for class smooth_fn

Suggested change

{

inline constexpr SmoothFn smooth{};

include/utils/actions/smooth.h

github-actions · 2023-07-20T13:34:24Z

include/utils/actions/smooth.h

@@ -124,15 +125,24 @@ struct smooth_fn
    }

    template<class Vector>
-        requires utils::point2d<Vector> || utils::junction<Vector> constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
+    requires utils::point2d<Vector> || utils::junction<Vector>
+    constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
    {
        return std::get<"X">(*point_0) * std::get<"X">(*point_1) + std::get<"Y">(*point_0) * std::get<"Y">(*point_1);
    }

    template<class Point>


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter A

Suggested change

template<class Point>

constexpr auto isWithinAllowedDeviations(Point* a, Point* B, Point* C, Point* D, const utils::floating_point auto fluid_angle, const utils::integral auto max_resolution,

github-actions · 2023-07-20T13:34:24Z

include/utils/actions/smooth.h

+        Point* B,
+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter A

Suggested change

const utils::floating_point auto fluid_angle,

const double cos_A = std::acos(cosAngle(a, B, C, AB_magnitude, BC_magnitude));

github-actions · 2023-07-20T13:34:24Z

include/utils/actions/smooth.h

+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,
+        const utils::integral auto max_resolution,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter A

Suggested change

const utils::integral auto max_resolution,

const double cos_B = std::acos(cosAngle(a, B, D, AB_magnitude, CD_magnitude));

github-actions · 2023-07-20T13:34:24Z

include/utils/actions/smooth.h

@@ -124,15 +125,24 @@ struct smooth_fn
    }

    template<class Vector>
-        requires utils::point2d<Vector> || utils::junction<Vector> constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
+    requires utils::point2d<Vector> || utils::junction<Vector>
+    constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
    {
        return std::get<"X">(*point_0) * std::get<"X">(*point_1) + std::get<"Y">(*point_0) * std::get<"Y">(*point_1);
    }

    template<class Point>


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter B

Suggested change

template<class Point>

constexpr auto isWithinAllowedDeviations(Point* A, Point* b, Point* C, Point* D, const utils::floating_point auto fluid_angle, const utils::integral auto max_resolution,

github-actions · 2023-07-20T13:34:24Z

include/utils/actions/smooth.h

+        Point* B,
+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter B

Suggested change

const utils::floating_point auto fluid_angle,

const double cos_A = std::acos(cosAngle(A, b, C, AB_magnitude, BC_magnitude));

github-actions · 2023-07-20T13:34:24Z

include/utils/actions/smooth.h

+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,
+        const utils::integral auto max_resolution,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter B

Suggested change

const utils::integral auto max_resolution,

const double cos_B = std::acos(cosAngle(A, b, D, AB_magnitude, CD_magnitude));

github-actions · 2023-07-20T13:34:24Z

include/utils/actions/smooth.h

@@ -124,15 +125,24 @@ struct smooth_fn
    }

    template<class Vector>
-        requires utils::point2d<Vector> || utils::junction<Vector> constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
+    requires utils::point2d<Vector> || utils::junction<Vector>
+    constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
    {
        return std::get<"X">(*point_0) * std::get<"X">(*point_1) + std::get<"Y">(*point_0) * std::get<"Y">(*point_1);
    }

    template<class Point>


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter C

Suggested change

template<class Point>

constexpr auto isWithinAllowedDeviations(Point* A, Point* B, Point* c, Point* D, const utils::floating_point auto fluid_angle, const utils::integral auto max_resolution,

github-actions · 2023-07-20T13:34:25Z

include/utils/actions/smooth.h

+        Point* B,
+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter C

Suggested change

const utils::floating_point auto fluid_angle,

const double cos_A = std::acos(cosAngle(A, B, c, AB_magnitude, BC_magnitude));

github-actions

⚠️ clang-tidy found issue(s) with the introduced code (2/10)

github-actions · 2023-07-20T13:34:37Z

include/utils/actions/smooth.h

@@ -124,15 +125,24 @@ struct smooth_fn
    }

    template<class Vector>
-        requires utils::point2d<Vector> || utils::junction<Vector> constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
+    requires utils::point2d<Vector> || utils::junction<Vector>
+    constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
    {
        return std::get<"X">(*point_0) * std::get<"X">(*point_1) + std::get<"Y">(*point_0) * std::get<"Y">(*point_1);
    }

    template<class Point>


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter D

Suggested change

template<class Point>

constexpr auto isWithinAllowedDeviations(Point* A, Point* B, Point* C, Point* d, const utils::floating_point auto fluid_angle, const utils::integral auto max_resolution,

github-actions · 2023-07-20T13:34:38Z

include/utils/actions/smooth.h

+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,
+        const utils::integral auto max_resolution,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter D

Suggested change

const utils::integral auto max_resolution,

const double cos_B = std::acos(cosAngle(A, B, d, AB_magnitude, CD_magnitude));

github-actions · 2023-07-20T13:34:38Z

include/utils/actions/smooth.h

@@ -124,15 +125,24 @@ struct smooth_fn
    }

    template<class Vector>
-        requires utils::point2d<Vector> || utils::junction<Vector> constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
+    requires utils::point2d<Vector> || utils::junction<Vector>
+    constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
    {
        return std::get<"X">(*point_0) * std::get<"X">(*point_1) + std::get<"Y">(*point_0) * std::get<"Y">(*point_1);
    }

    template<class Point>
    requires utils::point2d<Point> || utils::junction<Point>


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter AB_magnitude

Suggested change

requires utils::point2d<Point> || utils::junction<Point>

const utils::floating_point auto ab_magnitude, const utils::floating_point auto BC_magnitude, const utils::floating_point auto CD_magnitude) const noexcept

github-actions · 2023-07-20T13:34:38Z

include/utils/actions/smooth.h

+        Point* B,
+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter AB_magnitude

Suggested change

const utils::floating_point auto fluid_angle,

const double cos_A = std::acos(cosAngle(A, B, C, ab_magnitude, BC_magnitude));

github-actions · 2023-07-20T13:34:38Z

include/utils/actions/smooth.h

+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,
+        const utils::integral auto max_resolution,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter AB_magnitude

Suggested change

const utils::integral auto max_resolution,

const double cos_B = std::acos(cosAngle(A, B, D, ab_magnitude, CD_magnitude));

github-actions · 2023-07-20T13:34:38Z

include/utils/actions/smooth.h

@@ -124,15 +125,24 @@ struct smooth_fn
    }

    template<class Vector>
-        requires utils::point2d<Vector> || utils::junction<Vector> constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
+    requires utils::point2d<Vector> || utils::junction<Vector>
+    constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
    {
        return std::get<"X">(*point_0) * std::get<"X">(*point_1) + std::get<"Y">(*point_0) * std::get<"Y">(*point_1);
    }

    template<class Point>
    requires utils::point2d<Point> || utils::junction<Point>


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter BC_magnitude

Suggested change

requires utils::point2d<Point> || utils::junction<Point>

const utils::floating_point auto AB_magnitude, const utils::floating_point auto bc_magnitude, const utils::floating_point auto CD_magnitude) const noexcept

github-actions · 2023-07-20T13:34:39Z

include/utils/actions/smooth.h

-    constexpr auto isWithinAllowedDeviations(Point* A, Point* B, Point* C, Point* D, const utils::floating_point auto fluid_angle, const utils::integral auto max_resolution,
-                                          const utils::floating_point auto AB_magnitude, const utils::floating_point auto BC_magnitude, const utils::floating_point auto CD_magnitude) const noexcept
+    constexpr auto isWithinAllowedDeviations(
+        Point* A,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter BC_magnitude

Suggested change

Point* A,

if (bc_magnitude > max_resolution / 10) // TODO: make dedicated front-end setting for this

github-actions · 2023-07-20T13:34:39Z

include/utils/actions/smooth.h

+        Point* B,
+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter BC_magnitude

Suggested change

const utils::floating_point auto fluid_angle,

const double cos_A = std::acos(cosAngle(A, B, C, AB_magnitude, bc_magnitude));

github-actions · 2023-07-20T13:34:39Z

include/utils/actions/smooth.h

@@ -124,15 +125,24 @@ struct smooth_fn
    }

    template<class Vector>
-        requires utils::point2d<Vector> || utils::junction<Vector> constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
+    requires utils::point2d<Vector> || utils::junction<Vector>
+    constexpr auto dotProduct(Vector* point_0, Vector* point_1) const noexcept
    {
        return std::get<"X">(*point_0) * std::get<"X">(*point_1) + std::get<"Y">(*point_0) * std::get<"Y">(*point_1);
    }

    template<class Point>
    requires utils::point2d<Point> || utils::junction<Point>


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter CD_magnitude

Suggested change

requires utils::point2d<Point> || utils::junction<Point>

const utils::floating_point auto AB_magnitude, const utils::floating_point auto BC_magnitude, const utils::floating_point auto cd_magnitude) const noexcept

github-actions · 2023-07-20T13:34:39Z

include/utils/actions/smooth.h

+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,
+        const utils::integral auto max_resolution,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter CD_magnitude

Suggested change

const utils::integral auto max_resolution,

const double cos_B = std::acos(cosAngle(A, B, D, AB_magnitude, cd_magnitude));

github-actions

⚠️ clang-tidy found issue(s) with the introduced code (3/10)

github-actions · 2023-07-20T13:34:53Z

include/utils/actions/smooth.h

-    constexpr auto isWithinAllowedDeviations(Point* A, Point* B, Point* C, Point* D, const utils::floating_point auto fluid_angle, const utils::integral auto max_resolution,
-                                          const utils::floating_point auto AB_magnitude, const utils::floating_point auto BC_magnitude, const utils::floating_point auto CD_magnitude) const noexcept
+    constexpr auto isWithinAllowedDeviations(
+        Point* A,


⚠️ cppcoreguidelines-avoid-magic-numbers ⚠️
10 is a magic number; consider replacing it with a named constant

github-actions · 2023-07-20T13:34:53Z

include/utils/actions/smooth.h

+        Point* B,
+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,


⚠️ readability-identifier-naming ⚠️
invalid case style for variable cos_A

Suggested change

const utils::floating_point auto fluid_angle,

const double cos_a = std::acos(cosAngle(A, B, C, AB_magnitude, BC_magnitude));

github-actions · 2023-07-20T13:34:53Z

include/utils/actions/smooth.h

+        Point* D,
+        const utils::floating_point auto fluid_angle,
+        const utils::integral auto max_resolution,
+        const utils::floating_point auto AB_magnitude,


⚠️ readability-identifier-naming ⚠️
invalid case style for variable cos_A

Suggested change

const utils::floating_point auto AB_magnitude,

const auto abs_angle = std::abs(cos_a - cos_B);

github-actions · 2023-07-20T13:34:53Z

include/utils/actions/smooth.h

+        Point* B,
+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,


⚠️ clang-diagnostic-error ⚠️
no member named acos in namespace std

github-actions · 2023-07-20T13:34:53Z

include/utils/actions/smooth.h

+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,
+        const utils::integral auto max_resolution,


⚠️ readability-identifier-naming ⚠️
invalid case style for variable cos_B

Suggested change

const utils::integral auto max_resolution,

const double cos_b = std::acos(cosAngle(A, B, D, AB_magnitude, CD_magnitude));

github-actions · 2023-07-20T13:34:53Z

include/utils/actions/smooth.h

+        Point* D,
+        const utils::floating_point auto fluid_angle,
+        const utils::integral auto max_resolution,
+        const utils::floating_point auto AB_magnitude,


⚠️ readability-identifier-naming ⚠️
invalid case style for variable cos_B

Suggested change

const utils::floating_point auto AB_magnitude,

const auto abs_angle = std::abs(cos_A - cos_b);

github-actions · 2023-07-20T13:34:53Z

include/utils/actions/smooth.h

+        Point* C,
+        Point* D,
+        const utils::floating_point auto fluid_angle,
+        const utils::integral auto max_resolution,


⚠️ clang-diagnostic-error ⚠️
no member named acos in namespace std

github-actions · 2023-07-20T13:34:54Z

src/SkirtBrim.cpp

 #include "SkirtBrim.h"

 #include "Application.h"
 #include "ExtruderTrain.h"
 #include "Slice.h"
+#include "settings/EnumSettings.h"
 #include "settings/types/Ratio.h"
 #include "sliceDataStorage.h"


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "sliceDataStorage.h"

#include "settings/EnumSettings.h"

github-actions · 2023-07-20T13:34:54Z

src/SkirtBrim.cpp

 #include "SkirtBrim.h"

 #include "Application.h"
 #include "ExtruderTrain.h"
 #include "Slice.h"
+#include "settings/EnumSettings.h"
 #include "settings/types/Ratio.h"
 #include "sliceDataStorage.h"
 #include "support.h"


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "support.h"

#include "settings/types/Ratio.h"

github-actions · 2023-07-20T13:34:54Z

src/SkirtBrim.cpp

 #include "settings/types/Ratio.h"
 #include "sliceDataStorage.h"
 #include "support.h"
-#include "utils/Simplify.h" //Simplifying the brim/skirt at every inset.
-#include "settings/EnumSettings.h"
 #include "utils/PolylineStitcher.h"


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "utils/PolylineStitcher.h"

#include "sliceDataStorage.h"

github-actions

⚠️ clang-tidy found issue(s) with the introduced code (4/10)

github-actions · 2023-07-20T13:35:07Z

src/SkirtBrim.cpp

 #include "utils/PolylineStitcher.h"
+#include "utils/Simplify.h" //Simplifying the brim/skirt at every inset.


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "utils/Simplify.h" //Simplifying the brim/skirt at every inset.

#include "support.h"

github-actions · 2023-07-20T13:35:07Z

src/SkirtBrim.cpp

 #include "utils/PolylineStitcher.h"
+#include "utils/Simplify.h" //Simplifying the brim/skirt at every inset.
+


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "utils/PolylineStitcher.h"

github-actions · 2023-07-20T13:35:07Z

src/SkirtBrim.cpp

 #include "utils/PolylineStitcher.h"
+#include "utils/Simplify.h" //Simplifying the brim/skirt at every inset.
+
+#include <spdlog/spdlog.h>


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include <spdlog/spdlog.h>

#include "utils/Simplify.h" //Simplifying the brim/skirt at every inset.

github-actions · 2023-07-20T13:35:07Z

src/SkirtBrim.cpp

+SkirtBrim::SkirtBrim(SliceDataStorage& storage)
+    : storage(storage)
+    , adhesion_type(Application::getInstance().current_slice->scene.current_mesh_group->settings.get<EPlatformAdhesion>("adhesion_type"))
+    , has_ooze_shield(storage.oozeShield.size() > 0 && storage.oozeShield[0].size() > 0)


⚠️ readability-container-size-empty ⚠️
the empty method should be used to check for emptiness instead of size

Suggested change

, has_ooze_shield(storage.oozeShield.size() > 0 && storage.oozeShield[0].size() > 0)

has_ooze_shield(!storage.oozeShield.empty() && storage.oozeShield[0].size() > 0),

github-actions · 2023-07-20T13:35:07Z

src/SkirtBrim.cpp

+SkirtBrim::SkirtBrim(SliceDataStorage& storage)
+    : storage(storage)
+    , adhesion_type(Application::getInstance().current_slice->scene.current_mesh_group->settings.get<EPlatformAdhesion>("adhesion_type"))
+    , has_ooze_shield(storage.oozeShield.size() > 0 && storage.oozeShield[0].size() > 0)


⚠️ readability-container-size-empty ⚠️
the empty method should be used to check for emptiness instead of size

Suggested change

, has_ooze_shield(storage.oozeShield.size() > 0 && storage.oozeShield[0].size() > 0)

has_ooze_shield(storage.oozeShield.size() > 0 && !storage.oozeShield[0].empty()),

github-actions · 2023-07-20T13:35:08Z

src/SkirtBrim.cpp

+    : storage(storage)
+    , adhesion_type(Application::getInstance().current_slice->scene.current_mesh_group->settings.get<EPlatformAdhesion>("adhesion_type"))
+    , has_ooze_shield(storage.oozeShield.size() > 0 && storage.oozeShield[0].size() > 0)
+    , has_draft_shield(storage.draft_protection_shield.size() > 0)


⚠️ readability-container-size-empty ⚠️
the empty method should be used to check for emptiness instead of size

Suggested change

, has_draft_shield(storage.draft_protection_shield.size() > 0)

has_draft_shield(!storage.draft_protection_shield.empty()),

github-actions · 2023-07-20T13:35:08Z

src/SkirtBrim.cpp

+    , has_ooze_shield(storage.oozeShield.size() > 0 && storage.oozeShield[0].size() > 0)
+    , has_draft_shield(storage.draft_protection_shield.size() > 0)
+    , extruders(Application::getInstance().current_slice->scene.extruders)
+    , extruder_count(extruders.size())


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from std::vector::size_type (aka unsigned long) to signed type int is implementation-defined

github-actions · 2023-07-20T13:35:08Z

src/SkirtBrim.cpp

+    , has_draft_shield(storage.draft_protection_shield.size() > 0)
+    , extruders(Application::getInstance().current_slice->scene.extruders)
+    , extruder_count(extruders.size())
+    , extruder_is_used(storage.getExtrudersUsed())


⚠️ cppcoreguidelines-prefer-member-initializer ⚠️
first_used_extruder_nr should be initialized in a member initializer of the constructor

Suggested change

, extruder_is_used(storage.getExtrudersUsed())

extruder_is_used(storage.getExtrudersUsed()), first_used_extruder_nr(0)

github-actions · 2023-07-20T13:35:08Z

src/SkirtBrim.cpp

+    , has_draft_shield(storage.draft_protection_shield.size() > 0)
+    , extruders(Application::getInstance().current_slice->scene.extruders)
+    , extruder_count(extruders.size())
+    , extruder_is_used(storage.getExtrudersUsed())
 {
    first_used_extruder_nr = 0;


⚠️ cppcoreguidelines-prefer-member-initializer ⚠️
first_used_extruder_nr should be initialized in a member initializer of the constructor

Suggested change

first_used_extruder_nr = 0;

github-actions · 2023-07-20T13:35:08Z

src/SkirtBrim.cpp

@@ -192,7 +192,7 @@ void SkirtBrim::generate()

    // Secondary brim of all other materials which don;t meet minimum length constriant yet
    generateSecondarySkirtBrim(covered_area, allowed_areas_per_extruder, total_length);
-    
+
    // simplify paths to prevent buffer unnerruns in firmware
    const Settings& global_settings = Application::getInstance().current_slice->scene.current_mesh_group->settings;
    const coord_t maximum_resolution = global_settings.get<coord_t>("meshfix_maximum_resolution");


⚠️ hicpp-use-auto ⚠️
use auto when initializing with a template cast to avoid duplicating the type name

Suggested change

const coord_t maximum_resolution = global_settings.get<coord_t>("meshfix_maximum_resolution");

const auto maximum_resolution = global_settings.get<coord_t>("meshfix_maximum_resolution");

github-actions

⚠️ clang-tidy found issue(s) with the introduced code (5/10)

github-actions · 2023-07-20T13:35:22Z

src/SkirtBrim.cpp

-            total_length[offset.extruder_nr] < skirt_brim_minimal_length[offset.extruder_nr] && // This was the last offset of this extruder, but the brim lines don't meet minimal length yet
+
+        if (offset.is_last && total_length[offset.extruder_nr] < skirt_brim_minimal_length[offset.extruder_nr]
+            && // This was the last offset of this extruder, but the brim lines don't meet minimal length yet
            total_length[offset.extruder_nr] > 0u // No lines got added; we have no extrusion lines to build on
        )
        {


⚠️ hicpp-uppercase-literal-suffix ⚠️
integer literal has suffix u, which is not uppercase

Suggested change

{

total_length[offset.extruder_nr] > 0U // No lines got added; we have no extrusion lines to build on

github-actions · 2023-07-20T13:35:23Z

src/SkirtBrim.cpp

@@ -267,7 +263,7 @@ Polygons SkirtBrim::getInternalHoleExclusionArea(const Polygons& outline, const
        {
            Polygon hole_poly = part[hole_idx];
            hole_poly.reverse();
-            Polygons disallowed_region = hole_poly.offset(10u).difference(hole_poly.offset( - line_widths[extruder_nr] / 2 - hole_brim_distance));
+            Polygons disallowed_region = hole_poly.offset(10u).difference(hole_poly.offset(-line_widths[extruder_nr] / 2 - hole_brim_distance));


⚠️ altera-id-dependent-backward-branch ⚠️
backward branch (for loop) is ID-dependent due to variable reference to part and may cause performance degradation

github-actions · 2023-07-20T13:35:23Z

src/SkirtBrim.cpp

            local_brim.add(closed_polygons_brim);

-            auto open_polylines_brim = storage.skirt_brim[offset.extruder_nr][reference_idx].open_polylines
-                                           .offsetPolyLine(offset_dist, ClipperLib::jtRound);
+            auto open_polylines_brim = storage.skirt_brim[offset.extruder_nr][reference_idx].open_polylines.offsetPolyLine(offset_dist, ClipperLib::jtRound);
            local_brim.add(open_polylines_brim);


⚠️ cppcoreguidelines-pro-bounds-constant-array-index ⚠️
do not use array subscript when the index is not an integer constant expression

github-actions · 2023-07-20T13:35:23Z

src/SkirtBrim.cpp

            local_brim.add(closed_polygons_brim);

-            auto open_polylines_brim = storage.skirt_brim[offset.extruder_nr][reference_idx].open_polylines
-                                           .offsetPolyLine(offset_dist, ClipperLib::jtRound);
+            auto open_polylines_brim = storage.skirt_brim[offset.extruder_nr][reference_idx].open_polylines.offsetPolyLine(offset_dist, ClipperLib::jtRound);
            local_brim.add(open_polylines_brim);
            local_brim.unionPolygons();


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from __gnu_cxx::__alloc_traits<std::allocator<long long>, long long>::value_type (aka long long) to signed type int is implementation-defined

github-actions · 2023-07-20T13:35:23Z

src/SkirtBrim.cpp

@@ -373,29 +367,39 @@ Polygons SkirtBrim::getFirstLayerOutline(const int extruder_nr /* = -1 */)
    Polygons first_layer_outline;
    Settings& global_settings = Application::getInstance().current_slice->scene.current_mesh_group->settings;
    int reference_extruder_nr = skirt_brim_extruder_nr;
-    assert( ! (reference_extruder_nr == -1 && extruder_nr == -1) && "We should only request the outlines of all layers when the brim is being generated for only one material");
+    assert(! (reference_extruder_nr == -1 && extruder_nr == -1) && "We should only request the outlines of all layers when the brim is being generated for only one material");
    if (reference_extruder_nr == -1)


⚠️ readability-function-cognitive-complexity ⚠️
function getFirstLayerOutline has cognitive complexity of 43 (threshold 25)

github-actions · 2023-07-20T13:35:23Z

src/SkirtBrim.cpp

@@ -373,29 +367,39 @@ Polygons SkirtBrim::getFirstLayerOutline(const int extruder_nr /* = -1 */)
    Polygons first_layer_outline;
    Settings& global_settings = Application::getInstance().current_slice->scene.current_mesh_group->settings;
    int reference_extruder_nr = skirt_brim_extruder_nr;
-    assert( ! (reference_extruder_nr == -1 && extruder_nr == -1) && "We should only request the outlines of all layers when the brim is being generated for only one material");
+    assert(! (reference_extruder_nr == -1 && extruder_nr == -1) && "We should only request the outlines of all layers when the brim is being generated for only one material");
    if (reference_extruder_nr == -1)


⚠️ readability-function-size ⚠️
function getFirstLayerOutline exceeds recommended size/complexity thresholds

github-actions · 2023-07-20T13:35:23Z

src/SkirtBrim.cpp

+            for (const auto& extruder : Application::getInstance().current_slice->scene.extruders)
+            {
+                first_layer_outline
+                    = first_layer_outline.unionPolygons(storage.getLayerOutlines(i_layer, include_support, include_prime_tower, external_only, extruder.extruder_nr));


⚠️ altera-id-dependent-backward-branch ⚠️
backward branch (for loop) is ID-dependent due to variable reference to i_layer and may cause performance degradation

github-actions · 2023-07-20T13:35:24Z

src/SkirtBrim.cpp

@@ -543,7 +544,8 @@ void SkirtBrim::generateShieldBrim(Polygons& brim_covered_area, std::vector<Poly
        while (shield_brim.size() > 0)


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from long long to signed type int is implementation-defined

github-actions · 2023-07-20T13:35:24Z

src/SkirtBrim.cpp

@@ -543,7 +544,8 @@ void SkirtBrim::generateShieldBrim(Polygons& brim_covered_area, std::vector<Poly
        while (shield_brim.size() > 0)
        {
            shield_brim = shield_brim.offset(-primary_extruder_skirt_brim_line_width);
-            storage.skirt_brim[extruder_nr].back().closed_polygons.add(shield_brim); // throw all polygons for the shileds onto one heap; because the brim lines are generated from both sides the order will not be important
+            storage.skirt_brim[extruder_nr].back().closed_polygons.add(
+                shield_brim); // throw all polygons for the shileds onto one heap; because the brim lines are generated from both sides the order will not be important


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from long long to signed type int is implementation-defined

github-actions · 2023-07-20T13:35:24Z

src/SkirtBrim.cpp

@@ -588,19 +590,19 @@ void SkirtBrim::generateSecondarySkirtBrim(Polygons& covered_area, std::vector<P
            }


⚠️ cppcoreguidelines-init-variables ⚠️
variable offset_from_reference is not initialized

Suggested change

}

coord_t offset_from_reference = 0;

github-actions

⚠️ clang-tidy found issue(s) with the introduced code (6/10)

github-actions · 2023-07-20T13:35:37Z

src/SkirtBrim.cpp

            storage.skirt_brim[extruder_nr].emplace_back();
            SkirtBrimLine& output_location = storage.skirt_brim[extruder_nr].back();
            coord_t added_length = generateOffset(extra_offset, covered_area, allowed_areas_per_extruder, output_location);

-            if ( ! added_length)
+            if (! added_length)


⚠️ cppcoreguidelines-pro-bounds-constant-array-index ⚠️
do not use array subscript when the index is not an integer constant expression

github-actions · 2023-07-20T13:35:37Z

src/SkirtBrim.cpp

            storage.skirt_brim[extruder_nr].emplace_back();
            SkirtBrimLine& output_location = storage.skirt_brim[extruder_nr].back();
            coord_t added_length = generateOffset(extra_offset, covered_area, allowed_areas_per_extruder, output_location);

-            if ( ! added_length)
+            if (! added_length)
            {
                spdlog::warn("Couldn't satisfy minimum length constraint of extruder {}!\n", extruder_nr);
                break;
            }


⚠️ cppcoreguidelines-pro-bounds-constant-array-index ⚠️
do not use array subscript when the index is not an integer constant expression

github-actions · 2023-07-20T13:35:37Z

src/SkirtBrim.cpp

            storage.skirt_brim[extruder_nr].emplace_back();
            SkirtBrimLine& output_location = storage.skirt_brim[extruder_nr].back();
            coord_t added_length = generateOffset(extra_offset, covered_area, allowed_areas_per_extruder, output_location);

-            if ( ! added_length)
+            if (! added_length)
            {
                spdlog::warn("Couldn't satisfy minimum length constraint of extruder {}!\n", extruder_nr);
                break;
            }

            total_length[extra_offset.extruder_nr] += added_length;


⚠️ cppcoreguidelines-pro-bounds-constant-array-index ⚠️
do not use array subscript when the index is not an integer constant expression

github-actions · 2023-07-20T13:35:37Z

src/SkirtBrim.cpp

            {
                spdlog::warn("Couldn't satisfy minimum length constraint of extruder {}!\n", extruder_nr);
                break;
            }

            total_length[extra_offset.extruder_nr] += added_length;
-            
+


⚠️ cppcoreguidelines-pro-bounds-constant-array-index ⚠️
do not use array subscript when the index is not an integer constant expression

github-actions · 2023-07-20T13:35:37Z

src/SkirtBrim.cpp

            {
                spdlog::warn("Couldn't satisfy minimum length constraint of extruder {}!\n", extruder_nr);
                break;
            }

            total_length[extra_offset.extruder_nr] += added_length;
-            
+
            first = false;
        }
    }


⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion cura::coord_t (aka long long) -> bool

Suggested change

}

if ( added_length)

github-actions · 2023-07-20T13:35:37Z

src/SkirtBrim.cpp

            {
                spdlog::warn("Couldn't satisfy minimum length constraint of extruder {}!\n", extruder_nr);
                break;
            }

            total_length[extra_offset.extruder_nr] += added_length;
-            
+
            first = false;
        }
    }


⚠️ readability-implicit-bool-conversion ⚠️
implicit conversion cura::coord_t (aka long long) -> bool

Suggested change

}

if ( ! added_length == 0)

github-actions · 2023-07-20T13:35:38Z

src/pathPlanning/Comb.cpp

-#include "utils/linearAlg2D.h"
-#include "utils/PolygonsPointIndex.h"
+#include "pathPlanning/CombPaths.h"
+#include "pathPlanning/LinePolygonsCrossings.h"


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "pathPlanning/LinePolygonsCrossings.h"

#include "Application.h"

github-actions · 2023-07-20T13:35:38Z

src/pathPlanning/Comb.cpp

-#include "utils/linearAlg2D.h"
-#include "utils/PolygonsPointIndex.h"
+#include "pathPlanning/CombPaths.h"
+#include "pathPlanning/LinePolygonsCrossings.h"
 #include "sliceDataStorage.h"


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "sliceDataStorage.h"

#include "ExtruderTrain.h"

github-actions · 2023-07-20T13:35:38Z

src/pathPlanning/Comb.cpp

 #include "sliceDataStorage.h"
+#include "utils/PolygonsPointIndex.h"


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "utils/PolygonsPointIndex.h"

#include "Slice.h"

github-actions · 2023-07-20T13:35:38Z

src/pathPlanning/Comb.cpp

 #include "sliceDataStorage.h"
+#include "utils/PolygonsPointIndex.h"
 #include "utils/SVG.h"


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "utils/SVG.h"

#include "pathPlanning/CombPaths.h"

github-actions

⚠️ clang-tidy found issue(s) with the introduced code (7/10)

github-actions · 2023-07-20T13:35:51Z

src/pathPlanning/Comb.cpp

 #include "utils/SVG.h"
+#include "utils/linearAlg2D.h"


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "utils/linearAlg2D.h"

#include "pathPlanning/LinePolygonsCrossings.h"

github-actions · 2023-07-20T13:35:51Z

src/pathPlanning/Comb.cpp

 #include "utils/SVG.h"
+#include "utils/linearAlg2D.h"



⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include "sliceDataStorage.h"

github-actions · 2023-07-20T13:35:52Z

src/pathPlanning/Comb.cpp


-namespace cura {
+#include <algorithm>
+#include <functional> // function


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include <functional> // function

#include "utils/SVG.h"

github-actions · 2023-07-20T13:35:52Z

src/pathPlanning/Comb.cpp

-namespace cura {
+#include <algorithm>
+#include <functional> // function
+#include <unordered_set>


⚠️ llvm-include-order ⚠️
#includes are not sorted properly

Suggested change

#include <unordered_set>

#include "utils/linearAlg2D.h"

github-actions · 2023-07-20T13:35:52Z

src/pathPlanning/Comb.cpp


 LocToLineGrid& Comb::getOutsideLocToLine(const ExtruderTrain& train)
 {
    if (outside_loc_to_line[train.extruder_nr] == nullptr)
    {
-        outside_loc_to_line[train.extruder_nr] =
-            PolygonUtils::createLocToLineGrid(getBoundaryOutside(train), offset_from_inside_to_outside * 3 / 2);
+        outside_loc_to_line[train.extruder_nr] = PolygonUtils::createLocToLineGrid(getBoundaryOutside(train), offset_from_inside_to_outside * 3 / 2);


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from long long to signed type int is implementation-defined

github-actions · 2023-07-20T13:35:52Z

src/pathPlanning/Comb.cpp

@@ -34,8 +34,7 @@ Polygons& Comb::getBoundaryOutside(const ExtruderTrain& train)
    if (boundary_outside[train.extruder_nr].empty())
    {
        bool travel_avoid_supports = train.settings.get<bool>("travel_avoid_supports");
-        boundary_outside[train.extruder_nr] =
-            storage.getLayerOutlines(layer_nr, travel_avoid_supports, travel_avoid_supports).offset(travel_avoid_distance);
+        boundary_outside[train.extruder_nr] = storage.getLayerOutlines(layer_nr, travel_avoid_supports, travel_avoid_supports).offset(travel_avoid_distance);
    }


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from cura::coord_t (aka long long) to signed type int is implementation-defined

github-actions · 2023-07-20T13:35:52Z

src/pathPlanning/Comb.cpp

@@ -55,32 +53,39 @@ LocToLineGrid& Comb::getModelBoundaryLocToLine(const ExtruderTrain& train)
 {
    if (model_boundary_loc_to_line[train.extruder_nr] == nullptr)
    {
-        model_boundary_loc_to_line[train.extruder_nr] =
-            PolygonUtils::createLocToLineGrid(getModelBoundary(train), offset_from_inside_to_outside * 3 / 2);
+        model_boundary_loc_to_line[train.extruder_nr] = PolygonUtils::createLocToLineGrid(getModelBoundary(train), offset_from_inside_to_outside * 3 / 2);
    }
    return *model_boundary_loc_to_line[train.extruder_nr];
 }


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from long long to signed type int is implementation-defined

github-actions · 2023-07-20T13:35:53Z

src/pathPlanning/Comb.cpp

 #include "Application.h"
 #include "ExtruderTrain.h"
 #include "Slice.h"
-#include "utils/linearAlg2D.h"
-#include "utils/PolygonsPointIndex.h"
+#include "pathPlanning/CombPaths.h"


⚠️ modernize-pass-by-value ⚠️
pass by value and use std::move

Suggested change

#include "pathPlanning/CombPaths.h"

#include <utility>

github-actions · 2023-07-20T13:35:53Z

src/pathPlanning/Comb.cpp

+Comb::Comb(
+    const SliceDataStorage& storage,
+    const LayerIndex layer_nr,
+    const Polygons& comb_boundary_inside_minimum,


⚠️ modernize-pass-by-value ⚠️
pass by value and use std::move

Suggested change

const Polygons& comb_boundary_inside_minimum,

Comb::Comb(const SliceDataStorage& storage, const LayerIndex layer_nr, Polygons comb_boundary_inside_minimum, const Polygons& comb_boundary_inside_optimal, coord_t comb_boundary_offset, coord_t travel_avoid_distance, coord_t move_inside_distance)

github-actions · 2023-07-20T13:35:53Z

src/pathPlanning/Comb.cpp

+    coord_t move_inside_distance)
+    : storage(storage)
+    , layer_nr(layer_nr)
+    , offset_from_outlines(comb_boundary_offset) // between second wall and infill / other walls


⚠️ modernize-pass-by-value ⚠️
pass by value and use std::move

Suggested change

, offset_from_outlines(comb_boundary_offset) // between second wall and infill / other walls

, boundary_inside_minimum(std::move( comb_boundary_inside_minimum ) // copy the boundary, because the partsView_inside will reorder the polygons

github-actions

⚠️ clang-tidy found issue(s) with the introduced code (8/10)

github-actions · 2023-07-20T13:36:07Z

src/pathPlanning/Comb.cpp

+    coord_t move_inside_distance)
+    : storage(storage)
+    , layer_nr(layer_nr)
+    , offset_from_outlines(comb_boundary_offset) // between second wall and infill / other walls


⚠️ modernize-pass-by-value ⚠️
pass by value and use std::move

Suggested change

, offset_from_outlines(comb_boundary_offset) // between second wall and infill / other walls

, boundary_inside_minimum( comb_boundary_inside_minimum )) // copy the boundary, because the partsView_inside will reorder the polygons

github-actions · 2023-07-20T13:36:07Z

src/pathPlanning/Comb.cpp

+Comb::Comb(
+    const SliceDataStorage& storage,
+    const LayerIndex layer_nr,
+    const Polygons& comb_boundary_inside_minimum,


⚠️ modernize-pass-by-value ⚠️
pass by value and use std::move

Suggested change

const Polygons& comb_boundary_inside_minimum,

Comb::Comb(const SliceDataStorage& storage, const LayerIndex layer_nr, const Polygons& comb_boundary_inside_minimum, Polygons comb_boundary_inside_optimal, coord_t comb_boundary_offset, coord_t travel_avoid_distance, coord_t move_inside_distance)

github-actions · 2023-07-20T13:36:07Z

src/pathPlanning/Comb.cpp

+    : storage(storage)
+    , layer_nr(layer_nr)
+    , offset_from_outlines(comb_boundary_offset) // between second wall and infill / other walls
+    , max_moveInside_distance2(offset_from_outlines * offset_from_outlines)


⚠️ modernize-pass-by-value ⚠️
pass by value and use std::move

Suggested change

, max_moveInside_distance2(offset_from_outlines * offset_from_outlines)

, boundary_inside_optimal(std::move( comb_boundary_inside_optimal ) // copy the boundary, because the partsView_inside will reorder the polygons

github-actions · 2023-07-20T13:36:07Z

src/pathPlanning/Comb.cpp

+    : storage(storage)
+    , layer_nr(layer_nr)
+    , offset_from_outlines(comb_boundary_offset) // between second wall and infill / other walls
+    , max_moveInside_distance2(offset_from_outlines * offset_from_outlines)


⚠️ modernize-pass-by-value ⚠️
pass by value and use std::move

Suggested change

, max_moveInside_distance2(offset_from_outlines * offset_from_outlines)

, boundary_inside_optimal( comb_boundary_inside_optimal )) // copy the boundary, because the partsView_inside will reorder the polygons

github-actions · 2023-07-20T13:36:07Z

src/pathPlanning/Comb.cpp

+    , max_moveInside_distance2(offset_from_outlines * offset_from_outlines)
+    , offset_from_inside_to_outside(offset_from_outlines + travel_avoid_distance)
+    , max_crossing_dist2(
+          offset_from_inside_to_outside * offset_from_inside_to_outside


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from cura::coord_t (aka long long) to signed type int is implementation-defined

github-actions · 2023-07-20T13:36:08Z

src/pathPlanning/Comb.cpp

+    , offset_from_inside_to_outside(offset_from_outlines + travel_avoid_distance)
+    , max_crossing_dist2(
+          offset_from_inside_to_outside * offset_from_inside_to_outside
+          * 2) // so max_crossing_dist = offset_from_inside_to_outside * sqrt(2) =approx 1.5 to allow for slightly diagonal crossings and slightly inaccurate crossing computation


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from cura::coord_t (aka long long) to signed type int is implementation-defined

github-actions · 2023-07-20T13:36:08Z

src/pathPlanning/Comb.cpp

+    , partsView_inside_minimum(boundary_inside_minimum.splitIntoPartsView()) // WARNING !! changes the order of boundary_inside !!
+    , partsView_inside_optimal(boundary_inside_optimal.splitIntoPartsView()) // WARNING !! changes the order of boundary_inside !!
+    , inside_loc_to_line_minimum(PolygonUtils::createLocToLineGrid(boundary_inside_minimum, comb_boundary_offset))
+    , inside_loc_to_line_optimal(PolygonUtils::createLocToLineGrid(boundary_inside_optimal, comb_boundary_offset))


⚠️ readability-function-cognitive-complexity ⚠️
function calc has cognitive complexity of 57 (threshold 25)

github-actions · 2023-07-20T13:36:08Z

src/pathPlanning/Comb.cpp

+    , partsView_inside_minimum(boundary_inside_minimum.splitIntoPartsView()) // WARNING !! changes the order of boundary_inside !!
+    , partsView_inside_optimal(boundary_inside_optimal.splitIntoPartsView()) // WARNING !! changes the order of boundary_inside !!
+    , inside_loc_to_line_minimum(PolygonUtils::createLocToLineGrid(boundary_inside_minimum, comb_boundary_offset))
+    , inside_loc_to_line_optimal(PolygonUtils::createLocToLineGrid(boundary_inside_optimal, comb_boundary_offset))


⚠️ readability-function-size ⚠️
function calc exceeds recommended size/complexity thresholds

github-actions · 2023-07-20T13:36:08Z

src/pathPlanning/Comb.cpp

 {
 }

-bool Comb::calc
-(
+bool Comb::calc(
    bool perform_z_hops,
    bool perform_z_hops_only_when_collides,


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter _start_inside

Suggested change

bool perform_z_hops_only_when_collides,

bool start_inside,

github-actions · 2023-07-20T13:36:08Z

src/pathPlanning/Comb.cpp

 {
-    if(shorterThen(end_point - start_point, max_comb_distance_ignored))
+    if (shorterThen(end_point - start_point, max_comb_distance_ignored))
    {
        return true;
    }


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter _start_inside

Suggested change

}

const bool start_inside = moveInside(boundary_inside_optimal, start_inside, inside_loc_to_line_optimal.get(), start_point, start_inside_poly);

github-actions

⚠️ clang-tidy found issue(s) with the introduced code (9/10)

github-actions · 2023-07-20T13:36:21Z

src/pathPlanning/Comb.cpp

+            *inside_loc_to_line_optimal,
+            start_point,
+            end_point,
+            comb_paths.back(),


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter _start_inside

Suggested change

comb_paths.back(),

const bool start_inside_min = moveInside(boundary_inside_minimum, start_inside, inside_loc_to_line_minimum.get(), start_point, start_inside_poly_min);

github-actions · 2023-07-20T13:36:21Z

src/pathPlanning/Comb.cpp

 {
 }

-bool Comb::calc
-(
+bool Comb::calc(
    bool perform_z_hops,
    bool perform_z_hops_only_when_collides,
    const ExtruderTrain& train, // NOTE: USe for travel settings and 'extruder-nr' only, don't use for z-hop/retraction/wipe settings, as that should also be settable per mesh!


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter _end_inside

Suggested change

const ExtruderTrain& train, // NOTE: USe for travel settings and 'extruder-nr' only, don't use for z-hop/retraction/wipe settings, as that should also be settable per mesh!

bool end_inside,

github-actions · 2023-07-20T13:36:22Z

src/pathPlanning/Comb.cpp

    {
        return true;
    }
    const Point travel_end_point_before_combing = end_point;
-    //Move start and end point inside the optimal comb boundary
+    // Move start and end point inside the optimal comb boundary
    unsigned int start_inside_poly = NO_INDEX;


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter _end_inside

Suggested change

unsigned int start_inside_poly = NO_INDEX;

const bool end_inside = moveInside(boundary_inside_optimal, end_inside, inside_loc_to_line_optimal.get(), end_point, end_inside_poly);

github-actions · 2023-07-20T13:36:22Z

src/pathPlanning/Comb.cpp

+            comb_paths.back(),
+            -offset_dist_to_get_from_on_the_polygon_to_outside,
+            max_comb_distance_ignored,
+            fail_on_unavoidable_obstacles);


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter _end_inside

Suggested change

fail_on_unavoidable_obstacles);

const bool end_inside_min = moveInside(boundary_inside_minimum, end_inside, inside_loc_to_line_minimum.get(), end_point, end_inside_poly_min);

github-actions · 2023-07-20T13:36:22Z

src/pathPlanning/Comb.cpp

+            combing_succeeded = LinePolygonsCrossings::comb(
+                end_crossing.dest_part,
+                *inside_loc_to_line_minimum,
+                end_crossing.in_or_mid,


⚠️ readability-make-member-function-const ⚠️
method moveInside can be made const

Suggested change

end_crossing.in_or_mid,

bool Comb::moveInside(Polygons& boundary_inside, bool is_inside, LocToLineGrid* inside_loc_to_line, Point& dest_point, unsigned int& inside_poly) const

github-actions · 2023-07-20T13:36:22Z

src/pathPlanning/Comb.cpp

+                comb_paths.back(),
+                -offset_dist_to_get_from_on_the_polygon_to_outside,
+                max_comb_distance_ignored,
+                fail_on_unavoidable_obstacles);
        }
        // If the endpoint of the travel path changes with combing, then it means that we are moving to an outer wall
        // and we should unretract before the last travel move when traveling to that outer wall
        unretract_before_last_travel_move = combing_succeeded && end_point != travel_end_point_before_combing;


⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change

unretract_before_last_travel_move = combing_succeeded && end_point != travel_end_point_before_combing;

github-actions · 2023-07-20T13:36:22Z

src/pathPlanning/Comb.cpp

        }
        // If the endpoint of the travel path changes with combing, then it means that we are moving to an outer wall
        // and we should unretract before the last travel move when traveling to that outer wall
        unretract_before_last_travel_move = combing_succeeded && end_point != travel_end_point_before_combing;
-        if (!combing_succeeded)
-        { // Couldn't comb between end point and computed crossing to the end part! Happens for very thin parts when the offset_to_get_off_boundary moves points to outside the polygon
+        if (! combing_succeeded)


⚠️ llvm-else-after-return ⚠️
do not use else after return

Suggested change

if (! combing_succeeded)

inside_poly = cpp.poly_idx;

return true;

github-actions · 2023-07-20T13:36:23Z

src/pathPlanning/Comb.cpp

@@ -318,7 +383,7 @@ void Comb::moveCombPathInside(Polygons& boundary_inside, Polygons& boundary_insi
        return;
    }
    comb_path_output.push_back(comb_path_input[0]);


⚠️ readability-identifier-naming ⚠️
invalid case style for parameter partsView_inside

Suggested change

comb_path_output.push_back(comb_path_input[0]);

for (unsigned int poly_idx : parts_view_inside[dest_part_idx])

github-actions · 2023-07-20T13:36:23Z

src/pathPlanning/Comb.cpp

@@ -336,28 +401,35 @@ void Comb::moveCombPathInside(Polygons& boundary_inside, Polygons& boundary_insi
    {
        comb_path_output.push_back(comb_path_input[comb_path_input.size() - 1]);


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from unsigned int to signed type int is implementation-defined

github-actions · 2023-07-20T13:36:23Z

src/pathPlanning/Comb.cpp

@@ -336,28 +401,35 @@ void Comb::moveCombPathInside(Polygons& boundary_inside, Polygons& boundary_insi
    {
        comb_path_output.push_back(comb_path_input[comb_path_input.size() - 1]);


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from unsigned int to signed type int is implementation-defined

github-actions

⚠️ clang-tidy found issue(s) with the introduced code (10/10)

github-actions · 2023-07-20T13:36:35Z

src/pathPlanning/Comb.cpp

+            [_dest_point](Point candidate)
+            {
+                return vSize2((candidate - _dest_point) / 10);
+            });
        dest_part = partsView_inside.assemblePart(dest_part_idx);



⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from cura::coord_t (aka long long) to signed type int is implementation-defined

github-actions · 2023-07-20T13:36:35Z

src/pathPlanning/Comb.cpp

+            [_dest_point](Point candidate)
+            {
+                return vSize2((candidate - _dest_point) / 10);
+            });
        dest_part = partsView_inside.assemblePart(dest_part_idx);



⚠️ readability-static-accessed-through-instance ⚠️
static member accessed through instance

Suggested change

in_or_mid = PolygonUtils::moveInside(best->first, cura::Comb::offset_dist_to_get_from_on_the_polygon_to_outside);

github-actions · 2023-07-20T13:36:36Z

src/pathPlanning/Comb.cpp

+            [_dest_point](Point candidate)
+            {
+                return vSize2((candidate - _dest_point) / 10);
+            });
        dest_part = partsView_inside.assemblePart(dest_part_idx);

        ClosestPolygonPoint boundary_crossing_point;


⚠️ bugprone-narrowing-conversions ⚠️
narrowing conversion from cura::coord_t (aka long long) to signed type int is implementation-defined

github-actions · 2023-07-20T13:36:36Z

src/pathPlanning/Comb.cpp

+            [_dest_point](Point candidate)
+            {
+                return vSize2((candidate - _dest_point) / 10);
+            });
        dest_part = partsView_inside.assemblePart(dest_part_idx);

        ClosestPolygonPoint boundary_crossing_point;


⚠️ readability-static-accessed-through-instance ⚠️
static member accessed through instance

Suggested change

ClosestPolygonPoint boundary_crossing_point;

out = PolygonUtils::moveOutside(best->second, cura::Comb::offset_dist_to_get_from_on_the_polygon_to_outside);

github-actions · 2023-07-20T13:36:36Z

src/pathPlanning/Comb.cpp

                = [close_to, _dest_point, &boundary_crossing_point, &dist2_score, &dest_part_poly_indices](const PolygonsPointIndex& boundary_segment)
+            {
+                if (dest_part_poly_indices.find(boundary_segment.poly_idx) == dest_part_poly_indices.end())


⚠️ performance-unnecessary-value-param ⚠️
the parameter from is copied for each invocation but only used as a const reference; consider making it a const reference

Suggested change

if (dest_part_poly_indices.find(boundary_segment.poly_idx) == dest_part_poly_indices.end())

std::shared_ptr<std::pair<ClosestPolygonPoint, ClosestPolygonPoint>> Comb::Crossing::findBestCrossing(const ExtruderTrain& train, const Polygons& outside, const ConstPolygonRef from, const Point estimated_start, const Point estimated_end, Comb& comber)

github-actions · 2023-07-20T13:36:36Z

src/pathPlanning/Comb.cpp

                = [close_to, _dest_point, &boundary_crossing_point, &dist2_score, &dest_part_poly_indices](const PolygonsPointIndex& boundary_segment)
+            {
+                if (dest_part_poly_indices.find(boundary_segment.poly_idx) == dest_part_poly_indices.end())


⚠️ performance-unnecessary-value-param ⚠️
the parameter from is copied for each invocation but only used as a const reference; consider making it a const reference

Suggested change

if (dest_part_poly_indices.find(boundary_segment.poly_idx) == dest_part_poly_indices.end())

std::shared_ptr<std::pair<ClosestPolygonPoint, ClosestPolygonPoint>> Comb::Crossing::findBestCrossing(const ExtruderTrain& train, const Polygons& outside, ConstPolygonRef& from, const Point estimated_start, const Point estimated_end, Comb& comber)

github-actions · 2023-07-20T13:36:36Z

src/pathPlanning/Comb.cpp

+                    return true; // a.k.a. continue;
+                }
+                Point closest_here = LinearAlg2D::getClosestOnLineSegment(close_to, boundary_segment.p(), boundary_segment.next().p());
+                coord_t dist2_score_here = vSize2(close_to - closest_here) + vSize2(_dest_point - closest_here) / 10;


⚠️ cppcoreguidelines-init-variables ⚠️
variable best_crossing_dist2 is not initialized

Suggested change

coord_t dist2_score_here = vSize2(close_to - closest_here) + vSize2(_dest_point - closest_here) / 10;

coord_t best_crossing_dist2 = 0;

github-actions · 2023-07-20T13:36:37Z

src/pathPlanning/Comb.cpp

@@ -435,7 +518,11 @@ bool Comb::Crossing::findOutside(const ExtruderTrain& train, const Polygons& out
    if (dest_is_inside || outside.inside(in_or_mid, true)) // start in_between


⚠️ modernize-return-braced-init-list ⚠️
avoid repeating the return type from the declaration; use a braced initializer list instead

Suggested change

if (dest_is_inside || outside.inside(in_or_mid, true)) // start in_between

return {);

github-actions · 2023-07-20T13:36:37Z

src/pathPlanning/Comb.cpp

@@ -435,7 +518,11 @@ bool Comb::Crossing::findOutside(const ExtruderTrain& train, const Polygons& out
    if (dest_is_inside || outside.inside(in_or_mid, true)) // start in_between


⚠️ modernize-return-braced-init-list ⚠️
avoid repeating the return type from the declaration; use a braced initializer list instead

Suggested change

if (dest_is_inside || outside.inside(in_or_mid, true)) // start in_between

return std::shared_ptr<std::pair<ClosestPolygonPoint, ClosestPolygonPoint>>(};

# Conflicts: # src/SkirtBrim.cpp

saumyaj3 added 2 commits July 14, 2023 13:06

Skirt Height for individual extruder instead of using the global setting

724d35d

CURA-10728

Skirt height adjusted for multiple extruders.

685bc4b

CURA-10728

nallath approved these changes Jul 17, 2023

View reviewed changes

casperlamboo reviewed Jul 17, 2023

View reviewed changes

include/utils/actions/smooth.h Show resolved Hide resolved

onitake and others added 5 commits July 18, 2023 16:35

Replace size_t format string in InfillTest.cpp

6338fba

This will fix an incorrect format causing test failure on 32-bit architectures.

Replace custom sprintf formatter with fmt::format

bc050f2

Use correct fmt syntax

bb12882

Added robustness for performance.

0f22953

If user tries to give skirt height more than the model_height. CURA-10728

Merge branch 'main' into CURA-10728_skirt_height_for_skirt_outline

7a39dcd

casperlamboo reviewed Jul 19, 2023

View reviewed changes

saumyaj3 and others added 3 commits July 19, 2023 17:32

Fixes based on review comments

4169359

CURA-10728

Merge branch 'main' into CURA-10728_skirt_height_for_skirt_outline

74687b8

Applied clang-format.

b8a6e9b

github-actions bot requested changes Jul 20, 2023

View reviewed changes

casperlamboo approved these changes Jul 24, 2023

View reviewed changes

Merge branch 'main' into CURA-10728_skirt_height_for_skirt_outline

8c08af0

# Conflicts: # src/SkirtBrim.cpp

casperlamboo merged commit 1c016a0 into main Jul 24, 2023
5 checks passed

saumyaj3 deleted the CURA-10728_skirt_height_for_skirt_outline branch August 3, 2023 14:50

		skirt_height = std::min(std::max(skirt_height, extruder.settings.get<int>("skirt_height")), static_cast<int>(storage.print_layer_count));
		}

-            skirt_height = std::min(std::max(skirt_height, extruder.settings.get<int>("skirt_height")), static_cast<int>(storage.print_layer_count));
-        }
+            skirt_height = std::max(skirt_height, extruder.settings.get<int>("skirt_height"));
+        }
+        skirt_height = std::min(skirt_height, static_cast<int>(storage.print_layer_count));

	template<class Point>
	constexpr auto isWithinAllowedDeviations(Point* a, Point* B, Point* C, Point* D, const utils::floating_point auto fluid_angle, const utils::integral auto max_resolution,

	const utils::floating_point auto fluid_angle,
	const double cos_A = std::acos(cosAngle(a, B, C, AB_magnitude, BC_magnitude));

	const utils::integral auto max_resolution,
	const double cos_B = std::acos(cosAngle(a, B, D, AB_magnitude, CD_magnitude));

	requires utils::point2d<Point> \|\| utils::junction<Point>
	const utils::floating_point auto ab_magnitude, const utils::floating_point auto BC_magnitude, const utils::floating_point auto CD_magnitude) const noexcept

	Point* A,
	if (bc_magnitude > max_resolution / 10) // TODO: make dedicated front-end setting for this

	const utils::floating_point auto AB_magnitude,
	const auto abs_angle = std::abs(cos_a - cos_B);

	#include "sliceDataStorage.h"
	#include "settings/EnumSettings.h"

	#include "utils/PolylineStitcher.h"
	#include "sliceDataStorage.h"

		#include "utils/PolylineStitcher.h"
		#include "utils/Simplify.h" //Simplifying the brim/skirt at every inset.

	#include "utils/Simplify.h" //Simplifying the brim/skirt at every inset.
	#include "support.h"

	#include <spdlog/spdlog.h>
	#include "utils/Simplify.h" //Simplifying the brim/skirt at every inset.

	, has_ooze_shield(storage.oozeShield.size() > 0 && storage.oozeShield[0].size() > 0)
	has_ooze_shield(!storage.oozeShield.empty() && storage.oozeShield[0].size() > 0),

	, has_ooze_shield(storage.oozeShield.size() > 0 && storage.oozeShield[0].size() > 0)
	has_ooze_shield(storage.oozeShield.size() > 0 && !storage.oozeShield[0].empty()),

	, has_draft_shield(storage.draft_protection_shield.size() > 0)
	has_draft_shield(!storage.draft_protection_shield.empty()),

	, extruder_is_used(storage.getExtrudersUsed())
	extruder_is_used(storage.getExtrudersUsed()), first_used_extruder_nr(0)

	const coord_t maximum_resolution = global_settings.get<coord_t>("meshfix_maximum_resolution");
	const auto maximum_resolution = global_settings.get<coord_t>("meshfix_maximum_resolution");

	{
	total_length[offset.extruder_nr] > 0U // No lines got added; we have no extrusion lines to build on

		@@ -543,7 +544,8 @@ void SkirtBrim::generateShieldBrim(Polygons& brim_covered_area, std::vector<Poly
		while (shield_brim.size() > 0)

Cura 10728 skirt height for skirt outline #1904

Cura 10728 skirt height for skirt outline #1904

Conversation

saumyaj3 commented Jul 14, 2023

Description

Type of change

How Has This Been Tested?

Checklist:

github-actions bot commented Jul 14, 2023 • edited Loading

Unit Test Results

casperlamboo Jul 19, 2023

Choose a reason for hiding this comment

casperlamboo Jul 19, 2023

Choose a reason for hiding this comment

casperlamboo Jul 19, 2023

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot Jul 20, 2023

Choose a reason for hiding this comment

github-actions bot commented Jul 14, 2023 •

edited

Loading

		@@ -588,19 +590,19 @@ void SkirtBrim::generateSecondarySkirtBrim(Polygons& covered_area, std::vector<P
		}

	#include "pathPlanning/LinePolygonsCrossings.h"
	#include "Application.h"

		#include "sliceDataStorage.h"
		#include "utils/PolygonsPointIndex.h"

	#include "utils/linearAlg2D.h"
	#include "pathPlanning/LinePolygonsCrossings.h"

	const Polygons& comb_boundary_inside_minimum,
	Comb::Comb(const SliceDataStorage& storage, const LayerIndex layer_nr, Polygons comb_boundary_inside_minimum, const Polygons& comb_boundary_inside_optimal, coord_t comb_boundary_offset, coord_t travel_avoid_distance, coord_t move_inside_distance)

	, offset_from_outlines(comb_boundary_offset) // between second wall and infill / other walls
	, boundary_inside_minimum(std::move( comb_boundary_inside_minimum ) // copy the boundary, because the partsView_inside will reorder the polygons

	, offset_from_outlines(comb_boundary_offset) // between second wall and infill / other walls
	, boundary_inside_minimum( comb_boundary_inside_minimum )) // copy the boundary, because the partsView_inside will reorder the polygons

	, max_moveInside_distance2(offset_from_outlines * offset_from_outlines)
	, boundary_inside_optimal(std::move( comb_boundary_inside_optimal ) // copy the boundary, because the partsView_inside will reorder the polygons

	, max_moveInside_distance2(offset_from_outlines * offset_from_outlines)
	, boundary_inside_optimal( comb_boundary_inside_optimal )) // copy the boundary, because the partsView_inside will reorder the polygons

	}
	const bool start_inside = moveInside(boundary_inside_optimal, start_inside, inside_loc_to_line_optimal.get(), start_point, start_inside_poly);