Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CURA-11359] sanitize code #1986

Merged
merged 50 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
5a4ca90
Fixed basic code compilation warnings
wawanbreton Nov 20, 2023
853d47c
Fixed many more warnings
wawanbreton Nov 20, 2023
b7445cf
Fixed variable shadowing warnings
wawanbreton Nov 20, 2023
def4713
Fixed numeric conversion warnings
wawanbreton Nov 20, 2023
7469dea
Fixed number conversions warnings
wawanbreton Nov 20, 2023
ffbb28b
Fixed more number conversion warnings
wawanbreton Nov 20, 2023
1a5baf9
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
14ace62
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
cdb108f
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
5e849dc
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
01b0324
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
f40e6c3
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
32d17c5
Fix variable shadowing warnings
wawanbreton Nov 21, 2023
38f4559
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
403061a
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
434d51b
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
39098a2
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
6720d7a
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
bdbdd17
Removed last occurences of M_PI
wawanbreton Nov 21, 2023
bfe91cb
Use double instead of float, unless explicitely necessary
wawanbreton Nov 21, 2023
eb461e4
Geometry classes naming consistency
wawanbreton Nov 21, 2023
db39ede
Geometry classes variables naming consistency
wawanbreton Nov 21, 2023
4763b3b
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
e52f91a
Fixed variable shadowing warnings
wawanbreton Nov 21, 2023
4bcc29c
Fixed shadowed variables warnings
wawanbreton Nov 21, 2023
c56e755
Fixed last variable shadowing warnings
wawanbreton Nov 22, 2023
ddfbb1e
Applied clang-format.
wawanbreton Nov 22, 2023
3b65d4b
Removed unused ClientSocket class
wawanbreton Nov 22, 2023
98ed771
Merge remote-tracking branch 'origin/main' into CURA-11359_sanitize_code
wawanbreton Nov 22, 2023
6c24ce4
Applied clang-format.
wawanbreton Nov 22, 2023
4556a18
Geometry classes naming consistency
wawanbreton Nov 22, 2023
c74f4bb
Fixed variable initialization ordering warnings
wawanbreton Nov 22, 2023
1c37545
Applied clang-format.
wawanbreton Nov 22, 2023
511971f
Fixed unused variables warnings
wawanbreton Nov 22, 2023
46f0f97
Fixed dangling reference warnings
wawanbreton Nov 22, 2023
4749362
Fixed numeric conversion warnings
wawanbreton Nov 22, 2023
fda3e1b
Fixed unit tests build
wawanbreton Nov 22, 2023
fd3b4dc
Applied clang-format.
wawanbreton Nov 22, 2023
f08aadf
Fixed conversion warning, with potential side-effects on combing
wawanbreton Nov 22, 2023
cdca811
Fixed crash
wawanbreton Nov 22, 2023
b7776f4
Upload unit-test report for Github action
wawanbreton Nov 22, 2023
7625e4f
Upload detailed test report only on Linux
wawanbreton Nov 22, 2023
50366ad
Force unit test report publishing
wawanbreton Nov 22, 2023
332b2f3
Fixed GCodeExport unit test
wawanbreton Nov 22, 2023
a336b22
Fixed some numeric conversion warnings
wawanbreton Nov 22, 2023
65f0d9b
Fixed minimum layer time application regression
wawanbreton Nov 23, 2023
1884fbc
Update src/utils/PolygonsPointIndex.cpp
wawanbreton Nov 30, 2023
4d20359
Update src/utils/ExtrusionLine.cpp
wawanbreton Nov 30, 2023
fad6bd5
Merge remote-tracking branch 'origin/main' into CURA-11359_sanitize_code
wawanbreton Dec 1, 2023
a33ad67
Use explicit `std::optional` utility functions
casperlamboo Dec 1, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/unit-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -141,5 +141,14 @@ jobs:
files: |
**/*.xml

- name: Upload the detailed tests report
uses: actions/upload-artifact@v3
if: ${{ always() }}
with:
name: LastTest.log
path: |
build/Release/Testing/Temporary/LastTest.log
retention-days: 5

Comment on lines +144 to +152
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be part of this PR? seems seperated to me

can it be removed?

Suggested change
- name: Upload the detailed tests report
uses: actions/upload-artifact@v3
if: ${{ always() }}
with:
name: LastTest.log
path: |
build/Release/Testing/Temporary/LastTest.log
retention-days: 5

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.. I thought this could be part of it, otherwise it would be lost. Should I make a mini-PR for it ?

- name: Conclusion
run: echo "Conclusion is ${{ fromJSON( steps.test-results.outputs.json ).conclusion }}"
8 changes: 3 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -130,22 +130,20 @@ set(engine_SRCS # Except main.cpp.
src/utils/ExtrusionJunction.cpp
src/utils/ExtrusionLine.cpp
src/utils/ExtrusionSegment.cpp
src/utils/FMatrix4x3.cpp
src/utils/gettime.cpp
src/utils/LinearAlg2D.cpp
src/utils/ListPolyIt.cpp
src/utils/Matrix4x3D.cpp
src/utils/MinimumSpanningTree.cpp
src/utils/Point3.cpp
src/utils/Point3LL.cpp
src/utils/PolygonConnector.cpp
src/utils/PolygonsPointIndex.cpp
src/utils/PolygonsSegmentIndex.cpp
src/utils/polygonUtils.cpp
src/utils/polygon.cpp
src/utils/PolylineStitcher.cpp
src/utils/ProximityPointLink.cpp
src/utils/Simplify.cpp
src/utils/SVG.cpp
src/utils/socket.cpp
src/utils/SquareGrid.cpp
src/utils/ThreadPool.cpp
src/utils/ToolpathVisualizer.cpp
Expand Down Expand Up @@ -278,4 +276,4 @@ endif ()

if (ENABLE_BENCHMARKS)
add_subdirectory(benchmark)
endif ()
endif ()
4 changes: 2 additions & 2 deletions conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ def requirements(self):
self.requires("boost/1.82.0")
self.requires("rapidjson/1.1.0")
self.requires("stb/20200203")
self.requires("spdlog/1.10.0")
self.requires("fmt/9.0.0")
self.requires("spdlog/1.12.0")
self.requires("fmt/10.1.1")
self.requires("range-v3/0.12.0")
self.requires("scripta/0.1.0@ultimaker/testing")
self.requires("neargye-semver/0.3.0")
Expand Down
20 changes: 10 additions & 10 deletions include/Application.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
#ifndef APPLICATION_H
#define APPLICATION_H

#include "utils/NoCopy.h"

#include <cassert>
#include <cstddef>
#include <string>

#include "utils/NoCopy.h"


namespace cura
{
Expand Down Expand Up @@ -38,19 +38,21 @@ class Application : NoCopy
* can assume that it is safe to access this without checking whether it is
* initialised.
*/
Communication* communication = nullptr;
Communication* communication_ = nullptr;

