From ec9587639a56f0a1fb1100ff1097c72d5a55cee0 Mon Sep 17 00:00:00 2001 From: Emma Krantz Date: Fri, 3 May 2019 10:56:17 +1000 Subject: [PATCH 1/8] Added support for Bing Maps' AerialWithLabelsOnDemand and RoadOnDemand imagery sets. Cherry-picked from downstream fork, TerriaJS/cesium. --- Source/Core/Resource.js | 2 + Source/Scene/BingMapsImageryProvider.js | 56 +++++++++++++++------ Source/Scene/BingMapsStyle.js | 20 ++++++++ Source/Scene/DiscardEmptyTileImagePolicy.js | 43 ++++++++++++++++ 4 files changed, 106 insertions(+), 15 deletions(-) create mode 100644 Source/Scene/DiscardEmptyTileImagePolicy.js diff --git a/Source/Core/Resource.js b/Source/Core/Resource.js index 0a22f64fe66e..fb9984911628 100644 --- a/Source/Core/Resource.js +++ b/Source/Core/Resource.js @@ -944,6 +944,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 a403bd1338de..d17d2a57de9b 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'; @@ -61,15 +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. + * is invalid and should be discarded. The default value will depend on the map style. If + * `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 @@ -171,11 +174,18 @@ 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 + || 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 { + 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 +543,23 @@ 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.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 + // 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) { + return DiscardEmptyTilePolicy.EMPTY_IMAGE; + } + return when.reject(error); + }); + } + + return undefined; }; /** diff --git a/Source/Scene/BingMapsStyle.js b/Source/Scene/BingMapsStyle.js index 1415ed6f0220..151d23f3e3ba 100644 --- a/Source/Scene/BingMapsStyle.js +++ b/Source/Scene/BingMapsStyle.js @@ -25,17 +25,37 @@ 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. * * @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 new file mode 100644 index 000000000000..f63dbaed3ea6 --- /dev/null +++ b/Source/Scene/DiscardEmptyTileImagePolicy.js @@ -0,0 +1,43 @@ +define([ + '../Core/defined', + '../Core/defaultValue' +], function( + defined, + defaultValue) { + + /** + * 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) { + return DiscardEmptyTileImagePolicy.EMPTY_IMAGE === image; + }; + + /** + * Default value for representing an empty image. + */ + DiscardEmptyTileImagePolicy.EMPTY_IMAGE = {}; + + return DiscardEmptyTileImagePolicy; +}); From 5e41791d8433c1eaf5fcc03e6ca33eabfa8907c3 Mon Sep 17 00:00:00 2001 From: Emma Krantz Date: Fri, 3 May 2019 15:31:32 +1000 Subject: [PATCH 2/8] Added tests for updated bing maps imagery --- CONTRIBUTORS.md | 3 +- Specs/Core/ResourceSpec.js | 21 ++++++++++ Specs/Scene/BingMapsImageryProviderSpec.js | 45 +++++++++++++++++++++- 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 67c60a7b77d3..99ca86db2caa 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -50,12 +50,13 @@ See [CONTRIBUTING.md](CONTRIBUTING.md) for details on how to contribute to Cesiu * [Omar Shehata](https://github.com/OmarShehata) * [Matt Petry](https://github.com/MattPetry) * [Michael Squires](https://github.com/mksquires) -* [NICTA](http://www.nicta.com.au/) +* [NICTA/CSIRO's Data61](https://www.data61.csiro.au/) * [Kevin Ring](https://github.com/kring) * [Keith Grochow](https://github.com/kgrochow) * [Chloe Chen](https://github.com/chloeleichen) * [Arthur Street](https://github.com/RacingTadpole) * [Alex Gilleran](https://github.com/AlexGilleran) + * [Emma Krantz](https://github.com/KeyboardSounds) * [EU Edge](http://euedge.com/) * [Ákos Maróy](https://github.com/akosmaroy) * [Raytheon Intelligence and Information Systems](http://www.raytheon.com/) diff --git a/Specs/Core/ResourceSpec.js b/Specs/Core/ResourceSpec.js index 813881670383..8dbc6021dc81 100644 --- a/Specs/Core/ResourceSpec.js +++ b/Specs/Core/ResourceSpec.js @@ -1310,6 +1310,27 @@ defineSuite([ expect(error).toBeInstanceOf(RequestErrorEvent); }); }); + + it('rejects the promise with extra error information when image errors and options.preferBlob is true', function() { + if (!supportsImageBitmapOptions) { + return; + } + + // Force the fetching of a bad blob that is not an image to trigger the error + spyOn(Resource.prototype, 'fetch').and.returnValue(when.resolve(new Blob([new Uint8Array([])], { type: 'text/plain' }))); + + return Resource.fetchImage({ + url: 'http://example.invalid/testuri.png', + preferImageBitmap: true, + preferBlob: true + }) + .then(function() { + fail('expected promise to reject'); + }) + .otherwise(function(error) { + expect(error.blob).toBeInstanceOf(Blob); + }); + }); }); describe('fetchImage without ImageBitmap', function() { diff --git a/Specs/Scene/BingMapsImageryProviderSpec.js b/Specs/Scene/BingMapsImageryProviderSpec.js index b2621591aa7d..f2a2def3e14f 100644 --- a/Specs/Scene/BingMapsImageryProviderSpec.js +++ b/Specs/Scene/BingMapsImageryProviderSpec.js @@ -14,7 +14,9 @@ defineSuite([ 'Scene/ImageryProvider', 'Scene/ImageryState', 'Specs/pollToPromise', - 'ThirdParty/Uri' + 'ThirdParty/Uri', + 'ThirdParty/when', + 'Scene/DiscardEmptyTileImagePolicy' ], function( BingMapsImageryProvider, appendForwardSlash, @@ -31,7 +33,9 @@ defineSuite([ ImageryProvider, ImageryState, pollToPromise, - Uri) { + Uri, + when, + DiscardEmptyTileImagePolicy) { 'use strict'; var supportsImageBitmapOptions; @@ -498,4 +502,41 @@ defineSuite([ }); }); }); + + it('correctly handles empty tiles', function() { + var url = 'http://foo.bar.invalid'; + var mapStyle = BingMapsStyle.ROAD_ON_DEMAND; + + installFakeMetadataRequest(url, mapStyle); + + var provider = new BingMapsImageryProvider({ + url : url, + mapStyle : mapStyle + }); + + var layer = new ImageryLayer(provider); + + // Fake ImageryProvider.loadImage's expected output in the case of an empty tile + var e = new Error(); + e.blob = {size: 0}; + var errorPromise = when.reject(e); + + spyOn(ImageryProvider, 'loadImage').and.returnValue(errorPromise); + + return pollToPromise(function() { + return provider.ready; + }).then(function() { + var imagery = new Imagery(layer, 0, 0, 0); + imagery.addReference(); + layer._requestImagery(imagery); + RequestScheduler.update(); + + return pollToPromise(function() { + return imagery.state === ImageryState.RECEIVED; + }).then(function() { + expect(imagery.image).toBe(DiscardEmptyTileImagePolicy.EMPTY_IMAGE); + imagery.releaseReference(); + }); + }); + }); }); From 7a4b700396604a2e34e53c680b5a81792dfcc8f4 Mon Sep 17 00:00:00 2001 From: Emma Krantz Date: Fri, 3 May 2019 16:44:44 +1000 Subject: [PATCH 3/8] Added tests for DiscardEmptyTileImagePolicy, updated CHANGES.md --- CHANGES.md | 4 ++ Source/Core/Resource.js | 1 + .../Scene/DiscardEmptyTileImagePolicySpec.js | 54 +++++++++++++++++++ 3 files changed, 59 insertions(+) create mode 100644 Specs/Scene/DiscardEmptyTileImagePolicySpec.js diff --git a/CHANGES.md b/CHANGES.md index 7156ae3d4e9e..a7d2639489cf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,9 @@ Change Log ========== +### 1.58 - 2019-06-01 + +##### Additions :tada: +* Added support for new Bing Maps imagery styles RoadOnDemand and AerialWithLabelsOnDemand. The other versions of these, Road and AerialWithLabels, have been deprecated. [#7808] (https://github.com/AnalyticalGraphicsInc/cesium/pull/7808) ### 1.57 - 2019-05-01 diff --git a/Source/Core/Resource.js b/Source/Core/Resource.js index fb9984911628..b5648423aa31 100644 --- a/Source/Core/Resource.js +++ b/Source/Core/Resource.js @@ -944,6 +944,7 @@ define([ window.URL.revokeObjectURL(generatedBlobResource.url); } + // We attach additional information to the error here so that it can be handled by an ImageProvider. error.blob = generatedBlob; return when.reject(error); diff --git a/Specs/Scene/DiscardEmptyTileImagePolicySpec.js b/Specs/Scene/DiscardEmptyTileImagePolicySpec.js new file mode 100644 index 000000000000..dc0024f9001b --- /dev/null +++ b/Specs/Scene/DiscardEmptyTileImagePolicySpec.js @@ -0,0 +1,54 @@ +defineSuite([ + 'Scene/DiscardEmptyTileImagePolicy', + 'Core/Resource', + 'Specs/pollToPromise', + 'ThirdParty/when' + ], function( + DiscardEmptyTileImagePolicy, + Resource, + pollToPromise, + when) { + 'use strict'; + + afterEach(function() { + Resource._Implementations.createImage = Resource._DefaultImplementations.createImage; + Resource._Implementations.loadWithXhr = Resource._DefaultImplementations.loadWithXhr; + }); + + describe('shouldDiscardImage', function() { + fit('does not discard a non-empty image', function() { + var promises = []; + promises.push(Resource.fetchImage('Data/Images/Green4x4.png')); + + var policy = new DiscardEmptyTileImagePolicy(); + + promises.push(pollToPromise(function() { + return policy.isReady(); + })); + + return when.all(promises, function(results) { + var greenImage = results[0]; + + expect(policy.shouldDiscardImage(greenImage)).toEqual(false); + }); + }); + + fit('discards an empty image', function() { + var promises = []; + promises.push(when.resolve(DiscardEmptyTileImagePolicy.EMPTY_IMAGE)); + + var policy = new DiscardEmptyTileImagePolicy(); + + promises.push(pollToPromise(function() { + return policy.isReady(); + })); + + return when.all(promises, function(results) { + var emptyImage = results[0]; + + expect(policy.shouldDiscardImage(emptyImage)).toEqual(true); + }); + }); + + }); +}); From 5a8d583d1978d473bdefbd9fd0074ce84c74b110 Mon Sep 17 00:00:00 2001 From: Emma Krantz Date: Fri, 3 May 2019 16:58:56 +1000 Subject: [PATCH 4/8] Made comment on image blob loading error handling clearer. --- Source/Core/Resource.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/Core/Resource.js b/Source/Core/Resource.js index b5648423aa31..ad53a48f2ee4 100644 --- a/Source/Core/Resource.js +++ b/Source/Core/Resource.js @@ -944,7 +944,8 @@ define([ window.URL.revokeObjectURL(generatedBlobResource.url); } - // We attach additional information to the error here so that it can be handled by an ImageProvider. + // If the blob load succeeded but the image decode failed, provide access to the blob on the error object because it may provide useful insight. + // In particular, BingMapsImageryProvider uses this to detect the zero-length "image" that some map styles return when a real tile is not available. error.blob = generatedBlob; return when.reject(error); From 7f34fdd8ab843f8a5ee37d75e4794aee15b14839 Mon Sep 17 00:00:00 2001 From: Emma Krantz Date: Fri, 3 May 2019 17:11:28 +1000 Subject: [PATCH 5/8] fixed linter error - 'missing use strict' --- Source/Scene/DiscardEmptyTileImagePolicy.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Source/Scene/DiscardEmptyTileImagePolicy.js b/Source/Scene/DiscardEmptyTileImagePolicy.js index f63dbaed3ea6..2e10ddb1825d 100644 --- a/Source/Scene/DiscardEmptyTileImagePolicy.js +++ b/Source/Scene/DiscardEmptyTileImagePolicy.js @@ -2,8 +2,9 @@ define([ '../Core/defined', '../Core/defaultValue' ], function( - defined, - defaultValue) { + defined, + defaultValue) { + 'use strict'; /** * A policy for discarding tile images that contain no data (and so aren't actually images). From 3ced8e5457a87a18a04332e47fe6e8eca55e1103 Mon Sep 17 00:00:00 2001 From: Scott Hunter Date: Fri, 3 May 2019 10:45:55 -0400 Subject: [PATCH 6/8] Fix typo. --- Source/Scene/BingMapsImageryProvider.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Scene/BingMapsImageryProvider.js b/Source/Scene/BingMapsImageryProvider.js index d17d2a57de9b..df393d739827 100644 --- a/Source/Scene/BingMapsImageryProvider.js +++ b/Source/Scene/BingMapsImageryProvider.js @@ -66,7 +66,7 @@ define([ * is invalid and should be discarded. The default value will depend on the map style. If * `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 + * a missing tile image, a behaviour specific to that imagery set. In all other 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 From 6f6dc98084fb1b8e122a2465333b934be110be7f Mon Sep 17 00:00:00 2001 From: Scott Hunter Date: Fri, 3 May 2019 10:46:02 -0400 Subject: [PATCH 7/8] Remove fit. --- Specs/Scene/DiscardEmptyTileImagePolicySpec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Specs/Scene/DiscardEmptyTileImagePolicySpec.js b/Specs/Scene/DiscardEmptyTileImagePolicySpec.js index dc0024f9001b..eca1d6b1308e 100644 --- a/Specs/Scene/DiscardEmptyTileImagePolicySpec.js +++ b/Specs/Scene/DiscardEmptyTileImagePolicySpec.js @@ -16,7 +16,7 @@ defineSuite([ }); describe('shouldDiscardImage', function() { - fit('does not discard a non-empty image', function() { + it('does not discard a non-empty image', function() { var promises = []; promises.push(Resource.fetchImage('Data/Images/Green4x4.png')); @@ -33,7 +33,7 @@ defineSuite([ }); }); - fit('discards an empty image', function() { + it('discards an empty image', function() { var promises = []; promises.push(when.resolve(DiscardEmptyTileImagePolicy.EMPTY_IMAGE)); From 255081b66d4dac4513aaa0de9ee4fe32b0351ea2 Mon Sep 17 00:00:00 2001 From: Scott Hunter Date: Fri, 3 May 2019 10:54:42 -0400 Subject: [PATCH 8/8] Update CHANGES. --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index a7d2639489cf..d1f88570a8e8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,7 +3,7 @@ Change Log ### 1.58 - 2019-06-01 ##### Additions :tada: -* Added support for new Bing Maps imagery styles RoadOnDemand and AerialWithLabelsOnDemand. The other versions of these, Road and AerialWithLabels, have been deprecated. [#7808] (https://github.com/AnalyticalGraphicsInc/cesium/pull/7808) +* Added support for new `BingMapsStyle` values `ROAD_ON_DEMAND` and `AERIAL_WITH_LABELS_ON_DEMAND`. The older versions of these, `ROAD` and `AERIAL_WITH_LABELS`, have been deprecated by Bing. [#7808] (https://github.com/AnalyticalGraphicsInc/cesium/pull/7808) ### 1.57 - 2019-05-01