Skip to content

Commit 0c5db49

Browse files
authored
Fix inRange for full circle arc (#9871)
* Update misleading sample comment * Fix inRange for full circle arc
1 parent 0dc733a commit 0c5db49

File tree

7 files changed

+65
-14
lines changed

7 files changed

+65
-14
lines changed

src/elements/element.arc.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import Element from '../core/core.element';
2-
import {_angleBetween, getAngleFromPoint, TAU, HALF_PI} from '../helpers/index';
3-
import {PI, _limitValue} from '../helpers/helpers.math';
2+
import {_angleBetween, getAngleFromPoint, TAU, HALF_PI, valueOrDefault} from '../helpers/index';
3+
import {PI, _isBetween, _limitValue} from '../helpers/helpers.math';
44
import {_readValueToProps} from '../helpers/helpers.options';
55

66
function clipArc(ctx, element, endAngle) {
@@ -282,8 +282,9 @@ export default class ArcElement extends Element {
282282
'circumference'
283283
], useFinalPosition);
284284
const rAdjust = this.options.spacing / 2;
285-
const betweenAngles = circumference >= TAU || _angleBetween(angle, startAngle, endAngle);
286-
const withinRadius = (distance >= innerRadius + rAdjust && distance <= outerRadius + rAdjust);
285+
const _circumference = valueOrDefault(circumference, endAngle - startAngle);
286+
const betweenAngles = _circumference >= TAU || _angleBetween(angle, startAngle, endAngle);
287+
const withinRadius = _isBetween(distance, innerRadius + rAdjust, outerRadius + rAdjust);
287288

288289
return (betweenAngles && withinRadius);
289290
}

src/elements/element.bar.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import Element from '../core/core.element';
2-
import {isObject, _limitValue} from '../helpers';
2+
import {isObject, _isBetween, _limitValue} from '../helpers';
33
import {addRoundedRectPath} from '../helpers/helpers.canvas';
44
import {toTRBL, toTRBLCorners} from '../helpers/helpers.options';
55

@@ -105,8 +105,8 @@ function inRange(bar, x, y, useFinalPosition) {
105105
const bounds = bar && !skipBoth && getBarBounds(bar, useFinalPosition);
106106

107107
return bounds
108-
&& (skipX || x >= bounds.left && x <= bounds.right)
109-
&& (skipY || y >= bounds.top && y <= bounds.bottom);
108+
&& (skipX || _isBetween(x, bounds.left, bounds.right))
109+
&& (skipY || _isBetween(y, bounds.top, bounds.bottom));
110110
}
111111

