Link pointer when hovering over clickable feature in map.

[steff-o]

Implementation for #1259

Changes the mouse pointer to link pointer when hovering over a feature that can be queried for feature info.

Feature is opt in to be backwards compatible and is activated by adding "linkPointer": true to featureInfoOptions.

Make sure to turn off queryable on background maps, as queryable is default true and would quite spoil the usefulness of this feature.

The solution is based on forEachLayerAtPixel to detect features from WMS as well. As forEachLayerAtPixel requries that each layer has a unique css class, a class is added to each queryable layer if the layer does not have one and the feature is activated.

[tonnyandersson]

Only tested on WMS and GEOJSON type layers, but it seems to do the trick.

A few questions, though.

As I understand it, giving layers a class name may have a negative impact on performance since layers with class names will be rendered as separate HTML elements. With that in mind, I wonder if it's wise to set a class name on all layers like this. Perhaps class namnes should be set on non-vector type layers only? But then there's the issue with possible false positives. I didn't notice anything like that when testing, but I only tested on a map with very few layers/features. I guess it comes down to a tradeoff between performance and possible false positives. Not sure which one will have the most negative effect on the user experience at this moment.

Also, I don't think the property name linkPointer feels very intuitive since we're not dealing with links here. I suggest renaming it to pointerOnHover or something like that. Just a suggestion, though.

[steff-o]

@tonnyandersson: I have experienced false positives using only two layers, one WMS and one WFS on separate canvases but same class name.

The idea is to only set class name on layers that are configured as "queryable" to get a pointer change on all layers, regarding of type, as it also works on vector layers. Maybe it is possible to use forEachFeatureAtPixel for feature layers if that produces better performance.

I have also had concerns about performance, that's one reason the feature is opt-in. But when I have tested, my map is always drawn with with a separate div and canvas for each layer, regardless if I use a class name or not. There might be something wrong with my setup, as I also think that all layers not using separate class name should use the same canvas. At least I have seen other OL-applications using only one canvas-element. I have to look into it and see if it is OL, origo or my configuration that creates all those canvases. Not sure how much it affects performance though. But in another application we have serious performance issues when when we have a lot of WMS:es using separate canvases for each layer. The perfomance issue in that case is drawing the map, the mouse pointer works fine. Never managed to make origo merge them in to one canvas there either. In that application I traced the performance issue to have something to do with drawing the actual pixels (according to chrome dev tools), as it was slow even if all tiles where fetched from local cache. But changing layer type to WFS made it lightning fast, even with separate classes (and canvases) for each layer.

I also think the option should be renamed, I never liked it myself as it could be interpreted as that the pointer should be linked (whatever that would mean). Maybe changePointerOnHover.

To sum it up: I will do some more performance analysis.

[steff-o]

After investingating how forEachLayerAtPixel actually behaves, I have changed the code so it will not add a unique className to each queryable layer, but instead adds a o-layer-queryable class to all layers that do not already have a className. In this way queryable and non-queryable layers are not mixed in the same canvas (as forEachLayerAtPixel only checks if canvas has a pixel, not which actual layer). If consecutive layers have the same class, they will be drawn on the same canvas. For optimal performance the configuration should not intersperse queryable and non-queryable layers in the configuration.

That said, after testing actual performance there is no improvement in drawing speed between using one canvas or one canvas for each layer, but there may be some other advantages of keeping the number of canvases low. (Tested with 100 layers wich is slooow in both cases).

Also renamed the option to changePointerOnHover

[jokd]

@tonnyandersson Thoughts on this?

[tonnyandersson]

@steff-o It seems layers are automatically rendered as separate elements if you add any kind of visual trickery on one or more layers. E.g. in the default config, the opacity is changed on the mask layer. If you remove that property, you'll see that all layers are rendered on the same canvas.

I think this feels pretty solid and I guess a (possible) slight decrease in performance is preferable to false positives.

However, there are some linter errors in layer.js, mainly regarding trailing spaces and spaces after // in your comments. Please clean them up before I approve.

[steff-o]

New lint friendly comments. Seems like lint autocorrects those errors, so it passed lint on my machine but left repo changed as I had already commited before running lint the last time.