Allow map and layer coordinates to be provided as latitude and longitude.

[ahocevar]

As a follow-up to bce3ace, this change adds more convenience to coordinate handling. Instead of configuring a map like this:

var map = new OpenLayers.Map({
    div: "map",
    projection: "EPSG:3857",
    maxExtent: [-18924313.432222, -15538711.094146, 18924313.432222, 15538711.094146],
    restrictedExtent: [-13358338.893333, -9608371.5085962, 13358338.893333, 9608371.5085962],
    center: [-12356463.476333, 5621521.4854095]
});

uses can now use latitudes and longitudes to set extents and center:

var map = new OpenLayers.Map({
    div: "map",
    projection: "EPSG:3857",
    maxExtent: [{lat: -80, lon: -170}, {lat: 80, lon: 170}],
    restrictedExtent: [{lat: -65, lon: -120}, {lat: 65, lon: 120}],
    center: {lat: 45, lon: -111}
});

And instead of configuring a layer like this:

var layer = new OpenLayers.Layer(null, {
    projection: "EPSG:3857",
    maxExtent: [-20037508.34, 5621521.4854095, 0, 19971868.877628]
});

it is now possible to work with latitude and longitude as well:

var layer = new OpenLayers.Layer(null, {
    projection: "EPSG:3857",
    maxExtent: [{lat: 45, lon: -180}, {lat: 85, lon: 0}]
});

[probins]

I like this idea, but a couple of comments.

At the moment, Map.center is not an api property, so if the recommendation is to set it in initialize() (which does simplify things) I think this should be changed (the same applies to zoom). Also, I think examples should be changed to reflect this. It also implies that setCenter() no longer requires a LonLat either, so I think the api docs for that should be changed too.

I seem to remember in the discussions about v3 talk of having a new OpenLayers.Location class, like LonLat but with projection, and have setCenter() use that. Have you gone off that idea in favour of using simple objects?

[ahocevar]

@probins Couple of things:

• Map.center is not an api property indeed, but the initial map configuration has been taking zoom and center options since 2.10 or so. So this needs to be documented in the Map constructor.
• Documentation is still missing in the pull request. You're absolutely right that map.setCenter also works with these objects now, as well as map.zoomToExtent.
• The introduction of OpenLayers.Location is not really possible without an api change, so we'll leave that for 3.0 as planned. The change here is the furthest we can get to convenience without making that API change.

[crschmidt]

I think that we could add a Location object, and use it in Center() operations without breaking backwards compatibility? It would just mean we'd have to duplicate all the code paths in some way, right?

