-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add polygon area calculation #21
Changes from 1 commit
d77070d
e340401
d89c654
4adfa4f
a56f773
0c4382e
dd6eac8
d3ead9a
4b4ff4e
944e581
047fe58
dea04ac
ead6d97
51f86dc
6e57647
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,10 +4,9 @@ public typealias LocationRadians = Double | |
public typealias RadianDistance = Double | ||
public typealias RadianDirection = Double | ||
|
||
struct Constants { | ||
static let metersPerRadian = 6_373_000.0 | ||
static let equatorialRadius:Double = 6378137 | ||
} | ||
|
||
let metersPerRadian: CLLocationDistance = 6_373_000.0 | ||
let equatorialRadius: CLLocationDistance = 6378137 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: similar to in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
/** | ||
A `RadianCoordinate2D` is a coordinate represented in radians as opposed to | ||
|
@@ -289,22 +288,8 @@ public struct Polyline { | |
} | ||
} | ||
|
||
public struct Polygon { | ||
var polygonCoordinates: [[CLLocationCoordinate2D]] | ||
|
||
var polygonArea: Double { | ||
var area:Double = 0 | ||
|
||
if (!polygonCoordinates.isEmpty && polygonCoordinates.count > 0) { | ||
|
||
area += abs(ringArea(polygonCoordinates[0])) | ||
|
||
for coordinate in polygonCoordinates.suffix(from: 1) { | ||
area -= abs(ringArea(coordinate)) | ||
} | ||
} | ||
return area | ||
} | ||
struct Ring { | ||
var coordinates: [CLLocationCoordinate2D] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Even better now encapsulated in a Ring. |
||
|
||
/** | ||
* Calculate the approximate area of the polygon were it projected onto the earth. | ||
|
@@ -315,40 +300,47 @@ public struct Polygon { | |
* Laboratory, Pasadena, CA, June 2007 http://trs-new.jpl.nasa.gov/dspace/handle/2014/40409 | ||
* | ||
*/ | ||
private func ringArea(_ coordinates: [CLLocationCoordinate2D]) -> Double { | ||
var p1: CLLocationCoordinate2D | ||
var p2: CLLocationCoordinate2D | ||
var p3: CLLocationCoordinate2D | ||
var lowerIndex: Int | ||
var middleIndex: Int | ||
var upperIndex: Int | ||
internal func area() -> Double { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turn this method into a computed property to reduce the friction of using it. |
||
var area: Double = 0 | ||
let coordinatesCount: Int = coordinates.count | ||
|
||
if (coordinatesCount > 2) { | ||
for index in 0...coordinatesCount - 1 { | ||
if (index == coordinatesCount - 2) { | ||
lowerIndex = coordinatesCount - 2 | ||
middleIndex = coordinatesCount - 1 | ||
upperIndex = 0 | ||
} else if(index == coordinatesCount - 1) { | ||
lowerIndex = coordinatesCount - 1 | ||
middleIndex = 0 | ||
upperIndex = 1 | ||
if coordinatesCount > 2 { | ||
for index in 0..<coordinatesCount { | ||
|
||
let controlPoints: (CLLocationCoordinate2D, CLLocationCoordinate2D, CLLocationCoordinate2D) | ||
|
||
if index == coordinatesCount - 2 { | ||
controlPoints = (coordinates[coordinatesCount - 2], | ||
coordinates[coordinatesCount - 1], | ||
coordinates[0]) | ||
} else if index == coordinatesCount - 1 { | ||
controlPoints = (coordinates[coordinatesCount - 1], | ||
coordinates[0], | ||
coordinates[1]) | ||
} else { | ||
lowerIndex = index | ||
middleIndex = index + 1 | ||
upperIndex = index + 2 | ||
controlPoints = (coordinates[index], | ||
coordinates[index + 1], | ||
coordinates[index + 2]) | ||
} | ||
|
||
p1 = coordinates[lowerIndex] | ||
p2 = coordinates[middleIndex] | ||
p3 = coordinates[upperIndex] | ||
area += (p3.longitude.toRadians() - p1.longitude.toRadians()) * sin(p2.latitude.toRadians()) | ||
area += (controlPoints.2.longitude.toRadians() - controlPoints.0.longitude.toRadians()) * sin(controlPoints.1.latitude.toRadians()) | ||
} | ||
|
||
area = area * Constants.equatorialRadius * Constants.equatorialRadius / 2 | ||
area *= equatorialRadius * equatorialRadius / 2 | ||
} | ||
return area | ||
} | ||
} | ||
|
||
public struct Polygon { | ||
var outerRing: Ring | ||
var innerRings: [Ring] | ||
|
||
// Ported from https://github.com/Turfjs/turf/blob/a94151418cb969868fdb42955a19a133512da0fd/packages/turf-area/index.js | ||
|
||
var area: Double { | ||
return abs(outerRing.area()) - innerRings | ||
.map { abs($0.area()) } | ||
.reduce(0, +) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -291,12 +291,13 @@ class TurfTests: XCTestCase { | |
let json = Fixture.JSONFromFileNamed(name: "polygon") | ||
let geometry = json["geometry"] as! [String: Any] | ||
let geoJSONCoordinates = geometry["coordinates"] as! [[[Double]]] | ||
let coordinates = geoJSONCoordinates.map { | ||
let allRings = geoJSONCoordinates.map { | ||
$0.map { CLLocationCoordinate2D(latitude: $0[1], longitude: $0[0]) } | ||
} | ||
let outerRing = Ring(coordinates: allRings.first!) | ||
|
||
let polygon = Polygon(polygonCoordinates: coordinates) | ||
let polygon = Polygon(outerRing: outerRing, innerRings: []) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the inner rings in |
||
|
||
XCTAssertEqual(polygon.polygonArea, 7766240997209, accuracy: 0.1) | ||
XCTAssertEqual(polygon.area, 7766240997209, accuracy: 0.1) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently this value of 6 373 000 m came from an old version of turf-distance that I originally ported to an internal Swift application, before it got copied into another internal Swift application, then copied into the navigation SDK, then moved here. It’s close to the value of 6 372 797.560 856 that Osmium describes as “Earth’s quadratic mean radius for WGS84”.
These days, Turf.js uses a spherical approximation of 6 671 008.8 m for its radian-to-meter conversion and Haversine formula. The Haversine formula was always meant to be used with a spherical meters-per-radian value. We should probably change this value to match Turf.js. 🙀
/ref Turfjs/turf#978 Turfjs/turf#1012 Turfjs/turf#1176
/cc @frederoni @bsudekum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the polygon area calculation doesn’t depend on
metersPerRadian
, we can treat this change as tail work: #26.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going to change
metersPerRadian
to6 671 008.8 m
, should this happen in a different PR since it would globally impact this library? Or are you ok with me making the change here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let’s do it in a separate PR for #26, so that any fallout is easier to track down.
In the meantime, can you add some comments explaining why these two constants differ?