API review: setting map center #457

Closed
tschaub opened this Issue Apr 3, 2013 · 42 comments

Comments

Projects
None yet
9 participants
Owner

tschaub commented Apr 3, 2013

Compare the task of setting the map center in a few different libs:

OpenLayers 3:

map.getView().getView2D().setCenter(ol.projection.transform(new ol.Coordinate(-111, 46), 'EPSG:4326', 'EPSG:3857'));

OpenLayers 2:

map.setCenter(new OpenLayers.LonLat(-111, 46).transform('EPSG:4326', 'EPSG:3857'));

Google Maps v3:

map.setCenter(new google.maps.LatLng(46, -111));

Leaflet:

map.panTo(new L.LatLng(46, -111));
// or
map.panTo(L.latLng([46, -111]));
// or
map.panTo(L.latLng(46, -111));
// or
map.panTo([46, -111]);

Leaflet gets the award for most convenient (and most different ways to do the same thing). Google Maps gets the award for least likely to be guessed wrong. OpenLayers 2 gets the award for most misleading (due to LonLat not implying geographic coords). OpenLayers 3 gets the award for most conceptual overhead for a trivial task.

While an argument can be made that the individual abstractions in ol3 are simple (map, view, view2d, coordinate, projection), we force users to build a bigger mental model than is necessary (in my opinion) to do trivial things.

If the map were a 2d scene (forcing a 2d view) and we had the concept of a "user projection" on a scene, I believe we could simplify things to this:

map.setCenter(new ol.Coordinate(-111, 46));

This would leave open the possibility of constructing a scene with a 3d view, using arbitrary projections, etc. - and I don't believe it excessively mixes concerns. It simply means a map is a convenient way to construct a scene with a 2d view (very common use case).

I'm open to hearing other ideas from folks that also think we can improve on the current API.

Owner

elemoine commented Apr 3, 2013

Very very quick note: I think you can remove the getView2D call in the ol3 case above.

Owner

tschaub commented Apr 3, 2013

I think you can remove the getView2D call in the ol3 case above.

That depends on how the map was constructed, right? (Unless we agree that 3d views will also have setCenter.)

@tschaub tschaub added a commit to tschaub/openlayers that referenced this issue Apr 3, 2013

@tschaub tschaub Avoid exposing extra abstractions (see #457) eeda86f
Owner

ahocevar commented Apr 3, 2013

I agree with @tschaub and still think that having a 2d map with a user projection and a setCenter method is a great idea.

Owner

elemoine commented Apr 3, 2013

I think you can remove the getView2D call in the ol3 case above.

That depends on how the map was constructed, right? (Unless we agree that 3d views will also have setCenter.)

I was referring to the simple 2D case, where the map is configured with a View2D.

Owner

elemoine commented Apr 3, 2013

As I already said on ol3-dev, I don't like the idea that a “map” become a 2D thing. I anticipate that in the future we will want to create maps with 3d views/cameras, to view the map with more degrees of freedom.

I wouldn't find the following too ugly personally:

map.getView().centerToLatLon(lat, lon);

but I may be the only one :)

Contributor

twpayne commented Apr 3, 2013

I believe that the current API can be improved, but, when comparing APIs, please consider:

  • Google Maps only supports a EPSG:4326 interface to a EPSG:3857 map.
  • Leaflet only supports an EPSG:4326 interface to an EPSG:3857 map. It claims to support other projections, but in practice it's a mess.
  • OpenLayers 2 supports different map projections.
  • OpenLayers 3 not only has to support different map projections, but also flat 2D, tilted 2D, Cartesian space 3D and 3D globe modes.

"Center" has no well defined meaning for any view other than flat 2D. Please re-read the page on cameras in the Wiki if needed.

When your functionality is heavily limited (as is the case for Leaflet and Google Maps) the API is correspondingly simple (i.e. trivial). When you have rich and varied functionality then it is harder to make a trivial API.

This is not to say that the current API cannot be improved, but it would be a monumental error to bake the assumption of a 2D map into ol3's core objects.

