The attached patch adds a "cluster" behavior, the views interface for it and popup/tooltip support for it.

Next steps would include:
- allow specifying multiple layers to cluster (currently only one is supported)
- allow styling to use clustering information (tipical use: icon size dependent on number of elements in a cluster)

Files: 
CommentFileSizeAuthor
#30 cluster_popup_missing_drupalFID.patch1.34 KBstrk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch cluster_popup_missing_drupalFID.patch.
[ View ]
#27 multilayer_cluster.patch2.8 KBstrk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch multilayer_cluster.patch.
[ View ]
#25 popup_callbacks.diff4.19 KBstrk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch popup_callbacks.diff.
[ View ]
#19 ClusterPopupOverride.patch1.95 KBstrk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ClusterPopupOverride.patch.
[ View ]
#18 PopupsClean2.patch1.77 KBstrk
PASSED: [[SimpleTest]]: [MySQL] 221 pass(es).
[ View ]
#16 PopupsClean.patch1.62 KBstrk
PASSED: [[SimpleTest]]: [MySQL] 221 pass(es).
[ View ]
#11 ClusterClean.patch4.08 KBstrk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ClusterClean_0.patch.
[ View ]
#2 ClusterAndPopups2.patch7.4 KBstrk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ClusterAndPopups2.patch.
[ View ]
ClusterAndPopups.patch9.19 KBstrk
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ClusterAndPopups.patch.
[ View ]

Comments

tmcw’s picture

Awesome to see this happening, this could be very useful. Here are a few notes on the patch:

  • Please use CVS diff or a compatible format for patching - this patch doesn't apply without specifying paths, and it patches CVS/Entries.
  • Also this git patch creates a directory b/sites/all/modules/openlayers within the OpenLayers directory
  • openlayers_behavior_cluster.inc needs to be run through Coder - it doesn't comply with Drupal coding standards
  • The if-not-continue syntax is okay, but it's much preferable to just use an if that surrounds the inside of loops - it's much easier to read for most. Same with the if-return syntax - it's okay, but not preferable.
  • Could you more thoroughly explain the 'pseudofeature' concept and why it's necessary / this implementation is best? Could this be something done in PHP instead of JS? We're quite cautious of Javascript bloat, because it adds a layer of complexity in an area which is very hard to debug
  • It seems like a bad idea to have interdependencies between behaviors. Could the cluster behavior do an alter to add its own popup content function instead of this changeset including changes within the popup function itself?
strk’s picture

StatusFileSize
new7.4 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ClusterAndPopups2.patch.
[ View ]

Please see if this new patch applies cleanly (for points 1 and 2). patch -p1 works fine with it here.

Will check out Coder when I have a chance but it'd be much simpler if all or at least portion of the patch is committed
as I'm keeping to add things and can't freeze easily.

About the 'pseudofeature' it's not a concept I'm adding myself but it's what happens with the current code.
A feature is a bag of attributes associated with a geometrical object.
The way I see it it maps pretty much with a spatial-enabled drupal node, that is a node with a spatial attribute.
What current code does is exploding the spatial attribute (defined by WKT) so that simple component geometries
are extracted. For example if you have a GEOMETRYCOLLECTION(POINT POINT POINT) the current code extracts
those 3 points and makes them available in core attaching the node attributes to each of them again.
For OL there would be 3 features, not one.
I haven't tested how would OL render the GEOMETRYCOLLECTION directly, nor I know if it'd be any better.
In any case the "pseudofeature" I add to mean that those OL features aren't really the original nodes but an explosion
of them. Multiple "pseudofeature" can map to the same _real_ feauture (the node).

Reguardless of the pseudofeature thing, if WKT was parsed by PHP it'd be much better, for many reasons, including the one
you might want to support a different storage format (say GEO or lat/lon or whatever else).

For behaviour interdependencies I've no idea about the 'alter' concept in drupal, I'm new here.
All I know is that the current popup/tooltip behaviours use an hard-coded callback for fetching the contents, while if they allowed
for registering one the cluster might register its own.

tmcw’s picture

It seems that from your description of pseudofeatures, the word 'geometry' is already established and means the same thing.

Reguardless of the pseudofeature thing, if WKT was parsed by PHP it'd be much better, for many reasons, including the one
you might want to support a different storage format (say GEO or lat/lon or whatever else).

OpenLayers 1.x tried to parse WKT, but this was removed in the 2.x branch. The reasoning is that we should only have a WKT parser if it is truly complete and stable, because otherwise it is adding a lot of code to the OpenLayers module that duplicates the efforts of the OpenLayers javascript library. However, the WFS project, by design, requires WKT parsing, so has something of a port of the OpenLayers parser that I've been writing. I'm still not convinced that this should be put into the OpenLayers project - I think that standardizing fomats is a much better move than supporting more formats - this is already a sprawling module.

