From 1596d584d84c93a2f86a0bd04f06bee1ac19de2d Mon Sep 17 00:00:00 2001 From: Akihiko Kusanagi Date: Sun, 8 Jul 2018 12:34:14 +0800 Subject: [PATCH 1/2] Refactor helpers.canvas.drawPoint() --- src/helpers/helpers.canvas.js | 23 ++++------------------- test/specs/element.point.tests.js | 21 +++------------------ test/specs/global.deprecations.tests.js | 1 - test/specs/helpers.canvas.tests.js | 3 ++- 4 files changed, 9 insertions(+), 39 deletions(-) diff --git a/src/helpers/helpers.canvas.js b/src/helpers/helpers.canvas.js index 26f7d37212f..cb1c96927c9 100644 --- a/src/helpers/helpers.canvas.js +++ b/src/helpers/helpers.canvas.js @@ -41,6 +41,7 @@ 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(); } else { ctx.rect(x, y, width, height); } @@ -65,17 +66,16 @@ 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); @@ -86,16 +86,14 @@ var exports = module.exports = { 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); + ctx.fill(); 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 @@ -103,12 +101,10 @@ var exports = module.exports = { // 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); @@ -117,25 +113,20 @@ var exports = module.exports = { 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,19 +137,14 @@ 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; } @@ -224,5 +210,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 b2f803416b5..c6b34624cdc 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: [] @@ -357,9 +357,6 @@ describe('Point element tests', function() { }, { name: 'lineTo', args: [2, 0], - }, { - name: 'closePath', - args: [], }, { name: 'stroke', args: [] @@ -405,9 +402,6 @@ 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: 'stroke', args: [] @@ -465,9 +459,6 @@ 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: 'stroke', args: [] @@ -507,9 +498,6 @@ describe('Point element tests', function() { }, { name: 'lineTo', args: [2, 0], - }, { - name: 'closePath', - args: [], }, { name: 'stroke', args: [] @@ -549,9 +537,6 @@ describe('Point element tests', function() { }, { name: 'lineTo', args: [2, 0], - }, { - name: 'closePath', - args: [], }, { name: 'stroke', args: [] diff --git a/test/specs/global.deprecations.tests.js b/test/specs/global.deprecations.tests.js index 535b9af3307..11d25d1d9b6 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 0c20628d456..3a3cbee4547 100644 --- a/test/specs/helpers.canvas.tests.js +++ b/test/specs/helpers.canvas.tests.js @@ -36,7 +36,8 @@ 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: []} ]); }); it('should optimize path if radius is 0', function() { From c53bd3c87708ec4d0dd8b601911a8e1ef110a2bf Mon Sep 17 00:00:00 2001 From: Akihiko Kusanagi Date: Tue, 10 Jul 2018 23:54:37 +0800 Subject: [PATCH 2/2] Move fill() call to the end and add moveTo() call to the end of roundedRect() --- src/helpers/helpers.canvas.js | 8 ++------ test/specs/element.point.tests.js | 15 +++++++++++++++ test/specs/helpers.canvas.tests.js | 3 ++- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/helpers/helpers.canvas.js b/src/helpers/helpers.canvas.js index cb1c96927c9..60fb6e1a299 100644 --- a/src/helpers/helpers.canvas.js +++ b/src/helpers/helpers.canvas.js @@ -42,6 +42,7 @@ var exports = module.exports = { 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); } @@ -73,7 +74,6 @@ var exports = module.exports = { default: ctx.arc(0, 0, radius, 0, Math.PI * 2); ctx.closePath(); - ctx.fill(); break; case 'triangle': edgeLength = 3 * radius / Math.sqrt(3); @@ -82,12 +82,10 @@ var exports = module.exports = { 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.rect(-size, -size, 2 * size, 2 * size); - ctx.fill(); break; case 'rectRounded': var offset = radius / Math.SQRT2; @@ -100,8 +98,6 @@ var exports = module.exports = { // 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.fill(); break; case 'rectRot': size = 1 / Math.SQRT2 * radius; @@ -110,7 +106,6 @@ var exports = module.exports = { ctx.lineTo(size, 0); ctx.lineTo(0, -size); ctx.closePath(); - ctx.fill(); break; case 'cross': ctx.moveTo(0, radius); @@ -148,6 +143,7 @@ var exports = module.exports = { break; } + ctx.fill(); ctx.stroke(); ctx.restore(); }, diff --git a/test/specs/element.point.tests.js b/test/specs/element.point.tests.js index c6b34624cdc..f1da35a50d4 100644 --- a/test/specs/element.point.tests.js +++ b/test/specs/element.point.tests.js @@ -357,6 +357,9 @@ describe('Point element tests', function() { }, { name: 'lineTo', args: [2, 0], + }, { + name: 'fill', + args: [] }, { name: 'stroke', args: [] @@ -402,6 +405,9 @@ describe('Point element tests', function() { }, { name: 'lineTo', args: [0 + Math.cos(Math.PI / 4) * 2, 0 - Math.sin(Math.PI / 4) * 2], + }, { + name: 'fill', + args: [] }, { name: 'stroke', args: [] @@ -459,6 +465,9 @@ describe('Point element tests', function() { }, { name: 'lineTo', args: [0 + Math.cos(Math.PI / 4) * 2, 0 - Math.sin(Math.PI / 4) * 2], + }, { + name: 'fill', + args: [] }, { name: 'stroke', args: [] @@ -498,6 +507,9 @@ describe('Point element tests', function() { }, { name: 'lineTo', args: [2, 0], + }, { + name: 'fill', + args: [] }, { name: 'stroke', args: [] @@ -537,6 +549,9 @@ describe('Point element tests', function() { }, { name: 'lineTo', args: [2, 0], + }, { + name: 'fill', + args: [] }, { name: 'stroke', args: [] diff --git a/test/specs/helpers.canvas.tests.js b/test/specs/helpers.canvas.tests.js index 3a3cbee4547..1a342c1cb3b 100644 --- a/test/specs/helpers.canvas.tests.js +++ b/test/specs/helpers.canvas.tests.js @@ -37,7 +37,8 @@ describe('Chart.helpers.canvas', function() { {name: 'arcTo', args: [10, 60, 10, 55, 5]}, {name: 'lineTo', args: [10, 25]}, {name: 'arcTo', args: [10, 20, 15, 20, 5]}, - {name: 'closePath', args: []} + {name: 'closePath', args: []}, + {name: 'moveTo', args: [10, 20]} ]); }); it('should optimize path if radius is 0', function() {