Skip to content

Commit

Permalink
Further improve linear tick collision estimation (#9129)
Browse files Browse the repository at this point in the history
* Further improve linear tick collision estimation

* More tolerance

* Re-create fixtures

* And more tolerance
  • Loading branch information
kurkle committed May 20, 2021
1 parent 7800939 commit a6f0b37
Show file tree
Hide file tree
Showing 25 changed files with 142 additions and 37 deletions.
2 changes: 1 addition & 1 deletion src/core/core.scale.js
Expand Up @@ -1665,6 +1665,6 @@ export default class Scale extends Element {
_maxDigits() {
const me = this;
const fontSize = me._resolveTickFontOptions(0).lineHeight;
return me.isHorizontal() ? me.width / fontSize / 0.7 : me.height / fontSize;
return (me.isHorizontal() ? me.width : me.height) / fontSize;
}
}
11 changes: 6 additions & 5 deletions src/scales/scale.linear.js
@@ -1,6 +1,7 @@
import {isFinite} from '../helpers/helpers.core';
import LinearScaleBase from './scale.linearbase';
import Ticks from '../core/core.ticks';
import {toRadians} from '../helpers';

export default class LinearScale extends LinearScaleBase {

Expand All @@ -21,12 +22,12 @@ export default class LinearScale extends LinearScaleBase {
*/
computeTickLimit() {
const me = this;

if (me.isHorizontal()) {
return Math.ceil(me.width / 40);
}
const horizontal = me.isHorizontal();
const length = horizontal ? me.width : me.height;
const minRotation = toRadians(me.options.ticks.minRotation);
const ratio = (horizontal ? Math.sin(minRotation) : Math.cos(minRotation)) || 0.001;
const tickFont = me._resolveTickFontOptions(0);
return Math.ceil(me.height / tickFont.lineHeight);
return Math.ceil(length / Math.min(40, tickFont.lineHeight / ratio));
}

// Utils
Expand Down
12 changes: 5 additions & 7 deletions src/scales/scale.linearbase.js
Expand Up @@ -36,7 +36,7 @@ function generateTicks(generationOptions, dataRange) {
const minDefined = !isNullOrUndef(min);
const maxDefined = !isNullOrUndef(max);
const countDefined = !isNullOrUndef(count);
const minSpacing = (rmax - rmin) / maxDigits;
const minSpacing = (rmax - rmin) / (maxDigits + 1);
let spacing = niceNum((rmax - rmin) / maxSpaces / unit) * unit;
let factor, niceMin, niceMax, numSpaces;

Expand Down Expand Up @@ -135,12 +135,10 @@ function generateTicks(generationOptions, dataRange) {
}

function relativeLabelSize(value, minSpacing, {horizontal, minRotation}) {
const rot = toRadians(minRotation);
const useLength = (horizontal && minRotation <= 45) || (!horizontal && minRotation >= 45);
const l = useLength ? minSpacing * ('' + value).length : 0;
const sin = Math.sin(rot);
const cos = Math.cos(rot);
return horizontal ? cos * l + sin * minSpacing : sin * l + cos * minSpacing;
const rad = toRadians(minRotation);
const ratio = (horizontal ? Math.sin(rad) : Math.cos(rad)) || 0.001;
const length = 0.75 * minSpacing * ('' + value).length;
return Math.min(minSpacing / ratio, length);
}

export default class LinearScaleBase extends Scale {
Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/scale.linear/min-max-skip/edge-case-1.js
Expand Up @@ -24,8 +24,8 @@ module.exports = {
options: {
spriteText: true,
canvas: {
height: 211,
width: 408
height: 196,
width: 407
}
}
};
Binary file modified test/fixtures/scale.linear/min-max-skip/edge-case-1.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions test/fixtures/scale.linear/min-max-skip/edge-case-2.js
Expand Up @@ -24,8 +24,8 @@ module.exports = {
options: {
spriteText: true,
canvas: {
height: 212,
width: 409
height: 197,
width: 420
}
}
};
Binary file modified test/fixtures/scale.linear/min-max-skip/edge-case-2.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions test/fixtures/scale.linear/min-max-skip/edge-case-3.js
Expand Up @@ -24,8 +24,8 @@ module.exports = {
options: {
spriteText: true,
canvas: {
height: 213,
width: 536
height: 199,
width: 556
}
}
};
Binary file modified test/fixtures/scale.linear/min-max-skip/edge-case-3.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions test/fixtures/scale.linear/min-max-skip/edge-case-4.js
Expand Up @@ -24,8 +24,8 @@ module.exports = {
options: {
spriteText: true,
canvas: {
height: 214,
width: 537
height: 200,
width: 557
}
}
};
Binary file modified test/fixtures/scale.linear/min-max-skip/edge-case-4.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 4 additions & 4 deletions test/fixtures/scale.linear/min-max-skip/rotated-case-1.js
@@ -1,6 +1,6 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/9025',
threshold: 0.15,
threshold: 0.2,
config: {
type: 'scatter',
options: {
Expand All @@ -18,7 +18,7 @@ module.exports = {
min: 230,
ticks: {
autoSkip: false,
minRotation: 35
minRotation: 67.5
}
}
}
Expand All @@ -27,8 +27,8 @@ module.exports = {
options: {
spriteText: true,
canvas: {
height: 211,
width: 415
height: 231,
width: 221
}
}
};
Binary file modified test/fixtures/scale.linear/min-max-skip/rotated-case-1.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 4 additions & 4 deletions test/fixtures/scale.linear/min-max-skip/rotated-case-2.js
@@ -1,6 +1,6 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/9025',
threshold: 0.15,
threshold: 0.2,
config: {
type: 'scatter',
options: {
Expand All @@ -18,7 +18,7 @@ module.exports = {
min: 230,
ticks: {
autoSkip: false,
minRotation: 35
minRotation: 67.5
}
}
}
Expand All @@ -27,8 +27,8 @@ module.exports = {
options: {
spriteText: true,
canvas: {
height: 214,
width: 416
height: 232,
width: 222
}
}
};
Binary file modified test/fixtures/scale.linear/min-max-skip/rotated-case-2.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 4 additions & 4 deletions test/fixtures/scale.linear/min-max-skip/rotated-case-3.js
@@ -1,6 +1,6 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/9025',
threshold: 0.15,
threshold: 0.2,
config: {
type: 'scatter',
options: {
Expand All @@ -18,7 +18,7 @@ module.exports = {
min: 230,
ticks: {
autoSkip: false,
minRotation: 35
minRotation: 67.5
}
}
}
Expand All @@ -27,8 +27,8 @@ module.exports = {
options: {
spriteText: true,
canvas: {
height: 216,
width: 520
height: 234,
width: 224
}
}
};
Binary file modified test/fixtures/scale.linear/min-max-skip/rotated-case-3.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 4 additions & 4 deletions test/fixtures/scale.linear/min-max-skip/rotated-case-4.js
@@ -1,6 +1,6 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/9025',
threshold: 0.15,
threshold: 0.2,
config: {
type: 'scatter',
options: {
Expand All @@ -18,7 +18,7 @@ module.exports = {
min: 230,
ticks: {
autoSkip: false,
minRotation: 35
minRotation: 67.5
}
}
}
Expand All @@ -27,8 +27,8 @@ module.exports = {
options: {
spriteText: true,
canvas: {
height: 217,
width: 521
height: 235,
width: 225
}
}
};
Binary file modified test/fixtures/scale.linear/min-max-skip/rotated-case-4.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
38 changes: 38 additions & 0 deletions test/fixtures/scale.linear/rotated-45.js
@@ -0,0 +1,38 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/9025',
threshold: 0.2,
config: {
type: 'scatter',
options: {
scales: {
y: {
min: 1612781975085.5466,
max: 1620287255085.5466,
ticks: {
autoSkip: false,
minRotation: 45,
maxRotation: 45,
count: 13
}
},
x: {
min: 1612781975085.5466,
max: 1620287255085.5466,
ticks: {
autoSkip: false,
minRotation: 45,
maxRotation: 45,
count: 13
}
}
}
}
},
options: {
spriteText: true,
canvas: {
height: 350,
width: 350
}
}
};
Binary file added test/fixtures/scale.linear/rotated-45.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
34 changes: 34 additions & 0 deletions test/fixtures/scale.linear/rotated-5.js
@@ -0,0 +1,34 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/9025',
threshold: 0.2,
config: {
type: 'scatter',
options: {
scales: {
y: {
min: 0,
max: 500000,
ticks: {
minRotation: 5,
maxRotation: 5,
}
},
x: {
min: 0,
max: 500000,
ticks: {
minRotation: 5,
maxRotation: 5,
}
}
}
}
},
options: {
spriteText: true,
canvas: {
height: 350,
width: 350
}
}
};
Binary file added test/fixtures/scale.linear/rotated-5.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
34 changes: 34 additions & 0 deletions test/fixtures/scale.linear/rotated-85.js
@@ -0,0 +1,34 @@
module.exports = {
description: 'https://github.com/chartjs/Chart.js/issues/9025',
threshold: 0.2,
config: {
type: 'scatter',
options: {
scales: {
y: {
min: 0,
max: 500000,
ticks: {
minRotation: 85,
maxRotation: 85,
}
},
x: {
min: 0,
max: 500000,
ticks: {
minRotation: 85,
maxRotation: 85,
}
}
}
}
},
options: {
spriteText: true,
canvas: {
height: 350,
width: 350
}
}
};
Binary file added test/fixtures/scale.linear/rotated-85.png
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 a6f0b37

Please sign in to comment.