From 14ead62f8fa3788f6f54029b75a33145edaed86c Mon Sep 17 00:00:00 2001 From: Akihiko Kusanagi Date: Mon, 24 Dec 2018 13:40:38 +0800 Subject: [PATCH 1/3] Prevent bezier points from being capped when a data point is off the chart --- src/controllers/controller.line.js | 14 ++++++++++---- src/elements/element.point.js | 4 +--- src/helpers/helpers.canvas.js | 7 +++++++ test/specs/helpers.canvas.tests.js | 14 ++++++++++++++ 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index fffb1ce55d2..9bfb138838f 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -281,10 +281,16 @@ module.exports = DatasetController.extend({ if (me.chart.options.elements.line.capBezierPoints) { for (i = 0, ilen = points.length; i < ilen; ++i) { model = points[i]._model; - model.controlPointPreviousX = capControlPoint(model.controlPointPreviousX, area.left, area.right); - model.controlPointPreviousY = capControlPoint(model.controlPointPreviousY, area.top, area.bottom); - model.controlPointNextX = capControlPoint(model.controlPointNextX, area.left, area.right); - model.controlPointNextY = capControlPoint(model.controlPointNextY, area.top, area.bottom); + if (helpers.canvas.isPointInArea(model, area)) { + if (i > 0 && helpers.canvas.isPointInArea(points[i - 1]._model, area)) { + model.controlPointPreviousX = capControlPoint(model.controlPointPreviousX, area.left, area.right); + model.controlPointPreviousY = capControlPoint(model.controlPointPreviousY, area.top, area.bottom); + } + if (i < points.length - 1 && helpers.canvas.isPointInArea(points[i + 1]._model, area)) { + model.controlPointNextX = capControlPoint(model.controlPointNextX, area.left, area.right); + model.controlPointNextY = capControlPoint(model.controlPointNextY, area.top, area.bottom); + } + } } } }, diff --git a/src/elements/element.point.js b/src/elements/element.point.js index b7909116a7c..1f7876d83ce 100644 --- a/src/elements/element.point.js +++ b/src/elements/element.point.js @@ -65,14 +65,12 @@ module.exports = Element.extend({ draw: function(chartArea) { var vm = this._view; - var model = this._model; var ctx = this._chart.ctx; var pointStyle = vm.pointStyle; var rotation = vm.rotation; var radius = vm.radius; var x = vm.x; var y = vm.y; - var epsilon = 0.0000001; // 0.0000001 is margin in pixels for Accumulated error. var globalDefaults = defaults.global; var defaultColor = globalDefaults.defaultColor; // eslint-disable-line no-shadow @@ -81,7 +79,7 @@ module.exports = Element.extend({ } // Clipping for Points. - if (chartArea === undefined || (model.x > chartArea.left - epsilon && chartArea.right + epsilon > model.x && model.y > chartArea.top - epsilon && chartArea.bottom + epsilon > model.y)) { + if (chartArea === undefined || helpers.canvas.isPointInArea(vm, chartArea)) { ctx.strokeStyle = vm.borderColor || defaultColor; ctx.lineWidth = helpers.valueOrDefault(vm.borderWidth, globalDefaults.elements.point.borderWidth); ctx.fillStyle = vm.backgroundColor || defaultColor; diff --git a/src/helpers/helpers.canvas.js b/src/helpers/helpers.canvas.js index b1212dff46d..885e3bcd4ee 100644 --- a/src/helpers/helpers.canvas.js +++ b/src/helpers/helpers.canvas.js @@ -172,6 +172,13 @@ var exports = { ctx.stroke(); }, + isPointInArea: function(point, area) { + var epsilon = 1e-6; // 1e-6 is margin in pixels for Accumulated error. + + return point.x > area.left - epsilon && point.x < area.right + epsilon && + point.y > area.top - epsilon && point.y < area.bottom + epsilon; + }, + clipArea: function(ctx, area) { ctx.save(); ctx.beginPath(); diff --git a/test/specs/helpers.canvas.tests.js b/test/specs/helpers.canvas.tests.js index ee42a414e6f..05f515fec52 100644 --- a/test/specs/helpers.canvas.tests.js +++ b/test/specs/helpers.canvas.tests.js @@ -86,4 +86,18 @@ describe('Chart.helpers.canvas', function() { expect(context.getCalls()).toEqual([{name: 'rect', args: [10, 20, 30, 40]}]); }); }); + + describe('isPointInArea', function() { + it('should determine if a point is in the area', function() { + var isPointInArea = helpers.canvas.isPointInArea; + var area = {left: 0, top: 0, right: 512, bottom: 256}; + + expect(isPointInArea({x: 0, y: 0}, area)).toBe(true); + expect(isPointInArea({x: -1e-12, y: -1e-12}, area)).toBe(true); + expect(isPointInArea({x: 512, y: 256}, area)).toBe(true); + expect(isPointInArea({x: 512 + 1e-12, y: 256 + 1e-12}, area)).toBe(true); + expect(isPointInArea({x: -1e-3, y: 0}, area)).toBe(false); + expect(isPointInArea({x: 0, y: 256 + 1e-3}, area)).toBe(false); + }); + }); }); From 45413129f6a222b3ef8db3f24d574b4859b36ebd Mon Sep 17 00:00:00 2001 From: Akihiko Kusanagi Date: Sun, 30 Dec 2018 00:54:10 +0800 Subject: [PATCH 2/3] Refactor for code minimization --- src/controllers/controller.line.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index 9bfb138838f..562a1c291b3 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -243,13 +243,16 @@ module.exports = DatasetController.extend({ updateBezierControlPoints: function() { var me = this; + var chart = me.chart; var meta = me.getMeta(); - var area = me.chart.chartArea; - var points = (meta.data || []); + var lineModel = meta.dataset._model; + var area = chart.chartArea; + var points = meta.data || []; + var isPointInArea = helpers.canvas.isPointInArea; var i, ilen, point, model, controlPoints; // Only consider points that are drawn in case the spanGaps option is used - if (meta.dataset._model.spanGaps) { + if (lineModel.spanGaps) { points = points.filter(function(pt) { return !pt._model.skip; }); @@ -259,7 +262,7 @@ module.exports = DatasetController.extend({ return Math.max(Math.min(pt, max), min); } - if (meta.dataset._model.cubicInterpolationMode === 'monotone') { + if (lineModel.cubicInterpolationMode === 'monotone') { helpers.splineCurveMonotone(points); } else { for (i = 0, ilen = points.length; i < ilen; ++i) { @@ -269,7 +272,7 @@ module.exports = DatasetController.extend({ helpers.previousItem(points, i)._model, model, helpers.nextItem(points, i)._model, - meta.dataset._model.tension + lineModel.tension ); model.controlPointPreviousX = controlPoints.previous.x; model.controlPointPreviousY = controlPoints.previous.y; @@ -278,15 +281,15 @@ module.exports = DatasetController.extend({ } } - if (me.chart.options.elements.line.capBezierPoints) { + if (chart.options.elements.line.capBezierPoints) { for (i = 0, ilen = points.length; i < ilen; ++i) { model = points[i]._model; - if (helpers.canvas.isPointInArea(model, area)) { - if (i > 0 && helpers.canvas.isPointInArea(points[i - 1]._model, area)) { + if (isPointInArea(model, area)) { + if (i > 0 && isPointInArea(points[i - 1]._model, area)) { model.controlPointPreviousX = capControlPoint(model.controlPointPreviousX, area.left, area.right); model.controlPointPreviousY = capControlPoint(model.controlPointPreviousY, area.top, area.bottom); } - if (i < points.length - 1 && helpers.canvas.isPointInArea(points[i + 1]._model, area)) { + if (i < points.length - 1 && isPointInArea(points[i + 1]._model, area)) { model.controlPointNextX = capControlPoint(model.controlPointNextX, area.left, area.right); model.controlPointNextY = capControlPoint(model.controlPointNextY, area.top, area.bottom); } From 71fa1455f12a945746811992ce4d6141d29438e3 Mon Sep 17 00:00:00 2001 From: Akihiko Kusanagi Date: Sat, 5 Jan 2019 03:20:56 +0800 Subject: [PATCH 3/3] Make helpers.canvas.isPointInArea a private method --- src/controllers/controller.line.js | 9 +++++---- src/elements/element.point.js | 2 +- src/helpers/helpers.canvas.js | 11 +++++++++-- test/specs/helpers.canvas.tests.js | 2 +- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index 562a1c291b3..03f7faf96d2 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -5,6 +5,8 @@ var defaults = require('../core/core.defaults'); var elements = require('../elements/index'); var helpers = require('../helpers/index'); +var _isPointInArea = helpers.canvas._isPointInArea; + defaults._set('line', { showLines: true, spanGaps: false, @@ -248,7 +250,6 @@ module.exports = DatasetController.extend({ var lineModel = meta.dataset._model; var area = chart.chartArea; var points = meta.data || []; - var isPointInArea = helpers.canvas.isPointInArea; var i, ilen, point, model, controlPoints; // Only consider points that are drawn in case the spanGaps option is used @@ -284,12 +285,12 @@ module.exports = DatasetController.extend({ if (chart.options.elements.line.capBezierPoints) { for (i = 0, ilen = points.length; i < ilen; ++i) { model = points[i]._model; - if (isPointInArea(model, area)) { - if (i > 0 && isPointInArea(points[i - 1]._model, area)) { + if (_isPointInArea(model, area)) { + if (i > 0 && _isPointInArea(points[i - 1]._model, area)) { model.controlPointPreviousX = capControlPoint(model.controlPointPreviousX, area.left, area.right); model.controlPointPreviousY = capControlPoint(model.controlPointPreviousY, area.top, area.bottom); } - if (i < points.length - 1 && isPointInArea(points[i + 1]._model, area)) { + if (i < points.length - 1 && _isPointInArea(points[i + 1]._model, area)) { model.controlPointNextX = capControlPoint(model.controlPointNextX, area.left, area.right); model.controlPointNextY = capControlPoint(model.controlPointNextY, area.top, area.bottom); } diff --git a/src/elements/element.point.js b/src/elements/element.point.js index 1f7876d83ce..8ae37eb8a28 100644 --- a/src/elements/element.point.js +++ b/src/elements/element.point.js @@ -79,7 +79,7 @@ module.exports = Element.extend({ } // Clipping for Points. - if (chartArea === undefined || helpers.canvas.isPointInArea(vm, chartArea)) { + if (chartArea === undefined || helpers.canvas._isPointInArea(vm, chartArea)) { ctx.strokeStyle = vm.borderColor || defaultColor; ctx.lineWidth = helpers.valueOrDefault(vm.borderWidth, globalDefaults.elements.point.borderWidth); ctx.fillStyle = vm.backgroundColor || defaultColor; diff --git a/src/helpers/helpers.canvas.js b/src/helpers/helpers.canvas.js index 885e3bcd4ee..1513386a8cb 100644 --- a/src/helpers/helpers.canvas.js +++ b/src/helpers/helpers.canvas.js @@ -172,8 +172,15 @@ var exports = { ctx.stroke(); }, - isPointInArea: function(point, area) { - var epsilon = 1e-6; // 1e-6 is margin in pixels for Accumulated error. + /** + * Returns true if the point is inside the rectangle + * @param {Object} point - The point to test + * @param {Object} area - The rectangle + * @returns {Boolean} + * @private + */ + _isPointInArea: function(point, area) { + var epsilon = 1e-6; // 1e-6 is margin in pixels for accumulated error. return point.x > area.left - epsilon && point.x < area.right + epsilon && point.y > area.top - epsilon && point.y < area.bottom + epsilon; diff --git a/test/specs/helpers.canvas.tests.js b/test/specs/helpers.canvas.tests.js index 05f515fec52..9654ef28e7b 100644 --- a/test/specs/helpers.canvas.tests.js +++ b/test/specs/helpers.canvas.tests.js @@ -89,7 +89,7 @@ describe('Chart.helpers.canvas', function() { describe('isPointInArea', function() { it('should determine if a point is in the area', function() { - var isPointInArea = helpers.canvas.isPointInArea; + var isPointInArea = helpers.canvas._isPointInArea; var area = {left: 0, top: 0, right: 512, bottom: 256}; expect(isPointInArea({x: 0, y: 0}, area)).toBe(true);