fix: Account for marker size when picking the hovered WebGL point

draftlogs/8170_fix.md

@@ -0,0 +1 @@
+- Account for marker size when picking the hovered point in `scattergl`, `scatterpolargl` and `splom` (matching `scatter`) [[#PRNUMBER](https://github.com/plotly/plotly.js/pull/PRNUMBER)]

src/traces/scatter/hover.js

@@ -7,6 +7,24 @@ var getTraceColor = require('./get_trace_color');
 var Color = require('../../components/color');
 var fillText = Lib.fillText;
 
+// Hover distance along one axis, for the 'x' and 'y' hovermodes.
+// `d` is the pixel offset from the marker center and `mrc` the marker radius.
+// Inside the marker, the distance stays below 1 and grows with the offset relative to the radius.
+const getAxisDistance = (d, mrc) => {
+    const rad = Math.max(3, mrc || 0);
+    const kink = 1 - 1 / rad;
+    const dRaw = Math.abs(d);
+    return dRaw < rad ? (kink * dRaw) / rad : dRaw - rad + kink;
+};
+
+// Hover distance to a marker for the 'closest' hovermode.
+// The distance is measured to the marker edge, so a point inside a marker always shows up.
+// Inside the marker the distance is floored at `1 - minRad / rad`, so a smaller marker wins.
+const getPointDistance = (dx, dy, mrc, minRad) => {
+    const rad = Math.max(minRad, mrc || 0);
+    return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - minRad / rad);
+};
+
 module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
     var cd = pointData.cd;
     var trace = cd[0].trace;
@@ -39,10 +57,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
                 ) ? 0 : Infinity;
             }
 
-            var rad = Math.max(3, di.mrc || 0);
-            var kink = 1 - 1 / rad;
-            var dxRaw = Math.abs(xa.c2p(di.x) - xpx);
-            return (dxRaw < rad) ? (kink * dxRaw / rad) : (dxRaw - rad + kink);
+            return getAxisDistance(xa.c2p(di.x) - xpx, di.mrc);
         };
         var dy = function(di) {
             if(yPeriod) {
@@ -55,22 +70,11 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
                 ) ? 0 : Infinity;
             }
 
-            var rad = Math.max(3, di.mrc || 0);
-            var kink = 1 - 1 / rad;
-            var dyRaw = Math.abs(ya.c2p(di.y) - ypx);
-            return (dyRaw < rad) ? (kink * dyRaw / rad) : (dyRaw - rad + kink);
+            return getAxisDistance(ya.c2p(di.y) - ypx, di.mrc);
         };
 
         // scatter points: d.mrc is the calculated marker radius
-        // adjust the distance so if you're inside the marker it
-        // always will show up regardless of point size, but
-        // prioritize smaller points
-        var dxy = function(di) {
-            var rad = Math.max(minRad, di.mrc || 0);
-            var dx = xa.c2p(di.x) - xpx;
-            var dy = ya.c2p(di.y) - ypx;
-            return Math.max(Math.sqrt(dx * dx + dy * dy) - rad, 1 - minRad / rad);
-        };
+        const dxy = (di) => getPointDistance(xa.c2p(di.x) - xpx, ya.c2p(di.y) - ypx, di.mrc, minRad);
         var distfn = Fx.getDistanceFunction(hovermode, dx, dy, dxy);
 
         Fx.getClosest(cd, distfn, pointData);
@@ -261,3 +265,6 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
         }
     }
 };
+
+module.exports.getAxisDistance = getAxisDistance;
+module.exports.getPointDistance = getPointDistance;

src/traces/scattergl/hover.js

@@ -4,6 +4,27 @@ var Registry = require('../../registry');
 var Lib = require('../../lib');
 var getTraceColor = require('../scatter/get_trace_color');
 const Drawing = require('../../components/drawing');
