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.

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

Hiding markers off-screen to increase performance #1324

Closed
skunkfu2 opened this issue Jan 29, 2013 · 15 comments
Closed

Hiding markers off-screen to increase performance #1324

skunkfu2 opened this issue Jan 29, 2013 · 15 comments

Comments

@skunkfu2
Copy link

I’m not sure if this is a minor or major feature, and that it has probably already been suggested (with the MarkerCluster plugin being the result) but I will give it a shot.

I am placing custom markers on a game map for loot locations. Currently there are approx. 200 markers on the map, and I have covered around 10% of it so far. When viewing the map with all the markers displayed on either the laptop or a Galaxy Nexus, I notice a slowdown of the map. I fear this will grow significantly when I've placed all the markers.

I would consider using MarkerCluster if it wasn't for the nature of needing to see all of the marker locations at once, and the whole two clicking bug in Android using a WebView (even with jerel’s fix).

Question: How easy would it be to implement a function that would hide markers that are off screen, just like how the tiles are rendered? This would boost the map performance considerably, and be of much help for many people.

I realise this has probably been looked at before, and there is a reason why such a feature hasn't been implemented, but I thought I’d ask.

Thank you for Leaflet Vladimir and its contributors, it has been bliss for me so far :)

@MFuglsang
Copy link

Im am doing that by hand in one of my applications.

I am using MySQL and php to dynamically render the markers within the bbox on each move - it is quite simple to impliment...

@skunkfu2
Copy link
Author

Hey MFuglsang,

That sounds interesting. Would it be possible you could share your code? If not that's well understandable.

I have touched on MySQL and PHP before but I can't say I'm good at it.

I'm looking at a simple Boolean option such as 'unloadInvisibleTiles' for L.TileLayer, for markers.

This is what Leaflet is good at!

@danzel
Copy link
Member

danzel commented Jan 30, 2013

There isn't an option built in to leaflet to do this.

You should be able to achieve it by doing something like this:
(Note: totally untested, just wrote it in here, some function names are probably wrong)

var myMarkers = [... this is an array of all the L.Marker for your data... ];

map.on('moveend, placeMarkersInBounds);

placeMarkersInBounds();

function placeMarkersInBounds() {
    var mapBounds = map.getBounds();
    for (var i = myMarkers.length -1; i >= 0; i--) {
        var m = myMarkers[i];
        var shouldBeVisible = mapBounds.contains(m.getLatLng());
        if (m._icon && !shouldBeVisible) {
            map.removeLayer(m);
        } else if (!m._icon && shouldBeVisible) {
            map.addLayer(m);
        }
    }
}

@skunkfu2
Copy link
Author

Hey danzel, thanks for helping out with this.

I have implemented this function into one of the Leaflet examples, but I don't think it is working? When dragging the map until the marker is not visible, then panning back to it, the marker is still visible before I stop the dragging. I've tried other events to test the function out, but to no avail. Here is the code I'm using, I'd appreciate it if you could test it out! I'm probably missing something.

var cloudmadeUrl = 'http://{s}.tile.cloudmade.com/BC9A493B41014CAABB98F0471D759707/997/256/{z}/{x}/{y}.png',
    cloudmadeAttribution = 'Map data © 2011 OpenStreetMap contributors, Imagery © 2011 CloudMade',
    cloudmade = new L.TileLayer(cloudmadeUrl, {maxZoom: 18, attribution: cloudmadeAttribution}),
    latlng = new L.LatLng(50.5, 30.51);

var map = new L.Map('map', {center: latlng, zoom: 15, layers: [cloudmade]});

var marker = new L.Marker(latlng);
map.addLayer(marker);

map.on('moveend', placeMarkersInBounds);

placeMarkersInBounds();

function placeMarkersInBounds() {
    var mapBounds = map.getBounds();
    for (var i = marker.length -1; i >= 0; i--) {
        var m = marker[i];
        var shouldBeVisible = mapBounds.contains(m.getLatLng());
        if (m._icon && !shouldBeVisible) {
            map.removeLayer(m);
        } else if (!m._icon && shouldBeVisible) {
            map.addLayer(m);
        }
    }
}

Thank you.

@skunkfu2
Copy link
Author

Hey danzel, after much trial and error, it appears I am not doing the whole marker array properly. How would I go about creating an array of markers? Sorry for the ignorance.

Thank you.

@danzel
Copy link
Member

danzel commented Jan 30, 2013

Change

var marker = new L.Marker(latlng);
to
var markers = [ new L.Marker(latlng) ];

Then in your loop change marker to markers.
That should do it I think.

@skunkfu2
Copy link
Author

Life saver, woks perfect, cheers danzel!

Would there be a chance such a function could be incorporated into Leaflet, or a plugin perhaps? I imagine this would benefit many who are in the same situation I was.

Thanks again mucka.

@danzel
Copy link
Member

danzel commented Jan 30, 2013

Glad we've solved your issue.

I'd recommend using the MarkerCluster layer in almost all cases rather than something like this.
You can have it disable clustering below a certain level which I believe would allow you to provide a better user experience.
Have the top levels have clustering then disable it when you are low enough that all the points can be visible.

@danzel danzel closed this as completed Jan 30, 2013
@skunkfu2
Copy link
Author

Sound advice. I'll look into MarkerCluster again once the two clicking bug is fixed. Cheers bud :)

@Danielku15
Copy link
Contributor

Even though I might be reviving corpses here with this comment I wanted to share my fiddle with you guys. We today encountered a similar issue that with 1000+ markers the map starts to lag.

Enabling markercluster as plugin might be a bit an overhead and also did not provide satisfying results for us. MarkerCluster might delay the icon creation leading to problems when you also want to update icon styles for hidden markers.

I created a small mixin/extension to the L.Marker class which does nothing but removing/adding the icons and shadows from/to the DOM depending on their visibility. The result can be seen here:

https://jsfiddle.net/n5rqfhg9/7/ (Virtualization Enabled - Smooth)
https://jsfiddle.net/n5rqfhg9/8/ (Virtualization Disabled - Laggy)

The demo also shows the number of marker and shadow DOM elements. Also it does not improve the loading time but once the markers are initialized, the zooming and panning is a lot smoother than without virtualization.

Also be aware that the add/remove of the icon from the DOM might have side effects like a spoiled z-index of elements.

@dyardyGIT
Copy link

@Danielku15 I am not sure i notice difference in #7 and #8 above. Is this still best choice for showing/hiding based on bounding box?
thanks

@Danielku15
Copy link
Contributor

@dyardyGIT What browser did you use in testing? I can see a noticeable performance difference on the latest chrome. I tried to record it but due to the compression some details might be lost. here the gif on giphy.

@joachimoak
Copy link

Thanks @Danielku15 ; very useful.

@underrobyn
Copy link

Thank you @Danielku15 !

The code snippet you posted increased the performance of my map significantly (I'm rendering about 19,000 markers spread across the UK)

@bhekuwenza
Copy link

👍 @Danielku15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants