[Pkg-javascript-commits] [leaflet-markercluster] 37/128: Manage adding nested Layer Groups

Jonas Smedegaard dr at jones.dk
Sun Apr 16 06:26:01 UTC 2017


This is an automated email from the git hooks/post-receive script.

js pushed a commit to branch master
in repository leaflet-markercluster.

commit 7cf8152dd0059a5531e6ae2142ac3d136571fe19
Author: ghybs <ghybs1 at gmail.com>
Date:   Thu Jan 14 11:11:47 2016 +0400

    Manage adding nested Layer Groups
    
    Following #622 and #623, modified how addLayer manages adding a Layer Group.
    It now passes control to addLayers. The latter checks in its main loop if the current layer is a Layer Group.
    In that case, it appends all children to its input array and skips the current group.
    
    Also added corresponding test specs in AddLayer.SingleSpec and AddLayersSpec.
    Took the opportunity to modify these test suite to avoid PhantomJS memory leak.
---
 spec/suites/AddLayer.SingleSpec.js | 110 +++++++++++++++++++++++++++++++------
 spec/suites/AddLayersSpec.js       | 108 ++++++++++++++++++++++++++++++------
 src/MarkerClusterGroup.js          |  86 ++++++++++++++++++++++-------
 3 files changed, 249 insertions(+), 55 deletions(-)

diff --git a/spec/suites/AddLayer.SingleSpec.js b/spec/suites/AddLayer.SingleSpec.js
index 1b78f6d..521ac15 100644
--- a/spec/suites/AddLayer.SingleSpec.js
+++ b/spec/suites/AddLayer.SingleSpec.js
@@ -1,26 +1,70 @@
 describe('addLayer adding a single marker', function () {
-	var map, div;
+
+	/**
+	 * 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.
+	 */
+
+	/////////////////////////////
+	// SETUP FOR EACH TEST
+	/////////////////////////////
+
 	beforeEach(function () {
-		div = document.createElement('div');
-		div.style.width = '200px';
-		div.style.height = '200px';
-		document.body.appendChild(div);
 
-		map = L.map(div, { maxZoom: 18 });
+		// Nothing for this test suite.
 
-		map.fitBounds(new L.LatLngBounds([
-			[1, 1],
-			[2, 2]
-		]));
 	});
+
 	afterEach(function () {
-		document.body.removeChild(div);
+
+		if (group instanceof L.MarkerClusterGroup) {
+			group.clearLayers();
+			map.removeLayer(group);
+		}
+
+		// Throw away group as it can be assigned with different configurations between tests.
+		group = null;
+
 	});
 
 
+	/////////////////////////////
+	// PREPARATION CODE
+	/////////////////////////////
+
+	var div, map, group;
+
+	div = document.createElement('div');
+	div.style.width = '200px';
+	div.style.height = '200px';
+	document.body.appendChild(div);
+
+	map = L.map(div, { maxZoom: 18 });
+
+	// Corresponds to zoom level 8 for the above div dimensions.
+	map.fitBounds(new L.LatLngBounds([
+		[1, 1],
+		[2, 2]
+	]));
+
+
+	/////////////////////////////
+	// TESTS
+	/////////////////////////////
+
 	it('appears when added to the group before the group is added to the map', function () {
 
-		var group = new L.MarkerClusterGroup();
+		group = new L.MarkerClusterGroup();
+
 		var marker = new L.Marker([1.5, 1.5]);
 
 		group.addLayer(marker);
@@ -29,9 +73,11 @@
 		expect(marker._icon).to.not.be(undefined);
 		expect(marker._icon.parentNode).to.be(map._panes.markerPane);
 	});
+
 	it('appears when added to the group after the group is added to the map', function () {
 
-		var group = new L.MarkerClusterGroup();
+		group = new L.MarkerClusterGroup();
+
 		var marker = new L.Marker([1.5, 1.5]);
 
 		map.addLayer(group);
@@ -40,9 +86,11 @@
 		expect(marker._icon).to.not.be(undefined);
 		expect(marker._icon.parentNode).to.be(map._panes.markerPane);
 	});
+
 	it('appears (using animations) when added after the group is added to the map', function () {
 
-		var group = new L.MarkerClusterGroup({ animateAddingMarkers: true });
+		group = new L.MarkerClusterGroup({ animateAddingMarkers: true });
+
 		var marker = new L.Marker([1.5, 1.5]);
 
 		map.addLayer(group);
@@ -52,10 +100,10 @@
 		expect(marker._icon.parentNode).to.be(map._panes.markerPane);
 	});
 
-
 	it('does not appear when too far away when added before the group is added to the map', function () {
 
-		var group = new L.MarkerClusterGroup();
+		group = new L.MarkerClusterGroup();
+
 		var marker = new L.Marker([3.5, 1.5]);
 
 		group.addLayer(marker);
@@ -63,9 +111,11 @@
 
 		expect(marker._icon).to.be(undefined);
 	});
+
 	it('does not appear when too far away when added after the group is added to the map', function () {
 
-		var group = new L.MarkerClusterGroup();
+		group = new L.MarkerClusterGroup();
+
 		var marker = new L.Marker([3.5, 1.5]);
 
 		map.addLayer(group);
@@ -74,5 +124,31 @@
 		expect(marker._icon).to.be(undefined);
 	});
 
+	it('passes control to addLayers when marker is a Layer Group', function () {
+
+		group = new L.MarkerClusterGroup();
+
+		var marker1 = new L.Marker([1.5, 1.5]);
+		var marker2 = new L.Marker([1.5, 1.5]);
+		var layerGroup = new L.LayerGroup([marker1, marker2]);
+
+		map.addLayer(group);
+		group.addLayer(layerGroup);
+
+		expect(group._topClusterLevel.getChildCount()).to.equal(2);
+
+		expect(marker1._icon).to.be(undefined);
+		expect(marker2._icon).to.be(undefined);
+
+		expect(map._panes.markerPane.childNodes.length).to.be(1);
+	});
+
+
+	/////////////////////////////
+	// CLEAN UP CODE
+	/////////////////////////////
+
+	map.remove();
+	document.body.removeChild(div);
 
 });
