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

KmlDataSource uses degrees instead of radians #5992

Closed
ggetz opened this issue Nov 20, 2017 · 6 comments
Closed

KmlDataSource uses degrees instead of radians #5992

ggetz opened this issue Nov 20, 2017 · 6 comments
Labels
category - kml cleanup good first issue An opportunity for first time contributors

Comments

@hpinkos
Copy link
Contributor

hpinkos commented Nov 20, 2017

@ggetz the first is using fromDegrees so it should be fine. The second should probably be using that as well

@pjcozzi pjcozzi added good first issue An opportunity for first time contributors category - kml cleanup labels Nov 21, 2017
@jaforom
Copy link
Contributor

jaforom commented Nov 21, 2017

@hpinkos, tilt is converted to radians in line 156361:

tilt = CesiumMath.toRadians(defaultValue(tilt, 0.0));

so 90.0 must be Cesium.Math.PI / 2 in line 156365:

var hpr = new HeadingPitchRange(heading, tilt - 90.0, range);

Regards

@hpinkos
Copy link
Contributor

hpinkos commented Nov 21, 2017

Thanks @jaforom, you're right! I didn't look at the code very closely.
If you have a minute, we would gladly accept a pull request with this fix

@jaforom
Copy link
Contributor

jaforom commented Nov 22, 2017

Sorry, @hpinkos, it would be my first pull request and I don't know how to do it. Could you tell me how to do that?

Regards

@jaforom
Copy link
Contributor

jaforom commented Nov 22, 2017

Well, I don't know if it's right, but here it is: #5997

Regards

@cesium-concierge
Copy link

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/forum/#!topic/cesium-dev/QrwGDAOgt8w

If this issue affects any of these threads, please post a comment like the following:

The issue at #5992 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.


I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category - kml cleanup good first issue An opportunity for first time contributors
Projects
None yet
Development

No branches or pull requests

5 participants