Merge pull request #2705 from plotly/decimation-fix

fix line decimation for segments crossing the viewport

Author: alexcjohnson

src/traces/scatter/line_points.js

@@ -61,17 +61,44 @@ module.exports = function linePoints(d, opts) {
     // turn one calcdata point into pixel coordinates
     function getPt(index) {
         var di = d[index];
+        if(!di) return false;
         var x = xa.c2p(di.x);
         var y = ya.c2p(di.y);
         if(x === BADNUM || y === BADNUM) return di.intoCenter || false;
         return [x, y];
     }
 
+    function crossesViewport(xFrac0, yFrac0, xFrac1, yFrac1) {
+        var dx = xFrac1 - xFrac0;
+        var dy = yFrac1 - yFrac0;
+        var dx0 = 0.5 - xFrac0;
+        var dy0 = 0.5 - yFrac0;
+        var norm2 = dx * dx + dy * dy;
+        var dot = dx * dx0 + dy * dy0;
+        if(dot > 0 && dot < norm2) {
+            var cross = dx0 * dy - dy0 * dx;
+            if(cross * cross < norm2) return true;
+        }
+    }
+
+    var latestXFrac, latestYFrac;
     // if we're off-screen, increase tolerance over baseTolerance
-    function getTolerance(pt) {
+    function getTolerance(pt, nextPt) {
         var xFrac = pt[0] / xa._length;
         var yFrac = pt[1] / ya._length;
-        return (1 + constants.toleranceGrowth * Math.max(0, -xFrac, xFrac - 1, -yFrac, yFrac - 1)) * baseTolerance;
+        var offScreenFraction = Math.max(0, -xFrac, xFrac - 1, -yFrac, yFrac - 1);
+        if(offScreenFraction && (latestXFrac !== undefined) &&
+            crossesViewport(xFrac, yFrac, latestXFrac, latestYFrac)
+        ) {
+            offScreenFraction = 0;
+        }
+        if(offScreenFraction && nextPt &&
+            crossesViewport(xFrac, yFrac, nextPt[0] / xa._length, nextPt[1] / ya._length)
+        ) {
+            offScreenFraction = 0;
+        }
+
+        return (1 + constants.toleranceGrowth * offScreenFraction) * baseTolerance;
     }
 
     function ptDist(pt1, pt2) {
@@ -239,6 +266,8 @@ module.exports = function linePoints(d, opts) {
     }
 
     function addPt(pt) {
+        latestXFrac = pt[0] / xa._length;
+        latestYFrac = pt[1] / ya._length;
         // Are we more than maxScreensAway off-screen any direction?
         // if so, clip to this box, but in such a way that on-screen
         // drawing is unchanged
@@ -333,9 +362,11 @@ module.exports = function linePoints(d, opts) {
                 continue;
             }
 
+            var nextPt = getPt(i + 1);
+
             clusterRefDist = ptDist(clusterHighPt, clusterStartPt);
 
-            if(clusterRefDist < getTolerance(clusterHighPt) * minTolerance) continue;
+            if(clusterRefDist < getTolerance(clusterHighPt, nextPt) * minTolerance) continue;
 
             clusterUnitVector = [
                 (clusterHighPt[0] - clusterStartPt[0]) / clusterRefDist,
@@ -350,7 +381,8 @@ module.exports = function linePoints(d, opts) {
 
             // loop over one cluster of points that collapse onto one line
             for(i++; i < d.length; i++) {
-                thisPt = getPt(i);
+                thisPt = nextPt;
+                nextPt = getPt(i + 1);
                 if(!thisPt) {
                     if(connectGaps) continue;
                     else break;
@@ -364,7 +396,7 @@ module.exports = function linePoints(d, opts) {
                 clusterMinDeviation = Math.min(clusterMinDeviation, thisDeviation);
                 clusterMaxDeviation = Math.max(clusterMaxDeviation, thisDeviation);
 
-                if(clusterMaxDeviation - clusterMinDeviation > getTolerance(thisPt)) break;
+                if(clusterMaxDeviation - clusterMinDeviation > getTolerance(thisPt, nextPt)) break;
 
                 clusterEndPt = thisPt;
                 thisVal = thisVector[0] * clusterUnitVector[0] + thisVector[1] * clusterUnitVector[1];

test/jasmine/tests/scatter_test.js

@@ -405,6 +405,10 @@ describe('Test scatter', function() {
 
         // TODO: test coarser decimation outside plot, and removing very near duplicates from the four of a cluster
 
+        function reverseXY(v) { return [v[1], v[0]]; }
+
+        function reverseXY2(v) { return v.map(reverseXY); }
+
         it('should clip extreme points without changing on-screen paths', function() {
             var ptsIn = [
                 // first bunch: rays going in/out in many directions
@@ -458,8 +462,6 @@ describe('Test scatter', function() {
                 [[2100, 2100], [-2000, 2100], [-2000, -2000], [2100, -2000], [2100, 2100]]
             ];
 
-            function reverseXY(v) { return [v[1], v[0]]; }
-
             ptsIn.forEach(function(ptsIni, i) {
                 // disable clustering for these tests
                 var ptsOut = callLinePoints(ptsIni, {simplify: false});
@@ -472,6 +474,47 @@ describe('Test scatter', function() {
                 expect(ptsOut2[0]).toBeCloseTo2DArray(ptsExpected[i].map(reverseXY), 1, i);
             });
         });
+
+        it('works when far off-screen points cross the viewport', function() {
+            function _check(ptsIn, ptsExpected) {
+                var ptsOut = callLinePoints(ptsIn);
+                expect(JSON.stringify(ptsOut)).toEqual(JSON.stringify([ptsExpected]));
+
+                var ptsOut2 = callLinePoints(ptsIn.map(reverseXY)).map(reverseXY2);
+                expect(JSON.stringify(ptsOut2)).toEqual(JSON.stringify([ptsExpected]));
+            }
+
+            // first cross the viewport horizontally/vertically
+            _check([
+                [-822, 20], [-802, 2], [-801.5, 1.1], [-800, 0],
+                [900, 0], [901.5, 1.1], [902, 2], [922, 20]
+            ], [
+                // all that's really important here (and the next check) is that
+                // the points [-800, 0] and [900, 0] are connected. What we do
+                // with other points beyond those doesn't matter too much.
+                [-822, 20], [-800, 0],
+                [900, 0], [922, 20]
+            ]);
+
+            _check([
+                [-804, 4], [-802, 2], [-801.5, 1.1], [-800, 0],
+                [900, 0], [901.5, 1.1], [902, 2], [904, 4]
+            ], [
+                [-804, 4], [-800, 0],
+                [900, 0]
+            ]);
+
+            // now cross the viewport diagonally
+            _check([
+                [-801, 925], [-800, 902], [-800.5, 901.1], [-800, 900],
+                [900, -800], [900.5, -801.1], [900, -802], [901, -825]
+            ], [
+                // similarly here, we just care that
+                // [-800, 900] connects to [900, -800]
+                [-801, 925], [-800, 900],
+                [900, -800], [901, -825]
+            ]);
+        });
     });
 
 });