From 10c62aa2111c3cd400f7093c5c9e279c30102e23 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Wed, 6 Jun 2018 23:01:52 -0400 Subject: [PATCH 1/2] fix line decimation for segments crossing the viewport from far away on either side --- src/traces/scatter/line_points.js | 42 ++++++++++++++++++++++++++---- test/jasmine/tests/scatter_test.js | 42 ++++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/traces/scatter/line_points.js b/src/traces/scatter/line_points.js index 6814f27125a..a198ba450be 100644 --- a/src/traces/scatter/line_points.js +++ b/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]; diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index d1c6c30dbff..feac4c78c1a 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/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,42 @@ 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] + ], [ + [-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] + ], [ + [-801, 925], [-800, 900], + [900, -800], [901, -825] + ]); + }); }); }); From 99a550a115d9de3ee27e9e0591a13c45e16df744 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Wed, 6 Jun 2018 23:13:45 -0400 Subject: [PATCH 2/2] comment on what the new decimation test really cares about --- test/jasmine/tests/scatter_test.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index feac4c78c1a..3a837b9dfde 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -489,6 +489,9 @@ describe('Test scatter', function() { [-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] ]); @@ -506,6 +509,8 @@ describe('Test scatter', function() { [-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] ]);