Using SVGElement.isPointInFill instead of custom polygons for hover tests in scatter plots.

lumip · lumip · commit 5ba718fa4e96 · 2024-01-23T20:28:26.000+02:00
However, SVGElement does not allow for an easy way to determine the positioning of
the hover label, so the polygons are still in use for that.
diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js
@@ -119,64 +119,115 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
         }
     }
 
-    // even if hoveron is 'fills', only use it if we have polygons too
-    if(hoveron.indexOf('fills') !== -1 && trace._polygons && trace._polygons.length > 0) {
-        var polygons = trace._polygons;
+    function isHoverPointInFillElement(el) {
+        // Uses SVGElement.isPointInFill to accurately determine wether
+        // the hover point / cursor is contained in the fill, taking
+        // curved or jagged edges into account, which the Polygon-based
+        // approach does not.
+        if(!el) {
+            return false;
+        }
+        var svgElement = el.node();
+        try {
+            var domPoint = new DOMPoint(pt[0], pt[1]);
+            return svgElement.isPointInFill(domPoint);
+        } catch(TypeError) {
+            var svgPoint = svgElement.ownerSVGElement.createSVGPoint();
+            svgPoint.x = pt[0];
+            svgPoint.y = pt[1];
+            return svgElement.isPointInFill(svgPoint);
+        }
+    }
+
+    function getHoverLabelPosition(polygons) {
+        // Uses Polygon s to determine the left- and right-most x-coordinates
+        // of the subshape of the fill that contains the hover point / cursor.
+        // Doing this with the SVGElement directly is quite tricky, so this falls
+        // back to the existing relatively simple code, accepting some small inaccuracies
+        // of label positioning for curved/jagged edges.
+        var i;
         var polygonsIn = [];
-        var inside = false;
         var xmin = Infinity;
         var xmax = -Infinity;
         var ymin = Infinity;
         var ymax = -Infinity;
-
-        var i, j, polygon, pts, xCross, x0, x1, y0, y1;
+        var yminAll = Infinity;
+        var ymaxAll = -Infinity;
+        var yPos;
 
         for(i = 0; i < polygons.length; i++) {
-            polygon = polygons[i];
-            // TODO: this is not going to work right for curved edges, it will
-            // act as though they're straight. That's probably going to need
-            // the elements themselves to capture the events. Worth it?
+            var polygon = polygons[i];
+            // This is not going to work right for curved or jagged edges, it will
+            // act as though they're straight.
+            yminAll = Math.min(yminAll, polygon.ymin);
+            ymaxAll = Math.max(ymaxAll, polygon.ymax);
             if(polygon.contains(pt)) {
-                inside = !inside;
-                // TODO: need better than just the overall bounding box
                 polygonsIn.push(polygon);
                 ymin = Math.min(ymin, polygon.ymin);
                 ymax = Math.max(ymax, polygon.ymax);
             }
         }
 
-        if(inside) {
-            // constrain ymin/max to the visible plot, so the label goes
-            // at the middle of the piece you can see
-            ymin = Math.max(ymin, 0);
-            ymax = Math.min(ymax, ya._length);
-
-            // find the overall left-most and right-most points of the
-            // polygon(s) we're inside at their combined vertical midpoint.
-            // This is where we will draw the hover label.
-            // Note that this might not be the vertical midpoint of the
-            // whole trace, if it's disjoint.
-            var yAvg = (ymin + ymax) / 2;
-            for(i = 0; i < polygonsIn.length; i++) {
-                pts = polygonsIn[i].pts;
-                for(j = 1; j < pts.length; j++) {
-                    y0 = pts[j - 1][1];
-                    y1 = pts[j][1];
-                    if((y0 > yAvg) !== (y1 >= yAvg)) {
-                        x0 = pts[j - 1][0];
-                        x1 = pts[j][0];
-                        if(y1 - y0) {
-                            xCross = x0 + (x1 - x0) * (yAvg - y0) / (y1 - y0);
-                            xmin = Math.min(xmin, xCross);
-                            xmax = Math.max(xmax, xCross);
-                        }
+        // The above found no polygon that contains the cursor, but we know that
+        // the cursor must be inside the fill as determined by the SVGElement
+        // (so we are probably close to a curved/jagged edge...). In this case
+        // as a crude approximation, simply consider all polygons for determination
+        // of the hover label position.
+        // TODO: This might cause some jumpiness of the label close to edges...
+        if(polygonsIn.length === 0) {
+            polygonsIn = polygons;
+            ymin = yminAll;
+            ymax = ymaxAll;
+        }
+
+        // constrain ymin/max to the visible plot, so the label goes
+        // at the middle of the piece you can see
+        ymin = Math.max(ymin, 0);
+        ymax = Math.min(ymax, ya._length);
+
+        yPos = (ymin + ymax) / 2;
+
+        // find the overall left-most and right-most points of the
+        // polygon(s) we're inside at their combined vertical midpoint.
+        // This is where we will draw the hover label.
+        // Note that this might not be the vertical midpoint of the
+        // whole trace, if it's disjoint.
+        var j, pts, xAtYPos, x0, x1, y0, y1;
+        for(i = 0; i < polygonsIn.length; i++) {
+            pts = polygonsIn[i].pts;
+            for(j = 1; j < pts.length; j++) {
+                y0 = pts[j - 1][1];
+                y1 = pts[j][1];
+                if((y0 > yPos) !== (y1 >= yPos)) {
+                    x0 = pts[j - 1][0];
+                    x1 = pts[j][0];
+                    if(y1 - y0) {
+                        xAtYPos = x0 + (x1 - x0) * (yPos - y0) / (y1 - y0);
+                        xmin = Math.min(xmin, xAtYPos);
+                        xmax = Math.max(xmax, xAtYPos);
                     }
                 }
             }
+        }
+
+        // constrain xmin/max to the visible plot now too
+        xmin = Math.max(xmin, 0);
+        xmax = Math.min(xmax, xa._length);
+
+        return {
+            x0: xmin,
+            x1: xmax,
+            y0: yPos,
+            y1: yPos,
+        };
+    }
 
-            // constrain xmin/max to the visible plot now too
-            xmin = Math.max(xmin, 0);
-            xmax = Math.min(xmax, xa._length);
+    // even if hoveron is 'fills', only use it if we have a fill element too
+    if(hoveron.indexOf('fills') !== -1 && trace._fillElement) {
+        var inside = isHoverPointInFillElement(trace._fillElement) && !isHoverPointInFillElement(trace._fillExclusionElement);
+
+        if(inside) {
+            var hoverLabelCoords = getHoverLabelPosition(trace._polygons);
 
             // get only fill or line color for the hover color
             var color = Color.defaultLine;
@@ -189,10 +240,10 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
                 // never let a 2D override 1D type as closest point
                 // also: no spikeDistance, it's not allowed for fills
                 distance: pointData.maxHoverDistance,
-                x0: xmin,
-                x1: xmax,
-                y0: yAvg,
-                y1: yAvg,
+                x0: hoverLabelCoords.x0,
+                x1: hoverLabelCoords.x1,
+                y0: hoverLabelCoords.y0,
+                y1: hoverLabelCoords.y1,
                 color: color,
                 hovertemplate: false
             });
diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js
@@ -159,12 +159,14 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
     var prevPolygons = [];
     var prevtrace = trace._prevtrace;
     var prevFillsegments = null;
+    var prevFillElement = null;
 
     if(prevtrace) {
         prevRevpath = prevtrace._prevRevpath || '';
         tonext = prevtrace._nextFill;
         prevPolygons = prevtrace._ownPolygons;
         prevFillsegments = prevtrace._fillsegments;
+        prevFillElement = prevtrace._fillElement;
     }
 
     var thispath;
@@ -257,6 +259,10 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                 curpoints.push.apply(curpoints, pts);
             }
         }
+
+        trace._fillElement = null;
+        trace._fillExclusionElement = prevFillElement;
+
         trace._fillsegments = fillsegments.slice(0, fillsegmentCount);
         fillsegments = trace._fillsegments;
 
@@ -398,6 +404,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                 }
             }
             trace._polygons = thisPolygons;
+            trace._fillElement = ownFillEl3;
         } else if(tonext) {
             if(trace.fill.substr(0, 6) === 'tonext' && fullpath && prevRevpath) {
                 // fill to next: full trace path, plus the previous path reversed
@@ -409,7 +416,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                     transition(tonext).attr('d', fullpath + 'Z' + prevRevpath + 'Z')
                         .call(Drawing.singleFillStyle, gd);
 
-                    // and simply emit hover polygons for each segment
+                                            // and simply emit hover polygons for each segment
                     thisPolygons = makeSelfPolygons();
 
                     // we add the polygons of the previous trace which causes hover
@@ -431,6 +438,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                     // so must not include previous trace polygons for hover detection.
                     trace._polygons = thisPolygons;
                 }
+                trace._fillElement = tonext;
             } else {
                 clearFill(tonext);
             }
diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js
@@ -1576,15 +1576,15 @@ describe('scatter hoverFills', function() {
         var traceOffset = 0;
 
         var testPoints = [ // all the following points should be in fill region of corresponding tozeroy traces 0-4
-            [[1.5, 1.24], [1.5, 1.06]], // single point has "fill" along line to zero
+            [], // single point has no "fill" when using SVG element containment tests
             [[0.1, 0.9], [0.1, 0.8], [1.5, 0.9], [1.5, 1.04], [2, 0.8], [2, 1.09], [3, 0.8]],
             [[0.1, 0.75], [0.1, 0.61], [1.01, 0.501], [1.5, 0.8], [1.5, 0.55], [2, 0.74], [2, 0.55], [3, 0.74], [3, 0.51]],
             [[0.1, 0.599], [0.1, 0.5], [0.1, 0.3], [0.99, 0.59], [1, 0.49], [1, 0.36], [1.5, 0.26], [2, 0.49], [2, 0.16], [3, 0.49], [3, 0.26]],
             [[0.1, 0.25], [0.1, 0.1], [1, 0.34], [1.5, 0.24], [2, 0.14], [3, 0.24], [3, 0.1]],
         ];
 
         var outsidePoints = [ // all these should not result in a hover detection, for any trace
-            [1, 1.1], [2, 1.14],
+            [1, 1.1], [2, 1.14], [1.5, 1.24], [1.5, 1.06]
         ];
 
         Plotly.newPlot(gd, mock).then(function() {
