Improve tick generation for linear scales (#5938)

* Improve tick generation for linear scales
* Simplify the tick generation code
* Refactor getTickLimit

Author: nagix

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');
@@ -133,20 +132,16 @@ module.exports = function(Chart) {
 			// Common base implementation to handle ticks.min, ticks.max, ticks.beginAtZero
 			this.handleTickRangeOptions();
 		},
-		getTickLimit: function() {
-			var maxTicks;
+		// Returns the maximum number of ticks based on the scale dimension
+		_computeTickLimit: function() {
 			var me = this;
-			var tickOpts = me.options.ticks;
+			var tickFont;
 
 			if (me.isHorizontal()) {
-				maxTicks = Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(me.width / 50));
-			} 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)));
+				return Math.ceil(me.width / 40);
 			}
-
-			return maxTicks;
+			tickFont = helpers.options._parseFont(me.options.ticks);
+			return Math.ceil(me.height / tickFont.lineHeight);
 		},
 		// Called after the ticks are built. We need
 		handleDirectionalChanges: function() {

src/scales/scale.linearbase.js

@@ -16,35 +16,41 @@ function generateTicks(generationOptions, dataRange) {
 	// for details.
 
 	var stepSize = generationOptions.stepSize;
+	var unit = stepSize || 1;
+	var maxNumSpaces = generationOptions.maxTicks - 1;
 	var min = generationOptions.min;
 	var max = generationOptions.max;
-	var spacing, precision, factor, niceRange, niceMin, niceMax, numSpaces;
-
-	if (stepSize && stepSize > 0) {
-		spacing = stepSize;
-	} else {
-		niceRange = helpers.niceNum(dataRange.max - dataRange.min, false);
-		spacing = helpers.niceNum(niceRange / (generationOptions.maxTicks - 1), true);
-
-		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 || 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;
 	}
+
 	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) {
 		// 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;
 		}
 	}
@@ -146,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() {
@@ -155,7 +186,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();

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

src/scales/scale.radialLinear.js

@@ -357,11 +357,9 @@ module.exports = function(Chart) {
 			// Common base implementation to handle ticks.min, ticks.max, ticks.beginAtZero
 			me.handleTickRangeOptions();
 		},
-		getTickLimit: function() {
-			var opts = this.options;
-			var tickOpts = opts.ticks;
-			var tickBackdropHeight = getTickBackdropHeight(opts);
-			return Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(this.drawingArea / tickBackdropHeight));
+		// 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;

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() {
@@ -749,6 +749,53 @@ 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']);
+
+		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() {
 		var chart = window.acquireChart({
 			type: 'bar',

test/specs/scale.radialLinear.tests.js

@@ -282,6 +282,43 @@ 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']);
+
+		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() {
 		var chart = window.acquireChart({
 			type: 'radar',