(I'm not saying it's right, only that it's technically feasible.)

[probins]

One additional restriction is that center and zoom options are only applied if options.layers is present - which makes sense but should be documented.

As for Location, yes, I was thinking that you could create a new Location class and set center to either a LonLat or a Location. However, I'm not sure it's a good idea. LonLat (and presumably Location) do have some logic in them (add(), equals() ...) but in this case all you need is the coordinates, and it simplifies people's code if they are able to supply a simple coords obj or array rather than having to instantiate an object

[ahocevar]

@probins For the missing documentation, can you please issue a pull request against my branch? Thanks.

[probins]

thinking a bit more on this, I think the examples in Map constructor could do with updating too:

1. change bounds to array or lat/lon as your examples above
2. remove units (AIUI, this only needs to be set if different from projection)
3. "EPSG:41001" does not exist; better would be 3857 as with your examples above

[probins]

and change docs similarly for Layer.MaxExtent

[ahocevar]

@probins These suggestions sound reasonable. It would be great if you could issue a pull request. Thanks!

[probins]

@ahocevar Yes, I can, but ... there's more. There are really 2 different parts to what you are proposing:

1. any LonLat or Bounds can be created with an array or lon/lat obj as well as the usual arguments, so documentation of these 3 possibilities belongs in those classes.
2. in addition, extent, maxExtent, restrictedExtent, zoomToExtent() plus center/setCenter() on Map, plus maxExtent and minExtent on Layer, can also be an array or lon/lat obj instead of a Bounds/LonLat. ISTM all these should be documented appropriately

Also, for consistency, ISTM Map.minExtent should also have this capability.

[ahocevar]

@probins: #1 is only partially true. You can only provide lat/lon object literals in the context of a map or layer when the target projection is known. The array of coordinates in the target projection works everywhere. You are right about #2. And you are also right about Map.minExtent. A pull request of yours would still be much appreciated!

[probins]

@ahocevar Re #1, I can do
ll=new OpenLayers.LonLat({lon: 1, lat: 52})
or
ll=new OpenLayers.LonLat({lon: 1, lat: 52}, proj)
where proj is my projection object, and it will create the appropriate LonLat transformed as needed. Similarly for Bounds.
I think this is a significant usability enhancement

[ahocevar]

@probins yes, but I didn't mean to expose this to the API, because it is confusing (without the proj argument, you won't get what you expect, and with the proj argument, your ll won't be lattitude and longitude).

[probins]

hm, well, personally, I don't find that any more confusing than new OpenLayers.LonLat(123456,123456), as long as it's documented. OpenLayers.LonLat is confusing, however you do it.
However, if you prefer not to have it documented, we can just list the array option. It's true that with the simple {lon/lat} case, you don't gain much over lon,lat, apart perhaps from the fact that you don't have to worry which order they're in.

[elemoine]

@ahocevar do you think that's 2.12 material? It's currently marked as so. (I haven't looked at the patch yet.)

[ahocevar]

@elemoine: @tschaub took a look at it yesterday. He had some concerns that we wanted to discuss, but it was late at night and I had to go offline. I think it's safer to merge this after 2.12.

[probins]

whether or not this patch goes into 2.12, I think what I wrote documenting the ability to enter coordinates as an array should go in.

[ahocevar]

@tschaub, was it intentional that you didn't add docs in bce3ace? If not, and if you still have doubts about this pull request here, I'm inclined to commit at least the doc improvements for 2.12.

[elemoine]

This patch relates to the APIs so I think it needs discussions. My first impression is that this patch does improve the API a bit, but it is not really satisfactory. I even fear that it makes the OpenLayers API even more being confusing. I don't want to sound harsh here, but that's my first feeling about this patch.

Couldn't we introduce LatLng, LatLngBounds, and Location types, deprecate LonLat and make it an alias to Location or something? LatLng and LatLngBounds would API objects, converted into Location and Bounds objects for internal usage. Reusing one your above examples we'd have:

var map = new OpenLayers.Map({
    div: "map",
    projection: "EPSG:3857",
    maxExtent: new LatLngBounds{-80, -170, 80, 170),
    restrictedExtent: new LatLngBounds{-65, -120, 65, 210),
    center: new LatLng(45, -111)
});

Just a random idea. Feel free to throw tomatoes at me :)

[ahocevar]

No worries @elemoine - you don't sound harsh at all, and it's good to have this discussion! Let's others share their opinions as well.

[ahocevar]

Leaving this open for further discussion, but removing it from the 2.12 milestone. I'm not so convinced any more that this pull request should be merged as-is.

[probins]

@elemoine What do you gain by creating these extra classes? Seems over-complicated for something that ought to be simple. Surely all I as a developer want to do is specify coords and which projection they are in. I don't need classes for this in my code, so why do I have to instantiate them? Similarly, I shouldn't have to transform every coord I give; OL should be clever enough to do it itself.
So how about:

1. leave LatLon and Bounds as they are
2. add the ability to set extents and center using an object like {x:1.2, y: 33.44, projection: string} with default projection being 4326
3. then change, for example, Map.moveTo() to: if !instanceOf LonLat && object, instanciate LonLat using x/y; if projection != map.projection, transform
4. similar for extents
5. make this the recommended way of giving coords, and at some point deprecate use of LonLat and Bounds in these cases

[tschaub]

Things I think we should do:

1. Add a Location constructor. Would have x, y, and projection properties. All could be set in the (single arg) constructor. Projection would default to "EPSG:4326". Eventually Point would extend Location (and all geometries would be projection aware).
2. Give Bounds a projection property. Can't set a default without breaking backwards compatibility. Make the constructor accept a single arg (e.g. {minx: -180, maxx: 180, miny: -90, maxy: 90}). If we want an envelope with a default projection, we could add Envelope, accept the arg described above, and default to "EPSG:4326".
3. Make setCenter work with location objects. Make zoomToExtent et al. take into account bounds CRS.
4. Accept constructor argument in place of instances wherever possible (e.g. map.setCenter({x: -110, y: 45})).

Even if we don't add Location right away, let's leave room for it. My note of caution above was because I don't think we should go too far accepting arbitrarily structured object literals without thinking about what other improvements we might want to add later (e.g. Location and projection aware Bounds).

[elemoine]

@tschaub This is more or less similar to what I've proposed with LatLng and LatLngBounds, except that your Location and Envelope types aren't EPSG:4325 only.

One thing: if the map is not 4326 you'll always need to set a projection in your Location and Envelope objects. That bothers me a bit, I find it nice to be able to specify the projection once (in the map).

[elemoine]

@probins I don't think custom types like LonLat and Bounds are that evil. I think they can even make the application code more readable and descriptive. But I do think we should provide conveniences where the app developer can work with specs (constructor args) instead of calling constructors himself.

[probins]

"if the map is not 4326 you'll always need to set a projection in your Location and Envelope objects"
not as I see it. The Location projection would be the projection of the Location, not the map. Locations not in map.projection would be transformed (by OL not the developer).

I wouldn't describe LonLat or Bounds as evil; they encapsulate logic that can be useful (bounds intersects/contains/etc). My point is that you don't need any of that logic when specifying a center or extent. I'd be happy with having convenience args as well as providing a Location/Bounds instance, so developers can provide either.

[elemoine]

I think we agree.

What I'm saying is if the map is not 4326 and if Location is 4326 by default then the app developper will need to set "projection" to the map projection in Location/Envelope objects he passes to the map ("center" option for example).

[probins]

if developer wants to submit coords in map projection, yes. Which is a change from current situation where coords have to be in map projection and converted by developer if not.

[elemoine]

The Location and Envelope types could expose a static function/property that allows changing the default projection.