-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Improve Camera Flights #2825
Improve Camera Flights #2825
Conversation
…hange flight duration based on distance.
I updated the TODO list in this issue:
|
@@ -2299,6 +2315,8 @@ define([ | |||
* @param {Matrix4} [options.endTransform] Transform matrix representing the reference frame the camera will be in when the flight is completed. | |||
* @param {Boolean} [options.convert=true] When <code>true</code>, the destination is converted to the correct coordinate system for each scene mode. When <code>false</code>, the destination is expected | |||
* to be in the correct coordinate system. | |||
* @param {Number} [options.altitude] The maximum altitude at the peak of the flight. |
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.
Throughout Cesium, we call this height
. We should do the same here unless we have a compelling reason.
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.
Or maximumHeight
.
Also added this to the TODO:
|
Switch to CV, then back to 3D. The home view is too close. It is: http://localhost:8080/Apps/CesiumViewer/index.html?view=-90%2C26.641969128869164%2C6382419.6691765785%2C360%2C-89.84575039834716%2C0 |
The flight from, for example, Exton to LA feels a bit too "square" if you know what I mean. |
When it is a bit slower, I think this is fine. |
The Cesium API and Geocoder/Home flights seem to have different durations. Why not have the same default for both? |
To freeze when morphing:
I'm pretty sure this is because the morph started before the flight finished. |
The CV flights tend to dip early then fly low to the ground. Instead, if we dip late then zoom closer to directly in, I bet it will look better and load fewer tiles since it is not requesting all the high res tiles when moving quickly. For example:
|
The home flight is now WAY to long. Can we change it so the unified defaults use the old home flight value. After playing with it some more, the default flight time in general is way to long. I would recommend we make it no longer than 1 second. I think this also has to do with the fact that flight length (in meters traveled) seems to be significantly less without the bouncing. |
Change the last line of the below example to 1.10: http://cesiumjs.org/Cesium/Apps/Sandcastle/index.html?src=Polygon.html&label=Geometries |
var duration = options.duration; | ||
if (!defined(duration)) { | ||
duration = Math.ceil(Cartesian3.distance(camera.position, destination) / 1000000.0) + 3.0; | ||
duration = Math.min(duration, 10.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.
This line seems to be the cause of the extremely slow fights that I'm seeing (10 seconds). I would suggest changing these two lines to the below, which I feel works much better in all of the test cases I've tried.
duration = Math.ceil(Cartesian3.distance(camera.position, destination) / 1000000.0) + 1.0;
duration = Math.min(duration, 2.0);
That's all I've got for now, this is an awesome change. |
@bagnell can you reply here: https://groups.google.com/forum/#!topic/cesium-dev/90utjnigscg I imagine this is just use |
* Deprecated | ||
* `CatmullRomSpline` has been deprecated and will be removed in Cesium 1.12. |
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.
Maybe we should rethink this: https://groups.google.com/forum/#!topic/cesium-dev/MMIdPKFNR2s
Deprecating HermiteSpline
is still OK though.
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.
Might want to keep both and add others. Depends on what direction is taken for tools in Viewer. Also, GML uses b-spline and cubic spline which I believe are for roads.
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'm fine with not deprecating these and reevaluating in the future.
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.
@GatorScott I added cubic b-splines and bezier splines a while back for model animation. They're sitting in the splines
branch if we ever need them.
@bagnell anything left here? Let's aim to merge this on Monday for 1.11. |
#2468 still seems to be a problem. |
@pjcozzi I removed the deprecation from |
It seems that #2073 is fixed but I haven't been able to reproduce it in master either. |
Conflicts: CHANGES.md
A big 👍 from me. @pjcozzi feel free to merge if you're happy. |
Awesome, will review and merge first thing tomorrow. |
Camera.clone
Remove Camera.clone #2763TODO
CC #1060