Owner

tschaub commented Apr 3, 2013

I was referring to the simple 2D case, where the map is configured with a View2D.

@elemoine and @twpayne can hash this out - see tschaub/openlayers@eeda86f#commitcomment-2939515

Owner

tschaub commented Apr 3, 2013

This is not to say that the current API cannot be improved, but it would be a monumental error to bake the assumption of a 2D map into ol3's core objects

I'm essentially suggesting a name change - make the current ol.Map into ol.Scene (which will allow arbitrary views). Then we'd create a new ol.Map that created a 2d view. I think everyone would find it very intuitive that a ol.Map is 2d. It fits the meaning of the word very well.

Nowhere in the library do we create an ol.Map instance - and I don't imagine we ever will. So I'm not suggesting baking 2d deeply into the library. Instead I'm suggesting providing a convenience layer (ol.Map) for users that only care about 2d. Again, ol.Scene could provide all the richness others need.

Contributor

gjn commented Apr 4, 2013

Allow me to bring a user perspective: I think a use might expect to have similar functionality for similar named objects, even if a major jump in an API is to happen. But I also understand that ol3 will rock the world with its 3D support and this should be directly visible by the API that ol3 will export. In that sense, having 3D in names is important and should be pushed.

I agree with the initial statement that to set a center on a map, ol3 currently is too complicated. It's not intuitive at all (especially users coming from existing libraries). Therefore, I would recommend a convenience layer too. I would probably name those ol.Map2D and ol.Map3D, both of which would be a thin wrapper around ol.Map with smart defaults.

While 3D will probably be the killer feature of ol3, the mapping world won't change to 3D once ol3 hits the shelves. For many reasons, it will be a slow adaption process. And 2D will stay relevant during the live time of ol3.

Owner

bartvde commented Apr 4, 2013

I'm in favour of making the 2D case as simple as possible for the end user.
I even think that Leaflet's function name, panTo is better than setCenter, but that's not a big deal to me.

Contributor

twpayne commented Apr 4, 2013

I'm in favour of making the 2D case as simple as possible for the end user.

I hope we all share that goal. I also hope that people agree with the statement "things should be as simple as possible, but no simpler".

I even think that Leaflet's function name, panTo is better than setCenter, but that's not a big deal to me.

To a native English speaker, they mean different things. panTo implies movement (panning) whereas setCenter does not.

Contributor

vmx commented Apr 4, 2013

I prefer doing the simplest way for the most common case. So map.setCenter([-111, 46]); could be a shortcut for map.setCenter(new ol.Coordinate(-111, 46));. If you want to do additional things, like using a certain different projection, you would of course need to use use longer way.

Contributor

twpayne commented Apr 4, 2013

I hope it is obvious to all that we want the simple case to be as simple as feasibly possible.

In your proposals for the API, please also include an example of how it should work for 3D, and how it should work with the type checking imposed by the Closure Compiler.

Owner

elemoine commented Apr 4, 2013

Leaflet has map.setView for changing the map center without "moving/panning". I think that's the API method we should compare ol3's and ol2's setCenter to.

twpayne referenced this issue Apr 4, 2013

Closed

Add ol.Map2D #463

Contributor

twpayne commented Apr 4, 2013

The map2d branch is now hosted here in the openlayers/ol3 repository for people to experiment with:

master...map2d

Contributor

probins commented Apr 11, 2013

I'm essentially suggesting a name change - make the current ol.Map into ol.Scene (which will allow arbitrary views). Then we'd create a new ol.Map that created a 2d view. I think everyone would find it very intuitive that a ol.Map is 2d. It fits the meaning of the word very well.

I agree that maps are 2d. To quote wikipedia: "Cartography or map-making is the study and practice of crafting representations of the Earth upon a flat surface". I would call a 3d representation a globe, not a map. For many, probably most, users, a map is a piece of paper, and what they see on the screen is the same as what they would get if they bought such a piece of paper in the shops.

