From 52b3c655ffd43ba4ad2819729ece4485aa4346e5 Mon Sep 17 00:00:00 2001 From: Vojtech Bubnik Date: Fri, 7 May 2021 11:42:01 +0200 Subject: [PATCH] Fixed Polygon::centroid() Ported Polygon unit tests from Perl to C++. --- src/libslic3r/Polygon.cpp | 22 ++++--- src/libslic3r/Polyline.hpp | 3 + tests/libslic3r/test_polygon.cpp | 106 +++++++++++++++++++++++++++++++ xs/t/06_polygon.t | 78 +---------------------- 4 files changed, 122 insertions(+), 87 deletions(-) diff --git a/src/libslic3r/Polygon.cpp b/src/libslic3r/Polygon.cpp index 66aa224ed..9bfd359c2 100644 --- a/src/libslic3r/Polygon.cpp +++ b/src/libslic3r/Polygon.cpp @@ -155,17 +155,19 @@ void Polygon::triangulate_convex(Polygons* polygons) const // center of mass Point Polygon::centroid() const { - double area_temp = this->area(); - double x_temp = 0; - double y_temp = 0; - - Polyline polyline = this->split_at_first_point(); - for (Points::const_iterator point = polyline.points.begin(); point != polyline.points.end() - 1; ++point) { - x_temp += (double)( point->x() + (point+1)->x() ) * ( (double)point->x()*(point+1)->y() - (double)(point+1)->x()*point->y() ); - y_temp += (double)( point->y() + (point+1)->y() ) * ( (double)point->x()*(point+1)->y() - (double)(point+1)->x()*point->y() ); + double area_sum = 0.; + Vec2d c(0., 0.); + if (points.size() >= 3) { + Vec2d p1 = points.back().cast(); + for (const Point &p : points) { + Vec2d p2 = p.cast(); + double a = cross2(p1, p2); + area_sum += a; + c += (p1 + p2) * a; + p1 = p2; + } } - - return Point(x_temp/(6*area_temp), y_temp/(6*area_temp)); + return Point(Vec2d(c / (3. * area_sum))); } // find all concave vertices (i.e. having an internal angle greater than the supplied angle) diff --git a/src/libslic3r/Polyline.hpp b/src/libslic3r/Polyline.hpp index 88f910590..51dcf9d36 100644 --- a/src/libslic3r/Polyline.hpp +++ b/src/libslic3r/Polyline.hpp @@ -78,6 +78,9 @@ public: bool is_closed() const { return this->points.front() == this->points.back(); } }; +inline bool operator==(const Polyline &lhs, const Polyline &rhs) { return lhs.points == rhs.points; } +inline bool operator!=(const Polyline &lhs, const Polyline &rhs) { return lhs.points != rhs.points; } + // Don't use this class in production code, it is used exclusively by the Perl binding for unit tests! #ifdef PERL_UCHAR_MIN class PolylineCollection diff --git a/tests/libslic3r/test_polygon.cpp b/tests/libslic3r/test_polygon.cpp index d45e37fb1..f2c78cace 100644 --- a/tests/libslic3r/test_polygon.cpp +++ b/tests/libslic3r/test_polygon.cpp @@ -5,6 +5,112 @@ using namespace Slic3r; +SCENARIO("Converted Perl tests", "[Polygon]") { + GIVEN("ccw_square") { + Polygon ccw_square{ { 100, 100 }, { 200, 100 }, { 200, 200 }, { 100, 200 } }; + Polygon cw_square(ccw_square); + cw_square.reverse(); + + THEN("ccw_square is valid") { + REQUIRE(ccw_square.is_valid()); + } + THEN("cw_square is valid") { + REQUIRE(cw_square.is_valid()); + } + THEN("ccw_square.area") { + REQUIRE(ccw_square.area() == 100 * 100); + } + THEN("cw_square.area") { + REQUIRE(cw_square.area() == - 100 * 100); + } + THEN("ccw_square.centroid") { + REQUIRE(ccw_square.centroid() == Point { 150, 150 }); + } + THEN("cw_square.centroid") { + REQUIRE(cw_square.centroid() == Point { 150, 150 }); + } + THEN("ccw_square.contains_point(150, 150)") { + REQUIRE(ccw_square.contains({ 150, 150 })); + } + THEN("cw_square.contains_point(150, 150)") { + REQUIRE(cw_square.contains({ 150, 150 })); + } + THEN("conversion to lines") { + REQUIRE(ccw_square.lines() == Lines{ + { { 100, 100 }, { 200, 100 } }, + { { 200, 100 }, { 200, 200 } }, + { { 200, 200 }, { 100, 200 } }, + { { 100, 200 }, { 100, 100 } } }); + } + THEN("split_at_first_point") { + REQUIRE(ccw_square.split_at_first_point() == Polyline { ccw_square[0], ccw_square[1], ccw_square[2], ccw_square[3], ccw_square[0] }); + } + THEN("split_at_index(2)") { + REQUIRE(ccw_square.split_at_index(2) == Polyline { ccw_square[2], ccw_square[3], ccw_square[0], ccw_square[1], ccw_square[2] }); + } + THEN("split_at_vertex(ccw_square[2])") { + REQUIRE(ccw_square.split_at_vertex(ccw_square[2]) == Polyline { ccw_square[2], ccw_square[3], ccw_square[0], ccw_square[1], ccw_square[2] }); + } + THEN("is_counter_clockwise") { + REQUIRE(ccw_square.is_counter_clockwise()); + } + THEN("! is_counter_clockwise") { + REQUIRE(! cw_square.is_counter_clockwise()); + } + THEN("make_counter_clockwise") { + cw_square.make_counter_clockwise(); + REQUIRE(cw_square.is_counter_clockwise()); + } + THEN("make_counter_clockwise^2") { + cw_square.make_counter_clockwise(); + cw_square.make_counter_clockwise(); + REQUIRE(cw_square.is_counter_clockwise()); + } + THEN("first_point") { + REQUIRE(&ccw_square.first_point() == &ccw_square.points.front()); + } + } + GIVEN("Triangulating hexagon") { + Polygon hexagon{ { 100, 0 } }; + for (size_t i = 1; i < 6; ++ i) { + Point p = hexagon.points.front(); + p.rotate(PI / 3 * i); + hexagon.points.emplace_back(p); + } + Polygons triangles; + hexagon.triangulate_convex(&triangles); + THEN("right number of triangles") { + REQUIRE(triangles.size() == 4); + } + THEN("all triangles are ccw") { + auto it = std::find_if(triangles.begin(), triangles.end(), [](const Polygon &tri) { return tri.is_clockwise(); }); + REQUIRE(it == triangles.end()); + } + } + GIVEN("General triangle") { + Polygon polygon { { 50000000, 100000000 }, { 300000000, 102000000 }, { 50000000, 104000000 } }; + Line line { { 175992032, 102000000 }, { 47983964, 102000000 } }; + Point intersection; + bool has_intersection = polygon.intersection(line, &intersection); + THEN("Intersection with line") { + REQUIRE(has_intersection); + REQUIRE(intersection == Point { 50000000, 102000000 }); + } + } +} + +TEST_CASE("Centroid of Trapezoid must be inside", "[Polygon][Utils]") +{ + Slic3r::Polygon trapezoid { + { 4702134, 1124765853 }, + { -4702134, 1124765853 }, + { -9404268, 1049531706 }, + { 9404268, 1049531706 }, + }; + Point centroid = trapezoid.centroid(); + CHECK(trapezoid.contains(centroid)); +} + // This test currently only covers remove_collinear_points. // All remaining tests are to be ported from xs/t/06_polygon.t diff --git a/xs/t/06_polygon.t b/xs/t/06_polygon.t index 779c7deec..7bbcd5356 100644 --- a/xs/t/06_polygon.t +++ b/xs/t/06_polygon.t @@ -3,11 +3,8 @@ use strict; use warnings; -use List::Util qw(first); use Slic3r::XS; -use Test::More tests => 21; - -use constant PI => 4 * atan2(1, 1); +use Test::More tests => 3; my $square = [ # ccw [100, 100], @@ -17,81 +14,8 @@ my $square = [ # ccw ]; my $polygon = Slic3r::Polygon->new(@$square); -my $cw_polygon = $polygon->clone; -$cw_polygon->reverse; - -ok $polygon->is_valid, 'is_valid'; -is_deeply $polygon->pp, $square, 'polygon roundtrip'; - is ref($polygon->arrayref), 'ARRAY', 'polygon arrayref is unblessed'; isa_ok $polygon->[0], 'Slic3r::Point::Ref', 'polygon point is blessed'; - -my $lines = $polygon->lines; -is_deeply [ map $_->pp, @$lines ], [ - [ [100, 100], [200, 100] ], - [ [200, 100], [200, 200] ], - [ [200, 200], [100, 200] ], - [ [100, 200], [100, 100] ], -], 'polygon lines'; - -is_deeply $polygon->split_at_first_point->pp, [ @$square[0,1,2,3,0] ], 'split_at_first_point'; -is_deeply $polygon->split_at_index(2)->pp, [ @$square[2,3,0,1,2] ], 'split_at_index'; -is_deeply $polygon->split_at_vertex(Slic3r::Point->new(@{$square->[2]}))->pp, [ @$square[2,3,0,1,2] ], 'split_at'; -is $polygon->area, 100*100, 'area'; - -ok $polygon->is_counter_clockwise, 'is_counter_clockwise'; -ok !$cw_polygon->is_counter_clockwise, 'is_counter_clockwise'; -{ - my $clone = $polygon->clone; - $clone->reverse; - ok !$clone->is_counter_clockwise, 'is_counter_clockwise'; - $clone->make_counter_clockwise; - ok $clone->is_counter_clockwise, 'make_counter_clockwise'; - $clone->make_counter_clockwise; - ok $clone->is_counter_clockwise, 'make_counter_clockwise'; -} - ok ref($polygon->first_point) eq 'Slic3r::Point', 'first_point'; -ok $polygon->contains_point(Slic3r::Point->new(150,150)), 'ccw contains_point'; -ok $cw_polygon->contains_point(Slic3r::Point->new(150,150)), 'cw contains_point'; - -{ - my @points = (Slic3r::Point->new(100,0)); - foreach my $i (1..5) { - my $point = $points[0]->clone; - $point->rotate(PI/3*$i, [0,0]); - push @points, $point; - } - my $hexagon = Slic3r::Polygon->new(@points); - my $triangles = $hexagon->triangulate_convex; - is scalar(@$triangles), 4, 'right number of triangles'; - ok !(defined first { $_->is_clockwise } @$triangles), 'all triangles are ccw'; -} - -{ - is_deeply $polygon->centroid->pp, [150,150], 'centroid'; -} - -{ - my $polygon = Slic3r::Polygon->new( - [50000000, 100000000], - [300000000, 102000000], - [50000000, 104000000], - ); - my $line = Slic3r::Line->new([175992032,102000000], [47983964,102000000]); - my $intersection = $polygon->intersection($line); - is_deeply $intersection->pp, [50000000, 102000000], 'polygon-line intersection'; -} - -# this is not a test: this just demonstrates bad usage, where $polygon->clone gets -# DESTROY'ed before the derived object ($point), causing bad memory access -if (0) { - my $point; - { - $point = $polygon->clone->[0]; - } - $point->scale(2); -} - __END__