From 04c104d795f1c1857c6e499417ca61684cdcfce3 Mon Sep 17 00:00:00 2001 From: Klaus Weidner Date: Fri, 2 Mar 2012 13:09:47 -0800 Subject: [PATCH] Fix connectSeparatedPoints, add test The original connectSeparatedPoints code renumbered points within series, with the effect that 'row' as used in setSelection didn't work as expected - it selects unrelated points. This change keeps the point numbering intact, and instead moves the logic to skip missing points to the canvas renderer and setSelection(). A new auto_test confirms this behavior. --- auto_tests/tests/callback.js | 51 ++++++++++++++++++++++++++++++++++++++++++++ dygraph-canvas.js | 44 ++++++++++++++++++++++++++++++-------- dygraph-layout.js | 4 ++++ dygraph-utils.js | 9 +++++--- dygraph.js | 25 +++++++++++----------- 5 files changed, 108 insertions(+), 25 deletions(-) diff --git a/auto_tests/tests/callback.js b/auto_tests/tests/callback.js index ceb9963..9551eb0 100644 --- a/auto_tests/tests/callback.js +++ b/auto_tests/tests/callback.js @@ -308,3 +308,54 @@ CallbackTestCase.prototype.testNaNData = function() { assertEquals(1, res.row); assertEquals('c', res.seriesName); }; + +CallbackTestCase.prototype.testGapHighlight = function() { +var dataGap = [ + [1, null, 3], + [2, 2, null], + [3, null, 5], + [4, 4, null], + [5, null, 7], + [6, NaN, null], + [8, 8, null], + [10, 10, null]]; + + var h_row; + var h_pts; + + var highlightCallback = function(e, x, pts, row) { + h_row = row; + h_pts = pts; + }; + + var graph = document.getElementById("graph"); + var g = new Dygraph(graph, dataGap, { + width: 400, + height: 300, + //stackedGraph: true, + connectSeparatedPoints: true, + drawPoints: true, + labels: ['x', 'A', 'B'], + highlightCallback : highlightCallback + }); + + DygraphOps.dispatchMouseMove(g, 1.1, 10); + //point from series B + assertEquals(0, h_row); + assertEquals(1, h_pts.length); + assertEquals(3, h_pts[0].yval); + assertEquals('B', h_pts[0].name); + + DygraphOps.dispatchMouseMove(g, 6.1, 10); + // A is NaN at x=6 + assertEquals(1, h_pts.length); + assert(isNaN(h_pts[0].yval)); + assertEquals('A', h_pts[0].name); + + DygraphOps.dispatchMouseMove(g, 8.1, 10); + //point from series A + assertEquals(6, h_row); + assertEquals(1, h_pts.length); + assertEquals(8, h_pts[0].yval); + assertEquals('A', h_pts[0].name); +}; diff --git a/dygraph-canvas.js b/dygraph-canvas.js index 5fd02e5..6e15a58 100644 --- a/dygraph-canvas.js +++ b/dygraph-canvas.js @@ -658,6 +658,19 @@ DygraphCanvasRenderer.prototype._renderAnnotations = function() { } }; +DygraphCanvasRenderer.makeNextPointStep_ = function(connect, points, end) { + if (connect) { + return function(j) { + while (++j < end) { + if (!(points[j].yval === null)) break; + } + return j; + } + } else { + return function(j) { return j + 1 }; + } +}; + DygraphCanvasRenderer.prototype._drawStyledLine = function( ctx, i, setName, color, strokeWidth, strokePattern, drawPoints, drawPointCallback, pointSize) { @@ -678,8 +691,10 @@ DygraphCanvasRenderer.prototype._drawStyledLine = function( } var point; + var next = DygraphCanvasRenderer.makeNextPointStep_( + this.attr_('connectSeparatedPoints'), points, afterLastIndexInSet); ctx.save(); - for (var j = firstIndexInSet; j < afterLastIndexInSet; j++) { + for (var j = firstIndexInSet; j < afterLastIndexInSet; j = next(j)) { point = points[j]; if (isNullOrNaN(point.canvasy)) { if (stepPlot && prevX !== null) { @@ -736,7 +751,6 @@ DygraphCanvasRenderer.prototype._drawStyledLine = function( this.dygraph_, setName, ctx, cb[0], cb[1], color, pointSize); ctx.restore(); } - firstIndexInSet = afterLastIndexInSet; ctx.restore(); }; @@ -813,6 +827,14 @@ DygraphCanvasRenderer.prototype._renderLineChart = function() { axis = this.dygraph_.axisPropertiesForSeries(setName); color = this.colors[setName]; + var firstIndexInSet = this.layout.setPointsOffsets[i]; + var setLength = this.layout.setPointsLengths[i]; + var afterLastIndexInSet = firstIndexInSet + setLength; + + var next = DygraphCanvasRenderer.makeNextPointStep_( + this.attr_('connectSeparatedPoints'), points, + afterLastIndexInSet); + // setup graphics context prevX = NaN; prevY = NaN; @@ -824,9 +846,9 @@ DygraphCanvasRenderer.prototype._renderLineChart = function() { fillAlpha + ')'; ctx.fillStyle = err_color; ctx.beginPath(); - for (j = 0; j < pointsLength; j++) { + for (j = firstIndexInSet; j < afterLastIndexInSet; j = next(j)) { point = points[j]; - if (point.name == setName) { + if (point.name == setName) { // TODO(klausw): this is always true if (!Dygraph.isOK(point.y)) { prevX = NaN; continue; @@ -876,6 +898,13 @@ DygraphCanvasRenderer.prototype._renderLineChart = function() { if (axisY < 0.0) axisY = 0.0; else if (axisY > 1.0) axisY = 1.0; axisY = this.area.h * axisY + this.area.y; + var firstIndexInSet = this.layout.setPointsOffsets[i]; + var setLength = this.layout.setPointsLengths[i]; + var afterLastIndexInSet = firstIndexInSet + setLength; + + var next = DygraphCanvasRenderer.makeNextPointStep_( + this.attr_('connectSeparatedPoints'), points, + afterLastIndexInSet); // setup graphics context prevX = NaN; @@ -887,9 +916,9 @@ DygraphCanvasRenderer.prototype._renderLineChart = function() { fillAlpha + ')'; ctx.fillStyle = err_color; ctx.beginPath(); - for (j = 0; j < pointsLength; j++) { + for (j = firstIndexInSet; j < afterLastIndexInSet; j = next(j)) { point = points[j]; - if (point.name == setName) { + if (point.name == setName) { // TODO(klausw): this is always true if (!Dygraph.isOK(point.y)) { prevX = NaN; continue; @@ -923,9 +952,6 @@ DygraphCanvasRenderer.prototype._renderLineChart = function() { } // Drawing the lines. - var firstIndexInSet = 0; - var afterLastIndexInSet = 0; - var setLength = 0; for (i = 0; i < setCount; i += 1) { this._drawLine(ctx, i); } diff --git a/dygraph-layout.js b/dygraph-layout.js index 1563817..4938ada 100644 --- a/dygraph-layout.js +++ b/dygraph-layout.js @@ -215,6 +215,7 @@ DygraphLayout.prototype._evaluateLineCharts = function() { this.setPointsLengths = []; this.setPointsOffsets = []; + var connectSeparated = this.attr_('connectSeparatedPoints'); for (var setIdx = 0; setIdx < this.datasets.length; ++setIdx) { var dataset = this.datasets[setIdx]; var setName = this.setNames[setIdx]; @@ -241,6 +242,9 @@ DygraphLayout.prototype._evaluateLineCharts = function() { yval: yValue, name: setName }; + if (connectSeparated && item[1] === null) { + point.yval = null; + } this.points.push(point); setPointsLength += 1; } diff --git a/dygraph-utils.js b/dygraph-utils.js index 3103986..689f8eb 100644 --- a/dygraph-utils.js +++ b/dygraph-utils.js @@ -334,12 +334,15 @@ Dygraph.isOK = function(x) { /** * @private * @param { Object } p The point to consider, valid points are {x, y} objects + * @param { Boolean } allowNaNY Treat point with y=NaN as valid * @return { Boolean } Whether the point has numeric x and y. */ -Dygraph.isValidPoint = function(p) { +Dygraph.isValidPoint = function(p, allowNaNY) { if (!p) return false; // null or undefined object - if (isNaN(p.x) || p.x === null || p.x === undefined) return false; - if (isNaN(p.y) || p.y === null || p.y === undefined) return false; + if (p.yval === null) return false; // missing point + if (p.x === null || p.x === undefined) return false; + if (p.y === null || p.y === undefined) return false; + if (isNaN(p.x) || (!allowNaNY && isNaN(p.y))) return false; return true; }; diff --git a/dygraph.js b/dygraph.js index 01b10a3..b52a689 100644 --- a/dygraph.js +++ b/dygraph.js @@ -1515,7 +1515,7 @@ Dygraph.prototype.findClosestRow = function(domX) { var l = points.length; for (var i = 0; i < l; i++) { var point = points[i]; - if (!Dygraph.isValidPoint(point)) continue; + if (!Dygraph.isValidPoint(point, true)) continue; var dist = Math.abs(point.canvasx - domX); if (dist < minDistX) { minDistX = dist; @@ -1999,7 +1999,7 @@ Dygraph.prototype.setSelection = function(row, opt_seriesName) { point = this.layout_.unstackPointAtIndex(pos+row); } - this.selPoints_.push(point); + if (!(point.yval === null)) this.selPoints_.push(point); } pos += set.length; } @@ -2197,9 +2197,8 @@ Dygraph.prototype.predraw_ = function() { // rolling averages. this.rolledSeries_ = [null]; // x-axis is the first series and it's special for (var i = 1; i < this.numColumns(); i++) { - var connectSeparatedPoints = this.attr_('connectSeparatedPoints', i); - var logScale = this.attr_('logscale', i); - var series = this.extractSeries_(this.rawData_, i, logScale, connectSeparatedPoints); + var logScale = this.attr_('logscale', i); // TODO(klausw): this looks wrong + var series = this.extractSeries_(this.rawData_, i, logScale); series = this.rollingAverage(series, this.rollPeriod_); this.rolledSeries_.push(series); } @@ -2296,6 +2295,11 @@ Dygraph.prototype.gatherDatasets_ = function(rolledSeries, dateWindow) { } actual_y = series[j][1]; + if (actual_y === null) { + series[j] = [x, null]; + continue; + } + cumulative_y[x] += actual_y; series[j] = [x, cumulative_y[x]]; @@ -2682,24 +2686,19 @@ Dygraph.prototype.computeYAxisRanges_ = function(extremes) { * * @private */ -Dygraph.prototype.extractSeries_ = function(rawData, i, logScale, connectSeparatedPoints) { +Dygraph.prototype.extractSeries_ = function(rawData, i, logScale) { var series = []; for (var j = 0; j < rawData.length; j++) { var x = rawData[j][0]; var point = rawData[j][i]; if (logScale) { // On the log scale, points less than zero do not exist. - // This will create a gap in the chart. Note that this ignores - // connectSeparatedPoints. + // This will create a gap in the chart. if (point <= 0) { point = null; } - series.push([x, point]); - } else { - if (point !== null || !connectSeparatedPoints) { - series.push([x, point]); - } } + series.push([x, point]); } return series; }; -- 2.7.4