Skip to content

Commit

Permalink
Improve linear tick generators collision estimation (#8983)
Browse files Browse the repository at this point in the history
* Increase distance to min/max tick to 1/3 space

* Better estimation on linear tick collision

* Lint fix

* Remove unused change
  • Loading branch information
kurkle committed Apr 28, 2021
1 parent 0f1d07a commit 44e62e7
Show file tree
Hide file tree
Showing 10 changed files with 139 additions and 3 deletions.
9 changes: 9 additions & 0 deletions src/core/core.scale.js
Expand Up @@ -1658,4 +1658,13 @@ export default class Scale extends Element {
const opts = this.options.ticks.setContext(this.getContext(index));
return toFont(opts.font);
}

/**
* @protected
*/
_maxDigits() {
const me = this;
const fontSize = me._resolveTickFontOptions(0).lineHeight;
return me.isHorizontal() ? me.width / fontSize / 0.7 : me.height / fontSize;
}
}
9 changes: 6 additions & 3 deletions src/scales/scale.linearbase.js
Expand Up @@ -29,13 +29,14 @@ function generateTicks(generationOptions, dataRange) {
// for details.

const MIN_SPACING = 1e-14;
const {step, min, max, precision, count, maxTicks} = generationOptions;
const {step, min, max, precision, count, maxTicks, maxDigits, horizontal} = generationOptions;
const unit = step || 1;
const maxSpaces = maxTicks - 1;
const {min: rmin, max: rmax} = dataRange;
const minDefined = !isNullOrUndef(min);
const maxDefined = !isNullOrUndef(max);
const countDefined = !isNullOrUndef(count);
const minSpacing = (rmax - rmin) / maxDigits;
let spacing = niceNum((rmax - rmin) / maxSpaces / unit) * unit;
let factor, niceMin, niceMax, numSpaces;

Expand Down Expand Up @@ -102,7 +103,7 @@ function generateTicks(generationOptions, dataRange) {
j++;
}
// If the next nice tick is close to min, skip that too
if (almostEquals(Math.round((niceMin + j * spacing) * factor) / factor, min, spacing / 10)) {
if (almostEquals(Math.round((niceMin + j * spacing) * factor) / factor, min, minSpacing * (horizontal ? ('' + min).length : 1))) {
j++;
}
}
Expand All @@ -113,7 +114,7 @@ function generateTicks(generationOptions, dataRange) {

if (maxDefined) {
// If the previous tick is close to max, replace it with max, else add max
if (almostEquals(ticks[ticks.length - 1].value, max, spacing / 10)) {
if (almostEquals(ticks[ticks.length - 1].value, max, minSpacing * (horizontal ? ('' + max).length : 1))) {
ticks[ticks.length - 1].value = max;
} else {
ticks.push({value: max});
Expand Down Expand Up @@ -230,6 +231,8 @@ export default class LinearScaleBase extends Scale {
precision: tickOpts.precision,
step: tickOpts.stepSize,
count: tickOpts.count,
maxDigits: me._maxDigits(),
horizontal: me.isHorizontal()
};
const dataRange = me._range || me;
const ticks = generateTicks(numericGeneratorOptions, dataRange);
Expand Down
31 changes: 31 additions & 0 deletions test/fixtures/scale.linear/min-max-skip-edge-case-1.js
@@ -0,0 +1,31 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/8982',
config: {
type: 'scatter',
options: {
scales: {
y: {
max: 1069,
min: 230,
ticks: {
autoSkip: false
}
},
x: {
max: 1069,
min: 230,
ticks: {
autoSkip: false
}
}
}
}
},
options: {
spriteText: true,
canvas: {
height: 211,
width: 408
}
}
};
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
31 changes: 31 additions & 0 deletions test/fixtures/scale.linear/min-max-skip-edge-case-2.js
@@ -0,0 +1,31 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/8982',
config: {
type: 'scatter',
options: {
scales: {
y: {
max: 1069,
min: 230,
ticks: {
autoSkip: false
}
},
x: {
max: 1069,
min: 230,
ticks: {
autoSkip: false
}
}
}
}
},
options: {
spriteText: true,
canvas: {
height: 212,
width: 409
}
}
};
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
31 changes: 31 additions & 0 deletions test/fixtures/scale.linear/min-max-skip-edge-case-3.js
@@ -0,0 +1,31 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/8982',
config: {
type: 'scatter',
options: {
scales: {
y: {
max: 1069,
min: 230,
ticks: {
autoSkip: false
}
},
x: {
max: 1069,
min: 230,
ticks: {
autoSkip: false
}
}
}
}
},
options: {
spriteText: true,
canvas: {
height: 213,
width: 536
}
}
};
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
31 changes: 31 additions & 0 deletions test/fixtures/scale.linear/min-max-skip-edge-case-4.js
@@ -0,0 +1,31 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/8982',
config: {
type: 'scatter',
options: {
scales: {
y: {
max: 1069,
min: 230,
ticks: {
autoSkip: false
}
},
x: {
max: 1069,
min: 230,
ticks: {
autoSkip: false
}
}
}
}
},
options: {
spriteText: true,
canvas: {
height: 214,
width: 537
}
}
};
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 44e62e7

Please sign in to comment.