Skip to content
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

Support for (storing) altitude #134

Open
Jonas-Witt opened this issue Jul 28, 2023 · 6 comments
Open

Support for (storing) altitude #134

Jonas-Witt opened this issue Jul 28, 2023 · 6 comments

Comments

@Jonas-Witt
Copy link

Hi Paul,

We work with 3D geographical data and are currently transitioning part of our backend to go. However, since the Point type has a fixed size we can't use it without forking. Although the altitude would just be passively carried around in all your algorithms, support for it would be nice IMO (and the GeoJSON spec allows it). What are your thoughts on either redefining the Point to [3]float64 or a slice? Would this break anything? Altitude could conditionally be marshaled whether it's non-zero.

@martinrode
Copy link

We would also very much like that! So +1

@wenbert
Copy link

wenbert commented Nov 18, 2023

This would be https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.1

A position is an array of numbers. There MUST be two or more
elements. The first two elements are longitude and latitude, or
easting and northing, precisely in that order and using decimal
numbers. Altitude or elevation MAY be included as an optional third
element.

Currently, when I do this:

// Create a GeoJSON LineString
lineString := orb.LineString{}

// Iterate through each result and add coordinates to the LineString
for _, result := range results {
    coordinate := orb.Point{result.Location.Lng, result.Location.Lat} // I want to add the 3rd value here; elevation
    lineString = append(lineString, coordinate)
}

I am not able to append the orb.Point{result.Location.Lng, result.Location.Lat, result.Elevation} (see 3rd value "elevation")

@paulmach
Copy link
Owner

The Point type is defined as a [2]float64 for performance reasons. Making it a []float64 would make a pointer to slice header with a pointer to data. much slower.

I think you could maintain a fork, or orb3, where point is [3]float64. I don't know if you could update all the helpers to be "generic", but that would be cool. Otherwise the libraries would not be cross compatible.

Some, solvable issues that come to mind:

  • It looks like tests would need to be update/rewritten as points become [0,0,0] (vs. [0,0]) in json.
  • wkt,wkb,ewkb would need to be checked they represent the extra point correctly.

Altitude could conditionally be marshaled whether it's non-zero.

I'm not sure I agree with this.

@scottbarnesg
Copy link

I am also interested being able to add altitude to a GeoJSON point. I've scoured multiple available go libraries and haven't been able to find an implementation that supports this. I recognize that orb is a 2d library, that this is out of scope for 2d functionality, and I agree with @paulmach 's assessment about the performance implications.

That said, it seems a little silly to maintain a fork or start a new library just to get this feature. All I want to do is create a Point with altitude and add it to a GeoJSON Feature. Since it sounds like this isn't going to be implemented as part of orb, has anyone else found a viable library that supports this before I go roll my own implementation?

@martinrode
Copy link

has anyone else found a viable library that supports this before I go roll my own implementation?

I ended up using "https://github.com/twpayne/go-geom"

@selam
Copy link

selam commented Apr 10, 2024

   type Point struct {
      orb.Point
     altitude float64
   }

   tree .Add(Point{point, 2.5})  // altitude 2.5 

i think this will work if you want just carrying around information

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

No branches or pull requests

6 participants