Ported encloses_point() to XS and renamed to contains_point()

lib/Slic3r/ExPolygon.pm

@@ -39,36 +39,11 @@ sub noncollapsing_offset_ex {
     return $self->offset_ex($distance + 1, @params);
 }
 
-sub encloses_point {
-    my $self = shift;
-    my ($point) = @_;
-    return Boost::Geometry::Utils::point_covered_by_polygon($point->pp, $self->pp);
-}
-
-# A version of encloses_point for use when hole borders do not matter.
-# Useful because point_on_segment is probably slower (this was true
-# before the switch to Boost.Geometry, not sure about now)
-sub encloses_point_quick {
-    my $self = shift;
-    my ($point) = @_;
-    return Boost::Geometry::Utils::point_within_polygon($point->pp, $self->pp);
-}
-
 sub bounding_box {
     my $self = shift;
     return $self->contour->bounding_box;
 }
 
-sub clip_line {
-    my $self = shift;
-    my ($line) = @_;  # line must be a Slic3r::Line object
-    
-    return [
-        map Slic3r::Line->new(@$_),
-            @{Slic3r::Geometry::Clipper::intersection_pl([ $line->as_polyline ], \@$self)}
-    ];
-}
-
 sub simplify_as_polygons {
     my $self = shift;
     my ($tolerance) = @_;
@@ -192,8 +167,13 @@ sub _medial_axis_voronoi {
         # ignore lines going to infinite
         next if $edge->[1] == -1 || $edge->[2] == -1;
         
-        next if !$self->encloses_point_quick(Slic3r::Point->new(@{$vertices->[$edge->[1]]}))
+        my $line = Slic3r::Line->new($vertices->[$edge->[1]], $vertices->[$edge->[2]]);
-             || !$self->encloses_point_quick(Slic3r::Point->new(@{$vertices->[$edge->[2]]}));
+        next if !$self->contains_line($line);
+        
+        # contains_point() could be faster, but we need an implementation that
+        # reliably considers points on boundary
+        #next if !$self->contains_point(Slic3r::Point->new(@{$vertices->[$edge->[1]]}))
+        #     || !$self->contains_point(Slic3r::Point->new(@{$vertices->[$edge->[2]]}));
         
         push @skeleton_lines, [$edge->[1], $edge->[2]];
     }

lib/Slic3r/Fill/Honeycomb.pm

@@ -94,7 +94,7 @@ sub fill_surface {
         )};
         
         # connect paths
