Make Geocoder terrain-aware #6876
+95
−21
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,27 +1,37 @@ | ||
| define([ | ||
| '../../Core/IonGeocoderService', | ||
| '../../Core/Cartesian3', | ||
| '../../Core/CartographicGeocoderService', | ||
| '../../Core/defaultValue', | ||
| '../../Core/defined', | ||
| '../../Core/defineProperties', | ||
| '../../Core/DeveloperError', | ||
| '../../Core/Event', | ||
| '../../Core/GeocodeType', | ||
| '../../Core/Math', | ||
| '../../Core/Matrix4', | ||
| '../../Core/Rectangle', | ||
| '../../Core/sampleTerrainMostDetailed', | ||
| '../../Scene/SceneMode', | ||
| '../../ThirdParty/knockout', | ||
| '../../ThirdParty/when', | ||
| '../createCommand', | ||
| '../getElement' | ||
| ], function( | ||
| IonGeocoderService, | ||
| Cartesian3, | ||
| CartographicGeocoderService, | ||
| defaultValue, | ||
| defined, | ||
| defineProperties, | ||
| DeveloperError, | ||
| Event, | ||
| GeocodeType, | ||
| CesiumMath, | ||
| Matrix4, | ||
| Rectangle, | ||
| sampleTerrainMostDetailed, | ||
| SceneMode, | ||
| knockout, | ||
| when, | ||
| createCommand, | ||
| @@ -331,14 +341,79 @@ define([ | ||
| } | ||
|
|
||
| function updateCamera(viewModel, destination) { | ||
| var scene = viewModel._scene; | ||
| var globe = scene.globe; | ||
| var ellipsoid; | ||
| if (defined(globe)) { | ||
| ellipsoid = globe.ellipsoid; | ||
| } else { | ||
| ellipsoid = scene.mapProjection.ellipsoid; | ||
| } | ||
|
|
||
| var camera = scene.camera; | ||
| var terrainProvider = scene.terrainProvider; | ||
| var availability = defined(terrainProvider) ? terrainProvider.availability : undefined; | ||
|
|
||
| // Always expand single points so we don't zoom | ||
| // directly into the ground, even when not using terrain. | ||
| if (destination instanceof Cartesian3) { | ||
| var carto = ellipsoid.cartesianToCartographic(destination); | ||
| var offset = CesiumMath.toRadians(0.001); | ||
| destination = new Rectangle( | ||
| carto.longitude - offset, | ||
| carto.latitude - offset, | ||
| carto.longitude + offset, | ||
| carto.latitude + offset); | ||
| } | ||
|
|
||
| var newDestination = destination; | ||
| if (defined(availability)) { | ||
| var cartographics; | ||
|
|
||
| cartographics = [ | ||
|
There was a problem hiding this comment. @mramato These don't need to be on two separate lines but oh well. And shouldn't you use result parameters throughout this? I know it won't really impact performance but it's still a good habit There was a problem hiding this comment. We use result parameters as a necessary evil for performance in hot areas of the code. Not using them makes the code cleaner and is perfectly OK if performance is not going to be an issue. |
||
| Rectangle.center(destination), | ||
| Rectangle.southeast(destination), | ||
| Rectangle.southwest(destination), | ||
| Rectangle.northeast(destination), | ||
| Rectangle.northwest(destination) | ||
| ]; | ||
|
|
||
| newDestination = sampleTerrainMostDetailed(terrainProvider, cartographics) | ||
| .then(function(positionsOnTerrain) { | ||
| var maxHeight = -Number.MAX_VALUE; | ||
| positionsOnTerrain.forEach(function(p) { | ||
| maxHeight = Math.max(p.height, maxHeight); | ||
| }); | ||
|
|
||
| var tmp = camera.getRectangleCameraCoordinates(destination); | ||
| var finalPosition; | ||
| if (scene.mode === SceneMode.SCENE3D) { | ||
| finalPosition = ellipsoid.cartesianToCartographic(tmp); | ||
| } else { | ||
| finalPosition = scene.mapProjection.unproject(tmp, tmp); | ||
| } | ||
|
|
||
| finalPosition.height += maxHeight * 2; | ||
| return ellipsoid.cartographicToCartesian(finalPosition); | ||
| }); | ||
| } | ||
|
|
||
| var finalDestination = destination; | ||
| when(newDestination) | ||
| .then(function(result) { | ||
| finalDestination = result; | ||
| }) | ||
| .always(function(){ | ||
| // Whether terrain querying succeeded or not, fly to the destination. | ||
| camera.flyTo({ | ||
| destination: finalDestination, | ||
| complete: function() { | ||
| viewModel._complete.raiseEvent(); | ||
| }, | ||
| duration: viewModel._flightDuration, | ||
| endTransform: Matrix4.IDENTITY | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| function chainPromise(promise, geocoderService, query, geocodeType) { | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 not convinced this is the right thing to do. The rectangle covers a larger area closer to the equator than the poles so the camera destination will have a greater height at the equator. From experimenting, the camera height at the equator is around 500m and at the poles it's 350m.
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.
Yeah, I agree. I was just moving code over from
PeliasGeocoderand the thought just didn't occur to me. Instead I'm going to apply a fixed offset of 500m to any single point rather than expanding it into a rectangle at all. I'll still query the terrain at that single point, so it will still adjust properly and this will make for consistent zooming with and without terrain.