preliminary version of clusters #1903

Kenny806 · 2014-03-26T16:47:32Z

Adresses issue #1573 . Preparation for clustering, with an example that clusters random points and prints the clusters to console. Things to be done:

Tests
Refactoring
Not clustering features outside the given extent
Recalculating clusters on zoom/extent change
Clicking on cluster should zoom to the extent of the clustered features
(maybe add support for more clustering algorithms?)

Any other suggestions are welcome.

tschaub · 2014-03-26T18:07:09Z

Clicking on cluster should zoom to the extent of the clustered features

This should be application specific behavior. It would be nice if a cluster carried some information about the features it represents (count, extent, maybe list of ids). But behavior when clicking on a feature should be determined by the application.

I look forward to taking a closer look at this later. Just wanted to add a quick comment.

ahocevar · 2014-03-26T18:11:28Z

@Kenny806 Did I misunderstand @jachym, or are you attending the OSGeo code sprint in Vienna? If so, we can take a look at this pull request together tomorrow.

Kenny806 · 2014-03-26T19:05:40Z

@ahocevar I was there, but I left yesterday, just a few hours after @jachym

Kenny806 · 2014-03-29T12:25:02Z

I am having a bit of trouble with recalculating the clusters on zoom/extent change. I need to listen to a map event (preferably moveend, which seems to be fired less often than postrender, but still is fired avery time when needed). Which means that either ol.Cluster should have a 'map' property, in order to be able to listen to events dispatched by map, or ol.Map must have a property which would store active ol.Cluster instances so that ol.Map can call the reclustering method when needed.

I prefer the latter option. Currently, ol.Map has properties like controls_, so I was thinking about adding a strategies_ property to ol.Map (OL2 had a 'strategies' namespace, which included clustering) and renaming ol.Cluster to ol.strategies.Cluster. Any thoughts?

tschaub · 2014-03-29T17:26:58Z

As suggested by @twpayne, this could be implemented as ol.source.Cluster. This source would take another vector source in its constructor. Requests to loadFeatures(extent, resolution, projection) could trigger cluster calculation based on features in the underlying source. This should likely wait until the work on #1744 is complete.

An alternative would be to work in the idea of a customizable transform before adding loaded features.

Kenny806 · 2014-04-07T15:35:33Z

Thanks for the input @tschaub, I have implemented it in today's commit. Could you take a look at the code when you have the time to do it? Right now, I believe that tests and some typdefs are the only things that are missing.

melechi · 2014-05-09T08:44:08Z

Hi @Kenny806, any update on this?

Kenny806 · 2014-05-10T09:27:03Z

Hi, I haven't touched this in a while. Right now, I only need to write some tests and make the build process pass (on my local machine, everything passes, but the Travis CI build fails), otherwise everything seems to be working. I will try and look into it this week , thanks for the reminder.

melechi · 2014-05-10T20:33:31Z

Thanks @Kenny806, I look forward to trying it.

mvaivre · 2014-05-22T14:17:02Z

Yes @Kenny806 ! I can't wait to test it ;)

jachym · 2014-05-24T21:25:18Z

Hi there, me and @Kenny806 do have same strange experience: the ./build.py integration-test works at ours desktops, but fails in travis. Any hint here?

mvaivre · 2014-06-07T13:08:26Z

That's strange. Is there anyone to help @Kenny806 and @jachym ? I'm sure the clustering functionality is awaited by a lot of people...

pagameba · 2014-07-08T00:38:33Z

src/ol/source/clustersource.js