/*
* \brief The slice that is currently ongoing.
*
* If no slice has started yet, this will be a nullptr.
*/
Slice* current_slice = nullptr;
Slice* current_slice_ = nullptr;

/*!
* \brief ThreadPool with lifetime tied to Application
*/
ThreadPool* thread_pool = nullptr;
ThreadPool* thread_pool_ = nullptr;

std::string instance_uuid_;

/*!
* Gets the instance of this application class.
Expand Down Expand Up @@ -92,8 +94,6 @@ class Application : NoCopy
*/
void startThreadPool(int nworkers = 0);

std::string instance_uuid;

protected:
#ifdef ARCUS
/*!
Expand All @@ -120,13 +120,13 @@ class Application : NoCopy
/*
* \brief The number of arguments that the application was called with.
*/
size_t argc;
size_t argc_;

/*
* \brief An array of C strings containing the arguments that the
* application was called with.
*/
char** argv;
char** argv_;

/*!
* \brief Constructs a new Application instance.
Expand All @@ -147,4 +147,4 @@ class Application : NoCopy

} // namespace cura

#endif // APPLICATION_H
#endif // APPLICATION_H
23 changes: 14 additions & 9 deletions include/BeadingStrategy/BeadingStrategy.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

#include "../settings/types/Angle.h"
#include "../settings/types/Ratio.h" //For the wall transition threshold.
#include "../utils/IntPoint.h"
#include "../utils/Point2LL.h"

namespace cura
{
Expand Down Expand Up @@ -36,7 +36,12 @@ class BeadingStrategy
coord_t left_over; //! The distance not covered by any bead; gap area.
};

BeadingStrategy(coord_t optimal_width, Ratio wall_split_middle_threshold, Ratio wall_add_middle_threshold, coord_t default_transition_length, float transitioning_angle = pi_div(3));
BeadingStrategy(
coord_t optimal_width,
Ratio wall_split_middle_threshold,
Ratio wall_add_middle_threshold,
coord_t default_transition_length,
double transitioning_angle = std::numbers::pi / 3.0);

BeadingStrategy(const BeadingStrategy& other);

Expand Down Expand Up @@ -80,7 +85,7 @@ class BeadingStrategy
*
* Transitions are used to smooth out the jumps in integer bead count; the jumps turn into ramps which could be positioned relative to the jump location.
*/
virtual float getTransitionAnchorPos(coord_t lower_bead_count) const;
virtual double getTransitionAnchorPos(coord_t lower_bead_count) const;

