Skip to content

Commit

Permalink
Use DiscardEmptyTileImagePolicy for all Bing styles.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
shunter committed May 3, 2019
1 parent f84594a commit c3e60fa
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 51 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 8 additions & 6 deletions Source/Core/Resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ define([
'./defineProperties',
'./DeveloperError',
'./freezeObject',
'./FeatureDetection',
'./getAbsoluteUri',
'./getBaseUri',
'./getExtensionFromUri',
Expand Down Expand Up @@ -38,7 +37,6 @@ define([
defineProperties,
DeveloperError,
freezeObject,
FeatureDetection,
getAbsoluteUri,
getBaseUri,
getExtensionFromUri,
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down
51 changes: 16 additions & 35 deletions Source/Scene/BingMapsImageryProvider.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
define([
'../Core/BingMapsApi',
'../Core/buildModuleUrl',
'../Core/Cartesian2',
'../Core/Check',
'../Core/Credit',
'../Core/defaultValue',
Expand All @@ -17,13 +16,11 @@ define([
'../Core/WebMercatorTilingScheme',
'../ThirdParty/when',
'./BingMapsStyle',
'./DiscardMissingTileImagePolicy',
'./DiscardEmptyTileImagePolicy',
'./ImageryProvider'
], function(
BingMapsApi,
buildModuleUrl,
Cartesian2,
Check,
Credit,
defaultValue,
Expand All @@ -39,7 +36,6 @@ define([
WebMercatorTilingScheme,
when,
BingMapsStyle,
DiscardMissingTileImagePolicy,
DiscardEmptyTilePolicy,
ImageryProvider) {
'use strict';
Expand All @@ -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
Expand Down Expand Up @@ -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>');

Expand Down Expand Up @@ -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 = [];
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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'
}
});
}
Expand Down
6 changes: 5 additions & 1 deletion Source/Scene/DiscardEmptyTileImagePolicy.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ define([

/**
* 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
Expand Down Expand Up @@ -38,7 +40,9 @@ define([
/**
* Default value for representing an empty image.
*/
DiscardEmptyTileImagePolicy.EMPTY_IMAGE = {};
DiscardEmptyTileImagePolicy.EMPTY_IMAGE = new Image();
// load a blank data URI with a 1x1 transparent pixel.
DiscardEmptyTileImagePolicy.EMPTY_IMAGE.src = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=';

return DiscardEmptyTileImagePolicy;
});
14 changes: 5 additions & 9 deletions Specs/Scene/BingMapsImageryProviderSpec.js
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit c3e60fa

Please sign in to comment.