ArcGIS (AGS) Tile Cache Layer Contribution

[dmiddlecamp]

Hi Everybody!

This is a contribution of a new Layer for OpenLayers that is compatible with newer ESRI ArcGIS Server setups that use cached tile sets. (e.g. arcgis.com, arcgis online). This layer fixes the problem of the tiles 'jumping around' during zooming ( as seen in previous versions  http://trac.osgeo.org/openlayers/browser/sandbox/tschaub/arcgiscache/lib/OpenLayers/Layer/ArcGISCache.js) , by correctly calculating the extent based on tile information. This layer can also configure itself using server provided ESRI specific json. This layer is an improvement on wrapping the ESRI js api, since all the work is done in native OpenLayers code, and it provides the added benefit of accessing tiles directly.

This layer should represent a working fix to at least one existing ticket  http://trac.osgeo.org/openlayers/ticket/1967

Our company Azavea has an existing code contribution agreement, so I'm submitting under that. I've also provided some unit tests, and three typical examples of using and configuring the layer. I've tried to review and clean this code as much as possible, but please let me know if anything needs to be changed or tweaked. I'm very excited about contributing this layer back, and I hope it can help users who would like to use AGS tile sets. At the very least I hope it helps people to understand more recent ArcGIS tile sets.

Thanks! David M.

[dmiddlecamp]

ArcGIS Cache layer (AGS)

[bartvde]

Looking into this now for review.

[bartvde]

@dmiddlecamp is there any specific reason why you are subclassing OpenLayers.Layer.TMS and not OpenLayers.Layer.XYZ?

[bartvde]

This is a great patch with great examples, however this needs more work before it can go into trunk, I'll provide more info later, but the override/copy of initGriddedTiles is not a good thing. We need to find a more clean solution for that.

[dmiddlecamp]

This was a situation where the layer is compensating for slightly atypical server results. This override has been working very well across the configurations we've tested, but this layer will require a hook at the start of initGriddedTiles in some fashion, so we can adjust the extent appropriately. I thought this was considerably safer than making changes to a fundamental base class. Would an alternative version with some kind of pre-initGriddedTiles hook be a better solution?

[dmiddlecamp]

revised patch, with preInitGriddedTiles

[dmiddlecamp]

If I didn't say so earlier, thanks for the review, much appreciated! I'm pretty excited about the potential for others to get to use this layer. I made another patch with your suggestion to not override initGriddedTiles (not my favorite either), and replaced it with something I hope is closer to what you had in mind. I completely removed the override of initGriddedTiles, and replaced it with a small hook in the base class / override in the child. Please let me know if there is a preferred way to set something like this up in OpenLayers.

The TMS url format was closer to what the arcgis server expected, so it made sense to override that instead. Please see the revised attached patch.

Thanks again, David

[bartvde]

David, I have opened up ticket:3136 for the change needed in Layer.Grid, and I will provide a patch there. Hopefully we'll get a review on that change, so we can move this new layer forward.

[bartvde]

Hi David, I've added a new patch (work in progress though) that makes the following changes:

• use getMaxExtent as proposed in ticket:3136
• change internal methods from APIMethod to Method
• use zeroPad from tschaub's sandbox instead
• inherit from OpenLayers.Layer.XYZ instead

I still have a few questions for you:

• Why do you add 0 to the lod.scale in initialize?
• What is the main difference between your getURL function and the one from tschaub in his sandbox? Any chance you can adapt his work instead of rewriting? Or at least point out why you did it differently.

TIA for your patience.

[dmiddlecamp]

Thanks again for your efforts moving this functionality forward. I really appreciate your feedback, and I wanted to try and address your changes / questions:

• getMaxExtent override:
  • Cool, probably a change I wouldn't have considered since it involves changing core functionality beyond this layer, but it seems like a clever solution to the larger issue of changing extents.
• use zeroPad from tschaub's sandbox instead
  • I had missed this function, which does largely the same as that small helper 'padLeft' that I included. The major difference is that 'padLeft' lets you specify the pad char, and doesn't require the initial value to be numeric. Tschaub's is more specific to this situation, and that probably makes calls to it a bit cleaner.
• inherit from OpenLayers.Layer.XYZ instead
  • XYZ and TMS seem fairly similar to me with the exception of the maxExtent / tileOrigin difference. I had the impression that the ArcGis cache layers made use of the tileOrigin as well as the maximum extent, so I attempted to pick the layer closest to the server behavior. XYZ also seems to include some auto-configuration behavior for sphericalMercator that doesn't seem to match the server-specific initialization concept of the ArcGISCache layer. TMS seemed the most appropriate, and it's been working well so far, but if someone with more experience recommends XYZ, and it maintains the desired behavior, then I'm all for it.
• Why do you add 0 to the lod.scale in initialize?
  • That is probably leftover debug code, amazing those 3 characters stayed in there for so long. :)
• What is the main difference between your getURL function and the one from tschaub in his sandbox? Any chance you can adapt his work instead of rewriting? Or at least point out why you did it differently.
  • We had a version of this layer internally that I think may have been based on tschaub's initial code as well as that of my peers here. I was originally trying to debug that version, and fix the issues with it across the different tile and server configurations we needed to support. It made sense to develop against that layer instead of another user's sandbox, and through the course of debugging, I was able to add code to help bridge the differences between the Esri server capabilities object and OpenLayers. In the event that some small math transcription was the source of the problem, a fresh code slate seemed safer. Also, I suspected it might not be well received to start suggesting patches to someone else's sandbox... :)

[bartvde]

updated patch which needs no changes to the Grid layer

[bartvde]

(In [11693]) adding ArcGIS (AGS) Tile Cache Layer, great contribution from Azavea, thanks, p=dmiddlecamp, r=me (closes #3030)

[jorix]

Replying to bartvde:

(In [11693]) adding ArcGIS (AGS) Tile Cache Layer, great contribution from Azavea, thanks, p=dmiddlecamp, r=me (closes #3030)

ATTENTION:

..\lib\OpenLayers\Layer\ArcGISCache.js:274: ERROR - variable 
calculateMaxExtentWithLOD is undefined
        return calculateMaxExtentWithLOD(lod);
               ^

I think that misses the "this"

[bartvde]

(In [11702]) fix up undefined error in ArcGISCache layer, thanks jorix, non-functional change (closes #3030)