strk’s picture

"It seems that from your description of pseudofeatures, the word 'geometry' is already established and means the same thing."

Not really, 'feature' is a 'geometry' with associated attributes.
The concept of 'geometry' includes 'collection' of geometries.

Here I'm adding the 'pseudo' part because the current addFeatures method
of drupal openlayer module takes each feature and create multiple features
from it, one for each simple geometry composing a collection geometry, so
from a single feature whose geometry component is a collection of 100 simple
geometries the module creates 100 features. I hope it's clearer now.

"I think that standardizing fomats is a much better move than supporting more formats"

Not sure about this. You may be getting the features from an external source.
Anyway I'm still too new in the drupal/openlayers concept to have an informed opinion on the matter.

tmcw’s picture

Interesting, I'll think more about naming. You have a good point.

As far as supporting more formats: it's easy for other modules to implement, say, GML parsing and output WKT for the OpenLayers module. However, the current direction of the OpenLayers project is to reduce bugs until we have serious stability, and then go from there. Very little of my or zzolo's current work regards any new features, because the addition of untested code means the addition of bugs, and the addition of code that few people will utilize means more stale code in the future. So, currently the responsibility for supporting things outside the scope of this module lies on other modules and we will pour effort in 100% to make the layer type plugin system and behavior system complete enough to make that possible.

strk’s picture

Ok, so are you suggesting I try to implement all my needs as a separate module and only come back here when that's impossible or too hard ?

tmcw’s picture