-        {
+        if (@paths) {  # prevent calling leftmost_point() on empty collections
             my $collection = Slic3r::Polyline::Collection->new(@paths);
             @paths = ();
             foreach my $path (@{$collection->chained_path_from($collection->leftmost_point, 0)}) {

lib/Slic3r/Fill/Rectilinear.pm

@@ -61,7 +61,7 @@ sub fill_surface {
     my @polylines = @{intersection_pl(\@vertical_lines, $expolygon->offset($line_spacing*0.05))};
     
     # connect lines
-    unless ($params{dont_connect}) {
+    unless ($params{dont_connect} || !@polylines) {  # prevent calling leftmost_point() on empty collections
         my ($expolygon_off) = @{$expolygon->offset_ex(scale $params{flow_spacing}/2)};
         my $collection = Slic3r::Polyline::Collection->new(@polylines);
         @polylines = ();

lib/Slic3r/GCode/Layer.pm

@@ -139,7 +139,7 @@ sub process_layer {
                     for 1 .. (@{$layer->slices} || 1);  # make sure we have at least one island hash to avoid failure of the -1 subscript below
                 PERIMETER: foreach my $perimeter (@{$layerm->perimeters}) {
                     for my $i (0 .. $#{$layer->slices}-1) {
-                        if ($layer->slices->[$i]->contour->encloses_point($perimeter->first_point)) {
+                        if ($layer->slices->[$i]->contour->contains_point($perimeter->first_point)) {
                             push @{ $islands[$i]{perimeters} }, $perimeter;
                             next PERIMETER;
                         }
@@ -148,7 +148,7 @@ sub process_layer {
                 }
                 FILL: foreach my $fill (@{$layerm->fills}) {
                     for my $i (0 .. $#{$layer->slices}-1) {
-                        if ($layer->slices->[$i]->contour->encloses_point($fill->first_point)) {
+                        if ($layer->slices->[$i]->contour->contains_point($fill->first_point)) {
                             push @{ $islands[$i]{fills} }, $fill;
                             next FILL;
                         }

lib/Slic3r/GCode/MotionPlanner.pm

@@ -192,13 +192,13 @@ sub find_node {
     
     # if we're inside a hole, move to a point on hole;
     {
-        my $polygon = first { $_->encloses_point($point) } (map @{$_->holes}, map @$_, @{$self->_inner});
+        my $polygon = first { $_->contains_point($point) } (map @{$_->holes}, map @$_, @{$self->_inner});
         return $point->nearest_point([ @$polygon ]) if $polygon;
     }
     
     # if we're inside an expolygon move to a point on contour or holes
     {
-        my $expolygon = first { $_->encloses_point_quick($point) } (map @$_, @{$self->_inner});
+        my $expolygon = first { $_->contains_point($point) } (map @$_, @{$self->_inner});
         return $point->nearest_point([ map @$_, @$expolygon ]) if $expolygon;
     }
     
@@ -206,11 +206,11 @@ sub find_node {
         my $outer_polygon_idx;
         if (!$self->no_internal) {
             # look for an outer expolygon whose contour contains our point
-            $outer_polygon_idx = first { first { $_->contour->encloses_point($point) } @{$self->_contours_ex->[$_]} }
+            $outer_polygon_idx = first { first { $_->contour->contains_point($point) } @{$self->_contours_ex->[$_]} }
                 0 .. $#{ $self->_contours_ex };
         } else {
             # # look for an outer expolygon containing our point
-            $outer_polygon_idx = first { first { $_->encloses_point($point) } @{$self->_outer->[$_]} }
+            $outer_polygon_idx = first { first { $_->contains_point($point) } @{$self->_outer->[$_]} }
                 0 .. $#{ $self->_outer };
         }
         my $candidates = defined $outer_polygon_idx

lib/Slic3r/GUI/Plater.pm

@@ -1024,7 +1024,7 @@ sub mouse_event {
         $parent->selection_changed(0);
         for my $preview (@{$parent->{object_previews}}) {
             my ($obj_idx, $instance_idx, $thumbnail) = @$preview;
-            if (defined first { $_->contour->encloses_point($pos) } @$thumbnail) {
+            if (defined first { $_->contour->contains_point($pos) } @$thumbnail) {
                 $parent->{selected_objects} = [ [$obj_idx, $instance_idx] ];
                 $parent->{list}->Select($obj_idx, 1);
                 $parent->selection_changed(1);
@@ -1053,7 +1053,7 @@ sub mouse_event {
     } elsif ($event->Moving) {
         my $cursor = wxSTANDARD_CURSOR;
         for my $preview (@{$parent->{object_previews}}) {
-            if (defined first { $_->contour->encloses_point($pos) } @{ $preview->[2] }) {
+            if (defined first { $_->contour->contains_point($pos) } @{ $preview->[2] }) {
                 $cursor = Wx::Cursor->new(wxCURSOR_HAND);
                 last;
             }

lib/Slic3r/Layer/Region.pm

@@ -114,7 +114,7 @@ sub _merge_loops {
     # supply everything to offset_ex() instead of performing several union/diff calls.
     
     # we sort by area assuming that the outermost loops have larger area;
-    # the previous sorting method, based on $b->encloses_point($a->[0]), failed to nest
+    # the previous sorting method, based on $b->contains_point($a->[0]), failed to nest
     # loops correctly in some edge cases when original model had overlapping facets
     my @abs_area = map abs($_), my @area = map $_->area, @$loops;
     my @sorted = sort { $abs_area[$b] <=> $abs_area[$a] } 0..$#$loops;  # outer first
@@ -568,10 +568,12 @@ sub _detect_bridge_direction {
             my @clipped_lines = map Slic3r::Line->new(@$_), @{ intersection_pl(\@lines, [ map @$_, @$inset ]) };
             
             # remove any line not having both endpoints within anchors
+            # NOTE: these calls to contains_point() probably need to check whether the point 
+            # is on the anchor boundaries too
             @clipped_lines = grep {
                 my $line = $_;
-                !(first { $_->encloses_point_quick($line->a) } @$anchors)
+                !(first { $_->contains_point($line->a) } @$anchors)
-                    && !(first { $_->encloses_point_quick($line->b) } @$anchors);
+                    && !(first { $_->contains_point($line->b) } @$anchors);
             } @clipped_lines;
             
             # sum length of bridged lines

lib/Slic3r/Polygon.pm

@@ -27,12 +27,6 @@ sub remove_acute_vertices {
     polygon_remove_acute_vertices($self);
 }
 
-sub encloses_point {
-    my $self = shift;
-    my ($point) = @_;
-    return Boost::Geometry::Utils::point_covered_by_polygon($point->pp, [$self->pp]);
-}
-
 sub grow {
     my $self = shift;
     return $self->split_at_first_point->grow(@_);

lib/Slic3r/Print.pm

@@ -527,7 +527,7 @@ EOF
             my $layer = $self->objects->[$obj_idx]->layers->[$layer_id] or next;
             
             # sort slices so that the outermost ones come first
-            my @slices = sort { $a->contour->encloses_point($b->contour->[0]) ? 0 : 1 } @{$layer->slices};
+            my @slices = sort { $a->contour->contains_point($b->contour->first_point) ? 0 : 1 } @{$layer->slices};
             foreach my $copy (@{$self->objects->[$obj_idx]->copies}) {
                 foreach my $slice (@slices) {
                     my $expolygon = $slice->clone;

lib/Slic3r/Surface.pm

@@ -8,7 +8,7 @@ our @EXPORT_OK   = qw(S_TYPE_TOP S_TYPE_BOTTOM S_TYPE_INTERNAL S_TYPE_INTERNALSO
 our %EXPORT_TAGS = (types => \@EXPORT_OK);
 
 # delegate handles
-sub encloses_point  { $_[0]->expolygon->encloses_point }
+sub contains_point  { $_[0]->expolygon->contains_point }
 sub lines           { $_[0]->expolygon->lines }
 sub contour         { $_[0]->expolygon->contour }
 sub holes           { $_[0]->expolygon->holes }

t/geometry.t

@@ -2,7 +2,7 @@ use Test::More;
 use strict;
 use warnings;
 
-plan tests => 26;
+plan tests => 25;
 
 BEGIN {
     use FindBin;
@@ -85,13 +85,6 @@ my $polygons = [
     ),
 ];
 
-my $points = [
-    Slic3r::Point->new(73631077, 371742392),
-    Slic3r::Point->new(73631077, 501742392),
-];
-
-is Slic3r::Geometry::can_connect_points(@$points, $polygons), 0, 'can_connect_points';
-
 #==========================================================
 
 {

t/multi.t

@@ -55,7 +55,7 @@ use Slic3r::Test;
         }
     });
     my $convex_hull = Slic3r::Polygon->new(@{convex_hull([ map $_->pp, @extrusion_points ])});
-    ok !(first { $convex_hull->encloses_point($_) } @toolchange_points), 'all toolchanges happen outside skirt';
+    ok !(first { $convex_hull->contains_point($_) } @toolchange_points), 'all toolchanges happen outside skirt';
 }
 
 __END__

t/perimeters.t

@@ -60,7 +60,7 @@ use Slic3r::Test;
                         my $move_dest = Slic3r::Point->new_scale(@$info{qw(new_X new_Y)});
                         $external_loops{$self->Z}++;
                         $has_outwards_move = 1
-                            if !Slic3r::Polygon->new_scale(@$cur_loop)->encloses_point($move_dest)
+                            if !Slic3r::Polygon->new_scale(@$cur_loop)->contains_point($move_dest)
                                 ? ($external_loops{$self->Z} == 2)  # contour should include destination
                                 : ($external_loops{$self->Z} == 1); # hole should not
                     }

t/polyclip.t

@@ -2,7 +2,7 @@ use Test::More;
 use strict;
 use warnings;
 
-plan tests => 23;
+plan tests => 19;
 
 BEGIN {
     use FindBin;
@@ -10,6 +10,7 @@ BEGIN {
 }
 
 use Slic3r;
+use Slic3r::Geometry::Clipper qw(intersection_pl);
 
 #==========================================================
 
@@ -29,8 +30,6 @@ my $square = Slic3r::Polygon->new(  # ccw
     [100, 200],
 );
 
-my $line = Slic3r::Line->new([50, 150], [300, 150]);
-
 #==========================================================
 
 {
@@ -41,41 +40,41 @@ my $line = Slic3r::Line->new([50, 150], [300, 150]);
         [160, 140],
     ];
     my $expolygon = Slic3r::ExPolygon->new($square, $hole_in_square);
-    is $expolygon->encloses_point(Slic3r::Point->new(100, 100)), 1, 'corner point is recognized';
+    #is $expolygon->contains_point(Slic3r::Point->new(100, 100)), 1, 'corner point is recognized';
-    is $expolygon->encloses_point(Slic3r::Point->new(100, 180)), 1, 'point on contour is recognized';
+    #is $expolygon->contains_point(Slic3r::Point->new(100, 180)), 1, 'point on contour is recognized';
-    is $expolygon->encloses_point(Slic3r::Point->new(140, 150)), 1, 'point on hole contour is recognized';
+    #is $expolygon->contains_point(Slic3r::Point->new(140, 150)), 1, 'point on hole contour is recognized';
-    is $expolygon->encloses_point(Slic3r::Point->new(140, 140)), 1, 'point on hole corner is recognized';
+    #is $expolygon->contains_point(Slic3r::Point->new(140, 140)), 1, 'point on hole corner is recognized';
     {
-        my $intersection = $expolygon->clip_line(Slic3r::Line->new([150,180], [150,150]));
+        my $intersection = intersection_pl([Slic3r::Polyline->new([150,180], [150,150])], \@$expolygon);
         is $intersection->[0]->length, Slic3r::Line->new([150, 180], [150, 160])->length,
             'line is clipped to square with hole';
     }
     {
-        my $intersection = $expolygon->clip_line(Slic3r::Line->new([150,150], [150,120]));
+        my $intersection = intersection_pl([Slic3r::Polyline->new([150,150], [150,120])], \@$expolygon);
         is $intersection->[0]->length, Slic3r::Line->new([150, 140], [150, 120])->length,
             'line is clipped to square with hole';
     }
     {
-        my $intersection = $expolygon->clip_line(Slic3r::Line->new([120,180], [180,180]));
+        my $intersection = intersection_pl([Slic3r::Polyline->new([120,180], [180,180])], \@$expolygon);
         is $intersection->[0]->length, Slic3r::Line->new([120,180], [180,180])->length,
             'line is clipped to square with hole';
     }
     {
-        my $intersection = $expolygon->clip_line($line);
+        my $intersection = intersection_pl([Slic3r::Polyline->new([50, 150], [300, 150])], \@$expolygon);
         is $intersection->[0]->length, Slic3r::Line->new([100, 150], [140, 150])->length,
             'line is clipped to square with hole';
         is $intersection->[1]->length, Slic3r::Line->new([160, 150], [200, 150])->length,
             'line is clipped to square with hole';
     }
     {
-        my $intersection = $expolygon->clip_line(Slic3r::Line->new(reverse @$line));
+        my $intersection = intersection_pl([Slic3r::Polyline->new([300, 150], [50, 150])], \@$expolygon);
         is $intersection->[0]->length, Slic3r::Line->new([200, 150], [160, 150])->length,
             'reverse line is clipped to square with hole';
         is $intersection->[1]->length, Slic3r::Line->new([140, 150], [100, 150])->length,
             'reverse line is clipped to square with hole';
     }
     {
-        my $intersection = $expolygon->clip_line(Slic3r::Line->new([100,180], [200,180]));
+        my $intersection = intersection_pl([Slic3r::Polyline->new([100,180], [200,180])], \@$expolygon);
         is $intersection->[0]->length, Slic3r::Line->new([100,180], [200,180])->length,
             'tangent line is clipped to square with hole';
     }
@@ -117,9 +116,9 @@ my $line = Slic3r::Line->new([50, 150], [300, 150]);
     ok $small_circle->is_clockwise, "hole is clockwise";
     
     my $expolygon = Slic3r::ExPolygon->new($large_circle, $small_circle);
-    $line = Slic3r::Line->new_scale([152.742,288.086671142818], [152.742,34.166466971035]);
+    my $line = Slic3r::Polyline->new_scale([152.742,288.086671142818], [152.742,34.166466971035]);
     
-    my $intersection = $expolygon->clip_line($line);
+    my $intersection = intersection_pl([$line], \@$expolygon);
     is $intersection->[0]->length, Slic3r::Line->new([152742000, 288086661], [152742000, 215178843])->length,
         'line is clipped to square with hole';
     is $intersection->[1]->length, Slic3r::Line->new([152742000, 108087507], [152742000, 35166477])->length,

xs/src/ExPolygon.cpp

@@ -62,7 +62,7 @@ ExPolygon::is_valid() const
 }
 
 bool
-ExPolygon::contains_line(Line* line) const
+ExPolygon::contains_line(const Line* line) const
 {
     Polylines pl(1);
     pl.push_back(*line);
@@ -72,6 +72,16 @@ ExPolygon::contains_line(Line* line) const
     return pl_out.empty();
 }
 
+bool
+ExPolygon::contains_point(const Point* point) const
+{
+    if (!this->contour.contains_point(point)) return false;
+    for (Polygons::const_iterator it = this->holes.begin(); it != this->holes.end(); ++it) {
+        if (it->contains_point(point)) return false;
+    }
+    return true;
+}
+
 #ifdef SLIC3RXS
 SV*
 ExPolygon::to_AV() {

xs/src/ExPolygon.hpp

@@ -17,7 +17,8 @@ class ExPolygon
     void rotate(double angle, Point* center);
     double area() const;
     bool is_valid() const;
-    bool contains_line(Line* line) const;
+    bool contains_line(const Line* line) const;
+    bool contains_point(const Point* point) const;
     
     #ifdef SLIC3RXS
     void from_SV(SV* poly_sv);

xs/src/Polygon.cpp

@@ -110,6 +110,21 @@ Polygon::is_valid() const
     return this->points.size() >= 3;
 }
 
+bool
+Polygon::contains_point(const Point* point) const
+{
+    // http://www.ecse.rpi.edu/Homepages/wrf/Research/Short_Notes/pnpoly.html
+    bool result;
+    Points::const_iterator i = this->points.begin();
+    Points::const_iterator j = this->points.end() - 1;
+    for (; i != this->points.end(); j = i++) {
+        if ( ((i->y > point->y) != (j->y > point->y))
+            && (point->x < (j->x - i->x) * (point->y - i->y) / (j->y - i->y) + i->x) )
+            result = !result;
+    }
+    return result;
+}
+
 #ifdef SLIC3RXS
 SV*
 Polygon::to_SV_ref() {

xs/src/Polygon.hpp

@@ -23,6 +23,7 @@ class Polygon : public MultiPoint {
     bool make_counter_clockwise();
     bool make_clockwise();
     bool is_valid() const;
+    bool contains_point(const Point* point) const;
     
     #ifdef SLIC3RXS
     SV* to_SV_ref();

xs/src/PolylineCollection.cpp

@@ -49,6 +49,7 @@ PolylineCollection::leftmost_point() const
         if (p == NULL || it->points.front().x < p->x)
             p = &(it->points.front());
     }
+    if (p == NULL) return NULL;
     return new Point (*p);
 }
 

xs/t/06_polygon.t

@@ -4,7 +4,7 @@ use strict;
 use warnings;
 
 use Slic3r::XS;
-use Test::More tests => 14;
+use Test::More tests => 17;
 
 my $square = [  # ccw
     [100, 100],
@@ -14,6 +14,9 @@ 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';
 
@@ -34,6 +37,7 @@ is_deeply $polygon->split_at(Slic3r::Point->new(@{$square->[2]}))->pp, [ @$squar
 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;
@@ -46,6 +50,9 @@ ok $polygon->is_counter_clockwise, 'is_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';
+
 # 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) {

xs/xsp/ExPolygon.xsp

@@ -22,6 +22,7 @@
     double area();
     bool is_valid();
     bool contains_line(Line* line);
+    bool contains_point(Point* point);
 %{
 
 ExPolygon*

xs/xsp/Polygon.xsp

@@ -33,6 +33,7 @@
     bool is_valid();
     Point* first_point()
         %code{% const char* CLASS = "Slic3r::Point"; RETVAL = THIS->first_point(); %};
+    bool contains_point(Point* point);
 %{
 
 Polygon*