coordLimits #88

Closed
tmcw opened this Issue Jan 19, 2012 · 11 comments

Projects

None yet

2 participants

@tmcw
Collaborator
tmcw commented Jan 19, 2012

1.0.0 more or less abandons the previous approach to restricting zoom levels based on layers - only map.coordLimits can be specifically set on the map object. I think this is a pretty serious step back - going to take a look at inheriting this from layers.

@RandomEtc
Contributor

Which bit is a step back in your opinion?

It was a pretty high priority for me to clear up the difference between interaction limits (now coordLimits on the map, limiting center coordinates) and the metadata on a provider that describes the tiles that exist (now tileLimits, limiting the urls that will be generated). Basically the provider data is "facts" that you want to set when you make the provider and initialize the tiles. The map data is "settings" which you should configure on a per-project basic, perhaps inheriting from the provider or perhaps not.

They are the same format so you can set coordLimits to be the tileLimits from a provider and it should "just work". I can see the need for a convenience method (or something) that wires this up for you but a "step back" is a bit harsh :-p

Use-cases I had in mind:

  • not zooming too far in or out, obviously (I think this is what you're talking about; it should be a quick fix)
  • vector layers which don't have the same scaling restrictions as a tile layer shouldn't restrict pan and zoom of the map
  • some tile layers don't cover the whole world, you want to limit the pan to the extents of the tiles

Use-cases I don't think we can cover (interaction-wise) without serious forethought:

  • multiple layers with different valid extents and zoom ranges

This last case is why tileLimits are separate from mapLimits and not automatically wired up. The priority was to not generate bad URLs by default, but to keep the panning/zooming behavior squarely under the control of people using the library. map.setZoomRange(min,max) and provider.setZoomRange(min,max) are new and useful and require way less understanding of the internals than previous versions.

@tmcw
Collaborator
tmcw commented Jan 19, 2012

The problem I'm currently looking at is that most of the time I'm automatically configuring a provider with Wax and that provider has correct limits coming from JSON. All of the code that uses that pattern is broken by this change. This basically addressed two of the three use-cases you had pointed out.

It's also unclear how to use the new tileLimits API when you don't have immediate access to the Map object, as is the case with wax.mm.connector.

Potentially, I could write something like

var map = new MM.Map('map', new wax.mm.connector(tilejson));
map.coordLimits = wax.mm.limits(tilejson);

I'll try out something of that sort and see if it addresses the problem.

@RandomEtc
Contributor

Or if you don't want to write a new wax limits function, I think this should work too?

var provider = new wax.mm.connector(tilejson);
var map = new MM.Map('map', provider);
map.coordLimits = provider.outerLimits();

Sorry this (breaking) change wasn't communicated more clearly. Semver or no, the switch to 1.0 should really have involved better documentation of issues like this (from whoever introduced the issue, me in this case).

@tmcw
Collaborator
tmcw commented Jan 19, 2012

Any recommendations for the tileLimits API without access to the Map object? All of the examples I'm seeing have ready access to Map, but that isn't generally the case when you're calculating these externally to the map, and don't seem to lack access to .locationCoordinate() from providers?

@RandomEtc
Contributor

Not sure - is it too much to introduce a dependency on a projection instance? Not every provider needs one but if you do it's easy to make one (doesn't need a DOM like a map instance would at the moment).

@tmcw
Collaborator
tmcw commented Jan 19, 2012

More detail:

It looks like this segment of provider is calculating whether coord.row is greater than the size of the extent, rather than whether it's outside of the extent. Unclear how this works - changing it to compare against just BR.row somewhat alleviates the problem. (there's also the accidental binary |, which I'll fix in a second).

Stashing this work for the time being because I'm hitting a bit of a wall in understanding what should do what: http://bl.ocks.org/1642008 The code in question is https://github.com/mapbox/wax/blob/master/connectors/mm/waxconnector.js#L6 - both setting tileLimits and coordLimits breaks the map in unusual ways.

@RandomEtc
Contributor

Eek on accidental binary |, good catch.

Also you're right, the comparison to vSize is too specific for the default behavior and checking against TL and BR would be more correct.

I now realise that my advice before (to assign provider.tileLimits to map.coordLimits) wasn't right because of the nature of sourceCoordinate :-/ See the enforce-limits example for an overridden one that correctly deals with extents that don't wrap around.

We probably need a more intelligent sourceCoordinate that can handle both cases (wrap around the whole world, don't wrap around cropped regions). Modest Maps for Flash used to detect this by setting wrappable limits to Infinity - the problem with this is that you can't then use the limit itself to wrap the coordinate used to generate the tile url.

@RandomEtc
Contributor

BTW, the check against vSize was for non-square plate carrée projections, but it's been a while since I've followed through with an example for one of those. Anyone got tiles?

@tmcw
Collaborator
tmcw commented Feb 1, 2012

Hmm, okay - can you go into a bit more detail about the varieties of layers you're looking to support here? I don't have any non-square tilesets around, and am trying to fix up this part of the code today. Is the assumption that MapProvider can expect the same coord sizes (# tiles/zoom) across projections?

@tmcw tmcw added a commit that referenced this issue Feb 1, 2012
@tmcw tmcw First shot at making a wrapping, but correctly cutting-off sourceCoor…
…dinate function. Refs #88.
c3dfdb9
@RandomEtc
Contributor

Honestly I'm not sure - I'd have to work through the exercise of setting up a map in a different projection to be certain. But 100% certain that other projections should be possible, and definitely were possible in the first public releases of the lib.

If you don't have any non-Mercator maps, and Stamen don't have any, it can't be that big a priority :)

(I'm not able to work on mm.js at the moment so don't let me be a blocker)

@tmcw
Collaborator
tmcw commented Mar 27, 2012

Okay, coordLimits and enforcing extent was fixed in 34db798 - not seeing any more actions here, closing.

@tmcw tmcw closed this Mar 27, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment