Skip to content

Commit

Permalink
Caution: Clipper.ReverseSolution and Clipper.PreserveCollinear now me…
Browse files Browse the repository at this point in the history 
…thod calls in C++.

Fixed an obscure clipping bug (#720)
  • Loading branch information
@AngusJohnson
AngusJohnson committed Nov 22, 2023
1 parent 84d8012 commit 9299eb6
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 73 deletions.
20 changes: 13 additions & 7 deletions CPP/Clipper2Lib/include/clipper2/clipper.core.h
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*******************************************************************************
* Author : Angus Johnson *
* Date : 1 November 2023 *
* Date : 22 November 2023 *
* Website : http://www.angusj.com *
* Copyright : Angus Johnson 2010-2023 *
* Purpose : Core Clipper Library structures and functions *
Expand Down Expand Up @@ -43,13 +43,16 @@ namespace Clipper2Lib
"Invalid scale (either 0 or too large)";
static const char* non_pair_error =
"There must be 2 values for each coordinate";
static const char* undefined_error =
"There is an undefined error in Clipper2";
#endif

// error codes (2^n)
const int precision_error_i = 1; // non-fatal
const int scale_error_i = 2; // non-fatal
const int non_pair_error_i = 4; // non-fatal
const int range_error_i = 64;
const int precision_error_i = 1; // non-fatal
const int scale_error_i = 2; // non-fatal
const int non_pair_error_i = 4; // non-fatal
const int undefined_error_i = 32; // fatal
const int range_error_i = 64;

#ifndef PI
static const double PI = 3.141592653589793238;
Expand Down Expand Up @@ -80,6 +83,8 @@ namespace Clipper2Lib
throw Clipper2Exception(scale_error);
case non_pair_error_i:
throw Clipper2Exception(non_pair_error);
case undefined_error_i:
throw Clipper2Exception(undefined_error);
case range_error_i:
throw Clipper2Exception(range_error);
}
Expand All @@ -88,6 +93,7 @@ namespace Clipper2Lib
#endif
}


//By far the most widely used filling rules for polygons are EvenOdd
//and NonZero, sometimes called Alternate and Winding respectively.
//https://en.wikipedia.org/wiki/Nonzero-rule
Expand Down Expand Up @@ -144,7 +150,7 @@ namespace Clipper2Lib

friend std::ostream& operator<<(std::ostream& os, const Point& point)
{
os << point.x << "," << point.y << "," << point.z << " ";
os << point.x << "," << point.y << "," << point.z;
return os;
}

Expand Down Expand Up @@ -181,7 +187,7 @@ namespace Clipper2Lib