+
+
+/**
+ *


this requires a description to build.

pagameba · 2014-07-08T01:58:49Z

Ok, with these changes, this PR will build when rebased on master.

I only looked at the minimum to get travis happy. There probably needs to be more documentation and annotations in the source, and I didn't look at the test or example.

Kenny806 · 2014-07-09T11:59:14Z

Thanks for the help, @pagameba, I will try to fix this soon :-)

pagameba · 2014-07-09T12:23:54Z

no problem, happy to help out and I'd like to see this make it in to 3.0.

elemoine · 2014-07-09T12:25:03Z

I also plan to review this. (But I probably plan too much…)

pagameba · 2014-07-17T12:45:44Z

src/ol/source/clustersource.js

+ * @param {number} resolution
+ */
+ol.source.Cluster.prototype.loadFeatures = function(extent, resolution) {
+  if (extent !== this.getExtent() || resolution !== this.getResolution()) {


it turns out this isn't a great test for several reasons:

the extent returned by this.getExtent comes from an internal rbush thing and not from a previous call to setExtent so the extent is never the same and loadFeatures is called continuously because clear followed by addFeatures triggers a new map render.

you cannot compare extents this way, you need to do something like !ol.extent.equals(extent, this.getExtent())

For the purpose of caching the currently rendered extent, I think a separate instance var will be needed (perhaps clusteredExtent_)

pagameba · 2014-07-17T13:01:46Z

@Kenny806 I've updated my copy of your branch with most the of the changes above. Please feel free to use whatever you want from that branch however you want or to ignore it altogether :) In particular, I was messing around with the example so you probably want to change that back. It would be good, though, to at least through more points at it and perhaps try to label the clustered features with the count of features clustered or something.

Kenny806 · 2014-07-21T16:24:24Z

@pagameba many thanks for your comments and for implementing them as well! I have applied patches from your commits on this branch. Now I just hope to finish writing tests ASAP. I am also thinking about modifying the example to show how many features is in each cluster (just like you suggested). This should probably be application specific behavior, so it would be nice to have in the example.

pagameba · 2014-07-21T16:49:29Z

You are welcome :)
Please rebase and fix your commit messages up to match the guidelines, in particular you should begin each commit message with a (capitalized) verb (i.e. Fix jsdoc instead of jsdoc fixed).
Also, the commits should be squashed and reorganized - one for each source file adding a new class, one for the example and one for the tests. The history of how it got to this point doesn't need to be preserved for merging.

ahocevar · 2014-07-24T15:34:08Z

@marcjansen is going to rebase and merge. Thanks @Kenny806 for the continued work on this.

marcjansen · 2014-07-24T15:34:19Z

I'll clean the history and then merge. Thanks everybody so far.

fredj · 2014-07-24T15:39:09Z

examples/cluster.js

+var source = new ol.source.Vector();
+
+var clusterOptions = {
+  'source': source


the quotes are not necessary

marcjansen · 2014-07-24T15:40:30Z

@Kenny806 I think we need a CLA from you before we can merge this.

fredj · 2014-07-24T15:50:37Z

externs/olx.js

+ * @typedef {{features: (Array.<ol.Feature>)
+ * }}
+ */
+olx.ClusterFeatureOptions;


olx.ClusterFeatureOptions.prototype.features is missing

fredj · 2014-07-24T16:00:01Z

@marcjansen please see fredj@80ba469

Fixes some issues in externs/olx.js

marcjansen · 2014-07-24T17:15:24Z

Thanks, @fredj. Let's see if we can get the cla in time...

bartvde · 2014-07-25T07:45:21Z

closing this one in favour of #2439

marcjansen · 2014-07-25T07:45:23Z

#2439 is the replacement PR with a clean history and the changes suggested by @fredj,

Kenny806 · 2014-07-25T09:12:30Z

I have just sent the CLA. Let me know if there is anything else

elemoine modified the milestones: v3.0.0, v3.0.0-gamma.2 Jun 26, 2014

elemoine self-assigned this Jul 3, 2014

tonio added the feature label Jul 3, 2014

elemoine mentioned this pull request Jul 3, 2014

Add Clustering #1573

Closed

pagameba reviewed Jul 8, 2014
View reviewed changes

src/ol/source/clustersource.js

/**

*

Copy link

Member

pagameba Jul 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this requires a description to build.

elemoine removed this from the v3.0.0-gamma.2 milestone Jul 11, 2014

pagameba reviewed Jul 17, 2014
View reviewed changes

Jiri Matyas added 10 commits July 21, 2014 12:50

preliminary version of clusters

2f5811f

cluster as source

9e3e632

jsdoc fixed

905ab1e

exports

d4f26d8

tests

84321f4

make travis build pass

10c1031

use ol.source.Vector for gettin features

15ea0d4

fix text for previous extent

632bfb1

changeloops to ol3style

36b90dc

exports cluster feature and its getFeatures method

3ca7b93

fredj reviewed Jul 24, 2014
View reviewed changes

examples/cluster.js

var source = new ol.source.Vector();

var clusterOptions = {

'source': source

Copy link

Member

fredj Jul 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the quotes are not necessary

fredj reviewed Jul 24, 2014
View reviewed changes

bartvde modified the milestones: v3.0.0-gamma.4, v3.0.0-gamma.3 Jul 25, 2014

marcjansen mentioned this pull request Jul 25, 2014

Add clustering functionality #2439

Closed

bartvde closed this Jul 25, 2014

marcjansen mentioned this pull request Jul 28, 2014

Add clustering functionality #2458

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

preliminary version of clusters #1903

preliminary version of clusters #1903

Kenny806 commented Mar 26, 2014

tschaub commented Mar 26, 2014

ahocevar commented Mar 26, 2014

Kenny806 commented Mar 26, 2014

Kenny806 commented Mar 29, 2014

tschaub commented Mar 29, 2014

Kenny806 commented Apr 7, 2014

melechi commented May 9, 2014

Kenny806 commented May 10, 2014

melechi commented May 10, 2014

mvaivre commented May 22, 2014

jachym commented May 24, 2014

mvaivre commented Jun 7, 2014

pagameba Jul 8, 2014

pagameba commented Jul 8, 2014

Kenny806 commented Jul 9, 2014

pagameba commented Jul 9, 2014

elemoine commented Jul 9, 2014

pagameba Jul 17, 2014

pagameba commented Jul 17, 2014

Kenny806 commented Jul 21, 2014

pagameba commented Jul 21, 2014

ahocevar commented Jul 24, 2014

marcjansen commented Jul 24, 2014

fredj Jul 24, 2014

marcjansen commented Jul 24, 2014

fredj Jul 24, 2014

fredj commented Jul 24, 2014

marcjansen commented Jul 24, 2014

bartvde commented Jul 25, 2014

marcjansen commented Jul 25, 2014

Kenny806 commented Jul 25, 2014

preliminary version of clusters #1903

preliminary version of clusters #1903

Conversation

Kenny806 commented Mar 26, 2014

tschaub commented Mar 26, 2014

ahocevar commented Mar 26, 2014

Kenny806 commented Mar 26, 2014

Kenny806 commented Mar 29, 2014

tschaub commented Mar 29, 2014

Kenny806 commented Apr 7, 2014

melechi commented May 9, 2014

Kenny806 commented May 10, 2014

melechi commented May 10, 2014

mvaivre commented May 22, 2014

jachym commented May 24, 2014

mvaivre commented Jun 7, 2014

pagameba Jul 8, 2014

Choose a reason for hiding this comment

pagameba commented Jul 8, 2014

Kenny806 commented Jul 9, 2014

pagameba commented Jul 9, 2014

elemoine commented Jul 9, 2014

pagameba Jul 17, 2014

Choose a reason for hiding this comment

pagameba commented Jul 17, 2014

Kenny806 commented Jul 21, 2014

pagameba commented Jul 21, 2014

ahocevar commented Jul 24, 2014

marcjansen commented Jul 24, 2014

fredj Jul 24, 2014

Choose a reason for hiding this comment

marcjansen commented Jul 24, 2014

fredj Jul 24, 2014

Choose a reason for hiding this comment

fredj commented Jul 24, 2014

marcjansen commented Jul 24, 2014

bartvde commented Jul 25, 2014

marcjansen commented Jul 25, 2014

Kenny806 commented Jul 25, 2014