Tracer-bullet impl of fast click-to-select [1852]

Reason: to be reviewable from a perf standpoint.

Author: rmoestl

Diff for: src/lib/polygon.js

@@ -179,25 +179,29 @@ polygon.tester = function tester(ptsIn) {
  */
 polygon.multitester = function multitester(list) {
     var testers = [],
-        xmin = list[0][0][0],
+        xmin = list[0].contains ? 0 : list[0][0][0],
         xmax = xmin,
-        ymin = list[0][0][1],
+        ymin = list[0].contains ? 0 : list[0][0][1],
         ymax = ymin;
 
     for(var i = 0; i < list.length; i++) {
-        var tester = polygon.tester(list[i]);
-        tester.subtract = list[i].subtract;
-        testers.push(tester);
-        xmin = Math.min(xmin, tester.xmin);
-        xmax = Math.max(xmax, tester.xmax);
-        ymin = Math.min(ymin, tester.ymin);
-        ymax = Math.max(ymax, tester.ymax);
+        if(list[i].contains) {
+            testers.push(list[i]);
+        } else {
+            var tester = polygon.tester(list[i]);
+            tester.subtract = list[i].subtract;
+            testers.push(tester);
+            xmin = Math.min(xmin, tester.xmin);
+            xmax = Math.max(xmax, tester.xmax);
+            ymin = Math.min(ymin, tester.ymin);
+            ymax = Math.max(ymax, tester.ymax);
+        }
     }
 
-    function contains(pt, arg) {
+    function contains(pt, arg, index, expandedTraceIndex) {
         var yes = false;
         for(var i = 0; i < testers.length; i++) {
-            if(testers[i].contains(pt, arg)) {
+            if(testers[i].contains(pt, arg, index, expandedTraceIndex)) {
                 // if contained by subtract polygon - exclude the point
                 yes = testers[i].subtract === false;
             }

Diff for: src/plots/cartesian/select.js

@@ -264,13 +264,8 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
         }
 
         // draw selection
-        var paths = [];
-        for(i = 0; i < mergedPolygons.length; i++) {
-            var ppts = mergedPolygons[i];
-            paths.push(ppts.join('L') + 'L' + ppts[0]);
-        }
-        outlines
-            .attr('d', 'M' + paths.join('M') + 'Z');
+        drawSelection(mergedPolygons, outlines);
+
 
         throttle.throttle(
             throttleID,
@@ -320,6 +315,65 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
                 gd.emit('plotly_deselect', null);
             }
             else {
+
+
+
+                var hoverData = gd._hoverdata;
+                var selection = [];
+                var traceSelection;
+                var thisTracesSelection;
+                var pointSelected;
+                var subtract;
+
+                if(isHoverDataSet(hoverData)) {
+                    var clickedPtInfo = extractClickedPtInfo(hoverData, searchTraces);
+
+                    // TODO perf: call potentially costly operation (see impl comment) only when needed
+                    pointSelected = isPointSelected(clickedPtInfo.searchInfo.cd[0].trace,
+                      clickedPtInfo.pointNumber);
+
+                    if(pointSelected && isOnlyOnePointSelected(searchTraces)) {
+                        // TODO DRY see doubleClick handling above
+                        outlines.remove();
+                        for(i = 0; i < searchTraces.length; i++) {
+                            searchInfo = searchTraces[i];
+                            searchInfo._module.selectPoints(searchInfo, false);
+                        }
+
+                        updateSelectedState(gd, searchTraces);
+                        gd.emit('plotly_deselect', null);
+                    } else {
+                        subtract = evt.shiftKey && pointSelected;
+                        currentPolygon = createPtNumTester(clickedPtInfo.pointNumber,
+                          clickedPtInfo.searchInfo.cd[0].trace._expandedIndex, subtract);
+
+                        var concatenatedPolygons = dragOptions.polygons.concat([currentPolygon]);
+                        testPoly = multipolygonTester(concatenatedPolygons);
+
+                        for(i = 0; i < searchTraces.length; i++) {
+                            traceSelection = searchTraces[i]._module.selectPoints(searchTraces[i], testPoly);
+                            thisTracesSelection = fillSelectionItem(traceSelection, searchTraces[i]);
+
+                            if(selection.length) {
+                                for(var j = 0; j < thisTracesSelection.length; j++) {
+                                    selection.push(thisTracesSelection[j]);
+                                }
+                            }
+                            else selection = thisTracesSelection;
+                        }
+
+                        eventData = {points: selection};
+                        updateSelectedState(gd, searchTraces, eventData);
+
+                        if(currentPolygon && dragOptions.polygons) {
+                            dragOptions.polygons.push(currentPolygon);
+                        }
+                    }
+
+                }
+
+                drawSelection(dragOptions.mergedPolygons, outlines);
+
                 // TODO: remove in v2 - this was probably never intended to work as it does,
                 // but in case anyone depends on it we don't want to break it now.
                 gd.emit('plotly_selected', undefined);
@@ -349,6 +403,126 @@ function prepSelect(e, startX, startY, dragOptions, mode) {
     };
 }
 
+function drawSelection(polygons, outlines) {
+    var paths = [];
+    var i;
+    var d;
+
+    for(i = 0; i < polygons.length; i++) {
+        var ppts = polygons[i];
+        paths.push(ppts.join('L') + 'L' + ppts[0]);
+    }
+
+    d = polygons.length > 0 ?
+      'M' + paths.join('M') + 'Z' :
+      ''; // TODO empty d attribute works in Chrome, but is it valid / can we rely on it?
+    outlines.attr('d', d);
+}
+
+function isHoverDataSet(hoverData) {
+    return hoverData &&
+      Array.isArray(hoverData) &&
+      hoverData[0].hoverOnBox !== true;
+}
+
+function extractClickedPtInfo(hoverData, searchTraces) {
+    var hoverDatum = hoverData[0];
+    var pointNumber = -1;
+    var pointNumbers = [];
+    var searchInfo;
+    var i;
+
+    for(i = 0; i < searchTraces.length; i++) {
+        searchInfo = searchTraces[i];
+        if(hoverDatum.fullData._expandedIndex === searchInfo.cd[0].trace._expandedIndex) {
+
+            // Special case for box (and violin)
+            if(hoverDatum.hoverOnBox === true) {
+                break;
+            }
+
+            // TODO hoverDatum not having a pointNumber but a binNumber seems to be an oddity of histogram only
+            // Not deleting .pointNumber in histogram/event_data.js would simplify code here and in addition
+            // would not break the hover event structure
+            // documented at https://plot.ly/javascript/hover-events/
+            if(hoverDatum.pointNumber !== undefined) {
+                pointNumber = hoverDatum.pointNumber;
+            } else if(hoverDatum.binNumber !== undefined) {
+                pointNumber = hoverDatum.binNumber;
+                pointNumbers = hoverDatum.pointNumbers;
+            }
+
+            break;
+        }
+    }
+
+    return {
+        pointNumber: pointNumber,
+        pointNumbers: pointNumbers,
+        searchInfo: searchInfo
+    };
+}
+
+// TODO What about passing a searchInfo instead of wantedExpandedTraceIndex?
+function createPtNumTester(wantedPointNumber, wantedExpandedTraceIndex, subtract) {
+    return {
+        xmin: 0,
+        xmax: 0,
+        ymin: 0,
+        ymax: 0,
+        pts: [],
+        // TODO Consider making signature of contains more lean
+        contains: function(pt, omitFirstEdge, pointNumber, expandedTraceIndex) {
+            return expandedTraceIndex === wantedExpandedTraceIndex && pointNumber === wantedPointNumber;
+        },
+        isRect: false,
+        degenerate: false,
+        subtract: subtract
+    };
+}
+
+function isPointSelected(trace, pointNumber) {
+    // TODO improve perf
+    // Primarily we need this function to determine if a click adds or subtracts from a selection.
+    //
+    // IME best user experience would be
+    // - that Shift+Click an unselected points adds to selection
+    // - and Shift+Click a selected point subtracts from selection.
+    //
+    // Several options:
+    // 1. Avoid problem at all by binding subtract-selection-by-click operation to Shift+Alt-Click.
+    //    Slightly less intuitive. A lot of programs deselect an already selected element when you
+    //    Shift+Click it.
+    // 2. Delegate decision to the traces module through an additional
+    //    isSelected(searchInfo, pointNumber) function. Traces like scatter or bar have
+    //    a selected flag attached to each calcData element, thus access to that information
+    //    would be fast. However, scattergl only maintains selectBatch and unselectBatch arrays.
+    //    So simply searching through those arrays in scattegl would be slow. Just imagine
+    //    a user selecting all data points with one lasso polygon. So scattergl would require some
+    //    work.
+    return trace.selectedpoints ? trace.selectedpoints.indexOf(pointNumber) > -1 : false;
+}
+
+function isOnlyOnePointSelected(searchTraces) {
+    var len = 0;
+    var searchInfo;
+    var trace;
+    var i;
+
+    for(i = 0; i < searchTraces.length; i++) {
+        searchInfo = searchTraces[i];
+        trace = searchInfo.cd[0].trace;
+        if(trace.selectedpoints) {
+            if(trace.selectedpoints.length > 1) return false;
+
+            len += trace.selectedpoints.length;
+            if(len > 1) return false;
+        }
+    }
+
+    return len === 1;
+}
+
 function updateSelectedState(gd, searchTraces, eventData) {
     var i, j, searchInfo, trace;
 

Diff for: src/traces/bar/select.js

@@ -24,7 +24,7 @@ module.exports = function selectPoints(searchInfo, polygon) {
         for(i = 0; i < cd.length; i++) {
             var di = cd[i];
 
-            if(polygon.contains(di.ct)) {
+            if(polygon.contains(di.ct, false, i, searchInfo.cd[0].trace._expandedIndex)) {
                 selection.push({
                     pointNumber: i,
                     x: xa.c2d(di.x),

Diff for: src/traces/scatter/select.js

@@ -36,7 +36,7 @@ module.exports = function selectPoints(searchInfo, polygon) {
             x = xa.c2p(di.x);
             y = ya.c2p(di.y);
 
-            if(polygon.contains([x, y])) {
+            if(polygon.contains([x, y], false, i, searchInfo.cd[0].trace._expandedIndex)) {
                 selection.push({
                     pointNumber: i,
                     x: xa.c2d(di.x),

Diff for: src/traces/scattergl/index.js

@@ -840,7 +840,7 @@ function selectPoints(searchInfo, polygon) {
     if(polygon !== false && !polygon.degenerate) {
         els = [], unels = [];
         for(i = 0; i < stash.count; i++) {
-            if(polygon.contains([stash.xpx[i], stash.ypx[i]])) {
+            if(polygon.contains([stash.xpx[i], stash.ypx[i]], false, i, searchInfo.cd[0].trace._expandedIndex)) {
                 els.push(i);
                 selection.push({
                     pointNumber: i,