From e41ddda34dc8b1ef598d38f97116a9b19241f0c9 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 31 Jul 2024 11:10:46 +0200 Subject: [PATCH 1/2] Remove early-out: Restore 'side-effects' of unioning with empty. This is actually actively wanted and used in the code (see the unionPolygons() signature, note the missing argument), to solve self-overlap and the like. The added early outs where blocking that use here, leading to hard to trace problems, like tree support stopping midway. CURA-12061 --- src/geometry/Shape.cpp | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/src/geometry/Shape.cpp b/src/geometry/Shape.cpp index ed5dffbbf0..7c52d249ca 100644 --- a/src/geometry/Shape.cpp +++ b/src/geometry/Shape.cpp @@ -177,18 +177,7 @@ Shape Shape::difference(const Polygon& other) const Shape Shape::unionPolygons(const Shape& other, ClipperLib::PolyFillType fill_type) const { - if (empty() && other.empty()) - { - return {}; - } - if (empty() && other.size() <= 1) - { - return other; - } - if (other.empty() && size() <= 1) - { - return *this; - } + // No early out, as shapes should be able to be 'unioned' with themselves, which will resolve certain issues like self-overlapping polygons. ClipperLib::Paths ret; ClipperLib::Clipper clipper(clipper_init); addPaths(clipper, ClipperLib::ptSubject); @@ -199,18 +188,7 @@ Shape Shape::unionPolygons(const Shape& other, ClipperLib::PolyFillType fill_typ Shape Shape::unionPolygons(const Polygon& polygon, ClipperLib::PolyFillType fill_type) const { - if (empty() && polygon.empty()) - { - return {}; - } - if (empty()) - { - return Shape(polygon); - } - if (polygon.empty() && size() <= 1) - { - return *this; - } + // No early out, as unioning even with another empty polygon has some beneficial side-effects, such as removing self-overlapping polygons. ClipperLib::Paths ret; ClipperLib::Clipper clipper(clipper_init); addPaths(clipper, ClipperLib::ptSubject); From 078c20a04b71d220fb9dc22f54c6a70230e43263 Mon Sep 17 00:00:00 2001 From: Remco Burema Date: Wed, 31 Jul 2024 11:22:08 +0200 Subject: [PATCH 2/2] Reintroduce _one_ of the early outs (_both_ empty should still be ok). CURA-12061 --- src/geometry/Shape.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/geometry/Shape.cpp b/src/geometry/Shape.cpp index 7c52d249ca..61d029c4e2 100644 --- a/src/geometry/Shape.cpp +++ b/src/geometry/Shape.cpp @@ -177,7 +177,11 @@ Shape Shape::difference(const Polygon& other) const Shape Shape::unionPolygons(const Shape& other, ClipperLib::PolyFillType fill_type) const { - // No early out, as shapes should be able to be 'unioned' with themselves, which will resolve certain issues like self-overlapping polygons. + if (empty() && other.empty()) + { + return {}; + } + // No further early outs, as shapes should be able to be 'unioned' with themselves, which will resolve certain issues like self-overlapping polygons. ClipperLib::Paths ret; ClipperLib::Clipper clipper(clipper_init); addPaths(clipper, ClipperLib::ptSubject); @@ -188,7 +192,11 @@ Shape Shape::unionPolygons(const Shape& other, ClipperLib::PolyFillType fill_typ Shape Shape::unionPolygons(const Polygon& polygon, ClipperLib::PolyFillType fill_type) const { - // No early out, as unioning even with another empty polygon has some beneficial side-effects, such as removing self-overlapping polygons. + if (empty() && polygon.empty()) + { + return {}; + } + // No further early outs, as unioning even with another empty polygon has some beneficial side-effects, such as removing self-overlapping polygons. ClipperLib::Paths ret; ClipperLib::Clipper clipper(clipper_init); addPaths(clipper, ClipperLib::ptSubject);