/*!
* Get the locations in a bead count region where \ref BeadingStrategy::compute exhibits a bend in the widths.
Expand All @@ -99,21 +104,21 @@ class BeadingStrategy
AngleRadians getTransitioningAngle() const;

protected:
std::string name;
std::string name_;

coord_t optimal_width; //! Optimal bead width, nominal width off the walls in 'ideal' circumstances.
coord_t optimal_width_; //! Optimal bead width, nominal width off the walls in 'ideal' circumstances.

Ratio wall_split_middle_threshold; //! Threshold when a middle wall should be split into two, as a ratio of the optimal wall width.
Ratio wall_split_middle_threshold_; //! Threshold when a middle wall should be split into two, as a ratio of the optimal wall width.

Ratio wall_add_middle_threshold; //! Threshold when a new middle wall should be added between an even number of walls, as a ratio of the optimal wall width.
Ratio wall_add_middle_threshold_; //! Threshold when a new middle wall should be added between an even number of walls, as a ratio of the optimal wall width.

coord_t default_transition_length; //! The length of the region to smoothly transfer between bead counts
coord_t default_transition_length_; //! The length of the region to smoothly transfer between bead counts

/*!
* The maximum angle between outline segments smaller than which we are going to add transitions
* Equals 180 - the "limit bisector angle" from the paper
*/
AngleRadians transitioning_angle;
AngleRadians transitioning_angle_;
};

