SINTEF-9012 / PruneCluster

Fast and realtime marker clustering for Leaflet
MIT License
551 stars 131 forks source link

PruneCluster(which is the best from lealet cluster plugins) development and updates #140

Open cordovapolymer opened 7 years ago

cordovapolymer commented 7 years ago

Hi, I've compared all leaflet clustering plugins and PruneCluster is the best. But some of the reported and still unfixed issues https://github.com/SINTEF-9012/PruneCluster/issues/138 https://github.com/SINTEF-9012/PruneCluster/issues/131 https://github.com/SINTEF-9012/PruneCluster/issues/129 https://github.com/SINTEF-9012/PruneCluster/issues/125 https://github.com/SINTEF-9012/PruneCluster/issues/124 block me from using it in my project.

@yellowiscool @simison @FalloutPL @mordka @vitalik74 @rwwagner90 @brunolellis @maxvonhippel @Cadrach @undr @uniphil @valerio-bozzolan @rolanddu @adimasci @maxhoffmann Is there any hope for fixes?

maxvonhippel commented 7 years ago

This should not be a new issue. Moreover, the issues you've commented on almost definitely all arise from a common problem in the code base for animations and pointer rendering. They should probably just be one issue, rather than now 4 in which you've tagged tons of people, or 5 counting this one.

Why don't you dig into the source code for the particular error you are having and comment in one of those issues when you find some useful information. Then I and/or others can work off of that. In the mean time, this issue should be closed. And in my personal opinion, an administrator should close 129, 125, and 124 and simply reiterate the information therein in 131 (or some similar re-ordering and collapsing) since I believe that all the linked issues arise from the same root cause in the animations code.

cordovapolymer commented 7 years ago

@maxvonhippel , thanks for a promt answer. I've found that there were no updates on the issues so decided to call all people involved in the project to comment on this. I have to make a decision if PruneCluster is worth using as a long-term Leaflet clustering solution.

Good think you've figured out that it's the same issue, it definitely looked like multiple bugs from the first glance, do you have ideas how to fix it?

valerio-bozzolan commented 7 years ago

@cordovapolymer Absolutely please stop pinging everybody in every bug.

For a funny explanation about why it's not good to think that everybody work for you, see https://github.com/valerio-bozzolan/AcrylicPaint/issues/40.

maxvonhippel commented 7 years ago

I have not figured out that it is the same issue. It just looks that way. I quote:

(131)

When I bindTooltip to markers and add them to prunecluster, the tooltips didn't show up, but when I Unzoom or zoom, the tooltip works.

(138)

when I add class 'prunecluster-anim' to marker the marker is animated on moving, but the binded Tooltip don't follow the marker animation. How can I animate it?

(129)

When I bind the label into marker, the label still showing up if I zoom out and marker are clustered. Also when I remove a marker, label still showing up.

(125)

So currently the markers are not updated when they're in a spiderfied cluster when e.g. new data arrives although they should.

(124)

When a cluster is spiderfied and a marker at this location is added afterwards, it should either unspiderfy the open cluster (like the original Leaflet.markercluster does) or update the spiderfied cluster so that the new marker is included.

Ok, so clearly 125 is because the spiderfied markers are not dynamic yet, as someone commented in 124. So making it so that the spiderfied clusters update when new data arrives fixes 124 and 125.

The other three issues in my eyes say the same basic thing: add a label to a marker, then do some animation (click, zoom, drag, whatever), and that label does not behave how you expect. In other words, the markers are not sufficiently dynamic.

So either you have two issues, one with label rendering pre and post animation and one with marker spiderfication reanimation, or you have one root issue with markers needing to be more dynamic and "reload" when new data arrives or parameters change. Seems fairly clear to me.

And thank you for backing me up on this @valerio-bozzolan . It's bad form to ping everyone, and also this is not an issue.

@valerio-bozzolan if you have admin rights could you please close this issue?

