From 73bf8c040c58f2fc8e676323d4e10cb6cb1f450c Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Tue, 7 Aug 2018 20:50:54 -0400 Subject: [PATCH 1/3] Fix viewer.zoomTo and viewer.flyTo imagery when using terrain. 1. Pull `computeFlyToLocationForRectangle` out of Geocoder and make it a standalone private function. Add tests for it too. 2. Have Viewer call the function when zooming or flying to terrain. --- CHANGES.md | 1 + .../Scene/computeFlyToLocationForRectangle.js | 68 ++++++++++ Source/Widgets/Geocoder/GeocoderViewModel.js | 42 +------ Source/Widgets/Viewer/Viewer.js | 17 ++- .../computeFlyToLocationForRectangleSpec.js | 117 ++++++++++++++++++ 5 files changed, 203 insertions(+), 42 deletions(-) create mode 100644 Source/Scene/computeFlyToLocationForRectangle.js create mode 100644 Specs/Scene/computeFlyToLocationForRectangleSpec.js diff --git a/CHANGES.md b/CHANGES.md index 9e9fb139f571..4713a0e4f377 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,6 +11,7 @@ Change Log ##### Fixes :wrench: * The Geocoder widget now takes terrain altitude into account when calculating its final destination. +* The Viewer widget now takes terrain altitude into account when zooming or flying to imagery layers. * Fixed bug that caused a new `ClippingPlaneCollection` to be created every frame when used with a model entity [#6872](https://github.com/AnalyticalGraphicsInc/cesium/pull/6872) ### 1.48 - 2018-08-01 diff --git a/Source/Scene/computeFlyToLocationForRectangle.js b/Source/Scene/computeFlyToLocationForRectangle.js new file mode 100644 index 000000000000..1b028c524cb4 --- /dev/null +++ b/Source/Scene/computeFlyToLocationForRectangle.js @@ -0,0 +1,68 @@ +define([ + '../Core/defined', + '../Core/Rectangle', + '../Core/sampleTerrainMostDetailed', + './SceneMode', + '../ThirdParty/when' +], function( + defined, + Rectangle, + sampleTerrainMostDetailed, + SceneMode, + when) { +'use strict'; + + /** + * Computes the final camera location to view a rectangle adjusted for terrain. + * + * @param {Rectangle} rectangle The rectangle being zoomed to. + * @param {Scene} scene The scene being used. + * + * @returns {Cartographic|Rectangle} The location to place the camera or the original rectangle if terrain does not have availability. + * + * @private + */ + function computeFlyToLocationForRectangle(rectangle, scene) { + var terrainProvider = scene.terrainProvider; + var availability = defined(terrainProvider) ? terrainProvider.availability : undefined; + + if (!defined(availability)) { + return when.resolve(rectangle); + } + + var cartographics = [ + Rectangle.center(rectangle), + Rectangle.southeast(rectangle), + Rectangle.southwest(rectangle), + Rectangle.northeast(rectangle), + Rectangle.northwest(rectangle) + ]; + + return computeFlyToLocationForRectangle._sampleTerrainMostDetailed(terrainProvider, cartographics) + .then(function(positionsOnTerrain) { + var maxHeight = positionsOnTerrain.reduce(function(currentMax, item) { + return Math.max(item.height, currentMax); + }, -Number.MAX_VALUE); + + var finalPosition; + + var camera = scene.camera; + var mapProjection = scene.mapProjection; + var ellipsoid = mapProjection.ellipsoid; + var tmp = camera.getRectangleCameraCoordinates(rectangle); + if (scene.mode === SceneMode.SCENE3D) { + finalPosition = ellipsoid.cartesianToCartographic(tmp); + } else { + finalPosition = mapProjection.unproject(tmp); + } + + finalPosition.height += maxHeight; + return finalPosition; + }); + } + + //Exposed for testing. + computeFlyToLocationForRectangle._sampleTerrainMostDetailed = sampleTerrainMostDetailed; + + return computeFlyToLocationForRectangle; +}); diff --git a/Source/Widgets/Geocoder/GeocoderViewModel.js b/Source/Widgets/Geocoder/GeocoderViewModel.js index 64b128b6247d..344d83e9dedf 100644 --- a/Source/Widgets/Geocoder/GeocoderViewModel.js +++ b/Source/Widgets/Geocoder/GeocoderViewModel.js @@ -11,7 +11,7 @@ define([ '../../Core/Matrix4', '../../Core/Rectangle', '../../Core/sampleTerrainMostDetailed', - '../../Scene/SceneMode', + '../../Scene/computeFlyToLocationForRectangle', '../../ThirdParty/knockout', '../../ThirdParty/when', '../createCommand', @@ -29,7 +29,7 @@ define([ Matrix4, Rectangle, sampleTerrainMostDetailed, - SceneMode, + computeFlyToLocationForRectangle, knockout, when, createCommand, @@ -341,42 +341,6 @@ define([ adjustSuggestionsScroll(viewModel, next); } - function computeFlyToLocationForRectangle(rectangle, terrainProvider, camera, mapProjection, sceneMode) { - var availability = defined(terrainProvider) ? terrainProvider.availability : undefined; - - if (!defined(availability)) { - return when.resolve(rectangle); - } - - var cartographics = [ - Rectangle.center(rectangle), - Rectangle.southeast(rectangle), - Rectangle.southwest(rectangle), - Rectangle.northeast(rectangle), - Rectangle.northwest(rectangle) - ]; - - return sampleTerrainMostDetailed(terrainProvider, cartographics) - .then(function(positionsOnTerrain) { - var maxHeight = positionsOnTerrain.reduce(function(currentMax, item) { - return Math.max(item.height, currentMax); - }, -Number.MAX_VALUE); - - var finalPosition; - - var ellipsoid = mapProjection.ellipsoid; - var tmp = camera.getRectangleCameraCoordinates(rectangle); - if (sceneMode === SceneMode.SCENE3D) { - finalPosition = ellipsoid.cartesianToCartographic(tmp); - } else { - finalPosition = mapProjection.unproject(tmp, tmp); - } - - finalPosition.height += maxHeight; - return finalPosition; - }); - } - function computeFlyToLocationForCartographic(cartographic, terrainProvider) { var availability = defined(terrainProvider) ? terrainProvider.availability : undefined; @@ -410,7 +374,7 @@ define([ // destination is now a Cartographic destination = Rectangle.center(destination); } else { - promise = computeFlyToLocationForRectangle(destination, terrainProvider, camera, mapProjection, scene.mode); + promise = computeFlyToLocationForRectangle(destination, scene); } } else { // destination is a Cartesian3 destination = ellipsoid.cartesianToCartographic(destination); diff --git a/Source/Widgets/Viewer/Viewer.js b/Source/Widgets/Viewer/Viewer.js index b55c5b124afb..57419cdd78d0 100644 --- a/Source/Widgets/Viewer/Viewer.js +++ b/Source/Widgets/Viewer/Viewer.js @@ -1,6 +1,7 @@ define([ '../../Core/BoundingSphere', '../../Core/Cartesian3', + '../../Core/Cartographic', '../../Core/Clock', '../../Core/defaultValue', '../../Core/defined', @@ -22,6 +23,7 @@ define([ '../../DataSources/EntityView', '../../DataSources/Property', '../../Scene/Cesium3DTileset', + '../../Scene/computeFlyToLocationForRectangle', '../../Scene/ImageryLayer', '../../Scene/SceneMode', '../../Scene/TimeDynamicPointCloud', @@ -49,6 +51,7 @@ define([ ], function( BoundingSphere, Cartesian3, + Cartographic, Clock, defaultValue, defined, @@ -70,6 +73,7 @@ define([ EntityView, Property, Cesium3DTileset, + computeFlyToLocationForRectangle, ImageryLayer, SceneMode, TimeDynamicPointCloud, @@ -1810,9 +1814,11 @@ Either specify options.terrainProvider instead or set options.baseLayerPicker to //If the zoom target is a rectangular imagery in an ImageLayer if (zoomTarget instanceof ImageryLayer) { zoomTarget.getViewableRectangle().then(function(rectangle) { + return computeFlyToLocationForRectangle(rectangle, that.scene); + }).then(function(position) { //Only perform the zoom if it wasn't cancelled before the promise was resolved if (that._zoomPromise === zoomPromise) { - that._zoomTarget = rectangle; + that._zoomTarget = position; } }); return; @@ -1976,9 +1982,14 @@ Either specify options.terrainProvider instead or set options.baseLayerPicker to } // If zoomTarget was an ImageryLayer - if (target instanceof Rectangle) { + var isCartographic = target instanceof Cartographic; + if (target instanceof Rectangle || isCartographic) { + var destination = target; + if (isCartographic) { + destination = scene.mapProjection.ellipsoid.cartographicToCartesian(destination); + } options = { - destination : target, + destination : destination, duration : zoomOptions.duration, maximumHeight : zoomOptions.maximumHeight, complete : function() { diff --git a/Specs/Scene/computeFlyToLocationForRectangleSpec.js b/Specs/Scene/computeFlyToLocationForRectangleSpec.js new file mode 100644 index 000000000000..b85376740514 --- /dev/null +++ b/Specs/Scene/computeFlyToLocationForRectangleSpec.js @@ -0,0 +1,117 @@ +defineSuite([ + 'Scene/computeFlyToLocationForRectangle', + 'Core/EllipsoidTerrainProvider', + 'Core/Rectangle', + 'Scene/Globe', + 'Scene/SceneMode', + 'Specs/createScene', + 'ThirdParty/when' + ], function( + computeFlyToLocationForRectangle, + EllipsoidTerrainProvider, + Rectangle, + Globe, + SceneMode, + createScene, + when) { + 'use strict'; + + var scene; + + beforeEach(function() { + scene = createScene(); + }); + + afterEach(function() { + scene.destroyForSpecs(); + }); + + function sampleTest(sceneMode){ + //Pretend we have terrain with availability. + var terrainProvider = new EllipsoidTerrainProvider(); + terrainProvider.availability = {}; + + scene.globe = new Globe(); + scene.terrainProvider = terrainProvider; + scene.mode = sceneMode; + + var rectangle = new Rectangle(0.2, 0.4, 0.6, 0.8); + var cartographics = [ + Rectangle.center(rectangle), + Rectangle.southeast(rectangle), + Rectangle.southwest(rectangle), + Rectangle.northeast(rectangle), + Rectangle.northwest(rectangle) + ]; + + // Mock sampleTerrainMostDetailed with same positions but with heights. + var maxHeight = 1234; + var sampledResults = [ + Rectangle.center(rectangle), + Rectangle.southeast(rectangle), + Rectangle.southwest(rectangle), + Rectangle.northeast(rectangle), + Rectangle.northwest(rectangle) + ]; + sampledResults[0].height = 145; + sampledResults[1].height = 1211; + sampledResults[2].height = -123; + sampledResults[3].height = maxHeight; + + spyOn(computeFlyToLocationForRectangle, '_sampleTerrainMostDetailed').and.returnValue(when.resolve(sampledResults)); + + // Basically do the computation ourselves with our known values; + var expectedResult; + if (sceneMode === SceneMode.SCENE3D) { + expectedResult = scene.mapProjection.ellipsoid.cartesianToCartographic(scene.camera.getRectangleCameraCoordinates(rectangle)); + expectedResult.height += maxHeight; + } else { + expectedResult = scene.mapProjection.unproject(scene.camera.getRectangleCameraCoordinates(rectangle)); + expectedResult.height += maxHeight; + } + + return computeFlyToLocationForRectangle(rectangle, scene) + .then(function(result) { + expect(result).toEqual(expectedResult); + expect(computeFlyToLocationForRectangle._sampleTerrainMostDetailed).toHaveBeenCalledWith(terrainProvider, cartographics); + }); + } + + it('samples terrain and returns expected result in 3D', function() { + return sampleTest(SceneMode.SCENE2D); + }); + + it('samples terrain and returns expected result in 2D', function() { + return sampleTest(SceneMode.SCENE3D); + }); + + it('samples terrain and returns expected result in CV', function() { + return sampleTest(SceneMode.COLUMBUS_VIEW); + }); + + it('returns original rectangle when terrain not available', function() { + scene.globe = new Globe(); + scene.terrainProvider = new EllipsoidTerrainProvider(); + + var rectangle = new Rectangle(0.2, 0.4, 0.6, 0.8); + spyOn(computeFlyToLocationForRectangle, '_sampleTerrainMostDetailed'); + + return computeFlyToLocationForRectangle(rectangle, scene) + .then(function(result) { + expect(result).toBe(rectangle); + expect(computeFlyToLocationForRectangle._sampleTerrainMostDetailed).not.toHaveBeenCalled(); + }); + }); + + it('returns original rectangle when terrain undefined', function() { + scene.terrainProvider = undefined; + var rectangle = new Rectangle(0.2, 0.4, 0.6, 0.8); + spyOn(computeFlyToLocationForRectangle, '_sampleTerrainMostDetailed'); + + return computeFlyToLocationForRectangle(rectangle, scene) + .then(function(result) { + expect(result).toBe(rectangle); + expect(computeFlyToLocationForRectangle._sampleTerrainMostDetailed).not.toHaveBeenCalled(); + }); + }); +}); From ac460d42ca1fa49b89d1c42ec02d8cbf15a83b38 Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Tue, 7 Aug 2018 21:01:15 -0400 Subject: [PATCH 2/3] There's no reason to sample terrain in 2D. --- .../Scene/computeFlyToLocationForRectangle.js | 2 +- .../computeFlyToLocationForRectangleSpec.js | 27 ++++++++++++++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/Source/Scene/computeFlyToLocationForRectangle.js b/Source/Scene/computeFlyToLocationForRectangle.js index 1b028c524cb4..296e3ddb1bbf 100644 --- a/Source/Scene/computeFlyToLocationForRectangle.js +++ b/Source/Scene/computeFlyToLocationForRectangle.js @@ -26,7 +26,7 @@ define([ var terrainProvider = scene.terrainProvider; var availability = defined(terrainProvider) ? terrainProvider.availability : undefined; - if (!defined(availability)) { + if (!defined(availability) || scene.mode === SceneMode.SCENE2D) { return when.resolve(rectangle); } diff --git a/Specs/Scene/computeFlyToLocationForRectangleSpec.js b/Specs/Scene/computeFlyToLocationForRectangleSpec.js index b85376740514..a489564a8c12 100644 --- a/Specs/Scene/computeFlyToLocationForRectangleSpec.js +++ b/Specs/Scene/computeFlyToLocationForRectangleSpec.js @@ -1,4 +1,4 @@ -defineSuite([ +fdefineSuite([ 'Scene/computeFlyToLocationForRectangle', 'Core/EllipsoidTerrainProvider', 'Core/Rectangle', @@ -64,11 +64,10 @@ defineSuite([ var expectedResult; if (sceneMode === SceneMode.SCENE3D) { expectedResult = scene.mapProjection.ellipsoid.cartesianToCartographic(scene.camera.getRectangleCameraCoordinates(rectangle)); - expectedResult.height += maxHeight; } else { expectedResult = scene.mapProjection.unproject(scene.camera.getRectangleCameraCoordinates(rectangle)); - expectedResult.height += maxHeight; } + expectedResult.height += maxHeight; return computeFlyToLocationForRectangle(rectangle, scene) .then(function(result) { @@ -78,10 +77,6 @@ defineSuite([ } it('samples terrain and returns expected result in 3D', function() { - return sampleTest(SceneMode.SCENE2D); - }); - - it('samples terrain and returns expected result in 2D', function() { return sampleTest(SceneMode.SCENE3D); }); @@ -89,6 +84,24 @@ defineSuite([ return sampleTest(SceneMode.COLUMBUS_VIEW); }); + it('returns original rectangle in 2D', function() { + var terrainProvider = new EllipsoidTerrainProvider(); + terrainProvider.availability = {}; + + scene.globe = new Globe(); + scene.terrainProvider = terrainProvider; + scene.mode = SceneMode.SCENE2D; + + var rectangle = new Rectangle(0.2, 0.4, 0.6, 0.8); + spyOn(computeFlyToLocationForRectangle, '_sampleTerrainMostDetailed'); + + return computeFlyToLocationForRectangle(rectangle, scene) + .then(function(result) { + expect(result).toBe(rectangle); + expect(computeFlyToLocationForRectangle._sampleTerrainMostDetailed).not.toHaveBeenCalled(); + }); + }); + it('returns original rectangle when terrain not available', function() { scene.globe = new Globe(); scene.terrainProvider = new EllipsoidTerrainProvider(); From dfd1d5dc1b8af414986142f842acb779a42de908 Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Tue, 7 Aug 2018 21:17:27 -0400 Subject: [PATCH 3/3] Remove stray fdefineSuite --- Specs/Scene/computeFlyToLocationForRectangleSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Specs/Scene/computeFlyToLocationForRectangleSpec.js b/Specs/Scene/computeFlyToLocationForRectangleSpec.js index a489564a8c12..3b1048afb958 100644 --- a/Specs/Scene/computeFlyToLocationForRectangleSpec.js +++ b/Specs/Scene/computeFlyToLocationForRectangleSpec.js @@ -1,4 +1,4 @@ -fdefineSuite([ +defineSuite([ 'Scene/computeFlyToLocationForRectangle', 'Core/EllipsoidTerrainProvider', 'Core/Rectangle',