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

KML tours: processing KMLTourFlyTo #5997

Closed
wants to merge 160 commits into from
Closed

KML tours: processing KMLTourFlyTo #5997

wants to merge 160 commits into from

Conversation

jaforom
Copy link
Contributor

@jaforom jaforom commented Nov 22, 2017

Hi all,

I recorded a tour with Google Earth and then I tried to load it in my Cesium viewer. It went fine, but I noted that the pitch was wrong (the camera was always vertical). In my tour, I used several pitches.

When I found the code where the KMLTourFlyTo is processed, I found this code line, inside processLookAt function:

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

This is wrong, because tilt must be in radians, so the correct line code will be:

var hpr = new HeadingPitchRange(heading, tilt - Cesium.Math.PI / 2, range);

The Cesium version is 1.39 and the bug is in line 156365.

I modified the line in CesiumUnminified/Cesium.js and then I loaded the KML again: the tour was right, using the different pitches I recorded in the tour.

Regards

shunter and others added 30 commits April 27, 2012 11:20
Conflicts:
	Examples/Sandbox/CodeSnippets/Imagery.js
Conflicts:
	Examples/Sandbox/CodeSnippets/Imagery.js
	Tools/jsdoc3/templates/default/tmpl/layout.tmpl
Conflicts:
	Apps/CesiumViewer/CesiumViewer.css
	Apps/CesiumViewer/index.html
Conflicts:
	Examples/Sandbox/CodeSnippets/Imagery.js
	Examples/Sandbox/Sandbox.js
	Examples/Sandbox/index.html
Conflicts:
	Apps/Sandcastle/Sandcastle.html
	Apps/Sandcastle/index.html
@cesium-concierge
Copy link

Please sign the CLA before we review this PR.

Welcome to the Cesium community @jaforom!

Can you please send in a Contributor License Agreement (CLA) so that we can review and merge this pull request?

⚠️ I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to CHANGES.md and comment on this pull request so we know it was updated. For more info, see the Pull Request Guidelines.


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

🌍 🌎 🌏

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 22, 2017

We now have a CLA from @jaforom. Thanks again for the contribution!

@ggetz
Copy link
Contributor

ggetz commented Nov 22, 2017

Hi @jaforom, are you should your PR is using the right branch? It looks like you're attempting to merge the cesiumjs.org branch with master. cesiumjs.org should be your branch of Cesium where you made your changes. Checkout the Contributor's Guides if you need more info. Thanks!

@jaforom
Copy link
Contributor Author

jaforom commented Nov 23, 2017 via email

@jaforom
Copy link
Contributor Author

jaforom commented Nov 23, 2017

Maybe I must close this PR and leave #5998 as the right PR.

@ggetz
Copy link
Contributor

ggetz commented Nov 27, 2017

@jaforom Yes, it looks like you set up the PR correctly in #5998, thanks! You can go ahead and close this PR.

@jaforom jaforom closed this Nov 27, 2017
@jaforom
Copy link
Contributor Author

jaforom commented Nov 27, 2017

The right one is #5998.

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.