From 48aecb47449da883b29c1ad61ee4e1cfff1bd74d Mon Sep 17 00:00:00 2001 From: Akihiko Kusanagi Date: Thu, 27 Dec 2018 01:20:14 +0800 Subject: [PATCH 1/3] Improve tick generation for linear scales --- src/scales/scale.linear.js | 4 +-- src/scales/scale.linearbase.js | 26 +++++++++++++--- src/scales/scale.logarithmic.js | 4 --- test/specs/scale.linear.tests.js | 41 ++++++++++++++++++++++++++ test/specs/scale.radialLinear.tests.js | 31 +++++++++++++++++++ 5 files changed, 96 insertions(+), 10 deletions(-) diff --git a/src/scales/scale.linear.js b/src/scales/scale.linear.js index a980615c5d1..228ce099849 100644 --- a/src/scales/scale.linear.js +++ b/src/scales/scale.linear.js @@ -139,11 +139,11 @@ module.exports = function(Chart) { var tickOpts = me.options.ticks; if (me.isHorizontal()) { - maxTicks = Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(me.width / 50)); + maxTicks = Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(me.width / 40)); } else { // The factor of 2 used to scale the font size has been experimentally determined. var tickFontSize = helpers.valueOrDefault(tickOpts.fontSize, defaults.global.defaultFontSize); - maxTicks = Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(me.height / (2 * tickFontSize))); + maxTicks = Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(me.height / (1.5 * tickFontSize))); } return maxTicks; diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index e2898e1ea38..490bc2ab6b8 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -18,13 +18,30 @@ function generateTicks(generationOptions, dataRange) { var stepSize = generationOptions.stepSize; var min = generationOptions.min; var max = generationOptions.max; - var spacing, precision, factor, niceRange, niceMin, niceMax, numSpaces; + var spacing, precision, factor, niceMin, niceMax, numSpaces, maxNumSpaces; if (stepSize && stepSize > 0) { spacing = stepSize; + if (generationOptions.maxTicksLimit) { + maxNumSpaces = generationOptions.maxTicksLimit - 1; + // spacing is set to stepSize multiplied by a nice number of + // Math.ceil((max - min) / maxNumSpaces / stepSize) = num of steps that should be grouped + spacing *= helpers.niceNum(Math.ceil((dataRange.max - dataRange.min) / maxNumSpaces / stepSize)); + numSpaces = Math.ceil(dataRange.max / spacing) - Math.floor(dataRange.min / spacing); + if (numSpaces > maxNumSpaces) { + // If the calculated num of spaces exceeds maxNumSpaces, recalculate it + spacing = helpers.niceNum(Math.ceil(numSpaces * spacing / maxNumSpaces / stepSize)) * stepSize; + } + } } else { - niceRange = helpers.niceNum(dataRange.max - dataRange.min, false); - spacing = helpers.niceNum(niceRange / (generationOptions.maxTicks - 1), true); + maxNumSpaces = generationOptions.maxTicks - 1; + // spacing is set to a nice number of (max - min) / maxNumSpaces + spacing = helpers.niceNum((dataRange.max - dataRange.min) / maxNumSpaces); + numSpaces = Math.ceil(dataRange.max / spacing) - Math.floor(dataRange.min / spacing); + if (numSpaces > maxNumSpaces) { + // If the calculated num of spaces exceeds maxNumSpaces, recalculate it + spacing = helpers.niceNum(numSpaces * spacing / maxNumSpaces); + } precision = generationOptions.precision; if (!helpers.isNullOrUndef(precision)) { @@ -155,7 +172,7 @@ module.exports = function(Chart) { var tickOpts = opts.ticks; // Figure out what the max number of ticks we can support it is based on the size of - // the axis area. For now, we say that the minimum tick spacing in pixels must be 50 + // the axis area. For now, we say that the minimum tick spacing in pixels must be 40 // We also limit the maximum number of ticks to 11 which gives a nice 10 squares on // the graph. Make sure we always have at least 2 ticks var maxTicks = me.getTickLimit(); @@ -163,6 +180,7 @@ module.exports = function(Chart) { var numericGeneratorOptions = { maxTicks: maxTicks, + maxTicksLimit: tickOpts.maxTicksLimit, min: tickOpts.min, max: tickOpts.max, precision: tickOpts.precision, diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 0365568e772..240df0faf46 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -15,10 +15,6 @@ function generateTicks(generationOptions, dataRange) { var ticks = []; var valueOrDefault = helpers.valueOrDefault; - // Figure out what the max number of ticks we can support it is based on the size of - // the axis area. For now, we say that the minimum tick spacing in pixels must be 50 - // We also limit the maximum number of ticks to 11 which gives a nice 10 squares on - // the graph var tickVal = valueOrDefault(generationOptions.min, Math.pow(10, Math.floor(helpers.log10(dataRange.min)))); var endExp = Math.floor(helpers.log10(dataRange.max)); diff --git a/test/specs/scale.linear.tests.js b/test/specs/scale.linear.tests.js index ad35e2a4347..9b866a5a762 100644 --- a/test/specs/scale.linear.tests.js +++ b/test/specs/scale.linear.tests.js @@ -749,6 +749,47 @@ describe('Linear Scale', function() { expect(chart.scales.yScale0.ticks).toEqual(['0.06', '0.05', '0.04', '0.03', '0.02', '0.01', '0']); }); + it('Should correctly limit the maximum number of ticks', function() { + var chart = window.acquireChart({ + type: 'bar', + data: { + labels: ['a', 'b'], + datasets: [{ + data: [0.5, 2.5] + }] + }, + options: { + scales: { + yAxes: [{ + id: 'yScale' + }] + } + } + }); + + expect(chart.scales.yScale.ticks).toEqual(['2.5', '2.0', '1.5', '1.0', '0.5']); + + chart.options.scales.yAxes[0].ticks.maxTicksLimit = 11; + chart.update(); + + expect(chart.scales.yScale.ticks).toEqual(['2.5', '2.0', '1.5', '1.0', '0.5']); + + chart.options.scales.yAxes[0].ticks.maxTicksLimit = 21; + chart.update(); + + expect(chart.scales.yScale.ticks).toEqual([ + '2.5', '2.4', '2.3', '2.2', '2.1', '2.0', '1.9', '1.8', '1.7', '1.6', + '1.5', '1.4', '1.3', '1.2', '1.1', '1.0', '0.9', '0.8', '0.7', '0.6', + '0.5' + ]); + + chart.options.scales.yAxes[0].ticks.maxTicksLimit = 11; + chart.options.scales.yAxes[0].ticks.stepSize = 0.01; + chart.update(); + + expect(chart.scales.yScale.ticks).toEqual(['2.5', '2.0', '1.5', '1.0', '0.5']); + }); + it('Should build labels using the user supplied callback', function() { var chart = window.acquireChart({ type: 'bar', diff --git a/test/specs/scale.radialLinear.tests.js b/test/specs/scale.radialLinear.tests.js index 49ef34b46d6..b564564ec66 100644 --- a/test/specs/scale.radialLinear.tests.js +++ b/test/specs/scale.radialLinear.tests.js @@ -282,6 +282,37 @@ describe('Test the radial linear scale', function() { expect(chart.scale.end).toBe(0); }); + it('Should correctly limit the maximum number of ticks', function() { + var chart = window.acquireChart({ + type: 'radar', + data: { + labels: ['label1', 'label2', 'label3'], + datasets: [{ + data: [0.5, 1.5, 2.5] + }] + }, + options: { + scale: { + pointLabels: { + display: false + } + } + } + }); + + expect(chart.scale.ticks).toEqual(['0.5', '1.0', '1.5', '2.0', '2.5']); + + chart.options.scale.ticks.maxTicksLimit = 11; + chart.update(); + + expect(chart.scale.ticks).toEqual(['0.5', '1.0', '1.5', '2.0', '2.5']); + + chart.options.scale.ticks.stepSize = 0.01; + chart.update(); + + expect(chart.scale.ticks).toEqual(['0.5', '1.0', '1.5', '2.0', '2.5']); + }); + it('Should build labels using the user supplied callback', function() { var chart = window.acquireChart({ type: 'radar', From e33e1ff55b9df4012926172fed1bdf19802c9af7 Mon Sep 17 00:00:00 2001 From: Akihiko Kusanagi Date: Sat, 29 Dec 2018 00:03:44 +0800 Subject: [PATCH 2/3] Simplify the tick generation code --- src/scales/scale.linear.js | 21 +++++---- src/scales/scale.linearbase.js | 60 +++++++++++--------------- src/scales/scale.radialLinear.js | 19 ++++++-- test/specs/scale.linear.tests.js | 8 +++- test/specs/scale.radialLinear.tests.js | 6 +++ 5 files changed, 66 insertions(+), 48 deletions(-) diff --git a/src/scales/scale.linear.js b/src/scales/scale.linear.js index 228ce099849..36655559ad3 100644 --- a/src/scales/scale.linear.js +++ b/src/scales/scale.linear.js @@ -1,6 +1,5 @@ 'use strict'; -var defaults = require('../core/core.defaults'); var helpers = require('../helpers/index'); var scaleService = require('../core/core.scaleService'); var Ticks = require('../core/core.ticks'); @@ -134,16 +133,22 @@ module.exports = function(Chart) { this.handleTickRangeOptions(); }, getTickLimit: function() { - var maxTicks; var me = this; var tickOpts = me.options.ticks; - - if (me.isHorizontal()) { - maxTicks = Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(me.width / 40)); + var stepSize = tickOpts.stepSize; + var maxTicksLimit = tickOpts.maxTicksLimit; + var maxTicks, tickFont; + + if (stepSize > 0) { + maxTicks = Math.ceil(me.max / stepSize) - Math.floor(me.min / stepSize) + 1; + } else if (me.isHorizontal()) { + maxTicks = Math.ceil(me.width / 40); } else { - // The factor of 2 used to scale the font size has been experimentally determined. - var tickFontSize = helpers.valueOrDefault(tickOpts.fontSize, defaults.global.defaultFontSize); - maxTicks = Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(me.height / (1.5 * tickFontSize))); + tickFont = helpers.options._parseFont(tickOpts); + maxTicks = Math.ceil(me.height / tickFont.lineHeight); + } + if (maxTicksLimit || !(stepSize > 0)) { + maxTicks = Math.min(maxTicksLimit || 11, maxTicks); } return maxTicks; diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index 490bc2ab6b8..54ed7ca806d 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -16,52 +16,41 @@ function generateTicks(generationOptions, dataRange) { // for details. var stepSize = generationOptions.stepSize; + var unit = stepSize > 0 ? stepSize : 1; + var maxNumSpaces = generationOptions.maxTicks - 1; var min = generationOptions.min; var max = generationOptions.max; - var spacing, precision, factor, niceMin, niceMax, numSpaces, maxNumSpaces; - - if (stepSize && stepSize > 0) { - spacing = stepSize; - if (generationOptions.maxTicksLimit) { - maxNumSpaces = generationOptions.maxTicksLimit - 1; - // spacing is set to stepSize multiplied by a nice number of - // Math.ceil((max - min) / maxNumSpaces / stepSize) = num of steps that should be grouped - spacing *= helpers.niceNum(Math.ceil((dataRange.max - dataRange.min) / maxNumSpaces / stepSize)); - numSpaces = Math.ceil(dataRange.max / spacing) - Math.floor(dataRange.min / spacing); - if (numSpaces > maxNumSpaces) { - // If the calculated num of spaces exceeds maxNumSpaces, recalculate it - spacing = helpers.niceNum(Math.ceil(numSpaces * spacing / maxNumSpaces / stepSize)) * stepSize; - } - } - } else { - maxNumSpaces = generationOptions.maxTicks - 1; - // spacing is set to a nice number of (max - min) / maxNumSpaces - spacing = helpers.niceNum((dataRange.max - dataRange.min) / maxNumSpaces); - numSpaces = Math.ceil(dataRange.max / spacing) - Math.floor(dataRange.min / spacing); - if (numSpaces > maxNumSpaces) { - // If the calculated num of spaces exceeds maxNumSpaces, recalculate it - spacing = helpers.niceNum(numSpaces * spacing / maxNumSpaces); - } - - precision = generationOptions.precision; - if (!helpers.isNullOrUndef(precision)) { - // If the user specified a precision, round to that number of decimal places - factor = Math.pow(10, precision); - spacing = Math.ceil(spacing * factor) / factor; - } + var precision = generationOptions.precision; + var spacing, factor, niceMin, niceMax, numSpaces; + + // spacing is set to a nice number of the dataRange divided by maxNumSpaces. + // stepSize is used as a minimum unit if it is specified. + spacing = helpers.niceNum((dataRange.max - dataRange.min) / maxNumSpaces / unit) * unit; + numSpaces = Math.ceil(dataRange.max / spacing) - Math.floor(dataRange.min / spacing); + if (numSpaces > maxNumSpaces) { + // If the calculated num of spaces exceeds maxNumSpaces, recalculate it + spacing = helpers.niceNum(numSpaces * spacing / maxNumSpaces / unit) * unit; } - // If a precision is not specified, calculate factor based on spacing - if (!factor) { + + if (!(stepSize > 0) && !helpers.isNullOrUndef(precision)) { + // If the user specified a precision, round to that number of decimal places + factor = Math.pow(10, precision); + spacing = Math.ceil(spacing * factor) / factor; + } else { + // If a precision is not specified, calculate factor based on spacing factor = Math.pow(10, helpers.decimalPlaces(spacing)); } + niceMin = Math.floor(dataRange.min / spacing) * spacing; niceMax = Math.ceil(dataRange.max / spacing) * spacing; // If min, max and stepSize is set and they make an evenly spaced scale use it. - if (!helpers.isNullOrUndef(min) && !helpers.isNullOrUndef(max) && stepSize) { + if (stepSize > 0) { // If very close to our whole number, use it. - if (helpers.almostWhole((max - min) / stepSize, spacing / 1000)) { + if (!helpers.isNullOrUndef(min) && helpers.almostWhole(min / spacing, spacing / 1000)) { niceMin = min; + } + if (!helpers.isNullOrUndef(max) && helpers.almostWhole(max / spacing, spacing / 1000)) { niceMax = max; } } @@ -180,7 +169,6 @@ module.exports = function(Chart) { var numericGeneratorOptions = { maxTicks: maxTicks, - maxTicksLimit: tickOpts.maxTicksLimit, min: tickOpts.min, max: tickOpts.max, precision: tickOpts.precision, diff --git a/src/scales/scale.radialLinear.js b/src/scales/scale.radialLinear.js index f0c38317e8a..aaeb33cd10b 100644 --- a/src/scales/scale.radialLinear.js +++ b/src/scales/scale.radialLinear.js @@ -358,10 +358,23 @@ module.exports = function(Chart) { me.handleTickRangeOptions(); }, getTickLimit: function() { - var opts = this.options; + var me = this; + var opts = me.options; var tickOpts = opts.ticks; - var tickBackdropHeight = getTickBackdropHeight(opts); - return Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(this.drawingArea / tickBackdropHeight)); + var stepSize = tickOpts.stepSize; + var maxTicksLimit = tickOpts.maxTicksLimit; + var maxTicks; + + if (stepSize > 0) { + maxTicks = Math.ceil(me.max / stepSize) - Math.floor(me.min / stepSize) + 1; + } else { + maxTicks = Math.ceil(me.drawingArea / getTickBackdropHeight(opts)); + } + if (maxTicksLimit || !(stepSize > 0)) { + maxTicks = Math.min(maxTicksLimit || 11, maxTicks); + } + + return maxTicks; }, convertTicksToLabels: function() { var me = this; diff --git a/test/specs/scale.linear.tests.js b/test/specs/scale.linear.tests.js index 9b866a5a762..a186c6cc7fe 100644 --- a/test/specs/scale.linear.tests.js +++ b/test/specs/scale.linear.tests.js @@ -570,7 +570,7 @@ describe('Linear Scale', function() { expect(chart.scales.yScale0).not.toEqual(undefined); // must construct expect(chart.scales.yScale0.min).toBe(1); expect(chart.scales.yScale0.max).toBe(11); - expect(chart.scales.yScale0.ticks).toEqual(['11', '9', '7', '5', '3', '1']); + expect(chart.scales.yScale0.ticks).toEqual(['11', '10', '8', '6', '4', '2', '1']); }); it('Should create decimal steps if stepSize is a decimal number', function() { @@ -788,6 +788,12 @@ describe('Linear Scale', function() { chart.update(); expect(chart.scales.yScale.ticks).toEqual(['2.5', '2.0', '1.5', '1.0', '0.5']); + + chart.options.scales.yAxes[0].ticks.min = 0.3; + chart.options.scales.yAxes[0].ticks.max = 2.8; + chart.update(); + + expect(chart.scales.yScale.ticks).toEqual(['2.8', '2.5', '2.0', '1.5', '1.0', '0.5', '0.3']); }); it('Should build labels using the user supplied callback', function() { diff --git a/test/specs/scale.radialLinear.tests.js b/test/specs/scale.radialLinear.tests.js index b564564ec66..2102bfaefc9 100644 --- a/test/specs/scale.radialLinear.tests.js +++ b/test/specs/scale.radialLinear.tests.js @@ -311,6 +311,12 @@ describe('Test the radial linear scale', function() { chart.update(); expect(chart.scale.ticks).toEqual(['0.5', '1.0', '1.5', '2.0', '2.5']); + + chart.options.scale.ticks.min = 0.3; + chart.options.scale.ticks.max = 2.8; + chart.update(); + + expect(chart.scale.ticks).toEqual(['0.3', '0.5', '1.0', '1.5', '2.0', '2.5', '2.8']); }); it('Should build labels using the user supplied callback', function() { From e1756453d8cd5e850dd4f25eba1ac54afca2ded8 Mon Sep 17 00:00:00 2001 From: Akihiko Kusanagi Date: Sun, 30 Dec 2018 00:10:21 +0800 Subject: [PATCH 3/3] Refactor getTickLimit --- src/scales/scale.linear.js | 26 +++++++-------------- src/scales/scale.linearbase.js | 39 ++++++++++++++++++++++++++------ src/scales/scale.radialLinear.js | 21 +++-------------- 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/scales/scale.linear.js b/src/scales/scale.linear.js index 36655559ad3..60a291e626f 100644 --- a/src/scales/scale.linear.js +++ b/src/scales/scale.linear.js @@ -132,26 +132,16 @@ module.exports = function(Chart) { // Common base implementation to handle ticks.min, ticks.max, ticks.beginAtZero this.handleTickRangeOptions(); }, - getTickLimit: function() { + // Returns the maximum number of ticks based on the scale dimension + _computeTickLimit: function() { var me = this; - var tickOpts = me.options.ticks; - var stepSize = tickOpts.stepSize; - var maxTicksLimit = tickOpts.maxTicksLimit; - var maxTicks, tickFont; - - if (stepSize > 0) { - maxTicks = Math.ceil(me.max / stepSize) - Math.floor(me.min / stepSize) + 1; - } else if (me.isHorizontal()) { - maxTicks = Math.ceil(me.width / 40); - } else { - tickFont = helpers.options._parseFont(tickOpts); - maxTicks = Math.ceil(me.height / tickFont.lineHeight); - } - if (maxTicksLimit || !(stepSize > 0)) { - maxTicks = Math.min(maxTicksLimit || 11, maxTicks); - } + var tickFont; - return maxTicks; + if (me.isHorizontal()) { + return Math.ceil(me.width / 40); + } + tickFont = helpers.options._parseFont(me.options.ticks); + return Math.ceil(me.height / tickFont.lineHeight); }, // Called after the ticks are built. We need handleDirectionalChanges: function() { diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index 54ed7ca806d..8dfe15a08cb 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -16,7 +16,7 @@ function generateTicks(generationOptions, dataRange) { // for details. var stepSize = generationOptions.stepSize; - var unit = stepSize > 0 ? stepSize : 1; + var unit = stepSize || 1; var maxNumSpaces = generationOptions.maxTicks - 1; var min = generationOptions.min; var max = generationOptions.max; @@ -32,20 +32,20 @@ function generateTicks(generationOptions, dataRange) { spacing = helpers.niceNum(numSpaces * spacing / maxNumSpaces / unit) * unit; } - if (!(stepSize > 0) && !helpers.isNullOrUndef(precision)) { + if (stepSize || helpers.isNullOrUndef(precision)) { + // If a precision is not specified, calculate factor based on spacing + factor = Math.pow(10, helpers.decimalPlaces(spacing)); + } else { // If the user specified a precision, round to that number of decimal places factor = Math.pow(10, precision); spacing = Math.ceil(spacing * factor) / factor; - } else { - // If a precision is not specified, calculate factor based on spacing - factor = Math.pow(10, helpers.decimalPlaces(spacing)); } niceMin = Math.floor(dataRange.min / spacing) * spacing; niceMax = Math.ceil(dataRange.max / spacing) * spacing; // If min, max and stepSize is set and they make an evenly spaced scale use it. - if (stepSize > 0) { + if (stepSize) { // If very close to our whole number, use it. if (!helpers.isNullOrUndef(min) && helpers.almostWhole(min / spacing, spacing / 1000)) { niceMin = min; @@ -152,7 +152,32 @@ module.exports = function(Chart) { } } }, - getTickLimit: noop, + + getTickLimit: function() { + var me = this; + var tickOpts = me.options.ticks; + var stepSize = tickOpts.stepSize; + var maxTicksLimit = tickOpts.maxTicksLimit; + var maxTicks; + + if (stepSize) { + maxTicks = Math.ceil(me.max / stepSize) - Math.floor(me.min / stepSize) + 1; + } else { + maxTicks = me._computeTickLimit(); + maxTicksLimit = maxTicksLimit || 11; + } + + if (maxTicksLimit) { + maxTicks = Math.min(maxTicksLimit, maxTicks); + } + + return maxTicks; + }, + + _computeTickLimit: function() { + return Number.POSITIVE_INFINITY; + }, + handleDirectionalChanges: noop, buildTicks: function() { diff --git a/src/scales/scale.radialLinear.js b/src/scales/scale.radialLinear.js index aaeb33cd10b..bc8129845ce 100644 --- a/src/scales/scale.radialLinear.js +++ b/src/scales/scale.radialLinear.js @@ -357,24 +357,9 @@ module.exports = function(Chart) { // Common base implementation to handle ticks.min, ticks.max, ticks.beginAtZero me.handleTickRangeOptions(); }, - getTickLimit: function() { - var me = this; - var opts = me.options; - var tickOpts = opts.ticks; - var stepSize = tickOpts.stepSize; - var maxTicksLimit = tickOpts.maxTicksLimit; - var maxTicks; - - if (stepSize > 0) { - maxTicks = Math.ceil(me.max / stepSize) - Math.floor(me.min / stepSize) + 1; - } else { - maxTicks = Math.ceil(me.drawingArea / getTickBackdropHeight(opts)); - } - if (maxTicksLimit || !(stepSize > 0)) { - maxTicks = Math.min(maxTicksLimit || 11, maxTicks); - } - - return maxTicks; + // Returns the maximum number of ticks based on the scale dimension + _computeTickLimit: function() { + return Math.ceil(this.drawingArea / getTickBackdropHeight(this.options)); }, convertTicksToLabels: function() { var me = this;