Hand cursor, mousover and mouseout events for canvas paths

[snkashis]

Provides the proper cursor if options.clickable = true for canvas paths.

Also, fixed one of the canvas test pages that was broken, since it was my testbed.

[mourner]

I hesitated to add this because of performance concerns, did you run any benchmarks? How does it cope with the debug/vector/vector-canvas.html example with 50k points? What does the profiler say? And also change polyline to polygon and check that too.

Also, I think if we add mouseover event for paths, we need to add mouseout too.

[snkashis]

Yeah, I was betting that was the reason this isn't in yet. I'm no expert on canvas or JS benchmarking, but figured I would give it a try because I feel it will help usability in our app. It definitely it takes up some resources from what the profiler says,but entirely usable on my MBA(until you try to zoom).Then things get a bit choppy.

[snkashis]

So on that zoom point, this helped at the top of _onMouseMove

if (this._map._animatingZoom) { return; }

[snkashis]

mouseout in now too.

[mourner]

Thanks! I think I'll merge it in a separate branch, make some performance experiments and see if we can optimize it (e.g. using some requestAnimFrame or timer tricks).

[danzel]

We could try doing something like client-side utfgrid if performance isn't great ;)
Or storing vectors in something like a quadtree etc etc.

[mourner]

But on the other hand we wouldn't want to write lots of code to support a relatively minor feature. :) So we need to compromise.

[danzel]

Yeah of course :)

[mourner]

Started playing with the pull. It seems that currently the bottleneck is not running point-in-poly algorithm on all features, but the redrawing mechanism itself. See #1466.

[mourner]

Merged the pull and cleaned up a bit.

[jfgirard]

On layer remove, the mousemove event is not cleared. It creates null errors if I remove a clickable path because it still receive mousemove events while having a null map.

onRemove: function (map) {
    map
        .off('viewreset', this.projectLatlngs, this)
        .off('moveend', this._updatePath, this);
    if (this.options.clickable) {
        this._map.off('click', this._onClick, this);
                    **** the following line is missing ****
        this._map.off('mousemove', this._onMouseMove, this);
    }
    this._requestUpdate();
    this._map = null;
},

[snkashis]

@jfgirard can you provide a fiddle for that?I understand what you are saying and agree, but can't reproduce it. Wondering if it got fixed in another place somehow?

[jfgirard]

@snkashis right, there are no more error because of https://github.com/Leaflet/Leaflet/blob/master/src/layer/vector/canvas/Path.Canvas.js#L139
A null check for map as been added: if (!this._map || this._map._animatingZoom) { return; }

But the removed layer creates a memory leak by being a listener on mousemove event.

[snkashis]

Thanks for the heads up! PR submitted.

[jfgirard]

Cool. You're welcome.