diff --git a/draftlogs/8170_fix.md b/draftlogs/8170_fix.md new file mode 100644 index 00000000000..4670ad1e869 --- /dev/null +++ b/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)] 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..28fcdfdf06d 100644 --- a/test/jasmine/tests/hover_test.js +++ b/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); }); }); 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) {