Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further improve linear tick collision estimation #9129

Merged
merged 4 commits into from May 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -131,12 +131,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Text is now cut off in these images. Is that expected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, missed that. Will update

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out the fixture comparison forces the new image to be same size as old image. So just needed to delete and re-create those.

}
}
};
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.