As for setting the center on 2d maps, I would imagine the most usual case is for coordinates to be given as latlons, whereas the map (or view or scene or whatever) is projected (how many people know any coordinates in web mercator?). A latlon is simply 2 numbers which I would like to be able to define as

setCenter({lat: 1.1, lon: 2.2})

I don't see why I should have to write anything longer than that.

Contributor

probins commented Oct 26, 2013

I wouldn't find the following too ugly personally:

map.getView().centerToLatLon(lat, lon);

I like this too. Clear and simple.
And how about centerToXy(x, y) as well, so it's clear in which order the coords should be specified.

Owner

elemoine commented Oct 26, 2013

centerToLonLat would transform the lon lat coords to coords in the view projection.

Contributor

probins commented Oct 26, 2013

yes, requiring developers to transform all the time is unfriendly - should be encapsulated in the library. Perhaps centerAtLatLon() would be a better name.

Owner

elemoine commented Oct 27, 2013

If we introduced something like centerAtLatLon would users expect getCenterLatLon or something? And they may expect the same sort of functions for other types, ol.Overlay positionLatLon for example.

Contributor

probins commented Oct 27, 2013

If we introduced something like centerAtLatLon would users expect getCenterLatLon or something? And they may expect the same sort of functions for other types, ol.Overlay positionLatLon for example.

yes, they probably would :-) And rightly so. So we come back to @tschaub's original suggestion of a 'user projection'. I think this is linked to my question on the list: what is view.projection actually for? In fact, what is view for? In MVC terms, is it a view or a controller? An MVC view is supposed to be from the user perspective, but most end users know nothing about projections, they just know latlons. They also know nothing about resolutions, though they understand that by clicking/tapping they can zoom in/out and so get the equivalent of maps at different scales. So, I think if I were designing this in an MVC way, the view would by default be 4326 with zooms, and this view would include all the visible elements that use coordinates (ol2 sort of had this with displayProjection). There would then be a separate controller to convert that into the projections/resolutions that the servers understand, and control the layers/sources.

Contributor

probins commented Mar 13, 2014

I see Google have just added the ability to set center with "a more standard pattern of using plain JavaScript objects"
http://googlegeodevelopers.blogspot.co.uk/2014/03/streamlining-code-in-javascript-maps-api.html

Contributor

twpayne commented Mar 13, 2014

@probins, your point being?

Contributor

probins commented Mar 14, 2014

that Google's api is now the simplest of the lot

Contributor

twpayne commented Mar 14, 2014

Can you tell me how to use maps with a different projection to EPSG:3857 in the Google Maps API?

Owner

ahocevar commented Mar 14, 2014

However, we could now easily add the same convenience: ahocevar/openlayers@openlayers:master...ahocevar:latLng

Owner

bartvde commented Mar 14, 2014

nice, I like it

Contributor

probins commented Mar 14, 2014

ditto

Contributor

twpayne commented Mar 14, 2014

Interesting, but it has strange behaviour:

var center = {lat: 45, lng: 5};
view.setCenter(center);
window.console.log(view.getCenter().lat); // => undefined! but I just set it!
Owner

elemoine commented Mar 14, 2014

view.setCenterLatLng(45, 5) and view.setCenterLatLng([45, 5]) are other options. The latter may make more sense if we want to also add getCenterLatLng.

Owner

ahocevar commented Mar 14, 2014

I like the latter. And a centerLatLng option for the constructor. Great
suggestion @elemoine.
On 14 Mar 2014 11:18, "Éric Lemoine" notifications@github.com wrote:

view.setCenterLatLng(45, 5) and view.setCenterLatLng([45, 5]) are other
options. The latter may make more sense if we want to also add
getCenterLatLng.

Reply to this email directly or view it on GitHubhttps://github.com/openlayers/ol3/issues/457#issuecomment-37633329
.

Owner

elemoine commented Mar 14, 2014

This more or less what I suggested a year ago :-) openlayers#457 (comment)

Owner

elemoine commented Mar 14, 2014

view.setCenterLatLng(45, 5) and view.setCenterLatLng([45, 5]) are other options. The latter may make more sense if we want to also add getCenterLatLng.