Oh - not at all. I'm just referring to things that are 'outside the scope of the OpenLayers module,' so it really depends on what you're trying to do. To clarify (and we really should nail this down at some point), right now, the module aims to support Views, CCK, and Lat/Lon input, and support all of the functionality provided by OpenLayers, the Javascript library. So, clustering falls entirely within that scope - it's OpenLayers functionality and is generally useful. Bits of Javascript that are outside of OpenLayers, like MapBox, and bits of data-functionality which are outside this module - like OpenLayers KML, Geo Taxonomy, and Spatial - are implemented by other modules unless it's a very common use (which is why the CloudMade layer is supported, even though it's not part of OpenLayers JS proper)

zzolo’s picture

Concerning patch:

* It seems like this is mixed up with one of your other patches. This makes it hard to read what is actually trying to be accomplished.
* I dont see this file: openlayers_behavior_cluster.inc or openlayers_behavior_cluster.js

Concerning the talk of features. Whats happens if the OpenLayers library has a feature that is a GEOMETRYCOLLECTION() and a popup gets attached to it? I would assume it creates a single popup for the whole collection. This seems like a very unintuitive interface. People see a point and want to click.

On the other hand, how does clustering get handled? Will native OpenLayers clustering be able to cluster geometries from different collections? If so, then there shouldn't be any problem.

I think tan important question with clustering and popups is how to handle what is in the display. I personally would like to see a callback method with a provided default.

Concerning WKT storage and parsing: This module should definitely not be implementing a WKT PHP parser. There is no need. This module is all about presentation except for that annoying CCK field, which is just more of a convenience. This module is set up to accept any data that OpenLayers itself excepts and easily can pull data via Views. So, if you need one feature per node, then that is more on the responsibility of data query side.

We did not really implement a WKT parser in 1.x. The main goal in 1.x was to actually utilize CCK's multiple storage mechanism for each geometry in a field, instead of just storing the GEOMETRYCOLLECTION in 2.x. But in reality, both these methods have the same basic result in the majority of use cases: any node with multiple geometries will produce multiple features with the same popup. 2.x ends up being more efficient, while the 1.x method offers more flexibility with the data. But again, this module should not focus on data; someone should use Geo for that.

Sorry, if I am not making sense or just wrong, its late.

strk’s picture

About the patch, yeah I'm having problems producing nice patches as I can't really stop with a single patch and move on at the moment. I shall freeze a working tree just for that but it's too late right now.

About the COLLECTION in OL, you click in a single point, you get a single popup. What's counter-intuitive about it ?

Please let's keep WKT parsing discussion elsewhere as it's more about redesign that adding cluster behaviour.

About hooking with popup behaviour, the current behaviour hook is really only using logic from OL itself (the .cluster property
is added by the core Cluster strategy) _and_ logic from the core Drupal OL module (which added, via my patch, the concept
of a drupalFID [drupal-specific feature id]). So it's not really a cross-behaviour interaction.

If we get to an agreement I'll work on getting a cleaner patch.

zzolo’s picture

So, I encourage you to re-create the patch and just keep it simple. I would suggest just a patch that adds the clustering behavior.

Please see this ticket for context-based styling:
#710908: Support dynamic styles

I still think there is a big unknown on how to handle the contents of a cluster popup.

strk’s picture

StatusFileSize
new4.08 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ClusterClean_0.patch.
[ View ]

Hopefully the attached patch is clean enough. Only adds Cluster behaviour (if I haven't messed with file upload).
Please let me know.

In two other patches I have the code adding a "DrupalFID" to the OL features (to tell association of a psefudofeature
to the actual API feature/node), and the tooltip/popup changes which require DrupalFID.

strk’s picture

I've attached the drupalFID patch to http://drupal.org/node/820650

zzolo’s picture

Title:[PATCH] Add cluster behaviour» Add cluster behaviour

Cleaned up and committed:
http://drupal.org/cvs?commit=376726

We still need to figure out how to do the popup support.

I think the default behavior should be to take the names of the features in a cluster and list them. We could utilize Drupal JS theming for this. I am not sure how worth it is to create a callback method for this, though; i think we shoudl eventually, but someone can make a new feature if needed.

strk’s picture

Commit tested, thank you.

For the popup I think we should really first agree on the pseudofeature/feature thingy.

My case here is that a single "project" node has an associated set of points (say, for example, wells in Burundi).
That's a _single_ project, composed by multiple points which are very close each other (say, for example, 10 points/wells).

Currently, the drupal openlayers module (in addFeatures method), parses the WKT and comes out with 10 geometries
(the 10 points/wells) then adds the _same_ attribute (the attribute of the actual node/project) to each of them.

This is obviously suboptimal when you have lots of points (or other kind of geometries) for the same node/project.

An clean approach might be passing the geometry as-is (complex) but I haven't tested the effects of this on OL
nor Clustering. Another approach could be only storing a drupalFID with each pseudofeature (each of the 10 points)
and use an external table to lookup attributes.

Matching by name, as you suggest, is error prone, unless it is guaranteed that no two nodes can have the same
name (what's the name, btw? the node title?).

strk’s picture

I'm working on a clean patch for making popup behaviour aware of clustering.
My precedent patch doesn't apply anymore.
The new patch will only be for popup, leaving tooltip behind for now (as suggested by zzolo).

The patch will make information retrival extract informations from actual features within each cluster
removing the duplicates (multiple pseudofeatures of the same feature in a cluster) by using
the drupalFID available in the branch (see http://drupal.org/node/820650)

strk’s picture

StatusFileSize
new1.62 KB
PASSED: [[SimpleTest]]: [MySQL] 221 pass(es).
[ View ]

Here's the patch for popups.
Verified to work fine with clustering on or off.

strk’s picture

For the proper clustering behaviour, I think we should allow associating the cluster for multiple layers too, rather than just one.
The popup/tooltip configuration form seems good as a target (except maybe it'd be nice to have better names for view-driven layers [ie: title rather than name ?])

strk’s picture

StatusFileSize
new1.77 KB
PASSED: [[SimpleTest]]: [MySQL] 221 pass(es).
[ View ]

This other patch adds an outer with class 'openlayers-popup-feature' to the HTML content, so in the clustering case
there's a cleaner separation between features for easier theming.

NOTE: it's been suggested by tmcw that support for cluster isn't built into the default popup handler but rather implemented
in an override of it installed by the cluster behaviour itself. I haven't had time to evaluate or implement that mechanism.

strk’s picture

StatusFileSize
new1.95 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ClusterPopupOverride.patch.
[ View ]

So, this new patch implements "cluster knowledge in popups" in the Cluster behaviour itself.
What I'm trying to obtain is a way to re-use existing code, so the patch defines Drupal.theme.openlayersPopup
calling Drupal.theme.prototype.openlayersPopup for the per-feature information extraction.

This still doesn't allow for further per-feature info extraction override by user, but adding that in
the Drupal.theme.prototype.openlayersPopup function (the one defined in popup behaviour) would
magically get it exposed when also adding clustering.

zzolo’s picture

Thoughts based an only looking at patch (not testing):

  • Please do not put discussion points in the patch itself; that is what issues queues are for.
  • Please follow coding standards. lowerCamelCase, spaces, full use of {} for control structures, etc.

Thinking about this some more, I think the overriding is a bad way to go (as I had suggested before). The logic we (@strk) are attempting to create is specifically for display of data and specifically for an OL feature. Also, the Drupal JS theming mechanism is fairly limited when compared to the PHP side. I'll try testing this soon.

zzolo’s picture

Status:Active» Needs review
tmcw’s picture

Status:Needs review» Fixed

Tested, cleaned up, committed: http://drupal.org/cvs?commit=388566

zzolo: The theming done here can't be done in PHP - it's done on the fly since innumerable clusters can be generated by maps of points and various zoom levels. I'm not seeing any mention of how overriding is bad earlier in the thread, and this implementation isn't overriding per-say as much as extending the existing behavior. Improvements to the openlayersPopup theme function will be brought into this function naturally, since it calls its parent.

zzolo’s picture

When I said "as I had suggested before", I meant that I had suggested the opposite before; bad wording on my part.

I think logically, this code does belong in the original function as it is addressing designations of the OL library (feature.cluster) and not something that the module provides. Also, if someone override the Popup JS theme and do not realize that they should go from the cluster version, then they will screw themselves in being able to use the cluster behavior correctly (though this could try to be addressed by documentation).

strk’s picture

Status:Fixed» Needs review

Exactly, the problem is that people overriding theming as specified in the documentation would break
the Cluster override (would override the override).

Ideally, the function which the Cluster override calls would be the one extended by user.
One option here is to have more functions:

1: find an array of features (actual, not pseudo) coming from the query
2: output the array of feature (theme function, overridable, default to invoke 3)
3: output a single feature (theme function, overridable)

A similar approach might also work for non-puntual queries (say a WMS query by bounding box or radius).
In other words: the behaviours implements the query, which results in an array of features; theming takes care of
writing info about the results.

Simplest theming would only extend function 3.
Theming requiring header/footer or the like may extend 2
Other behaviours (like Cluster) may extend 1 w/out touching 2 and 3

strk’s picture

StatusFileSize
new4.19 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch popup_callbacks.diff.
[ View ]

I attach a proof of concept (working patch, applicable to branch) exploding the current single callback "openlayersPopup" into more:

- openlayersPopupCallback : main entry point, registered to core OL Select control
- openlayersPopupHeader : called to fill header of content
- openlayersPopupFooter : called to fill footer of content
- openlayersPopup : called for each feature

The patch also includes modifications in the Cluster behaviour so it overrides 'openlayersPopupCallback' only, still calling
any user-thematized Header/Footer and feature function.
With this patch any existing overridden-by-user 'openlayersPopup' would still work when Cluster behaviour is enabled.

The header/footer are a pure addition excercise...

strk’s picture

(comment for a different issue removed, sorry for the noise)

strk’s picture

StatusFileSize
new2.8 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch multilayer_cluster.patch.
[ View ]

The attached patch adds cluster support for multiple layers at once. Also adds support for all vector layers, not just the ones provided by views.

strk’s picture

FYI: I'm working on fixing a bug with the cluster behavior being incapable of attaching the behavior when
layers are not yet loaded in the map at time of behavior activation. Next patch will be against the code resulting
from applying previous patch so I'd appreciate if that one is committed before that.
Note that the layer-specific behaviour bug is likely also in the popup behavior, which I'll address after the cluster one.

strk’s picture

Ok, comment #28 was wrong. The bug was not in cluster/popup behaviour but in KML layers themselves, failing to add a drupalFID.
A patch for KML layers is here: http://drupal.org/node/853966

The patch for cluster behaviour in #27 is still good.

strk’s picture

StatusFileSize
new1.34 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch cluster_popup_missing_drupalFID.patch.
[ View ]

This other patch makes the popup override in cluster do the right thing when 'drupalFID' is unavailable.
This happens for any vector layer which doesn't come from views (KML, for example).
So, when 'drupalFID' is not present we'll assume every "pseudofeature" is a "real feature".

strk’s picture

Status:Needs review» Fixed

Committed patches in comment #27 and comment #30
http://drupal.org/cvs?commit=395872

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

ckreutz’s picture

Title:Add cluster behaviour» Numbers in clusters

This feature is awesome. Thanks a lot for that! One suggestion: It would be great to have numbers in clusters like here: http://chicago.everyblock.com/. Is that difficult to implement?

zzolo’s picture

Title:Numbers in clusters» Add cluster behaviour

Hi @ckreutz, thanks for this, but you should create a new issue for this feature request.

ckreutz’s picture

petednz’s picture

Component:Behaviors» OL API

Nice work and thanks for these features.

Hoping I can get a bit of clarification. you said

Next steps would include:
- allow styling to use clustering information (tipical use: icon size dependent on number of elements in a cluster)

Just wondering if that came to pass? I also saw a ref to using OL Plus from DevSeed git but not clear if that is required or sufficient. Any one actually getting clustering size to be relative to number of items in the cluster?

FiNeX’s picture

@petednz: it looks that those type of clustering is not still available (tested on OL 7.x-2.0-beta1)

masher’s picture

Just want to clarify, have the above patches been included in the 6.x-2.0-beta1 release?

If not, which one to use.

I'm unable to get pop-ups/tooltips with clustering enable, just returns 'undefined' instead of Title/Description.