Fix offset on doughnut charts

[Zivangu9]

Fix https://github.com/chartjs/Chart.js/issues/10465

Adding another half offset can fix the cropped graphics error.

Reproducible sample

I use the examples that doc-code-hub gave in the issue https://stackblitz.com/edit/web-platform-dr6rsv?file=js%2Fdata.js,index.html

[Zivangu9]

Do you think the solution is correct, or should I look for a different one?

[etimberg]

@Zivangu9 sorry, I didn't have internet yesterday

I think I understand where this is coming from. Would it be possible to add a test for the 2 arcs that are each 180 degrees? I'd be curious to see how this performs

[Zivangu9]

I was testing with two 180 degree arcs and now I understand a little better. The center offset is always 1/4, but the radius increment is 1/2 for 180 degree arcs, so when it's 2 180 arcs it's just 1.5 offset. When it is only one arc greater than 180 it becomes 1.25. And if there are no arcs greater than 180, the increment is 1 offset. https://stackblitz.com/edit/web-platform-dr6rsv?file=js%2Fdata.js,index.html

[etimberg]

It almost looks like the angle changes should only apply to arcs that are not 180 degrees exactly.

[kurkle]

There is a jump at the end of the animation, when the arcs reach 180 degree circumference. That is another problem.

I think a proper fix would be getting rid of this kind of condition in the arc element.

[kurkle]

I experimented a little with it. Replaced

https://github.com/chartjs/Chart.js/blob/7432b609c487e1b17c8000fe9f94c58fa65936b8/src/elements/element.arc.js#L340-L342

with

      const fix = 1 + Math.sin(-Math.min(PI, circumference));
      radiusOffset = offset / 2 * fix;

to make the animations smooth for large arcs. @etimberg any thoughts?

https://stackblitz.com/edit/web-platform-ayr13m?file=js%2Fdata.js

[etimberg]

I experimented a little with it. Replaced

https://github.com/chartjs/Chart.js/blob/7432b609c487e1b17c8000fe9f94c58fa65936b8/src/elements/element.arc.js#L340-L342

with

      const fix = 1 + Math.sin(-Math.min(PI, circumference));
      radiusOffset = offset / 2 * fix;

to make the animations smooth for large arcs. @etimberg any thoughts?

https://stackblitz.com/edit/web-platform-ayr13m?file=js%2Fdata.js

The animation in that version looks really good 👍

[Zivangu9]

That seems like a better solution to the problem, do I make the changes in the PR?

[stockiNail]

sorry if I'm jumping in the thread. Issue #10220 was addressing the issue about hover offset (similar of the offset one). I didn't check the PR in deep, but I was thinking if make sense to address hoverOffset option and not only the offset one.

[kurkle]

@stockiNail probably the same issue.

[LeeLenaleee]

Since this can drastically change how some charts are shown I am not sure if this should be considered a bug fix or a breaking change

[etimberg]

Hmm, not sure if this is breaking or not. The previous behaviour was pretty buggy.

[LeeLenaleee]

That is true, but since we want to start working on V4 after 3.9 I think it will be the safer option to postpone it till V4

[etimberg]

Fair, if we aim to get a 3.9 out quickly this won't have to sit around for a while