Skip to content

fix line decimation for segments crossing the viewport #2705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 37 additions & 5 deletions src/traces/scatter/line_points.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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];
Expand Down
47 changes: 45 additions & 2 deletions test/jasmine/tests/scatter_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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});
Expand All @@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done. 💃

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]
]);
});
});

});
Expand Down