From 407dd286a9294d2507ca091ae12a5d55988cd9a4 Mon Sep 17 00:00:00 2001 From: Emma Krantz Date: Fri, 26 Apr 2019 10:26:03 +1000 Subject: [PATCH 1/5] added support for Bing Maps' AerialWithLabelsOnDemand imagery set, as requested in terriajs#3113 --- Source/Scene/BingMapsImageryProvider.js | 46 ++++++++++++++++++--- Source/Scene/BingMapsStyle.js | 10 +++++ Source/Scene/DiscardEmptyTileImagePolicy.js | 36 ++++++++++++++++ 3 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 Source/Scene/DiscardEmptyTileImagePolicy.js diff --git a/Source/Scene/BingMapsImageryProvider.js b/Source/Scene/BingMapsImageryProvider.js index a403bd1338de..df9fb26b8534 100644 --- a/Source/Scene/BingMapsImageryProvider.js +++ b/Source/Scene/BingMapsImageryProvider.js @@ -18,6 +18,7 @@ define([ '../ThirdParty/when', './BingMapsStyle', './DiscardMissingTileImagePolicy', + './DiscardEmptyTileImagePolicy', './ImageryProvider' ], function( BingMapsApi, @@ -39,6 +40,7 @@ define([ when, BingMapsStyle, DiscardMissingTileImagePolicy, + DiscardEmptyTilePolicy, ImageryProvider) { 'use strict'; @@ -71,6 +73,18 @@ define([ * that no tiles are discarded, construct and pass a {@link NeverTileDiscardPolicy} for this * parameter. * + * @param {TileDiscardPolicy} [options.tileDiscardPolicy] The policy that determines if a tile + * is invalid and should be discarded. The default value will depend on the map style. If + * BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND is used, then a {@link DiscardEmptyTileImagePolicy} will be + * used to handle the Bing Maps API sending no content instead of a missing tile image, a behaviour specific + * to that imagery set. In all over cases, a default {@link DiscardMissingTileImagePolicy} is used which + * requests tile 0,0 at the maximum tile level and checks pixels (0,0), (120,140), (130,160), (200,50), and + * (200,200). If all of these pixels are transparent, the discard check is disabled and no tiles are + * discarded. If any of them have a non-transparent color, any tile that has the same values in these pixel + * locations is discarded. The end result of these defaults should be correct tile discarding for a standard + * Bing Maps server. To ensure that no tiles are discarded, construct and pass a + * {@link NeverTileDiscardPolicy} for this parameter. + * * @see ArcGisMapServerImageryProvider * @see GoogleEarthEnterpriseMapsProvider * @see createOpenStreetMapImageryProvider @@ -171,11 +185,17 @@ define([ // Install the default tile discard policy if none has been supplied. if (!defined(that._tileDiscardPolicy)) { - that._tileDiscardPolicy = new DiscardMissingTileImagePolicy({ - missingImageUrl : buildImageResource(that, 0, 0, that._maximumLevel).url, - pixelsToCheck : [new Cartesian2(0, 0), new Cartesian2(120, 140), new Cartesian2(130, 160), new Cartesian2(200, 50), new Cartesian2(200, 200)], - disableCheckIfAllPixelsAreTransparent : true - }); + // Our default depends on which map style we're using. + if (that._mapStyle === BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND) { + // this map style uses a different API, which returns a tile with no data instead of a placeholder image + that._tileDiscardPolicy = new DiscardEmptyTilePolicy(); + } else { + that._tileDiscardPolicy = new DiscardMissingTileImagePolicy({ + missingImageUrl : buildImageResource(that, 0, 0, that._maximumLevel).url, + pixelsToCheck : [new Cartesian2(0, 0), new Cartesian2(120, 140), new Cartesian2(130, 160), new Cartesian2(200, 50), new Cartesian2(200, 200)], + disableCheckIfAllPixelsAreTransparent : true + }); + } } var attributionList = that._attributionList = resource.imageryProviders; @@ -533,7 +553,21 @@ define([ } //>>includeEnd('debug'); - return ImageryProvider.loadImage(this, buildImageResource(this, x, y, level, request)); + var promise = ImageryProvider.loadImage(this, buildImageResource(this, x, y, level, request)); + + if(defined(promise)) { + return promise.then(() => promise, (error) => { + // If there isn't any image, we get an InvalidStateError. + // The version of the Bing maps API that we hit if we're using `BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND` does this + // instead of returning a "This image is missing" image. + // Since this isn't really an error, we want to supress the one that just got thrown. + if (error.name === 'InvalidStateError' && error.message === 'The source image could not be decoded.') { + return {}; + } + }); + } + + return undefined; }; /** diff --git a/Source/Scene/BingMapsStyle.js b/Source/Scene/BingMapsStyle.js index 1415ed6f0220..20dec725b899 100644 --- a/Source/Scene/BingMapsStyle.js +++ b/Source/Scene/BingMapsStyle.js @@ -25,9 +25,19 @@ define([ * * @type {String} * @constant + * @deprecated See https://github.com/AnalyticalGraphicsInc/cesium/issues/7128. + * Use `BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND` instead */ AERIAL_WITH_LABELS : 'AerialWithLabels', + /** + * Aerial imagery with a road overlay. + * + * @type {String} + * @constant + */ + AERIAL_WITH_LABELS_ON_DEMAND : 'AerialWithLabelsOnDemand', + /** * Roads without additional imagery. * diff --git a/Source/Scene/DiscardEmptyTileImagePolicy.js b/Source/Scene/DiscardEmptyTileImagePolicy.js new file mode 100644 index 000000000000..e788786da92c --- /dev/null +++ b/Source/Scene/DiscardEmptyTileImagePolicy.js @@ -0,0 +1,36 @@ +define([ + '../Core/defined' +], function( + defined) { + /** + * A policy for discarding tile images that contain no data (and so aren't actually images). + * + * @alias DiscardEmptyTileImagePolicy + * @constructor + * + * @see DiscardMissingTileImagePolicy + */ + function DiscardEmptyTileImagePolicy(options) { + } + + /** + * Determines if the discard policy is ready to process images. + * @returns {Boolean} True if the discard policy is ready to process images; otherwise, false. + */ + DiscardEmptyTileImagePolicy.prototype.isReady = function() { + return true; + }; + + /** + * Given a tile image, decide whether to discard that image. + * + * @param {Image} image An image to test. + * @returns {Boolean} True if the image should be discarded; otherwise, false. + */ + DiscardEmptyTileImagePolicy.prototype.shouldDiscardImage = function(image) { + // If the image blob is non-existent, or its size is zero, then discard it + return !defined(image.blob) || image.blob.size === 0; + }; + + return DiscardEmptyTileImagePolicy; +}); From f0e3195f4bb276b706e7c9714994b3dc0f1a8ffe Mon Sep 17 00:00:00 2001 From: Emma Krantz Date: Thu, 2 May 2019 14:22:32 +1000 Subject: [PATCH 2/5] Better error handling for empty imagery tiles. Incorporates PR suggestions from https://github.com/TerriaJS/cesium/pull/41 --- Source/Core/Resource.js | 2 ++ Source/Scene/BingMapsImageryProvider.js | 22 ++++++++++++++------- Source/Scene/BingMapsStyle.js | 10 ++++++++++ Source/Scene/DiscardEmptyTileImagePolicy.js | 20 +++++++++++++++---- 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/Source/Core/Resource.js b/Source/Core/Resource.js index 54afd88ea9d0..00c387e0ab32 100644 --- a/Source/Core/Resource.js +++ b/Source/Core/Resource.js @@ -957,6 +957,8 @@ define([ window.URL.revokeObjectURL(generatedBlobResource.url); } + error.blob = generatedBlob; + return when.reject(error); }); }; diff --git a/Source/Scene/BingMapsImageryProvider.js b/Source/Scene/BingMapsImageryProvider.js index df9fb26b8534..e40241f5d58d 100644 --- a/Source/Scene/BingMapsImageryProvider.js +++ b/Source/Scene/BingMapsImageryProvider.js @@ -556,14 +556,22 @@ define([ var promise = ImageryProvider.loadImage(this, buildImageResource(this, x, y, level, request)); if(defined(promise)) { - return promise.then(() => promise, (error) => { - // If there isn't any image, we get an InvalidStateError. - // The version of the Bing maps API that we hit if we're using `BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND` does this - // instead of returning a "This image is missing" image. - // Since this isn't really an error, we want to supress the one that just got thrown. - if (error.name === 'InvalidStateError' && error.message === 'The source image could not be decoded.') { - return {}; + return promise.otherwise( (error) => { + + // One possible cause of an error here is that the image we tried to load was empty. This isn't actually + // a problem. In some imagery sets (eg. `BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND`), an empty image is + // returned rather than a blank "This Image is Missing" placeholder image. In this case, we supress the + // error. + if (defined(error.blob) && error.blob.size === 0) { + + // If our tile discard policy tells us how to represent an empty image, we return that. + if (defined(this._tileDiscardPolicy.emptyImage)) { + return this._tileDiscardPolicy.emptyImage; + } + // Otherwise, we return a default. + return DiscardEmptyTilePolicy.DEFAULT_EMPTY_IMAGE; } + return when.reject(error); }); } diff --git a/Source/Scene/BingMapsStyle.js b/Source/Scene/BingMapsStyle.js index 20dec725b899..151d23f3e3ba 100644 --- a/Source/Scene/BingMapsStyle.js +++ b/Source/Scene/BingMapsStyle.js @@ -43,9 +43,19 @@ define([ * * @type {String} * @constant + * @deprecated See https://github.com/AnalyticalGraphicsInc/cesium/issues/7128. + * Use `BingMapsStyle.ROAD_ON_DEMAND` instead */ ROAD : 'Road', + /** + * Roads without additional imagery. + * + * @type {String} + * @constant + */ + ROAD_ON_DEMAND : 'RoadOnDemand', + /** * A dark version of the road maps. * diff --git a/Source/Scene/DiscardEmptyTileImagePolicy.js b/Source/Scene/DiscardEmptyTileImagePolicy.js index e788786da92c..5b82c6a671c8 100644 --- a/Source/Scene/DiscardEmptyTileImagePolicy.js +++ b/Source/Scene/DiscardEmptyTileImagePolicy.js @@ -1,16 +1,24 @@ define([ - '../Core/defined' + '../Core/defined', + '../Core/defaultValue' ], function( - defined) { + defined, + defaultValue) { + /** * A policy for discarding tile images that contain no data (and so aren't actually images). * * @alias DiscardEmptyTileImagePolicy * @constructor * + * @param {any} [options.emptyImage={}] to compare an image to when deciding whether to discard a tile. + * * @see DiscardMissingTileImagePolicy */ function DiscardEmptyTileImagePolicy(options) { + options = defaultValue(options, {emptyImage: DiscardEmptyTileImagePolicy.DEFAULT_EMPTY_IMAGE}); + + this.emptyImage = options.emptyImage; } /** @@ -28,9 +36,13 @@ define([ * @returns {Boolean} True if the image should be discarded; otherwise, false. */ DiscardEmptyTileImagePolicy.prototype.shouldDiscardImage = function(image) { - // If the image blob is non-existent, or its size is zero, then discard it - return !defined(image.blob) || image.blob.size === 0; + return this.emptyImage === image; }; + /** + * Default value for representing an empty image. + */ + DiscardEmptyTileImagePolicy.DEFAULT_EMPTY_IMAGE = {}; + return DiscardEmptyTileImagePolicy; }); From 64cbe063d33a20380974de549538728b5ec9420c Mon Sep 17 00:00:00 2001 From: Emma Krantz Date: Thu, 2 May 2019 14:32:02 +1000 Subject: [PATCH 3/5] Fixed documentation for BingMapsImageryProvider, corrected default tile discard policy for BingMapsStyle.ROAD_ON_DEMAND --- Source/Scene/BingMapsImageryProvider.js | 32 +++++++++---------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/Source/Scene/BingMapsImageryProvider.js b/Source/Scene/BingMapsImageryProvider.js index e40241f5d58d..a0c333f5952f 100644 --- a/Source/Scene/BingMapsImageryProvider.js +++ b/Source/Scene/BingMapsImageryProvider.js @@ -63,27 +63,16 @@ define([ * for information on the supported cultures. * @param {Ellipsoid} [options.ellipsoid] The ellipsoid. If not specified, the WGS84 ellipsoid is used. * @param {TileDiscardPolicy} [options.tileDiscardPolicy] The policy that determines if a tile - * is invalid and should be discarded. If this value is not specified, a default - * {@link DiscardMissingTileImagePolicy} is used which requests - * tile 0,0 at the maximum tile level and checks pixels (0,0), (120,140), (130,160), - * (200,50), and (200,200). If all of these pixels are transparent, the discard check is - * disabled and no tiles are discarded. If any of them have a non-transparent color, any - * tile that has the same values in these pixel locations is discarded. The end result of - * these defaults should be correct tile discarding for a standard Bing Maps server. To ensure - * that no tiles are discarded, construct and pass a {@link NeverTileDiscardPolicy} for this - * parameter. - * - * @param {TileDiscardPolicy} [options.tileDiscardPolicy] The policy that determines if a tile * is invalid and should be discarded. The default value will depend on the map style. If - * BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND is used, then a {@link DiscardEmptyTileImagePolicy} will be - * used to handle the Bing Maps API sending no content instead of a missing tile image, a behaviour specific - * to that imagery set. In all over cases, a default {@link DiscardMissingTileImagePolicy} is used which - * requests tile 0,0 at the maximum tile level and checks pixels (0,0), (120,140), (130,160), (200,50), and - * (200,200). If all of these pixels are transparent, the discard check is disabled and no tiles are - * discarded. If any of them have a non-transparent color, any tile that has the same values in these pixel - * locations is discarded. The end result of these defaults should be correct tile discarding for a standard - * Bing Maps server. To ensure that no tiles are discarded, construct and pass a - * {@link NeverTileDiscardPolicy} for this parameter. + * `BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND` or `BingMapsStyle.ROADS_ON_DEMAND` is used, then a + * {@link DiscardEmptyTileImagePolicy} will be used to handle the Bing Maps API sending no content instead of + * a missing tile image, a behaviour specific to that imagery set. In all over cases, a default + * {@link DiscardMissingTileImagePolicy} is used which requests tile 0,0 at the maximum tile level and checks + * pixels (0,0), (120,140), (130,160), (200,50), and (200,200). If all of these pixels are transparent, the + * discard check is disabled and no tiles are discarded. If any of them have a non-transparent color, any + * tile that has the same values in these pixel locations is discarded. The end result of these defaults + * should be correct tile discarding for a standard Bing Maps server. To ensure that no tiles are discarded, + * construct and pass a {@link NeverTileDiscardPolicy} for this parameter. * * @see ArcGisMapServerImageryProvider * @see GoogleEarthEnterpriseMapsProvider @@ -186,7 +175,8 @@ define([ // Install the default tile discard policy if none has been supplied. if (!defined(that._tileDiscardPolicy)) { // Our default depends on which map style we're using. - if (that._mapStyle === BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND) { + if (that._mapStyle === BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND + || that._mapStyle === BingMapsStyle.ROAD_ON_DEMAND) { // this map style uses a different API, which returns a tile with no data instead of a placeholder image that._tileDiscardPolicy = new DiscardEmptyTilePolicy(); } else { From f2049ff40a719271d85e003191e0284ec97b46e2 Mon Sep 17 00:00:00 2001 From: Emma Krantz Date: Thu, 2 May 2019 17:52:14 +1000 Subject: [PATCH 4/5] unparameterize DiscardEmptyTileImagePolicy to reduce unneccessary complexity --- Source/Scene/BingMapsImageryProvider.js | 10 ++-------- Source/Scene/DiscardEmptyTileImagePolicy.js | 9 ++------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/Source/Scene/BingMapsImageryProvider.js b/Source/Scene/BingMapsImageryProvider.js index a0c333f5952f..fac03720bd38 100644 --- a/Source/Scene/BingMapsImageryProvider.js +++ b/Source/Scene/BingMapsImageryProvider.js @@ -546,20 +546,14 @@ define([ var promise = ImageryProvider.loadImage(this, buildImageResource(this, x, y, level, request)); if(defined(promise)) { - return promise.otherwise( (error) => { + return promise.otherwise((error) => { // One possible cause of an error here is that the image we tried to load was empty. This isn't actually // a problem. In some imagery sets (eg. `BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND`), an empty image is // returned rather than a blank "This Image is Missing" placeholder image. In this case, we supress the // error. if (defined(error.blob) && error.blob.size === 0) { - - // If our tile discard policy tells us how to represent an empty image, we return that. - if (defined(this._tileDiscardPolicy.emptyImage)) { - return this._tileDiscardPolicy.emptyImage; - } - // Otherwise, we return a default. - return DiscardEmptyTilePolicy.DEFAULT_EMPTY_IMAGE; + return DiscardEmptyTilePolicy.EMPTY_IMAGE; } return when.reject(error); }); diff --git a/Source/Scene/DiscardEmptyTileImagePolicy.js b/Source/Scene/DiscardEmptyTileImagePolicy.js index 5b82c6a671c8..f63dbaed3ea6 100644 --- a/Source/Scene/DiscardEmptyTileImagePolicy.js +++ b/Source/Scene/DiscardEmptyTileImagePolicy.js @@ -11,14 +11,9 @@ define([ * @alias DiscardEmptyTileImagePolicy * @constructor * - * @param {any} [options.emptyImage={}] to compare an image to when deciding whether to discard a tile. - * * @see DiscardMissingTileImagePolicy */ function DiscardEmptyTileImagePolicy(options) { - options = defaultValue(options, {emptyImage: DiscardEmptyTileImagePolicy.DEFAULT_EMPTY_IMAGE}); - - this.emptyImage = options.emptyImage; } /** @@ -36,13 +31,13 @@ define([ * @returns {Boolean} True if the image should be discarded; otherwise, false. */ DiscardEmptyTileImagePolicy.prototype.shouldDiscardImage = function(image) { - return this.emptyImage === image; + return DiscardEmptyTileImagePolicy.EMPTY_IMAGE === image; }; /** * Default value for representing an empty image. */ - DiscardEmptyTileImagePolicy.DEFAULT_EMPTY_IMAGE = {}; + DiscardEmptyTileImagePolicy.EMPTY_IMAGE = {}; return DiscardEmptyTileImagePolicy; }); From 3d2dc9e8d3118a5f9e9f1c4fdf688a3d57255310 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 2 May 2019 20:53:53 +1000 Subject: [PATCH 5/5] Don't use arrow function because Cesium targets ES5. --- Source/Scene/BingMapsImageryProvider.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Scene/BingMapsImageryProvider.js b/Source/Scene/BingMapsImageryProvider.js index fac03720bd38..d17d2a57de9b 100644 --- a/Source/Scene/BingMapsImageryProvider.js +++ b/Source/Scene/BingMapsImageryProvider.js @@ -545,8 +545,8 @@ define([ var promise = ImageryProvider.loadImage(this, buildImageResource(this, x, y, level, request)); - if(defined(promise)) { - return promise.otherwise((error) => { + if (defined(promise)) { + return promise.otherwise(function(error) { // One possible cause of an error here is that the image we tried to load was empty. This isn't actually // a problem. In some imagery sets (eg. `BingMapsStyle.AERIAL_WITH_LABELS_ON_DEMAND`), an empty image is