Add a simple fallback for Polygon#contains? #224
Conversation
| @@ -17,4 +17,5 @@ def setup | |||
|
|
|||
| # These tests suffer from floating point issues | |||
| undef_method :test_point_on_surface | |||
| undef_method :test_contains_point | |||
There was a problem hiding this comment.
Undef this test for simple_mercator, since with libgeos the result is false aswell,
factory = RGeo::Geographic.simple_mercator_factory()
point1 = factory.point(0, 0)
point2 = factory.point(1, 2)
point3 = factory.point(3, 9)
line_string = factory.line_string([point1, point2, point3, point1])
point = factory.point(0.5, 1)
p line_string.projection
p point.projection
p line_string.contains?(point)
With libgeos:
#<RGeo::Geos::CAPILineStringImpl:0x3fea4a5127b8 "LINESTRING (0.0 -7.081154551613622E-10, 111319.49079327357 222684.20850554318, 333958.4723798207 1006021.0627551326, 0.0 -7.081154551613622E-10)">
#<RGeo::Geos::CAPIPointImpl:0x3fea4a512344 "POINT (55659.74539663678 111325.14286638486)">
#<RGeo::Geographic::ProjectedPointImpl:0x3fea4a512b14 "POINT (0.5 1.0)">
false
Without:
#<RGeo::Cartesian::LineStringImpl:0x3fea7b502224 "LINESTRING (0.0 -7.081154551613622E-10, 111319.49079327357 222684.20850554318, 333958.4723798207 1006021.0627551326, 0.0 -7.081154551613622E-10)">
#<RGeo::Cartesian::PointImpl:0x3fea7b507cc4 "POINT (55659.74539663678 111325.14286638486)">
#<RGeo::Geographic::ProjectedPointImpl:0x3fea7b5028b4 "POINT (0.5 1.0)">
false
Implement default Polygon#contains? and LineString#contains? which handle a Point as input.
Quiwin
force-pushed
the
fallback-contains-point
branch
from
5661bb6
to
42d5e7f
Compare
| def ring_encloses_point?(ring, point, on_border_return: false) | ||
| # This is an implementation of the ray casting algorithm, greatly inspired | ||
| # by https://wrf.ecse.rpi.edu/Research/Short_Notes/pnpoly.html | ||
| # Since this algorithm does not handle point on edge, we check first if | ||
| # the ring is on the border. | ||
| # on_border_return is used for exclusion ring | ||
| return on_border_return if ring.contains?(point) |
There was a problem hiding this comment.
I think that we should get rid of this optional hash parameter on_border_return and just make it false in the code. We can always change it if we determine that should be true later on or some optional parameter, but as it stands this isn't configurable by the user so I think it's cleaner just to make the statement:
return false if ring.contains?(point)and get rid of the parameter.
There was a problem hiding this comment.
@keithdoggett This parameter is used to change the border behavior for exclusion rings. I'm not sure how we could remove it.
There was a problem hiding this comment.
Apologies, you're correct. I somehow missed that it was used with both true and false.
This looks good then.
|
Thanks for submitting this. I think it looks great except for the one tweak that I noted above. |
| def ring_encloses_point?(ring, point, on_border_return: false) | ||
| # This is an implementation of the ray casting algorithm, greatly inspired | ||
| # by https://wrf.ecse.rpi.edu/Research/Short_Notes/pnpoly.html | ||
| # Since this algorithm does not handle point on edge, we check first if | ||
| # the ring is on the border. | ||
| # on_border_return is used for exclusion ring | ||
| return on_border_return if ring.contains?(point) |
There was a problem hiding this comment.
Apologies, you're correct. I somehow missed that it was used with both true and false.
This looks good then.
Implement default Polygon#contains? and LineString#contains? which handle a Point as input.
Summary
Add a fallback on
Polygon and LineString containsso that ifgeosis not available, rgeo can still be used to determine if a point is inside a polygon.Its using a raycasting algorithm (https://wrf.ecse.rpi.edu/Research/Short_Notes/pnpoly.html) with a border check.
Other Information
Hello rgeo!
At Klaxit, we are planning to migrate from our internal geometry library to rgeo.
Since we will use rgeo in some of our open source projects we dont want to add mandatory c dependencies.
Happy to iterate if some changes are needed.
thank you!