Prevent camera from going underground during mouse navigation

[lilleyse]

Fixes #5837
Fixes #5999 (I think)

Since #4105 the camera has been allowed to underground if globe tiles are loading in order to prevent unwanted shifts during camera flights or setView calls. However this leads to poor user experience when manually navigating around terrain and accidentally slipping underground, especially on slower internet connections where it happens much more frequently.

This PR undoes #4105 and moves the onus to adjust height back to ScreenSpaceCameraController, but only if the camera has moved during the controller's update. Camera flyTo and setView are not affected because terrain adjustment only happens when the user interacts with the camera which cannot occur during these two operations. This behavior can be confirmed in this sandcastle.

This method also removes some pretty bad coupling between Camera, ScreenSpaceCameraController, and Globe. Especially lines in Camera like:

var globeFinishedUpdating = !defined(globe) || (globe._surface.tileProvider.ready && globe._surface._tileLoadQueueHigh.length === 0 && globe._surface._tileLoadQueueMedium.length === 0 && globe._surface._tileLoadQueueLow.length === 0 && globe._surface._debug.tilesWaitingForChildren === 0);

@mramato @gberaudo @IanLilleyT

[cesium-concierge]

Thanks for the pull request @lilleyse!

• ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[mramato]

This sounds like a good fix to me. I retested the initial issue (#4075) and it still works as of this PR.

[mramato]

There seem to still be some cases where you can go underground:

To recreate the above, I modified http://localhost:8080/Apps/Sandcastle/index.html?src=KML.html&label=DataSources and enabled clampToGround for the facilities KML and turned on terrain.

Assuming this isn't a trivial fix or is an unrelated issue, I'm okay with this going in as-is and having another issue written up, just wanted to bring it to your attention.

[mramato]

It's actually even easier to reproduce with just the stock example: http://localhost:8080/Apps/Sandcastle/index.html?src=GeoJSON%20simplestyle.html&label=DataSources using the ellipsoid provider

[lilleyse]

@mramato I pushed a commit that should fix that. I actually had this fix on underground-camera but didn't think to move it over here.

The problem was globe.getHeight would return undefined if the deepest tile didn't have a rendered mesh, which would result in frames where the camera could go underground. The fix was to keep track of the deepest tile with a rendered mesh instead of just the deepest tile.

The GeoJSON demo now works much smoother. Any remaining flicker problems are due to log-depth.

[mramato]

Looks awesome @lilleyse this is a huge improvement. I couldn't get the camera to go permanently underground at all and only saw very rare flashes as tiles were loading in high-detail terrain areas.

@IanLilleyT please review and merge if you're satisfied.

@lilleyse any reason this can't make Monday's release? Seems like a huge win and a relatively minor change, but if you're not comfortable with putting it in just yet, I'm happy to have us hold off until Feb 1st.

[lilleyse]

@lilleyse any reason this can't make Monday's release?

I'd prefer to wait for February in case this change has some unintended side affect that we're not aware of yet. I'm going to be working in the camera system a lot in January so there will be plenty of opportunity to catch any mistakes.

[IanLilleyT]

This looks good. It fixes both the linked issues. Going to merge after the January release.

Future work: Adjusting the height only when the camera moves is not always enough. For example: If you pivot the camera and release before all the high detail tiles are loaded, the camera can get stuck underground and does not recover until you move the camera again. Or if, in the future, terrain is dynamic and goes above the camera, the camera won't snap upwards.

Not sure what the best fix is. It has to play nicely with camera.setView which wants to preserve the camera location.