added support for Bing Maps' AerialWithLabelsOnDemand imagery set, as… #41
Conversation
… requested in terriajs#3113
| // 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.') { |
There was a problem hiding this comment.
This test isn't great, because there are other reasons that image decoding could fail. I think the best solution is to change Resource.prototype.fetchImage to attach the generatedBlob to the error object in the otherwise block. Then you can check for it on the error and check if its size is zero.
| var promise = ImageryProvider.loadImage(this, buildImageResource(this, x, y, level, request)); | ||
|
|
||
| if(defined(promise)) { | ||
| return promise.then(() => promise, (error) => { |
There was a problem hiding this comment.
Use promise.otherwise instead of two-parameter then.
| // 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 {}; | ||
| } |
There was a problem hiding this comment.
If you're not handling the error, you need to re-throw it. Otherwise it acts like a successful result yielding undefined.
| // 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 {}; |
There was a problem hiding this comment.
Declare a module-scope missingImage field, intiailized to {}, and return that here.
There was a problem hiding this comment.
Better yet, declare the field on DiscardEmptyTileImagePolicy. Then in the discard policy you can just check for a missingImage rather than looking at the blob at all.
| * @type {String} | ||
| * @constant | ||
| */ | ||
| AERIAL_WITH_LABELS_ON_DEMAND : 'AerialWithLabelsOnDemand', |
There was a problem hiding this comment.
Let's add the other new styles, too: https://docs.microsoft.com/en-us/bingmaps/rest-services/imagery/get-imagery-metadata
It doesn't make sense to add the BirdsEye or Streetside ones, but the others should be added.
There was a problem hiding this comment.
There are a couple still missing (unless there's some reason I'm not aware of that they can't be used):
CanvasDark, CanvasLight, CanvasGray, and OrdnanceSurvey
Incorporates PR suggestions from #41
| 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)) { |
There was a problem hiding this comment.
This is cool but probably unnecessary complexity. Just returning DiscardEmptyTilePolicy.DEFAULT_EMPTY_IMAGE is fine.
There was a problem hiding this comment.
And at that point just rename it to DiscardEmptyTilePolicy.EMPTY_IMAGE.
There was a problem hiding this comment.
What if someone wanted to reuse the DiscardEmptyTilePolicy for another imagery provider though, where the behaviour for an empty tile was different? It seems silly to make them either implement a brand new discard policy or update the code for the existing one, and then you'd have to update this existing code to respect the setting.
…le discard policy for BingMapsStyle.ROAD_ON_DEMAND
|
Thanks @KeyboardSounds! I made one small tweak to swap a regular |
… requested in TerriaJS/terriajs#3113