I can provide a PR if others like it.

Owner

ahocevar commented Mar 14, 2014

@elemoine I would love to see the above in a pull request.

indus commented Apr 11, 2014

I think its wrong to focus only on "setCenter". It is more an overall problem with OL3.

when I saw the following in my code, just to get the map-bounds with OL3...

var extent = map.getView().getView2D().calculateExtent(map.getSize());
var bounds = [ol.proj.EPSG3857.toEPSG4326([extent[0], extent[1]]),
              ol.proj.EPSG3857.toEPSG4326([extent[2], extent[3]])]

...I realized that I have to stick to leaflet...

var bounds = map.getBounds()

...for a while.

"It is often a challenge to know how and where to set resolutions, maxExtent, etc. for the map. Projection handling between map and layers and the interplay between projection and properties like maxExtent and center are complicated at best." (written one and half years ago about OL2)

Please bring back the most basic map-functions before you release this fancy beast!

Owner

bartvde commented Apr 11, 2014

@indus for more background see: #747

Owner

elemoine commented Apr 16, 2014

FWIW it is not that bad:

var extent = map.getView().calculateExtent(map.getSize());
var bounds = ol.proj.transform(extent, 'EPSG:3857', 'EPSG:4326');
Contributor

probins commented Jun 22, 2014

FWIW it is not that bad:

var extent = map.getView().calculateExtent(map.getSize());
var bounds = ol.proj.transform(extent, 'EPSG:3857', 'EPSG:4326');

if we're saying that ol.proj.transform is only for coords, this would be (I think):

var bounds = ol.extent.applyTransform(map.getView().calculateExtent(map.getSize()),
    ol.proj.getTransform('EPSG:3857', 'EPSG:4326'));

User-friendly? Concise? Easy to understand? Easy to remember?

These transforms in application code shouldn't be necessary. This example is the most common use case: layer sources and view in projected coordinates; input/output coordinates 4326. I should be able to specify at the start of my app in which projection I would like to provide/receive coordinates/extents, 4326 in this example. I think this is what Tim means by 'user projection' in the first comment. The transforms should all be handled automatically by the library. The library should know that if it's getting coordinates from the view, they will be in view projection, and, if that is not the same as the 'user projection', then they should be transformed before being presented to the application. Similarly with setCenter() and view center option: if user projection != view projection, transform.

Owner

elemoine commented Jun 22, 2014

See openlayers#1860 as well, where the idea of adding setCenterLatLon and getCenterLatLon was discussed. Adding these methods was correctly considered as a partial solution.

If we added a "user projection" and if you set it to 4326 I guess you'd expect view2d.getCenter to return lat/lon (or lon/lat) values. Internally, for its own purpose, the lib also calls view2d.getCenter, so it will need to transform the coordinates back to the view projection. I think this will quickly get very messy, especially when you consider such transformations would need to be done at lot of different places (view, overlay, events...) Working on a patch that implements "user projection" should prove this.

Contributor

probins commented Jun 27, 2014

I was thinking more in terms of having the user projection at the map level, as it would apply to all views. View projection, setCenter, etc would stop as they are, so the whole layer/source/tile logic would be as now, but there would be convenience functions at the map level for center/extent which would basically just call the view functions after/before transforming as necessary. Controls like mousePosition would display map projection by default. The current coordinate functions in Map (getCoordinateFromPixel etc) would use map projection, so those places where these are currently used in the lib would have to transform if necessary; this isn't however very many. This would include Overlay. event.coordinate would also use map projection; not sure at the moment how much this is used internally, and how much work this would require.

Is this worth pursuing? I can look into it further if so. It would mean some extra work in the library, but should make things much simpler for the app developer.

Owner

elemoine commented Jul 3, 2014

With ol.View2D now replaced by ol.View we can close this issue. Let's open new issues if more discussion is needed.

elemoine closed this Jul 3, 2014

akashisama referenced this issue in openlayers/ol2 Jun 18, 2014

Closed

Fix for issue #457 #795

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