Skip to content

Commit

Permalink
Fixed a bug in Clipper.Offset
Browse files Browse the repository at this point in the history 
(was too agressive in removing 'over-shrunk' paths)
  • Loading branch information
@AngusJohnson
AngusJohnson committed Mar 16, 2024
1 parent b3eabae commit 0e27e16
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 125 deletions.
5 changes: 1 addition & 4 deletions CPP/Clipper2Lib/include/clipper2/clipper.offset.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ class ClipperOffset {
class Group {
public:
Paths64 paths_in;
std::vector<bool> is_hole_list;
std::vector<double> areas_list;
//std::vector<Rect64> bounds_list;
int lowest_path_idx = -1;
bool is_reversed = false;
JoinType join_type;
Expand Down Expand Up @@ -75,7 +72,7 @@ class ClipperOffset {
void DoMiter(const Path64& path, size_t j, size_t k, double cos_a);
void DoRound(const Path64& path, size_t j, size_t k, double angle);
void BuildNormals(const Path64& path);
void OffsetPolygon(Group& group, const Path64& path, bool is_shrinking, double area);
void OffsetPolygon(Group& group, const Path64& path);
void OffsetOpenJoined(Group& group, const Path64& path);
void OffsetOpenPath(Group& group, const Path64& path);
void OffsetPoint(Group& group, const Path64& path, size_t j, size_t k);
Expand Down
42 changes: 8 additions & 34 deletions CPP/Clipper2Lib/src/clipper.offset.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,31 +125,19 @@ ClipperOffset::Group::Group(const Paths64& _paths, JoinType _join_type, EndType

if (end_type == EndType::Polygon)
{
is_hole_list.reserve(paths_in.size());
areas_list.reserve(paths_in.size());
for (const Path64& path : paths_in)
{
double a = Area(path);
areas_list.push_back(a);
is_hole_list.push_back(a < 0);
}
lowest_path_idx = GetLowestClosedPathIdx(paths_in);
// 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) && is_hole_list[lowest_path_idx];
if (is_reversed) is_hole_list.flip();
is_reversed = (lowest_path_idx >= 0) && Area(paths_in[lowest_path_idx]) < 0;
}
else
{
lowest_path_idx = -1;
is_reversed = false;
is_hole_list.resize(paths_in.size());
areas_list.resize(paths_in.size());
}
}


//------------------------------------------------------------------------------
// ClipperOffset methods
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -353,24 +341,17 @@ void ClipperOffset::OffsetPoint(Group& group, const Path64& path, size_t j, size
DoSquare(path, j, k);
}

void ClipperOffset::OffsetPolygon(Group& group, const Path64& path, bool is_shrinking, double area)
void ClipperOffset::OffsetPolygon(Group& group, const Path64& path)
{
path_out.clear();
for (Path64::size_type j = 0, k = path.size() - 1; j < path.size(); k = j, ++j)
OffsetPoint(group, path, j, k);

// make sure that polygon areas aren't reversing which would indicate
// that the polygon has shrunk too far and that it should be discarded.
// See also - #593 & #715
if (is_shrinking && area // area == 0.0 when JoinType::Joined
&& ((area < 0) != (Area(path_out) < 0))) return;

OffsetPoint(group, path, j, k);
solution.push_back(path_out);
}

void ClipperOffset::OffsetOpenJoined(Group& group, const Path64& path)
{
OffsetPolygon(group, path, false, 0);
OffsetPolygon(group, path);
Path64 reverse_path(path);
std::reverse(reverse_path.begin(), reverse_path.end());

Expand All @@ -380,7 +361,7 @@ void ClipperOffset::OffsetOpenJoined(Group& group, const Path64& path)
norms.erase(norms.begin());
NegatePath(norms);

OffsetPolygon(group, reverse_path, true, 0);
OffsetPolygon(group, reverse_path);
}

void ClipperOffset::OffsetOpenPath(Group& group, const Path64& path)
Expand Down Expand Up @@ -477,17 +458,10 @@ void ClipperOffset::DoGroupOffset(Group& group)
steps_per_rad_ = steps_per_360 / (2 * PI);
}

double min_area = PI * Sqr(group_delta_);
std::vector<bool>::const_iterator is_hole_it = group.is_hole_list.cbegin();
std::vector<double>::const_iterator area_it = group.areas_list.cbegin();
//double min_area = PI * Sqr(group_delta_);
Paths64::const_iterator path_in_it = group.paths_in.cbegin();
for ( ; path_in_it != group.paths_in.cend(); ++path_in_it, ++is_hole_it, ++area_it)
for ( ; path_in_it != group.paths_in.cend(); ++path_in_it)
{
bool is_shrinking =
(group.end_type == EndType::Polygon) &&
(group.is_reversed == ((group_delta_ < 0) == *is_hole_it));
if (is_shrinking && (std::fabs(*area_it) < min_area)) continue;

Path64::size_type pathLen = path_in_it->size();
path_out.clear();

Expand Down Expand Up @@ -532,7 +506,7 @@ void ClipperOffset::DoGroupOffset(Group& group)
EndType::Square;

BuildNormals(*path_in_it);
if (end_type_ == EndType::Polygon) OffsetPolygon(group, *path_in_it, is_shrinking, *area_it);
if (end_type_ == EndType::Polygon) OffsetPolygon(group, *path_in_it);
else if (end_type_ == EndType::Joined) OffsetOpenJoined(group, *path_in_it);
else OffsetOpenPath(group, *path_in_it);
}
Expand Down
59 changes: 57 additions & 2 deletions CPP/Tests/TestOffsets.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#include <gtest/gtest.h>
#include "clipper2/clipper.offset.h"
#include "ClipFileLoad.h"
//#include "clipper.svg.utils.h"

using namespace Clipper2Lib;

TEST(Clipper2Tests, TestOffsets) {
std::ifstream ifs("Offsets.txt");
ASSERT_TRUE(ifs.good());
Expand Down Expand Up @@ -69,6 +70,7 @@ TEST(Clipper2Tests, TestOffsets2) { // see #448 & #456
EXPECT_GE(min_dist + 1, delta - arc_tol); // +1 for rounding errors
EXPECT_LE(solution[0].size(), 21);
}

TEST(Clipper2Tests, TestOffsets3) // see #424
{
Paths64 subjects = {{
Expand All @@ -94,6 +96,7 @@ TEST(Clipper2Tests, TestOffsets3) // see #424
Paths64 solution = InflatePaths(subjects, -209715, JoinType::Miter, EndType::Polygon);
EXPECT_LE(solution[0].size() - subjects[0].size(), 1);
}

TEST(Clipper2Tests, TestOffsets4) // see #482
{
Paths64 paths = { { {0, 0}, {20000, 200},
Expand Down Expand Up @@ -121,6 +124,7 @@ TEST(Clipper2Tests, TestOffsets4) // see #482
//std::cout << solution[0].size() << std::endl;
EXPECT_GT(solution[0].size(), 5);
}

TEST(Clipper2Tests, TestOffsets5) // modified from #593 (tests offset clean up)
{
Paths64 subject = {
Expand Down Expand Up @@ -348,6 +352,7 @@ TEST(Clipper2Tests, TestOffsets5) // modified from #593 (tests offset clean up)
Paths64 solution = InflatePaths(subject, -10000, JoinType::Round, EndType::Polygon);
EXPECT_EQ(solution.size(), 2);
}

TEST(Clipper2Tests, TestOffsets6) // also modified from #593 (tests rounded ends)
{
Paths64 subjects = {
Expand All @@ -370,6 +375,7 @@ TEST(Clipper2Tests, TestOffsets6) // also modified from #593 (tests rounded ends
double area = Area<int64_t>(solution[1]);
EXPECT_LT(area, -47500);
}

TEST(Clipper2Tests, TestOffsets7) // (#593 & #715)
{
Paths64 solution;
Expand All @@ -387,13 +393,15 @@ TEST(Clipper2Tests, TestOffsets7) // (#593 & #715)
solution = InflatePaths(subject, -50, JoinType::Miter, EndType::Polygon);
EXPECT_EQ(solution.size(), 0);
}

struct OffsetQual
{
PointD smallestInSub; // smallestInSub & smallestInSol are the points in subject and solution
PointD smallestInSol; // that define the place that most falls short of the expected offset
PointD largestInSub; // largestInSub & largestInSol are the points in subject and solution
PointD largestInSol; // that define the place that most exceeds the expected offset
};

template<typename T>
inline PointD GetClosestPointOnSegment(const PointD& offPt,
const Point<T>& seg1, const Point<T>& seg2)
Expand All @@ -410,6 +418,7 @@ inline PointD GetClosestPointOnSegment(const PointD& offPt,
static_cast<double>(seg1.x) + (q * dx),
static_cast<double>(seg1.y) + (q * dy));
}

template<typename T>
static OffsetQual GetOffsetQuality(const Path<T>& subject, const Path<T>& solution, const double delta)
{
Expand Down Expand Up @@ -462,6 +471,7 @@ static OffsetQual GetOffsetQuality(const Path<T>& subject, const Path<T>& soluti
}
return oq;
}

TEST(Clipper2Tests, TestOffsets8) // (#724)
{
Paths64 subject = { MakePath({
Expand Down Expand Up @@ -567,6 +577,7 @@ TEST(Clipper2Tests, TestOffsets8) // (#724)
EXPECT_LE(offset - smallestDist - rounding_tolerance, arc_tol);
EXPECT_LE(largestDist - offset - rounding_tolerance, arc_tol);
}

TEST(Clipper2Tests, TestOffsets9) // (#733)
{
// solution orientations should match subject orientations UNLESS
Expand Down Expand Up @@ -602,4 +613,48 @@ TEST(Clipper2Tests, TestOffsets9) // (#733)
EXPECT_TRUE(IsPositive(solution[0]));
solution = InflatePaths(subject, -15, JoinType::Miter, EndType::Polygon);
EXPECT_EQ(solution.size(), 0);
}
}

TEST(Clipper2Tests, TestOffsets10) // see #715
{
Paths64 subjects = {
{{508685336, -435806096},
{509492982, -434729201},
{509615525, -434003092},
{509615525, 493372891},
{509206033, 494655198},
{508129138, 495462844},
{507403029, 495585387},
{-545800889, 495585387},
{-547083196, 495175895},
{-547890842, 494099000},
{-548013385, 493372891},
{-548013385, -434003092},
{-547603893, -435285399},
{-546526998, -436093045},
{-545800889, -436215588},
{507403029, -436215588}},
{{106954765, -62914568},
{106795129, -63717113},
{106340524, -64397478},
{105660159, -64852084},
{104857613, -65011720},
{104055068, -64852084},
{103374703, -64397478},
{102920097, -63717113},
{102760461, -62914568},
{102920097, -62112022},
{103374703, -61431657},
{104055068, -60977052},
{104857613, -60817416},
{105660159, -60977052},
{106340524, -61431657},
{106795129, -62112022}} };

Clipper2Lib::ClipperOffset offseter(2, 104857.61318750000);
Paths64 solution;
offseter.AddPaths(subjects, Clipper2Lib::JoinType::Round, Clipper2Lib::EndType::Polygon);
offseter.Execute(-2212495.6382562499, solution);
EXPECT_EQ(solution.size(), 2);
}

43 changes: 6 additions & 37 deletions CSharp/Clipper2Lib/Clipper.Offset.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ public class ClipperOffset
private class Group
{
internal Paths64 inPaths;
internal List<double> areasList;
internal List<bool> isHoleList;
internal JoinType joinType;
internal EndType endType;
internal bool pathsReversed;
Expand All @@ -56,29 +54,15 @@ public Group(Paths64 paths, JoinType joinType, EndType endType = EndType.Polygon

if (endType == EndType.Polygon)
{
isHoleList = new List<bool>(inPaths.Count);
areasList = new List<double>(inPaths.Count);

foreach (Path64 path in inPaths)
{
double a = Clipper.Area(path);
areasList.Add(a);
isHoleList.Add(a < 0);
}

lowestPathIdx = GetLowestPathIdx(inPaths);
// the lowermost path must be an outer path, so if its orientation is negative,
// then flag that the whole group is 'reversed' (will negate delta etc.)
// as this is much more efficient than reversing every path.
pathsReversed = (lowestPathIdx >= 0) && isHoleList[lowestPathIdx];
if (pathsReversed)
for (int i = 0; i < isHoleList.Count; i++) isHoleList[i] = !isHoleList[i];
pathsReversed = (lowestPathIdx >= 0) && (Clipper.Area(inPaths[lowestPathIdx]) < 0);
}
else
{
lowestPathIdx = -1;
isHoleList = new List<bool>(new bool[inPaths.Count]);
areasList = new List<double>(new double[inPaths.Count]);
pathsReversed = false;
}
}
Expand Down Expand Up @@ -599,29 +583,22 @@ private void OffsetPoint(Group group, Path64 path, int j, ref int k)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void OffsetPolygon(Group group, Path64 path, bool is_shrinking, double area)
private void OffsetPolygon(Group group, Path64 path)
{
pathOut = new Path64();
int cnt = path.Count, prev = cnt - 1;
for (int i = 0; i < cnt; i++)
OffsetPoint(group, path, i, ref prev);

// make sure that polygon areas aren't reversing which would indicate
// that the polygon has shrunk too far and that it should be discarded.
// See also - #593 & #715
if (is_shrinking && area != 0 && // area == 0.0 when JoinType.Joined
((area < 0) != (Clipper.Area(pathOut) < 0))) return;

_solution.Add(pathOut);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void OffsetOpenJoined(Group group, Path64 path)
{
OffsetPolygon(group, path, false, 0);
OffsetPolygon(group, path);
path = Clipper.ReversePath(path);
BuildNormals(path);
OffsetPolygon(group, path, true, 0);
OffsetPolygon(group, path);
}

private void OffsetOpenPath(Group group, Path64 path)
Expand Down Expand Up @@ -720,17 +697,9 @@ private void DoGroupOffset(Group group)

double min_area = Math.PI * Clipper.Sqr(_groupDelta);
using List<Path64>.Enumerator pathIt = group.inPaths.GetEnumerator();
using List<bool>.Enumerator isHoleIt = group.isHoleList.GetEnumerator();
using List<double>.Enumerator areaIt = group.areasList.GetEnumerator();
while (pathIt.MoveNext() && isHoleIt.MoveNext() && areaIt.MoveNext())
while (pathIt.MoveNext())
{
bool isShrinking =
(group.endType == EndType.Polygon) &&
(group.pathsReversed == ((_groupDelta < 0) == isHoleIt.Current));
if (isShrinking && (Math.Abs(areaIt.Current) < min_area)) continue;

Path64 p = pathIt.Current;
bool isHole = isHoleIt.Current;

pathOut = new Path64();
int cnt = p.Count;
Expand Down Expand Up @@ -776,7 +745,7 @@ private void DoGroupOffset(Group group)
EndType.Square;

BuildNormals(p);
if (_endType == EndType.Polygon) OffsetPolygon(group, p, isShrinking, areaIt.Current);
if (_endType == EndType.Polygon) OffsetPolygon(group, p);
else if (_endType == EndType.Joined) OffsetOpenJoined(group, p);
else OffsetOpenPath(group, p);
}
Expand Down
Loading

0 comments on commit 0e27e16

Please sign in to comment.