112112
function hasRadius(radius) {

src/helpers/helpers.math.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,21 @@ export function _limitValue(value, min, max) {
173173
return Math.max(min, Math.min(max, value));
174174
}
175175

176+
/**
177+
* @param {number} value
178+
* @private
179+
*/
176180
export function _int16Range(value) {
177181
return _limitValue(value, -32768, 32767);
178182
}
183+
184+
/**
185+
* @param {number} value
186+
* @param {number} start
187+
* @param {number} end
188+
* @param {number} [epsilon]
189+
* @private
190+
*/
191+
export function _isBetween(value, start, end, epsilon = 1e-6) {
192+
return value >= Math.min(start, end) - epsilon && value <= Math.max(start, end) + epsilon;
193+
}

src/helpers/helpers.segment.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {_angleBetween, _angleDiff, _normalizeAngle} from './helpers.math';
1+
import {_angleBetween, _angleDiff, _isBetween, _normalizeAngle} from './helpers.math';
22
import {createContext} from './helpers.options';
33

44
/**
@@ -16,7 +16,7 @@ function propertyFn(property) {
1616
};
1717
}
1818
return {
19-
between: (n, s, e) => n >= Math.min(s, e) && n <= Math.max(e, s),
19+
between: _isBetween,
2020
compare: (a, b) => a - b,
2121
normalize: x => x
2222
};

src/plugins/plugin.filler.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import LineElement from '../elements/element.line';
88
import {_boundSegment, _boundSegments} from '../helpers/helpers.segment';
99
import {clipArea, unclipArea} from '../helpers/helpers.canvas';
1010
import {isArray, isFinite, isObject, valueOrDefault} from '../helpers/helpers.core';
11-
import {TAU, _normalizeAngle} from '../helpers/helpers.math';
11+
import {TAU, _isBetween, _normalizeAngle} from '../helpers/helpers.math';
1212

1313
/**
1414
* @typedef { import('../core/core.controller').default } Chart
@@ -293,7 +293,7 @@ function findPoint(line, sourcePoint, property) {
293293
const segment = segments[i];
294294
const firstValue = linePoints[segment.start][property];
295295
const lastValue = linePoints[segment.end][property];
296-
if (pointValue >= firstValue && pointValue <= lastValue) {
296+
if (_isBetween(pointValue, firstValue, lastValue)) {
297297
first = pointValue === firstValue;
298298
last = pointValue === lastValue;
299299
break;

src/plugins/plugin.legend.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {addRoundedRectPath, drawPoint, renderText} from '../helpers/helpers.canv
55
import {
66
callback as call, valueOrDefault, toFont,
77
toPadding, getRtlAdapter, overrideTextDirection, restoreTextDirection,
8-
clipArea, unclipArea
8+
clipArea, unclipArea, _isBetween
99
} from '../helpers/index';
1010
import {_toLeftRightCenter, _alignStartEnd, _textX} from '../helpers/helpers.extras';
1111
import {toTRBLCorners} from '../helpers/helpers.options';
@@ -493,13 +493,15 @@ export class Legend extends Element {
493493
_getLegendItemAt(x, y) {
494494
let i, hitBox, lh;
495495

496-
if (x >= this.left && x <= this.right && y >= this.top && y <= this.bottom) {
496+
if (_isBetween(x, this.left, this.right)
497+
&& _isBetween(y, this.top, this.bottom)) {
497498
// See if we are touching one of the dataset boxes
498499
lh = this.legendHitBoxes;
499500
for (i = 0; i < lh.length; ++i) {
500501
hitBox = lh[i];
501502

502-
if (x >= hitBox.left && x <= hitBox.left + hitBox.width && y >= hitBox.top && y <= hitBox.top + hitBox.height) {
503+
if (_isBetween(x, hitBox.left, hitBox.left + hitBox.width)
504+
&& _isBetween(y, hitBox.top, hitBox.top + hitBox.height)) {
503505
// Touching an element
504506
return this.legendItems[i];
505507
}

test/specs/element.arc.tests.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,39 @@ describe('Arc element tests', function() {
2323
expect(arc.inRange(-1.0 * Math.sqrt(7), Math.sqrt(7))).toBe(false);
2424
});
2525

26+
it ('should determine if in range when full circle', function() {
27+
// Mock out the arc as if the controller put it there
28+
var arc = new Chart.elements.ArcElement({
29+
startAngle: 0,
30+
endAngle: Math.PI * 2,
31+
x: 0,
32+
y: 0,
33+
innerRadius: 5,
34+
outerRadius: 10,
35+
options: {
36+
spacing: 0,
37+
offset: 0,
38+
}
39+
});
40+
41+
for (const radius of [5, 7.5, 10]) {
42+
for (let angle = 0; angle <= 360; angle += 22.5) {
43+
const rad = angle / 180 * Math.PI;
44+
const x = Math.sin(rad) * radius;
45+
const y = Math.cos(rad) * radius;
46+
expect(arc.inRange(x, y)).withContext(`radius: ${radius}, angle: ${angle}`).toBeTrue();
47+
}
48+
}
49+
for (const radius of [4, 11]) {
50+
for (let angle = 0; angle <= 360; angle += 22.5) {
51+
const rad = angle / 180 * Math.PI;
52+
const x = Math.sin(rad) * radius;
53+
const y = Math.cos(rad) * radius;
54+
expect(arc.inRange(x, y)).withContext(`radius: ${radius}, angle: ${angle}`).toBeFalse();
55+
}
56+
}
57+
});
58+
2659
it ('should include spacing for in range check', function() {
2760
// Mock out the arc as if the controller put it there
2861
var arc = new Chart.elements.ArcElement({

0 commit comments

Comments
 (0)