Fix the rounding issue of floating point numbers in category scale

- Remove `Math.round` in the category scale code - Add `helpers.alignLinePixel` to align grid/tick/axis border lines - Fix grid/tick/axis border line calculation - Add a check of the width of the axis border - Refactor core.scale code
chartjs · Dec 4, 2018 · 1e80895 · 1e80895
1 parent 3cb2d70
commit 1e80895
Show file tree

Hide file tree

Showing 17 changed files with 139 additions and 93 deletions.
diff --git a/src/core/core.helpers.js b/src/core/core.helpers.js
@@ -214,9 +214,27 @@ module.exports = function() {
 	helpers.distanceBetweenPoints = function(pt1, pt2) {
 		return Math.sqrt(Math.pow(pt2.x - pt1.x, 2) + Math.pow(pt2.y - pt1.y, 2));
 	};
+
+	/**
+	 * Provided for backward compatibility, not available anymore
+	 * @function Chart.helpers.indexOf
+	 * @deprecated since version 2.8.0
+	 * @todo remove at version 3
+	 */
 	helpers.aliasPixel = function(pixelWidth) {
 		return (pixelWidth % 2 === 0) ? 0 : 0.5;
 	};
+
+	/**
+	 * Returns the aligned line pixel value to avoid anti-aliasing blur
+	 * @param {Number} linePixel - A line pixel value.
+	 * @param {Number} lineWidth - A line width.
+	 * @returns {Number} The aligned line pixel value.
+	 */
+	helpers.alignLinePixel = function(linePixel, lineWidth) {
+		return Math.floor(lineWidth) === lineWidth ? Math.round(linePixel - lineWidth / 2) + lineWidth / 2 : linePixel;
+	};
+
 	helpers.splineCurve = function(firstPoint, middlePoint, afterPoint, t) {
 		// Props to Rob Spencer at scaled innovation for his post on splining between points
 		// http://scaledinnovation.com/analytics/splines/aboutSplines.html

diff --git a/src/core/core.scale.js b/src/core/core.scale.js
@@ -580,7 +580,7 @@ module.exports = Element.extend({
 				pixel += tickWidth / 2;
 			}
 
-			var finalVal = me.left + Math.round(pixel);
+			var finalVal = me.left + pixel;
 			finalVal += me.isFullWidth() ? me.margins.left : 0;
 			return finalVal;
 		}
@@ -598,7 +598,7 @@ module.exports = Element.extend({
 			var innerWidth = me.width - (me.paddingLeft + me.paddingRight);
 			var valueOffset = (innerWidth * decimal) + me.paddingLeft;
 
-			var finalVal = me.left + Math.round(valueOffset);
+			var finalVal = me.left + valueOffset;
 			finalVal += me.isFullWidth() ? me.margins.left : 0;
 			return finalVal;
 		}
@@ -689,15 +689,19 @@ module.exports = Element.extend({
 		var optionMajorTicks = options.ticks.major || optionTicks;
 		var gridLines = options.gridLines;
 		var scaleLabel = options.scaleLabel;
+		var position = options.position;
 
 		var isRotated = me.labelRotation !== 0;
+		var isMirrored = optionTicks.mirror;
 		var isHorizontal = me.isHorizontal();
 
 		var ticks = optionTicks.autoSkip ? me._autoSkip(me.getTicks()) : me.getTicks();
 		var tickFontColor = helpers.valueOrDefault(optionTicks.fontColor, globalDefaults.defaultFontColor);
 		var tickFont = parseFontOptions(optionTicks);
 		var majorTickFontColor = helpers.valueOrDefault(optionMajorTicks.fontColor, globalDefaults.defaultFontColor);
 		var majorTickFont = parseFontOptions(optionMajorTicks);
+		var tickPadding = optionTicks.padding;
+		var labelOffset = optionTicks.labelOffset;
 
 		var tl = gridLines.drawTicks ? gridLines.tickMarkLength : 0;
 
@@ -708,11 +712,28 @@ module.exports = Element.extend({
 
 		var itemsToDraw = [];
 
-		var axisWidth = helpers.valueAtIndexOrDefault(me.options.gridLines.lineWidth, 0);
-		var xTickStart = options.position === 'right' ? me.left : me.right - axisWidth - tl;
-		var xTickEnd = options.position === 'right' ? me.left + tl : me.right;
-		var yTickStart = options.position === 'bottom' ? me.top + axisWidth : me.bottom - tl - axisWidth;
-		var yTickEnd = options.position === 'bottom' ? me.top + axisWidth + tl : me.bottom + axisWidth;
+		var axisWidth = helpers.valueAtIndexOrDefault(gridLines.lineWidth, 0, 0);
+		var alignedLeft = helpers.alignLinePixel(me.left, axisWidth, position === 'right');
+		var alignedRight = helpers.alignLinePixel(me.right, axisWidth);
+		var alignedTop = helpers.alignLinePixel(me.top, axisWidth, position === 'bottom');
+		var alignedBottom = helpers.alignLinePixel(me.bottom, axisWidth);
+
+		var xTickStart, xTickEnd, yTickStart, yTickEnd;
+		if (isHorizontal) {
+			if (position === 'top') {
+				yTickStart = me.bottom - tl;
+				yTickEnd = alignedBottom - axisWidth / 2;
+			} else {
+				yTickStart = alignedTop + axisWidth / 2;
+				yTickEnd = me.top + tl;
+			}
+		} else if (position === 'left') {
+			xTickStart = me.right - tl;
+			xTickEnd = alignedRight - axisWidth / 2;
+		} else {
+			xTickStart = alignedLeft + axisWidth / 2;
+			xTickEnd = me.left + tl;
+		}
 
 		var epsilon = 0.0000001; // 0.0000001 is margin in pixels for Accumulated error.
 
@@ -738,66 +759,60 @@ module.exports = Element.extend({
 			}
 
 			// Common properties
-			var tx1, ty1, tx2, ty2, x1, y1, x2, y2, labelX, labelY;
-			var textAlign = 'middle';
+			var tx1, ty1, tx2, ty2, x1, y1, x2, y2, labelX, labelY, textAlign;
 			var textBaseline = 'middle';
-			var tickPadding = optionTicks.padding;
+			var lineValue = getPixelForGridLine(me, index, gridLines.offsetGridLines);
 
 			if (isHorizontal) {
 				var labelYOffset = tl + tickPadding;
 
-				if (options.position === 'bottom') {
-					// bottom
-					textBaseline = !isRotated ? 'top' : 'middle';
-					textAlign = !isRotated ? 'center' : 'right';
-					labelY = me.top + labelYOffset;
-				} else {
-					// top
-					textBaseline = !isRotated ? 'bottom' : 'middle';
-					textAlign = !isRotated ? 'center' : 'left';
-					labelY = me.bottom - labelYOffset;
-				}
-
-				var xLineValue = getPixelForGridLine(me, index, gridLines.offsetGridLines);
-				if (xLineValue < me.left - epsilon) {
+				if (lineValue < me.left - epsilon) {
 					lineColor = 'rgba(0,0,0,0)';
 				}
-				xLineValue += helpers.aliasPixel(lineWidth);
-
-				labelX = me.getPixelForTick(index) + optionTicks.labelOffset; // x values for optionTicks (need to consider offsetLabel option)
 
-				tx1 = tx2 = x1 = x2 = xLineValue;
+				tx1 = tx2 = x1 = x2 = helpers.alignLinePixel(lineValue, lineWidth, lineValue === chartArea.right);
 				ty1 = yTickStart;
 				ty2 = yTickEnd;
 				y1 = chartArea.top;
-				y2 = chartArea.bottom + axisWidth;
-			} else {
-				var isLeft = options.position === 'left';
-				var labelXOffset;
+				y2 = chartArea.bottom;
+
+				labelX = me.getPixelForTick(index) + labelOffset; // x values for optionTicks (need to consider offsetLabel option)
 
-				if (optionTicks.mirror) {
-					textAlign = isLeft ? 'left' : 'right';
-					labelXOffset = tickPadding;
+				if (position === 'top') {
+					y1 = helpers.alignLinePixel(y1, axisWidth) + axisWidth / 2;
+					textBaseline = !isRotated ? 'bottom' : 'middle';
+					textAlign = !isRotated ? 'center' : 'left';
+					labelY = me.bottom - labelYOffset;
 				} else {
-					textAlign = isLeft ? 'right' : 'left';
-					labelXOffset = tl + tickPadding;
+					y2 = helpers.alignLinePixel(y2, axisWidth, true) - axisWidth / 2;
+					textBaseline = !isRotated ? 'top' : 'middle';
+					textAlign = !isRotated ? 'center' : 'right';
+					labelY = me.top + labelYOffset;
 				}
+			} else {
+				var labelXOffset = (isMirrored ? 0 : tl) + tickPadding;
 
-				labelX = isLeft ? me.right - labelXOffset : me.left + labelXOffset;
-
-				var yLineValue = getPixelForGridLine(me, index, gridLines.offsetGridLines);
-				if (yLineValue < me.top - epsilon) {
+				if (lineValue < me.top - epsilon) {
 					lineColor = 'rgba(0,0,0,0)';
 				}
-				yLineValue += helpers.aliasPixel(lineWidth);
-
-				labelY = me.getPixelForTick(index) + optionTicks.labelOffset;
 
 				tx1 = xTickStart;
 				tx2 = xTickEnd;
 				x1 = chartArea.left;
-				x2 = chartArea.right + axisWidth;
-				ty1 = ty2 = y1 = y2 = yLineValue;
+				x2 = chartArea.right;
+				ty1 = ty2 = y1 = y2 = helpers.alignLinePixel(lineValue, lineWidth, lineValue === chartArea.bottom);
+
+				labelY = me.getPixelForTick(index) + labelOffset;
+
+				if (position === 'left') {
+					x1 = helpers.alignLinePixel(x1, axisWidth) + axisWidth / 2;
+					textAlign = isMirrored ? 'left' : 'right';
+					labelX = me.right - labelXOffset;
+				} else {
+					x2 = helpers.alignLinePixel(x2, axisWidth, true) - axisWidth / 2;
+					textAlign = isMirrored ? 'right' : 'left';
+					labelX = me.left + labelXOffset;
+				}
 			}
 
 			itemsToDraw.push({
@@ -867,7 +882,7 @@ module.exports = Element.extend({
 				if (helpers.isArray(label)) {
 					var lineCount = label.length;
 					var lineHeight = tickFont.size * 1.5;
-					var y = me.isHorizontal() ? 0 : -lineHeight * (lineCount - 1) / 2;
+					var y = isHorizontal ? 0 : -lineHeight * (lineCount - 1) / 2;
 
 					for (var i = 0; i < lineCount; ++i) {
 						// We just make sure the multiline element is a string here..
@@ -891,11 +906,11 @@ module.exports = Element.extend({
 
 			if (isHorizontal) {
 				scaleLabelX = me.left + ((me.right - me.left) / 2); // midpoint of the width
-				scaleLabelY = options.position === 'bottom'
+				scaleLabelY = position === 'bottom'
 					? me.bottom - halfLineHeight - scaleLabelPadding.bottom
 					: me.top + halfLineHeight + scaleLabelPadding.top;
 			} else {
-				var isLeft = options.position === 'left';
+				var isLeft = position === 'left';
 				scaleLabelX = isLeft
 					? me.left + halfLineHeight + scaleLabelPadding.top
 					: me.right - halfLineHeight - scaleLabelPadding.top;
@@ -916,28 +931,27 @@ module.exports = Element.extend({
 
 		if (gridLines.drawBorder) {
 			// Draw the line at the edge of the axis
-			context.lineWidth = helpers.valueAtIndexOrDefault(gridLines.lineWidth, 0);
-			context.strokeStyle = helpers.valueAtIndexOrDefault(gridLines.color, 0);
-			var x1 = me.left;
-			var x2 = me.right + axisWidth;
-			var y1 = me.top;
-			var y2 = me.bottom + axisWidth;
-
-			var aliasPixel = helpers.aliasPixel(context.lineWidth);
+			var firstLineWidth = axisWidth;
+			var lastLineWidth = helpers.valueAtIndexOrDefault(gridLines.lineWidth, ticks.length - 1, 0);
+			var x1 = helpers.alignLinePixel(me.left, firstLineWidth) - firstLineWidth / 2;
+			var x2 = helpers.alignLinePixel(me.right, lastLineWidth, true) + lastLineWidth / 2;
+			var y1 = helpers.alignLinePixel(me.top, firstLineWidth) - firstLineWidth / 2;
+			var y2 = helpers.alignLinePixel(me.bottom, lastLineWidth, true) + lastLineWidth / 2;
+
 			if (isHorizontal) {
-				y1 = y2 = options.position === 'top' ? me.bottom : me.top;
-				y1 += aliasPixel;
-				y2 += aliasPixel;
+				y1 = y2 = position === 'top' ? alignedBottom : alignedTop;
 			} else {
-				x1 = x2 = options.position === 'left' ? me.right : me.left;
-				x1 += aliasPixel;
-				x2 += aliasPixel;
+				x1 = x2 = position === 'left' ? alignedRight : alignedLeft;
 			}
 
-			context.beginPath();
-			context.moveTo(x1, y1);
-			context.lineTo(x2, y2);
-			context.stroke();
+			if (axisWidth) {
+				context.lineWidth = axisWidth;
+				context.strokeStyle = helpers.valueAtIndexOrDefault(gridLines.color, 0);
+				context.beginPath();
+				context.moveTo(x1, y1);
+				context.lineTo(x2, y2);
+				context.stroke();
+			}
 		}
 	}
 });
diff --git a/src/core/core.tooltip.js b/src/core/core.tooltip.js
@@ -124,8 +124,8 @@ var positioners = {
 		}
 
 		return {
-			x: Math.round(x / count),
-			y: Math.round(y / count)
+			x: x / count,
+			y: y / count
 		};
 	},
 
@@ -619,8 +619,8 @@ var exports = module.exports = Element.extend({
 			model.footer = me.getFooter(tooltipItems, data);
 
 			// Initial positioning and colors
-			model.x = Math.round(tooltipPosition.x);
-			model.y = Math.round(tooltipPosition.y);
+			model.x = tooltipPosition.x;
+			model.y = tooltipPosition.y;
 			model.caretPadding = opts.caretPadding;
 			model.labelColors = labelColors;
 			model.labelTextColors = labelTextColors;

diff --git a/src/scales/scale.category.js b/src/scales/scale.category.js
@@ -90,7 +90,7 @@ module.exports = function() {
 					widthOffset += (valueWidth / 2);
 				}
 
-				return me.left + Math.round(widthOffset);
+				return me.left + widthOffset;
 			}
 			var valueHeight = me.height / offsetAmt;
 			var heightOffset = (valueHeight * (index - me.minIndex));
@@ -99,7 +99,7 @@ module.exports = function() {
 				heightOffset += (valueHeight / 2);
 			}
 
-			return me.top + Math.round(heightOffset);
+			return me.top + heightOffset;
 		},
 		getPixelForTick: function(index) {
 			return this.getPixelForValue(this.ticks[index], index + this.minIndex, null);

diff --git a/src/scales/scale.radialLinear.js b/src/scales/scale.radialLinear.js
@@ -477,8 +477,8 @@ module.exports = function(Chart) {
 			var me = this;
 			var thisAngle = me.getIndexAngle(index) - (Math.PI / 2);
 			return {
-				x: Math.round(Math.cos(thisAngle) * distanceFromCenter) + me.xCenter,
-				y: Math.round(Math.sin(thisAngle) * distanceFromCenter) + me.yCenter
+				x: Math.cos(thisAngle) * distanceFromCenter + me.xCenter,
+				y: Math.sin(thisAngle) * distanceFromCenter + me.yCenter
 			};
 		},
 		getPointPositionForValue: function(index, value) {

diff --git a/test/fixtures/controller.line/point-style.png b/test/fixtures/controller.line/point-style.png
diff --git a/test/fixtures/controller.radar/point-style.png b/test/fixtures/controller.radar/point-style.png
diff --git a/test/fixtures/core.scale/tick-drawing.png b/test/fixtures/core.scale/tick-drawing.png
diff --git a/test/fixtures/core.tooltip/opacity.png b/test/fixtures/core.tooltip/opacity.png
diff --git a/test/fixtures/scale.radialLinear/border-dash.png b/test/fixtures/scale.radialLinear/border-dash.png
diff --git a/test/fixtures/scale.radialLinear/indexable-gridlines.png b/test/fixtures/scale.radialLinear/indexable-gridlines.png
diff --git a/test/specs/core.helpers.tests.js b/test/specs/core.helpers.tests.js
@@ -280,6 +280,13 @@ describe('Core helper tests', function() {
 		});
 	});
 
+	it('should get the aligned line pixel value', function() {
+		expect(helpers.alignLinePixel(0, 0.5)).toEqual(0);
+		expect(helpers.alignLinePixel(0, 1)).toEqual(0.5);
+		expect(helpers.alignLinePixel(0.5, 1)).toEqual(0.5);
+		expect(helpers.alignLinePixel(0, 2)).toEqual(0);
+	});
+
 	it('should spline curves', function() {
 		expect(helpers.splineCurve({
 			x: 0,

diff --git a/test/specs/core.scale.tests.js b/test/specs/core.scale.tests.js
@@ -29,7 +29,7 @@ describe('Core.scale', function() {
 		labels: ['tick1', 'tick2', 'tick3', 'tick4', 'tick5'],
 		offsetGridLines: false,
 		offset: true,
-		expected: [51.5, 154.5, 256.5, 358.5, 461.5]
+		expected: [51.5, 153.5, 256.5, 358.5, 460.5]
 	}, {
 		labels: ['tick1', 'tick2', 'tick3', 'tick4', 'tick5'],
 		offsetGridLines: true,
@@ -39,7 +39,7 @@ describe('Core.scale', function() {
 		labels: ['tick1', 'tick2', 'tick3', 'tick4', 'tick5'],
 		offsetGridLines: true,
 		offset: true,
-		expected: [0, 103, 205.5, 307.5, 410]
+		expected: [-0.5, 102.5, 204.5, 307.5, 409.5]
 	}, {
 		labels: ['tick1'],
 		offsetGridLines: false,
@@ -146,7 +146,7 @@ describe('Core.scale', function() {
 			chart.draw();
 
 			expect(yScale.ctx.getCalls().filter(function(x) {
-				return x.name === 'moveTo' && x.args[0] === 0;
+				return x.name === 'moveTo' && x.args[0] === 1;
 			}).map(function(x) {
 				return x.args[1];
 			})).toEqual(test.expected);

diff --git a/test/specs/core.tooltip.tests.js b/test/specs/core.tooltip.tests.js
@@ -145,7 +145,7 @@ describe('Core.Tooltip', function() {
 				}]
 			}));
 
-			expect(tooltip._view.x).toBeCloseToPixel(266);
+			expect(tooltip._view.x).toBeCloseToPixel(267);
 			expect(tooltip._view.y).toBeCloseToPixel(155);
 		});
 
@@ -343,7 +343,7 @@ describe('Core.Tooltip', function() {
 			}]
 		}));
 
-		expect(tooltip._view.x).toBeCloseToPixel(266);
+		expect(tooltip._view.x).toBeCloseToPixel(267);
 		expect(tooltip._view.y).toBeCloseToPixel(312);
 	});
 
@@ -576,7 +576,7 @@ describe('Core.Tooltip', function() {
 			}]
 		}));
 
-		expect(tooltip._view.x).toBeCloseToPixel(266);
+		expect(tooltip._view.x).toBeCloseToPixel(267);
 		expect(tooltip._view.y).toBeCloseToPixel(155);
 	});