diff --git a/spec/suites/AddLayersSpec.js b/spec/suites/AddLayersSpec.js
index dfa0066..6da479d 100644
--- a/spec/suites/AddLayersSpec.js
+++ b/spec/suites/AddLayersSpec.js
@@ -1,25 +1,70 @@
 describe('addLayers adding multiple markers', function () {
-	var map, div;
+
+	/**
+	 * 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.
+	 */
+
+	/////////////////////////////
+	// SETUP FOR EACH TEST
+	/////////////////////////////
+
 	beforeEach(function () {
-		div = document.createElement('div');
-		div.style.width = '200px';
-		div.style.height = '200px';
-		document.body.appendChild(div);
 
-		map = L.map(div, { maxZoom: 18 });
+		// Nothing for this test suite.
 
-		map.fitBounds(new L.LatLngBounds([
-			[1, 1],
-			[2, 2]
-		]));
 	});
+
 	afterEach(function () {
-		document.body.removeChild(div);
+
+		if (group instanceof L.MarkerClusterGroup) {
+			group.clearLayers();
+			map.removeLayer(group);
+		}
+
+		// Throw away group as it can be assigned with different configurations between tests.
+		group = null;
+
 	});
 
+
+	/////////////////////////////
+	// PREPARATION CODE
+	/////////////////////////////
+
+	var div, map, group;
+
+	div = document.createElement('div');
+	div.style.width = '200px';
+	div.style.height = '200px';
+	document.body.appendChild(div);
+
+	map = L.map(div, { maxZoom: 18 });
+
+	// Corresponds to zoom level 8 for the above div dimensions.
+	map.fitBounds(new L.LatLngBounds([
+		[1, 1],
+		[2, 2]
+	]));
+
+
+	/////////////////////////////
+	// TESTS
+	/////////////////////////////
+
 	it('creates a cluster when 2 overlapping markers are added before the group is added to the map', function () {
 
-		var group = new L.MarkerClusterGroup();
+		group = new L.MarkerClusterGroup();
+
 		var marker = new L.Marker([1.5, 1.5]);
 		var marker2 = new L.Marker([1.5, 1.5]);
 
@@ -34,7 +79,8 @@
 
 	it('creates a cluster when 2 overlapping markers are added after the group is added to the map', function () {
 
-		var group = new L.MarkerClusterGroup();
+		group = new L.MarkerClusterGroup();
+
 		var marker = new L.Marker([1.5, 1.5]);
 		var marker2 = new L.Marker([1.5, 1.5]);
 
@@ -47,11 +93,10 @@
 		expect(map._panes.markerPane.childNodes.length).to.be(1);
 	});
 
-
-
 	it('creates a cluster and marker when 2 overlapping markers and one non-overlapping are added before the group is added to the map', function () {
 
-		var group = new L.MarkerClusterGroup();
+		group = new L.MarkerClusterGroup();
+
 		var marker = new L.Marker([1.5, 1.5]);
 		var marker2 = new L.Marker([1.5, 1.5]);
 		var marker3 = new L.Marker([3.0, 1.5]);
@@ -65,9 +110,11 @@
 
 		expect(map._panes.markerPane.childNodes.length).to.be(2);
 	});
+
 	it('creates a cluster and marker when 2 overlapping markers and one non-overlapping are added after the group is added to the map', function () {
 
-		var group = new L.MarkerClusterGroup();
+		group = new L.MarkerClusterGroup();
+
 		var marker = new L.Marker([1.5, 1.5]);
 		var marker2 = new L.Marker([1.5, 1.5]);
 		var marker3 = new L.Marker([3.0, 1.5]);
@@ -82,4 +129,31 @@
 		expect(map._panes.markerPane.childNodes.length).to.be(2);
 	});
 
+	it('handles nested Layer Groups', function () {
+
+		group = new L.MarkerClusterGroup();
+
+		var marker1 = new L.Marker([1.5, 1.5]);
+		var marker2 = new L.Marker([1.5, 1.5]);
+		var marker3 = new L.Marker([3.0, 1.5]);
+		var layerGroup = new L.LayerGroup([marker1, new L.LayerGroup([marker2])]);
+
+		map.addLayer(group);
+		group.addLayers([layerGroup, marker3]);
+
+		expect(marker1._icon).to.be(undefined);
+		expect(marker2._icon).to.be(undefined);
+		expect(marker3._icon.parentNode).to.be(map._panes.markerPane);
+
+		expect(map._panes.markerPane.childNodes.length).to.be(2);
+	});
+
+
+	/////////////////////////////
+	// CLEAN UP CODE
+	/////////////////////////////
+
+	map.remove();
+	document.body.removeChild(div);
+
 });
\ No newline at end of file
diff --git a/src/MarkerClusterGroup.js b/src/MarkerClusterGroup.js
index 852194c..60dd443 100644
--- a/src/MarkerClusterGroup.js
+++ b/src/MarkerClusterGroup.js
@@ -74,11 +74,7 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 	addLayer: function (layer) {
 
 		if (layer instanceof L.LayerGroup) {
-			var array = [];
-			for (var i in layer._layers) {
-				array.push(layer._layers[i]);
-			}
-			return this.addLayers(array);
+			return this.addLayers([layer]);
 		}
 
 		//Don't cluster non point data
@@ -180,19 +176,24 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 
 	//Takes an array of markers and adds them in bulk
 	addLayers: function (layersArray) {
+		if (!L.Util.isArray(layersArray)) {
+			return this.addLayer(layersArray);
+		}
+
 		var fg = this._featureGroup,
-			npg = this._nonPointGroup,
-			chunked = this.options.chunkedLoading,
-			chunkInterval = this.options.chunkInterval,
-			chunkProgress = this.options.chunkProgress,
-			newMarkers, i, l, m;
+		    npg = this._nonPointGroup,
+		    chunked = this.options.chunkedLoading,
+		    chunkInterval = this.options.chunkInterval,
+		    chunkProgress = this.options.chunkProgress,
+		    l = layersArray.length,
+		    offset = 0,
+		    m;
 
 		if (this._map) {
-			var offset = 0,
-				started = (new Date()).getTime();
+			var started = (new Date()).getTime();
 			var process = L.bind(function () {
 				var start = (new Date()).getTime();
-				for (; offset < layersArray.length; offset++) {
+				for (; offset < l; offset++) {
 					if (chunked && offset % 200 === 0) {
 						// every couple hundred markers, instrument the time elapsed since processing started:
 						var elapsed = (new Date()).getTime() - start;
@@ -203,6 +204,14 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 
 					m = layersArray[offset];
 
+					// Group of layers, append children to layersArray and skip.
+					// Side effect: total increases, so chunkProgress ratio jumps backward.
+					if (m instanceof L.LayerGroup) {
+						this._extractNonGroupLayers(m, layersArray);
+						l = layersArray.length;
+						continue;
+					}
+
 					//Not point data, can't be clustered
 					if (!m.getLatLng) {
 						npg.addLayer(m);
@@ -219,7 +228,7 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 					if (m.__parent) {
 						if (m.__parent.getChildCount() === 2) {
 							var markers = m.__parent.getAllChildMarkers(),
-								otherMarker = markers[0] === m ? markers[1] : markers[0];
+							    otherMarker = markers[0] === m ? markers[1] : markers[0];
 							fg.removeLayer(otherMarker);
 						}
 					}
@@ -227,11 +236,11 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 
 				if (chunkProgress) {
 					// report progress and time elapsed:
-					chunkProgress(offset, layersArray.length, (new Date()).getTime() - started);
+					chunkProgress(offset, l, (new Date()).getTime() - started);
 				}
 
 				// Completed processing all markers.
-				if (offset === layersArray.length) {
+				if (offset === l) {
 
 					// Refresh bounds and weighted positions.
 					this._topClusterLevel._recalculateBounds();
@@ -251,9 +260,17 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 
 			process();
 		} else {
-			newMarkers = [];
-			for (i = 0, l = layersArray.length; i < l; i++) {
-				m = layersArray[i];
+			var needsClustering = this._needsClustering;
+
+			for (; offset < l; offset++) {
+				m = layersArray[offset];
+
+				// Group of layers, append children to layersArray and skip.
+				if (m instanceof L.LayerGroup) {
+					this._extractNonGroupLayers(m, layersArray);
+					l = layersArray.length;
+					continue;
+				}
 
 				//Not point data, can't be clustered
 				if (!m.getLatLng) {
@@ -265,9 +282,8 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 					continue;
 				}
 
-				newMarkers.push(m);
+				needsClustering.push(m);
 			}
-			this._needsClustering = this._needsClustering.concat(newMarkers);
 		}
 		return this;
 	},
@@ -997,6 +1013,34 @@ L.MarkerClusterGroup = L.FeatureGroup.extend({
 	},
 
 	/**
+	 * Extracts individual (i.e. non-group) layers from a Layer Group.
+	 * @param group to extract layers from.
+	 * @param output {Array} in which to store the extracted layers.
+	 * @returns {*|Array}
+	 * @private
+	 */
+	_extractNonGroupLayers: function (group, output) {
+		var layers = group.getLayers(),
+		    i = 0,
+		    layer;
+
+		output = output || [];
+
+		for (; i < layers.length; i++) {
+			layer = layers[i];
+
+			if (layer instanceof L.LayerGroup) {
+				this._extractNonGroupLayers(layer, output);
+				continue;
+			}
+
+			output.push(layer);
+		}
+
+		return output;
+	},
+
+	/**
 	 * Implements the singleMarkerMode option.
 	 * @param layer Marker to re-style using the Clusters iconCreateFunction.
 	 * @returns {L.Icon} The newly created icon.

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/leaflet-markercluster.git



More information about the Pkg-javascript-commits mailing list