UTFGrid Layer, Tile and Controls #244

wants to merge 8 commits into


None yet

5 participants


See the related email to dev list at:

More info here:

I'd love to see support for UTFGrids included in the next release. Let me know if there's anything I can do to clean things up to make this happen...

tschaub commented Feb 26, 2012

I've added a few commits that I'm hoping you'll consider. These are included in a pull request. I wanted to make some changes that were based on recent work in master. The bulk of the commits in that pull request will go away if you merge master with your branch.

The first change (34a9c46) just adds the new UTFGrid component to the debug loader so examples can work without a build. The next change (6284e44) updates the examples so they fit in a bit better with the rest (and run without the missing ./OpenLayers.js).

My understanding of the UTFGrid spec is that the structure of property values in the data member may be of any type. The existing implementation assumes that the property values will be objects with keys representing feature attribute names and values representing these attribute values. The UTFGrid spec examples show a simple data structure with strings that wouldn't work with this implementation. I've updated things (e1dffac) so that the user provided callback receives a data lookup - keys corresponding to layer index and values straight from the data member.

This change (e1dffac) also gives the layer a getData method. I think this is a bit more flexible/sensible than having the control extract data from the layer's tiles.

I'd be interested to here if others have comments on this. That's it for now. More review later.

tschaub commented Feb 26, 2012

I'm adding commits in my fork while reviewing.

  • 73e6973 - Removed image stuff from UTFGrid tile.
  • e6f0aa0 - Tweaked use of Script protocol. Open question: doesn't the GeoJSON format throw when parsing the UTFGrid data? Good candidate for a test.
  • c18f6a2 - I changed the JSON parsing to use the JSON format. This is essentially for IE7 support. I'm open for pushback here. I know there's a push to minimize our build size, and folks are scoffing at legacy IE support. But I've sat in some fairly big agencies (USGS, NOAA) and seen first hand the value in having OpenLayers be a bit more concerned about cross-browser compatibility than others.
  • 12fa9b5 - Since loadstart is fired, it makes sense to fire loadend as well.
  • faeb691 - Initial tests for UTFGrid tiles.
  • 551c582 - Updated layer and tile API so tile is responsible for getting feature id/data given pixel offsets and layer does the same for map locations.
Éric Lemoine and others added some commits Feb 26, 2012

This is all looking great from my end. I like the new API a lot and seems like the most sensible way to do it.

What else is left?

tschaub commented Feb 28, 2012

We should put together tests for the layer and control as well. I've grabbed the UTFGrid demo.json to validate the implementation and am finding that I can't get tests to pass (see this issue).

tschaub commented Feb 28, 2012

In 4d31a3e, I've changed the layer's getFeatureData method to getFeatureInfo. The old getFeatureData method was already retrieving feature ids, and it doesn't make sense to force the user to do two requests for the complete bundle of information.

In experimenting with the UTFGrid highlighting, I've noticed a few other issues with the layer. Will be good to write up tests for these.

tschaub commented Feb 29, 2012

I think it would be great to get this into 2.12. There are a few more changes I'd like to make. Matt, I'm interested to know how these sound to you.

Single arg layer constructor

I think single arg constructors are the way forward for the library. Details upon request. For this layer, here's the difference.

Instead of this

var layer = new OpenLayers.Layer.UTFGrid(
    {utfgridResolution: 4, displayInLayerSwitcher: false}

A user would write this

var layer = new OpenLayers.Layer.UTFGrid({
    url: "utfgrids/${z}/${x}/${y}.json",
    utfgridResolution: 4, 
    displayInLayerSwitcher: false

The difference gets more pronounced when you start assigning more layer properties in the constructor. The three arg constructor is particularly awkward when you have a bunch of options (highlightStyle, useJSONP, zoomOffset, etc.)


The layer and control still need tests.

I'm also interested to work out what is going on with character codes between 55296 and 57343 (mapbox/utfgrid-spec#1). I'm fairly confident there is nothing wrong with this implementation. It's either an encoding issue with the example/tests or a real consideration that needs to be accounted for in the spec. So, while I don't think it should block inclusion of these components, I'd like to figure out why the demo.js tests won't pass.


I'd like to see the JSONP functionality in action. Are there reliable servers that we could use in an example? I think it also makes sense to have examples with pan/zoom navigation. I understand it's tough to load up a bunch of static UTFGrids. But at least being able to pan a bit or zoom once would be nice.


Maybe persnickety, but I'm inclined to change utfgridResolution to gridResolution. While a bit ambiguous, I find the latter easier to type (the spec refers to this as factor - even tidier, but less descriptive).

I'd like to clean up the examples a bit more. Minor stuff: separate js file, some name/structure changes.


Single Arg constructor



I'm a bit unfamiliar with javascript unit testing and I probably won't have time to really delve into them until later next week unfortunately.


The problem is that OpenLayers script protocol uses a global callback registry so the returned JSONP file must dynamically wrap the json in that function. At the moment, the jsonp support is not very practical since the two main utfgrid servers don't support dynamic jsonp callbacks.

Tilestream serves up jsonp files with a hardcoded grid(..) callback and, for reasons of caching and CDN storage, probably will continue to do so.

I've added dynamic jsonp callbacks to my Tilestache fork at https://github.com/perrygeo/tilestache but that hasn't been pulled into the upstream master yet nor are there really any reliable servers out there running it.

Pan/Zoom in examples

I will update the examples and generate another zoom level of tiles if it's OK to add them to the repository.


-1 on gridResolution .. its a bit too ambiguous since we refer to the tile layout as the "grid" ... I would be in favor of utfgridFactor or gridFactor to match the specs more closely.

tschaub commented Feb 29, 2012

Thanks for the feedback. All sounds good. I'm happy to stick with your utfgridResolution - it is the most descriptive. If you can pile in a few more utfgrids, I think that would be great. You'll likely notice some dateline wrapping issues - not specific to this code I think, but something I want to check out. Might be easiest to have a multiple zoom level example with a restricted extent & set of resolutions.

I'll try to find a bit more time for the tests.

tschaub commented Mar 3, 2012

Making additional changes discussed above.

  • 2feb860 - example restructuring
  • 36d22bb - single arg constructor
  • 268b842 - test for layer and control (passing in IE6, Chrome 17, etc.)
  • 32db586 - providing more detail to the callback
  • c6aa996 - example with multiple zoom levels
@tschaub tschaub referenced this pull request Mar 4, 2012

UTFGrid support #274

tschaub commented Mar 4, 2012

As mentioned on the other pull request, I only opened that in case Matt doesn't get a chance to pull in my additional commits. I'd like to get some additional eyes on this, but I think it's in good shape. I still want to have an example that allows for some navigation. I've pulled down three zoom levels of grids from MapBox's geography-class layer. I'll try to put together an updated example with that before too long.

tschaub commented Mar 9, 2012

Complete batch of commits merged w/ 1a44458. Thanks @perrygeo for the collaboration on this.

@tschaub tschaub closed this Mar 9, 2012

I just got back from vacation and was psyched to see it merged already. Thanks for all the hard work on this - it looks great! Anything else to do to tie up loose ends before the release?


Using imageSize for the frame size does not make sense to me. The tileSize should be used for the frame size. imageSize should be for the image inside the frame.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment