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

Scale CoordinatesDisplayLayer latitude/longitude digits based on eye … #62

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

lowswaplab
Copy link

@lowswaplab lowswaplab commented Jun 2, 2021

Description of the Change

Scale latitude/longitude decimal places based on eye altitude. For example, if zoomed in really close, show latitude/longitude with 6 decimal places. If zoomed way out, only show 1 or 2 decimal places.

Why Should This Be In Core?

To improve display and accuracy of latitude/longitude display.

Benefits

Better display.

Potential Drawbacks

None.

Applicable Issues

None. But there are a few of hardcoded values in CoordinatesDisplayLayer that need to be taken care of (that were there prior to this PR). These hardcoded values appear to assume character width and other values.

@lowswaplab
Copy link
Author

Oh wait, do not accept this PR yet.
It looks like eye altitude is based on Mean Sea Level (or geoid height or something similar). For example, if you zoom in as far as you can on the Alps, eye altitude is set to 2km. Not 0m as I expected.
Instead of using eye altitude alone, I need to calculate height above ground: eye altitude - terrain altitude.

@ComBatVision
Copy link
Member

ComBatVision commented Jun 2, 2021

Why do you not using lookAt.range instead of eye.altitude?

LookAt.range is a distance to surface (including terrain elevation) of point in screen center.

@lowswaplab
Copy link
Author

Done

@ComBatVision
Copy link
Member

I have compared this feature with Java and Android code bases. They use constant amount of decimal places, e.g. 4 or 6.

Why do we need this dynamic decimal places? Can we simply replace 2 with 6 for all case?

@lowswaplab
Copy link
Author

I did this because having 6 decimal places doesn't make sense when a pixel is only a decimal place or two wide...

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.

None yet

2 participants