Adjust the size of rectRounded/rectRot point to fit pointRadius

- Calculate the vertices of the shapes so that they are inscribed in the circle that has the radius of `pointRadius`
- Remove `translate()` and `rotate()` to fix the regression introduced by #5319
- Refactor `rectRounded` for better performance

src/helpers/helpers.canvas.js

@@ -2,6 +2,13 @@
 
 var helpers = require('./helpers.core');
 
+var PI = Math.PI;
+var RAD_PER_DEG = PI / 180;
+var DOUBLE_PI = PI * 2;
+var HALF_PI = PI / 2;
+var QUARTER_PI = PI / 4;
+var TWO_THIRDS_PI = PI * 2 / 3;
+
 /**
  * @namespace Chart.helpers.canvas
  */
@@ -27,20 +34,26 @@ var exports = module.exports = {
 	 */
 	roundedRect: function(ctx, x, y, width, height, radius) {
 		if (radius) {
-			// NOTE(SB) `epsilon` helps to prevent minor artifacts appearing
-			// on Chrome when `r` is exactly half the height or the width.
-			var epsilon = 0.0000001;
-			var r = Math.min(radius, (height / 2) - epsilon, (width / 2) - epsilon);
-
-			ctx.moveTo(x + r, y);
-			ctx.lineTo(x + width - r, y);
-			ctx.arcTo(x + width, y, x + width, y + r, r);
-			ctx.lineTo(x + width, y + height - r);
-			ctx.arcTo(x + width, y + height, x + width - r, y + height, r);
-			ctx.lineTo(x + r, y + height);
-			ctx.arcTo(x, y + height, x, y + height - r, r);
-			ctx.lineTo(x, y + r);
-			ctx.arcTo(x, y, x + r, y, r);
+			var r = Math.min(radius, height / 2, width / 2);
+			var left = x + r;
+			var top = y + r;
+			var right = x + width - r;
+			var bottom = y + height - r;
+
+			if (left < right && top < bottom) {
+				ctx.arc(left, top, r, -PI, -HALF_PI);
+				ctx.arc(right, top, r, -HALF_PI, 0);
+				ctx.arc(right, bottom, r, 0, HALF_PI);
+				ctx.arc(left, bottom, r, HALF_PI, PI);
+			} else if (left < right) {
+				ctx.arc(left, top, r, HALF_PI, PI + HALF_PI);
+				ctx.arc(right, top, r, -HALF_PI, HALF_PI);
+			} else if (top < bottom) {
+				ctx.arc(left, top, r, -PI, 0);
+				ctx.arc(left, bottom, r, 0, PI);
+			} else {
+				ctx.arc(left, top, r, 0, DOUBLE_PI);
+			}
 			ctx.closePath();
 			ctx.moveTo(x, y);
 		} else {
@@ -49,8 +62,8 @@ var exports = module.exports = {
 	},
 
 	drawPoint: function(ctx, style, radius, x, y, rotation) {
-		var type, edgeLength, xOffset, yOffset, height, size;
-		rotation = rotation || 0;
+		var type, xOffset, yOffset, size, cornerRadius;
+		var rad = (rotation || 0) * RAD_PER_DEG;
 
 		if (style && typeof style === 'object') {
 			type = style.toString();
@@ -64,88 +77,95 @@ var exports = module.exports = {
 			return;
 		}
 
-		ctx.save();
-		ctx.translate(x, y);
-		ctx.rotate(rotation * Math.PI / 180);
 		ctx.beginPath();
 
 		switch (style) {
 		// Default includes circle
 		default:
-			ctx.arc(0, 0, radius, 0, Math.PI * 2);
+			ctx.arc(x, y, radius, 0, DOUBLE_PI);
 			ctx.closePath();
 			break;
 		case 'triangle':
-			edgeLength = 3 * radius / Math.sqrt(3);
-			height = edgeLength * Math.sqrt(3) / 2;
-			ctx.moveTo(-edgeLength / 2, height / 3);
-			ctx.lineTo(edgeLength / 2, height / 3);
-			ctx.lineTo(0, -2 * height / 3);
+			ctx.moveTo(x + Math.sin(rad) * radius, y - Math.cos(rad) * radius);
+			rad += TWO_THIRDS_PI;
+			ctx.lineTo(x + Math.sin(rad) * radius, y - Math.cos(rad) * radius);
+			rad += TWO_THIRDS_PI;
+			ctx.lineTo(x + Math.sin(rad) * radius, y - Math.cos(rad) * radius);
 			ctx.closePath();
 			break;
-		case 'rect':
-			size = 1 / Math.SQRT2 * radius;
-			ctx.rect(-size, -size, 2 * size, 2 * size);
-			break;
 		case 'rectRounded':
-			var offset = radius / Math.SQRT2;
-			var leftX = -offset;
-			var topY = -offset;
-			var sideSize = Math.SQRT2 * radius;
-
-			// NOTE(SB) the rounded rect implementation changed to use `arcTo`
-			// instead of `quadraticCurveTo` since it generates better results
-			// when rect is almost a circle. 0.425 (instead of 0.5) produces
-			// results visually closer to the previous impl.
-			this.roundedRect(ctx, leftX, topY, sideSize, sideSize, radius * 0.425);
+			// NOTE: the rounded rect implementation changed to use `arc` instead of
+			// `quadraticCurveTo` since it generates better results when rect is
+			// almost a circle. 0.516 (instead of 0.5) produces results with visually
+			// closer proportion to the previous impl and it is inscribed in the
+			// circle with `radius`. See #5597 and #5858 for more details.
+			cornerRadius = radius * 0.516;
+			size = radius - cornerRadius;
+			xOffset = Math.cos(rad + QUARTER_PI) * size;
+			yOffset = Math.sin(rad + QUARTER_PI) * size;
+			ctx.arc(x - xOffset, y - yOffset, cornerRadius, rad - PI, rad - HALF_PI);
+			ctx.arc(x + yOffset, y - xOffset, cornerRadius, rad - HALF_PI, rad);
+			ctx.arc(x + xOffset, y + yOffset, cornerRadius, rad, rad + HALF_PI);
+			ctx.arc(x - yOffset, y + xOffset, cornerRadius, rad + HALF_PI, rad + PI);
+			ctx.closePath();
 			break;
+		case 'rect':
+			if (!rotation) {
+				size = Math.SQRT1_2 * radius;
+				ctx.rect(x - size, y - size, 2 * size, 2 * size);
+				break;
+			}
+			rad += QUARTER_PI;
+			/* falls through */
 		case 'rectRot':
-			size = 1 / Math.SQRT2 * radius;
-			ctx.moveTo(-size, 0);
-			ctx.lineTo(0, size);
-			ctx.lineTo(size, 0);
-			ctx.lineTo(0, -size);
+			xOffset = Math.cos(rad) * radius;
+			yOffset = Math.sin(rad) * radius;
+			ctx.moveTo(x - xOffset, y - yOffset);
+			ctx.lineTo(x + yOffset, y - xOffset);
+			ctx.lineTo(x + xOffset, y + yOffset);
+			ctx.lineTo(x - yOffset, y + xOffset);
 			ctx.closePath();
 			break;
-		case 'cross':
-			ctx.moveTo(0, radius);
-			ctx.lineTo(0, -radius);
-			ctx.moveTo(-radius, 0);
-			ctx.lineTo(radius, 0);
-			break;
 		case 'crossRot':
-			xOffset = Math.cos(Math.PI / 4) * radius;
-			yOffset = Math.sin(Math.PI / 4) * radius;
-			ctx.moveTo(-xOffset, -yOffset);
-			ctx.lineTo(xOffset, yOffset);
-			ctx.moveTo(-xOffset, yOffset);
-			ctx.lineTo(xOffset, -yOffset);
+			rad += QUARTER_PI;
+			/* falls through */
+		case 'cross':
+			xOffset = Math.cos(rad) * radius;
+			yOffset = Math.sin(rad) * radius;
+			ctx.moveTo(x - xOffset, y - yOffset);
+			ctx.lineTo(x + xOffset, y + yOffset);
+			ctx.moveTo(x + yOffset, y - xOffset);
+			ctx.lineTo(x - yOffset, y + xOffset);
 			break;
 		case 'star':
-			ctx.moveTo(0, radius);
-			ctx.lineTo(0, -radius);
-			ctx.moveTo(-radius, 0);
-			ctx.lineTo(radius, 0);
-			xOffset = Math.cos(Math.PI / 4) * radius;
-			yOffset = Math.sin(Math.PI / 4) * radius;
-			ctx.moveTo(-xOffset, -yOffset);
-			ctx.lineTo(xOffset, yOffset);
-			ctx.moveTo(-xOffset, yOffset);
-			ctx.lineTo(xOffset, -yOffset);
+			xOffset = Math.cos(rad) * radius;
+			yOffset = Math.sin(rad) * radius;
+			ctx.moveTo(x - xOffset, y - yOffset);
+			ctx.lineTo(x + xOffset, y + yOffset);
+			ctx.moveTo(x + yOffset, y - xOffset);
+			ctx.lineTo(x - yOffset, y + xOffset);
+			rad += QUARTER_PI;
+			xOffset = Math.cos(rad) * radius;
+			yOffset = Math.sin(rad) * radius;
+			ctx.moveTo(x - xOffset, y - yOffset);
+			ctx.lineTo(x + xOffset, y + yOffset);
+			ctx.moveTo(x + yOffset, y - xOffset);
+			ctx.lineTo(x - yOffset, y + xOffset);
 			break;
 		case 'line':
-			ctx.moveTo(-radius, 0);
-			ctx.lineTo(radius, 0);
+			xOffset = Math.cos(rad) * radius;
+			yOffset = Math.sin(rad) * radius;
+			ctx.moveTo(x - xOffset, y - yOffset);
+			ctx.lineTo(x + xOffset, y + yOffset);
 			break;
 		case 'dash':
-			ctx.moveTo(0, 0);
-			ctx.lineTo(radius, 0);
+			ctx.moveTo(x, y);
+			ctx.lineTo(x + Math.cos(rad) * radius, y + Math.sin(rad) * radius);
 			break;
 		}
 
 		ctx.fill();
 		ctx.stroke();
-		ctx.restore();
 	},
 
 	clipArea: function(ctx, area) {

test/fixtures/element.point/rotation.js

@@ -0,0 +1,47 @@
+var datasets = ['circle', 'cross', 'crossRot', 'dash', 'line', 'rect', 'rectRounded', 'rectRot', 'star', 'triangle'].map(function(style, y) {
+	return {
+		pointStyle: style,
+		data: Array.apply(null, Array(17)).map(function(v, x) {
+			return {x: x, y: 10 - y};
+		})
+	};
+});
+
+var angles = Array.apply(null, Array(17)).map(function(v, i) {
+	return -180 + i * 22.5;
+});
+
+module.exports = {
+	config: {
+		type: 'bubble',
+		data: {
+			datasets: datasets
+		},
+		options: {
+			responsive: false,
+			legend: false,
+			title: false,
+			elements: {
+				point: {
+					rotation: angles,
+					radius: 10,
+					backgroundColor: '#00ff00',
+					borderColor: '#0000ff'
+				}
+			},
+			layout: {
+				padding: 20
+			},
+			scales: {
+				xAxes: [{display: false}],
+				yAxes: [{display: false}]
+			}
+		}
+	},
+	options: {
+		canvas: {
+			height: 256,
+			width: 512
+		}
+	}
+};

test/specs/element.point.tests.js

@@ -124,21 +124,12 @@ describe('Chart.elements.Point', function() {
 		}, {
 			name: 'setFillStyle',
 			args: ['rgba(0,0,0,0.1)']
-		}, {
-			name: 'save',
-			args: []
-		}, {
-			name: 'translate',
-			args: [10, 15]
-		}, {
-			name: 'rotate',
-			args: [0]
 		}, {
 			name: 'beginPath',
 			args: []
 		}, {
 			name: 'arc',
-			args: [0, 0, 2, 0, 2 * Math.PI]
+			args: [10, 15, 2, 0, 2 * Math.PI]
 		}, {
 			name: 'closePath',
 			args: [],
@@ -148,9 +139,6 @@ describe('Chart.elements.Point', function() {
 		}, {
 			name: 'stroke',
 			args: []
-		}, {
-			name: 'restore',
-			args: []
 		}]);
 	});
 

test/specs/helpers.canvas.tests.js

@@ -28,15 +28,45 @@ describe('Chart.helpers.canvas', function() {
 			helpers.canvas.roundedRect(context, 10, 20, 30, 40, 5);
 
 			expect(context.getCalls()).toEqual([
-				{name: 'moveTo', args: [15, 20]},
-				{name: 'lineTo', args: [35, 20]},
-				{name: 'arcTo', args: [40, 20, 40, 25, 5]},
-				{name: 'lineTo', args: [40, 55]},
-				{name: 'arcTo', args: [40, 60, 35, 60, 5]},
-				{name: 'lineTo', args: [15, 60]},
-				{name: 'arcTo', args: [10, 60, 10, 55, 5]},
-				{name: 'lineTo', args: [10, 25]},
-				{name: 'arcTo', args: [10, 20, 15, 20, 5]},
+				{name: 'arc', args: [15, 25, 5, -Math.PI, -Math.PI / 2]},
+				{name: 'arc', args: [35, 25, 5, -Math.PI / 2, 0]},
+				{name: 'arc', args: [35, 55, 5, 0, Math.PI / 2]},
+				{name: 'arc', args: [15, 55, 5, Math.PI / 2, Math.PI]},
+				{name: 'closePath', args: []},
+				{name: 'moveTo', args: [10, 20]}
+			]);
+		});
+		it('should optimize path if radius is exactly half of height', function() {
+			var context = window.createMockContext();
+
+			helpers.canvas.roundedRect(context, 10, 20, 40, 30, 15);
+
+			expect(context.getCalls()).toEqual([
+				{name: 'arc', args: [25, 35, 15, Math.PI / 2, Math.PI * 3 / 2]},
+				{name: 'arc', args: [35, 35, 15, -Math.PI / 2, Math.PI / 2]},
+				{name: 'closePath', args: []},
+				{name: 'moveTo', args: [10, 20]}
+			]);
+		});
+		it('should optimize path if radius is exactly half of width', function() {
+			var context = window.createMockContext();
+
+			helpers.canvas.roundedRect(context, 10, 20, 30, 40, 15);
+
+			expect(context.getCalls()).toEqual([
+				{name: 'arc', args: [25, 35, 15, -Math.PI, 0]},
+				{name: 'arc', args: [25, 45, 15, 0, Math.PI]},
+				{name: 'closePath', args: []},
+				{name: 'moveTo', args: [10, 20]}
+			]);
+		});
+		it('should optimize path if radius is exactly half of width and height', function() {
+			var context = window.createMockContext();
+
+			helpers.canvas.roundedRect(context, 10, 20, 30, 30, 15);
+
+			expect(context.getCalls()).toEqual([
+				{name: 'arc', args: [25, 35, 15, 0, Math.PI * 2]},
 				{name: 'closePath', args: []},
 				{name: 'moveTo', args: [10, 20]}
 			]);