Skip to content

Commit

Permalink
Compile warnings cleanup, fixes for sign-conversion issues (#858)
Browse files Browse the repository at this point in the history
* CPP: BencMark compile fix

* CPP: Fixed sing-conversion compile warnings, unsigned path indices
  • Loading branch information
masbug authored Jun 23, 2024
1 parent ff37866 commit 623f0a2
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 34 deletions.
2 changes: 1 addition & 1 deletion CPP/BenchMark/GetIntersectPtBenchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ struct SetConsoleTextColor
public:
SetConsoleTextColor(TextColor color) : _color(color) {};

static friend std::ostream& operator<< (std::ostream& out, SetConsoleTextColor const& scc)
friend std::ostream& operator<< (std::ostream& out, SetConsoleTextColor const& scc)
{
return out << "\x1B[" << scc._color << "m";
}
Expand Down
2 changes: 1 addition & 1 deletion CPP/BenchMark/PointInPolygonBenchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ struct SetConsoleTextColor
public:
SetConsoleTextColor(ConsoleTextColor color) : _color(color) {};

static friend std::ostream& operator<< (std::ostream& out, SetConsoleTextColor const& scc)
friend std::ostream& operator<< (std::ostream& out, SetConsoleTextColor const& scc)
{
return out << "\x1B[" << scc._color << "m";
}
Expand Down
2 changes: 1 addition & 1 deletion CPP/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ if (NOT (CLIPPER2_USINGZ STREQUAL "OFF"))
if (MSVC)
target_compile_options(Clipper2Z PRIVATE /W4 /WX)
else()
target_compile_options(Clipper2Z PRIVATE -Wall -Wextra -Wpedantic -Werror -Wno-c++20-compat)
target_compile_options(Clipper2Z PRIVATE -Wall -Wextra -Wpedantic -Werror -Wno-c++20-compat -Wsign-conversion)
target_link_libraries(Clipper2Z PUBLIC -lm)
endif()
endif()
Expand Down
1 change: 1 addition & 0 deletions CPP/Clipper2Lib/include/clipper2/clipper.core.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <algorithm>
#include <climits>
#include <numeric>
#include <optional>
#include "clipper2/clipper.version.h"

namespace Clipper2Lib
Expand Down
8 changes: 4 additions & 4 deletions CPP/Clipper2Lib/include/clipper2/clipper.h
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ namespace Clipper2Lib {
}

template <typename T>
inline Path<T> Ellipse(const Rect<T>& rect, int steps = 0)
inline Path<T> Ellipse(const Rect<T>& rect, size_t steps = 0)
{
return Ellipse(rect.MidPoint(),
static_cast<double>(rect.Width()) *0.5,
Expand All @@ -591,20 +591,20 @@ namespace Clipper2Lib {

template <typename T>
inline Path<T> Ellipse(const Point<T>& center,
double radiusX, double radiusY = 0, int steps = 0)
double radiusX, double radiusY = 0, size_t steps = 0)
{
if (radiusX <= 0) return Path<T>();
if (radiusY <= 0) radiusY = radiusX;
if (steps <= 2)
steps = static_cast<int>(PI * sqrt((radiusX + radiusY) / 2));
steps = static_cast<size_t>(PI * sqrt((radiusX + radiusY) / 2));

double si = std::sin(2 * PI / steps);
double co = std::cos(2 * PI / steps);
double dx = co, dy = si;
Path<T> result;
result.reserve(steps);
result.push_back(Point<T>(center.x + radiusX, static_cast<double>(center.y)));
for (int i = 1; i < steps; ++i)
for (size_t i = 1; i < steps; ++i)
{
result.push_back(Point<T>(center.x + radiusX * dx, center.y + radiusY * dy));
double x = dx * co - dy * si;
Expand Down
2 changes: 1 addition & 1 deletion CPP/Clipper2Lib/include/clipper2/clipper.offset.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ClipperOffset {
class Group {
public:
Paths64 paths_in;
int lowest_path_idx = -1;
std::optional<size_t> lowest_path_idx{};
bool is_reversed = false;
JoinType join_type;
EndType end_type;
Expand Down
4 changes: 2 additions & 2 deletions CPP/Clipper2Lib/include/clipper2/clipper.rectclip.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ namespace Clipper2Lib
OutPt2List edges_[8]; // clockwise and counter-clockwise
std::vector<Location> start_locs_;
void CheckEdges();
void TidyEdges(int idx, OutPt2List& cw, OutPt2List& ccw);
void TidyEdges(size_t idx, OutPt2List& cw, OutPt2List& ccw);
void GetNextLocation(const Path64& path,
Location& loc, int& i, int highI);
Location& loc, size_t& i, size_t highI);
OutPt2* Add(Point64 pt, bool start_new = false);
void AddCorner(Location prev, Location curr);
void AddCorner(Location& loc, bool isClockwise);
Expand Down
4 changes: 2 additions & 2 deletions CPP/Clipper2Lib/src/clipper.engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -618,9 +618,9 @@ namespace Clipper2Lib {
std::vector<Vertex*>& vertexLists, LocalMinimaList& locMinList)
{
const auto total_vertex_count =
std::accumulate(paths.begin(), paths.end(), 0,
std::accumulate(paths.begin(), paths.end(), size_t(0),
[](const auto& a, const Path64& path)
{return a + static_cast<unsigned>(path.size()); });
{return a + path.size(); });
if (total_vertex_count == 0) return;

Vertex* vertices = new Vertex[total_vertex_count], * v = vertices;
Expand Down
14 changes: 7 additions & 7 deletions CPP/Clipper2Lib/src/clipper.offset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ const double floating_point_tolerance = 1e-12;
// Miscellaneous methods
//------------------------------------------------------------------------------

int GetLowestClosedPathIdx(const Paths64& paths)
std::optional<size_t> GetLowestClosedPathIdx(const Paths64& paths)
{
int result = -1;
std::optional<size_t> result;
Point64 botPt = Point64(INT64_MAX, INT64_MIN);
for (size_t i = 0; i < paths.size(); ++i)
{
for (const Point64& pt : paths[i])
{
if ((pt.y < botPt.y) ||
((pt.y == botPt.y) && (pt.x >= botPt.x))) continue;
result = static_cast<int>(i);
result = i;
botPt.x = pt.x;
botPt.y = pt.y;
}
Expand Down Expand Up @@ -129,11 +129,11 @@ ClipperOffset::Group::Group(const Paths64& _paths, JoinType _join_type, EndType
// the lowermost path must be an outer path, so if its orientation is negative,
// then flag the whole group is 'reversed' (will negate delta etc.)
// as this is much more efficient than reversing every path.
is_reversed = (lowest_path_idx >= 0) && Area(paths_in[lowest_path_idx]) < 0;
is_reversed = (lowest_path_idx.has_value()) && Area(paths_in[lowest_path_idx.value()]) < 0;
}
else
{
lowest_path_idx = -1;
lowest_path_idx = std::nullopt;
is_reversed = false;
}
}
Expand Down Expand Up @@ -441,7 +441,7 @@ void ClipperOffset::DoGroupOffset(Group& group)
{
// a straight path (2 points) can now also be 'polygon' offset
// where the ends will be treated as (180 deg.) joins
if (group.lowest_path_idx < 0) delta_ = std::abs(delta_);
if (!group.lowest_path_idx.has_value()) delta_ = std::abs(delta_);
group_delta_ = (group.is_reversed) ? -delta_ : delta_;
}
else
Expand Down Expand Up @@ -491,7 +491,7 @@ void ClipperOffset::DoGroupOffset(Group& group)
if (group.join_type == JoinType::Round)
{
double radius = abs_delta;
int steps = static_cast<int>(std::ceil(steps_per_rad_ * 2 * PI)); //#617
size_t steps = steps_per_rad_ > 0 ? static_cast<size_t>(std::ceil(steps_per_rad_ * 2 * PI)) : 0; //#617
path_out = Ellipse(pt, radius, radius, steps);
#ifdef USINGZ
for (auto& p : path_out) p.z = pt.z;
Expand Down
35 changes: 20 additions & 15 deletions CPP/Clipper2Lib/src/clipper.rectclip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,9 @@ namespace Clipper2Lib {
// this method is only called by InternalExecute.
// Later splitting & rejoining won't create additional op's,
// though they will change the (non-storage) results_ count.
int curr_idx = static_cast<int>(results_.size()) - 1;
size_t curr_idx = results_.size();
OutPt2* result;
if (curr_idx < 0 || start_new)
if (curr_idx == 0 || start_new)
{
result = &op_container_.emplace_back(OutPt2());
result->pt = pt;
Expand All @@ -332,6 +332,7 @@ namespace Clipper2Lib {
}
else
{
--curr_idx;
OutPt2* prevOp = results_[curr_idx];
if (prevOp->pt == pt) return prevOp;
result = &op_container_.emplace_back(OutPt2());
Expand All @@ -349,27 +350,27 @@ namespace Clipper2Lib {
void RectClip64::AddCorner(Location prev, Location curr)
{
if (HeadingClockwise(prev, curr))
Add(rect_as_path_[static_cast<int>(prev)]);
Add(rect_as_path_[static_cast<size_t>(prev)]);
else
Add(rect_as_path_[static_cast<int>(curr)]);
Add(rect_as_path_[static_cast<size_t>(curr)]);
}

void RectClip64::AddCorner(Location& loc, bool isClockwise)
{
if (isClockwise)
{
Add(rect_as_path_[static_cast<int>(loc)]);
Add(rect_as_path_[static_cast<size_t>(loc)]);
loc = GetAdjacentLocation(loc, true);
}
else
{
loc = GetAdjacentLocation(loc, false);
Add(rect_as_path_[static_cast<int>(loc)]);
Add(rect_as_path_[static_cast<size_t>(loc)]);
}
}

void RectClip64::GetNextLocation(const Path64& path,
Location& loc, int& i, int highI)
Location& loc, size_t& i, size_t highI)
{
switch (loc)
{
Expand Down Expand Up @@ -425,26 +426,30 @@ namespace Clipper2Lib {

void RectClip64::ExecuteInternal(const Path64& path)
{
int i = 0, highI = static_cast<int>(path.size()) - 1;
if (path.size() < 1)
return;

size_t highI = path.size() - 1;
Location prev = Location::Inside, loc;
Location crossing_loc = Location::Inside;
Location first_cross_ = Location::Inside;
if (!GetLocation(rect_, path[highI], loc))
{
i = highI - 1;
while (i >= 0 && !GetLocation(rect_, path[i], prev)) --i;
if (i < 0)
size_t i = highI;
while (i > 0 && !GetLocation(rect_, path[i - 1], prev))
--i;
if (i == 0)
{
// all of path must be inside fRect
for (const auto& pt : path) Add(pt);
return;
}
if (prev == Location::Inside) loc = Location::Inside;
i = 0;
}
Location startingLoc = loc;

///////////////////////////////////////////////////
size_t i = 0;
while (i <= highI)
{
prev = loc;
Expand Down Expand Up @@ -639,7 +644,7 @@ namespace Clipper2Lib {
}
}

void RectClip64::TidyEdges(int idx, OutPt2List& cw, OutPt2List& ccw)
void RectClip64::TidyEdges(size_t idx, OutPt2List& cw, OutPt2List& ccw)
{
if (ccw.empty()) return;
bool isHorz = ((idx == 1) || (idx == 3));
Expand Down Expand Up @@ -867,7 +872,7 @@ namespace Clipper2Lib {

ExecuteInternal(path);
CheckEdges();
for (int i = 0; i < 4; ++i)
for (size_t i = 0; i < 4; ++i)
TidyEdges(i, edges_[i * 2], edges_[i * 2 + 1]);

for (OutPt2*& op : results_)
Expand Down Expand Up @@ -924,7 +929,7 @@ namespace Clipper2Lib {
op_container_ = std::deque<OutPt2>();
start_locs_.clear();

int i = 1, highI = static_cast<int>(path.size()) - 1;
size_t i = 1, highI = path.size() - 1;

Location prev = Location::Inside, loc;
Location crossing_loc;
Expand Down

0 comments on commit 623f0a2

Please sign in to comment.