Skip to content

Add geodesic option for measure #3222

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

Merged
merged 1 commit into from
Feb 11, 2015
Merged

Add geodesic option for measure #3222

merged 1 commit into from
Feb 11, 2015

Conversation

bartvde
Copy link
Member

@bartvde bartvde commented Feb 10, 2015

Add geodesic measurement option to the measure example.

@bartvde
Copy link
Member Author

bartvde commented Feb 11, 2015

Okay I've cleaned this up @ahocevar according to our discussions. To summarize:

  • use harversine distance on the sphere for distance measurements
  • add vincentyArea function (and pass on the coordinate array of the linear ring) on the sphere, include the same reference as in OpenLayers 2 since the code is a "migration" of that code
  • do not export ol.sphere.WGS84 but use the same snippet as is already in the API docs for ol.Sphere (so specify the radius in the example)

var area = 0;
var x1 = coordinates[coordinates.length - 1][0];
var y1 = coordinates[coordinates.length - 1][1];
for (var i = 0, ii = coordinates.length; i < ii; i++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but if you assign coordinates.length to a variable before assigning x1, you can reuse it instead of dereferencing coordinates.length 3 times.

@bartvde
Copy link
Member Author

bartvde commented Feb 11, 2015

@ahocevar I incorporated your latest review comments, let me know if you have more coming

@ahocevar
Copy link
Member

Looks good now. Please merge when Travis is happy.

@bartvde
Copy link
Member Author

bartvde commented Feb 11, 2015

thanks for the extensive review and discussions @ahocevar !

bartvde added a commit that referenced this pull request Feb 11, 2015
Add geodesic option for measure
@bartvde bartvde merged commit b529f0f into openlayers:master Feb 11, 2015
@bartvde bartvde deleted the geodesic-measure branch February 11, 2015 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants