Skip to content

Conversation

Starefossen
Copy link
Contributor

This pull adds a third parameter to the L.LatLon class for specifying altitude. This is in turn stored in the .alt property for the LatLng instance. Latitude property will only be set if the latitude parameter is not undefined, this is done in order to ensure backwards compability.

var latlng = new L.LatLng(10, 20, 30);
console.log(latlng.alt); // prints '30' to the console

Similar functionality has been added to L.GeoJSON coordsToLatLng() and latLngToCoords() methods in order to handle import and export of 3D GeoJSON.

var geoJSON = {
  type: 'Feature'
  ,properties: {}
  ,geometry: {
    type: 'Point'
    ,coordinates: [20, 10, 30]
  }
}
var layer = new L.GeoJSON();
layer.addData(geoJSON);
console.log(layer.getLayers()[0].getLatLng().alt);

NB It is important to notice that no logic has been added in order to prevent latitude and longitude to change without appropirate change in altitude – this must be handled by the application.

This commit adds a third parameter to the L.LatLon class for specifying
altitude. This is in turn stored in the `.latitude` property for the LatLng
instance. Latitude property will only be set if the latitude parameter is not
undefined, this is done in order to ensure backwards compability.

```javascript
var latlng = new L.LatLng(10, 20, 30);
console.log(latlng.altitude); // prints '30' to the console
```

Similar functionality has been added to L.GeoJSON coordsToLatLng() and
latLngToCoords() methods in order to handle import and export of 3D GeoJSON.

```javascript
var geoJSON = {
  type: 'Feature'
  ,properties: {}
  ,geometry: {
    type: 'Point'
    ,coordinates: [20, 10, 30]
  }
}
var layer = new L.GeoJSON();
layer.addData(geoJSON);
console.log(layer.getLayers()[0].getLatLng().altitude);
```

`NB` It is important to notice that no logic has been added in order to prevent
latitude and longitude to change without appropirate change in altitude – this
must be handled by the application.
@mourner
Copy link
Member

mourner commented Jul 3, 2013

Thanks for the pull, but you didn't provide any use cases for this. Why would anyone want Leaflet to understand 3D coordinates if it's not a 3D library?

@Starefossen
Copy link
Contributor Author

The use case for this is a routing application I am building for my employee (https://github.com/Turistforeningen/leaflet-routing). Our backend routing service returns a 3D GeoJSON LineString for each new, or changed, waypoint. When the user is happy with his route all the separate LineStrings are glued together into one big GeoJSON LineString and sent and stored in a database. This too should be represented as 3D GeoJSON as we want the altitude for the whole route.

Keeping the altitude, instead of just discarding them which is how LeafletJS treats them today, makes import and export of 3D GeoJSON a breeze. As of now one has to keep track of all the altitudes separately of the LineStrings which creates extra housekeeping work for the developer. This pull simply ensures that altitude, if it exists, is not just discarded :)

Also, the GeoJSON specification for positions states the following:

A position is represented by an array of numbers. There must be at least two elements, and may be more. The order of elements must follow x, y, z order (easting, northing, altitude for coordinates in a projected coordinate reference system, or longitude, latitude, altitude for coordinates in a geographic coordinate reference system).

This new feature is fully backwards compatible! As you can see I took the liberty of making two separate versions of all the GeoJSON unit tests, one for 2D and one for 3D, in order to ensure complete backwards comparability in all cases. The code change itself is only a very few lines of code (4 lines removed and 12 lines added).

@mourner
Copy link
Member

mourner commented Jul 3, 2013

Thanks for the explanation! @tmcw @jfirebaugh thoughts?

@jfirebaugh
Copy link
Member

Seems fine to me, though the property should be named alt, in parallel with lat and lng.

@tmcw
Copy link
Contributor

tmcw commented Jul 3, 2013

Seems fine to me, though it does seem sort of like there should be L.LatLng and L.LatLngAlt, similar to how most 3D code has Vec2d and Vec3d

@mourner
Copy link
Member

mourner commented Jul 3, 2013

@tmcw I don't think we should create another entity in the API for such a rare use case that is not used anywhere inside the library. An optional argument seems fine.

@Starefossen
Copy link
Contributor Author

@tmcw I only think adding this as a new class would make sense if Leaflet ought to implement a complete 3D API.

@ghost ghost assigned mourner Jul 10, 2013
@mourner
Copy link
Member

mourner commented Jul 10, 2013

Will be merged for 0.7.

mourner added a commit that referenced this pull request Nov 6, 2013
@mourner mourner merged commit 8e98e52 into Leaflet:master Nov 6, 2013
@mourner
Copy link
Member

mourner commented Nov 6, 2013

Merged finally. Thanks!

mourner added a commit that referenced this pull request Nov 6, 2013
@Starefossen
Copy link
Contributor Author

Thank you Vladimir!

On 6. nov. 2013, at 22:20, Vladimir Agafonkin notifications@github.com wrote:

Merged finally. Thanks!


Reply to this email directly or view it on GitHub.

@Starefossen Starefossen deleted the latlng-altitude branch March 13, 2014 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants