Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correct _getExpandedVisibleBounds for Max Latitude #587

Merged
merged 3 commits into from
Oct 26, 2015
Merged

Correct _getExpandedVisibleBounds for Max Latitude #587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
95dbec9
Added bounds expansion to Infinity
ghybs Oct 24, 2015
aa1ba52
Corrected test suite for max latitude check
ghybs Oct 25, 2015
d76e9a4
Improved naming of _checkBoundsMaxLat
ghybs Oct 25, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
263 changes: 175 additions & 88 deletions spec/suites/removeOutsideVisibleBoundsSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,75 +1,44 @@
describe('Option removeOutsideVisibleBounds', function () {

/**
* Wrapper for Mocha's `it` function, to avoid using `beforeEach` and `afterEach`
* which create problems with PhantomJS when total number of tests (across all suites)
* increases. Might be due to use of promises for which PhantomJS performs badly?
* @param testDescription string
* @param testInstructions function
* @param testFinally function to be executed just before afterEach2, in the `finally` block.
* Avoid as much as possible creating and destroying objects for each test.
* Instead, try re-using them, except for the ones under test of course.
* PhantomJS does not perform garbage collection for the life of the page,
* i.e. during the entire test process (Karma runs all tests in a single page).
* http://stackoverflow.com/questions/27239708/how-to-get-around-memory-error-with-karma-phantomjs
*
* The `beforeEach` and `afterEach do not seem to cause much issue.
* => they can still be used to initialize some setup between each test.
* Using them keeps a readable spec/index.
*
* But refrain from re-creating div and map every time. Re-use those objects.
*/
function it2(testDescription, testInstructions, testFinally) {

it(testDescription, function () {

// Before each test.
if (typeof beforeEach2 === "function") {
beforeEach2();
}

try {

// Perform the actual test instructions.
testInstructions();

} catch (e) {

// Re-throw the exception so that Mocha sees the failed test.
throw e;

} finally {

// If specific final instructions are provided.
if (typeof testFinally === "function") {
testFinally();
}

// After each test.
if (typeof afterEach2 === "function") {
afterEach2();
}

}
});
}


/////////////////////////////
// SETUP FOR EACH TEST
/////////////////////////////

/**
* Instructions to be executed before each test called with `it2`.
*/
function beforeEach2() {
beforeEach(function () {

// Nothing for this test suite.

}
});

/**
* Instructions to be executed after each test called with `it2`.
*/
function afterEach2() {
afterEach(function () {

// Restore the previous setting, so that even in case of test failure, next tests are not affected.
L.Browser.mobile = previousMobileSetting;

if (group instanceof L.MarkerClusterGroup) {
group.removeLayers(group.getLayers());
map.removeLayer(group);
}

// Throw away group as it can be assigned with different configurations between tests.
// group must be thrown away since we are testing it with a potentially
// different configuration at each test.
group = null;
}

});


/////////////////////////////
Expand All @@ -81,7 +50,9 @@ describe('Option removeOutsideVisibleBounds', function () {
marker3 = L.marker([1.5, 1.5]), // In view port.
marker4 = L.marker([1.5, 2.4]), // 1 screen width away.
marker5 = L.marker([1.5, 3.4]), // 2 screens width away.
div, map, group, previousMobileSetting;
markers = [marker1, marker2, marker3, marker4, marker5],
previousMobileSetting = L.Browser.mobile,
div, map, group, i;

div = document.createElement('div');
div.style.width = '200px';
Expand All @@ -97,32 +68,25 @@ describe('Option removeOutsideVisibleBounds', function () {
]));

// Add all markers once to map then remove them immediately so that their icon is null (instead of undefined).
map.removeLayer(marker1.addTo(map));
map.removeLayer(marker2.addTo(map));
map.removeLayer(marker3.addTo(map));
map.removeLayer(marker4.addTo(map));
map.removeLayer(marker5.addTo(map));
for (i = 0; i < markers.length; i++) {
map.removeLayer(markers[i].addTo(map));
}


function prepareGroup() {

// Group should be assigned with a Marker Cluster Group before calling this function.
// "group" should be assigned with a Marker Cluster Group before calling this function.
group.addTo(map);

// Add markers 1 by 1 to make sure we do not create an async process.
marker1.addTo(group);
marker2.addTo(group);
marker3.addTo(group);
marker4.addTo(group);
marker5.addTo(group);
group.addLayers(markers);
}


/////////////////////////////
// TESTS
/////////////////////////////

it2('removes objects more than 1 screen away from view port by default', function () {
it('removes objects more than 1 screen away from view port by default', function () {

group = L.markerClusterGroup();

Expand All @@ -134,43 +98,166 @@ describe('Option removeOutsideVisibleBounds', function () {

});

it2(
'removes objects out of view port by default for mobile device',
it('removes objects out of view port by default for mobile device', function () {

function () {
// Fool Leaflet, make it thinks it runs on a mobile device.
L.Browser.mobile = true;

// Fool Leaflet, make it thinks it runs on a mobile device.
previousMobileSetting = L.Browser.mobile;
L.Browser.mobile = true;
group = L.markerClusterGroup();

group = L.markerClusterGroup();
prepareGroup();

expect(marker1._icon).to.be(null);
expect(marker2._icon).to.be(null);
expect(map._panes.markerPane.childNodes.length).to.be(1); // marker 3 only.
expect(marker4._icon).to.be(null);
expect(marker5._icon).to.be(null);

});

it('leaves all objects on map when set to false', function () {

group = L.markerClusterGroup({
removeOutsideVisibleBounds: false
});

prepareGroup();

expect(map._panes.markerPane.childNodes.length).to.be(5); // All 5 markers.

});


// Following tests need markers at very high latitude.
// They test the _checkBoundsMaxLat method against the default Web/Spherical Mercator projection maximum latitude (85.0511287798).
// The actual map view should be '-1.0986328125,84.92929204957956,1.0986328125,85.11983467698401'
// The expdanded bounds without correction should be '-3.2958984375,84.7387494221751,3.2958984375,85.31037730438847'
var latLngsMaxLatDefault = [
[100, 3], // Impossible in real world, but nothing prevents the user from entering such latitude, and Web/Spherical Mercator projection will still display it at 85.0511287798
[85.2, 1.5], // 1 "screen" heights away.
[85, 0], // In center of view.
[84.8, -1.5], // 1 "screen" height away.
[84.6, -3] // 2 "screens" height away.
];

function moveMarkersAndMapToMaxLat(latLngs, isSouth) {
for (i = 0; i < markers.length; i++) {
if (isSouth) {
markers[i].setLatLng([-latLngs[i][0], latLngs[i][1]]);
} else {
markers[i].setLatLng(latLngs[i]);
}
}

prepareGroup();
map.fitBounds([
[isSouth ? -86 : 85, -1],
[isSouth ? -85 : 86, 1] // The actual map view longitude span will be wider. '-1.0986328125,84.92929204957956,1.0986328125,85.11983467698401'
]);
}

expect(marker1._icon).to.be(null);
expect(marker2._icon).to.be(null);
expect(map._panes.markerPane.childNodes.length).to.be(1); // marker 3 only.
expect(marker4._icon).to.be(null);
expect(marker5._icon).to.be(null);
function checkProjection(latLngs) {
expect(map.options.crs).to.equal(L.CRS.EPSG3857);
expect(L.CRS.EPSG3857.projection).to.equal(L.Projection.SphericalMercator);
expect(L.Projection.SphericalMercator.MAX_LATITUDE).to.be.a('number');

},
var mapZoom = map.getZoom();

// Extra final instruction to be called even on failure.
function () {
// Restore original setting, so that next tests are unaffected.
L.Browser.mobile = previousMobileSetting;
for (i = 0; i < markers.length; i++) {
markers[i].setLatLng(latLngs[i]);
try {
expect(markers[i].__parent._zoom).to.be.below(mapZoom);
} catch (e) {
console.log("Failed marker: " + (i + 1));
throw e;
}
}
);
}

it('includes objects above the Web Mercator projection maximum limit by default', function () {

moveMarkersAndMapToMaxLat(latLngsMaxLatDefault);

group = L.markerClusterGroup();

prepareGroup();

checkProjection(latLngsMaxLatDefault);

it2('leaves all objects on map when set to false', function () {
expect(map._panes.markerPane.childNodes.length).to.be(4); // Markers 1, 2, 3 and 4.
expect(marker5._icon).to.be(null);

});

it('includes objects below the Web Mercator projection minimum limit by default', function () {

moveMarkersAndMapToMaxLat(latLngsMaxLatDefault, true);

// Make sure we are really in Southern hemisphere.
expect(map.getBounds().getNorth()).to.be.below(-80);

group = L.markerClusterGroup();

prepareGroup();

checkProjection(latLngsMaxLatDefault);

expect(map._panes.markerPane.childNodes.length).to.be(4); // Markers 1, 2, 3 and 4.
expect(marker5._icon).to.be(null);

});


// The actual map view should be '-1.0986328125,84.92929204957956,1.0986328125,85.11983467698401'
var latLngsMaxLatMobile = [
[100, 1], // Impossible in real world, but nothing prevents the user from entering such latitude, and Web/Spherical Mercator projection will still display it at 85.0511287798
[85.2, 0.5], // 1 "screen" heights away, but should be included by the correction.
[85, 0], // In center of view.
[84.9, -1], // 1 "screen" height away.
[84.8, -1.5] // 2 "screens" height away.
];

it('includes objects above the Web Mercator projection maximum limit for mobile device', function () {

// Fool Leaflet, make it thinks it runs on a mobile device.
L.Browser.mobile = true;

moveMarkersAndMapToMaxLat(latLngsMaxLatMobile);

group = L.markerClusterGroup({
removeOutsideVisibleBounds: false
maxClusterRadius: 10
});

prepareGroup();

expect(map._panes.markerPane.childNodes.length).to.be(5); // All 5 markers.
checkProjection(latLngsMaxLatMobile);

expect(map._panes.markerPane.childNodes.length).to.be(3); // Markers 1, 2 and 3.
expect(marker4._icon).to.be(null);
expect(marker5._icon).to.be(null);

});

it('includes objects below the Web Mercator projection minimum limit for mobile device', function () {

// Fool Leaflet, make it thinks it runs on a mobile device.
L.Browser.mobile = true;

moveMarkersAndMapToMaxLat(latLngsMaxLatMobile, true);

// Make sure we are really in Southern hemisphere.
expect(map.getBounds().getNorth()).to.be.below(-80);

group = L.markerClusterGroup({
maxClusterRadius: 10
});

prepareGroup();

checkProjection(latLngsMaxLatMobile);

expect(map._panes.markerPane.childNodes.length).to.be(3); // Markers 1, 2 and 3.
expect(marker4._icon).to.be(null);
expect(marker5._icon).to.be(null);

});

Expand Down
33 changes: 30 additions & 3 deletions src/MarkerClusterGroup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,8 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
this._generateInitialClusters();
}

this._maxLat = map.options.crs.projection.MAX_LATITUDE;

for (i = 0, l = this._needsRemoving.length; i < l; i++) {
layer = this._needsRemoving[i];
this._removeLayer(layer, true);
Expand Down Expand Up @@ -542,7 +544,7 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
this._spiderfierOnRemove();
}


delete this._maxLat;

//Clean up all the layers we added to the map
this._hideCoverage();
Expand Down Expand Up @@ -920,10 +922,35 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
if (!this.options.removeOutsideVisibleBounds) {
return this._mapBoundsInfinite;
} else if (L.Browser.mobile) {
return this._map.getBounds();
return this._checkBoundsMaxLat(this._map.getBounds());
}

return this._map.getBounds().pad(1); // Padding expands the bounds by its own dimensions but scaled with the given factor.
return this._checkBoundsMaxLat(this._map.getBounds().pad(1)); // Padding expands the bounds by its own dimensions but scaled with the given factor.
},

/**
* Expands the latitude to Infinity (or -Infinity) if the input bounds reach the map projection maximum defined latitude
* (in the case of Web/Spherical Mercator, it is 85.0511287798 / see https://en.wikipedia.org/wiki/Web_Mercator#Formulas).
* Otherwise, the removeOutsideVisibleBounds option will remove markers beyond that limit, whereas the same markers without
* this option (or outside MCG) will have their position floored (ceiled) by the projection and rendered at that limit,
* making the user think that MCG "eats" them and never displays them again.
* @param bounds L.LatLngBounds
* @returns {L.LatLngBounds}
* @private
*/
_checkBoundsMaxLat: function (bounds) {
var maxLat = this._maxLat;

if (maxLat !== undefined) {
if (bounds.getNorth() >= maxLat) {
bounds._northEast.lat = Infinity;
}
if (bounds.getSouth() <= -maxLat) {
bounds._southWest.lat = -Infinity;
}
}

return bounds;
},

//Shared animation code
Expand Down