From 983d00402317a6a01229e0a20e7c141669b261aa Mon Sep 17 00:00:00 2001 From: adimalkar Date: Wed, 11 Nov 2026 03:12:47 -0500 Subject: [PATCH 1/4] Account for marker size when picking the hovered WebGL point scatter scores a point by its distance to the marker edge, and inside a marker it prefers the smaller one. scattergl and splom used the distance to the marker center only, so the marker drawn on top always won when markers of different sizes overlapped, and a large marker did not hover until the pointer came near its center. scatterpolargl inherits the scattergl result. Share the scatter scoring with scattergl and splom and take each radius from the WebGL marker sizes. Widen the spatial index search box by the largest radius, so large traces find markers whose edge is in range. Fixes #8082 --- draftlogs/PRNUMBER_fix.md | 1 + src/traces/scatter/hover.js | 41 ++++++++++++++---------- src/traces/scattergl/hover.js | 48 ++++++++++++++++++++++------ src/traces/splom/hover.js | 6 +++- test/jasmine/tests/hover_test.js | 54 ++++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 27 deletions(-) create mode 100644 draftlogs/PRNUMBER_fix.md diff --git a/draftlogs/PRNUMBER_fix.md b/draftlogs/PRNUMBER_fix.md new file mode 100644 index 00000000000..4670ad1e869 --- /dev/null +++ b/draftlogs/PRNUMBER_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)] diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js index 04b0de04e17..da7ef9778b2 100644 --- a/src/traces/scatter/hover.js +++ b/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; diff --git a/src/traces/scattergl/hover.js b/src/traces/scattergl/hover.js index 624bf589b66..92e089c223b 100644 --- a/src/traces/scattergl/hover.js +++ b/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 }; diff --git a/src/traces/splom/hover.js b/src/traces/splom/hover.js index 251016b31d7..6ebbce54712 100644 --- a/src/traces/splom/hover.js +++ b/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) { diff --git a/test/jasmine/tests/hover_test.js b/test/jasmine/tests/hover_test.js index 597f4224959..cccb906af05 100644 --- a/test/jasmine/tests/hover_test.js +++ b/test/jasmine/tests/hover_test.js @@ -5652,6 +5652,60 @@ 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(); + const trace = Lib.extendFlat(makeTrace([0, 1, 2], [0, 0, 0]), { marker: { size: 80 } }); + await Plotly.newPlot(gd, { + data: [trace], + layout: { width: 400, height: 400, hovermode: 'closest', hoverdistance: 20 } + }); + + // 30px from the center of the middle marker, 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(1); + }); + }); + + 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); }); }); From a50a450cc06e1b7c5a6d563dd193c483cc5d7ad3 Mon Sep 17 00:00:00 2001 From: adimalkar Date: Wed, 11 Nov 2026 03:55:18 -0500 Subject: [PATCH 2/4] Use one point in the hoverdistance test so no neighboring marker overlaps With three size-80 markers on a 400px plot, the pointer 30px from the middle marker was also inside the next one, which correctly won the tie. --- test/jasmine/tests/hover_test.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/hover_test.js b/test/jasmine/tests/hover_test.js index cccb906af05..28fcdfdf06d 100644 --- a/test/jasmine/tests/hover_test.js +++ b/test/jasmine/tests/hover_test.js @@ -5671,18 +5671,19 @@ describe('hover on points at the same position', () => { it(`hovers a ${type} marker whose edge, not center, is within hoverdistance`, async () => { const gd = createGraphDiv(); - const trace = Lib.extendFlat(makeTrace([0, 1, 2], [0, 0, 0]), { marker: { size: 80 } }); + // 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 center of the middle marker, inside its 40px radius + // 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(1); + expect(gd._hoverdata[0].pointNumber).toEqual(0); }); }); From 36c7508e7723f58c735a22ca2c6ec5bf8d0e1a53 Mon Sep 17 00:00:00 2001 From: adimalkar Date: Wed, 11 Nov 2026 03:59:08 -0500 Subject: [PATCH 3/4] Update scatterpolargl hover expectations to match scatterpolar The default pointer position is inside two same-size markers of one trace. scatterpolar gives the tie to the point drawn last (point 26), and now scatterpolargl does too, instead of picking the nearer center (point 2). --- test/jasmine/tests/scatterpolargl_test.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/jasmine/tests/scatterpolargl_test.js b/test/jasmine/tests/scatterpolargl_test.js index f964ed4bef5..c5a2b46a7a6 100644 --- a/test/jasmine/tests/scatterpolargl_test.js +++ b/test/jasmine/tests/scatterpolargl_test.js @@ -68,9 +68,11 @@ 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', @@ -78,7 +80,7 @@ describe('Test scatterpolargl hover:', function() { 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,7 +126,7 @@ 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', @@ -132,7 +134,7 @@ describe('Test scatterpolargl hover:', function() { 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) { From 36327357cbae1f19f3c0074ed04d5f8beb120bdf Mon Sep 17 00:00:00 2001 From: adimalkar Date: Wed, 11 Nov 2026 04:15:02 -0500 Subject: [PATCH 4/4] Add draftlog for #8170 --- draftlogs/{PRNUMBER_fix.md => 8170_fix.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename draftlogs/{PRNUMBER_fix.md => 8170_fix.md} (100%) diff --git a/draftlogs/PRNUMBER_fix.md b/draftlogs/8170_fix.md similarity index 100% rename from draftlogs/PRNUMBER_fix.md rename to draftlogs/8170_fix.md