Ticket #2759 (closed bug: fixed)

Opened 3 years ago

Last modified 3 years ago

allOverlays trouble with Google v3 layer

Reported by: tschaub Owned by:
Priority: minor Milestone: 2.10 Release
Component: Layer.SphericalMercator Version: 2.9
Keywords: Cc:
State: Complete

Description

If you create a map with allOverlays true and add a Google v3 layer as the first layer to the map, when you set the visibility of that layer to false, you cannot pan the map.

This simple example demonstrates the issue:

var map;

function init() {
    map = new OpenLayers.Map('map', {allOverlays: true});
    map.addControl(new OpenLayers.Control.LayerSwitcher());
    
    var gmap = new OpenLayers.Layer.Google(
        "Google Streets", // the default
        {numZoomLevels: 20, visibility: false}
    );
    var osm = new OpenLayers.Layer.OSM();

    map.addLayers([gmap, osm]);

    map.setCenter(new OpenLayers.LonLat(10.2, 48.9).transform(
        new OpenLayers.Projection("EPSG:4326"),
        map.getProjectionObject()
    ), 5);
}

Attachments

2759.patch Download (7.4 KB) - added by tschaub 3 years ago.
simplify spherical mercator layers
openlayers-2759.patch Download (8.4 KB) - added by ahocevar 3 years ago.

Change History

Changed 3 years ago by tschaub

This problem occurs because when the Google layer is the map.baseLayer the cooresponding gmap object doesn't correctly track the OpenLayers map center when the layer is not visible. Because all the pixel<->lonlat translations still go through the gmap object, things fall apart when panning.

It has bugged me for a while that the spherical mercator layers still ask the underlying map object to help with pixel<->lonlat translations. This was necessary when the underlying map object projection was not understood. It is not necessary now (with either spherical mercator or magic geographic mode).

A simple fix for the panning while base layer not visible problem is to override getLonLatFromViewPortPx and getViewPortPxFromLonLat on the SphericalMercator mixin. These should call the same methods on the OpenLayers.Layer prototype instead of going through the unnecessary calls to the underlying map object.

A consequence of doing this (avoiding calls to underlying map object) is that SphericalMercator-ized layers are treated a other layers - as if wrapDateLine is false by default. To maintain the current behavior, wrapDateLine would explicitly be true on SphericalMercator-ready layer prototypes.

I'm in favor of this change. I'll put together a patch with tests.

The alternative is to call map.setBaseLayer when layer visibility changes - but this goes against what allOverlays is trying to decouple.

Changed 3 years ago by tschaub

simplify spherical mercator layers

Changed 3 years ago by tschaub

  • state set to Review
  • component changed from Layer.Google.v3 to Layer.SphericalMercator

Summary of the patch:

  • SphericalMercator.js - This mixin already overrides the getExtent method of mercator layers (which it should do), bypassing the underlying map object. This change also overrides the getLonLatFromViewPortPx and getViewPortPxFromLonLat methods so they too act like normal layers (as they should). Since we know about the projection of the tiles, we can do the pixel to map location translations ourselves.
  • Google.js, Yahoo.js, and VirtualEarth.js - Because mercator layers now behave more like normal layers, they default to respecting the date line. Because this is different behavior than the previous release, wrapDateLine is set to true on the prototypes of layers that work with SphericalMercator. The VirtualEarth layer doesn't wrap with or without this change - and it is currently screwed up when you try to pan across the dateline. Not sure how long (perhaps forever) this has been a problem. The MultiMap layer example doesn't work for me.
  • v3.html tests - The first new test (test_allOverlays_pan) demonstrates the problem in the ticket description. It fails without the SphericalMercator changes and passes with these changes. The two additional tests confirm that wrapDateLine behaves as expected when set to true or false.

All tests (with the exception of an unrelated Request.html failure) pass with these changes. Examples perform as expected. Thanks for any review.

Changed 3 years ago by ahocevar

  • state changed from Review to Commit

I can confirm that tests still pass and examples work as expected. I was also able to test the MultiMap example (created a new API key and updated the multimap-mercator example to use it), and can confirm that it also works.

I am also uploading a patch which removes the getLonLatFromViewPortPx and getViewPortPxFromLonLat methods from Layer.EventPane instead of overriding them in Layer.SphericalMercator. The reason why I am doing this is that we have no place left where these methods from Layer.EventPane would get used.

Whatever patch you prefer, thanks for the hard work on this. Please commit.

Changed 3 years ago by ahocevar

Changed 3 years ago by tschaub

I'm pretty sure that Google, Yahoo, VirtualEarth, and MultiMap still use those methods on EventPane by default.

Changed 3 years ago by tschaub

Yeah, Google pans out of control with the second patch (remember, not everybody is on board with spherical mercator yet).

Changed 3 years ago by ahocevar

Oh right, I forgot about the poor non spherical souls out there. Well, then your patch it shall be :-). Thanks again!

Changed 3 years ago by tschaub

  • status changed from new to closed
  • state changed from Commit to Complete
  • resolution set to fixed

(In [10554]) Making it so layers that use the SphericalMercator mixin call getLonLatFromViewPortPx and getViewPortPxFromLonLat on the Layer prototype instead of relying on the underlying map object for pixel to map location translations. This allows the Google (and other SM) layers to be used as a base layer but not be visible (with allOverlays set true on the map). r=ahocevar (closes #2759)

Note: See TracTickets for help on using tickets.