-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 GreatCircleLayer #2640
Add GreatCircleLayer #2640
Conversation
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.
I really like this idea, fits well with our geospatial focus and the samples look really cool.
But shouldn't this be an example using a customized/subclassed ArcLayer
?
To me this seem like an odd feature to add to a generic layer, especially a layer that currently works in any coordinate system, not just geospatial.
Specifically, I see a lot of extra lng/lat based shader code and a new prop that only apply to a very specialized case. I am assuming that 95% of ArcLayer users won't use this?
8c6eb56
to
99cbf6b
Compare
Thanks, that makes a lot more sense. I could argue that this might be more of an "example specific layer" rather than an "experimental layer" (i.e. users would have to copy it from an example rather than import it from experimental layers), but I am fine with either approach. Having it in experimental layers makes sense if we think there is a future layer catalog where this layer could fit in, I could for instance imagine a layer catalog with various geospatial "helper layers", other layers could perhaps a "compass layer", ... Also, you could probably build some really cool geospatial grid layer/satellite orbit layers using this layer as a primitive. |
1878587
to
cfa7f99
Compare
modules/experimental-layers/src/great-circle-layer/great-circle-vertex.glsl.js
Outdated
Show resolved
Hide resolved
modules/experimental-layers/src/great-circle-layer/great-circle-vertex.glsl.js
Outdated
Show resolved
Hide resolved
modules/experimental-layers/src/great-circle-layer/great-circle-vertex.glsl.js
Outdated
Show resolved
Hide resolved
modules/experimental-layers/src/great-circle-layer/great-circle-vertex.glsl.js
Outdated
Show resolved
Hide resolved
modules/experimental-layers/src/great-circle-layer/great-circle-vertex.glsl.js
Outdated
Show resolved
Hide resolved
{source: [-20, 50], target: [-160, 50]}, | ||
{source: [-20, 60], target: [-160, 60]}, | ||
{source: [-20, 70], target: [-160, 70]} | ||
]; |
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.
Need more test cases. E.g. longitude 10 to longitude -170. Source and target not on the same latitude.
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.
Please update the PR title and summary to reflect the latest changes. |
506661b
to
bc2bdab
Compare
vec3 nextPos = vec3(degrees(interpolate(source, target, angularDist, nextSegmentRatio)), 0.0); | ||
|
||
vec4 curr = project_position_to_clipspace(currPos, instancePositions64Low.xy, vec3(0.0)); | ||
vec4 next = project_position_to_clipspace(nextPos, instancePositions64Low.zw, vec3(0.0)); |
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.
Although instancePositions64Low
is not significant in this use case, you need to make sure that the next
position in one segment and the curr
position in the following segment match.
vec3 currPos = vec3(degrees(interpolate(source, target, angularDist, segmentRatio)), 0.0); | ||
vec3 nextPos = vec3(degrees(interpolate(source, target, angularDist, nextSegmentRatio)), 0.0); | ||
|
||
vec2 currPos64Low = degrees(interpolate(instancePositions64Low.xy, instancePositions64Low.zw, angularDist, segmentRatio)); |
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.
- This is mathematically incorrect
- 64bit accuracy of interpolated points is not a concern for this layer
So just use linear interpolation.
For #2499
Background
customize ArcLayer to draw a great circle between two points on the map
Change List
result