From: Akihiko Kusanagi Date: Sun, 29 Jul 2018 09:31:28 +0000 (+0700) Subject: Refactor helpers.canvas.drawPoint() (#5623) X-Git-Tag: v2.7.3~1^2~14 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=493eaa842444b4e5b1f6d192fadf2bb1aaa02eba;p=thirdparty%2FChart.js.git Refactor helpers.canvas.drawPoint() (#5623) Bring ctx.beginPath() before switch and replace ctx.fillRect()/ctx.strokeRect() with ctx.rect()/ctx.fill() to make it consistent with the other styles. It is also preferable that helpers.canvas.roundedRect() include ctx.closePath() at the end because CanvasRenderingContext2D.rect() closes the subpath. Get rid of ctx.closePath() for cross, crossRot, star, line and dash because these have no closed path shape, and it should be avoided that ctx.closePath() makes a round-trip path. --- diff --git a/src/helpers/helpers.canvas.js b/src/helpers/helpers.canvas.js index 26f7d3721..60fb6e1a2 100644 --- a/src/helpers/helpers.canvas.js +++ b/src/helpers/helpers.canvas.js @@ -41,6 +41,8 @@ var exports = module.exports = { ctx.arcTo(x, y + height, x, y + height - r, r); ctx.lineTo(x, y + r); ctx.arcTo(x, y, x + r, y, r); + ctx.closePath(); + ctx.moveTo(x, y); } else { ctx.rect(x, y, width, height); } @@ -65,77 +67,61 @@ var exports = module.exports = { ctx.save(); ctx.translate(x, y); ctx.rotate(rotation * Math.PI / 180); + ctx.beginPath(); switch (style) { // Default includes circle default: - ctx.beginPath(); ctx.arc(0, 0, radius, 0, Math.PI * 2); ctx.closePath(); - ctx.fill(); break; case 'triangle': - ctx.beginPath(); 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.closePath(); - ctx.fill(); break; case 'rect': size = 1 / Math.SQRT2 * radius; - ctx.beginPath(); - ctx.fillRect(-size, -size, 2 * size, 2 * size); - ctx.strokeRect(-size, -size, 2 * size, 2 * size); + 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; - ctx.beginPath(); // 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); - - ctx.closePath(); - ctx.fill(); break; case 'rectRot': size = 1 / Math.SQRT2 * radius; - ctx.beginPath(); ctx.moveTo(-size, 0); ctx.lineTo(0, size); ctx.lineTo(size, 0); ctx.lineTo(0, -size); ctx.closePath(); - ctx.fill(); break; case 'cross': - ctx.beginPath(); ctx.moveTo(0, radius); ctx.lineTo(0, -radius); ctx.moveTo(-radius, 0); ctx.lineTo(radius, 0); - ctx.closePath(); break; case 'crossRot': - ctx.beginPath(); 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); - ctx.closePath(); break; case 'star': - ctx.beginPath(); ctx.moveTo(0, radius); ctx.lineTo(0, -radius); ctx.moveTo(-radius, 0); @@ -146,22 +132,18 @@ var exports = module.exports = { ctx.lineTo(xOffset, yOffset); ctx.moveTo(-xOffset, yOffset); ctx.lineTo(xOffset, -yOffset); - ctx.closePath(); break; case 'line': - ctx.beginPath(); ctx.moveTo(-radius, 0); ctx.lineTo(radius, 0); - ctx.closePath(); break; case 'dash': - ctx.beginPath(); ctx.moveTo(0, 0); ctx.lineTo(radius, 0); - ctx.closePath(); break; } + ctx.fill(); ctx.stroke(); ctx.restore(); }, @@ -224,5 +206,4 @@ helpers.clear = exports.clear; helpers.drawRoundedRectangle = function(ctx) { ctx.beginPath(); exports.roundedRect.apply(exports, arguments); - ctx.closePath(); }; diff --git a/test/specs/element.point.tests.js b/test/specs/element.point.tests.js index b2f803416..f1da35a50 100644 --- a/test/specs/element.point.tests.js +++ b/test/specs/element.point.tests.js @@ -232,11 +232,11 @@ describe('Point element tests', function() { name: 'beginPath', args: [] }, { - name: 'fillRect', + name: 'rect', args: [0 - 1 / Math.SQRT2 * 2, 0 - 1 / Math.SQRT2 * 2, 2 / Math.SQRT2 * 2, 2 / Math.SQRT2 * 2] }, { - name: 'strokeRect', - args: [0 - 1 / Math.SQRT2 * 2, 0 - 1 / Math.SQRT2 * 2, 2 / Math.SQRT2 * 2, 2 / Math.SQRT2 * 2] + name: 'fill', + args: [] }, { name: 'stroke', args: [] @@ -358,8 +358,8 @@ describe('Point element tests', function() { name: 'lineTo', args: [2, 0], }, { - name: 'closePath', - args: [], + name: 'fill', + args: [] }, { name: 'stroke', args: [] @@ -406,8 +406,8 @@ describe('Point element tests', function() { name: 'lineTo', args: [0 + Math.cos(Math.PI / 4) * 2, 0 - Math.sin(Math.PI / 4) * 2], }, { - name: 'closePath', - args: [], + name: 'fill', + args: [] }, { name: 'stroke', args: [] @@ -466,8 +466,8 @@ describe('Point element tests', function() { name: 'lineTo', args: [0 + Math.cos(Math.PI / 4) * 2, 0 - Math.sin(Math.PI / 4) * 2], }, { - name: 'closePath', - args: [], + name: 'fill', + args: [] }, { name: 'stroke', args: [] @@ -508,8 +508,8 @@ describe('Point element tests', function() { name: 'lineTo', args: [2, 0], }, { - name: 'closePath', - args: [], + name: 'fill', + args: [] }, { name: 'stroke', args: [] @@ -550,8 +550,8 @@ describe('Point element tests', function() { name: 'lineTo', args: [2, 0], }, { - name: 'closePath', - args: [], + name: 'fill', + args: [] }, { name: 'stroke', args: [] diff --git a/test/specs/global.deprecations.tests.js b/test/specs/global.deprecations.tests.js index 535b9af33..11d25d1d9 100644 --- a/test/specs/global.deprecations.tests.js +++ b/test/specs/global.deprecations.tests.js @@ -113,7 +113,6 @@ describe('Deprecations', function() { var calls = ctx.getCalls(); expect(calls[0]).toEqual({name: 'beginPath', args: []}); - expect(calls[calls.length - 1]).toEqual({name: 'closePath', args: []}); expect(Chart.helpers.canvas.roundedRect).toHaveBeenCalledWith(ctx, 10, 20, 30, 40, 5); }); }); diff --git a/test/specs/helpers.canvas.tests.js b/test/specs/helpers.canvas.tests.js index 0c20628d4..1a342c1cb 100644 --- a/test/specs/helpers.canvas.tests.js +++ b/test/specs/helpers.canvas.tests.js @@ -36,7 +36,9 @@ describe('Chart.helpers.canvas', function() { {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: 'arcTo', args: [10, 20, 15, 20, 5]}, + {name: 'closePath', args: []}, + {name: 'moveTo', args: [10, 20]} ]); }); it('should optimize path if radius is 0', function() {