Measure text size for autoscale when rotated

[teroman]

Currently this uses the cos() of the rotation to calculate the width of rotated text.
cos(90) = 0 so the text size is thought to be zero, and no labels are skipped.

Added a check to use the sin() of the width and use that if greater.
sin(90) = 1, so in this case the text height is used as the width taken of the tick text when drawn.

[benmccann]

@teroman would you be able to rebase this PR? thanks!

[benmccann]

@teroman a quick reminder to rebase this when you get a chance

[teroman]

Hi, I've not rebased a patch before, do you know of a good guide?

I tried cloning my patch branch locally and adding a Chart.js as remote repo, but got stuck when using the hash from merge-base with the rebase command. I thought it'd "just work" but instead I got some picking options and am not sure what to do.

Am I even close to doing the right thing?

[benmccann]

@teroman I always do the following (doesn't involve a hash):

git checkout master (switch to main branch)
git remote add upstream [email protected]:chartjs/Chart.js.git (add chartjs as upstream repo)
git fetch upstream (get the latest changes)
git merge upstream/master (apply to your master branch)
git checkout YOURBRANCH (switch to the branch with your changes)
git rebase master (rebase against master)
FIX CONFLICTS
git add . (mark conflicting files as fixed)
git rebase --continue (finish rebase now that conflicts are all fixed)
git push -f (push the changes to this PR)

[teroman]

Well that first commit didn't work! Hopefully the second is okay, if so I'll rebase my other PR.

[teroman]

Thanks @benmccann for the advice, a bit of ssh key tinkering and I got things working.

[simonbrunel]

It would be great to have unit tests to prevent regression.

[simonbrunel]

I'm not sure that's correct (neither the original formula) because it works only with a line (width or height), not a box (width and height). When angle is not 0, 90, 180, 270, we should take in account both dimensions:

var rot = helpers.toRadians(me.labelRotation);
var cos = Math.abs(Math.cos(rot));
var sin = Math.abs(Math.sin(rot));
var width = me.longestLabelWidth; 
var height = me.options.ticks.fontSize;
var skipSize =  (width * cos) + (height * sin)

// In case we want to support auto-skip on vertical scales
// var skipSize =  (width * sin) + (height * cos)

Also, me.options.ticks.fontSize works only for single line label, should we also support multilines?

[etimberg]

Great diagram @simonbrunel. I agree that this is the correct formula for the width of the text.

[simonbrunel]

My suggested skipSize wasn't optimal, correct formula in this pen, implemented in #5922.

[benmccann]

@teroman would you be able to update this PR to address Simon's comments?

[benmccann]

@teroman I'd really love to see this PR get merged. Will you be updating to address the comments? If not, I may have to close this PR at some point