Use DiscardEmptyTileImagePolicy for all Bing styles.

shunter · shunter · commit 9966a0831c12 · 2019-05-06T16:32:52.000-04:00
This removes the need to inspect pixel values to detect the placeholder "missing tile" image. Also change `DiscardEmptyTileImagePolicy.EMPTY_IMAGE` to be a real (blank) image. This way we always satisfy the documented API of `requestImage`. Fixes #1353.
diff --git a/CHANGES.md b/CHANGES.md
@@ -4,6 +4,7 @@ Change Log
 
 ##### Additions :tada:
 * 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)
+* `BingMapsImageryProvider` now uses `DiscardEmptyTileImagePolicy` by default to detect missing tiles as zero-length responses instead of inspecting pixel values. [#7810](https://github.com/AnalyticalGraphicsInc/cesium/pull/7810)
 
 ### 1.57 - 2019-05-01
 
diff --git a/Source/Core/Resource.js b/Source/Core/Resource.js
@@ -10,7 +10,6 @@ define([
         './defineProperties',
         './DeveloperError',
         './freezeObject',
-        './FeatureDetection',
         './getAbsoluteUri',
         './getBaseUri',
         './getExtensionFromUri',
@@ -38,7 +37,6 @@ define([
         defineProperties,
         DeveloperError,
         freezeObject,
-        FeatureDetection,
         getAbsoluteUri,
         getBaseUri,
         getExtensionFromUri,
@@ -929,9 +927,11 @@ define([
                 if (!defined(image)) {
                     return;
                 }
-                // This is because the blob object is needed for DiscardMissingTileImagePolicy
-                // See https://github.com/AnalyticalGraphicsInc/cesium/issues/1353
+
+                // The blob object may be needed for use by a TileDiscardPolicy,
+                // so attach it to the image.
                 image.blob = generatedBlob;
+
                 if (useImageBitmap) {
                     return image;
                 }
@@ -944,8 +944,10 @@ define([
                     window.URL.revokeObjectURL(generatedBlobResource.url);
                 }
 
-                // 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.
+                // If the blob load succeeded but the image decode failed, attach the blob
+                // to the error object for use by a TileDiscardPolicy.
+                // In particular, BingMapsImageryProvider uses this to detect the
+                // zero-length response that is returned when a tile is not available.
                 error.blob = generatedBlob;
 
                 return when.reject(error);
diff --git a/Source/Scene/BingMapsImageryProvider.js b/Source/Scene/BingMapsImageryProvider.js
@@ -1,7 +1,6 @@
 define([
         '../Core/BingMapsApi',
         '../Core/buildModuleUrl',
-        '../Core/Cartesian2',
         '../Core/Check',
         '../Core/Credit',
         '../Core/defaultValue',
@@ -17,13 +16,11 @@ define([
         '../Core/WebMercatorTilingScheme',
         '../ThirdParty/when',
         './BingMapsStyle',
-        './DiscardMissingTileImagePolicy',
         './DiscardEmptyTileImagePolicy',
         './ImageryProvider'
     ], function(
         BingMapsApi,
         buildModuleUrl,
-        Cartesian2,
         Check,
         Credit,
         defaultValue,
@@ -39,7 +36,6 @@ define([
         WebMercatorTilingScheme,
         when,
         BingMapsStyle,
-        DiscardMissingTileImagePolicy,
         DiscardEmptyTilePolicy,
         ImageryProvider) {
     'use strict';
@@ -63,16 +59,9 @@ 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.  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 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
-     *        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.  By default, a {@link DiscardEmptyTileImagePolicy}
+     *        will be used, with the expectation that the Bing Maps server will send a zero-length response for missing tiles.
+     *        To ensure that no tiles are discarded, construct and pass a {@link NeverTileDiscardPolicy} for this parameter.
      *
      * @see ArcGisMapServerImageryProvider
      * @see GoogleEarthEnterpriseMapsProvider
@@ -109,7 +98,12 @@ define([
         this._tileProtocol = options.tileProtocol;
         this._mapStyle = defaultValue(options.mapStyle, BingMapsStyle.AERIAL);
         this._culture = defaultValue(options.culture, '');
+
         this._tileDiscardPolicy = options.tileDiscardPolicy;
+        if (!defined(this._tileDiscardPolicy)) {
+            this._tileDiscardPolicy = new DiscardEmptyTilePolicy();
+        }
+
         this._proxy = options.proxy;
         this._credit = new Credit('<a href="http://www.bing.com"><img src="' + BingMapsImageryProvider.logoUrl + '" title="Bing Imagery"/></a>');
 
@@ -172,22 +166,6 @@ define([
 
             that._imageUrlTemplate = that._imageUrlTemplate.replace(/^http:/, tileProtocol);
 
-            // 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
-                    || 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;
             if (!attributionList) {
                 attributionList = that._attributionList = [];
@@ -547,11 +525,9 @@ define([
 
         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.
+                // One cause of an error here is that the image we tried to load was zero-length.
+                // This isn't actually a problem, since it indicates that there is no tile.
+                // So, in that case we return the EMPTY_IMAGE sentinel value for later discarding.
                 if (defined(error.blob) && error.blob.size === 0) {
                     return DiscardEmptyTilePolicy.EMPTY_IMAGE;
                 }
@@ -680,6 +656,11 @@ define([
                 quadkey: BingMapsImageryProvider.tileXYToQuadKey(x, y, level),
                 subdomain: subdomains[subdomainIndex],
                 culture: imageryProvider._culture
+            },
+            queryParameters: {
+                // this parameter tells the Bing servers to send a zero-length response
+                // instead of a placeholder image for missing tiles.
+                n: 'z'
             }
         });
     }
diff --git a/Source/Scene/DiscardEmptyTileImagePolicy.js b/Source/Scene/DiscardEmptyTileImagePolicy.js
@@ -1,13 +1,15 @@
 define([
-    '../Core/defined',
-    '../Core/defaultValue'
-], function(
+        '../Core/defined',
+        '../Core/defineProperties'
+    ], function(
         defined,
-        defaultValue) {
+        defineProperties) {
     'use strict';
 
     /**
      * A policy for discarding tile images that contain no data (and so aren't actually images).
+     * This policy discards {@link DiscardEmptyTileImagePolicy.EMPTY_IMAGE}, which is
+     * expected to be used in place of any empty tile images by the image loading code.
      *
      * @alias DiscardEmptyTileImagePolicy
      * @constructor
@@ -35,10 +37,26 @@ define([
         return DiscardEmptyTileImagePolicy.EMPTY_IMAGE === image;
     };
 
-    /**
-     * Default value for representing an empty image.
-     */
-    DiscardEmptyTileImagePolicy.EMPTY_IMAGE = {};
+    var emptyImage;
+
+    defineProperties(DiscardEmptyTileImagePolicy, {
+        /**
+         * Default value for representing an empty image.
+         * @type {Image}
+         * @readonly
+         * @memberof DiscardEmptyTileImagePolicy
+         */
+        EMPTY_IMAGE: {
+            get: function() {
+                if (!defined(emptyImage)) {
+                    emptyImage = new Image();
+                    // load a blank data URI with a 1x1 transparent pixel.
+                    emptyImage.src = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=';
+                }
+                return emptyImage;
+            }
+        }
+    });
 
     return DiscardEmptyTileImagePolicy;
 });
diff --git a/Source/Scene/DiscardMissingTileImagePolicy.js b/Source/Scene/DiscardMissingTileImagePolicy.js
@@ -3,15 +3,13 @@ define([
         '../Core/defined',
         '../Core/DeveloperError',
         '../Core/getImagePixels',
-        '../Core/Resource',
-        '../ThirdParty/when'
+        '../Core/Resource'
     ], function(
         defaultValue,
         defined,
         DeveloperError,
         getImagePixels,
-        Resource,
-        when) {
+        Resource) {
     'use strict';
 
     /**
diff --git a/Specs/Scene/BingMapsImageryProviderSpec.js b/Specs/Scene/BingMapsImageryProviderSpec.js
@@ -1,41 +1,37 @@
 defineSuite([
         'Scene/BingMapsImageryProvider',
         'Core/appendForwardSlash',
-        'Core/DefaultProxy',
         'Core/defined',
         'Core/queryToObject',
         'Core/RequestScheduler',
         'Core/Resource',
         'Core/WebMercatorTilingScheme',
         'Scene/BingMapsStyle',
-        'Scene/DiscardMissingTileImagePolicy',
+        'Scene/DiscardEmptyTileImagePolicy',
         'Scene/Imagery',
         'Scene/ImageryLayer',
         'Scene/ImageryProvider',
         'Scene/ImageryState',
         'Specs/pollToPromise',
         'ThirdParty/Uri',
-        'ThirdParty/when',
-        'Scene/DiscardEmptyTileImagePolicy'
+        'ThirdParty/when'
     ], function(
         BingMapsImageryProvider,
         appendForwardSlash,
-        DefaultProxy,
         defined,
         queryToObject,
         RequestScheduler,
         Resource,
         WebMercatorTilingScheme,
         BingMapsStyle,
-        DiscardMissingTileImagePolicy,
+        DiscardEmptyTileImagePolicy,
         Imagery,
         ImageryLayer,
         ImageryProvider,
         ImageryState,
         pollToPromise,
         Uri,
-        when,
-        DiscardEmptyTileImagePolicy) {
+        when) {
     'use strict';
 
     var supportsImageBitmapOptions;
@@ -366,7 +362,7 @@ defineSuite([
             expect(provider.tileHeight).toEqual(256);
             expect(provider.maximumLevel).toEqual(20);
             expect(provider.tilingScheme).toBeInstanceOf(WebMercatorTilingScheme);
-            expect(provider.tileDiscardPolicy).toBeInstanceOf(DiscardMissingTileImagePolicy);
+            expect(provider.tileDiscardPolicy).toBeInstanceOf(DiscardEmptyTileImagePolicy);
             expect(provider.rectangle).toEqual(new WebMercatorTilingScheme().rectangle);
             expect(provider.credit).toBeInstanceOf(Object);
 