valerio-bozzolan commented 7 years ago

@maxvonhippel The funny part is that I'm not the owner of this project. I've only contributed with a comment.

maxvonhippel commented 7 years ago

@valerio-bozzolan hahahah that's hilarious. I'm a contributor on a Netflix project for deleting a line of superfluous code, it's funny how things work out that way.

@cordovapolymer How much data are you working with? In my experience, on most average browsers on average internet connections outside of the US on average computers, you would be hard-pressed to get much dynamic or "updating" behavior in any clustering or heat map libraries after hitting around 50,000 to 100,000 nodes. When I contributed to this project (I made the JSON example) I was working with 3,000,000+ nodes. I was thrilled when I got the map to load at all on suboptimal internet and an old browser; getting it to do real-time dynamic updates wasn't even in the realm of possibility.

It is more than possible that you may be better served researching tile servers and other ways to outsource the computation involved in something like this. Even if the computer has a lot of RAM, you're limited by the security and usability limitations hard-coded into the browser. Web coding is inherently an exercise in inconvenience.

cordovapolymer commented 7 years ago

@maxvonhippel I want PruneCluster for it's https://github.com/SINTEF-9012/PruneCluster#setting-up-a-custom-cluster-icon . In my use case I have numbered markers and I want to use PruneCluster to create cluster icons containing all numbers from grouped markers in a chessboard style. It's very nice that PruneCluster is really fast, but I process my data on server side just like you suggest. In my use case it will be dealing with 100 to 5000 live moving markers.

maxvonhippel commented 7 years ago

@cordovapolymer how often do you want to refresh the view? That's a very small amount of markers. You might not even need to fix those problems to accomplish this, you could just redraw the map entirely, if you're only updating like twice a minute or something. 500 is nothing.

cordovapolymer commented 7 years ago

@maxvonhippel My implementation updates every marker independently because data comes from many devices via mqtt protocol, i think it's an advantage and wouldn't like to downgrade to redrawing map every time. Updates are sent every second, so it would be 5000 map redraws a second.

maxvonhippel commented 7 years ago

@cordovapolymer do you have code publicly I can look at? Anyway you can queue updates but then actually refresh once every other second or something, that's not a difficult technical challenge.

cordovapolymer commented 7 years ago

@maxhoffmann of course it's not a challenge, I just want to make it clean and shiny.

Here's an extract from my map class

        if(typeof this.markers[pointType][item._id] === 'undefined'){
       this.createPoint(data, pointType, coordinates, iconType);
      }
      else{
        this.updatePoint(data, pointType, coordinates, iconType);
      }

      createPoint: function(data, pointType, coordinates, iconType) {
        var latlng = [coordinates[1], coordinates[0]];
        this.markers[pointType][data._id] = L.marker(latlng,
          {icon: this.icon[iconType]});
        this.groups[pointType].addLayer(this.markers[pointType][data._id]);
      },

      updatePoint: function(data, pointType, coordinates, iconType) {
        var markerPos = this.markers[pointType][data._id].getLatLng();
        if(markerPos.lat !== coordinates[1] && markerPos.lng !== coordinates[0]){
          var latlng = [coordinates[1], coordinates[0]];

          this.markers[pointType][data._id].slideCancel();
          this.markers[pointType][data._id].slideTo( latlng, {
              duration: 5000
          });
        }
      }
simison commented 7 years ago

@cordovapolymer sorry but I'd appreciate not so much ping spam from you. We have watch and subscribe buttons at github for a reason. :P

FYI @yellowiscool (the maintainer)

RobbieTheWagner commented 7 years ago

@cordovapolymer definitely not cool to tag so many people. My email inbox was on fire with all these pings. Tagging everyone is not going to help you get a fix any quicker. Those who are active on the project will see your issue eventually, and I do agree all these issues should be consolidated, and you can help us fix them by digging into the code yourself 😄 .