Opened 6 years ago

Closed 6 years ago

#2975 closed feature (fixed)

Bing layer with direct tile access

Reported by: ahocevar Owned by: euzuro
Priority: minor Milestone: 2.11 Release
Component: Layer Version: 2.10
Keywords: Cc:
State: Complete


Bing now officially allows direct tile access. Let's use it! This ticket proposes a new layer type, Layer.Bing.

Attachments (5)

openlayers-2975.patch (5.0 KB) - added by ahocevar 6 years ago.
proof of concept
openlayers-2975.2.patch (11.9 KB) - added by ahocevar 6 years ago.
uses JSONP, and adds required attribution
openlayers-2975.3.patch (15.7 KB) - added by ahocevar 6 years ago.
complete with unit tests
2975.patch (17.8 KB) - added by tschaub 6 years ago.
bing tiles
openlayers-2975.4.patch (17.8 KB) - added by ahocevar 6 years ago.
known set of resolutions

Download all attachments as: .zip

Change History (14)

comment:1 Changed 6 years ago by ahocevar

The above patch squeezes the new layer type into the Layer.XYZ file. I think it should go in a separate file. Also, for now, it requires a proxy. Maybe Microsoft provides a JSON callback, but I couldn't find anything about it.

comment:2 Changed 6 years ago by ahocevar

Another TODO is that we need to set the attribution from the layer metadata, and fire the changelayer event so the Attribution control re-renders the attribution as soon as we get the metadata response.

Changed 6 years ago by ahocevar

proof of concept

comment:3 Changed 6 years ago by ahocevar

Note: Adding a jsonp param to the metadata request will create a json callback. This is documented here. The rest of the maps json api for getting imagery metadata is documented here

Changed 6 years ago by ahocevar

uses JSONP, and adds required attribution

comment:4 Changed 6 years ago by ahocevar

  • State set to Review

Example works and tests pass in IE6, IE7, IE8, FF3.6, Safari 5 and Opera 10. Thanks for any review.

Changed 6 years ago by ahocevar

complete with unit tests

comment:5 Changed 6 years ago by tschaub

Thanks for the work on this. It will make a great addition to the layer collection.

I came across a few things in my review. There are likely better ways to handle these, but I've made changes in the patch that I'll describe below:

The OpenLayers.Layer.Bing.processMetadata function is called when the Bing script loads. In the previous patch, this calls initLayer, which relies on layer.resolutions being defined. Resolutions are initialized in layer.setMap. So, if the script loads before the layer is added to a map, layer.resolutions may not be defined. The tests weren't catching this because the only test that constructs a layer without adding it to a map overrides OpenLayers.Layer.Bing.processMetadata. I've modified the tests so the original processMetadata method is still called (after logging the metadata).

To make this updated test pass (and more importantly to make the layer work in applications where it is constructed but not added to a map for a while), I've added a initLayerAfterAdd method. If the layer is not added to a map by the time the Bing script loads, the initLayer method is not immediately called. Instead, initLayerAfterAdd is called when the layer is added to a map. This method unregisters itself (and destroy unregisters the same).

To support calling initLayerAfterAdd, I added an "added" even to the layer. This would be useful for all layers. Instead of extending this patch so it modifies Layer, I created #2983. When this ticket is closed, the "added" method should be moved from the Bing layer to the base Layer.

Finally, this is trivial, but I wasn't just being crass when I said that xhtml is dead. The W3C officially stopped working on it in July of 2009. As an effectively deprecated spec, I think we need to justify its use where it is required (nowhere that I know of). I updated the example to use HTML (and features of 5 where available).

Changed 6 years ago by tschaub

bing tiles

comment:6 Changed 6 years ago by tschaub

  • State changed from Review to Commit

One more change that I hope you'll consider. I made it so the layer property that determines the tile type is named "type" instead of "layer". This will bring the Bing layer closer in line with the Google layer. It also will avoid confusion where a record of layer properties is kept (layer.layer is less intuitive than layer.type in my opinion).

The Bing and XYZ tests still pass with these changes, and examples still work.

Thanks for the excellent work on this. Please commit if you agree with the above changes.

PS - I think we can improve things to avoid pink tiles in certain cases, but that can be handled at the application level for now and I think this change should get in asap.

Changed 6 years ago by ahocevar

known set of resolutions

comment:7 Changed 6 years ago by ahocevar

Thanks for the review and the hard work to improve this. I agree with all your changes, except for initLayerAfterAdd. Instead, I gave the layer a fixed set of resolutions in a constant, so resolutions are always available for initLayer. This should also help to avoid pink tile issues.

comment:8 Changed 6 years ago by ahocevar

comment:9 Changed 6 years ago by ahocevar

  • Resolution set to fixed
  • State changed from Commit to Complete
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.