using BeadingStrategyPtr = std::unique_ptr<BeadingStrategy>;
Expand Down
8 changes: 3 additions & 5 deletions include/BeadingStrategy/BeadingStrategyFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@ namespace cura
class BeadingStrategyFactory
{
public:
static BeadingStrategyPtr makeStrategy
(
static BeadingStrategyPtr makeStrategy(
const coord_t preferred_bead_width_outer = MM2INT(0.5),
const coord_t preferred_bead_width_inner = MM2INT(0.5),
const coord_t preferred_transition_length = MM2INT(0.4),
const float transitioning_angle = M_PI / 4.0,
const double transitioning_angle = std::numbers::pi / 4.0,
const bool print_thin_walls = false,
const coord_t min_bead_width = 0,
const coord_t min_feature_size = 0,
Expand All @@ -27,8 +26,7 @@ class BeadingStrategyFactory
const coord_t max_bead_count = 0,
const coord_t outer_wall_offset = 0,
const int inward_distributed_center_wall_count = 2,
const Ratio minimum_variable_line_ratio = 0.5
);
const Ratio minimum_variable_line_ratio = 0.5);
};

} // namespace cura
Expand Down
16 changes: 8 additions & 8 deletions include/BeadingStrategy/DistributedBeadingStrategy.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,23 @@ namespace cura
class DistributedBeadingStrategy : public BeadingStrategy
{
protected:
float one_over_distribution_radius_squared; // (1 / distribution_radius)^2
double one_over_distribution_radius_squared_; // (1 / distribution_radius)^2

public:
/*!
* \param distribution_radius the radius (in number of beads) over which to distribute the discrepancy between the feature size and the optimal thickness
*/
DistributedBeadingStrategy
(
* \param distribution_radius the radius (in number of beads) over which to distribute the discrepancy between the feature size and the optimal thickness
*/
DistributedBeadingStrategy(
const coord_t optimal_width,
const coord_t default_transition_length,
const AngleRadians transitioning_angle,
const Ratio wall_split_middle_threshold,
const Ratio wall_add_middle_threshold,
const int distribution_radius
);
const int distribution_radius);

virtual ~DistributedBeadingStrategy() override {}
virtual ~DistributedBeadingStrategy() override
{
}

Beading compute(coord_t thickness, coord_t bead_count) const override;
coord_t getOptimalBeadCount(coord_t thickness) const override;
Expand Down
6 changes: 3 additions & 3 deletions include/BeadingStrategy/LimitedBeadingStrategy.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ class LimitedBeadingStrategy : public BeadingStrategy

coord_t getTransitioningLength(coord_t lower_bead_count) const override;

float getTransitionAnchorPos(coord_t lower_bead_count) const override;
double getTransitionAnchorPos(coord_t lower_bead_count) const override;

protected:
const coord_t max_bead_count;
const BeadingStrategyPtr parent;
const coord_t max_bead_count_;
const BeadingStrategyPtr parent_;
};


Expand Down
50 changes: 25 additions & 25 deletions include/BeadingStrategy/OuterWallInsetBeadingStrategy.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//Copyright (c) 2020 Ultimaker B.V.
//CuraEngine is released under the terms of the AGPLv3 or higher.
// Copyright (c) 2020 Ultimaker B.V.
// CuraEngine is released under the terms of the AGPLv3 or higher.

#ifndef OUTER_WALL_INSET_BEADING_STRATEGY_H
#define OUTER_WALL_INSET_BEADING_STRATEGY_H
Expand All @@ -8,28 +8,28 @@

namespace cura
{
/*
* This is a meta strategy that allows for the outer wall to be inset towards the inside of the model.
*/
class OuterWallInsetBeadingStrategy : public BeadingStrategy
{
public:
OuterWallInsetBeadingStrategy(coord_t outer_wall_offset, BeadingStrategyPtr parent);
virtual ~OuterWallInsetBeadingStrategy() = default;

Beading compute(coord_t thickness, coord_t bead_count) const override;
coord_t getOptimalThickness(coord_t bead_count) const override;
coord_t getTransitionThickness(coord_t lower_bead_count) const override;
coord_t getOptimalBeadCount(coord_t thickness) const override;
coord_t getTransitioningLength(coord_t lower_bead_count) const override;
virtual std::string toString() const;
private:
BeadingStrategyPtr parent;
coord_t outer_wall_offset;
};
/*
* This is a meta strategy that allows for the outer wall to be inset towards the inside of the model.
*/
class OuterWallInsetBeadingStrategy : public BeadingStrategy
{
public:
OuterWallInsetBeadingStrategy(coord_t outer_wall_offset, BeadingStrategyPtr parent);

virtual ~OuterWallInsetBeadingStrategy() = default;

Beading compute(coord_t thickness, coord_t bead_count) const override;

coord_t getOptimalThickness(coord_t bead_count) const override;
coord_t getTransitionThickness(coord_t lower_bead_count) const override;
coord_t getOptimalBeadCount(coord_t thickness) const override;
coord_t getTransitioningLength(coord_t lower_bead_count) const override;

virtual std::string toString() const;

private:
BeadingStrategyPtr parent_;
coord_t outer_wall_offset_;
};
} // namespace cura
#endif // OUTER_WALL_INSET_BEADING_STRATEGY_H
Loading
Loading