friend std::ostream& operator<<(std::ostream& os, const Point& point)
{
os << point.x << "," << point.y << " ";
os << point.x << "," << point.y;
return os;
}
#endif
Expand Down
10 changes: 7 additions & 3 deletions CPP/Clipper2Lib/include/clipper2/clipper.engine.h
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*******************************************************************************
* Author : Angus Johnson *
* Date : 25 October 2023 *
* Date : 22 November 2023 *
* Website : http://www.angusj.com *
* Copyright : Angus Johnson 2010-2023 *
* Purpose : This is the main polygon clipping module *
Expand Down Expand Up @@ -262,6 +262,8 @@ namespace Clipper2Lib {
inline void CheckJoinRight(Active& e,
const Point64& pt, bool check_curr_x = false);
protected:
bool preserve_collinear_ = true;
bool reverse_solution_ = false;
int error_code_ = 0;
bool has_open_paths_ = false;
bool succeeded_ = true;
Expand All @@ -281,8 +283,10 @@ namespace Clipper2Lib {
public:
virtual ~ClipperBase();
int ErrorCode() { return error_code_; };
bool PreserveCollinear = true;
bool ReverseSolution = false;
void PreserveCollinear(bool val) { preserve_collinear_ = val; };
bool PreserveCollinear() { return preserve_collinear_;};
void ReverseSolution(bool val) { reverse_solution_ = val; };
bool ReverseSolution() { return reverse_solution_; };
void Clear();
void AddReuseableData(const ReuseableDataContainer64& reuseable_data);
#ifdef USINGZ
Expand Down
18 changes: 9 additions & 9 deletions CPP/Clipper2Lib/include/clipper2/clipper.export.h
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*******************************************************************************
* Author : Angus Johnson *
* Date : 15 November 2023 *
* Date : 22 November 2023 *
* Website : http://www.angusj.com *
* Copyright : Angus Johnson 2010-2023 *
* Purpose : This module exports the Clipper2 Library (ie DLL/so) *
Expand Down Expand Up @@ -377,8 +377,8 @@ EXTERN_DLL_EXPORT int BooleanOp64(uint8_t cliptype,
clp = ConvertCPaths(clips);

Clipper64 clipper;
clipper.PreserveCollinear = preserve_collinear;
clipper.ReverseSolution = reverse_solution;
clipper.PreserveCollinear(preserve_collinear);
clipper.ReverseSolution(reverse_solution);
if (sub.size() > 0) clipper.AddSubject(sub);
if (sub_open.size() > 0) clipper.AddOpenSubject(sub_open);
if (clp.size() > 0) clipper.AddClip(clp);
Expand All @@ -404,8 +404,8 @@ EXTERN_DLL_EXPORT int BooleanOp_PolyTree64(uint8_t cliptype,

PolyTree64 tree;
Clipper64 clipper;
clipper.PreserveCollinear = preserve_collinear;
clipper.ReverseSolution = reverse_solution;
clipper.PreserveCollinear(preserve_collinear);
clipper.ReverseSolution(reverse_solution);
if (sub.size() > 0) clipper.AddSubject(sub);
if (sub_open.size() > 0) clipper.AddOpenSubject(sub_open);
if (clp.size() > 0) clipper.AddClip(clp);
Expand Down Expand Up @@ -434,8 +434,8 @@ EXTERN_DLL_EXPORT int BooleanOpD(uint8_t cliptype,
clp = ConvertCPathsDToPaths64(clips, scale);

Clipper64 clipper;
clipper.PreserveCollinear = preserve_collinear;
clipper.ReverseSolution = reverse_solution;
clipper.PreserveCollinear(preserve_collinear);
clipper.ReverseSolution(reverse_solution);
if (sub.size() > 0) clipper.AddSubject(sub);
if (sub_open.size() > 0) clipper.AddOpenSubject(sub_open);
if (clp.size() > 0) clipper.AddClip(clp);
Expand Down Expand Up @@ -466,8 +466,8 @@ EXTERN_DLL_EXPORT int BooleanOp_PolyTreeD(uint8_t cliptype,

PolyTree64 tree;
Clipper64 clipper;
clipper.PreserveCollinear = preserve_collinear;
clipper.ReverseSolution = reverse_solution;
clipper.PreserveCollinear(preserve_collinear);
clipper.ReverseSolution(reverse_solution);
if (sub.size() > 0) clipper.AddSubject(sub);
if (sub_open.size() > 0) clipper.AddOpenSubject(sub_open);
if (clp.size() > 0) clipper.AddClip(clp);
Expand Down
91 changes: 44 additions & 47 deletions CPP/Clipper2Lib/src/clipper.engine.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*******************************************************************************
* Author : Angus Johnson *
* Date : 1 November 2023 *
* Date : 22 November 2023 *
* Website : http://www.angusj.com *
* Copyright : Angus Johnson 2010-2023 *
* Purpose : This is the main polygon clipping module *
Expand Down Expand Up @@ -1543,7 +1543,7 @@ namespace Clipper2Lib {
//NB if preserveCollinear == true, then only remove 180 deg. spikes
if ((CrossProduct(op2->prev->pt, op2->pt, op2->next->pt) == 0) &&
(op2->pt == op2->prev->pt ||
op2->pt == op2->next->pt || !PreserveCollinear ||
op2->pt == op2->next->pt || !preserve_collinear_ ||
DotProduct(op2->prev->pt, op2->pt, op2->next->pt) < 0))
{

Expand Down Expand Up @@ -1708,6 +1708,28 @@ namespace Clipper2Lib {
return op;
}

inline void TrimHorz(Active& horzEdge, bool preserveCollinear)
{
bool wasTrimmed = false;
Point64 pt = NextVertex(horzEdge)->pt;
while (pt.y == horzEdge.top.y)
{
//always trim 180 deg. spikes (in closed paths)
//but otherwise break if preserveCollinear = true
if (preserveCollinear &&
((pt.x < horzEdge.top.x) != (horzEdge.bot.x < horzEdge.top.x)))
break;

horzEdge.vertex_top = NextVertex(horzEdge);
horzEdge.top = pt;
wasTrimmed = true;
if (IsMaxima(horzEdge)) break;
pt = NextVertex(horzEdge)->pt;
}

if (wasTrimmed) SetDx(horzEdge); // +/-infinity
}


inline void ClipperBase::UpdateEdgeIntoAEL(Active* e)
{
Expand All @@ -1719,9 +1741,13 @@ namespace Clipper2Lib {

if (IsJoined(*e)) Split(*e, e->bot);

if (IsHorizontal(*e)) return;
InsertScanline(e->top.y);
if (IsHorizontal(*e))
{
if (!IsOpen(*e)) TrimHorz(*e, preserve_collinear_);
return;
}

InsertScanline(e->top.y);
CheckJoinLeft(*e, e->bot);
CheckJoinRight(*e, e->bot, true); // (#500)
}
Expand Down Expand Up @@ -2453,35 +2479,6 @@ namespace Clipper2Lib {
}
}

inline bool HorzIsSpike(const Active& horzEdge)
{
Point64 nextPt = NextVertex(horzEdge)->pt;
return (nextPt.y == horzEdge.bot.y) &&
(horzEdge.bot.x < horzEdge.top.x) != (horzEdge.top.x < nextPt.x);
}

inline void TrimHorz(Active& horzEdge, bool preserveCollinear)
{
bool wasTrimmed = false;
Point64 pt = NextVertex(horzEdge)->pt;
while (pt.y == horzEdge.top.y)
{
//always trim 180 deg. spikes (in closed paths)
//but otherwise break if preserveCollinear = true
if (preserveCollinear &&
((pt.x < horzEdge.top.x) != (horzEdge.bot.x < horzEdge.top.x)))
break;

horzEdge.vertex_top = NextVertex(horzEdge);
horzEdge.top = pt;
wasTrimmed = true;
if (IsMaxima(horzEdge)) break;
pt = NextVertex(horzEdge)->pt;
}

if (wasTrimmed) SetDx(horzEdge); // +/-infinity
}

void ClipperBase::DoHorizontal(Active& horz)
/*******************************************************************************
* Notes: Horizontal edges (HEs) at scanline intersections (ie at the top or *
Expand All @@ -2507,10 +2504,10 @@ namespace Clipper2Lib {
else
vertex_max = GetCurrYMaximaVertex(horz);

// remove 180 deg.spikes and also simplify
// consecutive horizontals when PreserveCollinear = true
if (vertex_max && !horzIsOpen && vertex_max != horz.vertex_top)
TrimHorz(horz, PreserveCollinear);
//// remove 180 deg.spikes and also simplify
//// consecutive horizontals when PreserveCollinear = true
//if (!horzIsOpen && vertex_max != horz.vertex_top)
// TrimHorz(horz, PreserveCollinear);

int64_t horz_left, horz_right;
bool is_left_to_right =
Expand Down Expand Up @@ -2539,6 +2536,9 @@ namespace Clipper2Lib {
if (IsHotEdge(horz) && IsJoined(*e))
Split(*e, e->top);

//if (IsHotEdge(horz) != IsHotEdge(*e))
// DoError(undefined_error_i);

if (IsHotEdge(horz))
{
while (horz.vertex_top != vertex_max)
Expand Down Expand Up @@ -2637,9 +2637,6 @@ namespace Clipper2Lib {
AddOutPt(horz, horz.top);
UpdateEdgeIntoAEL(&horz);

if (PreserveCollinear && !horzIsOpen && HorzIsSpike(horz))
TrimHorz(horz, true);

is_left_to_right =
ResetHorzDirection(horz, vertex_max, horz_left, horz_right);
}
Expand Down Expand Up @@ -2876,7 +2873,7 @@ namespace Clipper2Lib {
if (!outrec->bounds.IsEmpty()) return true;
CleanCollinear(outrec);
if (!outrec->pts ||
!BuildPath64(outrec->pts, ReverseSolution, false, outrec->path)){
!BuildPath64(outrec->pts, reverse_solution_, false, outrec->path)){
return false;}
outrec->bounds = GetBounds(outrec->path);
return true;
Expand Down Expand Up @@ -2951,15 +2948,15 @@ namespace Clipper2Lib {
Path64 path;
if (solutionOpen && outrec->is_open)
{
if (BuildPath64(outrec->pts, ReverseSolution, true, path))
if (BuildPath64(outrec->pts, reverse_solution_, true, path))
solutionOpen->emplace_back(std::move(path));
}
else
{
// nb: CleanCollinear can add to outrec_list_
CleanCollinear(outrec);
//closed paths should always return a Positive orientation
if (BuildPath64(outrec->pts, ReverseSolution, false, path))
if (BuildPath64(outrec->pts, reverse_solution_, false, path))
solutionClosed.emplace_back(std::move(path));
}
}
Expand All @@ -2982,7 +2979,7 @@ namespace Clipper2Lib {
if (outrec->is_open)
{
Path64 path;
if (BuildPath64(outrec->pts, ReverseSolution, true, path))
if (BuildPath64(outrec->pts, reverse_solution_, true, path))
open_paths.push_back(path);
continue;
}
Expand Down Expand Up @@ -3059,14 +3056,14 @@ namespace Clipper2Lib {
PathD path;
if (solutionOpen && outrec->is_open)
{
if (BuildPathD(outrec->pts, ReverseSolution, true, path, invScale_))
if (BuildPathD(outrec->pts, reverse_solution_, true, path, invScale_))
solutionOpen->emplace_back(std::move(path));
}
else
{
CleanCollinear(outrec);
//closed paths should always return a Positive orientation
if (BuildPathD(outrec->pts, ReverseSolution, false, path, invScale_))
if (BuildPathD(outrec->pts, reverse_solution_, false, path, invScale_))
solutionClosed.emplace_back(std::move(path));
}
}
Expand All @@ -3088,7 +3085,7 @@ namespace Clipper2Lib {
if (outrec->is_open)
{
PathD path;
if (BuildPathD(outrec->pts, ReverseSolution, true, path, invScale_))
if (BuildPathD(outrec->pts, reverse_solution_, true, path, invScale_))
open_paths.push_back(path);
continue;
}
Expand Down
10 changes: 5 additions & 5 deletions CPP/Clipper2Lib/src/clipper.offset.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*******************************************************************************
* Author : Angus Johnson *
* Date : 19 November 2023 *
* Date : 22 November 2023 *
* Website : http://www.angusj.com *
* Copyright : Angus Johnson 2010-2023 *
* Purpose : Path Offset (Inflate/Shrink) *
Expand Down Expand Up @@ -669,9 +669,9 @@ void ClipperOffset::Execute(double delta, Paths64& paths)
bool paths_reversed = CheckReverseOrientation();
//clean up self-intersections ...
Clipper64 c;
c.PreserveCollinear = false;
c.PreserveCollinear(false);
//the solution should retain the orientation of the input
c.ReverseSolution = (reverse_solution_ != paths_reversed);
c.ReverseSolution(reverse_solution_ != paths_reversed);
#ifdef USINGZ
if (zCallback64_) { c.SetZCallback(zCallback64_); }
#endif
Expand All @@ -693,9 +693,9 @@ void ClipperOffset::Execute(double delta, PolyTree64& polytree)
bool paths_reversed = CheckReverseOrientation();
//clean up self-intersections ...
Clipper64 c;
c.PreserveCollinear = false;
c.PreserveCollinear(false);
//the solution should retain the orientation of the input
c.ReverseSolution = reverse_solution_ != paths_reversed;
c.ReverseSolution (reverse_solution_ != paths_reversed);
#ifdef USINGZ
if (zCallback64_) {
c.SetZCallback(zCallback64_);
Expand Down
16 changes: 15 additions & 1 deletion CPP/Tests/TestPolygons.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,19 @@ TEST(Clipper2Tests, TestMultiplePolygons)

Clipper2Lib::PathsD subjd, clipd, solutiond;
Clipper2Lib::FillRule frd = Clipper2Lib::FillRule::NonZero;
}

TEST(Clipper2Tests, TestHorzSpikes) //#720
{
Clipper2Lib::Paths64 paths = {
Clipper2Lib::MakePath({1600,0, 1600,100, 2050,100, 2050,300, 450,300, 450, 0}),
Clipper2Lib::MakePath({1800,200, 1800,100, 1600,100, 2000,100, 2000,200}) };

}
std::cout << paths << std::endl;

Clipper2Lib::Clipper64 c;
c.AddSubject(paths);
c.Execute(Clipper2Lib::ClipType::Union, Clipper2Lib::FillRule::NonZero, paths);

EXPECT_GE(paths.size(), 1);
}
Loading

0 comments on commit 9299eb6

Please sign in to comment.