From 9299eb6cbf683ee1545c31dbf14c67a5c5def1ff Mon Sep 17 00:00:00 2001 From: angusj Date: Wed, 22 Nov 2023 21:06:59 +1000 Subject: [PATCH] Caution: Clipper.ReverseSolution and Clipper.PreserveCollinear now method calls in C++. Fixed an obscure clipping bug (#720) --- .../include/clipper2/clipper.core.h | 20 ++-- .../include/clipper2/clipper.engine.h | 10 +- .../include/clipper2/clipper.export.h | 18 ++-- CPP/Clipper2Lib/src/clipper.engine.cpp | 91 +++++++++---------- CPP/Clipper2Lib/src/clipper.offset.cpp | 10 +- CPP/Tests/TestPolygons.cpp | 16 +++- CPP/Tests/TestPolytreeUnion.cpp | 2 +- 7 files changed, 94 insertions(+), 73 deletions(-) diff --git a/CPP/Clipper2Lib/include/clipper2/clipper.core.h b/CPP/Clipper2Lib/include/clipper2/clipper.core.h index 0d9ec7b1..e3e9824c 100644 --- a/CPP/Clipper2Lib/include/clipper2/clipper.core.h +++ b/CPP/Clipper2Lib/include/clipper2/clipper.core.h @@ -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 * @@ -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; @@ -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); } @@ -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 @@ -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; } @@ -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 diff --git a/CPP/Clipper2Lib/include/clipper2/clipper.engine.h b/CPP/Clipper2Lib/include/clipper2/clipper.engine.h index fb95f69d..cc41eb03 100644 --- a/CPP/Clipper2Lib/include/clipper2/clipper.engine.h +++ b/CPP/Clipper2Lib/include/clipper2/clipper.engine.h @@ -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 * @@ -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; @@ -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 diff --git a/CPP/Clipper2Lib/include/clipper2/clipper.export.h b/CPP/Clipper2Lib/include/clipper2/clipper.export.h index 1afd88f6..dbaca63b 100644 --- a/CPP/Clipper2Lib/include/clipper2/clipper.export.h +++ b/CPP/Clipper2Lib/include/clipper2/clipper.export.h @@ -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) * @@ -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); @@ -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); @@ -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); @@ -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); diff --git a/CPP/Clipper2Lib/src/clipper.engine.cpp b/CPP/Clipper2Lib/src/clipper.engine.cpp index 14f978ed..9358b74b 100644 --- a/CPP/Clipper2Lib/src/clipper.engine.cpp +++ b/CPP/Clipper2Lib/src/clipper.engine.cpp @@ -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 * @@ -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)) { @@ -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) { @@ -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) } @@ -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 * @@ -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 = @@ -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) @@ -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); } @@ -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; @@ -2951,7 +2948,7 @@ 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 @@ -2959,7 +2956,7 @@ namespace Clipper2Lib { // 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)); } } @@ -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; } @@ -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)); } } @@ -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; } diff --git a/CPP/Clipper2Lib/src/clipper.offset.cpp b/CPP/Clipper2Lib/src/clipper.offset.cpp index b8ee4481..2c7b167a 100644 --- a/CPP/Clipper2Lib/src/clipper.offset.cpp +++ b/CPP/Clipper2Lib/src/clipper.offset.cpp @@ -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) * @@ -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 @@ -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_); diff --git a/CPP/Tests/TestPolygons.cpp b/CPP/Tests/TestPolygons.cpp index 91746065..068dd318 100644 --- a/CPP/Tests/TestPolygons.cpp +++ b/CPP/Tests/TestPolygons.cpp @@ -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}) }; -} \ No newline at end of file + 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); +} diff --git a/CPP/Tests/TestPolytreeUnion.cpp b/CPP/Tests/TestPolytreeUnion.cpp index ccda169d..4d53da87 100644 --- a/CPP/Tests/TestPolytreeUnion.cpp +++ b/CPP/Tests/TestPolytreeUnion.cpp @@ -20,7 +20,7 @@ TEST(Clipper2Tests, TestPolytreeUnion) { else { //because clipping ops normally return Positive solutions - clipper.ReverseSolution = true; + clipper.ReverseSolution(true); clipper.Execute(ClipType::Union, FillRule::Negative, solution, open_paths); }