+const subTypes = require('../scatter/subtypes');
+const { getAxisDistance, getPointDistance } = require('../scatter/hover');
+
+// Marker radius of each point in px, from the sizes that `convert` computes for the WebGL markers.
+// The marker options hold diameters, so halve them: `mrc` in the SVG `scatter` trace is a radius.
+// `max` loops over every point, so call it only when needed.
+const getMarkerRadii = (markerOptions) => {
+    if (!markerOptions) return { radius: () => 0, max: () => 0 };
+
+    const { sizes, size } = markerOptions;
+    if (!sizes) return { radius: () => size / 2, max: () => size / 2 };
+
+    return {
+        radius: (i) => sizes[i] / 2,
+        max: () => {
+            let max = 0;
+            for (let i = 0; i < sizes.length; i++) max = Math.max(max, sizes[i]);
+            return max / 2;
+        }
+    };
+};
 
 function hoverPoints(pointData, xval, yval, hovermode) {
     var cd = pointData.cd;
@@ -18,12 +39,18 @@ function hoverPoints(pointData, xval, yval, hovermode) {
     var maxDistance = pointData.distance;
     var ids;
 
+    // Score points as the SVG `scatter` trace does, so that marker size counts in the same way
+    const minRad = subTypes.hasMarkers(trace) ? 3 : 0.5;
+    const markerRadii = getMarkerRadii(stash._scene && stash._scene.markerOptions[stash.index]);
+
     // FIXME: make sure this is a proper way to calc search radius
     if(stash.tree) {
-        var xl = xa.p2c(xpx - maxDistance);
-        var xr = xa.p2c(xpx + maxDistance);
-        var yl = ya.p2c(ypx - maxDistance);
-        var yr = ya.p2c(ypx + maxDistance);
+        // A marker can be in range of the pointer when its center is not, so widen the box by the largest radius
+        const searchDistance = maxDistance + markerRadii.max();
+        var xl = xa.p2c(xpx - searchDistance);
+        var xr = xa.p2c(xpx + searchDistance);
+        var yl = ya.p2c(ypx - searchDistance);
+        var yr = ya.p2c(ypx + searchDistance);
 
         if(hovermode === 'x') {
             ids = stash.tree.range(
@@ -53,12 +80,14 @@ function hoverPoints(pointData, xval, yval, hovermode) {
             k = ids[i];
             ptx = x[k];
 
-            dx = Math.abs(xa.c2p(ptx) - xpx);
+            const rad = markerRadii.radius(k);
+            let dxPoint = xa.c2p(ptx) - xpx;
+            dx = getAxisDistance(dxPoint, rad);
             if(xPeriod) {
                 var x0 = xa.c2p(trace._xStarts[k]);
                 var x1 = xa.c2p(trace._xEnds[k]);
 
-                dx = (
+                dx = dxPoint = (
                     xpx >= Math.min(x0, x1) &&
                     xpx <= Math.max(x0, x1)
                 ) ? 0 : Infinity;
@@ -80,7 +109,7 @@ function hoverPoints(pointData, xval, yval, hovermode) {
                     ) ? 0 : Infinity;
                 }
 
-                dxy = Math.sqrt(dx * dx + dy * dy);
+                dxy = getPointDistance(dxPoint, dy, rad, minRad);
                 closestId = ids[i];
             }
         }
@@ -92,7 +121,7 @@ function hoverPoints(pointData, xval, yval, hovermode) {
             dx = xa.c2p(ptx) - xpx;
             dy = ya.c2p(pty) - ypx;
 
-            dist = Math.sqrt(dx * dx + dy * dy);
+            dist = getPointDistance(dx, dy, markerRadii.radius(k), minRad);
             // Accept a tie so that the trace drawn last wins, as in `Fx.getClosest`
             if(dist <= minDist) {
                 minDist = dxy = dist;
@@ -224,5 +253,6 @@ function calcHover(pointData, x, y, trace) {
 
 module.exports = {
     hoverPoints: hoverPoints,
-    calcHover: calcHover
+    calcHover: calcHover,
+    getMarkerRadii
 };

src/traces/splom/hover.js

@@ -2,6 +2,8 @@
 
 var helpers = require('./helpers');
 var calcHover = require('../scattergl/hover').calcHover;
+const { getMarkerRadii } = require('../scattergl/hover');
+const { getPointDistance } = require('../scatter/hover');
 var getFromId = require('../../plots/cartesian/axes').getFromId;
 var extendFlat = require('../../lib/extend').extendFlat;
 
@@ -66,6 +68,8 @@ function _hoverPoints(pointData, xval, yval, axisHoversubplots) {
 
     var id, dxy;
     var minDist = maxDistance;
+    // splom traces always draw markers, so score them like `scatter` markers
+    const markerRadii = getMarkerRadii(scene.matrixOptions);
 
     for(var i = 0; i < x.length; i++) {
         if(axisHoversubplots && i !== pointData.index) continue;
@@ -74,7 +78,7 @@ function _hoverPoints(pointData, xval, yval, axisHoversubplots) {
         var pty = y[i];
         var dx = xa.c2p(ptx) - xpx;
         var dy = ya.c2p(pty) - ypx;
-        var dist = Math.sqrt(dx * dx + dy * dy);
+        const dist = getPointDistance(dx, dy, markerRadii.radius(i), 3);
 
         // Accept a tie so that the point drawn last wins, as in `Fx.getClosest`
         if(axisHoversubplots || dist <= minDist) {

test/jasmine/tests/hover_test.js

@@ -5652,6 +5652,61 @@ describe('hover on points at the same position', () => {
             expect(gd._hoverdata.length).toEqual(1);
             expect(gd._hoverdata[0].pointNumber).toEqual(2);
         });
+
+        it(`picks the smaller marker for ${type} when markers of different sizes overlap`, async () => {
+            const gd = createGraphDiv();
+            const small = Lib.extendFlat(makeTrace([0, 1, 2], [1, 0, 1]), { marker: { size: 6 } });
+            const big = Lib.extendFlat(makeTrace([0, 1, 2], [2, 0, 2]), { marker: { size: 20 } });
+            await Plotly.newPlot(gd, {
+                data: [small, big],
+                layout: { width: 400, height: 400, hovermode: 'closest' }
+            });
+
+            Fx.hover(gd, { xval: 1, yval: 0 }, subplot);
+
+            expect(gd._hoverdata.length).toEqual(1);
+            expect(gd._hoverdata[0].curveNumber).toEqual(0);
+            expect(gd._hoverdata[0].pointNumber).toEqual(1);
+        });
+
+        it(`hovers a ${type} marker whose edge, not center, is within hoverdistance`, async () => {
+            const gd = createGraphDiv();
+            // One point, so that the pointer is inside no other marker
+            const trace = Lib.extendFlat(makeTrace([1], [0]), { marker: { size: 80 } });
+            await Plotly.newPlot(gd, {
+                data: [trace],
+                layout: { width: 400, height: 400, hovermode: 'closest', hoverdistance: 20 }
+            });
+
+            // 30px from the marker center, inside its 40px radius
+            const xa = gd._fullLayout.xaxis;
+            Fx.hover(gd, { xval: xa.p2c(xa.c2p(1) + 30), yval: 0 }, subplot);
+
+            expect(gd._hoverdata.length).toEqual(1);
+            expect(gd._hoverdata[0].pointNumber).toEqual(0);
+        });
+    });
+
+    it('hovers a large scattergl marker whose center is outside the spatial index search box', async () => {
+        // Traces of 1e5 points or more are searched through a spatial index
+        const n = 1e5;
+        const x = new Array(n).fill(5);
+        const y = new Array(n).fill(0);
+        const size = new Array(n).fill(4);
+        x[0] = 1;
+        size[0] = 80;
+
+        const gd = createGraphDiv();
+        await Plotly.newPlot(gd, {
+            data: [{ type: 'scattergl', mode: 'markers', x, y, marker: { size } }],
+            layout: { width: 400, height: 400, hovermode: 'closest', hoverdistance: 20, xaxis: { range: [0, 6] } }
+        });
+
+        const xa = gd._fullLayout.xaxis;
+        Fx.hover(gd, { xval: xa.p2c(xa.c2p(1) + 30), yval: 0 });
+
+        expect(gd._hoverdata.length).toEqual(1);
+        expect(gd._hoverdata[0].pointNumber).toEqual(0);
     });
 });
 

test/jasmine/tests/scatterpolargl_test.js

@@ -68,17 +68,19 @@ describe('Test scatterpolargl hover:', function() {
         });
     }
 
+    // The default position is inside two 'Trial 3' markers (points 2 and 26) of the same size.
+    // As in `scatterpolar`, the tie goes to the point drawn last.
     [{
         desc: 'base',
-        nums: 'r: 3.886013\nθ: 125.2822°',
+        nums: 'r: 4.022892\nθ: 128.342°',
         name: 'Trial 3'
     }, {
         desc: 'with hovertemplate',
         patch: function(fig) {
             fig.data[2].hovertemplate = 'template %{r} %{theta}';
             return fig;
         },
-        nums: 'template 3.886013 125.2822°',
+        nums: 'template 4.022892 128.342°',
         name: 'Trial 3'
     }, {
         desc: '(no labels - out of sector)',
@@ -95,7 +97,7 @@ describe('Test scatterpolargl hover:', function() {
             fig.layout.polar.angularaxis.thetaunit = 'radians';
             return fig;
         },
-        nums: 'r: 3.886013\nθ: 2.186586',
+        nums: 'r: 4.022892\nθ: 2.239991',
         name: 'Trial 3'
     }, {
         desc: 'on log radial axis',
@@ -124,15 +126,15 @@ describe('Test scatterpolargl hover:', function() {
             fig.data.forEach(function(t) { t.text = 'a'; });
             return fig;
         },
-        nums: 'r: 3.886013\nθ: 125.2822°\na',
+        nums: 'r: 4.022892\nθ: 128.342°\na',
         name: 'Trial 3'
     }, {
         desc: 'with custom text array',
         patch: function(fig) {
             fig.data.forEach(function(t) { t.text = t.r.map(String); });
             return fig;
         },
-        nums: 'r: 3.886013\nθ: 125.2822°\n3.88601339194',
+        nums: 'r: 4.022892\nθ: 128.342°\n4.02289202968',
         name: 'Trial 3'
     }]
     .forEach(function(specs) {