From f0bf3954fe363eda86a3db170063aba34f1be08e Mon Sep 17 00:00:00 2001 From: jcopperfield <33193571+jcopperfield@users.noreply.github.com> Date: Wed, 13 Dec 2017 00:43:51 +0100 Subject: [PATCH] Fix issues #4572 and #4703 (#4959) - issue #4572: logarithmic type if all numbers are zero browser crashes "Out of memory" - issue #4703: [BUG] Browser unresponsive on bubble chart with logarithmic axes --- src/scales/scale.logarithmic.js | 59 ++- test/specs/scale.logarithmic.tests.js | 497 ++++++++++++++++++++------ 2 files changed, 435 insertions(+), 121 deletions(-) diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 09296c19568..87fe9b1af2b 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -18,11 +18,9 @@ module.exports = function(Chart) { determineDataLimits: function() { var me = this; var opts = me.options; - var tickOpts = opts.ticks; var chart = me.chart; var data = chart.data; var datasets = data.datasets; - var valueOrDefault = helpers.valueOrDefault; var isHorizontal = me.isHorizontal(); function IDMatches(meta) { return isHorizontal ? meta.xAxisID === me.id : meta.yAxisID === me.id; @@ -68,27 +66,23 @@ module.exports = function(Chart) { helpers.each(dataset.data, function(rawValue, index) { var values = valuesPerStack[key]; var value = +me.getRightValue(rawValue); - if (isNaN(value) || meta.data[index].hidden) { + // invalid, hidden and negative values are ignored + if (isNaN(value) || meta.data[index].hidden || value < 0) { return; } - values[index] = values[index] || 0; - - if (opts.relativePoints) { - values[index] = 100; - } else { - // Don't need to split positive and negative since the log scale can't handle a 0 crossing - values[index] += value; - } + values[index] += value; }); } }); helpers.each(valuesPerStack, function(valuesForType) { - var minVal = helpers.min(valuesForType); - var maxVal = helpers.max(valuesForType); - me.min = me.min === null ? minVal : Math.min(me.min, minVal); - me.max = me.max === null ? maxVal : Math.max(me.max, maxVal); + if (valuesForType.length > 0) { + var minVal = helpers.min(valuesForType); + var maxVal = helpers.max(valuesForType); + me.min = me.min === null ? minVal : Math.min(me.min, minVal); + me.max = me.max === null ? maxVal : Math.max(me.max, maxVal); + } }); } else { @@ -97,7 +91,8 @@ module.exports = function(Chart) { if (chart.isDatasetVisible(datasetIndex) && IDMatches(meta)) { helpers.each(dataset.data, function(rawValue, index) { var value = +me.getRightValue(rawValue); - if (isNaN(value) || meta.data[index].hidden) { + // invalid, hidden and negative values are ignored + if (isNaN(value) || meta.data[index].hidden || value < 0) { return; } @@ -121,6 +116,17 @@ module.exports = function(Chart) { }); } + // Common base implementation to handle ticks.min, ticks.max + this.handleTickRangeOptions(); + }, + handleTickRangeOptions: function() { + var me = this; + var opts = me.options; + var tickOpts = opts.ticks; + var valueOrDefault = helpers.valueOrDefault; + var DEFAULT_MIN = 1; + var DEFAULT_MAX = 10; + me.min = valueOrDefault(tickOpts.min, me.min); me.max = valueOrDefault(tickOpts.max, me.max); @@ -129,8 +135,25 @@ module.exports = function(Chart) { me.min = Math.pow(10, Math.floor(helpers.log10(me.min)) - 1); me.max = Math.pow(10, Math.floor(helpers.log10(me.max)) + 1); } else { - me.min = 1; - me.max = 10; + me.min = DEFAULT_MIN; + me.max = DEFAULT_MAX; + } + } + if (me.min === null) { + me.min = Math.pow(10, Math.floor(helpers.log10(me.max)) - 1); + } + if (me.max === null) { + me.max = me.min !== 0 + ? Math.pow(10, Math.floor(helpers.log10(me.min)) + 1) + : DEFAULT_MAX; + } + if (me.minNotZero === null) { + if (me.min > 0) { + me.minNotZero = me.min; + } else if (me.max < 1) { + me.minNotZero = Math.pow(10, Math.floor(helpers.log10(me.max))); + } else { + me.minNotZero = DEFAULT_MIN; } } }, diff --git a/test/specs/scale.logarithmic.tests.js b/test/specs/scale.logarithmic.tests.js index 3fd92197763..3d79e6cfc37 100644 --- a/test/specs/scale.logarithmic.tests.js +++ b/test/specs/scale.logarithmic.tests.js @@ -690,142 +690,433 @@ describe('Logarithmic Scale tests', function() { expect(chart.scales.yScale1.getLabelForIndex(0, 2)).toBe(150); }); - it('should get the correct pixel value for a point', function() { - var chart = window.acquireChart({ - type: 'line', - data: { - datasets: [{ - xAxisID: 'xScale', // for the horizontal scale - yAxisID: 'yScale', - data: [{x: 10, y: 10}, {x: 5, y: 5}, {x: 1, y: 1}, {x: 25, y: 25}, {x: 78, y: 78}] - }], + describe('when', function() { + var data = [ + { + data: [1, 39], + stack: 'stack' }, - options: { - scales: { - xAxes: [{ - id: 'xScale', - type: 'logarithmic' - }], + { + data: [1, 39], + stack: 'stack' + }, + ]; + var dataWithEmptyStacks = [ + { + data: [] + }, + { + data: [] + } + ].concat(data); + var config = [ + { + axis: 'y', + firstTick: 1, // start of the axis (minimum) + describe: 'all stacks are defined' + }, + { + axis: 'y', + data: dataWithEmptyStacks, + firstTick: 1, + describe: 'not all stacks are defined' + }, + { + axis: 'y', + scale: { yAxes: [{ - id: 'yScale', - type: 'logarithmic' + ticks: { + min: 0 + } }] - } + }, + firstTick: 0, + describe: 'all stacks are defined and ticks.min: 0' + }, + { + axis: 'y', + data: dataWithEmptyStacks, + scale: { + yAxes: [{ + ticks: { + min: 0 + } + }] + }, + firstTick: 0, + describe: 'not stacks are defined and ticks.min: 0' + }, + { + axis: 'x', + firstTick: 1, + describe: 'all stacks are defined' + }, + { + axis: 'x', + data: dataWithEmptyStacks, + firstTick: 1, + describe: 'not all stacks are defined' + }, + { + axis: 'x', + scale: { + xAxes: [{ + ticks: { + min: 0 + } + }] + }, + firstTick: 0, + describe: 'all stacks are defined and ticks.min: 0' + }, + { + axis: 'x', + data: dataWithEmptyStacks, + scale: { + xAxes: [{ + ticks: { + min: 0 + } + }] + }, + firstTick: 0, + describe: 'not all stacks are defined and ticks.min: 0' + }, + ]; + config.forEach(function(setup) { + var scaleConfig = {}; + var type, chartStart, chartEnd; + + if (setup.axis === 'x') { + type = 'horizontalBar'; + chartStart = 'left'; + chartEnd = 'right'; + } else { + type = 'bar'; + chartStart = 'bottom'; + chartEnd = 'top'; } - }); + scaleConfig[setup.axis + 'Axes'] = [{ + type: 'logarithmic' + }]; + Chart.helpers.extend(scaleConfig, setup.scale); + var description = 'dataset has stack option and ' + setup.describe + + ' and axis is "' + setup.axis + '";'; + describe(description, function() { + it('should define the correct axis limits', function() { + var chart = window.acquireChart({ + type: type, + data: { + labels: ['category 1', 'category 2'], + datasets: setup.data || data, + }, + options: { + scales: scaleConfig + } + }); - var xScale = chart.scales.xScale; - expect(xScale.getPixelForValue(80, 0, 0)).toBeCloseToPixel(495); // right - paddingRight - expect(xScale.getPixelForValue(1, 0, 0)).toBeCloseToPixel(37 + 6); // left + paddingLeft + lineSpace - expect(xScale.getPixelForValue(10, 0, 0)).toBeCloseToPixel(278 + 6 / 2); // halfway - expect(xScale.getPixelForValue(0, 0, 0)).toBeCloseToPixel(37 + 6); // 0 is invalid, put it on the left. + var axisID = setup.axis + '-axis-0'; + var scale = chart.scales[axisID]; + var firstTick = setup.firstTick; + var lastTick = 80; // last tick (should be first available tick after: 2 * 39) + var start = chart.chartArea[chartStart]; + var end = chart.chartArea[chartEnd]; - expect(xScale.getValueForPixel(495)).toBeCloseToPixel(80); - expect(xScale.getValueForPixel(48)).toBeCloseTo(1, 1e-4); - expect(xScale.getValueForPixel(278)).toBeCloseTo(10, 1e-4); + expect(scale.getPixelForValue(firstTick, 0, 0)).toBe(start); + expect(scale.getPixelForValue(lastTick, 0, 0)).toBe(end); - var yScale = chart.scales.yScale; - expect(yScale.getPixelForValue(80, 0, 0)).toBeCloseToPixel(32); // top + paddingTop - expect(yScale.getPixelForValue(1, 0, 0)).toBeCloseToPixel(484); // bottom - paddingBottom - expect(yScale.getPixelForValue(10, 0, 0)).toBeCloseToPixel(246); // halfway - expect(yScale.getPixelForValue(0, 0, 0)).toBeCloseToPixel(484); // 0 is invalid. force it on bottom - - expect(yScale.getValueForPixel(32)).toBeCloseTo(80, 1e-4); - expect(yScale.getValueForPixel(484)).toBeCloseTo(1, 1e-4); - expect(yScale.getValueForPixel(246)).toBeCloseTo(10, 1e-4); + expect(scale.getValueForPixel(start)).toBeCloseTo(firstTick, 4); + expect(scale.getValueForPixel(end)).toBeCloseTo(lastTick, 4); + + chart.scales[axisID].options.ticks.reverse = true; // Reverse mode + chart.update(); + + // chartArea might have been resized in update + start = chart.chartArea[chartEnd]; + end = chart.chartArea[chartStart]; + + expect(scale.getPixelForValue(firstTick, 0, 0)).toBe(start); + expect(scale.getPixelForValue(lastTick, 0, 0)).toBe(end); + + expect(scale.getValueForPixel(start)).toBeCloseTo(firstTick, 4); + expect(scale.getValueForPixel(end)).toBeCloseTo(lastTick, 4); + }); + }); + }); }); - it('should get the correct pixel value for a point when 0 values are present or min: 0', function() { + describe('when', function() { var config = [ { - dataset: [{x: 0, y: 0}, {x: 10, y: 10}, {x: 1.2, y: 1.2}, {x: 25, y: 25}, {x: 78, y: 78}], + dataset: [], firstTick: 1, // value of the first tick - lastTick: 80 + lastTick: 10, // value of the last tick + describe: 'empty dataset, without ticks.min/max' + }, + { + dataset: [], + scale: {stacked: true}, + firstTick: 1, + lastTick: 10, + describe: 'empty dataset, without ticks.min/max, with stacked: true' + }, + { + data: { + datasets: [ + {data: [], stack: 'stack'}, + {data: [], stack: 'stack'}, + ], + }, + type: 'bar', + firstTick: 1, + lastTick: 10, + describe: 'empty dataset with stack option, without ticks.min/max' + }, + { + data: { + datasets: [ + {data: [], stack: 'stack'}, + {data: [], stack: 'stack'}, + ], + }, + type: 'horizontalBar', + firstTick: 1, + lastTick: 10, + describe: 'empty dataset with stack option, without ticks.min/max' + }, + { + dataset: [], + scale: {ticks: {min: 1}}, + firstTick: 1, + lastTick: 10, + describe: 'empty dataset, ticks.min: 1, without ticks.max' + }, + { + dataset: [], + scale: {ticks: {max: 80}}, + firstTick: 1, + lastTick: 80, + describe: 'empty dataset, ticks.max: 80, without ticks.min' + }, + { + dataset: [], + scale: {ticks: {max: 0.8}}, + firstTick: 0.01, + lastTick: 0.8, + describe: 'empty dataset, ticks.max: 0.8, without ticks.min' + }, + { + dataset: [{x: 10, y: 10}, {x: 5, y: 5}, {x: 1, y: 1}, {x: 25, y: 25}, {x: 78, y: 78}], + firstTick: 1, + lastTick: 80, + describe: 'dataset min point {x: 1, y: 1}, max point {x:78, y:78}' + }, + ]; + config.forEach(function(setup) { + var axes = [ + { + id: 'x', // horizontal scale + start: 'left', + end: 'right' + }, + { + id: 'y', // vertical scale + start: 'bottom', + end: 'top' + } + ]; + axes.forEach(function(axis) { + var expectation = 'min = ' + setup.firstTick + ', max = ' + setup.lastTick; + describe(setup.describe + ' and axis is "' + axis.id + '"; expect: ' + expectation + ';', function() { + beforeEach(function() { + var xScaleConfig = { + type: 'logarithmic', + }; + var yScaleConfig = { + type: 'logarithmic', + }; + var data = setup.data || { + datasets: [{ + data: setup.dataset + }], + }; + Chart.helpers.extend(xScaleConfig, setup.scale); + Chart.helpers.extend(yScaleConfig, setup.scale); + Chart.helpers.extend(data, setup.data || {}); + this.chart = window.acquireChart({ + type: 'line', + data: data, + options: { + scales: { + xAxes: [xScaleConfig], + yAxes: [yScaleConfig] + } + } + }); + }); + + it('should get the correct pixel value for a point', function() { + var chart = this.chart; + var axisID = axis.id + '-axis-0'; + var scale = chart.scales[axisID]; + var firstTick = setup.firstTick; + var lastTick = setup.lastTick; + var start = chart.chartArea[axis.start]; + var end = chart.chartArea[axis.end]; + + expect(scale.getPixelForValue(firstTick, 0, 0)).toBe(start); + expect(scale.getPixelForValue(lastTick, 0, 0)).toBe(end); + expect(scale.getPixelForValue(0, 0, 0)).toBe(start); // 0 is invalid, put it at the start. + + expect(scale.getValueForPixel(start)).toBeCloseTo(firstTick, 4); + expect(scale.getValueForPixel(end)).toBeCloseTo(lastTick, 4); + + chart.scales[axisID].options.ticks.reverse = true; // Reverse mode + chart.update(); + + // chartArea might have been resized in update + start = chart.chartArea[axis.end]; + end = chart.chartArea[axis.start]; + + expect(scale.getPixelForValue(firstTick, 0, 0)).toBe(start); + expect(scale.getPixelForValue(lastTick, 0, 0)).toBe(end); + + expect(scale.getValueForPixel(start)).toBeCloseTo(firstTick, 4); + expect(scale.getValueForPixel(end)).toBeCloseTo(lastTick, 4); + }); + }); + }); + }); + }); + + describe('when', function() { + var config = [ + { + dataset: [], + scale: {ticks: {min: 0}}, + firstTick: 1, // value of the first tick + lastTick: 10, // value of the last tick + describe: 'empty dataset, ticks.min: 0, without ticks.max' + }, + { + dataset: [], + scale: {ticks: {min: 0, max: 80}}, + firstTick: 1, + lastTick: 80, + describe: 'empty dataset, ticks.min: 0, ticks.max: 80' + }, + { + dataset: [], + scale: {ticks: {min: 0, max: 0.8}}, + firstTick: 0.1, + lastTick: 0.8, + describe: 'empty dataset, ticks.min: 0, ticks.max: 0.8' + }, + { + dataset: [{x: 0, y: 0}, {x: 10, y: 10}, {x: 1.2, y: 1.2}, {x: 25, y: 25}, {x: 78, y: 78}], + firstTick: 1, + lastTick: 80, + describe: 'dataset min point {x: 0, y: 0}, max point {x:78, y:78}, minNotZero {x: 1.2, y: 1.2}' }, { dataset: [{x: 0, y: 0}, {x: 10, y: 10}, {x: 6.3, y: 6.3}, {x: 25, y: 25}, {x: 78, y: 78}], firstTick: 6, - lastTick: 80 + lastTick: 80, + describe: 'dataset min point {x: 0, y: 0}, max point {x:78, y:78}, minNotZero {x: 6.3, y: 6.3}' }, { dataset: [{x: 10, y: 10}, {x: 1.2, y: 1.2}, {x: 25, y: 25}, {x: 78, y: 78}], scale: {ticks: {min: 0}}, firstTick: 1, - lastTick: 80 + lastTick: 80, + describe: 'dataset min point {x: 1.2, y: 1.2}, max point {x:78, y:78}, ticks.min: 0' }, { dataset: [{x: 10, y: 10}, {x: 6.3, y: 6.3}, {x: 25, y: 25}, {x: 78, y: 78}], scale: {ticks: {min: 0}}, firstTick: 6, - lastTick: 80 + lastTick: 80, + describe: 'dataset min point {x: 6.3, y: 6.3}, max point {x:78, y:78}, ticks.min: 0' }, ]; - Chart.helpers.each(config, function(setup) { - var xScaleConfig = { - type: 'logarithmic' - }; - var yScaleConfig = { - type: 'logarithmic' - }; - Chart.helpers.extend(xScaleConfig, setup.scale); - Chart.helpers.extend(yScaleConfig, setup.scale); - var chart = window.acquireChart({ - type: 'line', - data: { - datasets: [{ - data: setup.dataset - }], - }, - options: { - scales: { - xAxes: [xScaleConfig], - yAxes: [yScaleConfig] - } - } - }); - - var chartArea = chart.chartArea; - var expectations = [ + config.forEach(function(setup) { + var axes = [ { - id: 'x-axis-0', // horizontal scale - axis: 'xAxes', - start: chartArea.left, - end: chartArea.right + id: 'x', // horizontal scale + start: 'left', + end: 'right' }, { - id: 'y-axis-0', // vertical scale - axis: 'yAxes', - start: chartArea.bottom, - end: chartArea.top + id: 'y', // vertical scale + start: 'bottom', + end: 'top' } ]; - Chart.helpers.each(expectations, function(expectation) { - var scale = chart.scales[expectation.id]; - var firstTick = setup.firstTick; - var lastTick = setup.lastTick; - var fontSize = chart.options.defaultFontSize; - var start = expectation.start; - var end = expectation.end; - var sign = scale.isHorizontal() ? 1 : -1; - - expect(scale.getPixelForValue(0, 0, 0)).toBeCloseToPixel(start); - expect(scale.getPixelForValue(lastTick, 0, 0)).toBeCloseToPixel(end); - expect(scale.getPixelForValue(firstTick, 0, 0)).toBeCloseToPixel(start + sign * fontSize); - - expect(scale.getValueForPixel(start)).toBeCloseTo(0); - expect(scale.getValueForPixel(end)).toBeCloseTo(lastTick); - expect(scale.getValueForPixel(start + sign * fontSize)).toBeCloseTo(firstTick); - - chart.options.scales[expectation.axis][0].ticks.reverse = true; // Reverse mode - chart.update(); - - expect(scale.getPixelForValue(0, 0, 0)).toBeCloseToPixel(end); - expect(scale.getPixelForValue(lastTick, 0, 0)).toBeCloseToPixel(start); - expect(scale.getPixelForValue(firstTick, 0, 0)).toBeCloseToPixel(end - sign * fontSize); - - expect(scale.getValueForPixel(end)).toBeCloseTo(0); - expect(scale.getValueForPixel(start)).toBeCloseTo(lastTick); - expect(scale.getValueForPixel(end - sign * fontSize)).toBeCloseTo(firstTick); + axes.forEach(function(axis) { + var expectation = 'min = 0, max = ' + setup.lastTick + ', first tick = ' + setup.firstTick; + describe(setup.describe + ' and axis is "' + axis.id + '"; expect: ' + expectation + ';', function() { + beforeEach(function() { + var xScaleConfig = { + type: 'logarithmic', + }; + var yScaleConfig = { + type: 'logarithmic', + }; + var data = setup.data || { + datasets: [{ + data: setup.dataset + }], + }; + Chart.helpers.extend(xScaleConfig, setup.scale); + Chart.helpers.extend(yScaleConfig, setup.scale); + Chart.helpers.extend(data, setup.data || {}); + this.chart = window.acquireChart({ + type: 'line', + data: data, + options: { + scales: { + xAxes: [xScaleConfig], + yAxes: [yScaleConfig] + } + } + }); + }); + + it('should get the correct pixel value for a point', function() { + var chart = this.chart; + var axisID = axis.id + '-axis-0'; + var scale = chart.scales[axisID]; + var firstTick = setup.firstTick; + var lastTick = setup.lastTick; + var fontSize = chart.options.defaultFontSize; + var start = chart.chartArea[axis.start]; + var end = chart.chartArea[axis.end]; + var sign = scale.isHorizontal() ? 1 : -1; + + expect(scale.getPixelForValue(0, 0, 0)).toBe(start); + expect(scale.getPixelForValue(lastTick, 0, 0)).toBe(end); + expect(scale.getPixelForValue(firstTick, 0, 0)).toBe(start + sign * fontSize); + + expect(scale.getValueForPixel(start)).toBeCloseTo(0, 4); + expect(scale.getValueForPixel(end)).toBeCloseTo(lastTick, 4); + expect(scale.getValueForPixel(start + sign * fontSize)).toBeCloseTo(firstTick, 4); + + chart.scales[axisID].options.ticks.reverse = true; // Reverse mode + chart.update(); + + // chartArea might have been resized in update + start = chart.chartArea[axis.end]; + end = chart.chartArea[axis.start]; + + expect(scale.getPixelForValue(0, 0, 0)).toBe(start); + expect(scale.getPixelForValue(lastTick, 0, 0)).toBe(end); + expect(scale.getPixelForValue(firstTick, 0, 0)).toBe(start - sign * fontSize, 4); + + expect(scale.getValueForPixel(start)).toBeCloseTo(0, 4); + expect(scale.getValueForPixel(end)).toBeCloseTo(lastTick, 4); + expect(scale.getValueForPixel(start - sign * fontSize)).toBeCloseTo(firstTick, 4); + }); + }); }); }); });